diff mbox series

drm/uapi: Move drm_color_ctm_3x4 out from drm_mode.h

Message ID 20240422085857.17651-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/uapi: Move drm_color_ctm_3x4 out from drm_mode.h | expand

Commit Message

Ville Syrjälä April 22, 2024, 8:58 a.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

drm_color_ctm_3x4 is some undocumented amgdpu private
uapi and thus has no business being in drm_mode.h.
At least move it to some amdgpu specific header, albeit
with the wrong namespace as maybe something somewhere
is using this already?

Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Joshua Ashton <joshua@froggi.es>
Cc: Alex Deucher <alexander.deucher@amd.com>
Fixes: 6872a189be50 ("drm/amd/display: Add 3x4 CTM support for plane CTM")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 include/uapi/drm/amdgpu_drm.h | 9 +++++++++
 include/uapi/drm/drm_mode.h   | 8 --------
 2 files changed, 9 insertions(+), 8 deletions(-)

Comments

Simon Ser April 24, 2024, 3:48 p.m. UTC | #1
On Monday, April 22nd, 2024 at 10:58, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:

> drm_color_ctm_3x4 is some undocumented amgdpu private
> uapi and thus has no business being in drm_mode.h.
> At least move it to some amdgpu specific header, albeit
> with the wrong namespace as maybe something somewhere
> is using this already?

If something somewhere is using this already, I don't think the name
matters? In other words, if there is a user, such user would be broken
when moving the declaration anyways, so it shouldn't be more risky to
do the rename as well?

I'm holding off the libdrm header update until we find a solution.
Harry Wentland April 30, 2024, 6:03 p.m. UTC | #2
On 2024-04-22 04:58, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> drm_color_ctm_3x4 is some undocumented amgdpu private
> uapi and thus has no business being in drm_mode.h.
> At least move it to some amdgpu specific header, albeit
> with the wrong namespace as maybe something somewhere
> is using this already?
> 
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Joshua Ashton <joshua@froggi.es>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Fixes: 6872a189be50 ("drm/amd/display: Add 3x4 CTM support for plane CTM")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

We'll bring this back with the color pipeline stuff but for now it doesn't
need to sit in drm_mode.h and probably shouldn't.

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  include/uapi/drm/amdgpu_drm.h | 9 +++++++++
>  include/uapi/drm/drm_mode.h   | 8 --------
>  2 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index 96e32dafd4f0..d5ebafacdd70 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -1269,6 +1269,15 @@ struct drm_amdgpu_info_gpuvm_fault {
>  #define AMDGPU_FAMILY_GC_10_3_7			151 /* GC 10.3.7 */
>  #define AMDGPU_FAMILY_GC_11_5_0			150 /* GC 11.5.0 */
>  
> +/* FIXME wrong namespace! */
> +struct drm_color_ctm_3x4 {
> +	/*
> +	 * Conversion matrix with 3x4 dimensions in S31.32 sign-magnitude
> +	 * (not two's complement!) format.
> +	 */
> +	__u64 matrix[12];
> +};
> +
>  #if defined(__cplusplus)
>  }
>  #endif
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 1ca5c7e418fd..d390011b89b4 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -846,14 +846,6 @@ struct drm_color_ctm {
>  	__u64 matrix[9];
>  };
>  
> -struct drm_color_ctm_3x4 {
> -	/*
> -	 * Conversion matrix with 3x4 dimensions in S31.32 sign-magnitude
> -	 * (not two's complement!) format.
> -	 */
> -	__u64 matrix[12];
> -};
> -
>  struct drm_color_lut {
>  	/*
>  	 * Values are mapped linearly to 0.0 - 1.0 range, with 0x0 == 0.0 and
Ville Syrjälä May 7, 2024, 11:49 a.m. UTC | #3
On Wed, Apr 24, 2024 at 03:48:24PM +0000, Simon Ser wrote:
> On Monday, April 22nd, 2024 at 10:58, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> 
> > drm_color_ctm_3x4 is some undocumented amgdpu private
> > uapi and thus has no business being in drm_mode.h.
> > At least move it to some amdgpu specific header, albeit
> > with the wrong namespace as maybe something somewhere
> > is using this already?
> 
> If something somewhere is using this already, I don't think the name
> matters? In other words, if there is a user, such user would be broken
> when moving the declaration anyways, so it shouldn't be more risky to
> do the rename as well?

I suppose. Not something I want to have to think about though.

I've pushed this to drm-misc-next with Harry's rb (thanks).

> 
> I'm holding off the libdrm header update until we find a solution.
diff mbox series

Patch

diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index 96e32dafd4f0..d5ebafacdd70 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -1269,6 +1269,15 @@  struct drm_amdgpu_info_gpuvm_fault {
 #define AMDGPU_FAMILY_GC_10_3_7			151 /* GC 10.3.7 */
 #define AMDGPU_FAMILY_GC_11_5_0			150 /* GC 11.5.0 */
 
+/* FIXME wrong namespace! */
+struct drm_color_ctm_3x4 {
+	/*
+	 * Conversion matrix with 3x4 dimensions in S31.32 sign-magnitude
+	 * (not two's complement!) format.
+	 */
+	__u64 matrix[12];
+};
+
 #if defined(__cplusplus)
 }
 #endif
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 1ca5c7e418fd..d390011b89b4 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -846,14 +846,6 @@  struct drm_color_ctm {
 	__u64 matrix[9];
 };
 
-struct drm_color_ctm_3x4 {
-	/*
-	 * Conversion matrix with 3x4 dimensions in S31.32 sign-magnitude
-	 * (not two's complement!) format.
-	 */
-	__u64 matrix[12];
-};
-
 struct drm_color_lut {
 	/*
 	 * Values are mapped linearly to 0.0 - 1.0 range, with 0x0 == 0.0 and