public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/msm/disp/dpu: consider SSPP line width during mode valid
@ 2026-03-28 17:15 Vishnu Saini
  2026-03-29  0:24 ` Dmitry Baryshkov
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Vishnu Saini @ 2026-03-28 17:15 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, David Airlie, Simona Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel,
	prahlad.valluru, Vishnu Saini

Few targets have lesser SSPP line width compared to mixer width,
SSPP line width also needs to be considered during mode valid
to avoid failures during atomic_check.

Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 6bf7c46379ae..af245c44959d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1591,6 +1591,7 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc,
 {
 	struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
 	u64 adjusted_mode_clk;
+	u32 max_width;
 
 	/* if there is no 3d_mux block we cannot merge LMs so we cannot
 	 * split the large layer into 2 LMs, filter out such modes
@@ -1613,11 +1614,14 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc,
 		return MODE_CLOCK_HIGH;
 
 	/*
-	 * max crtc width is equal to the max mixer width * 2 and max height is 4K
+	 * max crtc width is equal to the min of max mixer width * 2 and max sspp line width * 2
+	 * max height is 4K
 	 */
-	return drm_mode_validate_size(mode,
-				      2 * dpu_kms->catalog->caps->max_mixer_width,
-				      4096);
+	max_width = 2 * min_t(u32,
+			      dpu_kms->catalog->caps->max_mixer_width,
+			      dpu_kms->catalog->caps->max_linewidth);
+
+	return drm_mode_validate_size(mode, max_width, 4096);
 }
 
 /**

---
base-commit: 6efced27f5df9d7a57e4847fe2898cdd19f72311
change-id: 20260328-msm-next-70eb896ff64d

Best regards,
-- 
Vishnu Saini <vishnu.saini@oss.qualcomm.com>


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/msm/disp/dpu: consider SSPP line width during mode valid
  2026-03-28 17:15 [PATCH] drm/msm/disp/dpu: consider SSPP line width during mode valid Vishnu Saini
@ 2026-03-29  0:24 ` Dmitry Baryshkov
  2026-03-30 18:30   ` Vishnu Saini
  2026-03-31  7:55 ` Claude review: " Claude Code Review Bot
  2026-03-31  7:55 ` Claude Code Review Bot
  2 siblings, 1 reply; 8+ messages in thread
From: Dmitry Baryshkov @ 2026-03-29  0:24 UTC (permalink / raw)
  To: Vishnu Saini
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
	linux-arm-msm, dri-devel, freedreno, linux-kernel,
	prahlad.valluru

On Sat, Mar 28, 2026 at 10:45:35PM +0530, Vishnu Saini wrote:
> Few targets have lesser SSPP line width compared to mixer width,
> SSPP line width also needs to be considered during mode valid
> to avoid failures during atomic_check.

Technically this is not correct. There is no requirement for the
planes to cover the whole CRTC. Nor is there a requirement to use only 2
rectangles to cover the screen. As such, it is perfectly fine in
mode_valid, if CRTC is wider than 2x max_linewidth. It would be an error
if the user tries to stretch 2 rectangles in such a case.

> 
> Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/msm/disp/dpu: consider SSPP line width during mode valid
  2026-03-29  0:24 ` Dmitry Baryshkov
@ 2026-03-30 18:30   ` Vishnu Saini
  2026-03-30 18:39     ` Dmitry Baryshkov
  0 siblings, 1 reply; 8+ messages in thread
From: Vishnu Saini @ 2026-03-30 18:30 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
	linux-arm-msm, dri-devel, freedreno, linux-kernel,
	prahlad.valluru

On Sun, Mar 29, 2026 at 02:24:53AM +0200, Dmitry Baryshkov wrote:
> On Sat, Mar 28, 2026 at 10:45:35PM +0530, Vishnu Saini wrote:
> > Few targets have lesser SSPP line width compared to mixer width,
> > SSPP line width also needs to be considered during mode valid
> > to avoid failures during atomic_check.
> 
> Technically this is not correct. There is no requirement for the
> planes to cover the whole CRTC. Nor is there a requirement to use only 2
> rectangles to cover the screen. As such, it is perfectly fine in
> mode_valid, if CRTC is wider than 2x max_linewidth. It would be an error
> if the user tries to stretch 2 rectangles in such a case.

This is to fix an issue with 5k monitor on rb3gen2, since SSPP maxlinewidth is 2400
it can't cover the whole 5k buffer in left right split mode.
Do we need to fix it from drm backend by dividing 5k buffer into 2 planes and 
use 2 pipes in split mode.
4 SSPP rects --> 2 LMs --> 3d_mux --> DP
 
> > 
> > Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> 
> -- 
> With best wishes
> Dmitry

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/msm/disp/dpu: consider SSPP line width during mode valid
  2026-03-30 18:30   ` Vishnu Saini
@ 2026-03-30 18:39     ` Dmitry Baryshkov
  2026-03-30 18:57       ` Vishnu Saini
  0 siblings, 1 reply; 8+ messages in thread
From: Dmitry Baryshkov @ 2026-03-30 18:39 UTC (permalink / raw)
  To: Vishnu Saini
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
	linux-arm-msm, dri-devel, freedreno, linux-kernel,
	prahlad.valluru

On Tue, Mar 31, 2026 at 12:00:07AM +0530, Vishnu Saini wrote:
> On Sun, Mar 29, 2026 at 02:24:53AM +0200, Dmitry Baryshkov wrote:
> > On Sat, Mar 28, 2026 at 10:45:35PM +0530, Vishnu Saini wrote:
> > > Few targets have lesser SSPP line width compared to mixer width,
> > > SSPP line width also needs to be considered during mode valid
> > > to avoid failures during atomic_check.
> > 
> > Technically this is not correct. There is no requirement for the
> > planes to cover the whole CRTC. Nor is there a requirement to use only 2
> > rectangles to cover the screen. As such, it is perfectly fine in
> > mode_valid, if CRTC is wider than 2x max_linewidth. It would be an error
> > if the user tries to stretch 2 rectangles in such a case.
> 
> This is to fix an issue with 5k monitor on rb3gen2, since SSPP maxlinewidth is 2400
> it can't cover the whole 5k buffer in left right split mode.
> Do we need to fix it from drm backend by dividing 5k buffer into 2 planes and 
> use 2 pipes in split mode.

Quad pipe is pending for 7.2, most likely. However, I think, instead you
should teach compositor that if the commit fails, it should retry with
the lower resolution (it might require somethi g like -E2BIG from te
commit).

> 4 SSPP rects --> 2 LMs --> 3d_mux --> DP
>  
> > > 
> > > Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
> > > ---
> > >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 ++++++++----
> > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > > 
> > 
> > -- 
> > With best wishes
> > Dmitry

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/msm/disp/dpu: consider SSPP line width during mode valid
  2026-03-30 18:39     ` Dmitry Baryshkov
@ 2026-03-30 18:57       ` Vishnu Saini
  2026-03-30 19:38         ` Dmitry Baryshkov
  0 siblings, 1 reply; 8+ messages in thread
From: Vishnu Saini @ 2026-03-30 18:57 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
	linux-arm-msm, dri-devel, freedreno, linux-kernel,
	prahlad.valluru

On Mon, Mar 30, 2026 at 09:39:31PM +0300, Dmitry Baryshkov wrote:
> On Tue, Mar 31, 2026 at 12:00:07AM +0530, Vishnu Saini wrote:
> > On Sun, Mar 29, 2026 at 02:24:53AM +0200, Dmitry Baryshkov wrote:
> > > On Sat, Mar 28, 2026 at 10:45:35PM +0530, Vishnu Saini wrote:
> > > > Few targets have lesser SSPP line width compared to mixer width,
> > > > SSPP line width also needs to be considered during mode valid
> > > > to avoid failures during atomic_check.
> > > 
> > > Technically this is not correct. There is no requirement for the
> > > planes to cover the whole CRTC. Nor is there a requirement to use only 2
> > > rectangles to cover the screen. As such, it is perfectly fine in
> > > mode_valid, if CRTC is wider than 2x max_linewidth. It would be an error
> > > if the user tries to stretch 2 rectangles in such a case.
> > 
> > This is to fix an issue with 5k monitor on rb3gen2, since SSPP maxlinewidth is 2400
> > it can't cover the whole 5k buffer in left right split mode.
> > Do we need to fix it from drm backend by dividing 5k buffer into 2 planes and 
> > use 2 pipes in split mode.
> 
> Quad pipe is pending for 7.2, most likely. However, I think, instead you
> should teach compositor that if the commit fails, it should retry with
> the lower resolution (it might require somethi g like -E2BIG from te
> commit).
Yes, from driver we are returning "-E2BIG" during atomic_check.
Thank you for the clarity, will try to fix it from compositor.
 
> > 4 SSPP rects --> 2 LMs --> 3d_mux --> DP
> >  
> > > > 
> > > > Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
> > > > ---
> > > >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 ++++++++----
> > > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > > > 
> > > 
> > > -- 
> > > With best wishes
> > > Dmitry
> 
> -- 
> With best wishes
> Dmitry

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] drm/msm/disp/dpu: consider SSPP line width during mode valid
  2026-03-30 18:57       ` Vishnu Saini
@ 2026-03-30 19:38         ` Dmitry Baryshkov
  0 siblings, 0 replies; 8+ messages in thread
From: Dmitry Baryshkov @ 2026-03-30 19:38 UTC (permalink / raw)
  To: Vishnu Saini
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
	linux-arm-msm, dri-devel, freedreno, linux-kernel,
	prahlad.valluru

On Tue, Mar 31, 2026 at 12:27:19AM +0530, Vishnu Saini wrote:
> On Mon, Mar 30, 2026 at 09:39:31PM +0300, Dmitry Baryshkov wrote:
> > On Tue, Mar 31, 2026 at 12:00:07AM +0530, Vishnu Saini wrote:
> > > On Sun, Mar 29, 2026 at 02:24:53AM +0200, Dmitry Baryshkov wrote:
> > > > On Sat, Mar 28, 2026 at 10:45:35PM +0530, Vishnu Saini wrote:
> > > > > Few targets have lesser SSPP line width compared to mixer width,
> > > > > SSPP line width also needs to be considered during mode valid
> > > > > to avoid failures during atomic_check.
> > > > 
> > > > Technically this is not correct. There is no requirement for the
> > > > planes to cover the whole CRTC. Nor is there a requirement to use only 2
> > > > rectangles to cover the screen. As such, it is perfectly fine in
> > > > mode_valid, if CRTC is wider than 2x max_linewidth. It would be an error
> > > > if the user tries to stretch 2 rectangles in such a case.
> > > 
> > > This is to fix an issue with 5k monitor on rb3gen2, since SSPP maxlinewidth is 2400
> > > it can't cover the whole 5k buffer in left right split mode.
> > > Do we need to fix it from drm backend by dividing 5k buffer into 2 planes and 
> > > use 2 pipes in split mode.
> > 
> > Quad pipe is pending for 7.2, most likely. However, I think, instead you
> > should teach compositor that if the commit fails, it should retry with
> > the lower resolution (it might require somethi g like -E2BIG from te
> > commit).
> Yes, from driver we are returning "-E2BIG" during atomic_check.
> Thank you for the clarity, will try to fix it from compositor.

Just to point out: it's not different from the driver not being able to
perform the atomic commit for any other reason. For example, if card has
3 outputs but can support only 2 concurrent outputs, then each connector
will still have a list of valid modes, but trying to enable outputs on
all three of them would still fail (or trying to enable high-enough
resolution on all three, if that removes a possible question of limiting
number of outputs by other means).

>  
> > > 4 SSPP rects --> 2 LMs --> 3d_mux --> DP
> > >  
> > > > > 
> > > > > Signed-off-by: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
> > > > > ---
> > > > >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 12 ++++++++----
> > > > >  1 file changed, 8 insertions(+), 4 deletions(-)
> > > > > 
> > > > 
> > > > -- 
> > > > With best wishes
> > > > Dmitry
> > 
> > -- 
> > With best wishes
> > Dmitry

