public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
From: Berkant Koc <me@berkoc.com>
To: dri-devel@lists.freedesktop.org
Cc: Thomas Zimmermann <tzimmermann@suse.de>,
	Jocelyn Falempe <jfalempe@redhat.com>,
	Dave Airlie <airlied@redhat.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Simona Vetter <simona@ffwll.ch>, Sam Ravnborg <sam@ravnborg.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH] drm/mgag200: reject pixel clocks outside PLL range
Date: Sun, 17 May 2026 15:03:09 +0200	[thread overview]
Message-ID: <28e476e896dafdd31518dc5360ab7766@berkoc.com> (raw)

Commit 877507bb954e ("drm/mgag200: Provide per-device callbacks for
PIXPLLC") split the PLL compute code into per-model source files. The
g200se variant kept the existing permitteddelta sanity check and
returns -EINVAL when no register combination matches the requested
pixel clock within tolerance, which lets atomic_check reject the mode.

The matching helpers in g200wb, g200eh, g200er, g200ev, g200eh3 and
g200ew3 omit that check. When the search loop finds no candidate, m,
n, p and s stay at their initial zero values and the function still
returns success. atomic_update then programs those zero values into
the PIXPLLCM/N/P registers, which yields a corrupted output clock on
the affected models.

Apply the same permitteddelta test (clock * 5 / 1000, matching the
g200se reference) in all six helpers so that out-of-range modes are
rejected during atomic_check instead of producing garbage register
writes during atomic_update.

Fixes: 877507bb954e ("drm/mgag200: Provide per-device callbacks for PIXPLLC")
Cc: stable@vger.kernel.org # v6.1+
Signed-off-by: Berkant Koc <me@berkoc.com>
---

Build-tested with CONFIG_DRM_MGAG200=m, allmodconfig defaults; no new
warnings. checkpatch.pl --no-tree clean (0 errors, 0 warnings).

 drivers/gpu/drm/mgag200/mgag200_g200eh.c  | 8 +++++++-
 drivers/gpu/drm/mgag200/mgag200_g200eh3.c | 8 +++++++-
 drivers/gpu/drm/mgag200/mgag200_g200er.c  | 8 +++++++-
 drivers/gpu/drm/mgag200/mgag200_g200ev.c  | 8 +++++++-
 drivers/gpu/drm/mgag200/mgag200_g200ew3.c | 8 +++++++-
 drivers/gpu/drm/mgag200/mgag200_g200wb.c  | 8 +++++++-
 6 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_g200eh.c b/drivers/gpu/drm/mgag200/mgag200_g200eh.c
index d2aa931f579d..2387ff87550a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200eh.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200eh.c
@@ -51,13 +51,14 @@ static int mgag200_g200eh_pixpllc_atomic_check(struct drm_crtc *crtc,
 	struct mgag200_crtc_state *new_mgag200_crtc_state = to_mgag200_crtc_state(new_crtc_state);
 	long clock = new_crtc_state->mode.clock;
 	struct mgag200_pll_values *pixpllc = &new_mgag200_crtc_state->pixpllc;
-	unsigned int delta, tmpdelta;
+	unsigned int delta, tmpdelta, permitteddelta;
 	unsigned int testp, testm, testn;
 	unsigned int p, m, n, s;
 	unsigned int computed;
 
 	m = n = p = s = 0;
 	delta = 0xffffffff;
+	permitteddelta = clock * 5 / 1000;
 
 	for (testp = 16; testp > 0; testp >>= 1) {
 		if (clock * testp > vcomax)
@@ -82,6 +83,11 @@ static int mgag200_g200eh_pixpllc_atomic_check(struct drm_crtc *crtc,
 		}
 	}
 
+	if (delta > permitteddelta) {
+		pr_warn("PLL delta too large\n");
+		return -EINVAL;
+	}
+
 	pixpllc->m = m;
 	pixpllc->n = n;
 	pixpllc->p = p;
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200eh3.c b/drivers/gpu/drm/mgag200/mgag200_g200eh3.c
index 7bea7a728f56..322e93982ea2 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200eh3.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200eh3.c
@@ -26,13 +26,14 @@ static int mgag200_g200eh3_pixpllc_atomic_check(struct drm_crtc *crtc,
 	struct mgag200_crtc_state *new_mgag200_crtc_state = to_mgag200_crtc_state(new_crtc_state);
 	long clock = new_crtc_state->mode.clock;
 	struct mgag200_pll_values *pixpllc = &new_mgag200_crtc_state->pixpllc;
-	unsigned int delta, tmpdelta;
+	unsigned int delta, tmpdelta, permitteddelta;
 	unsigned int testp, testm, testn;
 	unsigned int p, m, n, s;
 	unsigned int computed;
 
 	m = n = p = s = 0;
 	delta = 0xffffffff;
+	permitteddelta = clock * 5 / 1000;
 	testp = 0;
 
 	for (testm = 150; testm >= 6; testm--) {
@@ -59,6 +60,11 @@ static int mgag200_g200eh3_pixpllc_atomic_check(struct drm_crtc *crtc,
 			break;
 	}
 
+	if (delta > permitteddelta) {
+		pr_warn("PLL delta too large\n");
+		return -EINVAL;
+	}
+
 	pixpllc->m = m;
 	pixpllc->n = n;
 	pixpllc->p = p;
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200er.c b/drivers/gpu/drm/mgag200/mgag200_g200er.c
index 8fa8fe943abf..e180db21902b 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200er.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200er.c
@@ -69,13 +69,14 @@ static int mgag200_g200er_pixpllc_atomic_check(struct drm_crtc *crtc,
 	struct mgag200_crtc_state *new_mgag200_crtc_state = to_mgag200_crtc_state(new_crtc_state);
 	long clock = new_crtc_state->mode.clock;
 	struct mgag200_pll_values *pixpllc = &new_mgag200_crtc_state->pixpllc;
-	unsigned int delta, tmpdelta;
+	unsigned int delta, tmpdelta, permitteddelta;
 	int testr, testn, testm, testo;
 	unsigned int p, m, n, s;
 	unsigned int computed, vco;
 
 	m = n = p = s = 0;
 	delta = 0xffffffff;
+	permitteddelta = clock * 5 / 1000;
 
 	for (testr = 0; testr < 4; testr++) {
 		if (delta == 0)
@@ -110,6 +111,11 @@ static int mgag200_g200er_pixpllc_atomic_check(struct drm_crtc *crtc,
 		}
 	}
 
+	if (delta > permitteddelta) {
+		pr_warn("PLL delta too large\n");
+		return -EINVAL;
+	}
+
 	pixpllc->m = m;
 	pixpllc->n = n;
 	pixpllc->p = p;
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ev.c b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
index 3fadbeb10af9..0e882872e968 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200ev.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200ev.c
@@ -57,13 +57,14 @@ static int mgag200_g200ev_pixpllc_atomic_check(struct drm_crtc *crtc,
 	struct mgag200_crtc_state *new_mgag200_crtc_state = to_mgag200_crtc_state(new_crtc_state);
 	long clock = new_crtc_state->mode.clock;
 	struct mgag200_pll_values *pixpllc = &new_mgag200_crtc_state->pixpllc;
-	unsigned int delta, tmpdelta;
+	unsigned int delta, tmpdelta, permitteddelta;
 	unsigned int testp, testm, testn;
 	unsigned int p, m, n, s;
 	unsigned int computed;
 
 	m = n = p = s = 0;
 	delta = 0xffffffff;
+	permitteddelta = clock * 5 / 1000;
 
 	for (testp = 16; testp > 0; testp--) {
 		if (clock * testp > vcomax)
@@ -89,6 +90,11 @@ static int mgag200_g200ev_pixpllc_atomic_check(struct drm_crtc *crtc,
 		}
 	}
 
+	if (delta > permitteddelta) {
+		pr_warn("PLL delta too large\n");
+		return -EINVAL;
+	}
+
 	pixpllc->m = m;
 	pixpllc->n = n;
 	pixpllc->p = p;
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200ew3.c b/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
index e387a455eae5..da8f2a66cde3 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200ew3.c
@@ -33,13 +33,14 @@ static int mgag200_g200ew3_pixpllc_atomic_check(struct drm_crtc *crtc,
 	struct mgag200_crtc_state *new_mgag200_crtc_state = to_mgag200_crtc_state(new_crtc_state);
 	long clock = new_crtc_state->mode.clock;
 	struct mgag200_pll_values *pixpllc = &new_mgag200_crtc_state->pixpllc;
-	unsigned int delta, tmpdelta;
+	unsigned int delta, tmpdelta, permitteddelta;
 	unsigned int testp, testm, testn, testp2;
 	unsigned int p, m, n, s;
 	unsigned int computed;
 
 	m = n = p = s = 0;
 	delta = 0xffffffff;
+	permitteddelta = clock * 5 / 1000;
 
 	for (testp = 1; testp < 8; testp++) {
 		for (testp2 = 1; testp2 < 8; testp2++) {
@@ -68,6 +69,11 @@ static int mgag200_g200ew3_pixpllc_atomic_check(struct drm_crtc *crtc,
 		}
 	}
 
+	if (delta > permitteddelta) {
+		pr_warn("PLL delta too large\n");
+		return -EINVAL;
+	}
+
 	pixpllc->m = m;
 	pixpllc->n = n;
 	pixpllc->p = p;
diff --git a/drivers/gpu/drm/mgag200/mgag200_g200wb.c b/drivers/gpu/drm/mgag200/mgag200_g200wb.c
index d847fa8ded8c..ca4775173ff0 100644
--- a/drivers/gpu/drm/mgag200/mgag200_g200wb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_g200wb.c
@@ -49,13 +49,14 @@ static int mgag200_g200wb_pixpllc_atomic_check(struct drm_crtc *crtc,
 	struct mgag200_crtc_state *new_mgag200_crtc_state = to_mgag200_crtc_state(new_crtc_state);
 	long clock = new_crtc_state->mode.clock;
 	struct mgag200_pll_values *pixpllc = &new_mgag200_crtc_state->pixpllc;
-	unsigned int delta, tmpdelta;
+	unsigned int delta, tmpdelta, permitteddelta;
 	unsigned int testp, testm, testn;
 	unsigned int p, m, n, s;
 	unsigned int computed;
 
 	m = n = p = s = 0;
 	delta = 0xffffffff;
+	permitteddelta = clock * 5 / 1000;
 
 	for (testp = 1; testp < 9; testp++) {
 		if (clock * testp > vcomax)
@@ -81,6 +82,11 @@ static int mgag200_g200wb_pixpllc_atomic_check(struct drm_crtc *crtc,
 		}
 	}
 
+	if (delta > permitteddelta) {
+		pr_warn("PLL delta too large\n");
+		return -EINVAL;
+	}
+
 	pixpllc->m = m;
 	pixpllc->n = n;
 	pixpllc->p = p;
-- 

             reply	other threads:[~2026-05-17 13:03 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-17 13:03 Berkant Koc [this message]
2026-05-18  6:11 ` Claude review: drm/mgag200: reject pixel clocks outside PLL range Claude Code Review Bot
2026-05-18  6:11 ` 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=28e476e896dafdd31518dc5360ab7766@berkoc.com \
    --to=me@berkoc.com \
    --cc=airlied@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=jfalempe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=sam@ravnborg.org \
    --cc=simona@ffwll.ch \
    --cc=tzimmermann@suse.de \
    /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