[Mesa-dev] [PATCH] loader_dri3: Handle mismatched depth 30 formats for Prime renderoffload.

[Mario Kleiner]

Ping. One more loose patch following the same logic/principle as the
x11+egl patch for nouveau depth 30, that would benefit from a r-b and
ideally make it into Mesa 18.2. to clear out nouveau depth 30 support.

Obsessively tested by now by myself on intel, amd, nouveau and
dri3/present with prime renderoffload for intel+nouveau, amd+nouveau
and nouveau+amd, on native x11 with the patched nouveau-ddx and
server-1.20 modesetting-ddx. It makes many things better, but nothing
worse, as far as my testing goes.

It would be great to get this merged for Mesa 18.2. Can i bribe
somebody into this, in exchange for a beer on xdc2018, assuming i
manage to make it there?

thanks,
-mario

On Thu, Jun 14, 2018 at 6:04 AM, Mario Kleiner
<mario.kleiner.de at gmail.com> wrote:
> Detect if the display (X-Server) gpu and Prime renderoffload gpu prefer
> different channel ordering for color depth 30 formats ([X/A]BGR2101010
> vs. [X/A]RGB2101010) and perform format conversion during the blitImage()
> detiling op from tiled backbuffer -> linear buffer.
>
> For this we need to find the visual (= red channel mask) for the
> X-Drawable used to display on the server gpu. We use the same proven
> logic for finding that visual as in commit "egl/x11: Handle both depth
> 30 formats for eglCreateImage()".
>
> This is mostly to allow "NVidia Optimus" at depth 30, as Intel/AMD
> gpu's prefer xRGB2101010 ordering, whereas NVidia gpu's prefer
> xBGR2101010 ordering, so we can offload to nouveau without getting
> funky colors.
>
> Tested on Intel single gpu, NVidia single gpu, Intel + NVidia prime
> offload with DRI3/Present.
>
> Note: An unintended but pleasant surprise of this patch is that it also
> seems to make the modesetting-ddx of server 1.20.0 work at depth 30
> on nouveau, at least with unredirected "classic" X rendering, and
> with redirected desktop compositing under XRender accel, and with OpenGL
> compositing under GLX. Only X11 compositing via OpenGL + EGL still gives
> funky colors. modesetting-ddx + glamor are not yet ready to deal with
> nouveau's ABGR2101010 format, and treat it as ARGB2101010, also exposing
> X-visuals with ARGB2101010 style channel masks. Seems somehow this triggers
> the logic in this patch on modesetting-ddx + depth 30 + DRI3 buffer sharing
> and does the "wrong" channel swizzling that then cancels out the "wrong"
> swizzling of glamor and we end up with the proper pixel formatting in
> the scanout buffer :). This so far tested on a NVA5 Tesla card under KDE5
> Plasma as shipping with Ubuntu 16.04.4 LTS.
>
> Signed-off-by: Mario Kleiner <mario.kleiner.de at gmail.com>
> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: Eric Engestrom <eric.engestrom at intel.com>
> ---
>  src/loader/loader_dri3_helper.c | 83 ++++++++++++++++++++++++++++++++-
>  src/loader/loader_dri3_helper.h |  1 +
>  2 files changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c
> index f0ff2f07bd..a9a18921ce 100644
> --- a/src/loader/loader_dri3_helper.c
> +++ b/src/loader/loader_dri3_helper.c
> @@ -64,6 +64,55 @@ dri3_flush_present_events(struct loader_dri3_drawable *draw);
>  static struct loader_dri3_buffer *
>  dri3_find_back_alloc(struct loader_dri3_drawable *draw);
>
> +static xcb_screen_t *
> +get_screen_for_root(xcb_connection_t *conn, xcb_window_t root)
> +{
> +   xcb_screen_iterator_t screen_iter =
> +   xcb_setup_roots_iterator(xcb_get_setup(conn));
> +
> +   for (; screen_iter.rem; xcb_screen_next (&screen_iter)) {
> +      if (screen_iter.data->root == root)
> +         return screen_iter.data;
> +   }
> +
> +   return NULL;
> +}
> +
> +static xcb_visualtype_t *
> +get_xcb_visualtype_for_depth(struct loader_dri3_drawable *draw, int depth)
> +{
> +   xcb_visualtype_iterator_t visual_iter;
> +   xcb_screen_t *screen = draw->screen;
> +   xcb_depth_iterator_t depth_iter;
> +
> +   if (!screen)
> +      return NULL;
> +
> +   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 drawable at given depth. */
> +static unsigned int
> +dri3_get_red_mask_for_depth(struct loader_dri3_drawable *draw, int depth)
> +{
> +   xcb_visualtype_t *visual = get_xcb_visualtype_for_depth(draw, depth);
> +
> +   if (visual)
> +      return visual->red_mask;
> +
> +   return 0;
> +}
> +
>  /**
>   * Do we have blit functionality in the image blit extension?
>   *
> @@ -323,6 +372,7 @@ loader_dri3_drawable_init(xcb_connection_t *conn,
>        return 1;
>     }
>
> +   draw->screen = get_screen_for_root(draw->conn, reply->root);
>     draw->width = reply->width;
>     draw->height = reply->height;
>     draw->depth = reply->depth;
> @@ -1030,6 +1080,36 @@ dri3_cpp_for_format(uint32_t format) {
>     }
>  }
>
> +/* Map format of render buffer to corresponding format for the linear_buffer
> + * used for sharing with the display gpu of a Prime setup (== is_different_gpu).
> + * Usually linear_format == format, except for depth >= 30 formats, where
> + * different gpu vendors have different preferences wrt. color channel ordering.
> + */
> +static uint32_t
> +dri3_linear_format_for_format(struct loader_dri3_drawable *draw, uint32_t format)
> +{
> +   switch (format) {
> +      case  __DRI_IMAGE_FORMAT_XRGB2101010:
> +      case  __DRI_IMAGE_FORMAT_XBGR2101010:
> +         /* Different preferred formats for different hw */
> +         if (dri3_get_red_mask_for_depth(draw, 30) == 0x3ff)
> +            return __DRI_IMAGE_FORMAT_XBGR2101010;
> +         else
> +            return __DRI_IMAGE_FORMAT_XRGB2101010;
> +
> +      case  __DRI_IMAGE_FORMAT_ARGB2101010:
> +      case  __DRI_IMAGE_FORMAT_ABGR2101010:
> +         /* Different preferred formats for different hw */
> +         if (dri3_get_red_mask_for_depth(draw, 30) == 0x3ff)
> +            return __DRI_IMAGE_FORMAT_ABGR2101010;
> +         else
> +            return __DRI_IMAGE_FORMAT_ARGB2101010;
> +
> +      default:
> +         return format;
> +   }
> +}
> +
>  /* the DRIimage createImage function takes __DRI_IMAGE_FORMAT codes, while
>   * the createImageFromFds call takes __DRI_IMAGE_FOURCC codes. To avoid
>   * complete confusion, just deal in __DRI_IMAGE_FORMAT codes for now and
> @@ -1227,7 +1307,8 @@ dri3_alloc_render_buffer(struct loader_dri3_drawable *draw, unsigned int format,
>
>        buffer->linear_buffer =
>          draw->ext->image->createImage(draw->dri_screen,
> -                                      width, height, format,
> +                                      width, height,
> +                                      dri3_linear_format_for_format(draw, format),
>                                        __DRI_IMAGE_USE_SHARE |
>                                        __DRI_IMAGE_USE_LINEAR |
>                                        __DRI_IMAGE_USE_BACKBUFFER,
> diff --git a/src/loader/loader_dri3_helper.h b/src/loader/loader_dri3_helper.h
> index 7e3d82947b..705e71f9bf 100644
> --- a/src/loader/loader_dri3_helper.h
> +++ b/src/loader/loader_dri3_helper.h
> @@ -112,6 +112,7 @@ struct loader_dri3_vtable {
>
>  struct loader_dri3_drawable {
>     xcb_connection_t *conn;
> +   xcb_screen_t *screen;
>     __DRIdrawable *dri_drawable;
>     xcb_drawable_t drawable;
>     int width;
> --
> 2.17.1
>