Re: [PATCH] drm/fourcc: document modifier uniqueness requirements

[Daniel Vetter <daniel@ffwll.ch>]

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@emersion.fr>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Daniel Stone <daniel@fooishbar.org>
> Cc: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
> Cc: Dave Airlie <airlied@gmail.com>
> Cc: Marek Olšák <maraeo@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.

With something like that added:

Reviewed-by: Daniel Vetter <daniel.vetter@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).
> + *
>   * 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel