[Mesa-dev] [PATCH] i965: Prevent creation of aux surface after intel_miptree_make_shareable()

[Ben Widawsky]

On 17-01-04 12:21:36, Chad Versace wrote:
>Pre-patch, if the user created an EGLImage from an intel_mipmap_tree
>before the miptree's first use, then intel_miptree_make_shareable()
>failed to prevent later creation of the miptree's auxilliary surface.
>This results in the original, exported miptree possessing an auxilliary
>surface, but the miptree later created by importing the EGLImage does
>not. The two miptrees then disagree on the primary surface's pixel data.
>
>To fix it, make intel_miptree_make_shareable() *really* disable future creation
>of the auxilliary surface.
>
>Fixes piglit "egl_khr_gl_renderbuffer_image-clear-shared-image GL_RGBA"
>on Skylake. (But the GL_DEPTH_COMPONENT24 test still crashes).
>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99265
>Cc: Ben Widawsky <ben at bwidawsk.net>
>---
>
>Hey Ben, this probably conflicts with your RBC patches, which I haven't
>read yet. How do you think this bug should really get fixed?
>

At least for ksmcube, with the latest codebase, this doesn't break me. As long
as make_shareable is always called before create_image_with_modifiers (the
creator of DRIimage), we're fine.

Is this generally the case based on your knowledge, or are there usages where
make_shareable is called after the DRIimage is created. I see 3 callers of
make_shareable:

intel_setup_image_from_mipmap_tree - don't care for winsys
intel_create_image_from_renderbuffer - don't care, using createImageWithModifiers
intelSetTexBuffer2 - not sure when this is used.

> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>index 25f8f39145..31fb71e196 100644
>--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>@@ -2334,18 +2334,18 @@ intel_miptree_make_shareable(struct brw_context *brw,
>
>    if (mt->mcs_buf) {
>       intel_miptree_all_slices_resolve_color(brw, mt, 0);
>-      mt->aux_disable |= (INTEL_AUX_DISABLE_CCS | INTEL_AUX_DISABLE_MCS);
>       drm_intel_bo_unreference(mt->mcs_buf->bo);
>       free(mt->mcs_buf);
>       mt->mcs_buf = NULL;
>    }
>
>    if (mt->hiz_buf) {
>-      mt->aux_disable |= INTEL_AUX_DISABLE_HIZ;
>       intel_miptree_all_slices_resolve_depth(brw, mt);
>       intel_miptree_hiz_buffer_free(mt->hiz_buf);
>       mt->hiz_buf = NULL;
>    }
>+
>+   mt->aux_disable = INTEL_AUX_DISABLE_ALL;
> }
>
>
>-- 
>2.11.0
>