[Mesa-dev] [PATCH] isl: Add assertions for render target swizzle restrictions

[Nanley Chery]

On Thu, Feb 02, 2017 at 12:28:02PM -0800, Jason Ekstrand wrote:
> On Thu, Feb 2, 2017 at 11:09 AM, Nanley Chery <nanleychery at gmail.com> wrote:
> 
> > On Tue, Jan 31, 2017 at 06:51:12PM -0800, Jason Ekstrand wrote:
> > > ---
> > >  src/intel/isl/isl_surface_state.c | 32 ++++++++++++++++++++++++++++++++
> > >  1 file changed, 32 insertions(+)
> > >
> > > diff --git a/src/intel/isl/isl_surface_state.c
> > b/src/intel/isl/isl_surface_state.c
> > > index b735478..c7b220b 100644
> > > --- a/src/intel/isl/isl_surface_state.c
> > > +++ b/src/intel/isl/isl_surface_state.c
> > > @@ -453,6 +453,38 @@ isl_genX(surf_fill_state_s)(const struct
> > isl_device *dev, void *state,
> > >  #endif
> > >
> > >  #if (GEN_GEN >= 8 || GEN_IS_HASWELL)
> > > +   if (info->view->usage & ISL_SURF_USAGE_RENDER_TARGET_BIT) {
> >
> > It seems that haswell actually allows this, though I don't know if it's
> > a feature we'd ever want to use.
> >
> > From the Haswell PRM, RENDER_SURFACE_STATE::Shader Channel Select R
> >
> >    If more than one shader channel select is set to the same
> >    surface channel only the first shader channel in RGBA order will
> >    be written.
> >
> 
> Neat!  I didn't realize it was actually well-defined on haswell.  Probably
> not something we're likely to use though, especially given that broadwell+
> don't give such nice guarantees.
> 

If you don't mind, could you update the comment below with this
information? Although I can't think of any, I'd imagine it would be
useful if we have to implement some workaround that relies on this
behavior. Either way, this patch is

Reviewed-by: Nanley Chery <nanley.g.chery at intel.com>

> 
> > -Nanley
> >
> > > +      /* From the Sky Lake PRM Vol. 2d,
> > > +       * RENDER_SURFACE_STATE::Shader Channel Select Red
> > > +       *
> > > +       *    "For Render Target, Red, Green and Blue Shader Channel
> > Selects
> > > +       *    MUST be such that only valid components can be swapped i.e.
> > only
> > > +       *    change the order of components in the pixel. Any other
> > values for
> > > +       *    these Shader Channel Select fields are not valid for Render
> > > +       *    Targets. This also means that there MUST not be multiple
> > shader
> > > +       *    channels mapped to the same RT channel."
> > > +       */
> > > +      assert(info->view->swizzle.r == ISL_CHANNEL_SELECT_RED ||
> > > +             info->view->swizzle.r == ISL_CHANNEL_SELECT_GREEN ||
> > > +             info->view->swizzle.r == ISL_CHANNEL_SELECT_BLUE);
> > > +      assert(info->view->swizzle.g == ISL_CHANNEL_SELECT_RED ||
> > > +             info->view->swizzle.g == ISL_CHANNEL_SELECT_GREEN ||
> > > +             info->view->swizzle.g == ISL_CHANNEL_SELECT_BLUE);
> > > +      assert(info->view->swizzle.b == ISL_CHANNEL_SELECT_RED ||
> > > +             info->view->swizzle.b == ISL_CHANNEL_SELECT_GREEN ||
> > > +             info->view->swizzle.b == ISL_CHANNEL_SELECT_BLUE);
> > > +      assert(info->view->swizzle.r != info->view->swizzle.g);
> > > +      assert(info->view->swizzle.r != info->view->swizzle.b);
> > > +      assert(info->view->swizzle.g != info->view->swizzle.b);
> > > +
> > > +      /* From the Sky Lake PRM Vol. 2d,
> > > +       * RENDER_SURFACE_STATE::Shader Channel Select Alpha
> > > +       *
> > > +       *    "For Render Target, this field MUST be programmed to
> > > +       *    value = SCS_ALPHA."
> > > +       */
> > > +      assert(info->view->swizzle.a == ISL_CHANNEL_SELECT_ALPHA);
> > > +   }
> > >     s.ShaderChannelSelectRed = info->view->swizzle.r;
> > >     s.ShaderChannelSelectGreen = info->view->swizzle.g;
> > >     s.ShaderChannelSelectBlue = info->view->swizzle.b;
> > > --
> > > 2.5.0.400.gff86faf
> > >
> > > _______________________________________________
> > > mesa-dev mailing list
> > > mesa-dev at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >