[Mesa-dev,01/14] i965: Don't tile 1D miptrees.

Submitted by Francisco Jerez on Feb. 6, 2015, 5:23 p.m.

Details

Message ID 1423243408-24744-1-git-send-email-currojerez@riseup.net
State New
Headers show

Not browsing as part of any series.

Commit Message

Francisco Jerez Feb. 6, 2015, 5:23 p.m.
It doesn't really improve locality of texture fetches, quite the
opposite it's a waste of memory bandwidth and space due to tile
alignment.
---
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 4 ++++
 1 file changed, 4 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 64752dd..311b204 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -488,6 +488,10 @@  intel_miptree_choose_tiling(struct brw_context *brw,
        base_format == GL_DEPTH_STENCIL_EXT)
       return I915_TILING_Y;
 
+   if (mt->target == GL_TEXTURE_1D ||
+       mt->target == GL_TEXTURE_1D_ARRAY)
+      return I915_TILING_NONE;
+
    int minimum_pitch = mt->total_width * mt->cpp;
 
    /* If the width is much smaller than a tile, don't bother tiling. */

Comments

On Friday, February 06, 2015 07:23:15 PM Francisco Jerez wrote:
> It doesn't really improve locality of texture fetches, quite the
> opposite it's a waste of memory bandwidth and space due to tile
> alignment.
> ---
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 64752dd..311b204 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -488,6 +488,10 @@ intel_miptree_choose_tiling(struct brw_context *brw,
>         base_format == GL_DEPTH_STENCIL_EXT)
>        return I915_TILING_Y;
>  
> +   if (mt->target == GL_TEXTURE_1D ||
> +       mt->target == GL_TEXTURE_1D_ARRAY)
> +      return I915_TILING_NONE;
> +
>     int minimum_pitch = mt->total_width * mt->cpp;
>  
>     /* If the width is much smaller than a tile, don't bother tiling. */
> 

What do you think about checking mt->logical_height0 == 1 instead?
GLES doesn't have 1D textures, but people might use a 2D texture with
height 1, and I think the same logic applies.

I've also been thinking of trying to make core Mesa hide 1D textures and
simply turn them into 2D textures with height = 1, so drivers don't have
to deal with them.

Either way, this seems like a good idea to me.
Kenneth Graunke <kenneth@whitecape.org> writes:

> On Friday, February 06, 2015 07:23:15 PM Francisco Jerez wrote:
>> It doesn't really improve locality of texture fetches, quite the
>> opposite it's a waste of memory bandwidth and space due to tile
>> alignment.
>> ---
>>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> index 64752dd..311b204 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> @@ -488,6 +488,10 @@ intel_miptree_choose_tiling(struct brw_context *brw,
>>         base_format == GL_DEPTH_STENCIL_EXT)
>>        return I915_TILING_Y;
>>  
>> +   if (mt->target == GL_TEXTURE_1D ||
>> +       mt->target == GL_TEXTURE_1D_ARRAY)
>> +      return I915_TILING_NONE;
>> +
>>     int minimum_pitch = mt->total_width * mt->cpp;
>>  
>>     /* If the width is much smaller than a tile, don't bother tiling. */
>> 
>
> What do you think about checking mt->logical_height0 == 1 instead?
> GLES doesn't have 1D textures, but people might use a 2D texture with
> height 1, and I think the same logic applies.
>
> I've also been thinking of trying to make core Mesa hide 1D textures and
> simply turn them into 2D textures with height = 1, so drivers don't have
> to deal with them.
>
> Either way, this seems like a good idea to me.

I guess that would work because the layer count of 1D array textures
ends up in logical_depth0 rather than logical_height0?  Sounds good to
me, but it would also have the side effect of not tiling "degenerate" 3D
textures with height=1 but depth>1, I guess we would still like to tile
those?  How about (mt->logical_height0 == 1 && (mt->logical_depth0 == 1 ||
mt->target != GL_TEXTURE_3D))?

Thanks Ken!
On Saturday, February 07, 2015 02:46:31 AM Francisco Jerez wrote:
> Kenneth Graunke <kenneth@whitecape.org> writes:
> 
> > On Friday, February 06, 2015 07:23:15 PM Francisco Jerez wrote:
> >> It doesn't really improve locality of texture fetches, quite the
> >> opposite it's a waste of memory bandwidth and space due to tile
> >> alignment.
> >> ---
> >>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >> 
> >> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> >> index 64752dd..311b204 100644
> >> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> >> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> >> @@ -488,6 +488,10 @@ intel_miptree_choose_tiling(struct brw_context *brw,
> >>         base_format == GL_DEPTH_STENCIL_EXT)
> >>        return I915_TILING_Y;
> >>  
> >> +   if (mt->target == GL_TEXTURE_1D ||
> >> +       mt->target == GL_TEXTURE_1D_ARRAY)
> >> +      return I915_TILING_NONE;
> >> +
> >>     int minimum_pitch = mt->total_width * mt->cpp;
> >>  
> >>     /* If the width is much smaller than a tile, don't bother tiling. */
> >> 
> >
> > What do you think about checking mt->logical_height0 == 1 instead?
> > GLES doesn't have 1D textures, but people might use a 2D texture with
> > height 1, and I think the same logic applies.
> >
> > I've also been thinking of trying to make core Mesa hide 1D textures and
> > simply turn them into 2D textures with height = 1, so drivers don't have
> > to deal with them.
> >
> > Either way, this seems like a good idea to me.
> 
> I guess that would work because the layer count of 1D array textures
> ends up in logical_depth0 rather than logical_height0?  Sounds good to
> me, but it would also have the side effect of not tiling "degenerate" 3D
> textures with height=1 but depth>1, I guess we would still like to tile
> those?  How about (mt->logical_height0 == 1 && (mt->logical_depth0 == 1 ||
> mt->target != GL_TEXTURE_3D))?
> 
> Thanks Ken!

I suppose that's fine, though frankly, trying to optimize a texture with
height 1 at all seems pretty unnecessary to me.  I've never seen a 3D
texture with height 1 in the wild...
I made a similar patch in my local tree because it will be necessary for
Skylake which doesn't support tiling for 1D textures. I made a little
test to time rendering a large (4096) 1D texture here:

https://github.com/bpeel/glthing/tree/time-1d-texture

It gives about an 11% increase in FPS with this patch.

It seems like a good idea to me to make it check the height rather than
the texture dimensions as Kenneth says.

Regards,
- Neil

Kenneth Graunke <kenneth@whitecape.org> writes:

> On Saturday, February 07, 2015 02:46:31 AM Francisco Jerez wrote:
>> Kenneth Graunke <kenneth@whitecape.org> writes:
>> 
>> > On Friday, February 06, 2015 07:23:15 PM Francisco Jerez wrote:
>> >> It doesn't really improve locality of texture fetches, quite the
>> >> opposite it's a waste of memory bandwidth and space due to tile
>> >> alignment.
>> >> ---
>> >>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 4 ++++
>> >>  1 file changed, 4 insertions(+)
>> >> 
>> >> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> >> index 64752dd..311b204 100644
>> >> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> >> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> >> @@ -488,6 +488,10 @@ intel_miptree_choose_tiling(struct brw_context *brw,
>> >>         base_format == GL_DEPTH_STENCIL_EXT)
>> >>        return I915_TILING_Y;
>> >>  
>> >> +   if (mt->target == GL_TEXTURE_1D ||
>> >> +       mt->target == GL_TEXTURE_1D_ARRAY)
>> >> +      return I915_TILING_NONE;
>> >> +
>> >>     int minimum_pitch = mt->total_width * mt->cpp;
>> >>  
>> >>     /* If the width is much smaller than a tile, don't bother tiling. */
>> >> 
>> >
>> > What do you think about checking mt->logical_height0 == 1 instead?
>> > GLES doesn't have 1D textures, but people might use a 2D texture with
>> > height 1, and I think the same logic applies.
>> >
>> > I've also been thinking of trying to make core Mesa hide 1D textures and
>> > simply turn them into 2D textures with height = 1, so drivers don't have
>> > to deal with them.
>> >
>> > Either way, this seems like a good idea to me.
>> 
>> I guess that would work because the layer count of 1D array textures
>> ends up in logical_depth0 rather than logical_height0?  Sounds good to
>> me, but it would also have the side effect of not tiling "degenerate" 3D
>> textures with height=1 but depth>1, I guess we would still like to tile
>> those?  How about (mt->logical_height0 == 1 && (mt->logical_depth0 == 1 ||
>> mt->target != GL_TEXTURE_3D))?
>> 
>> Thanks Ken!
>
> I suppose that's fine, though frankly, trying to optimize a texture with
> height 1 at all seems pretty unnecessary to me.  I've never seen a 3D
> texture with height 1 in the wild...
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev