public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
From: Harry Wentland <harry.wentland@amd.com>
To: <dri-devel@lists.freedesktop.org>, <amd-gfx@lists.freedesktop.org>
Cc: Harry Wentland <harry.wentland@amd.com>,
	Simon Ser <contact@emersion.fr>, Alex Hung <alex.hung@amd.com>,
	Daniel Stone <daniels@collabora.com>,
	"Melissa Wen" <mwen@igalia.com>,
	Sebastian Wick <sebastian.wick@redhat.com>,
	"Uma Shankar" <uma.shankar@intel.com>,
	Ville Syrjälä <ville.syrjala@linux.intel.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Jani Nikula <jani.nikula@intel.com>,
	Louis Chauvet <louis.chauvet@bootlin.com>,
	Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>,
	<stable@vger.kernel.org>
Subject: [PATCH] drm/colorop: Fix blob property reference tracking in state lifecycle
Date: Thu, 12 Mar 2026 16:41:45 -0400	[thread overview]
Message-ID: <20260312204145.829714-1-harry.wentland@amd.com> (raw)

The colorop state blob property handling had memory leaks during state
duplication, destruction, and reset operations. The implementation
failed to follow the established pattern from drm_crtc's handling of
DEGAMMA/GAMMA blob properties.

Issues fixed:
- drm_colorop_atomic_destroy_state() was freeing state memory without
  releasing the blob reference, causing a leak
- drm_colorop_reset() was directly freeing old state with kfree()
  instead of properly destroying it, leaking blob references
- drm_colorop_cleanup() had duplicate blob cleanup code

Changes:
- Add __drm_atomic_helper_colorop_destroy_state() helper to properly
  release blob references before freeing state memory
- Update drm_colorop_atomic_destroy_state() to call the helper
- Fix drm_colorop_reset() to use drm_colorop_atomic_destroy_state()
  for proper cleanup of old state
- Simplify drm_colorop_cleanup() to use the common destruction path

This matches the well-tested pattern used by drm_crtc since 2016 and
ensures proper reference counting throughout the state lifecycle.

Co-developed by Claude Sonnet 4.5.

Fixes: cfc27680ee20 ("drm/colorop: Introduce new drm_colorop mode object")
Cc: Simon Ser <contact@emersion.fr>
Cc: Alex Hung <alex.hung@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Daniel Stone <daniels@collabora.com>
Cc: Melissa Wen <mwen@igalia.com>
Cc: Sebastian Wick <sebastian.wick@redhat.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Louis Chauvet <louis.chauvet@bootlin.com>
Cc: Chaitanya Kumar Borah <chaitanya.kumar.borah@intel.com>
Cc: <stable@vger.kernel.org> #v6.19+
Signed-off-by: Harry Wentland <harry.wentland@amd.com>
---
 drivers/gpu/drm/drm_colorop.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_colorop.c b/drivers/gpu/drm/drm_colorop.c
index f421c623b3f0..647cf881f413 100644
--- a/drivers/gpu/drm/drm_colorop.c
+++ b/drivers/gpu/drm/drm_colorop.c
@@ -171,12 +171,8 @@ void drm_colorop_cleanup(struct drm_colorop *colorop)
 	list_del(&colorop->head);
 	config->num_colorop--;
 
-	if (colorop->state && colorop->state->data) {
-		drm_property_blob_put(colorop->state->data);
-		colorop->state->data = NULL;
-	}
-
-	kfree(colorop->state);
+	if (colorop->state)
+		drm_colorop_atomic_destroy_state(colorop, colorop->state);
 }
 EXPORT_SYMBOL(drm_colorop_cleanup);
 
@@ -485,9 +481,23 @@ drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colorop)
 	return state;
 }
 
+/**
+ * __drm_atomic_helper_colorop_destroy_state - release colorop state
+ * @state: colorop state object to release
+ *
+ * Releases all resources stored in the colorop state without actually freeing
+ * the memory of the colorop state. This is useful for drivers that subclass the
+ * colorop state.
+ */
+static void __drm_atomic_helper_colorop_destroy_state(struct drm_colorop_state *state)
+{
+	drm_property_blob_put(state->data);
+}
+
 void drm_colorop_atomic_destroy_state(struct drm_colorop *colorop,
 				      struct drm_colorop_state *state)
 {
+	__drm_atomic_helper_colorop_destroy_state(state);
 	kfree(state);
 }
 
@@ -538,7 +548,9 @@ static void __drm_colorop_reset(struct drm_colorop *colorop,
 
 void drm_colorop_reset(struct drm_colorop *colorop)
 {
-	kfree(colorop->state);
+	if (colorop->state)
+		drm_colorop_atomic_destroy_state(colorop, colorop->state);
+
 	colorop->state = kzalloc_obj(*colorop->state);
 
 	if (colorop->state)
-- 
2.53.0


             reply	other threads:[~2026-03-12 20:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-12 20:41 Harry Wentland [this message]
2026-03-12 21:14 ` [PATCH] drm/colorop: Fix blob property reference tracking in state lifecycle Alex Hung
2026-03-13  3:48 ` Claude review: " Claude Code Review Bot
2026-03-13  3:48 ` 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=20260312204145.829714-1-harry.wentland@amd.com \
    --to=harry.wentland@amd.com \
    --cc=alex.hung@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=chaitanya.kumar.borah@intel.com \
    --cc=contact@emersion.fr \
    --cc=daniels@collabora.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --cc=louis.chauvet@bootlin.com \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mwen@igalia.com \
    --cc=sebastian.wick@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=uma.shankar@intel.com \
    --cc=ville.syrjala@linux.intel.com \
    /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