[Mesa-dev] [PATCH 3/4] intel/isl: Refactor gen6_choose_image_alignment_el

[Jason Ekstrand]

On Thu, May 11, 2017 at 7:03 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:

> On Wed, May 10, 2017 at 02:30:31PM -0700, Jason Ekstrand wrote:
> > ---
> >  src/intel/isl/isl_gen6.c | 30 ++++++++++++------------------
> >  1 file changed, 12 insertions(+), 18 deletions(-)
> >
> > diff --git a/src/intel/isl/isl_gen6.c b/src/intel/isl/isl_gen6.c
> > index b746903..0de9620 100644
> > --- a/src/intel/isl/isl_gen6.c
> > +++ b/src/intel/isl/isl_gen6.c
> > @@ -88,6 +88,8 @@ isl_gen6_choose_image_alignment_el(const struct
> isl_device *dev,
> >      *    | format                 | halign | valign |
> >      *    +------------------------+--------+--------+
> >      *    | YUV 4:2:2 formats      |      4 |      * |
> > +    *    | BC1-5                  |      4 |      4 |
> > +    *    | FXT1                   |      8 |      4 |
> >      *    | uncompressed formats   |      4 |      * |
> >      *    +------------------------+--------+--------+
> >      *
> > @@ -110,29 +112,13 @@ isl_gen6_choose_image_alignment_el(const struct
> isl_device *dev,
> >      */
> >
> >     if (isl_format_is_compressed(info->format)) {
> > +      /* Compressed formats have an alignment equal to their block size
> */
> >        *image_align_el = isl_extent3d(1, 1, 1);
> >        return;
> >     }
> >
> > -   if (isl_format_is_yuv(info->format)) {
> > -      *image_align_el = isl_extent3d(4, 2, 1);
> > -      return;
> > -   }
> > -
> > -   if (info->samples > 1) {
> > -      *image_align_el = isl_extent3d(4, 4, 1);
> > -      return;
> > -   }
> > -
> > -   if (isl_surf_usage_is_depth_or_stencil(info->usage) &&
> > -       !ISL_DEV_USE_SEPARATE_STENCIL(dev)) {
>
> Maybe mention in the commit that we drop this as it is always false on
> gen6+?
> In isl.c: "dev->use_separate_stencil = ISL_DEV_GEN(dev) >= 6;"
>

No, I dropped it not because we always use separate stencil but because
it's redundant with the regular depth case.  The PRM says to use a 4x4
alignment for all depth buffers but 4x2 for separate stencil.  Combined
depth-stencil falls under the "depth" case so I didn't think we needed to
call it out explicitly.


> Other than that:
>
> Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>
> > -      /* interleaved depthstencil buffer */
> > -      *image_align_el = isl_extent3d(4, 4, 1);
> > -      return;
> > -   }
> > -
> >     if (isl_surf_usage_is_depth(info->usage)) {
> > -      /* separate depth buffer */
> > +      /* depth buffer (possibly interleaved with stencil) */
> >        *image_align_el = isl_extent3d(4, 4, 1);
> >        return;
> >     }
> > @@ -143,5 +129,13 @@ isl_gen6_choose_image_alignment_el(const struct
> isl_device *dev,
> >        return;
> >     }
> >
> > +   if (info->samples > 1) {
> > +      *image_align_el = isl_extent3d(4, 4, 1);
> > +      return;
> > +   }
> > +
> > +   /* For everything else, we can chose any vertical alignment we
> want.  We
> > +    * choose an alignment of 2 because it uses the least memory.
> > +    */
> >     *image_align_el = isl_extent3d(4, 2, 1);
> >  }
> > --
> > 2.5.0.400.gff86faf
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170511/0db8a54c/attachment-0001.html>