[PATCH] drm/fourcc: document modifier uniqueness requirements

[Brian Starkey]

On Wed, May 27, 2020 at 10:55:34AM +0200, Daniel Vetter wrote:
> On Wed, May 27, 2020 at 08:52:00AM +0000, Simon Ser wrote:
> > There have suggestions to bake pitch alignment, address alignement,
> > contiguous memory or other placement (hidden VRAM, GTT/BAR, etc)
> > constraints into modifiers. Last time this was brought up it seemed
> > like the consensus was to not allow this. Document this in drm_fourcc.h.
> > 
> > There are several reasons for this.
> > 
> > - Encoding all of these constraints in the modifiers would explode the
> >   search space pretty quickly (we only have 64 bits to work with).
> > - Modifiers need to be unambiguous: a buffer can only have a single
> >   modifier.
> > - Modifier users aren't expected to parse modifiers.
> > 
> > Signed-off-by: Simon Ser <contact at emersion.fr>
> > Cc: Daniel Vetter <daniel at ffwll.ch>
> > Cc: Daniel Stone <daniel at fooishbar.org>
> > Cc: Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl>
> > Cc: Dave Airlie <airlied at gmail.com>
> > Cc: Marek Olšák <maraeo at gmail.com>
> > ---
> >  include/uapi/drm/drm_fourcc.h | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> > index 490143500a50..97eb0f1cf9f8 100644
> > --- a/include/uapi/drm/drm_fourcc.h
> > +++ b/include/uapi/drm/drm_fourcc.h
> > @@ -58,6 +58,17 @@ extern "C" {
> >   * may preserve meaning - such as number of planes - from the fourcc code,
> >   * whereas others may not.
> >   *
> > + * Modifiers must uniquely encode buffer layout. In other words, a buffer must
> > + * match only a single modifier. A modifier must not be a subset of layouts of
> > + * another modifier. For instance, it's incorrect to encode pitch alignment in
> > + * a modifier: a buffer may match a 64-pixel aligned modifier and a 32-pixel
> > + * aligned modifier. That said, modifiers can have implicit minimal
> > + * requirements.
> 
> I think we should also add the aliasing when the fourcc codes are
> involved, with afbc as example. Maybe:
> 
> For modifiers where the combination of fourcc code and modifier can alias,
> a canonical pair needs to be defined and used by all drivers. An example
> is afbc, where both argb and abgr have the exact same compressed layout.

That's actually explicitly _not_ the case for AFBC, which was one of
the things I was trying to document in afbc.rst.

> 
> With something like that added:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> 
> 
> > + *
> > + * Users see modifiers as opaque tokens they can check for equality and
> > + * intersect. Users musn't need to know to reason about the modifier value
> > + * (i.e. users are not expected to extract information out of the modifier).
> > + *

Doesn't this conflict with "implicit minimal requirements" above?

Certainly for a bunch of different AFBC modifiers, the allocator would
need to understand some fields in order to properly align-up the
buffer size.

Thanks,
-Brian

> >   * Vendors should document their modifier usage in as much detail as
> >   * possible, to ensure maximum compatibility across devices, drivers and
> >   * applications.
> > -- 
> > 2.26.2
> > 
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel