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

[Nanley Chery]

Hey  Neil,

My testing method was to run the following command against master and
master + your patch:
./piglit run gpu -x glx -x glean -x fbo-depth-array # exclusions
necessary to prevent SKL hang.

I then ran a piglit summary comparison (console output) against the
result and grepped for "pass fail". The regressions I saw were:
spec/arb_texture_multisample/texelfetch/2-fs-sampler2dmsarray: pass fail
spec/arb_texture_float/fbo-clear-formats/GL_RGBA16F_ARB: pass fail
spec/glsl-1.30/execution/vs-float-uint-conversion: pass fail
spec/glsl-1.50/execution/built-in-functions/gs-op-mult-mat4x3-float: pass fail
spec/ext_framebuffer_multisample_blit_scaled/blit-scaled samples=2: pass fail
spec/arb_texture_compression_bptc/fbo-generatemipmap-formats
float/GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT: pass fail
spec/arb_texture_gather/texturegatheroffset/vs-rgb-one-int-2d: pass fail
spec/glsl-1.20/execution/uniform-initializer/vs-float-array: pass fail
spec/glsl-1.10/execution/variable-indexing/vs-varying-array-mat3-col-rd:
pass fail
spec/glsl-1.30/execution/interpolation/interpolation-smooth-gl_backsecondarycolor-smooth-fixed:
pass fail
spec/!opengl 1.1/texwrap 2d/GL_RGBA8: pass fail
spec/arb_texture_compression_bptc/fbo-generatemipmap-formats
float/GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT NPOT: pass fail
spec/glsl-1.30/execution/built-in-functions/fs-op-bitand-neg-abs-ivec3-ivec3:
pass fail
spec/glsl-1.50/execution/built-in-functions/gs-op-uplus-mat3: pass fail
spec/!opengl 2.0/vertex-program-two-side enabled front back front2
back2: pass fail
spec/arb_texture_multisample/texelfetch/4-fs-isampler2dms: pass fail
spec/glsl-1.20/execution/built-in-functions/fs-op-mult-mat4x2-mat3x4: pass fail
spec/ext_framebuffer_multisample/accuracy 6 depth_draw small: pass fail
spec/!opengl 3.0/clearbuffer-mixed-format: pass fail
spec/arb_texture_compression_bptc/fbo-generatemipmap-formats
float/GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT: pass fail
spec/arb_texture_float/fbo-clear-formats/GL_RGB16F_ARB: pass fail

However, I'm going through and testing some of them by hand as you
suggested and they've all been passing thus far. I'm not sure why I'm
getting different results now.

Thanks,
Nanley


On Fri, Jun 12, 2015 at 9:07 AM, Neil Roberts <neil at linux.intel.com> wrote:
> 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
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev