[PATCH v2 34/34] drm/amd/display: Use 3x4 CTM for plane CTM
From: Joshua Ashton Signed-off-by: Joshua Ashton Signed-off-by: Melissa Wen --- .../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. +* +* 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 -- 2.40.1
Re: [PATCH v2 34/34] drm/amd/display: Use 3x4 CTM for plane CTM
On 2023-08-10 12:03, Melissa Wen wrote: > From: Joshua Ashton > > Signed-off-by: Joshua Ashton > Signed-off-by: Melissa Wen > --- > .../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