[PATCH] uapi/drm/drm_fourcc.h: Note on platform specificity for format modifiers

[Daniel Vetter]

On Mon, May 4, 2020 at 11:08 AM Mika Kahola <mika.kahola at intel.com> wrote:
>
> Make an additional note on DRM format modifiers for x and y tiling. These
> format modifiers are defined for BDW+ platforms and therefore definition
> is not valid for older gens. This is due to address swizzling for tiled
> surfaces is no longer used. For newer platforms main memory controller has
> a more effective address swizzling algorithm.
>
> Signed-off-by: Mika Kahola <mika.kahola at intel.com>
> ---
>  include/uapi/drm/drm_fourcc.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 8bc0b31597d8..3e56c24cabb6 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -356,7 +356,8 @@ extern "C" {
>   *
>   * This format is highly platforms specific and not useful for cross-driver
>   * sharing. It exists since on a given platform it does uniquely identify the
> - * layout in a simple way for i915-specific userspace.
> + * layout in a simple way for i915-specific userspace.  The format is
> + * ill-defined for platforms older than BDW.

Hm this is a bit confusing, since you're just adding a new sentences
and not really clarifying the paragraph overall. Right above we have
the detailed description of the modifier on gen8+, I'd replace the
entire paragraph here with something like:

"Note that this layout is only accurate on intel gen 8+ or valleyview
chipsets. On earlier platforms the is highly platforms specific and
not useful for cross-driver
sharing. It exists since on a given platform it does uniquely identify
the layout in a simple way for i915-specific userspace, which
facilitated conversion of userspace to modifiers. Additionally the
exact format on some really old platforms is not known."

That way it's much clearer that the specification is well-defined and
where, with the old stuff as exceptions. Your patch kinda makes bdw+
the exception of the exception, and that's hard to understand.

Cheers, Daniel



>   */
>  #define I915_FORMAT_MOD_X_TILED        fourcc_mod_code(INTEL, 1)
>
> @@ -371,7 +372,8 @@ extern "C" {
>   *
>   * This format is highly platforms specific and not useful for cross-driver
>   * sharing. It exists since on a given platform it does uniquely identify the
> - * layout in a simple way for i915-specific userspace.
> + * layout in a simple way for i915-specific userspace. The format is
> + * ill-defined for platforms older than BDW.
>   */
>  #define I915_FORMAT_MOD_Y_TILED        fourcc_mod_code(INTEL, 2)
>
> --
> 2.20.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch