On Thu, Feb 09, 2017 at 09:41:11AM +0100, Juan A. Suarez Romero wrote:
> On Wed, 2017-02-08 at 10:53 -0800, Nanley Chery wrote:
> > On Wed, Feb 08, 2017 at 06:42:44PM +0100, Juan A. Suarez Romero wrote:
> > > 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 <jasua...@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.
> > >
> >
> > Oh, sorry, I meant code comment, not commit message. That would
> > definitely not work as a commit message. Thoughts?
>
>
> Yes, it works for me as code comment. Thanks!
>
Great!
>
> With that change, can I consider you grant R-b?
>
Yes, with that change, this patch is
Reviewed-by: Nanley Chery <nanley.g.ch...@intel.com>
> J.A.
>
> > >
> > > > 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@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > >
> > > >
> >
> >
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
