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

[Anuj Phogat]

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.