[Mesa-dev] [PATCH] i965: Fix aligning to the block size in intel_miptree_copy_slice

[Neil Roberts]

Hi,

Could you perhaps send me a list of the regressions? I ran it through on
my Skylake and the only changes I see are the two FXT1 tests. However, I
have to admit that my device is being a bit flakey so I cheated a little
bit to make this work. Originally when I compared the differences with
and without the patch I had about 150 differences (some fixes, some
regressions) that were pretty random. I got the list of these with
"piglit summary console -d" and then reran just those tests (with the
--test-list option) after a reboot with and without the patch. I then
have a noddy Python script to merge a Piglit results file so that it
replaces test results from the first file with any that appear in the
second file. Once I do that then there are no regressions and only the
two FXT1 tests appear in the fixes.

It might be interesting to see if you can run some of the regressing
tests by hand after a reboot to see if they fail consistently.

Regards,
- Neil

Nanley Chery <nanleychery at gmail.com> writes:

> Hey Neil,
>
> While this patch does fix FXT1, it also regresses 21 other Piglit tests on SKL.
>
> - Nanley
>
> On Thu, Jun 11, 2015 at 8:59 AM, Neil Roberts <neil at linux.intel.com> wrote:
>> This function was trying to align the width and height to a multiple
>> of the block size for compressed textures. It was using align_w/h as a
>> shortcut to get the block size as up until Gen9 this always happens to
>> match. However in Gen9+ the alignment values are expressed as
>> multiples of the block size so in effect the alignment values are
>> always 4 for compressed textures as that is the minimum value we can
>> pick. This happened to work for most compressed formats because the
>> block size is also 4, but for FXT1 this was breaking because it has a
>> block width of 8.
>>
>> This fixes some Piglit tests testing FXT1 such as
>>
>> spec at 3dfx_texture_compression_fxt1@fbo-generatemipmap-formats
>> ---
>>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> index 615cbfb..6c00581 100644
>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>> @@ -1168,8 +1168,10 @@ intel_miptree_copy_slice(struct brw_context *brw,
>>     assert(src_mt->format == dst_mt->format);
>>
>>     if (dst_mt->compressed) {
>> -      height = ALIGN(height, dst_mt->align_h) / dst_mt->align_h;
>> -      width = ALIGN(width, dst_mt->align_w);
>> +      unsigned int i, j;
>> +      _mesa_get_format_block_size(dst_mt->format, &i, &j);
>> +      height = ALIGN(height, j) / j;
>> +      width = ALIGN(width, i);
>>     }
>>
>>     /* If it's a packed depth/stencil buffer with separate stencil, the blit
>> --
>> 1.9.3
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev