[PATCH] drm/fourcc: add LINEAR modifiers with an exact pitch alignment

[Marek Olšák]

On Tue, Jan 14, 2025 at 1:34 PM Faith Ekstrand <faith at gfxstrand.net> wrote:

> On January 14, 2025 03:39:45 Marek Olšák <maraeo at gmail.com> wrote:
>
>> I would keep the existing modifier interfaces, API extensions, and
>> expectations the same as today for simplicity.
>>
>> The new linear modifier definition (proposal) will have these fields:
>>    5 bits for log2 pitch alignment in bytes
>>    5 bits for log2 height alignment in rows
>>    5 bits for log2 offset alignment in bytes
>>    5 bits for log2 minimum pitch in bytes
>>    5 bits for log2 minimum (2D) image size in bytes
>>
>
> I'm not strictly opposed to adding a new modifier or two but this seems
> massively over-designed. First off, no one uses anything but simple 2D
> images for WSI and BOs are allocated in units of 4k pages so 2, 4, and 5
> can go. If we assume pitch alignment and offset alignment are the same (and
> offset is almost always 0 anyway), 3 can go.
>
> Even with that, I'm struggling to see how useful this is. My understanding
> is that you're trying to solve a problem where you need an exact 64-byte
> alignment for some AMD scanout stuff. That's not even possible to support
> on Nvidia (minimum alignment is 128B) so practically you're looking at one
> modifier that's shared between AMD and Intel. Why can't we just add an AMD
> modifier, make Intel support it, and move on?
>
> Otherwise we're massively exploding the modifier space for... Why? Intel
> will have to advertise basically all of them. Nvidia will advertise most of
> them. AMD will advertise something. And now apps have tens of thousands of
> modifiers to sort through when we could have just added one and solved the
> problem.
>

I don't think I'm being understood. See my reply to Daniel. There is no
exploding of anything. It's the same thing we have today for vendor
modifiers - lots of fields with lots of possible values, but only a few
values are used.

It's most likely under-designed, but it exactly solves the problem. The
minimum requirement for every modifier is that it must exactly identify a
memory layout. Saying that we just need a pitch alignment of 256B (that's
the AMD one) is not enough. Height alignment and image size alignment are
required to make sure that the next plane doesn't start in the padding area
because it can be overwritten randomly OR it can be read and cause a page
fault if the full padding isn't allocated. Offset alignment is also
required for multi plane images. If you want all allocators to allocate
NV12 equally, the 5 fields are required.

Marek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20250114/9610f696/attachment-0001.htm>