Re: [Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
On Thu, Apr 12, 2018 at 2:25 PM, Mario Kleiner wrote: > On 04/12/2018 07:43 PM, Ilia Mirkin wrote: >> >> On Thu, Apr 12, 2018 at 1:18 PM, Mario Kleiner >> wrote: >>> >>> 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. >> >> >> FWIW the NVIDIA hardware is perfectly happy to render into BGR10_A2. >> Just can't display it. >> >>-ilia >> > > Yes, but due to one of your earlier depth 30 commits we only expose formats > with PIPE_BIND_DISPLAY_TARGET, so that ability is hidden from driver > independent code like the EGL platform code, GLX etc. to prevent exposing > formats that the hw can't display. Yeah. The generic code would have to distinguish between "rendering" and "displaying. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
On 04/12/2018 07:43 PM, Ilia Mirkin wrote: On Thu, Apr 12, 2018 at 1:18 PM, Mario Kleiner wrote: 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. FWIW the NVIDIA hardware is perfectly happy to render into BGR10_A2. Just can't display it. -ilia Yes, but due to one of your earlier depth 30 commits we only expose formats with PIPE_BIND_DISPLAY_TARGET, so that ability is hidden from driver independent code like the EGL platform code, GLX etc. to prevent exposing formats that the hw can't display. -mario ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
On Thu, Apr 12, 2018 at 1:18 PM, Mario Kleiner wrote: > 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. FWIW the NVIDIA hardware is perfectly happy to render into BGR10_A2. Just can't display it. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
On 04/10/2018 06:49 PM, Ilia Mirkin wrote: On Tue, Apr 10, 2018 at 4:42 AM, Michel Dänzer 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 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
On Tue, Apr 10, 2018 at 4:42 AM, Michel Dänzer 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? 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? -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
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? >>> 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. Really, the compositor should explicitly specify the format based on the window visual in this case. But from discussion on IRC, EGL doesn't seem to have a mechanism for that yet. >> 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. Yeah, I misunderstood what the two variants are here, sorry for the noise. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
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 wrote: On 2018-03-27 07:53 PM, Daniel Stone wrote: On 12 March 2018 at 20:45, Mario Kleiner 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 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
Re: [Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
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 >>> wrote: On 2018-03-27 07:53 PM, Daniel Stone wrote: > On 12 March 2018 at 20:45, Mario Kleiner > 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 > > 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. 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. > 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? 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 in
Re: [Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
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 wrote: On 2018-03-27 07:53 PM, Daniel Stone wrote: On 12 March 2018 at 20:45, Mario Kleiner 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 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. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer " 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. 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? Will have to do that later at the machine. But unless i misremember that as well, xdpyinfo always gives me this, if i run at DefaultDepth 24: "number of supported pixmap formats:7 supported pixmap formats: depth 1, bits_per_pixel 1, scanline_pad 32 depth 4, bits_per_pixel 8, scanline_pad 32 depth 8, bits_per_pixel 8, scanline_pad 32 depth 15, bits_per_pixel 16, scanline_pad 32 depth 16, bits_per_pixel 16, scanline_pad 32 depth 24, bits_per_pixel 32, scanline_pad 32 depth 32, bits_per_pixel 32, scanline_pad 32 keycode range:minimum 8, maximum 255 " 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? Iff depth 30 is selected, then the root window has depth 30, and a depth 30 visual. If each driver only exports one channel ordering for depth 30, then the channel ordering of any pixmaps associated drawable should be the same as the one of the root window. Repeat after me: "X11 pixmaps don't have a format." :) They're just bags of bits. I understand that :). Unfortunately it doesn't solve our problem that
Re: [Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
On 2018-04-06 06:18 PM, Mario Kleiner wrote: > On Fri, Apr 6, 2018 at 12:01 PM, Michel Dänzer wrote: >> On 2018-03-27 07:53 PM, Daniel Stone wrote: >>> On 12 March 2018 at 20:45, Mario Kleiner 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 >>> >>> 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? > 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? > Iff depth 30 is selected, then the root window has depth 30, and a depth 30 > visual. If each driver only exports one channel ordering for depth 30, > then the channel ordering of any pixmaps associated drawable should be > the same as the one of the root window. Repeat after me: "X11 pixmaps don't have a format." :) They're just bags of bits. Does __DRI_IMAGE_FORMAT_ARGB work for depth 30 as well, by any chance? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
On Fri, Apr 6, 2018 at 12:01 PM, Michel Dänzer wrote: > On 2018-03-27 07:53 PM, Daniel Stone wrote: >> On 12 March 2018 at 20:45, Mario Kleiner 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 >> >> 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? 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. Iff depth 30 is selected, then the root window has depth 30, and a depth 30 visual. If each driver only exports one channel ordering for depth 30, then the channel ordering of any pixmaps associated drawable should be the same as the one of the root window. Wrong thinking? -mario > > -- > Earthling Michel Dänzer | http://www.amd.com > Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
On 2018-03-27 07:53 PM, Daniel Stone wrote: > On 12 March 2018 at 20:45, Mario Kleiner 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 > > 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. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
Hi Mario, On 12 March 2018 at 20:45, Mario Kleiner 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 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. I worry that this isn't generically 'doing BGR correctly', but instead just detecting how xf86-video-nouveau implements things and compensating for that. I'd be much more comfortable if Michel could review this before it landed. Cheers, Daniel ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] egl/x11: Handle both depth 30 formats for eglCreateImage().
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. Signed-off-by: Mario Kleiner --- src/egl/drivers/dri2/egl_dri2.h | 7 ++ src/egl/drivers/dri2/platform_x11.c | 37 +++- src/egl/drivers/dri2/platform_x11_dri3.c | 7 +- 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h index d36d02c..a399b06 100644 --- a/src/egl/drivers/dri2/egl_dri2.h +++ b/src/egl/drivers/dri2/egl_dri2.h @@ -402,6 +402,8 @@ EGLBoolean dri2_initialize_x11(_EGLDriver *drv, _EGLDisplay *disp); void dri2_teardown_x11(struct dri2_egl_display *dri2_dpy); +unsigned int +dri2_x11_get_red_mask(struct dri2_egl_display *dri2_dpy); #else static inline EGLBoolean dri2_initialize_x11(_EGLDriver *drv, _EGLDisplay *disp) @@ -410,6 +412,11 @@ dri2_initialize_x11(_EGLDriver *drv, _EGLDisplay *disp) } static inline void dri2_teardown_x11(struct dri2_egl_display *dri2_dpy) {} +static inline unsigned int +dri2_x11_get_red_mask(struct dri2_egl_display *dri2_dpy) +{ + return 0; +} #endif #ifdef HAVE_DRM_PLATFORM diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c index 6c287b4..da28981 100644 --- a/src/egl/drivers/dri2/platform_x11.c +++ b/src/egl/drivers/dri2/platform_x11.c @@ -209,6 +209,37 @@ get_xcb_screen(xcb_screen_iterator_t iter, int screen) return NULL; } +static xcb_visualtype_t * +get_xcb_visualtype(struct dri2_egl_display *dri2_dpy) +{ + xcb_visualtype_iterator_t visual_iter; + xcb_screen_t *screen = dri2_dpy->screen; + xcb_visualid_t visual_id = screen->root_visual; + xcb_depth_iterator_t depth_iter = xcb_screen_allowed_depths_iterator(screen); + + for (; depth_iter.rem; xcb_depth_next(&depth_iter)) { + visual_iter = xcb_depth_visuals_iterator(depth_iter.data); + + for (; visual_iter.rem; xcb_visualtype_next(&visual_iter)) { + if (visual_iter.data->visual_id == visual_id) +return visual_iter.data; + } + } + + return NULL; +} + +/* Get red channel mask of the root windows visual for our x-screen */ +unsigned int +dri2_x11_get_red_mask(struct dri2_egl_display *dri2_dpy) +{ + unsigned int red_mask = 0; + xcb_visualtype_t *visual = get_xcb_visualtype(dri2_dpy); + if (visual) + red_mask = visual->red_mask; + + return red_mask; +} /** * Called via eglCreateWindowSurface(), drv->API.CreateWindowSurface(). @@ -1050,7 +1081,11 @@ dri2_create_image_khr_pixmap(_EGLDisplay *disp, _EGLContext *ctx, format = __DRI_IMAGE_FORMAT_XRGB; break; case 30: - format = __DRI_IMAGE_FORMAT_XRGB2101010; + /* Different preferred formats for different hw */ + if (dri2_x11_get_red_mask(dri2_dpy) == 0x3ff) + format = __DRI_IMAGE_FORMAT_XBGR2101010; + else + format = __DRI_IMAGE_FORMAT_XRGB2101010; break; case 32: format = __DRI_IMAGE_FORMAT_ARGB; diff --git a/src/egl/drivers/dri2/platform_x11_dri3.c b/src/egl/drivers/dri2/platform_x11_dri3.c index 2073c59..667c845 100644 --- a/src/egl/drivers/dri2/platform_x11_dri3.c +++ b/src/egl/drivers/dri2/platform_x11_dri3.c @@ -282,7 +282,12 @@ dri3_create_image_khr_pixmap(_EGLDisplay *disp, _EGLContext *ctx, format = __DRI_IMAGE_FORMAT_XRGB; break; case 30: - format = __DRI_IMAGE_FORMAT_XRGB2101010; + /* Different preferred formats for different hw */ + if (dri2_x11_get_red_mask(dri2_dpy) == 0x3ff) + format = __DRI_IMAGE_FORMAT_XBGR2101010; + else + format = __DRI_IMAGE_FORMAT_XRGB2101010; + break; case 32: format = __DRI_IMAGE_FORMAT_ARGB; -- 2.7.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev