* [PATCH v7 0/4] drm/atomic: track individual colorop updates
@ 2026-05-25 9:49 Melissa Wen
2026-05-25 9:49 ` [PATCH v7 1/4] drm/colorop: Remove read-only comments from interpolation fields Melissa Wen
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Melissa Wen @ 2026-05-25 9:49 UTC (permalink / raw)
To: airlied, alexander.deucher, christian.koenig, harry.wentland,
maarten.lankhorst, mripard, simona, siqueira, sunpeng.li,
tzimmermann
Cc: Alex Hung, Simon Ser, Uma Shankar, Chaitanya Kumar Borah,
Xaver Hugl, Pekka Paalanen, Louis Chauvet, Matthew Schwartz,
amd-gfx, kernel-dev, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, linux-arm-msm,
freedreno, intel-xe, intel-gfx, dri-devel
This is a partial of [1], only with patches related to individual
colorop update tracking. I.e., I'm detaching from here fixes regarding
attempts of changing colorops that are not part of an active color
pipeline, or in the transition between active and inactive color
pipelines.
This series focus on tracking updates for each individual color
operation, allowing the driver to react accordingly.
- Patches 1 and 2 make lut1d_interpolation and lut3d_interpolation
colorops correctly behave as mutable, handling their changes via
drm_colorop_state.
- Patches 3 and 4 track colorop updates of a given plane color
pipeline by setting plane `color_mgmt_changed` flag, similar to what
is done for tracking CRTC color mgmt property changes with CRTC
`color_mgmt_changed` flag. The flag also tracks when a different color
pipeline is set to a given plane, but doesn't consider as a change
when the same color pipeline value is set to the plane COLOR_PIPELINE
prop. That way, the driver can react accordingly and update their
color blocks. As interpolation properties become mutable, they are
also tracked here.
It also fixes shaper/3D LUT updates when changing night mode settings on
gamescope with a custom branch that supports `COLOR_PIPELINE`:
- https://github.com/ValveSoftware/gamescope/pull/2113
v1: https://lore.kernel.org/dri-devel/20260318162348.299807-1-mwen@igalia.com/
Changes:
- include linux types for function's bool return type (kernel bot on MSM
driver)
- add Harry's r-b tags
v2: https://lore.kernel.org/dri-devel/20260323131942.494217-1-mwen@igalia.com/
Changes:
- [NEW] two patches to only consider colorop updates from active color
pipelines (Chaitanya)
- [NEW] make lut interpolation properties mutable + Alex H patch for
kernel docs
- track lut(1/3)d_interpolation updates (Chaitanya)
- rebase changes according to new patches
v3: https://lore.kernel.org/dri-devel/20260403135909.214378-1-mwen@igalia.com/
Changes: rebase on drm-misc-next
v4: https://lore.kernel.org/dri-devel/20260501132527.522320-1-mwen@igalia.com/
Changes: fix kernel doc (kernel bot)
v5: https://lore.kernel.org/dri-devel/20260506192633.16066-1-mwen@igalia.com/
Changes:
- rebase on drm-misc-next
- fix kernel-doc and correctly reword (atomic) state to plane_state (Chaitanya)
- reject inactive colorop updates in atomic check time, instead of
during property's setup, to avoid ordering dependency as pointed out by Chaitanya
- use `|= replaced` for consistency (Chaitanya)
- add Chaitanya's r-b tags to patches 1,3-5
[1] v6: https://lore.kernel.org/dri-devel/20260519211111.228303-1-mwen@igalia.com/
Changes:
- detach patches that implement individual tracking from those related
to inactive colorop updates.
Alex Hung (1):
drm/colorop: Remove read-only comments from interpolation fields
Melissa Wen (3):
drm/colorop: make lut(1/3)d_interpolation mutable
drm/atomic: track individual colorop updates
drm/amd/display: use plane color_mgmt_changed to track colorop changes
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +-
drivers/gpu/drm/drm_atomic.c | 4 +-
drivers/gpu/drm/drm_atomic_uapi.c | 68 +++++++++++++++----
drivers/gpu/drm/drm_colorop.c | 16 ++++-
include/drm/drm_atomic_uapi.h | 4 +-
include/drm/drm_colorop.h | 34 +++++-----
6 files changed, 93 insertions(+), 39 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v7 1/4] drm/colorop: Remove read-only comments from interpolation fields
2026-05-25 9:49 [PATCH v7 0/4] drm/atomic: track individual colorop updates Melissa Wen
@ 2026-05-25 9:49 ` Melissa Wen
2026-05-25 21:18 ` Claude review: " Claude Code Review Bot
2026-05-25 9:49 ` [PATCH v7 2/4] drm/colorop: make lut(1/3)d_interpolation mutable Melissa Wen
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Melissa Wen @ 2026-05-25 9:49 UTC (permalink / raw)
To: airlied, alexander.deucher, christian.koenig, harry.wentland,
maarten.lankhorst, mripard, simona, siqueira, sunpeng.li,
tzimmermann
Cc: Alex Hung, Simon Ser, Uma Shankar, Chaitanya Kumar Borah,
Xaver Hugl, Pekka Paalanen, Louis Chauvet, Matthew Schwartz,
amd-gfx, kernel-dev, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, linux-arm-msm,
freedreno, intel-xe, intel-gfx, dri-devel
From: Alex Hung <alex.hung@amd.com>
The lut1d_interpolation and lut3d_interpolation fields and their
associated properties were marked as read-only, but userspace
can set them via drm_atomic_colorop_set_property().
Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Signed-off-by: Alex Hung <alex.hung@amd.com>
---
include/drm/drm_colorop.h | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index c873199c60da..53a2148082d5 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -309,7 +309,6 @@ struct drm_colorop {
/**
* @lut1d_interpolation:
*
- * Read-only
* Interpolation for DRM_COLOROP_1D_LUT
*/
enum drm_colorop_lut1d_interpolation_type lut1d_interpolation;
@@ -317,7 +316,6 @@ struct drm_colorop {
/**
* @lut3d_interpolation:
*
- * Read-only
* Interpolation for DRM_COLOROP_3D_LUT
*/
enum drm_colorop_lut3d_interpolation_type lut3d_interpolation;
@@ -325,7 +323,7 @@ struct drm_colorop {
/**
* @lut1d_interpolation_property:
*
- * Read-only property for DRM_COLOROP_1D_LUT interpolation
+ * Property for DRM_COLOROP_1D_LUT interpolation
*/
struct drm_property *lut1d_interpolation_property;
@@ -353,7 +351,7 @@ struct drm_colorop {
/**
* @lut3d_interpolation_property:
*
- * Read-only property for DRM_COLOROP_3D_LUT interpolation
+ * Property for DRM_COLOROP_3D_LUT interpolation
*/
struct drm_property *lut3d_interpolation_property;
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v7 2/4] drm/colorop: make lut(1/3)d_interpolation mutable
2026-05-25 9:49 [PATCH v7 0/4] drm/atomic: track individual colorop updates Melissa Wen
2026-05-25 9:49 ` [PATCH v7 1/4] drm/colorop: Remove read-only comments from interpolation fields Melissa Wen
@ 2026-05-25 9:49 ` Melissa Wen
2026-05-25 21:18 ` Claude review: " Claude Code Review Bot
2026-05-25 9:50 ` [PATCH v7 3/4] drm/atomic: track individual colorop updates Melissa Wen
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Melissa Wen @ 2026-05-25 9:49 UTC (permalink / raw)
To: airlied, alexander.deucher, christian.koenig, harry.wentland,
maarten.lankhorst, mripard, simona, siqueira, sunpeng.li,
tzimmermann
Cc: Alex Hung, Simon Ser, Uma Shankar, Chaitanya Kumar Borah,
Xaver Hugl, Pekka Paalanen, Louis Chauvet, Matthew Schwartz,
amd-gfx, kernel-dev, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, linux-arm-msm,
freedreno, intel-xe, intel-gfx, dri-devel
As it's not immutable anymore, any changes should be handled by
drm_colorop_state. Move their enum and make it correctly behaves as
mutable.
Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
drivers/gpu/drm/drm_atomic.c | 4 ++--
drivers/gpu/drm/drm_atomic_uapi.c | 8 ++++----
drivers/gpu/drm/drm_colorop.c | 16 ++++++++++++++--
include/drm/drm_colorop.h | 28 ++++++++++++++--------------
4 files changed, 34 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 170de30c28ae..080aec5a9774 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -830,7 +830,7 @@ static void drm_atomic_colorop_print_state(struct drm_printer *p,
case DRM_COLOROP_1D_LUT:
drm_printf_indent(p, 1, "size=%d\n", colorop->size);
drm_printf_indent(p, 1, "interpolation=%s\n",
- drm_get_colorop_lut1d_interpolation_name(colorop->lut1d_interpolation));
+ drm_get_colorop_lut1d_interpolation_name(state->lut1d_interpolation));
drm_printf_indent(p, 1, "data blob id=%d\n", state->data ? state->data->base.id : 0);
break;
case DRM_COLOROP_CTM_3X4:
@@ -842,7 +842,7 @@ static void drm_atomic_colorop_print_state(struct drm_printer *p,
case DRM_COLOROP_3D_LUT:
drm_printf_indent(p, 1, "size=%d\n", colorop->size);
drm_printf_indent(p, 1, "interpolation=%s\n",
- drm_get_colorop_lut3d_interpolation_name(colorop->lut3d_interpolation));
+ drm_get_colorop_lut3d_interpolation_name(state->lut3d_interpolation));
drm_printf_indent(p, 1, "data blob id=%d\n", state->data ? state->data->base.id : 0);
break;
default:
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 6441b55cc274..78423905051e 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -751,13 +751,13 @@ static int drm_atomic_colorop_set_property(struct drm_colorop *colorop,
if (property == colorop->bypass_property) {
state->bypass = val;
} else if (property == colorop->lut1d_interpolation_property) {
- colorop->lut1d_interpolation = val;
+ state->lut1d_interpolation = val;
} else if (property == colorop->curve_1d_type_property) {
state->curve_1d_type = val;
} else if (property == colorop->multiplier_property) {
state->multiplier = val;
} else if (property == colorop->lut3d_interpolation_property) {
- colorop->lut3d_interpolation = val;
+ state->lut3d_interpolation = val;
} else if (property == colorop->data_property) {
return drm_atomic_color_set_data_property(colorop, state,
property, val);
@@ -782,7 +782,7 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,
else if (property == colorop->bypass_property)
*val = state->bypass;
else if (property == colorop->lut1d_interpolation_property)
- *val = colorop->lut1d_interpolation;
+ *val = state->lut1d_interpolation;
else if (property == colorop->curve_1d_type_property)
*val = state->curve_1d_type;
else if (property == colorop->multiplier_property)
@@ -790,7 +790,7 @@ drm_atomic_colorop_get_property(struct drm_colorop *colorop,
else if (property == colorop->size_property)
*val = colorop->size;
else if (property == colorop->lut3d_interpolation_property)
- *val = colorop->lut3d_interpolation;
+ *val = state->lut3d_interpolation;
else if (property == colorop->data_property)
*val = (state->data) ? state->data->base.id : 0;
else
diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index 764d12060666..65fec53f70fa 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -342,7 +342,6 @@ int drm_plane_colorop_curve_1d_lut_init(struct drm_device *dev, struct drm_color
colorop->lut1d_interpolation_property = prop;
drm_object_attach_property(&colorop->base, prop, interpolation);
- colorop->lut1d_interpolation = interpolation;
/* data */
ret = drm_colorop_create_data_prop(dev, colorop);
@@ -442,7 +441,6 @@ int drm_plane_colorop_3dlut_init(struct drm_device *dev, struct drm_colorop *col
colorop->lut3d_interpolation_property = prop;
drm_object_attach_property(&colorop->base, prop, interpolation);
- colorop->lut3d_interpolation = interpolation;
/* data */
ret = drm_colorop_create_data_prop(dev, colorop);
@@ -521,6 +519,20 @@ static void __drm_colorop_state_reset(struct drm_colorop_state *colorop_state,
&val))
colorop_state->curve_1d_type = val;
}
+
+ if (colorop->lut1d_interpolation_property) {
+ if (!drm_object_property_get_default_value(&colorop->base,
+ colorop->lut1d_interpolation_property,
+ &val))
+ colorop_state->lut1d_interpolation = val;
+ }
+
+ if (colorop->lut3d_interpolation_property) {
+ if (!drm_object_property_get_default_value(&colorop->base,
+ colorop->lut3d_interpolation_property,
+ &val))
+ colorop_state->lut3d_interpolation = val;
+ }
}
/**
diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
index 53a2148082d5..d08a6a8a8392 100644
--- a/include/drm/drm_colorop.h
+++ b/include/drm/drm_colorop.h
@@ -183,6 +183,20 @@ struct drm_colorop_state {
*/
struct drm_property_blob *data;
+ /**
+ * @lut1d_interpolation:
+ *
+ * Interpolation for DRM_COLOROP_1D_LUT
+ */
+ enum drm_colorop_lut1d_interpolation_type lut1d_interpolation;
+
+ /**
+ * @lut3d_interpolation:
+ *
+ * Interpolation for DRM_COLOROP_3D_LUT
+ */
+ enum drm_colorop_lut3d_interpolation_type lut3d_interpolation;
+
/** @state: backpointer to global drm_atomic_commit */
struct drm_atomic_commit *state;
};
@@ -306,20 +320,6 @@ struct drm_colorop {
*/
uint32_t size;
- /**
- * @lut1d_interpolation:
- *
- * Interpolation for DRM_COLOROP_1D_LUT
- */
- enum drm_colorop_lut1d_interpolation_type lut1d_interpolation;
-
- /**
- * @lut3d_interpolation:
- *
- * Interpolation for DRM_COLOROP_3D_LUT
- */
- enum drm_colorop_lut3d_interpolation_type lut3d_interpolation;
-
/**
* @lut1d_interpolation_property:
*
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v7 3/4] drm/atomic: track individual colorop updates
2026-05-25 9:49 [PATCH v7 0/4] drm/atomic: track individual colorop updates Melissa Wen
2026-05-25 9:49 ` [PATCH v7 1/4] drm/colorop: Remove read-only comments from interpolation fields Melissa Wen
2026-05-25 9:49 ` [PATCH v7 2/4] drm/colorop: make lut(1/3)d_interpolation mutable Melissa Wen
@ 2026-05-25 9:50 ` Melissa Wen
2026-05-25 21:18 ` Claude review: " Claude Code Review Bot
2026-05-25 9:50 ` [PATCH v7 4/4] drm/amd/display: use plane color_mgmt_changed to track colorop changes Melissa Wen
2026-05-25 21:18 ` Claude review: drm/atomic: track individual colorop updates Claude Code Review Bot
4 siblings, 1 reply; 10+ messages in thread
From: Melissa Wen @ 2026-05-25 9:50 UTC (permalink / raw)
To: airlied, alexander.deucher, christian.koenig, harry.wentland,
maarten.lankhorst, mripard, simona, siqueira, sunpeng.li,
tzimmermann
Cc: Alex Hung, Simon Ser, Uma Shankar, Chaitanya Kumar Borah,
Xaver Hugl, Pekka Paalanen, Louis Chauvet, Matthew Schwartz,
amd-gfx, kernel-dev, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, linux-arm-msm,
freedreno, intel-xe, intel-gfx, dri-devel
As we do for CRTC color mgmt properties, use color_mgmt_changed flag to
track any value changes in the color pipeline of a given plane, so that
drivers can update color blocks as soon as plane color pipeline or
individual colorop values change. Since we're here, only announce and
track changes to plane COLOR_PIPELINE prop if its value is actually
changing.
Reviewed-by: Harry Wentland <harry.wentland@amd.com> #v1
Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
v3: track lut(1/3)d_interpolation property updates (Chaitanya)
v6: use `|= replaced` for consistency (Chaitanya)
---
drivers/gpu/drm/drm_atomic_uapi.c | 64 ++++++++++++++++++++++++-------
include/drm/drm_atomic_uapi.h | 4 +-
2 files changed, 54 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index 78423905051e..e997917819e8 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -265,13 +265,19 @@ EXPORT_SYMBOL(drm_atomic_set_fb_for_plane);
*
* Helper function to select the color pipeline on a plane by setting
* it to the first drm_colorop element of the pipeline.
+ *
+ * Return: true if plane color pipeline value changed, false otherwise.
*/
-void
+bool
drm_atomic_set_colorop_for_plane(struct drm_plane_state *plane_state,
struct drm_colorop *colorop)
{
struct drm_plane *plane = plane_state->plane;
+ /* Color pipeline didn't change */
+ if (plane_state->color_pipeline == colorop)
+ return false;
+
if (colorop)
drm_dbg_atomic(plane->dev,
"Set [COLOROP:%d] for [PLANE:%d:%s] state %p\n",
@@ -283,6 +289,8 @@ drm_atomic_set_colorop_for_plane(struct drm_plane_state *plane_state,
plane->base.id, plane->name, plane_state);
plane_state->color_pipeline = colorop;
+
+ return true;
}
EXPORT_SYMBOL(drm_atomic_set_colorop_for_plane);
@@ -604,7 +612,7 @@ static int drm_atomic_plane_set_property(struct drm_plane *plane,
if (val && !colorop)
return -EACCES;
- drm_atomic_set_colorop_for_plane(state, colorop);
+ state->color_mgmt_changed |= drm_atomic_set_colorop_for_plane(state, colorop);
} else if (property == config->prop_fb_damage_clips) {
ret = drm_property_replace_blob_from_id(dev,
&state->fb_damage_clips,
@@ -713,11 +721,11 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
static int drm_atomic_color_set_data_property(struct drm_colorop *colorop,
struct drm_colorop_state *state,
struct drm_property *property,
- uint64_t val)
+ uint64_t val,
+ bool *replaced)
{
ssize_t elem_size = -1;
ssize_t size = -1;
- bool replaced = false;
switch (colorop->type) {
case DRM_COLOROP_1D_LUT:
@@ -739,28 +747,45 @@ static int drm_atomic_color_set_data_property(struct drm_colorop *colorop,
&state->data,
val,
-1, size, elem_size,
- &replaced);
+ replaced);
}
static int drm_atomic_colorop_set_property(struct drm_colorop *colorop,
struct drm_colorop_state *state,
struct drm_file *file_priv,
struct drm_property *property,
- uint64_t val)
+ uint64_t val,
+ bool *replaced)
{
if (property == colorop->bypass_property) {
- state->bypass = val;
+ if (state->bypass != val) {
+ state->bypass = val;
+ *replaced = true;
+ }
} else if (property == colorop->lut1d_interpolation_property) {
- state->lut1d_interpolation = val;
+ if (state->lut1d_interpolation != val) {
+ state->lut1d_interpolation = val;
+ *replaced = true;
+ }
} else if (property == colorop->curve_1d_type_property) {
- state->curve_1d_type = val;
+ if (state->curve_1d_type != val) {
+ state->curve_1d_type = val;
+ *replaced = true;
+ }
} else if (property == colorop->multiplier_property) {
- state->multiplier = val;
+ if (state->multiplier != val) {
+ state->multiplier = val;
+ *replaced = true;
+ }
} else if (property == colorop->lut3d_interpolation_property) {
- state->lut3d_interpolation = val;
+ if (state->lut3d_interpolation != val) {
+ state->lut3d_interpolation = val;
+ *replaced = true;
+ }
} else if (property == colorop->data_property) {
return drm_atomic_color_set_data_property(colorop, state,
- property, val);
+ property, val,
+ replaced);
} else {
drm_dbg_atomic(colorop->dev,
"[COLOROP:%d:%d] unknown property [PROP:%d:%s]\n",
@@ -1275,8 +1300,10 @@ int drm_atomic_set_property(struct drm_atomic_commit *state,
break;
}
case DRM_MODE_OBJECT_COLOROP: {
+ struct drm_plane_state *plane_state;
struct drm_colorop *colorop = obj_to_colorop(obj);
struct drm_colorop_state *colorop_state;
+ bool replaced = false;
colorop_state = drm_atomic_get_colorop_state(state, colorop);
if (IS_ERR(colorop_state)) {
@@ -1285,7 +1312,18 @@ int drm_atomic_set_property(struct drm_atomic_commit *state,
}
ret = drm_atomic_colorop_set_property(colorop, colorop_state,
- file_priv, prop, prop_value);
+ file_priv, prop, prop_value,
+ &replaced);
+ if (ret || !replaced)
+ break;
+
+ plane_state = drm_atomic_get_plane_state(state, colorop->plane);
+ if (IS_ERR(plane_state)) {
+ ret = PTR_ERR(plane_state);
+ break;
+ }
+ plane_state->color_mgmt_changed |= replaced;
+
break;
}
default:
diff --git a/include/drm/drm_atomic_uapi.h b/include/drm/drm_atomic_uapi.h
index 436315523326..4e7e78f711e2 100644
--- a/include/drm/drm_atomic_uapi.h
+++ b/include/drm/drm_atomic_uapi.h
@@ -29,6 +29,8 @@
#ifndef DRM_ATOMIC_UAPI_H_
#define DRM_ATOMIC_UAPI_H_
+#include <linux/types.h>
+
struct drm_crtc_state;
struct drm_display_mode;
struct drm_property_blob;
@@ -50,7 +52,7 @@ drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state,
struct drm_crtc *crtc);
void drm_atomic_set_fb_for_plane(struct drm_plane_state *plane_state,
struct drm_framebuffer *fb);
-void drm_atomic_set_colorop_for_plane(struct drm_plane_state *plane_state,
+bool drm_atomic_set_colorop_for_plane(struct drm_plane_state *plane_state,
struct drm_colorop *colorop);
int __must_check
drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v7 4/4] drm/amd/display: use plane color_mgmt_changed to track colorop changes
2026-05-25 9:49 [PATCH v7 0/4] drm/atomic: track individual colorop updates Melissa Wen
` (2 preceding siblings ...)
2026-05-25 9:50 ` [PATCH v7 3/4] drm/atomic: track individual colorop updates Melissa Wen
@ 2026-05-25 9:50 ` Melissa Wen
2026-05-25 21:18 ` Claude review: " Claude Code Review Bot
2026-05-25 21:18 ` Claude review: drm/atomic: track individual colorop updates Claude Code Review Bot
4 siblings, 1 reply; 10+ messages in thread
From: Melissa Wen @ 2026-05-25 9:50 UTC (permalink / raw)
To: airlied, alexander.deucher, christian.koenig, harry.wentland,
maarten.lankhorst, mripard, simona, siqueira, sunpeng.li,
tzimmermann
Cc: Alex Hung, Simon Ser, Uma Shankar, Chaitanya Kumar Borah,
Xaver Hugl, Pekka Paalanen, Louis Chauvet, Matthew Schwartz,
amd-gfx, kernel-dev, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, linux-arm-msm,
freedreno, intel-xe, intel-gfx, dri-devel
Ensure the driver tracks changes in any colorop property of a plane
color pipeline by using the same mechanism of CRTC color management and
update plane color blocks when any colorop property changes. It fixes an
issue observed on gamescope settings for night mode which is done via
shaper/3D-LUT updates.
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index d590f0df6abd..36425d9c2a67 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10198,7 +10198,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_commit *state,
continue;
bundle->surface_updates[planes_count].surface = dc_plane;
- if (new_pcrtc_state->color_mgmt_changed) {
+ if (new_pcrtc_state->color_mgmt_changed || new_plane_state->color_mgmt_changed) {
bundle->surface_updates[planes_count].gamma = &dc_plane->gamma_correction;
bundle->surface_updates[planes_count].in_transfer_func = &dc_plane->in_transfer_func;
bundle->surface_updates[planes_count].gamut_remap_matrix = &dc_plane->gamut_remap_matrix;
@@ -12024,6 +12024,10 @@ static bool should_reset_plane(struct drm_atomic_commit *state,
if (new_crtc_state->color_mgmt_changed)
return true;
+ /* Plane color pipeline or its colorop changes. */
+ if (new_plane_state->color_mgmt_changed)
+ return true;
+
/*
* On zpos change, planes need to be reordered by removing and re-adding
* them one by one to the dc state, in order of descending zpos.
--
2.53.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Claude review: drm/atomic: track individual colorop updates
2026-05-25 9:49 [PATCH v7 0/4] drm/atomic: track individual colorop updates Melissa Wen
` (3 preceding siblings ...)
2026-05-25 9:50 ` [PATCH v7 4/4] drm/amd/display: use plane color_mgmt_changed to track colorop changes Melissa Wen
@ 2026-05-25 21:18 ` Claude Code Review Bot
4 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 21:18 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: drm/atomic: track individual colorop updates
Author: Melissa Wen <mwen@igalia.com>
Patches: 5
Reviewed: 2026-05-26T07:18:46.332271
---
This is a well-structured v7 series that solves a real bug: colorop property changes (e.g., shaper/3D-LUT updates in gamescope night mode) were not being tracked, so the AMD display driver never reprogrammed its color blocks. The fix is clean and follows the existing pattern used for CRTC `color_mgmt_changed`.
The series logically progresses: patches 1-2 make lut interpolation properties mutable (moving them from `drm_colorop` to `drm_colorop_state`), patch 3 wires up change tracking in the DRM core, and patch 4 consumes the new flag in the AMD driver. The approach is correct and the code is well-reviewed (Harry Wentland, Chaitanya Kumar Borah R-b tags).
No correctness bugs found. One minor observation and one minor nit below.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 10+ messages in thread
* Claude review: drm/colorop: Remove read-only comments from interpolation fields
2026-05-25 9:49 ` [PATCH v7 1/4] drm/colorop: Remove read-only comments from interpolation fields Melissa Wen
@ 2026-05-25 21:18 ` Claude Code Review Bot
0 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 21:18 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Author:** Alex Hung
Straightforward documentation fix removing stale "Read-only" from kernel-doc comments for `lut1d_interpolation`, `lut3d_interpolation`, `lut1d_interpolation_property`, and `lut3d_interpolation_property`. This is prep work for patch 2 which makes them mutable.
No issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 10+ messages in thread
* Claude review: drm/colorop: make lut(1/3)d_interpolation mutable
2026-05-25 9:49 ` [PATCH v7 2/4] drm/colorop: make lut(1/3)d_interpolation mutable Melissa Wen
@ 2026-05-25 21:18 ` Claude Code Review Bot
0 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 21:18 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
Moves `lut1d_interpolation` and `lut3d_interpolation` from `struct drm_colorop` (immutable hardware object) to `struct drm_colorop_state` (per-commit mutable state). All the right places are updated:
- **State print** (`drm_atomic_colorop_print_state`): reads from `state->` instead of `colorop->`.
- **Set/get property**: reads/writes `state->` instead of `colorop->`.
- **Init functions** (`drm_plane_colorop_curve_1d_lut_init`, `drm_plane_colorop_3dlut_init`): removes assignment to `colorop->lut1d/3d_interpolation` since it's now state.
- **State reset** (`__drm_colorop_state_reset`): initializes new state fields from property defaults via `drm_object_property_get_default_value()`.
The state reset correctly uses the same pattern as the existing `curve_1d_type` initialization. No issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 10+ messages in thread
* Claude review: drm/atomic: track individual colorop updates
2026-05-25 9:50 ` [PATCH v7 3/4] drm/atomic: track individual colorop updates Melissa Wen
@ 2026-05-25 21:18 ` Claude Code Review Bot
0 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 21:18 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
This is the core patch. It sets `plane_state->color_mgmt_changed` whenever a colorop property value actually changes.
**Change to `drm_atomic_set_colorop_for_plane`**: Now returns `bool` indicating whether the color pipeline pointer actually changed. The early-out when `plane_state->color_pipeline == colorop` avoids unnecessary state dirtying when userspace re-sets the same pipeline. The only call site at line 615 uses the return value correctly:
```c
state->color_mgmt_changed |= drm_atomic_set_colorop_for_plane(state, colorop);
```
The function is `EXPORT_SYMBOL` so theoretically out-of-tree callers exist, but since the return value is ignorable (void→bool), this is ABI-safe.
**Change to `drm_atomic_colorop_set_property`**: Each property branch now does a compare-before-assign and sets `*replaced = true` only on actual value change. This is correct for all scalar properties (bypass, lut1d_interpolation, curve_1d_type, multiplier, lut3d_interpolation). The `data_property` case passes `replaced` through to `drm_atomic_color_set_data_property` which delegates to `drm_property_replace_blob_from_id` — that function already computes `replaced` correctly for blobs.
**Change to `drm_atomic_set_property` (COLOROP case)**: After detecting a change, it pulls in the plane state and sets `color_mgmt_changed`:
```c
ret = drm_atomic_colorop_set_property(colorop, colorop_state,
file_priv, prop, prop_value,
&replaced);
if (ret || !replaced)
break;
plane_state = drm_atomic_get_plane_state(state, colorop->plane);
...
plane_state->color_mgmt_changed |= replaced;
```
**Minor observation**: At line 1325, `plane_state->color_mgmt_changed |= replaced;` is guarded by `if (ret || !replaced)` above (line 1317), so `replaced` is always `true` when line 1325 is reached. The `|=` is harmless (and consistent with the style used elsewhere), but `= true` would be equivalent. This is not a bug — the existing code is fine for consistency and future-proofing.
**`#include <linux/types.h>`** added to `drm_atomic_uapi.h` for the `bool` return type — correct.
No issues.
---
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 10+ messages in thread
* Claude review: drm/amd/display: use plane color_mgmt_changed to track colorop changes
2026-05-25 9:50 ` [PATCH v7 4/4] drm/amd/display: use plane color_mgmt_changed to track colorop changes Melissa Wen
@ 2026-05-25 21:18 ` Claude Code Review Bot
0 siblings, 0 replies; 10+ messages in thread
From: Claude Code Review Bot @ 2026-05-25 21:18 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
The AMD driver consumer of the new flag. Two changes:
1. In `amdgpu_dm_commit_planes` (line ~10201):
```c
if (new_pcrtc_state->color_mgmt_changed || new_plane_state->color_mgmt_changed) {
```
Previously only CRTC color_mgmt_changed triggered updating the surface color blocks (gamma, transfer func, gamut remap, shaper LUT, 3D LUT). Now plane-level colorop changes also trigger this. This is the actual bug fix for gamescope night mode.
2. In `should_reset_plane` (line ~12027):
```c
if (new_plane_state->color_mgmt_changed)
return true;
```
Forces a plane reset (remove and re-add to DC state) when plane color management changes. This parallels the existing CRTC `color_mgmt_changed` check just above it.
**Question worth considering**: `should_reset_plane` returning `true` triggers a full plane remove/re-add in DC. For pure colorop property changes (e.g., just changing an interpolation mode), this may be heavier than necessary — the `amdgpu_dm_commit_planes` surface update path might be sufficient without a full reset. However, this matches the existing behavior for CRTC color management changes and is the safe approach. If performance is a concern for high-frequency colorop updates, this could be revisited later, but correctness is ensured.
No bugs found.
---
**Summary**: The series is correct and ready to merge. The change tracking logic properly detects actual value changes (not just property sets), the state management for interpolation fields follows established patterns, and the AMD driver correctly consumes the new flag. All patches have appropriate reviewed-by tags.
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-05-25 21:18 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-25 9:49 [PATCH v7 0/4] drm/atomic: track individual colorop updates Melissa Wen
2026-05-25 9:49 ` [PATCH v7 1/4] drm/colorop: Remove read-only comments from interpolation fields Melissa Wen
2026-05-25 21:18 ` Claude review: " Claude Code Review Bot
2026-05-25 9:49 ` [PATCH v7 2/4] drm/colorop: make lut(1/3)d_interpolation mutable Melissa Wen
2026-05-25 21:18 ` Claude review: " Claude Code Review Bot
2026-05-25 9:50 ` [PATCH v7 3/4] drm/atomic: track individual colorop updates Melissa Wen
2026-05-25 21:18 ` Claude review: " Claude Code Review Bot
2026-05-25 9:50 ` [PATCH v7 4/4] drm/amd/display: use plane color_mgmt_changed to track colorop changes Melissa Wen
2026-05-25 21:18 ` Claude review: " Claude Code Review Bot
2026-05-25 21:18 ` Claude review: drm/atomic: track individual colorop updates Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox