[Mesa-dev] [PATCH 6/6] i965: Allow Y-tiled allocations for large surfaces

Neil Roberts neil at linux.intel.com
Mon Mar 23 07:52:50 PDT 2015


Sorry for the delay in replying to this.

Ben Widawsky <ben at bwidawsk.net> writes:

>> > +static inline uint32_t
>> > +intel_miptree_blit_height(struct intel_mipmap_tree *mt)
>> > +{
>> > +   switch (mt->target) {
>> > +   case GL_TEXTURE_CUBE_MAP:
>> > +   case GL_TEXTURE_1D_ARRAY:
>> > +   case GL_TEXTURE_2D_ARRAY:
>> > +   case GL_TEXTURE_2D_MULTISAMPLE_ARRAY:
>> > +   case GL_TEXTURE_CUBE_MAP_ARRAY:
>> > +      assert(mt->logical_depth0);
>> > +      return mt->qpitch;
>> > +   case GL_TEXTURE_3D:
>> > +      /* FIXME 3d textures don't have a qpitch. I think it's simply the tiled
>> > +       * aligned mt->physical_height0. Since 3D textures aren't used often, just
>> > +       * print the perf debug from the caller and bail
>> > +       */
>> > +      /* fallthrough */
>> > +   default:
>> > +      return mt->total_height;
>> > +   }
>> > +}
>> 
>> This function might stop working on Skylake if we land my patch to fix
>> the qpitch calculation. In that case the qpitch isn't necessarily a
>> count of the number of rows. In 1D textures it is the number of pixels
>> and for compressed textures it is the number of blocks. Maybe we could
>> also store the physical_qpitch that is calculated in
>> brw_miptree_layout_texture_array?
>> 
>
> I'm pretty sure today we never use the blitter for compressed
> textures. Therefore, I believe we can ignore that case. In the case
> where we use pixels, I believe it will still work fine as long as long
> as each layer is tightly packed (which I thought it was). If it's not,
> then I suppose we have a problem. I'm also totally fine with making 1D
> fallthrough since I don't think it's a particularly common case for it
> to surpass total_height anyway.

I'm not sure what you are getting at here. Regardless of whether the 1D
slices are tightly packed, we can't just return the qpitch value here
for 1D textures because it has no relation to the height of the image.
The height of the image is always 1. The images actually aren't tightly
packed on Skylake because they need to be aligned to 64 pixels.

Is there any reason why we can't just use mt->logical_height0 instead of
trying to look at the qpitch? If everything using the blitter is
operating on one slice at a time, why would it ever try to blit
something that is taller than the height? It would be pointless to try
to include the padding between slices in the blit, wouldn't it?

Looking at the patch again in more detail I noticed something else that
I missed the first time.

> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 16bd151..ee8fae4 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -86,7 +86,6 @@ compute_msaa_layout(struct brw_context *brw, mesa_format format, GLenum target)
>     }
>  }
>  
> -
>  /**
>   * For single-sampled render targets ("non-MSRT"), the MCS buffer is a
>   * scaled-down bitfield representation of the color buffer which is capable of
> @@ -437,6 +436,12 @@ intel_miptree_create_layout(struct brw_context *brw,
>     return mt;
>  }
>  
> +static bool
> +miptree_exceeds_blit_height(struct intel_mipmap_tree *mt)
> +{
> +   return intel_miptree_blit_height(mt) >= intel_blit_tile_height(mt->tiling);
> +}

Is that supposed to be >= intel_blit_max_height instead? Otherwise it's
going to disable tiling for any texture that is taller than a single
tile, right?

Regards,
- Neil


More information about the mesa-dev mailing list