[Mesa-dev] [PATCH] i965/blorp/gen8: Fix blitting of interleaved msaa surfaces

[Pohjolainen, Topi]

On Wed, Apr 27, 2016 at 09:17:32AM -0700, Kenneth Graunke wrote:
> On Wednesday, April 27, 2016 3:14:23 PM PDT Topi Pohjolainen wrote:
> > Fixes ES31-CTS.gtf.GL31Tests.texture_stencil8.texture_stencil8_multisample.
> > 
> > Current logic divides given layer of one by number of samples (four)
> > trashing the layer to zero. Layer adjustment is only to be used with
> > non-interleaved msaa surfaces where samples for particular layer are
> > in multiple slices.
> > 
> > I copy-pasted a bit of documentation from
> > brw_blorp.c::brw_blorp_compute_tile_offsets().
> > 
> > Also took the opportunity to fix the comment regarding sampling
> > as 2D, cube textures are the only exception.
> > 
> > CC: Kenneth Graunke <kenneth at whitecape.org>
> > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > ---
> >  src/mesa/drivers/dri/i965/gen8_blorp.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/gen8_blorp.c b/src/mesa/drivers/dri/
> i965/gen8_blorp.c
> > index 43eb4a7..c5aeea4 100644
> > --- a/src/mesa/drivers/dri/i965/gen8_blorp.c
> > +++ b/src/mesa/drivers/dri/i965/gen8_blorp.c
> > @@ -601,14 +601,28 @@ gen8_blorp_emit_surface_states(struct brw_context 
> *brw,
> >        const struct brw_blorp_surface_info *surface = &params->src;
> >        struct intel_mipmap_tree *mt = surface->mt;
> >  
> > -      /* Textures are always sampled as 2D. */
> > +      /* If src is a 2D multisample array texture on Gen7+ using
> > +       * INTEL_MSAA_LAYOUT_UMS or INTEL_MSAA_LAYOUT_CMS, src layer is the
> > +       * physical layer holding sample 0.  So, for example, if mt-
> >num_samples
> > +       * == 4, then logical layer n corresponds to layer == 4*n.
> > +       *
> > +       * Multisampled depth and stencil surfaces have the samples 
> interleaved
> > +       * (INTEL_MSAA_LAYOUT_IMS) and therefore the layer doesn't need
> > +       * adjustment.
> > +       */
> > +      const unsigned layer_divider =
> > +         (mt->msaa_layout == INTEL_MSAA_LAYOUT_UMS ||
> > +          mt->msaa_layout == INTEL_MSAA_LAYOUT_CMS) ?
> > +         MAX2(mt->num_samples, 1) : 1;
> > +
> > +      /* Cube textures are sampled as 2D array. */
> >        const bool is_cube = mt->target == GL_TEXTURE_CUBE_MAP_ARRAY ||
> >                             mt->target == GL_TEXTURE_CUBE_MAP;
> >        const unsigned depth = (is_cube ? 6 : 1) * mt->logical_depth0;
> >        const GLenum target = is_cube ? GL_TEXTURE_2D_ARRAY : mt->target;
> >        const unsigned max_level = surface->level + mt->last_level + 1;
> >        const unsigned layer = mt->target != GL_TEXTURE_3D ?
> > -                                surface->layer / MAX2(mt->num_samples, 1) : 
> 0;
> > +                                surface->layer / layer_divider : 0;
> >  
> >        brw->vtbl.emit_texture_surface_state(brw, mt, target,
> >                                             layer, layer + depth,
> > 
> 
> Topi, you rock!  I spent all day looking for this bug.  Once you see it,
> it becomes obvious :)

Thanks, although it was my mistake to begin with.

> 
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
> 
> It looks like Gen7 is still using tile_x/tile_y to access levels/layers;
> otherwise it would need the equivalent fix, too.

Correct, only gen8/9 use core i965 upload logic for setting texture surfaces
and therefore leverage sampler engine to resolve correct level/layer.