All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/msm/dpu: don't reassign resources on color management changes
@ 2026-11-01  9:57 Beat M. Liechti
  2026-11-01 10:15 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Beat M. Liechti @ 2026-11-01  9:57 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov
  Cc: Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
	David Airlie, Simona Vetter, Federico Amedeo Izzo, linux-arm-msm,
	dri-devel, freedreno, linux-kernel, Beat M. Liechti, stable

Since commit 39a750ff5fc9 ("drm/msm/dpu: Add DSPP GC driver to provide
GAMMA_LUT DRM property"), dpu_crtc_atomic_check() releases and reserves
the hardware blocks of a CRTC whenever color_mgmt_changed is set, not
only on a modeset. A commit that merely replaces the GAMMA_LUT or CTM
blob of an active CRTC can therefore move that CRTC to a different
CTL/LM pair without disabling and re-enabling the pipeline, and the
hardware keeps scanning out through blocks the driver no longer
considers in use.

This is easy to hit with GNOME/mutter on a laptop driving an external
display: closing the lid disables the eDP CRTC, and in the same atomic
commit mutter sets new color blobs on the still active HDMI CRTC. With
drm.debug enabled, dpu_rm_reserve() moves the HDMI CRTC from CTL_1 to
the just released CTL_0 while its encoder is neither disabled nor
re-enabled. A few seconds later the display SMMU reports context faults
at the refresh rate and the external screen fills with garbage. Opening
the lid again leads to a stream of "vblank timeout" and "frame done
timeout" errors and the machine has to be power cycled.

Color management affects the topology in only one way: DSPP blocks are
requested when either CTM or GAMMA_LUT is set. Handle that case in
dpu_crtc_check_mode_changed() by forcing a full modeset when the
presence of CTM/GAMMA_LUT changes, as msm_atomic_check() already does
for the CTM, and stop reallocating resources on a bare
color_mgmt_changed.

Fixes: 39a750ff5fc9 ("drm/msm/dpu: Add DSPP GC driver to provide GAMMA_LUT DRM property")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Beat M. Liechti <beat@softbase.tech>
---

Notes:
    Testing: ThinkPad T14s Gen 6 (X1E78100), GNOME 50 / mutter on Wayland,
    LG 4K60 monitor on the HDMI port (DP to HDMI bridge), logind set to not
    suspend on lid close while on external power. Without this patch every
    lid close reproduces the corruption and hang described above. With it,
    closing and opening the lid produces no SMMU faults, vblank timeouts or
    frame done timeouts, and both displays keep working.
    
    That testing was done on a v7.0.14 based distribution kernel (Ubuntu
    7.0.0-42.42). On msm-next the patch has only been compile tested (W=1,
    no new warnings), not run.
    
    Open questions:
    - Like the existing CTM check in msm_atomic_check() (see the FIXME
      there), this sets allow_modeset on behalf of userspace when the DSPP
      requirement changes. Rejecting such commits without
      DRM_MODE_ATOMIC_ALLOW_MODESET would be stricter, but could break
      compositors that toggle GAMMA_LUT at runtime (night light). I am happy
      to change it either way.
    - The RFC "drm/msm/dpu: Force modeset if new CTLs have been reserved"
      (https://lists.freedesktop.org/archives/freedreno/2025-March/036824.html)
      addresses reassignment without a modeset in general. This patch only
      removes the color management trigger.
    
    The root cause analysis of the drm.debug trace, the fix and the
    changelog were developed with an AI coding assistant (Claude by
    Anthropic). I reviewed the change and tested it on the hardware above.

 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 26 +++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 42d0a529b..3b304a896 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1516,6 +1516,20 @@ int dpu_crtc_check_mode_changed(struct drm_crtc_state *old_crtc_state,
 	    (!clone_mode_requested && clone_mode_enabled))
 		new_crtc_state->mode_changed = true;
 
+	/*
+	 * DSPP blocks are reserved only when CTM or GAMMA_LUT is set (see
+	 * dpu_crtc_get_topology()), so enabling or disabling color management
+	 * changes the hardware topology. Changing resources on an active CRTC
+	 * requires a full modeset; otherwise the CTL/LM blocks get swapped
+	 * underneath a running pipeline.
+	 */
+	if (new_crtc_state->enable &&
+	    (!!(old_crtc_state->ctm || old_crtc_state->gamma_lut) !=
+	     !!(new_crtc_state->ctm || new_crtc_state->gamma_lut))) {
+		new_crtc_state->mode_changed = true;
+		new_crtc_state->state->allow_modeset = true;
+	}
+
 	return 0;
 }
 
@@ -1534,9 +1548,15 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 
 	bool needs_dirtyfb = dpu_crtc_needs_dirtyfb(crtc_state);
 
-	/* don't reallocate resources if only ACTIVE has beeen changed */
-	if (crtc_state->mode_changed || crtc_state->connectors_changed ||
-	    crtc_state->color_mgmt_changed) {
+	/*
+	 * Don't reallocate resources if only ACTIVE or color management
+	 * values have changed. A change in DSPP requirements is turned into a
+	 * modeset by dpu_crtc_check_mode_changed(). Reallocating on a plain
+	 * color_mgmt_changed would move an active CRTC to different CTL/LM
+	 * blocks without a modeset, leaving the hardware scanning out through
+	 * stale CTL/LM state (SMMU faults, corrupted output, vblank timeouts).
+	 */
+	if (crtc_state->mode_changed || crtc_state->connectors_changed) {
 		rc = dpu_crtc_assign_resources(crtc, crtc_state);
 		if (rc < 0)
 			return rc;
-- 
2.53.0


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

* Re: [PATCH] drm/msm/dpu: don't reassign resources on color management changes
  2026-11-01  9:57 [PATCH] drm/msm/dpu: don't reassign resources on color management changes Beat M. Liechti
@ 2026-11-01 10:15 ` sashiko-bot
  2026-11-01 10:58 ` Dmitry Baryshkov
  2026-11-01 12:26 ` [PATCH v2 0/2] drm/msm: fix CTL/LM swap " Beat M. Liechti
  2 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-11-01 10:15 UTC (permalink / raw)
  To: Beat M. Liechti; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/msm/dpu: legacy CTM check in msm_atomic_check() overrides dpu_crtc_check_mode_changed()
--

commit d6607b12584d4668388dbb5415d60630f0aba092
Author: Beat M. Liechti <beat@softbase.tech>

drm/msm/dpu: don't reassign resources on color management changes

This commit stops reallocating hardware resources in dpu_crtc_atomic_check()
on plain color_mgmt_changed updates, and instead forces a full modeset in
dpu_crtc_check_mode_changed() when the combined CTM or GAMMA_LUT requirement
for DSPP blocks changes on an enabled CRTC.

> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 77a2d1f6700d0..d1cd3c6e2e3ab 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1516,6 +1516,20 @@ int dpu_crtc_check_mode_changed(struct drm_crtc_state *old_crtc_state,
>  	    (!clone_mode_requested && clone_mode_enabled))
>  		new_crtc_state->mode_changed = true;
>  
> +	/*
> +	 * DSPP blocks are reserved only when CTM or GAMMA_LUT is set (see
> +	 * dpu_crtc_get_topology()), so enabling or disabling color management
> +	 * changes the hardware topology. Changing resources on an active CRTC
> +	 * requires a full modeset; otherwise the CTL/LM blocks get swapped
> +	 * underneath a running pipeline.
> +	 */
> +	if (new_crtc_state->enable &&
> +	    (!!(old_crtc_state->ctm || old_crtc_state->gamma_lut) !=
> +	     !!(new_crtc_state->ctm || new_crtc_state->gamma_lut))) {
> +		new_crtc_state->mode_changed = true;
> +		new_crtc_state->state->allow_modeset = true;
> +	}

[Severity: Medium]
In dpu_crtc_check_mode_changed(), this check tests the combined
(ctm || gamma_lut) state on an enabled CRTC so that a modeset is only forced
when DSPP block reservation in dpu_crtc_get_topology() actually changes.

However, msm_atomic_check() in drivers/gpu/drm/msm/msm_atomic.c runs right
before kms->funcs->check_mode_changed() and still contains the earlier
CTM-only loop:

drivers/gpu/drm/msm/msm_atomic.c:msm_atomic_check() {
	/*
	 * FIXME: stop setting allow_modeset and move this check to the DPU
	 * driver.
	 */
	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
				      new_crtc_state, i) {
		if ((old_crtc_state->ctm && !new_crtc_state->ctm) ||
		    (!old_crtc_state->ctm && new_crtc_state->ctm)) {
			new_crtc_state->mode_changed = true;
			state->allow_modeset = true;
		}
	}

	if (kms && kms->funcs && kms->funcs->check_mode_changed)
		ret = kms->funcs->check_mode_changed(kms, state);
}

Because msm_atomic_check() unconditionally sets new_crtc_state->mode_changed
and state->allow_modeset whenever ctm is toggled, won't toggling ctm while
gamma_lut is already active still force an unnecessary full modeset and
resource reallocation even though DSPP blocks are already reserved, and also
bypass the new_crtc_state->enable guard when disabling a CRTC that had ctm
set?

Since DPU is the only sub-driver in drivers/gpu/drm/msm/ that enables color
management via drm_crtc_enable_color_mgmt(), should that CTM loop in
msm_atomic_check() be removed now that dpu_crtc_check_mode_changed() handles
both ctm and gamma_lut?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101095754.33840-1-beat@softbase.tech?part=1

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

* Re: [PATCH] drm/msm/dpu: don't reassign resources on color management changes
  2026-11-01  9:57 [PATCH] drm/msm/dpu: don't reassign resources on color management changes Beat M. Liechti
  2026-11-01 10:15 ` sashiko-bot
@ 2026-11-01 10:58 ` Dmitry Baryshkov
  2026-11-01 12:26 ` [PATCH v2 0/2] drm/msm: fix CTL/LM swap " Beat M. Liechti
  2 siblings, 0 replies; 6+ messages in thread
From: Dmitry Baryshkov @ 2026-11-01 10:58 UTC (permalink / raw)
  To: Beat M. Liechti
  Cc: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
	Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
	Federico Amedeo Izzo, linux-arm-msm, dri-devel, freedreno,
	linux-kernel, stable

On Sun, Nov 01, 2026 at 10:57:45AM +0100, Beat M. Liechti wrote:
> Since commit 39a750ff5fc9 ("drm/msm/dpu: Add DSPP GC driver to provide
> GAMMA_LUT DRM property"), dpu_crtc_atomic_check() releases and reserves
> the hardware blocks of a CRTC whenever color_mgmt_changed is set, not
> only on a modeset. A commit that merely replaces the GAMMA_LUT or CTM
> blob of an active CRTC can therefore move that CRTC to a different
> CTL/LM pair without disabling and re-enabling the pipeline, and the
> hardware keeps scanning out through blocks the driver no longer
> considers in use.
> 
> This is easy to hit with GNOME/mutter on a laptop driving an external
> display: closing the lid disables the eDP CRTC, and in the same atomic
> commit mutter sets new color blobs on the still active HDMI CRTC. With
> drm.debug enabled, dpu_rm_reserve() moves the HDMI CRTC from CTL_1 to
> the just released CTL_0 while its encoder is neither disabled nor
> re-enabled. A few seconds later the display SMMU reports context faults
> at the refresh rate and the external screen fills with garbage. Opening
> the lid again leads to a stream of "vblank timeout" and "frame done
> timeout" errors and the machine has to be power cycled.
> 
> Color management affects the topology in only one way: DSPP blocks are
> requested when either CTM or GAMMA_LUT is set. Handle that case in
> dpu_crtc_check_mode_changed() by forcing a full modeset when the
> presence of CTM/GAMMA_LUT changes, as msm_atomic_check() already does
> for the CTM, and stop reallocating resources on a bare
> color_mgmt_changed.
> 
> Fixes: 39a750ff5fc9 ("drm/msm/dpu: Add DSPP GC driver to provide GAMMA_LUT DRM property")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Beat M. Liechti <beat@softbase.tech>
> ---
> 
> Notes:
>     Testing: ThinkPad T14s Gen 6 (X1E78100), GNOME 50 / mutter on Wayland,
>     LG 4K60 monitor on the HDMI port (DP to HDMI bridge), logind set to not
>     suspend on lid close while on external power. Without this patch every
>     lid close reproduces the corruption and hang described above. With it,
>     closing and opening the lid produces no SMMU faults, vblank timeouts or
>     frame done timeouts, and both displays keep working.
>     
>     That testing was done on a v7.0.14 based distribution kernel (Ubuntu
>     7.0.0-42.42). On msm-next the patch has only been compile tested (W=1,
>     no new warnings), not run.
>     
>     Open questions:
>     - Like the existing CTM check in msm_atomic_check() (see the FIXME
>       there), this sets allow_modeset on behalf of userspace when the DSPP
>       requirement changes. Rejecting such commits without
>       DRM_MODE_ATOMIC_ALLOW_MODESET would be stricter, but could break
>       compositors that toggle GAMMA_LUT at runtime (night light). I am happy
>       to change it either way.
>     - The RFC "drm/msm/dpu: Force modeset if new CTLs have been reserved"
>       (https://lists.freedesktop.org/archives/freedreno/2025-March/036824.html)
>       addresses reassignment without a modeset in general. This patch only
>       removes the color management trigger.
>     
>     The root cause analysis of the drm.debug trace, the fix and the
>     changelog were developed with an AI coding assistant (Claude by
>     Anthropic). I reviewed the change and tested it on the hardware above.
> 
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 26 +++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 42d0a529b..3b304a896 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1516,6 +1516,20 @@ int dpu_crtc_check_mode_changed(struct drm_crtc_state *old_crtc_state,
>  	    (!clone_mode_requested && clone_mode_enabled))
>  		new_crtc_state->mode_changed = true;
>  
> +	/*
> +	 * DSPP blocks are reserved only when CTM or GAMMA_LUT is set (see
> +	 * dpu_crtc_get_topology()), so enabling or disabling color management
> +	 * changes the hardware topology. Changing resources on an active CRTC
> +	 * requires a full modeset; otherwise the CTL/LM blocks get swapped
> +	 * underneath a running pipeline.
> +	 */
> +	if (new_crtc_state->enable &&
> +	    (!!(old_crtc_state->ctm || old_crtc_state->gamma_lut) !=
> +	     !!(new_crtc_state->ctm || new_crtc_state->gamma_lut))) {
> +		new_crtc_state->mode_changed = true;
> +		new_crtc_state->state->allow_modeset = true;

Having allow_modeset in the atomic_check() code was a hack, which should
not have been allowed in. We are trying to get rid of it, not introduce
new instances of it.

> +	}
> +
>  	return 0;
>  }
>  
> @@ -1534,9 +1548,15 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
>  
>  	bool needs_dirtyfb = dpu_crtc_needs_dirtyfb(crtc_state);
>  
> -	/* don't reallocate resources if only ACTIVE has beeen changed */
> -	if (crtc_state->mode_changed || crtc_state->connectors_changed ||
> -	    crtc_state->color_mgmt_changed) {
> +	/*
> +	 * Don't reallocate resources if only ACTIVE or color management
> +	 * values have changed. A change in DSPP requirements is turned into a
> +	 * modeset by dpu_crtc_check_mode_changed(). Reallocating on a plain
> +	 * color_mgmt_changed would move an active CRTC to different CTL/LM
> +	 * blocks without a modeset, leaving the hardware scanning out through
> +	 * stale CTL/LM state (SMMU faults, corrupted output, vblank timeouts).
> +	 */
> +	if (crtc_state->mode_changed || crtc_state->connectors_changed) {
>  		rc = dpu_crtc_assign_resources(crtc, crtc_state);
>  		if (rc < 0)
>  			return rc;
> -- 
> 2.53.0
> 

-- 
With best wishes
Dmitry

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

* [PATCH v2 0/2] drm/msm: fix CTL/LM swap on color management changes
  2026-11-01  9:57 [PATCH] drm/msm/dpu: don't reassign resources on color management changes Beat M. Liechti
  2026-11-01 10:15 ` sashiko-bot
  2026-11-01 10:58 ` Dmitry Baryshkov
@ 2026-11-01 12:26 ` Beat M. Liechti
  2026-11-01 12:26   ` [PATCH v2 1/2] drm/msm/dpu: don't reassign resources " Beat M. Liechti
  2026-11-01 12:26   ` [PATCH v2 2/2] drm/msm: drop the CTM modeset hack from msm_atomic_check() Beat M. Liechti
  2 siblings, 2 replies; 6+ messages in thread
From: Beat M. Liechti @ 2026-11-01 12:26 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov
  Cc: Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
	David Airlie, Simona Vetter, Federico Amedeo Izzo, linux-arm-msm,
	dri-devel, freedreno, linux-kernel, Beat M. Liechti

Closing the lid of an X1E laptop under GNOME with an external display
attached moves the still active external CRTC to other CTL/LM blocks
without a modeset, which ends in SMMU faults and a hung display
pipeline. Patch 1 fixes that in DPU. Patch 2 removes the CTM-only
allow_modeset hack in msm_atomic_check(), which patch 1 makes
redundant.

Changes in v2:
- Patch 1: don't set allow_modeset on behalf of userspace (Dmitry)
- Patch 2: new, drops the legacy CTM check (Sashiko review)
- Link to v1: https://lore.kernel.org/r/20261101095754.33840-1-beat@softbase.tech

Beat M. Liechti (2):
  drm/msm/dpu: don't reassign resources on color management changes
  drm/msm: drop the CTM modeset hack from msm_atomic_check()

 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 25 +++++++++++++++++++++---
 drivers/gpu/drm/msm/msm_atomic.c         | 17 +---------------
 2 files changed, 23 insertions(+), 19 deletions(-)

-- 
2.53.0


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

* [PATCH v2 1/2] drm/msm/dpu: don't reassign resources on color management changes
  2026-11-01 12:26 ` [PATCH v2 0/2] drm/msm: fix CTL/LM swap " Beat M. Liechti
@ 2026-11-01 12:26   ` Beat M. Liechti
  2026-11-01 12:26   ` [PATCH v2 2/2] drm/msm: drop the CTM modeset hack from msm_atomic_check() Beat M. Liechti
  1 sibling, 0 replies; 6+ messages in thread
From: Beat M. Liechti @ 2026-11-01 12:26 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov
  Cc: Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
	David Airlie, Simona Vetter, Federico Amedeo Izzo, linux-arm-msm,
	dri-devel, freedreno, linux-kernel, Beat M. Liechti, stable

Since commit 39a750ff5fc9 ("drm/msm/dpu: Add DSPP GC driver to provide
GAMMA_LUT DRM property"), dpu_crtc_atomic_check() releases and reserves
the hardware blocks of a CRTC whenever color_mgmt_changed is set, not
only on a modeset. A commit that merely replaces the GAMMA_LUT or CTM
blob of an active CRTC can therefore move that CRTC to a different
CTL/LM pair without disabling and re-enabling the pipeline, and the
hardware keeps scanning out through blocks the driver no longer
considers in use.

This is easy to hit with GNOME/mutter on a laptop driving an external
display: closing the lid disables the eDP CRTC, and in the same atomic
commit mutter sets new color blobs on the still active HDMI CRTC. With
drm.debug enabled, dpu_rm_reserve() moves the HDMI CRTC from CTL_1 to
the just released CTL_0 while its encoder is neither disabled nor
re-enabled. A few seconds later the display SMMU reports context faults
at the refresh rate and the external screen fills with garbage. Opening
the lid again leads to a stream of "vblank timeout" and "frame done
timeout" errors and the machine has to be power cycled.

Color management affects the topology in only one way: DSPP blocks are
requested when either CTM or GAMMA_LUT is set. Handle that case in
dpu_crtc_check_mode_changed() by requiring a full modeset when the
presence of CTM/GAMMA_LUT changes, and stop reallocating resources on a
bare color_mgmt_changed. Don't set allow_modeset here: if userspace
did not allow a modeset, the atomic core rejects the commit instead of
the CRTC being silently moved to other blocks.

Fixes: 39a750ff5fc9 ("drm/msm/dpu: Add DSPP GC driver to provide GAMMA_LUT DRM property")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Beat M. Liechti <beat@softbase.tech>
---

Notes:
    Changes in v2:
    - Don't set allow_modeset on behalf of userspace; only set mode_changed
      and let drm_atomic_check_only() reject commits that did not allow a
      modeset (Dmitry)
    - New patch 2/2 removes the CTM-only allow_modeset hack from
      msm_atomic_check(), which is redundant with this check (Sashiko review)
    - Link to v1: https://lore.kernel.org/r/20261101095754.33840-1-beat@softbase.tech
    
    Testing: ThinkPad T14s Gen 6 (X1E78100), GNOME 50 / mutter on Wayland,
    LG 4K60 monitor on the HDMI port (DP to HDMI bridge), logind set to not
    suspend on lid close while on external power. Without this patch every
    lid close reproduces the corruption and hang described above. With it,
    closing and opening the lid produces no SMMU faults, vblank timeouts or
    frame done timeouts, and both displays keep working.
    
    v2 was retested on the same kernel with both patches applied: lid
    close/open, Night Light toggled repeatedly with the lid open and closed,
    and a color profile change. No SMMU faults or timeouts, no visual
    problems, and with drm.debug=0x16 no commit was rejected for requiring
    a modeset, so mutter is not affected by the stricter check.
    
    On msm-next the patch has only been compile tested (W=1, no new
    warnings), not run.
    
    The RFC "drm/msm/dpu: Force modeset if new CTLs have been reserved"
    (https://lists.freedesktop.org/archives/freedreno/2025-March/036824.html)
    addresses reassignment without a modeset in general. This patch only
    removes the color management trigger.
    
    The root cause analysis of the drm.debug trace, the fix and the
    changelog were developed with an AI coding assistant (Claude by
    Anthropic). I reviewed the change and tested it on the hardware above.

 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 25 +++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 42d0a529b..cfcc3556a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1516,6 +1516,19 @@ int dpu_crtc_check_mode_changed(struct drm_crtc_state *old_crtc_state,
 	    (!clone_mode_requested && clone_mode_enabled))
 		new_crtc_state->mode_changed = true;
 
+	/*
+	 * DSPP blocks are reserved only when CTM or GAMMA_LUT is set (see
+	 * dpu_crtc_get_topology()), so enabling or disabling color management
+	 * changes the hardware topology. Changing resources on an active CRTC
+	 * requires a full modeset; otherwise the CTL/LM blocks get swapped
+	 * underneath a running pipeline. If userspace did not allow a modeset,
+	 * drm_atomic_check_only() rejects the commit.
+	 */
+	if (new_crtc_state->enable &&
+	    (!!(old_crtc_state->ctm || old_crtc_state->gamma_lut) !=
+	     !!(new_crtc_state->ctm || new_crtc_state->gamma_lut)))
+		new_crtc_state->mode_changed = true;
+
 	return 0;
 }
 
@@ -1534,9 +1547,15 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 
 	bool needs_dirtyfb = dpu_crtc_needs_dirtyfb(crtc_state);
 
-	/* don't reallocate resources if only ACTIVE has beeen changed */
-	if (crtc_state->mode_changed || crtc_state->connectors_changed ||
-	    crtc_state->color_mgmt_changed) {
+	/*
+	 * Don't reallocate resources if only ACTIVE or color management
+	 * values have changed. A change in DSPP requirements is turned into a
+	 * modeset by dpu_crtc_check_mode_changed(). Reallocating on a plain
+	 * color_mgmt_changed would move an active CRTC to different CTL/LM
+	 * blocks without a modeset, leaving the hardware scanning out through
+	 * stale CTL/LM state (SMMU faults, corrupted output, vblank timeouts).
+	 */
+	if (crtc_state->mode_changed || crtc_state->connectors_changed) {
 		rc = dpu_crtc_assign_resources(crtc, crtc_state);
 		if (rc < 0)
 			return rc;
-- 
2.53.0


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

* [PATCH v2 2/2] drm/msm: drop the CTM modeset hack from msm_atomic_check()
  2026-11-01 12:26 ` [PATCH v2 0/2] drm/msm: fix CTL/LM swap " Beat M. Liechti
  2026-11-01 12:26   ` [PATCH v2 1/2] drm/msm/dpu: don't reassign resources " Beat M. Liechti
@ 2026-11-01 12:26   ` Beat M. Liechti
  1 sibling, 0 replies; 6+ messages in thread
From: Beat M. Liechti @ 2026-11-01 12:26 UTC (permalink / raw)
  To: Rob Clark, Dmitry Baryshkov
  Cc: Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
	David Airlie, Simona Vetter, Federico Amedeo Izzo, linux-arm-msm,
	dri-devel, freedreno, linux-kernel, Beat M. Liechti

msm_atomic_check() forces a modeset, and sets allow_modeset on behalf
of userspace, whenever the CTM property of a CRTC is set or cleared.
This was added so that DPU could reserve DSPP blocks for the CTM.

DPU is the only msm display driver that exposes color management, and
dpu_crtc_check_mode_changed() now requires a modeset whenever the DSPP
requirement changes, for both CTM and GAMMA_LUT. The generic check is
therefore redundant. It also forces needless modesets when the CTM is
toggled while a GAMMA_LUT keeps the DSPPs reserved, and it overrides
the userspace decision about modesets.

Remove it, as the FIXME suggested. Commits that toggle the CTM without
DRM_MODE_ATOMIC_ALLOW_MODESET now fail with -EINVAL instead of
silently becoming a modeset.

Assisted-by: LLM
Signed-off-by: Beat M. Liechti <beat@softbase.tech>
---
 drivers/gpu/drm/msm/msm_atomic.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index a8babf1db..7b480981b 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -185,22 +185,7 @@ int msm_atomic_check(struct drm_device *dev, struct drm_atomic_commit *state)
 {
 	struct msm_drm_private *priv = dev->dev_private;
 	struct msm_kms *kms = priv->kms;
-	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
-	struct drm_crtc *crtc;
-	int i, ret = 0;
-
-	/*
-	 * FIXME: stop setting allow_modeset and move this check to the DPU
-	 * driver.
-	 */
-	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
-				      new_crtc_state, i) {
-		if ((old_crtc_state->ctm && !new_crtc_state->ctm) ||
-		    (!old_crtc_state->ctm && new_crtc_state->ctm)) {
-			new_crtc_state->mode_changed = true;
-			state->allow_modeset = true;
-		}
-	}
+	int ret = 0;
 
 	if (kms && kms->funcs && kms->funcs->check_mode_changed)
 		ret = kms->funcs->check_mode_changed(kms, state);
-- 
2.53.0


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

end of thread, other threads:[~2026-11-01 12:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-01  9:57 [PATCH] drm/msm/dpu: don't reassign resources on color management changes Beat M. Liechti
2026-11-01 10:15 ` sashiko-bot
2026-11-01 10:58 ` Dmitry Baryshkov
2026-11-01 12:26 ` [PATCH v2 0/2] drm/msm: fix CTL/LM swap " Beat M. Liechti
2026-11-01 12:26   ` [PATCH v2 1/2] drm/msm/dpu: don't reassign resources " Beat M. Liechti
2026-11-01 12:26   ` [PATCH v2 2/2] drm/msm: drop the CTM modeset hack from msm_atomic_check() Beat M. Liechti

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.