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

Tue Apr 10 08:22:36 UTC 2018

On 04/09/2018 12:12 PM, Michel Dänzer wrote:
> On 2018-04-06 08:56 PM, Mario Kleiner wrote:
>> On 04/06/2018 06:41 PM, Michel Dänzer wrote:
>>> On 2018-04-06 06:18 PM, Mario Kleiner wrote:
>>>> On Fri, Apr 6, 2018 at 12:01 PM, Michel Dänzer <michel at daenzer.net>
>>>> wrote:
>>>>> On 2018-03-27 07:53 PM, Daniel Stone wrote:
>>>>>> On 12 March 2018 at 20:45, Mario Kleiner
>>>>>> <mario.kleiner.de at gmail.com> wrote:
>>>>>>> We need to distinguish if a backing pixmap of a window is
>>>>>>> XRGB2101010 or XBGR2101010, as different gpu hw supports
>>>>>>> different formats. NVidia hw prefers XBGR, whereas AMD and
>>>>>>> Intel are happy with XRGB.
>>>>>>>
>>>>>>> We use the red channel mask of the visual to distinguish at
>>>>>>> depth 30, but because we can't easily get the associated
>>>>>>> visual of a Pixmap, we use the visual of the x-screens root
>>>>>>> window instead as a proxy.
>>>>>>>
>>>>>>> This fixes desktop composition of color depth 30 windows
>>>>>>> when the X11 compositor uses EGL.
>>>>>>
>>>>>> I have no reason to doubt your testing, so this patch is:
>>>>>> Acked-by: Daniel Stone <daniels at collabora.com>
>>>>>>
>>>>>> But it does rather fill me with trepidation, given that X11 Pixmaps
>>>>>> are supposed to be a dumb 'bag of bits', doing nothing else than
>>>>>> providing the same number and size of channels to the actual client
>>>>>> data for the Visual associated with the Window.
>>>>>
>>>>> As far as X11 is concerned, the number of channels and their sizes
>>>>> don't
>>>>> even matter; a pixmap is simply a container for an unsigned integer
>>>>> of n
>>>>> bits (where n is the pixmap depth) per pixel, with no inherent meaning
>>>>> attached to those values.
>>>>>
>>>>> That said, I'm not sure this is true for EGL as well. But even if it
>>>>> isn't, there would have to be another mechanism to determine the
>>>>> format,
>>>>> e.g. a config associated with the EGL pixmap. The pixmap doesn't even
>>>>> necessarily have the same depth as the root window, so using the
>>>>> latter's visual doesn't make much sense.
>>>>
>>>> Hi Michel. I thought with this patch i was implementing what you
>>>> proposed earlier as a heuristic on how to get around the "pixmaps
>>>> don't have an inherent format, only a depth" problem?
>>>
>>> Do you have a pointer to that discussion?
>>
>> Ok, apologies, i think i was just taking your comment too far as an
>> inspiration. The best i can find in my inbox atm. is this message of
>> yours from 24th November 2017 10:44 AM in a mesa-dev thread "Re:
>> [Mesa-dev] 10-bit Mesa/Gallium support":
>>
>> "Apologies for the badly formatted followup before, let's try that again:
>>
>> On 2017-11-23 07:31 PM, Mario Kleiner wrote:
>>>
>>> 3. In principle the clean solution for nouveau would be to upgrade the
>>> ddx to drmAddFB2 ioctl, and use xbgr2101010 scanout to support
>>> everything back to nv50+, but everything we have in X or Wayland is
>>> meant for xrgb2101010 not xbgr2101010. And we run into ambiguities of
>>> what, e.g., a depth 30 pixmap means in some extensions like
>>> glx_texture_form_pixmap.
>>
>> A pixmap itself never has a format per se, it's just a container for an
>> n-bit integer value per pixel (where n is the pixmap depth). A
>> compositor using GLX_EXT_texture_from_pixmap has to determine the format
>> from the corresponding window's visual.
>> "
>>
>> There's nothing in there that suggests my root window solution.
>> I guess i thought given that we can not get the visual of the window corresponding to the pixmap, let's find some window which is a good enough proxy for onscreen windows with associated depth 30 pixmaps on the same x-screen.
> 
> A pixmap isn't necessarily associated with any window.
> 
> 
>>>> My (possibly inaccurate) understanding is that one can only create a
>>>> depth 30 pixmap if the x-screen runs at depth >= 30. It only exposes
>>>> depth 30 as supported pixmap format (xdpyinfo) if xorg.conf
>>>> DefaultDepth 30 is selected, whereas other depths like
>>>> 1,4,8,15,16,24,32 are always supported at default depth 24.
>>>
>>> That sounds like an X server issue. Just like 32, there's no fundamental
>>> reason a pixmap couldn't have depth 30 despite the screen depth being lower.
>>>
>>> Out of curiosity, can you share the output of xdpyinfo with nouveau at
>>> depth 30?
> 
> [...]
> 
>> At least i don't remember seeing any "depth 30, ..." line ever on any driver+gpu combo if i run X at default depth 24?
> 
> I'm not questioning that's currently the case, I'm saying there's no
> particular reason for it, so expect it to change at some point.
> 

Ah ok, thanks for the explanation.

> 
> 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.

> 
>> The basic problem with EGL based compositing is that for eglCreateImageKHR() all we have is the EGLDisplay and EGLContext used for importing an image resource.
> 
> Is there no EGLConfig associated somehow?

I guess we could get the EGLConfig from the context, but i assume this 
context of the importing application (e.g., typically x11 compositor) 
could have an EGLConfig possibly unrelated to depth 30 pixmaps.

> 
> 
> P.S. IME nouveau is in for a world of pain in general with a format
> which doesn't start at bit 0. Once upon a time, I explored this approach
> for depth 24 on big-endian hosts, but ran into lots of issues both in
> xserver and on the client side.
> 

Can you clarify for me what you mean with "doesn't start at bit 0"? The 
position of red and blue channel is swapped in nouveau's depth 30 
format, but the red channel fills the 10 LSBs. So if there are padding 
bits, they are the 2 MSBs.

thanks,
-mario