[Mesa-dev] [PATCH V2 22/22] i965/gen9: Disable Mip Tail for YF/YS tiled surfaces

[Ben Widawsky]

On Fri, Apr 17, 2015 at 04:51:43PM -0700, Anuj Phogat wrote:
> This fixed the buffer corruption happening in a FBO which use YF/YS
> tiled renderbuffer or texture as color attachment.
> 
> Spec recommends disabling mip tails for non-mip-mapped surfaces.
> But, with this enabled I couldn't get correct data out of YF/YS
> tiled surface. I get the expected data with this disabled.

The docs do say to disable "to disable the Mip Tail this field must be set to a
mip that larger than those present in the surface" So I don't see why you said,
"But". The docs also say you must set this field when trmode isn't None. 

In other words, if you don't want to use mip tails, you are using the correct
mechanism to disable it.

> 
> I haven't spent any time trying to understand miptails. So, I'm
> not sure if this patch is the right thing to do. But, It helps
> move things forward at the moment.

I don't understand them either, but I think your code is fine. Maybe add an
assert that max level is < 15?

So with some explanation in the commit message that this disables mip tails,
this is:
Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

> 
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> ---
>  src/mesa/drivers/dri/i965/brw_defines.h        |  3 +++
>  src/mesa/drivers/dri/i965/gen8_surface_state.c | 10 ++++++++--
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
> index c62c09b..1c50172 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -594,6 +594,9 @@
>  #define GEN9_SURFACE_TRMODE_TILEYF         1
>  #define GEN9_SURFACE_TRMODE_TILEYS         2
>  
> +#define GEN9_SURFACE_MIP_TAIL_START_LOD_SHIFT      8
> +#define GEN9_SURFACE_MIP_TAIL_START_LOD_MASK       INTEL_MASK(11, 8)
> +
>  /* Surface state DW6 */
>  #define GEN7_SURFACE_MCS_ENABLE                 (1 << 0)
>  #define GEN7_SURFACE_MCS_PITCH_SHIFT            3
> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> index 189f1db..155563f 100644
> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
> @@ -258,8 +258,11 @@ gen8_update_texture_surface(struct gl_context *ctx,
>                         GEN7_SURFACE_MIN_LOD) |
>               (intelObj->_MaxLevel - tObj->BaseLevel); /* mip count */
>  
> -   if (brw->gen >= 9)
> +   if (brw->gen >= 9) {
>        surf[5] |= SET_FIELD(tr_mode, GEN9_SURFACE_TRMODE);
> +      /* Disable Mip Tail by setting a large value. */
> +      surf[5] |= SET_FIELD(15, GEN9_SURFACE_MIP_TAIL_START_LOD);
> +   }
>  
>     if (aux_mt) {
>        surf[6] = SET_FIELD(mt->qpitch / 4, GEN8_SURFACE_AUX_QPITCH) |
> @@ -446,8 +449,11 @@ gen8_update_renderbuffer_surface(struct brw_context *brw,
>  
>     surf[5] = irb->mt_level - irb->mt->first_level;
>  
> -   if (brw->gen >= 9)
> +   if (brw->gen >= 9) {
>        surf[5] |= SET_FIELD(tr_mode, GEN9_SURFACE_TRMODE);
> +      /* Disable Mip Tail by setting a large value. */
> +      surf[5] |= SET_FIELD(15, GEN9_SURFACE_MIP_TAIL_START_LOD);
> +   }
>  
>     if (aux_mt) {
>        surf[6] = SET_FIELD(mt->qpitch / 4, GEN8_SURFACE_AUX_QPITCH) |
> -- 
> 2.3.4
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
Ben Widawsky, Intel Open Source Technology Center