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

[Michel Dänzer]

On 2024-12-16 22:54, Marek Olšák wrote:
> On Mon, Dec 16, 2024 at 5:46 AM Lucas Stach <l.stach at pengutronix.de <mailto:l.stach at pengutronix.de>> 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> <mailto: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.
> 
> 
> That's a good point.
> 
> It could be:
> - LINEAR_PITCH_ALIGN_EXACT_#B
> - LINEAR_PITCH_ALIGN_MULTIPLE_#B
ALIGN seems fundamentally confusing to me, since any multiple of #B is "aligned to #B".

Maybe something like:

LINEAR_PITCH_MIN_MULTIPLE_#B
LINEAR_PITCH_ANY_MULTIPLE_#B

"minimal multiple of #B" vs "any multiple of #B"


-- 
Earthling Michel Dänzer       \        GNOME / Xwayland / Mesa developer
https://redhat.com             \               Libre software enthusiast