On 15/11/2025 00:02, Alex Hung wrote:
This adds support for a 3x4 color transformation matrix.
With this change the following IGT tests pass:
kms_colorop --run plane-XR30-XR30-ctm_3x4_50_desat
kms_colorop --run plane-XR30-XR30-ctm_3x4_overdrive
kms_colorop --run plane-XR30-XR30-ctm_3x4_oversaturate
kms_colorop --run plane-XR30-XR30-ctm_3x4_bt709_enc
kms_colorop --run plane-XR30-XR30-ctm_3x4_bt709_dec
The color pipeline now consists of the following colorops:
1. 1D curve colorop
2. 3x4 CTM
3. 1D curve colorop
4. 1D LUT
5. 1D curve colorop
6. 1D LUT
Signed-off-by: Alex Hung <[email protected]>
Signed-off-by: Harry Wentland <[email protected]>
Reviewed-by: Daniel Stone <[email protected]>
Reviewed-by: Melissa Wen <[email protected]>
---
v13:
- Remove redundant ternary null check for drm_color_ctm_3x4 blob (Coverity
Scan)
V10:
- Change %lu to %zu for sizeof() in drm_warn (kernel test robot)
- Remove redundant DRM_ERROR(...)
V9:
- Update function names by _plane_ (Chaitanya Kumar Borah)
v8:
- Return -EINVAL when drm_color_ctm_3x4's size mismatches (Leo Li)
v7:
- Change %lu to %zu for sizeof() in drm_warn
v6:
- fix warnings in dbg prints
.../amd/display/amdgpu_dm/amdgpu_dm_color.c | 52 +++++++++++++++++++
.../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 15 ++++++
2 files changed, 67 insertions(+)
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 b958f9c0a0c2..298f337f0eb4 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
@@ -1378,6 +1378,47 @@ __set_dm_plane_colorop_degamma(struct drm_plane_state
*plane_state,
return __set_colorop_in_tf_1d_curve(dc_plane_state, colorop_state);
}
+static int
+__set_dm_plane_colorop_3x4_matrix(struct drm_plane_state *plane_state,
+ struct dc_plane_state *dc_plane_state,
+ struct drm_colorop *colorop)
+{
+ struct drm_colorop *old_colorop;
+ struct drm_colorop_state *colorop_state = NULL, *new_colorop_state;
+ struct drm_atomic_state *state = plane_state->state;
+ const struct drm_device *dev = colorop->dev;
+ const struct drm_property_blob *blob;
+ struct drm_color_ctm_3x4 *ctm = NULL;
+ int i = 0;
+
+ /* 3x4 matrix */
+ old_colorop = colorop;
+ for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
+ if (new_colorop_state->colorop == old_colorop &&
+ new_colorop_state->colorop->type == DRM_COLOROP_CTM_3X4) {
+ colorop_state = new_colorop_state;
+ break;
+ }
+ }
+
+ if (colorop_state && !colorop_state->bypass && colorop->type ==
DRM_COLOROP_CTM_3X4) {
+ drm_dbg(dev, "3x4 matrix colorop with ID: %d\n",
colorop->base.id);
+ blob = colorop_state->data;
+ if (blob->length == sizeof(struct drm_color_ctm_3x4)) {
+ ctm = (struct drm_color_ctm_3x4 *) blob->data;
+ __drm_ctm_3x4_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;
+ } else {
+ drm_warn(dev, "blob->length (%zu) isn't equal to
drm_color_ctm_3x4 (%zu)\n",
+ blob->length, sizeof(struct
drm_color_ctm_3x4));
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
static int
__set_dm_plane_colorop_shaper(struct drm_plane_state *plane_state,
struct dc_plane_state *dc_plane_state,
@@ -1581,6 +1622,17 @@ amdgpu_dm_plane_set_colorop_properties(struct
drm_plane_state *plane_state,
if (ret)
return ret;
+ /* 3x4 matrix */
+ colorop = colorop->next;
+ if (!colorop) {
+ drm_dbg(dev, "no 3x4 matrix colorop found\n");
+ return -EINVAL;
+ }
+
+ ret = __set_dm_plane_colorop_3x4_matrix(plane_state, dc_plane_state,
colorop);
+ if (ret)
+ return ret;
+
/* 1D Curve & LUT - SHAPER TF & LUT */
colorop = colorop->next;
if (!colorop) {
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
index 4845f26e4a8a..f2be75b9b073 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_colorop.c
@@ -74,6 +74,21 @@ int amdgpu_dm_initialize_default_pipeline(struct drm_plane
*plane, struct drm_pr
i++;
+ /* 3x4 matrix */
+ ops[i] = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
+ if (!ops[i]) {
+ ret = -ENOMEM;
+ goto cleanup;
Does this cleanup path leak the list->name allocated a few lines above,
outside the diff? Kmemleak appears to think it can leak from somewhere
at least:
unreferenced object 0xffff8881143c1e00 (size 32):
comm "(udev-worker)", pid 588, jiffies 4294888494
hex dump (first 32 bytes):
43 6f 6c 6f 72 20 50 69 70 65 6c 69 6e 65 20 33 Color Pipeline 3
31 33 00 00 00 00 00 00 00 00 00 00 00 00 00 00 13..............
backtrace (crc a75af242):
__kmalloc_node_track_caller_noprof+0x525/0x890
kvasprintf+0xb6/0x130
kasprintf+0xb2/0xe0
amdgpu_dm_initialize_default_pipeline+0x1ce/0x840 [amdgpu]
dm_plane_init_colorops+0x19c/0x2e0 [amdgpu]
amdgpu_dm_plane_init+0x4c4/0xf10 [amdgpu]
initialize_plane+0xf1/0x280 [amdgpu]
amdgpu_dm_init+0x23d0/0x7450 [amdgpu]
dm_hw_init+0x3d/0x200 [amdgpu]
amdgpu_device_init+0x67cd/0x9b20 [amdgpu]
amdgpu_driver_load_kms+0x13/0xf0 [amdgpu]
amdgpu_pci_probe+0x437/0xf30 [amdgpu]
local_pci_probe+0xda/0x180
pci_device_probe+0x381/0x730
really_probe+0x1da/0x970
__driver_probe_device+0x18c/0x3e0
Could it be a false positive, or leaking later after the success path,
or some other path I am not sure since I am not at home in this code.
There is no error checking on list->name either, so I supppose the code
which can touch that can handle a NULL harmlessly?
Regards,
Tvrtko
+ }
+
+ ret = drm_plane_colorop_ctm_3x4_init(dev, ops[i], plane);
+ if (ret)
+ goto cleanup;
+
+ drm_colorop_set_next_property(ops[i-1], ops[i]);
+
+ i++;
+
/* 1D curve - SHAPER TF */
ops[i] = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);
if (!ops[i]) {
