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

[Marek Olšák]

On Mon, Dec 16, 2024 at 9:53 AM Simona Vetter <simona.vetter at ffwll.ch>
wrote:

> On Mon, Dec 16, 2024 at 11:46:13AM +0100, Lucas Stach wrote:
> > Am Montag, dem 16.12.2024 um 10:27 +0100 schrieb Michel Dänzer:
> > > On 2024-12-15 21:53, Marek Olšák wrote:
> > > > The comment explains the problem with DRM_FORMAT_MOD_LINEAR.
> > > >
> > > > Signed-off-by: Marek Olšák <marek.olsak at amd.com <mailto:
> marek.olsak at amd.com>>
> > > >
> > > > diff --git a/include/uapi/drm/drm_fourcc.h
> b/include/uapi/drm/drm_fourcc.h
> > > > index 78abd819fd62e..8ec4163429014 100644
> > > > --- a/include/uapi/drm/drm_fourcc.h
> > > > +++ b/include/uapi/drm/drm_fourcc.h
> > > > @@ -484,9 +484,27 @@ extern "C" {
> > > >   * modifier (e.g. not setting DRM_MODE_FB_MODIFIERS in the
> DRM_ADDFB2 ioctl),
> > > >   * which tells the driver to also take driver-internal information
> into account
> > > >   * and so might actually result in a tiled framebuffer.
> > > > + *
> > > > + * WARNING:
> > > > + * There are drivers out there that expose DRM_FORMAT_MOD_LINEAR,
> but only
> > > > + * support a certain pitch alignment and can't import images with
> this modifier
> > > > + * if the pitch alignment isn't exactly the one supported. They can
> however
> > > > + * allocate images with this modifier and other drivers can import
> them only
> > > > + * if they support the same pitch alignment. Thus,
> DRM_FORMAT_MOD_LINEAR is
> > > > + * fundamentically incompatible across devices and is the only
> modifier that
> > > > + * has a chance of not working. The PITCH_ALIGN modifiers should be
> used
> > > > + * instead.
> > > >   */
> > > >  #define DRM_FORMAT_MOD_LINEAR  fourcc_mod_code(NONE, 0)
> > > >
> > > > +/* Linear layout modifiers with an explicit pitch alignment in
> bytes.
> > > > + * Exposing this modifier requires that the pitch alignment is
> exactly
> > > > + * the number in the definition.
> > > > + */
> > > > +#define DRM_FORMAT_MOD_LINEAR_PITCH_ALIGN_64B fourcc_mod_code(NONE,
> 1)
> > >
> > > It's not clear what you mean by "requires that the pitch alignment is
> exactly
> > > the number in the definition", since a pitch which is aligned to 256
> bytes is
> > > also aligned to 128 & 64 bytes. Do you mean the pitch must be exactly
> the width
> > > rounded up to the next / smallest possible multiple of the specified
> number of bytes?
> >
> > I guess that's the intention here, as some AMD GPUs apparently have
> > this limitation that they need an exact aligned pitch.
> >
> > If we open the can of worms to overhaul the linear modifier, I think it
> > would make sense to also add modifiers for the more common restriction,
> > where the pitch needs to be aligned to a specific granule, but the
> > engine doesn't care if things get overaligned to a multiple of the
> > granule. Having both sets would probably make it easier for the reader
> > to see the difference to the exact pitch modifiers proposed in this
> > patch.
>
> Yeah I think with linear modifiers that sepcificy alignment limitations we
> need to be _extremely_ precise in what exactly is required, and what not.
> There's all kinds of hilarious stuff that might be incompatible, and if we
> don't mind those we're right back to the "everyone agrees on what linear
> means" falacy.
>
> So if pitch must be a multiple of 64, but cannot be a multiple of 128
> (because the hw does not cope with the padding, then that's important).
> Other things are alignment constraints on the starting point, and any
> padding you need to add at the bottom (yeah some hw overscans and gets
> pissed if there's not memory there). So I think it's best to go overboard
> here with verbosity.
>
> There's also really funny stuff like power-of-two alignment and things
> like that, but I guess we'll get there if that's ever needed. That's also
> why I think we don't need to add all possible linear modifiers from the
> start, unless there's maybe too much confusion with stuff like "exactly
> 64b aligned pitch" and "at least 64b aligned pitch, but you can add lots
> of padding if you feel like".
>

Would it be possible and acceptable to require that the offset alignment is
always 4K and the slice padding is also always 4K to simplify those
constraints?

Marek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20241216/4cc1529f/attachment.htm>