[Mesa-dev] [PATCH] i965: correct alignment units for 2D compressed textures on Skylake

Nanley Chery nanleychery at gmail.com
Wed Jun 17 07:43:05 PDT 2015


Thanks for the diagram and explanation. The Bspec's Calculating Texel
Location for 2D Surfaces section says that each LOD's width and height is
determined by a formula using an alignment term. One reason is to calculate
the LOD positions. I'm assuming another is to prevent the sampler engine
from fetching texels outside of pages defined by the GTT by padding images
for cache alignment. Is this true? If we don't align LOD-{0,2} by the
alignment term and use a block dimension instead, don't we risk getting
this error? Even if this is not the case, the spec doesn't exempt LOD-0
from being aligned by the alignment term, right?

Thanks,
Nanley

On Mon, Jun 15, 2015 at 10:45 AM, Neil Roberts <neil at linux.intel.com> wrote:
> Nanley Chery <nanleychery at gmail.com> writes:
>
>> Although most of the patch is incorrect, the following change is still
>> necessary isn't it?
>> if (mt->compressed) {
>> mip1_width = ALIGN(minify(mt->physical_width0, 1), mt->align_w) +
>> - ALIGN(minify(mt->physical_width0, 2), bw);
>> + ALIGN(minify(mt->physical_width0, 2), mt->align_w);
>>
>> Each LOD is aligned by the alignment term. I don't see why we're
>> switching to block width here.
>
> I'm pretty sure the original version is correct and we do want to align
> to the block width here. This bit of code is not being used to work out
> the position of an image so it doesn't need to take into account the
> image alignment. It is only being used to calculate the mt->total_width
> value. That value is not aligned to the image alignment. You can see
> this because it will usually just be directly taken from
> mt->physical_width0 and that is not aligned. In the non-compressed case
> just below you can see that the second half of the addition is not
> aligned to anything at all. The total_width does however need to be at
> least a multiple of the block size because it isn't possible to allocate
> space for half of a block.
>
> If I understand correctly this code is just trying to cope with cases
> where the third image in the mipmap is positioned so that it extends
> past the width of the first image. If that happens then the width of the
> image containing all of the mipmap images needs to be extended slightly
> or it would crop the third mipmap image.
>
> Please see the attached SVG.
>
> Looking at it a bit more I think this bit above is wrong:
>
> if (mt->compressed) {
> mt->total_width = ALIGN(mt->physical_width0, mt->align_w);
> }
>
> That should also be using bw instead of mt->align_w for the same reason.
> I think it could only end up making the total_width a bit larger than
> necessary so it probably isn't causing any actual problems.
>
> I'll make a patch and test it.
>
> Regards,
> - Neil
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150617/cda0302b/attachment.html>


More information about the mesa-dev mailing list