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

[Marek Olšák]

On Fri, Dec 20, 2024 at 10:24 AM Simona Vetter <simona.vetter at ffwll.ch>
wrote:

> On Thu, Dec 19, 2024 at 05:09:33PM +0100, Michel Dänzer wrote:
> > On 2024-12-19 10:02, Daniel Stone wrote:
> > >
> > > How this would be used in practice is also way too underdocumented. We
> > > need to document that exact-round-up 64b is more restrictive than
> > > any-multiple-of 64b is more restrictive than 'classic' linear. We need
> > > to document what people should advertise - if we were starting from
> > > scratch, the clear answer would be that anything which doesn't care
> > > should advertise all three, anything advertising any-multiple-of
> > > should also advertise exact-round-up, etc.
> > >
> > > But we're not starting from scratch, and since linear is 'special',
> > > userspace already has explicit knowledge of it. So AMD is going to
> > > have to advertise LINEAR forever, because media frameworks know about
> > > DRM_FORMAT_MOD_LINEAR and pass that around explicitly when they know
> > > that the buffer is linear. That and not breaking older userspace
> > > running in containers or as part of a bisect or whatever.
> > >
> > > There's also the question of what e.g. gbm_bo_get_modifier() should
> > > return. Again, if we were starting from scratch, most restrictive
> > > would make sense. But we're not, so I think it has to return LINEAR
> > > for maximum compatibility (because modifiers can't be morphed into
> > > other ones for fun), which further cements that we're not removing
> > > LINEAR.
> > >
> > > And how should allocators determine what to go for? Given that, I
> > > think the only sensible semantics are, when only LINEAR has been
> > > passed, to pick the most restrictive set possible; when LINEAR
> > > variants have been passed as well as LINEAR, to act as if LINEAR were
> > > not passed at all.
> >
> > These are all very good points, which call for stricter-than-usual
> > application of the "new UAPI requires corresponding user-space patches"
> > rule:
> >
> > * Patches adding support for the new modifiers in Mesa (and kernel)
> > drivers for at least two separate vendors
>
> I think this is too strict? At least I could come up with other scenarios
> where we'd need a new linear variant:
> - one driver, but two different devices that happen to have incompatible
>   linear requirements which break and get fixed with the new linear mode.
> - one driver, one device, but non-driver userspace allocates the linear
>   buffer somewhere else (e.g. from dma-buf heaps) and gets pitch
>   constraints wrong
>
> > * Patches adding support in at least one non-Mesa user-space component,
> > e.g. a Wayland compositor which has code using the existing linear
> > modifier (e.g. mutter)
>
> This also feels a bit too strict, since I think what Daniel proposed is
> that drivers do the special LINEAR handling when there are variants
> present in the list of compatible modifiers for an alloation. Hence I
> don't think compositor patches are necessarily required, but we definitely
> need to test to make sure it actually works and there's not surprises.
>
> The exception is of course when non-mesa userspace allocates/sizes the
> buffer itself (maybe for an soc where the display is separate and the gpu
> has stricter stride constraints than the display).
>
> > Ideally also describe a specific multi-vendor scenario which can't work
> > with the existing linear modifier, and validate that it works with the
> > new ones.
>
> I think that's really the crucial part, because adding modifiers without
> an actual use-case that they fix is just asking for more future trouble I
> think.
>

It won't always "work" with the new linear modifiers, but when it fails, it
will fail in a manner that is debuggable, understandable, and explainable
by non-driver developers, such as getting 0 common modifiers between 2
devices. For example, the GUI can report to a user that DRI_PRIME failed
because there are no common modifiers between the 2 devices, which is
better than failing with an unqueriable difficult-to-handle reason
(rejected pitch) or continuing with corruption (invalid pitch) or hangs
(invalid pitch causing buffer overrun and corrupting shader binaries next
to it).

Marek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20241225/83f4de8f/attachment.htm>