diff mbox series

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

Message ID 20200504090843.27074-1-mika.kahola@intel.com (mailing list archive)
State New, archived
Headers show
Series uapi/drm/drm_fourcc.h: Note on platform specificity for format modifiers | expand

Commit Message

Kahola, Mika May 4, 2020, 9:08 a.m. UTC
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@intel.com>
---
 include/uapi/drm/drm_fourcc.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Daniel Vetter May 4, 2020, 11:17 a.m. UTC | #1
On Mon, May 4, 2020 at 11:08 AM Mika Kahola <mika.kahola@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@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@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
Kahola, Mika May 6, 2020, noon UTC | #2
> -----Original Message-----
> From: Daniel Vetter <daniel@ffwll.ch>
> Sent: Monday, May 4, 2020 2:17 PM
> To: Kahola, Mika <mika.kahola@intel.com>
> Cc: dri-devel <dri-devel@lists.freedesktop.org>
> Subject: Re: [PATCH] uapi/drm/drm_fourcc.h: Note on platform specificity for
> format modifiers
> 
> On Mon, May 4, 2020 at 11:08 AM Mika Kahola <mika.kahola@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@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.
I admit, that the wording may have been confusing. I'll update the patch with your proposal.

Cheers,
Mika

> 
> 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@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
diff mbox series

Patch

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.
  */
 #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)