[Mesa-dev] [PATCH 1/2] anv: allow blue in alpha component in swizzle for render

[Juan A. Suarez Romero]

On Wed, 2017-02-08 at 09:27 -0800, Nanley Chery wrote:
> On Wed, Feb 08, 2017 at 01:31:54PM +0100, Juan A. Suarez Romero wrote:
> > In pre-Broadwell devices, as B4G4R4A4 is not supported natively, we
> > workaround it by using a format with a more complex swizzle, that uses
> > blue in alpha component.
> > 
> > Signed-off-by: Juan A. Suarez Romero <jasuarez at igalia.com>
> > ---
> >  src/intel/vulkan/anv_private.h | 14 ++++++++++----
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
> > index 51e85c7..e9bf13c 100644
> > --- a/src/intel/vulkan/anv_private.h
> > +++ b/src/intel/vulkan/anv_private.h
> > @@ -1559,13 +1559,19 @@ static inline struct isl_swizzle
> >  anv_swizzle_for_render(struct isl_swizzle swizzle)
> >  {
> >     /* Sometimes the swizzle will have alpha map to one.  We do this to fake
> > -    * RGB as RGBA for texturing
> > +    * RGB as RGBA for texturing.
> > +    *
> > +    * It can have also alpha to blue. This happens to workaround support for
> > +    * B4G4R4A4 in gen < 8 devices, as this not supported natively.
> >      */
> >     assert(swizzle.a == ISL_CHANNEL_SELECT_ONE ||
> > -          swizzle.a == ISL_CHANNEL_SELECT_ALPHA);
> > +          swizzle.a == ISL_CHANNEL_SELECT_ALPHA ||
> > +          swizzle.a == ISL_CHANNEL_SELECT_BLUE);
> >  
> > -   /* But it doesn't matter what we render to that channel */
> > -   swizzle.a = ISL_CHANNEL_SELECT_ALPHA;
> > +   /* But it doesn't matter what we render to that channel, except for the
> > +    * B4G4R4A4 workaround */
> 
> I'm finding this commit message a tad confusing. Would you agree that 
> replacing it with the following would be better?
> 
> For RGB formats that have alpha mapped to one, we must remap the channel
> to alpha to satisfy the BDW+ surface state restriction.
> 

Uhm... I don't think so. That is what the code was doing so far.

The commit just takes in account that alpha can be mapped to blue too.
And in this case, we keep the blue.


> Aside from the comment, this patch looks good to me.
> 
> > +   if (swizzle.a == ISL_CHANNEL_SELECT_ONE)
> > +      swizzle.a = ISL_CHANNEL_SELECT_ALPHA;
> >  
> >     return swizzle;
> >  }
> > -- 
> > 2.9.3
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
>