[Mesa-dev] [PATCH 2/5] i965/miptree: Cleanup some of the miptree map logic

[Chad Versace]

On Wed 15 Jul 2015, Anuj Phogat wrote:
> On Tue, Jul 14, 2015 at 9:56 AM, Ben Widawsky
> <benjamin.widawsky at intel.com> wrote:
> > At the crux of this change is moving whether or not we can even use the hardware
> > blitter into the can_blit_slice check. Fundamentally this makes sense as
> > blitting a slice is a subset in functionality of being able to use the blitter
> > at all.
> >
> > NOTE: I think it's bad practice to have the assert in a function that is
> > determining whether or not we should use the blitter, but I tried the
> > alternatives, and they look worse IMO.
> >
> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > ---
> >  src/mesa/drivers/dri/i965/intel_blit.c        | 13 +++++++++++++
> >  src/mesa/drivers/dri/i965/intel_blit.h        |  3 +++
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 27 +++++++++++++++++----------
> >  3 files changed, 33 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_blit.c b/src/mesa/drivers/dri/i965/intel_blit.c
> > index bc39053..c4701e3 100644
> > --- a/src/mesa/drivers/dri/i965/intel_blit.c
> > +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> > @@ -241,6 +241,19 @@ intel_miptree_blit_compatible_formats(mesa_format src, mesa_format dst)
> >     return false;
> >  }
> >
> > +bool
> > +intel_miptree_can_hw_blit(struct brw_context *brw, struct intel_mipmap_tree *mt)
> > +{
> > +   if (mt->compressed)
> > +      return false;
> > +
> > +   /* Prior to Sandybridge, the blitter can't handle Y tiling */
> > +   if (brw->gen < 6 && mt->tiling == I915_TILING_Y)
> > +      return false;
> > +
> > +   return true;
> > +}
> > +
> >  /**
> >   * Implements a rectangular block transfer (blit) of pixels between two
> >   * miptrees.
> > diff --git a/src/mesa/drivers/dri/i965/intel_blit.h b/src/mesa/drivers/dri/i965/intel_blit.h
> > index c3d19a5..e60dd9b 100644
> > --- a/src/mesa/drivers/dri/i965/intel_blit.h
> > +++ b/src/mesa/drivers/dri/i965/intel_blit.h
> > @@ -50,6 +50,9 @@ intelEmitCopyBlit(struct brw_context *brw,
> >
> >  bool intel_miptree_blit_compatible_formats(mesa_format src, mesa_format dst);
> >
> > +bool intel_miptree_can_hw_blit(struct brw_context *brw,
> > +                               struct intel_mipmap_tree *mt);
> > +
> >  bool intel_miptree_blit(struct brw_context *brw,
> >                          struct intel_mipmap_tree *src_mt,
> >                          int src_level, int src_slice,
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index 72fba49..1330c2f 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -2600,9 +2600,14 @@ intel_miptree_release_map(struct intel_mipmap_tree *mt,
> >  }
> >
> >  static bool
> > -can_blit_slice(struct intel_mipmap_tree *mt,
> > +can_blit_slice(struct brw_context *brw,
> > +               struct intel_mipmap_tree *mt,
> >                 unsigned int level, unsigned int slice)
> >  {
> > +
> > +   if (!intel_miptree_can_hw_blit(brw, mt))
> > +      return false;
> > +
> >     uint32_t image_x;
> >     uint32_t image_y;
> >     intel_miptree_get_image_offset(mt, level, slice, &image_x, &image_y);
> > @@ -2624,20 +2629,22 @@ use_intel_mipree_map_blit(struct brw_context *brw,
> >                            unsigned int slice)
> >  {
> >     if (brw->has_llc &&
> > -      /* It's probably not worth swapping to the blit ring because of
> > -       * all the overhead involved.
> > -       */
> >         !(mode & GL_MAP_WRITE_BIT) &&
> > -       !mt->compressed &&
> > -       (mt->tiling == I915_TILING_X ||
> > -        /* Prior to Sandybridge, the blitter can't handle Y tiling */
> > -        (brw->gen >= 6 && mt->tiling == I915_TILING_Y)) &&
> > -       can_blit_slice(mt, level, slice))
> > +       can_blit_slice(brw, mt, level, slice))
> >        return true;
> >
> >     if (mt->tiling != I915_TILING_NONE &&
> >         mt->bo->size >= brw->max_gtt_map_object_size) {
> > -      assert(can_blit_slice(mt, level, slice));
> > +      /* XXX: This assertion is actually the final condition for platforms
> > +       * without SSE4.1.  Returning false is not the right thing to do with
> > +       * the current code. On those platforms, the goal of this function is to give
> > +       * preference to the GTT, and at this point we've determined we cannot use
> > +       * the GTT, and we cannot blit, so we are out of options.
> > +       *
> > +       * NOTE: It should be possible to actually handle the case, but AFAIK, we
> > +       * never get this assertion.
> > +       */
> > +      assert(can_blit_slice(brw, mt, level, slice));
> >        return true;
> >     }
> >
> > --
> > 2.4.5
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> This patch now allows using hw blitter with I915_TILING_NONE
> which is not allowed for unexplained reason at present. So, we
> have a bug fix in this patch. May be you should split this in to
> two? Changes in the patch look fine to me.

The XY_SETUP_BLT instruction *does* support linear blits (at least for
SNB+). I suspect that i965 doesn't use the blitter for linear buffers
here because reading/writing to a mmapped linear buffer is usually much
faster than using the blitter for downloading/uploading the buffer.

Note that this function's name is "use_intel_mipree_map_blit", not
"can_intel_miptree_map_blit".

So, even though we could use blitter for linear buffers, i965 should
continue not doing it.