[Mesa-dev] [PATCH 05/17] intel/isl: Aling non-tiled horizontally by cache line

[Jason Ekstrand]

On Fri, Jul 21, 2017 at 11:29 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:

> On Fri, Jul 21, 2017 at 11:09:47AM -0700, Jason Ekstrand wrote:
> > s/Aling/Align/
> >
> > On Fri, Jul 21, 2017 at 8:00 AM, Topi Pohjolainen <
> > topi.pohjolainen at gmail.com> wrote:
> >
> > > in order to support blit engine.
> > >
> > > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > > ---
> > >  src/intel/isl/isl.c | 16 +++++++++++++++-
> > >  1 file changed, 15 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/intel/isl/isl.c b/src/intel/isl/isl.c
> > > index 4393088409..90a8be2c58 100644
> > > --- a/src/intel/isl/isl.c
> > > +++ b/src/intel/isl/isl.c
> > > @@ -1268,9 +1268,23 @@ isl_calc_row_pitch(const struct isl_device *dev,
> > >                     const struct isl_extent2d *phys_total_el,
> > >                     uint32_t *out_row_pitch)
> > >  {
> > > -   const uint32_t alignment =
> > > +   uint32_t alignment =
> > >        isl_calc_row_pitch_alignment(surf_info, tile_info);
> > >
> > > +   /* If pitch isn't given and it can be chosen freely, align it by
> cache
> > > line
> > > +    * allowing one to use blit engine on the surface.
> > > +    */
> > > +   if (surf_info->row_pitch == 0 && tile_info->tiling ==
> > > ISL_TILING_LINEAR) {
> > > +      /* From the Broadwell PRM docs for XY_SRC_COPY_BLT::
> > > SourceBaseAddress:
> > > +       *
> > > +       *    "Base address of the destination surface: X=0, Y=0. Lower
> > > 32bits
> > > +       *    of the 48bit addressing. When Src Tiling is enabled
> (Bit_15
> > > +       *    enabled), this address must be 4KB-aligned. When Tiling
> is not
> > > +       *    enabled, this address should be CL (64byte) aligned."
> > > +       */
> > > +      alignment = MAX2(alignment, 64);
> > > +   }
> > >
> >
> > I think we want this to be part of isl_calc_row_pitch_alignment
>
> In that case we need to add out_row_pitch as argument so that it can omit
> the alignment restriction for already given pitch. In i965 we get buffers
> that
> don't align by 64.
>

Then those buffers are broken... maybe?  I think we do have breakage in
here somewhere but it's not where I thought.  Looking at the docs harder,
it appears that only the base address has the 64-byte alignment requirement
but the row pitch does not.  In this case, we should be able to just drop
this alignment stuff altogether and maybe do something with the alignment
field.  However, my gut tells me that this will probably break something
somewhere.  We have code in intel_blit.c for handling this for some
operations but probably not all.  I think the correct solution is to fix
intel_blit.c to just not need the restriction.  Until then, this is
probably ok. :(

I'd like to see something added to the comment along those lines.  Also,
please add cleaning this up to your TODO list if it isn't already there.

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


> I had it originally in isl_calc_row_pitch_alignment() but decided it
> would be cleaner in isl_calc_row_pitch() due to the special case.
>
> >
> > Also, eventually, I'd like us to add an ISL_SURF_USAGE_BLIT_BIT and key
> > this as well as the row pitch too large fallback in patch 3 to it.  When
> we
> > go to implement GL_EXT_external_objects, any of these sorts of things
> which
> > apply on gen7+ will have to be in ISL because we need to guarantee that
> > Vulkan and GL have the same fallbacks at least for external things.  For
> > external, Vulkan will have to set the ISL_SURF_USAGE_BLIT_BIT for this
> > reason.
> >
> > --Jason
> >
> >
> > > +
> > >     const uint32_t min_row_pitch =
> > >        isl_calc_min_row_pitch(dev, surf_info, tile_info, phys_total_el,
> > >                               alignment);
> > > --
> > > 2.11.0
> > >
> > > _______________________________________________
> > > 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/20170721/25aca1c3/attachment-0001.html>