On Mon, Dec 16, 2024 at 5:46 AM Lucas Stach <l.stach@xxxxxxxxxxxxxx> 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@xxxxxxx <mailto:marek.olsak@xxxxxxx>>
> >
> > 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.
It could be:
- LINEAR_PITCH_ALIGN_EXACT_#B
- LINEAR_PITCH_ALIGN_MULTIPLE_#B
Drivers that expose the MULTIPLE ones should also expose the EXACT ones that are equivalent. Other drivers will only expose the EXACT ones but not the MULTIPLE ones.
Marek
