From: Melissa Wen <mwen@igalia.com>
To: airlied@gmail.com, maarten.lankhorst@linux.intel.com,
mripard@kernel.org, simona@ffwll.ch, tzimmermann@suse.de
Cc: Alex Hung <alex.hung@amd.com>, Simon Ser <contact@emersion.fr>,
Uma Shankar <uma.shankar@intel.com>,
Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>,
Xaver Hugl <xaver.hugl@kde.org>,
Pekka Paalanen <pekka.paalanen@collabora.com>,
Louis Chauvet <louis.chauvet@bootlin.com>,
Matthew Schwartz <matthew.schwartz@linux.dev>,
John Harrison <John.Harrison@Igalia.com>,
Rodrigo Siqueira <siqueira@igalia.com>,
amd-gfx@lists.freedesktop.org, kernel-dev@igalia.com,
Rob Clark <robin.clark@oss.qualcomm.com>,
Dmitry Baryshkov <lumag@kernel.org>,
Abhinav Kumar <abhinav.kumar@linux.dev>,
Jessica Zhang <jesszhan0024@gmail.com>,
Sean Paul <sean@poorly.run>,
Marijn Suijten <marijn.suijten@somainline.org>,
linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
intel-xe@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
dri-devel@lists.freedesktop.org
Subject: [PATCH 3/3] drm/atomic: reject colorop update from inactive color pipeline
Date: Tue, 26 May 2026 16:17:10 +0200 [thread overview]
Message-ID: <20260526142940.504911-4-mwen@igalia.com> (raw)
In-Reply-To: <20260526142940.504911-1-mwen@igalia.com>
Only allow updates on colorops that are part of an active pipeline, i.e.
check if a colorop belongs to the color pipeline of a plane in its
current, new or old state. If not, reject the state change of this
inactive colorop. Performing this check later in drm_atomic_check_only()
to remove the ordering dependency that would exist if done at the time
of colorop property setting. Userspace is allowed to change colorops of
an active color pipeline, or when activating or deactivating its
pipeline in the same commit. However, changes in inactive color pipeline
is not allowed.
Suggested-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Signed-off-by: Melissa Wen <mwen@igalia.com>
---
drivers/gpu/drm/drm_atomic.c | 59 ++++++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 4fb3a23e862a..a0549435954b 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -865,6 +865,54 @@ drm_atomic_add_pipeline_colorops(struct drm_atomic_commit *state,
return 0;
}
+/**
+ * drm_atomic_colorop_check - check new colorop state
+ * @new_colorop_state: new colorop state to check
+ *
+ * Ensure that the colorop in @new_colorop_state belongs to an active color
+ * pipeline, i.e. it's in the chain of colorops set to the color_pipeline
+ * property of current, old or new plane state.
+ *
+ * Returns: 0 on success, -EINVAL otherwise.
+ */
+static int drm_atomic_colorop_check(const struct drm_colorop_state *new_colorop_state)
+{
+ struct drm_atomic_commit *state = new_colorop_state->state;
+ struct drm_plane *plane = new_colorop_state->colorop->plane;
+ struct drm_plane_state *new_plane_state, *old_plane_state;
+ struct drm_colorop *colorop;
+
+ new_plane_state = drm_atomic_get_new_plane_state(state, plane);
+ old_plane_state = drm_atomic_get_old_plane_state(state, plane);
+
+ /* No changes in the plane state. Check current-committed plane state */
+ if (!new_plane_state) {
+ for (colorop = plane->state->color_pipeline; colorop; colorop = colorop->next)
+ if (colorop == new_colorop_state->colorop)
+ return 0;
+ return -EINVAL;
+ }
+
+ if (WARN_ON(!old_plane_state)) return -EINVAL;
+
+ /* Check if the colorop is active in the new plane state */
+ for (colorop = new_plane_state->color_pipeline; colorop; colorop = colorop->next)
+ if (colorop == new_colorop_state->colorop)
+ return 0;
+
+ /* Same color pipeline as new; no point walking old. Colorop isn't active */
+ if (new_plane_state->color_pipeline == old_plane_state->color_pipeline)
+ return -EINVAL;
+
+ /* Check if the colorop was active in the old plane state */
+ for (colorop = old_plane_state->color_pipeline; colorop; colorop = colorop->next)
+ if (colorop == new_colorop_state->colorop)
+ return 0;
+
+ /* Colorop is not part of an active color pipeline. */
+ return -EINVAL;
+}
+
static void drm_atomic_colorop_print_state(struct drm_printer *p,
const struct drm_colorop_state *state)
{
@@ -1714,6 +1762,8 @@ int drm_atomic_check_only(struct drm_atomic_commit *state)
struct drm_plane *plane;
struct drm_plane_state *old_plane_state;
struct drm_plane_state *new_plane_state;
+ struct drm_colorop *colorop;
+ struct drm_colorop_state *new_colorop_state;
struct drm_crtc *crtc;
struct drm_crtc_state *old_crtc_state;
struct drm_crtc_state *new_crtc_state;
@@ -1730,6 +1780,15 @@ int drm_atomic_check_only(struct drm_atomic_commit *state)
requested_crtc |= drm_crtc_mask(crtc);
}
+ for_each_new_colorop_in_state(state, colorop, new_colorop_state, i) {
+ ret = drm_atomic_colorop_check(new_colorop_state);
+ if (ret) {
+ drm_dbg_atomic(dev, "[COLOROP:%d:%d] is not part of an active color pipeline.\n",
+ colorop->base.id, colorop->type);
+ return ret;
+ }
+ }
+
for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
ret = drm_atomic_plane_check(old_plane_state, new_plane_state);
if (ret) {
--
2.53.0
next prev parent reply other threads:[~2026-05-26 14:30 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-26 14:17 [PATCH 0/3] don't allow changes to inactive colorops Melissa Wen
2026-05-26 14:17 ` [PATCH 1/3] drm/atomic: only add states of active or transient active colorops Melissa Wen
2026-05-26 23:02 ` Alex Hung
2026-05-27 4:45 ` Claude review: " Claude Code Review Bot
2026-05-26 14:17 ` [PATCH 2/3] drm/atomic: duplicate state of all colorops Melissa Wen
2026-05-27 4:45 ` Claude review: " Claude Code Review Bot
2026-05-26 14:17 ` Melissa Wen [this message]
2026-05-26 23:04 ` [PATCH 3/3] drm/atomic: reject colorop update from inactive color pipeline Alex Hung
2026-05-27 4:45 ` Claude review: " Claude Code Review Bot
2026-05-27 4:45 ` Claude review: don't allow changes to inactive colorops Claude Code Review Bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260526142940.504911-4-mwen@igalia.com \
--to=mwen@igalia.com \
--cc=John.Harrison@Igalia.com \
--cc=abhinav.kumar@linux.dev \
--cc=airlied@gmail.com \
--cc=alex.hung@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=chaitanya.kumar.borah@intel.com \
--cc=contact@emersion.fr \
--cc=dri-devel@lists.freedesktop.org \
--cc=freedreno@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=intel-xe@lists.freedesktop.org \
--cc=jesszhan0024@gmail.com \
--cc=kernel-dev@igalia.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=louis.chauvet@bootlin.com \
--cc=lumag@kernel.org \
--cc=maarten.lankhorst@linux.intel.com \
--cc=marijn.suijten@somainline.org \
--cc=matthew.schwartz@linux.dev \
--cc=mripard@kernel.org \
--cc=pekka.paalanen@collabora.com \
--cc=robin.clark@oss.qualcomm.com \
--cc=sean@poorly.run \
--cc=simona@ffwll.ch \
--cc=siqueira@igalia.com \
--cc=tzimmermann@suse.de \
--cc=uma.shankar@intel.com \
--cc=xaver.hugl@kde.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox