diff mbox series

[v2,34/34] drm/amd/display: Use 3x4 CTM for plane CTM

Message ID 20230810160314.48225-35-mwen@igalia.com (mailing list archive)
State New, archived
Headers show
Series drm/amd/display: add AMD driver-specific properties for color mgmt | expand

Commit Message

Melissa Wen Aug. 10, 2023, 4:03 p.m. UTC
From: Joshua Ashton <joshua@froggi.es>

Signed-off-by: Joshua Ashton <joshua@froggi.es>
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 32 +++++++++++++++++--
 .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  2 +-
 include/uapi/drm/drm_mode.h                   |  8 +++++
 3 files changed, 38 insertions(+), 4 deletions(-)

Comments

Harry Wentland Sept. 6, 2023, 6:28 p.m. UTC | #1
On 2023-08-10 12:03, Melissa Wen wrote:
> From: Joshua Ashton <joshua@froggi.es>
> 
> Signed-off-by: Joshua Ashton <joshua@froggi.es>
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 32 +++++++++++++++++--
>  .../amd/display/amdgpu_dm/amdgpu_dm_plane.c   |  2 +-
>  include/uapi/drm/drm_mode.h                   |  8 +++++
>  3 files changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> index 7ff329101fd4..0a51af44efd5 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
> @@ -412,6 +412,32 @@ static void __drm_ctm_to_dc_matrix(const struct drm_color_ctm *ctm,
>  	}
>  }
>  
> +/**
> + * __drm_ctm2_to_dc_matrix - converts a DRM CTM2 to a DC CSC float matrix
> + * @ctm: DRM color transformation matrix
> + * @matrix: DC CSC float matrix
> + *
> + * The matrix needs to be a 3x4 (12 entry) matrix.
> + */
> +static void __drm_ctm2_to_dc_matrix(const struct drm_color_ctm2 *ctm,
> +				   struct fixed31_32 *matrix)
> +{
> +	int i;
> +
> +	/*
> +	 * DRM gives a 3x3 matrix, but DC wants 3x4. Assuming we're operating
> +	 * with homogeneous coordinates, augment the matrix with 0's.
> +	 *

Left-over copy-paste comment. This version takes 3x4 as input param.

> +	 * The format provided is S31.32, using signed-magnitude representation.
> +	 * Our fixed31_32 is also S31.32, but is using 2's complement. We have
> +	 * to convert from signed-magnitude to 2's complement.
> +	 */
> +	for (i = 0; i < 12; i++) {
> +		/* gamut_remap_matrix[i] = ctm[i - floor(i/4)] */
> +		matrix[i] = dc_fixpt_from_s3132(ctm->matrix[i]);
> +	}
> +}
> +
>  /**
>   * __set_legacy_tf - Calculates the legacy transfer function
>   * @func: transfer function
> @@ -1159,7 +1185,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
>  {
>  	struct amdgpu_device *adev = drm_to_adev(crtc->base.state->dev);
>  	struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
> -	struct drm_color_ctm *ctm = NULL;
> +	struct drm_color_ctm2 *ctm = NULL;
>  	struct dc_color_caps *color_caps = NULL;
>  	bool has_crtc_cm_degamma;
>  	int ret;
> @@ -1213,7 +1239,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
>  
>  	/* Setup CRTC CTM. */
>  	if (dm_plane_state->ctm) {
> -		ctm = (struct drm_color_ctm *)dm_plane_state->ctm->data;
> +		ctm = (struct drm_color_ctm2 *)dm_plane_state->ctm->data;
>  
>  		/*
>  		 * So far, if we have both plane and CRTC CTM, plane CTM takes
> @@ -1224,7 +1250,7 @@ int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
>  		 * provide support for both DPP and MPC matrix at the same
>  		 * time.
>  		 */
> -		__drm_ctm_to_dc_matrix(ctm, dc_plane_state->gamut_remap_matrix.matrix);
> +		__drm_ctm2_to_dc_matrix(ctm, dc_plane_state->gamut_remap_matrix.matrix);
>  
>  		dc_plane_state->gamut_remap_matrix.enable_remap = true;
>  		dc_plane_state->input_csc_color_matrix.enable_adjustment = false;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> index 0b1081c690cb..27962a3d30f5 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
> @@ -1543,7 +1543,7 @@ dm_atomic_plane_set_property(struct drm_plane *plane,
>  		ret = drm_property_replace_blob_from_id(plane->dev,
>  							&dm_plane_state->ctm,
>  							val,
> -							sizeof(struct drm_color_ctm), -1,
> +							sizeof(struct drm_color_ctm2), -1,

We need to update the comment for dm_plane_state.ctm in amdgpu_dm.h
to specify the property is of type drm_color_ctm2 (or drm_color_ctm_3x4).

>  							&replaced);
>  		dm_plane_state->base.color_mgmt_changed |= replaced;
>  		return ret;
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 46becedf5b2f..402288133e4c 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -838,6 +838,14 @@ struct drm_color_ctm {
>  	__u64 matrix[9];
>  };
>  
> +struct drm_color_ctm2 {

Calling this drm_color_ctm_3x4 might be good to make it clear this is
for a 3x4 matrix.

Harry

> +	/*
> +	 * Conversion matrix 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
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
index 7ff329101fd4..0a51af44efd5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c
@@ -412,6 +412,32 @@  static void __drm_ctm_to_dc_matrix(const struct drm_color_ctm *ctm,
 	}
 }
 
+/**
+ * __drm_ctm2_to_dc_matrix - converts a DRM CTM2 to a DC CSC float matrix
+ * @ctm: DRM color transformation matrix
+ * @matrix: DC CSC float matrix
+ *
+ * The matrix needs to be a 3x4 (12 entry) matrix.
+ */
+static void __drm_ctm2_to_dc_matrix(const struct drm_color_ctm2 *ctm,
+				   struct fixed31_32 *matrix)
+{
+	int i;
+
+	/*
+	 * DRM gives a 3x3 matrix, but DC wants 3x4. Assuming we're operating
+	 * with homogeneous coordinates, augment the matrix with 0's.
+	 *
+	 * The format provided is S31.32, using signed-magnitude representation.
+	 * Our fixed31_32 is also S31.32, but is using 2's complement. We have
+	 * to convert from signed-magnitude to 2's complement.
+	 */
+	for (i = 0; i < 12; i++) {
+		/* gamut_remap_matrix[i] = ctm[i - floor(i/4)] */
+		matrix[i] = dc_fixpt_from_s3132(ctm->matrix[i]);
+	}
+}
+
 /**
  * __set_legacy_tf - Calculates the legacy transfer function
  * @func: transfer function
@@ -1159,7 +1185,7 @@  int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
 {
 	struct amdgpu_device *adev = drm_to_adev(crtc->base.state->dev);
 	struct dm_plane_state *dm_plane_state = to_dm_plane_state(plane_state);
-	struct drm_color_ctm *ctm = NULL;
+	struct drm_color_ctm2 *ctm = NULL;
 	struct dc_color_caps *color_caps = NULL;
 	bool has_crtc_cm_degamma;
 	int ret;
@@ -1213,7 +1239,7 @@  int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
 
 	/* Setup CRTC CTM. */
 	if (dm_plane_state->ctm) {
-		ctm = (struct drm_color_ctm *)dm_plane_state->ctm->data;
+		ctm = (struct drm_color_ctm2 *)dm_plane_state->ctm->data;
 
 		/*
 		 * So far, if we have both plane and CRTC CTM, plane CTM takes
@@ -1224,7 +1250,7 @@  int amdgpu_dm_update_plane_color_mgmt(struct dm_crtc_state *crtc,
 		 * provide support for both DPP and MPC matrix at the same
 		 * time.
 		 */
-		__drm_ctm_to_dc_matrix(ctm, dc_plane_state->gamut_remap_matrix.matrix);
+		__drm_ctm2_to_dc_matrix(ctm, dc_plane_state->gamut_remap_matrix.matrix);
 
 		dc_plane_state->gamut_remap_matrix.enable_remap = true;
 		dc_plane_state->input_csc_color_matrix.enable_adjustment = false;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
index 0b1081c690cb..27962a3d30f5 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_plane.c
@@ -1543,7 +1543,7 @@  dm_atomic_plane_set_property(struct drm_plane *plane,
 		ret = drm_property_replace_blob_from_id(plane->dev,
 							&dm_plane_state->ctm,
 							val,
-							sizeof(struct drm_color_ctm), -1,
+							sizeof(struct drm_color_ctm2), -1,
 							&replaced);
 		dm_plane_state->base.color_mgmt_changed |= replaced;
 		return ret;
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 46becedf5b2f..402288133e4c 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -838,6 +838,14 @@  struct drm_color_ctm {
 	__u64 matrix[9];
 };
 
+struct drm_color_ctm2 {
+	/*
+	 * Conversion matrix 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