change the try order of DRI2 and DRI3

[XinfengZhang]

try DRI2 firstly, if suecess, it could detect driver name , also could use vaPutSurfaces.
if failed , try DRI3, it only support driver name detection could not use vaPutSurfaces.

if there are both DRI2 and DRI3
it could ensure vaPutSurfaces work

[ceyusa]

i guess it should be a way to force dri3 if both available.

[XinfengZhang]

i guess it should be a way to force dri3 if both available.

it is a good idea, but why you prefer to use dri3 instead of dri2 in this case?
if both them could retrieve the driver name , and vaPutSurfaces only support DRI2

[evelikov]

The code-flow - try DRI3, unless explicitly disabled, then go for DRI2 - has been in use in Mesa for 5+ years. Changing it is counter intuitive, but more importantly it will break use-cases where you want to use DRI3 and both DRI2 and DRI3 are available.

As mentioned before - the Intel driver needs to be fixed. Be that by copying (+ crediting) the mesa VA/winsys code or writing one from scratch.

[evelikov]

If fixing the Intel drivers is not an option for the short/mid term, another option is to explicitly fallback to DRI2 on those platforms.

I'm getting quite tempted at writing a PR for that actually. Community has been asking for DRI3 for 5+ years and even after rehashing that one year ago (when the DRI3 PR was opened), there is no action taken by Intel ☹️

[XinfengZhang]

which use- case is blocked? you know libva just use it to retrieve a library name, and draw a surface to display.
so, your assumption is: there are one backend driver, which just implement vaPutSurfaces basing on DRI3? right?

[pbiering]

For 2.18 this patch needs to be modified to

--- a/va/x11/va_x11.c.orig	2023-07-05 07:21:52.368356380 +0200
+++ b/va/x11/va_x11.c	2023-07-05 07:23:12.662381487 +0200
@@ -166,10 +166,9 @@
     else
         return VA_STATUS_ERROR_UNKNOWN;
 
-    if (!getenv("LIBVA_DRI3_DISABLE"))
+    vaStatus = va_DRI2_GetDriverName(pDisplayContext, driver_name, candidate_index);
+    if (!getenv("LIBVA_DRI3_DISABLE") && (vaStatus != VA_STATUS_SUCCESS))
         vaStatus = va_DRI3_GetDriverName(pDisplayContext, driver_name, candidate_index);
-    if (vaStatus != VA_STATUS_SUCCESS)
-        vaStatus = va_DRI2_GetDriverName(pDisplayContext, driver_name, candidate_index);
 #ifdef HAVE_NVCTRL
     if (vaStatus != VA_STATUS_SUCCESS)
         vaStatus = va_NVCTRL_GetDriverName(pDisplayContext, driver_name, candidate_index);

[evelikov]

Something like #727 would be a better fix.

It will not affect other drivers and i965/iHD will work as before. There is zero reason to penalise others, if i965/iHD has no time/interest/other in updating.

[XinfengZhang]

per: #677 (comment)
@evelikov , I still insist this patch is more suitable for current usage coverage.
just like mentioned in #677 (comment)
this PR could meet the request .

[evelikov]

Nearly(?) all the drivers out there but the Intel ones support DRI3. In some cases it's the required mode of operation.

Merging this will cause regressions and in some cases quite subtle ones.

Limiting everyone due to the feature set (or lack there of) of the Intel driver(s), does not seem practical and nor friendly to the ecosystem as a whole.

The earlier proposal #727 keeps DRI2 for Intel drivers and DRI3 for everyone else. Preserving the correct functionality for everyone.

Speaking of which - the PR has been opened for 5 months now - so happy anniversary. I would kindly repeat my earlier request - can we get timely reviews and feedback on MRs? Currently one has to borderline beg to get anything merged.

Thanks

[XinfengZhang]

Nearly(?) all the drivers out there but the Intel ones support DRI3. In some cases it's the required mode of operation.

Merging this will cause regressions and in some cases quite subtle ones.

this part is confusing, what kind of regressions? which driver and usage cases?

Limiting everyone due to the feature set (or lack there of) of the Intel driver(s), does not seem practical and nor friendly to the ecosystem as a whole.

The earlier proposal #727 keeps DRI2 for Intel drivers and DRI3 for everyone else. Preserving the correct functionality for everyone.

Speaking of which - the PR has been opened for 5 months now - so happy anniversary. I would kindly repeat my earlier request - can we get timely reviews and feedback on MRs? Currently one has to borderline beg to get anything merged.

the discussion is very disrupting , to respect your opinion, I did not merge this PR directly, to avoid direct offend ,
I asked which cases is blocked #716 (comment)
you did not convinced me , and open another PR. I also expressed my concern without reply

#727 (comment)

at least , you should let me know which case , who are blocked. if there are no example, I will merge it.

thanks

[XinfengZhang]

Nearly(?) all the drivers out there but the Intel ones support DRI3. In some cases it's the required mode of operation.
Merging this will cause regressions and in some cases quite subtle ones.

this part is confusing, what kind of regressions? which driver and usage cases?

Limiting everyone due to the feature set (or lack there of) of the Intel driver(s), does not seem practical and nor friendly to the ecosystem as a whole.
The earlier proposal #727 keeps DRI2 for Intel drivers and DRI3 for everyone else. Preserving the correct functionality for everyone.
Speaking of which - the PR has been opened for 5 months now - so happy anniversary. I would kindly repeat my earlier request - can we get timely reviews and feedback on MRs? Currently one has to borderline beg to get anything merged.

this is unfair , at least for this PR, the discussion is very disrupting , to respect your opinion, I did not merge this PR directly, to avoid direct offend , I asked which cases is blocked #716 (comment) you did not convince me , and open another PR. I also expressed my concern, still no reply

#727 (comment)

at least , you should let me know which case , who are blocked. if there are no example, I will merge it.

thanks