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: [email protected]
> Assisted-by: LLM
> Signed-off-by: Beat M. Liechti <[email protected]>
> ---
>
> 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
