[Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().

[Mario Kleiner]

On 04/10/2018 06:49 PM, Ilia Mirkin wrote:
> On Tue, Apr 10, 2018 at 4:42 AM, Michel Dänzer <michel at daenzer.net> wrote:
>> On 2018-04-10 10:22 AM, Mario Kleiner wrote:
>>> On 04/09/2018 12:12 PM, Michel Dänzer wrote:
>>>> On 2018-04-06 08:56 PM, Mario Kleiner wrote:
>>>>
>>>> I'm interested in the full xdpyinfo *at screen depth 30*, in particular
>>>> whether it lists only one variant of depth 30 visuals. If so, one
>>>> possibility for a kludge would be to just look at any depth 30 visual.
>>>
>>> Ok, the fresh v2 patch implements that kludge. This one retested to work
>>> on nouveau, ati, intel.
>>>
>>> On intel and nouveau we only get one channel mask for depth 30 visuals
>>> in xdpyinfo. On amd we get both masks for xrgb2101010 and xbgr2101010,
>>> as the amd gallium drivers expose both formats, but the ordering is
>>> xrgb2101010 first, so that's fine when picking the first depth 30 visual
>>> to get the channel mask for decisions.
>>
>> Hmm, that sounds fragile though when there are both variants; is there
>> any guarantee they can't appear in the opposite order?

Afaics the rough order is determined by how the state tracker builds the 
list of __DRIconfig's in

mesa/src/gallium/state_trackers/dri/dri_screen.c:dri_fill_in_modes(),

ie. the ordering in the mesa_formats[] table at the top of that 
function. MESA_FORMAT_B10G10R10A2_UNORM and 
MESA_FORMAT_B10G10R10X2_UNORM are before MESA_FORMAT_R10G10B10A2_UNORM 
and MESA_FORMAT_R10G10B10X2_UNORM, and that is a requirement of the 
implementation that BGR[A/X] always comes before RGB[A/X].

The server processes those in the order they were generated when 
building visuals: xserver/glx/glxscreens.c:__glXScreenInit() converts 
pGlxScreen->fbconfigs retrieved from the Mesa driver into visuals. That 
fbconfigs list is always traversed in forward order.

So this should hopefully guarantee that for a given depth we always get 
the bgr10 formats we want before the rgb10 formats, and the order stays 
the way we want, with the desired bgr10 format as first depth 30 format.

That said, looking at line 388 of xserver/glx/glxscreens.c, i wonder if 
that check "if (depth == pScreen->visuals[i].nplanes)" is actually 
sufficient? Maybe we should check for compatible channel masks there and 
reject fbconfigs which don't find an existing X visual with matching 
channel mask? We do check the channel mask in pickFBConfig() when 
choosing FBConfigs for existing X Visuals in pass 1, but not when adding 
new X Visuals for FBConfigs that don't have existing X Visuals in pass 
2. That might get rid of the ambiguity and prevent exposing visuals that 
the ddx/kms driver won't be able display properly?

> 
> It seems like nouveau is stirring a bit of a hornet's nest here.
> Unfortunately there's not a whole lot I can do about hw scanout format
> support (rgb10x2 only, no bgr10x2 support until Kepler), but is there
> something else that the DDX and/or mesa driver should be doing to
> avoid some of this pain?
> 
> Should we get the *other* ddx's to avoid exposing both masks?

I think the ddx's only expose one mask per ddx, and seing both might be 
a X-Server problem (see above)? Maybe it could be beneficial if we 
stopped the amd gallium drivers from marking the R10G10B10[X/A]2 configs 
as PIPE_BIND_DISPLAY_TARGET if there isn't a use case for which they do 
that, apart from the hw/driver being capable of supporting it? Intel 
only supports bgrx ordering, nouveau only rgbx, amd both.

I have some half-finished wayland egl patches where it might help for 
the multi-gpu prime renderoffload case to feed wl_buffers in a format 
optimal for display on the server gpu instead of feeding them in a 
format that can be displayed by the server gpu but requires an extra 
copy. Not sure yet, half finished/half tested stuff, may not work out at 
all in the end.

X11 Prime renderoffload is another unsolved problem with nouveau depth 
30. Currently we get swapped red-blue with intel + nvidia. We could 
extend the buffer creation code to convert nouveau's rgba format into 
the bgra format wanted by intel or amd display gpu's during the 
blitImage call for converting the tiled renderbuffer to the linear 
buffer, like i try for the wayland+egl case. But we'd need to know what 
the display gpu wants. Haven't looked into that.

-mario

> 
>    -ilia
>