-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Claude review: drm/msm/disp/dpu: consider SSPP line width during mode valid
  2026-03-28 17:15 [PATCH] drm/msm/disp/dpu: consider SSPP line width during mode valid Vishnu Saini
  2026-03-29  0:24 ` Dmitry Baryshkov
@ 2026-03-31  7:55 ` Claude Code Review Bot
  2026-03-31  7:55 ` Claude Code Review Bot
  2 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-03-31  7:55 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/msm/disp/dpu: consider SSPP line width during mode valid
Author: Vishnu Saini <vishnu.saini@oss.qualcomm.com>
Patches: 6
Reviewed: 2026-03-31T17:55:49.647743

---

This is a single patch that adds SSPP (Source Surface Pixel Processor) line width as an additional constraint during mode validation in the DPU CRTC. The intent is correct — several Qualcomm targets (sm6150, sm6125, sm6115, qcm2290, sm6375) have `max_linewidth` smaller than `max_mixer_width` (e.g., sm6150: max_linewidth=2160, max_mixer_width=2560), meaning modes could pass `dpu_crtc_mode_valid()` but fail later in `atomic_check`.

The code change itself is clean and straightforward. However, there is a gap in the fix.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Claude review: drm/msm/disp/dpu: consider SSPP line width during mode valid
  2026-03-28 17:15 [PATCH] drm/msm/disp/dpu: consider SSPP line width during mode valid Vishnu Saini
  2026-03-29  0:24 ` Dmitry Baryshkov
  2026-03-31  7:55 ` Claude review: " Claude Code Review Bot
@ 2026-03-31  7:55 ` Claude Code Review Bot
  2 siblings, 0 replies; 8+ messages in thread
From: Claude Code Review Bot @ 2026-03-31  7:55 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**The fix is incomplete for the non-3d_merge path.**

The patch only constrains the final `drm_mode_validate_size()` call, changing:
```c
2 * dpu_kms->catalog->caps->max_mixer_width
```
to:
```c
2 * min_t(u32, max_mixer_width, max_linewidth)
```

But earlier in the same function (around line 1598-1600), there is:
```c
if (!dpu_kms->catalog->caps->has_3d_merge &&
    mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width)
    return MODE_BAD_HVALUE;
```

For targets *without* `has_3d_merge` where `max_linewidth < max_mixer_width` (e.g., sm6150 has no `has_3d_merge`, max_mixer_width=2560, max_linewidth=2160), this early check still allows modes up to 2560 wide, even though the SSPP can only handle 2160. Those modes would pass `mode_valid` and then presumably fail at `atomic_check`.

**This early check should also be clamped by `max_linewidth`**, something like:
```c
if (!dpu_kms->catalog->caps->has_3d_merge &&
    mode->hdisplay > min_t(u32, dpu_kms->catalog->caps->max_mixer_width,
                           dpu_kms->catalog->caps->max_linewidth))
    return MODE_BAD_HVALUE;
```

**Minor: the commit message could be more specific.** The message says "few targets have lesser SSPP line width" — it would be helpful to name the affected targets (sm6150, sm6125, sm6115, qcm2290, sm6375, etc.) and perhaps add a `Fixes:` tag if this addresses a regression or user-reported issue, or at least a `Cc: stable` if backport is desired.

**The code itself is otherwise correct:** `min_t(u32, ...)` is the right macro, the `max_width` variable is properly typed, and the comment update accurately describes the new logic.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-03-31  7:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-28 17:15 [PATCH] drm/msm/disp/dpu: consider SSPP line width during mode valid Vishnu Saini
2026-03-29  0:24 ` Dmitry Baryshkov
2026-03-30 18:30   ` Vishnu Saini
2026-03-30 18:39     ` Dmitry Baryshkov
2026-03-30 18:57       ` Vishnu Saini
2026-03-30 19:38         ` Dmitry Baryshkov
2026-03-31  7:55 ` Claude review: " Claude Code Review Bot
2026-03-31  7:55 ` 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