<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Nov 6, 2017 at 1:30 PM, Louis-Francis Ratté-Boulianne <<a href="mailto:lfrb@collabora.com" target="_blank">lfrb@collabora.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Using modifier might allow the driver to use a more optimal format (e.g. tiled/compressed). Let's try to use those if possible. Signed-off-by: Louis-Francis Ratté-Boulianne <<a href="mailto:lfrb@collabora.com">lfrb@collabora.com</a>> --- glamor/glamor_egl.c | 44 ++++++++++++++++++++++++++++++++++++++----- hw/xwayland/xwayland-glamor.c | 22 +++++++++++++++++++--- 2 files changed, 58 insertions(+), 8 deletions(-) diff --git a/glamor/glamor_egl.c b/glamor/glamor_egl.c index bcf36cf9b..fdda56c04 100644 --- a/glamor/glamor_egl.c +++ b/glamor/glamor_egl.c @@ -270,13 +270,47 @@ glamor_make_pixmap_exportable(PixmapPtr pixmap) return FALSE; } - bo = gbm_bo_create(glamor_egl->gbm, width, height, - GBM_FORMAT_ARGB8888, +#ifdef GBM_BO_WITH_MODIFIERS + if (glamor_egl->dmabuf_capable) { + uint32_t num_modifiers; + uint64_t *modifiers = NULL; + int i, j; + + glamor_get_modifiers(screen, DRM_FORMAT_ARGB8888, + &num_modifiers, &modifiers); + + /* Filter out multi-plane modifiers */ + for (i = num_modifiers - 1; i >= 0; i--) { + if (gbm_device_get_format_modifier_plane_count(glamor_egl->gbm, + DRM_FORMAT_ARGB8888, + modifiers[i]) == 1) + continue; + for (j = i + 1; j < num_modifiers; j++) + modifiers[j - 1] = modifiers[j]; + num_modifiers--; + } </blockquote><div> </div><div>I was browsing through your branch today and was confused by this. Why are we filtering out multi-planar formats in glamor? The only time we need to filter out multi-planr formats is when we will be doing front-buffer rendering on a surface that is actively being scanned out. It should be safe to front-buffer render to multi-planar images that are shared with a compositor because it's unlikely that the 3D engine will race with itself. This is causing X to disable compression internally for the temporary pixmaps that get handed off to the compositor which is not what we want. If you did this to fix corruption you may have seen when using a compositor, that would be because we have a nasty texture_from_pixmap bug in mesa related to CCS that I'm currently in the process of fixing. Patches should be incoming before the end of the week. </div><div> </div><div>There are some theoretical ways this could get messed up if the kernel is preempting too aggressively, but I think it will be ok. If we do see a problem, then I think we can do something in mesa where we disable preemption whenever texturing from a pixmap or something like that. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + if (num_modifiers == 0 && modifiers) { + free(modifiers); + modifiers = NULL; + } + + bo = gbm_bo_create_with_modifiers(glamor_egl->gbm, width, height, + GBM_FORMAT_ARGB8888, + modifiers, num_modifiers); + free(modifiers); + } + else +#endif + { + bo = gbm_bo_create(glamor_egl->gbm, width, height, + GBM_FORMAT_ARGB8888, #ifdef GLAMOR_HAS_GBM_LINEAR - (pixmap->usage_hint == CREATE_PIXMAP_USAGE_SHARED ? - GBM_BO_USE_LINEAR : 0) | + (pixmap->usage_hint == CREATE_PIXMAP_USAGE_SHARED ? + GBM_BO_USE_LINEAR : 0) | #endif - GBM_BO_USE_RENDERING | GBM_BO_USE_SCANOUT); + GBM_BO_USE_RENDERING | GBM_BO_USE_SCANOUT); + } + if (!bo) { xf86DrvMsg(scrn->scrnIndex, X_ERROR, "Failed to make %dx%dx%dbpp GBM bo\n", diff --git a/hw/xwayland/xwayland-glamor.c b/hw/xwayland/xwayland-glamor.c index e3a7f6b5b..40772fb9a 100644 --- a/hw/xwayland/xwayland-glamor.c +++ b/hw/xwayland/xwayland-glamor.c @@ -226,14 +226,30 @@ xwl_glamor_create_pixmap(ScreenPtr screen, { struct xwl_screen *xwl_screen = xwl_screen_get(screen); struct gbm_bo *bo; + uint32_t format; if (width > 0 && height > 0 && depth >= 15 && (hint == 0 || hint == CREATE_PIXMAP_USAGE_BACKING_PIXMAP || hint == CREATE_PIXMAP_USAGE_SHARED)) { - bo = gbm_bo_create(xwl_screen->gbm, width, height, - gbm_format_for_depth(depth), - GBM_BO_USE_SCANOUT | GBM_BO_USE_RENDERING); + format = gbm_format_for_depth(depth); + +#ifdef GBM_BO_WITH_MODIFIERS + if (xwl_screen->dmabuf_capable) { + uint32_t num_modifiers; + uint64_t *modifiers = NULL; + + glamor_get_modifiers(screen, format, &num_modifiers, &modifiers); + bo = gbm_bo_create_with_modifiers(xwl_screen->gbm, width, height, + format, modifiers, num_modifiers); + free(modifiers); + } + else +#endif + { + bo = gbm_bo_create(xwl_screen->gbm, width, height, format, + GBM_BO_USE_SCANOUT | GBM_BO_USE_RENDERING); + } if (bo) return xwl_glamor_create_pixmap_for_bo(screen, bo, depth); -- 2.13.0 _______________________________________________ <a href="mailto:xorg-devel@lists.x.org">xorg-devel@lists.x.org</a>: X.Org development Archives: <a href="http://lists.x.org/archives/xorg-devel" rel="noreferrer" target="_blank">http://lists.x.org/archives/xorg-devel</a> Info: <a href="https://lists.x.org/mailman/listinfo/xorg-devel" rel="noreferrer" target="_blank">https://lists.x.org/mailman/listinfo/xorg-devel</a></blockquote></div> </div></div>