[Mesa-dev] [PATCH 08/64] i965/blorp: Only set src_z for gen8+ 3D textures

[Pohjolainen, Topi]

On Tue, Jun 21, 2016 at 03:51:15PM +0300, Pohjolainen, Topi wrote:
> On Mon, Jun 20, 2016 at 08:52:50PM -0700, Kenneth Graunke wrote:
> > On Friday, June 17, 2016 4:55:41 PM PDT Jason Ekstrand wrote:
> > > On Thu, Jun 16, 2016 at 10:08 AM, Chad Versace <chad.versace at intel.com>
> > > wrote:
> > > 
> > > > On Sat 11 Jun 2016, Jason Ekstrand wrote:
> > > > > Otherwise, we end up with a bogus value in the third component.  On
> > > > gen6-7
> > > > > where we always use 2D textures, this can cause problems if the
> > > > > SurfaceArray bit is set in the SURFACE_STATE.
> > > >
> > > > Enlighten me. Why does blorp use 3D surfaces on gen >= 8 but not
> > > > earlier?
> 
> Related (but not answering the question):
> on 9e4d19372b1d7f6ab12ddab1929a53335d9cce06:
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h b/src/mesa/drivers/dri/i965/brw_blorp.h
> index 7ebcee2..a1aaa20 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.h
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
> @@ -196,8 +196,14 @@ struct brw_blorp_wm_push_constants
>     float rect_grid_y1;
>     brw_blorp_coord_transform_params x_transform;
>     brw_blorp_coord_transform_params y_transform;
> +
> +   /* Minimum layer setting works for all the textures types but texture_3d
> +    * for which the setting has no effect. Use the z-coordinate instead.
> +    */
> +   uint32_t src_z;
> +
> 
> The setting actually works on SKL+ if I remember correctly but as I needed
> something for gen8 I thought using the mechanism for both instead of adding
> yet another path.
> 
> > > >
> > > 
> > > History?  TBH, I'm not really sure.  Probably because SKL 3-D is different
> > > but you'd have to ask topi to be sure.
> > 
> > Historically, we did everything via tile x/y offsets, and so BLORP
> > worked that way too.  However, we never applied any offsets to the
> > auxiliary surfaces (i.e. CMS).  This seems fairly sketchy, and got
> > us into trouble when we ported it to Gen8.  As a hack, falling back
> > from CMS to UMS made things work out.
> > 
> > Rather than try and figure out proper offset calculations for CCS_D,
> > CCS_E, and HiZ, Topi decided to just access everything using level/layer
> > like we do for normal GL.  It seemed a lot safer, and worked out well.
> > 
> > Gen6 never uses CMS, so it works out.  Topi seemed to think that Gen7
> > can't do CMS for array textures (though I can't seem to find that code
> > to confirm), so it might work out.  I'm still not sure why Gen7 works.
> 
> I need to dig some more for this, I can't remember either.

I started wonder if we actually have arrayed compressed multisampled blits
broken an all gen7+. While gen8+ blorp uses proper hw support for texture
surfaces it still does the old manual offsetting for render surface (as
gen7+ blorp). And as Ken mentioned, there is no logic for offsetting the mcs
, and therefore the blits shouldn't work at all. We have plenty of tests for
msaa blits (color, stencil, depth) but I don't think any of them actually use
arrayed textures.

> 
> > 
> > Honestly, I think it'd be better to convert Gen6-7 to be like Gen8+.
> 
> 
> 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev