On Saturday, 2018-05-19 05:32:40 +0200, Mario Kleiner wrote:
> We need to distinguish if the backing storage of a pixmap
> is XRGB2101010 or XBGR2101010, as different gpu hw supports
> different formats. NVidia hw prefers XBGR, whereas AMD and
> Intel are happy with XRGB.
>
> Use the red channel mask of the first depth 30 visual of
> the x-screen to distinguish which hw format to choose.
>
> This fixes desktop composition of color depth 30 windows
> when the X11 compositor uses EGL.
>
> v2: Switch from using the visual of the root window to simply
> using the first depth 30 visual for the x-screen, as testing
> shows that each driver only exports either xrgb ordering or
> xbgr ordering for the channel masks of its depth 30 visuals,
> so this should be unambiguous and avoid trouble if X ever
> supports depth 30 pixmaps on screens with a non-depth 30 root
> window visual. This per Michels suggestion.
>
> v3: No change to v2, but spent some time testing this more on
> AMD hw, with my software hacked up to intentionally choose
> pixel formats/visual with the non-preferred xBGR2101010
> ordering on the ati-ddx, also with a standard non-OpenGL
> X-Window with depth 30 visual, to make sure that things show
> up properly with the right colors on the screen when going
> through EGL+OpenGL based compositing on KDE-5. Iow. to confirm
> that my explanation to the v2 patch on the mailing list of why
> it should work and the actual practice agree (or possibly that
> i am good at fooling myself during testing ;).
>
> Signed-off-by: Mario Kleiner <mario.kleiner...@gmail.com>
> Cc: Michel Dänzer <michel.daen...@amd.com>
> ---
> src/egl/drivers/dri2/egl_dri2.h | 7 +++++++
> src/egl/drivers/dri2/platform_x11.c | 36
> +++++++++++++++++++++++++++++++-
> src/egl/drivers/dri2/platform_x11_dri3.c | 12 +++++++----
> 3 files changed, 50 insertions(+), 5 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
> index adabc527f8..7e7032d989 100644
> --- a/src/egl/drivers/dri2/egl_dri2.h
> +++ b/src/egl/drivers/dri2/egl_dri2.h
> @@ -413,6 +413,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_for_depth(struct dri2_egl_display *dri2_dpy, int
> depth);
> #else
> static inline EGLBoolean
> dri2_initialize_x11(_EGLDriver *drv, _EGLDisplay *disp)
> @@ -421,6 +423,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_for_depth(struct dri2_egl_display *dri2_dpy, int depth)
> +{
> + 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 7aca0a9020..aabae02adb 100644
> --- a/src/egl/drivers/dri2/platform_x11.c
> +++ b/src/egl/drivers/dri2/platform_x11.c
> @@ -209,6 +209,36 @@ get_xcb_screen(xcb_screen_iterator_t iter, int screen)
> return NULL;
> }
>
> +static xcb_visualtype_t *
> +get_xcb_visualtype_for_depth(struct dri2_egl_display *dri2_dpy, int depth)
> +{
> + xcb_visualtype_iterator_t visual_iter;
> + xcb_screen_t *screen = dri2_dpy->screen;
> + xcb_depth_iterator_t depth_iter =
> xcb_screen_allowed_depths_iterator(screen);
> +
> + for (; depth_iter.rem; xcb_depth_next(&depth_iter)) {
> + if (depth_iter.data->depth != depth)
> + continue;
> +
> + visual_iter = xcb_depth_visuals_iterator(depth_iter.data);
> + if (visual_iter.rem)
> + return visual_iter.data;
> + }
> +
> + return NULL;
> +}
> +
> +/* Get red channel mask for given depth. */
> +unsigned int
> +dri2_x11_get_red_mask_for_depth(struct dri2_egl_display *dri2_dpy, int depth)
> +{
> + unsigned int red_mask = 0;
> + xcb_visualtype_t *visual = get_xcb_visualtype_for_depth(dri2_dpy, depth);
> + if (visual)
> + red_mask = visual->red_mask;
> +
> + return red_mask;
Nit: drop the local `red_mask` and just `return visual->red_mask`/`return 0`
Reviewed-by: Eric Engestrom <eric.engest...@intel.com>
> +}
>
> /**
> * Called via eglCreateWindowSurface(), drv->API.CreateWindowSurface().
> @@ -1058,7 +1088,11 @@ dri2_create_image_khr_pixmap(_EGLDisplay *disp,
> _EGLContext *ctx,
> format = __DRI_IMAGE_FORMAT_XRGB8888;
> break;
> case 30:
> - format = __DRI_IMAGE_FORMAT_XRGB2101010;
> + /* Different preferred formats for different hw */
> + if (dri2_x11_get_red_mask_for_depth(dri2_dpy, 30) == 0x3ff)
> + format = __DRI_IMAGE_FORMAT_XBGR2101010;
> + else
> + format = __DRI_IMAGE_FORMAT_XRGB2101010;
> break;
> case 32:
> format = __DRI_IMAGE_FORMAT_ARGB8888;
> diff --git a/src/egl/drivers/dri2/platform_x11_dri3.c
> b/src/egl/drivers/dri2/platform_x11_dri3.c
> index 5cb6d65c0a..9525b20158 100644
> --- a/src/egl/drivers/dri2/platform_x11_dri3.c
> +++ b/src/egl/drivers/dri2/platform_x11_dri3.c
> @@ -40,7 +40,7 @@
> #include "loader_dri3_helper.h"
>
> static uint32_t
> -dri3_format_for_depth(uint32_t depth)
> +dri3_format_for_depth(struct dri2_egl_display *dri2_dpy, uint32_t depth)
> {
> switch (depth) {
> case 16:
> @@ -48,7 +48,11 @@ dri3_format_for_depth(uint32_t depth)
> case 24:
> return __DRI_IMAGE_FORMAT_XRGB8888;
> case 30:
> - return __DRI_IMAGE_FORMAT_XRGB2101010;
> + /* Different preferred formats for different hw */
> + if (dri2_x11_get_red_mask_for_depth(dri2_dpy, 30) == 0x3ff)
> + return __DRI_IMAGE_FORMAT_XBGR2101010;
> + else
> + return __DRI_IMAGE_FORMAT_XRGB2101010;
> case 32:
> return __DRI_IMAGE_FORMAT_ARGB8888;
> default:
> @@ -298,7 +302,7 @@ dri3_create_image_khr_pixmap(_EGLDisplay *disp,
> _EGLContext *ctx,
> return NULL;
> }
>
> - format = dri3_format_for_depth(bp_reply->depth);
> + format = dri3_format_for_depth(dri2_dpy, bp_reply->depth);
> if (format == __DRI_IMAGE_FORMAT_NONE) {
> _eglError(EGL_BAD_PARAMETER,
> "dri3_create_image_khr: unsupported pixmap depth");
> @@ -350,7 +354,7 @@ dri3_create_image_khr_pixmap_from_buffers(_EGLDisplay
> *disp, _EGLContext *ctx,
> return EGL_NO_IMAGE_KHR;
> }
>
> - format = dri3_format_for_depth(bp_reply->depth);
> + format = dri3_format_for_depth(dri2_dpy, bp_reply->depth);
> if (format == __DRI_IMAGE_FORMAT_NONE) {
> _eglError(EGL_BAD_PARAMETER,
> "dri3_create_image_khr: unsupported pixmap depth");
> --
> 2.13.0.rc1.294.g07d810a77f
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
