* [PATCH v5 01/10] drm/msm: serialise framebuffer pin state
2026-10-23 23:25 [PATCH v5 00/10] drm/msm: fix SMMU fault dumps Dmitry Baryshkov
@ 2026-10-23 23:25 ` Dmitry Baryshkov
2026-10-26 8:55 ` Konrad Dybcio
2026-10-23 23:25 ` [PATCH v5 02/10] drm/msm: fix framebuffer pin refcount leak on prepare failure Dmitry Baryshkov
` (8 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Dmitry Baryshkov @ 2026-10-23 23:25 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Antonino Maniscalco, Kalyan Thota, Federico Amedeo Izzo,
Helen Koike, Vignesh Raman, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
prepare_count, iova[] and the pins they describe are updated locklessly,
but drm_atomic_helper_commit() prepares a new state in parallel with the
completion of the previous one -- stall_checks() only stalls on the second
previous commit. Commit N+1's ->prepare_fb() thus runs while commit N is
in ->cleanup_fb(), and for a double-buffered flip that is the same
framebuffer:
cleanup: prepare_count 1 -> 0
prepare: prepare_count 0 -> 1, pins, stores iova[]
cleanup: memset(iova, 0)
leaving the plane programmed with a NULL base address:
arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402,
iova=0x00000100, fsynr=0x3e0023, cbfrsynra=0x1c00, cb=11
The opposite order is broken too since commit 8ac37c88f991 ("drm/msm:
Refcount framebuffer pins"): a prepare which finds the count non-zero
returns at once, assuming iova[] is populated.
Both callbacks may sleep, so a mutex will do. drm_framebuffer_init()
adds the framebuffer to the object idr, from where userspace can reach it
before msm_framebuffer_init() returns, so take the private state out of
its way and initialise it first.
Fixes: 8ac37c88f991 ("drm/msm: Refcount framebuffer pins")
Assisted-by: LLM
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/msm_fb.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c
index 60c108d35d2a..a38e5b3a1d1e 100644
--- a/drivers/gpu/drm/msm/msm_fb.c
+++ b/drivers/gpu/drm/msm/msm_fb.c
@@ -22,9 +22,12 @@ struct msm_framebuffer {
/* Count of # of attached planes which need dirtyfb: */
refcount_t dirtyfb;
+ /* Protects the pin state below: */
+ struct mutex lock;
+
/* Framebuffer per-plane address, if pinned, else zero: */
uint64_t iova[DRM_FORMAT_MAX_PLANES];
- atomic_t prepare_count;
+ unsigned int prepare_count;
};
#define to_msm_framebuffer(x) container_of(x, struct msm_framebuffer, base)
@@ -45,9 +48,17 @@ static int msm_framebuffer_dirtyfb(struct drm_framebuffer *fb,
clips, num_clips);
}
+static void msm_framebuffer_destroy(struct drm_framebuffer *fb)
+{
+ struct msm_framebuffer *msm_fb = to_msm_framebuffer(fb);
+
+ mutex_destroy(&msm_fb->lock);
+ drm_gem_fb_destroy(fb);
+}
+
static const struct drm_framebuffer_funcs msm_framebuffer_funcs = {
.create_handle = drm_gem_fb_create_handle,
- .destroy = drm_gem_fb_destroy,
+ .destroy = msm_framebuffer_destroy,
.dirty = msm_framebuffer_dirtyfb,
};
@@ -81,7 +92,9 @@ int msm_framebuffer_prepare(struct drm_framebuffer *fb, bool needs_dirtyfb)
if (needs_dirtyfb)
refcount_inc(&msm_fb->dirtyfb);
- if (atomic_inc_return(&msm_fb->prepare_count) > 1)
+ guard(mutex)(&msm_fb->lock);
+
+ if (msm_fb->prepare_count++)
return 0;
for (i = 0; i < n; i++) {
@@ -106,7 +119,9 @@ void msm_framebuffer_cleanup(struct drm_framebuffer *fb, bool needed_dirtyfb)
if (needed_dirtyfb)
refcount_dec(&msm_fb->dirtyfb);
- if (atomic_dec_return(&msm_fb->prepare_count))
+ guard(mutex)(&msm_fb->lock);
+
+ if (--msm_fb->prepare_count)
return;
memset(msm_fb->iova, 0, sizeof(msm_fb->iova));
@@ -199,14 +214,15 @@ msm_framebuffer_init(struct drm_device *dev, const struct drm_format_info *info,
drm_helper_mode_fill_fb_struct(dev, fb, info, mode_cmd);
+ refcount_set(&msm_fb->dirtyfb, 1);
+ mutex_init(&msm_fb->lock);
+
ret = drm_framebuffer_init(dev, fb, &msm_framebuffer_funcs);
if (ret) {
DRM_DEV_ERROR(dev->dev, "framebuffer init failed: %d\n", ret);
goto fail;
}
- refcount_set(&msm_fb->dirtyfb, 1);
-
drm_dbg_state(dev, "create: FB ID: %d (%p)\n", fb->base.id, fb);
return fb;
--
2.47.3
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 01/10] drm/msm: serialise framebuffer pin state
2026-10-23 23:25 ` [PATCH v5 01/10] drm/msm: serialise framebuffer pin state Dmitry Baryshkov
@ 2026-10-26 8:55 ` Konrad Dybcio
0 siblings, 0 replies; 20+ messages in thread
From: Konrad Dybcio @ 2026-10-26 8:55 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie,
Simona Vetter, Antonino Maniscalco, Kalyan Thota,
Federico Amedeo Izzo, Helen Koike, Vignesh Raman,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 10/24/26 1:25 AM, Dmitry Baryshkov wrote:
> prepare_count, iova[] and the pins they describe are updated locklessly,
> but drm_atomic_helper_commit() prepares a new state in parallel with the
> completion of the previous one -- stall_checks() only stalls on the second
> previous commit. Commit N+1's ->prepare_fb() thus runs while commit N is
> in ->cleanup_fb(), and for a double-buffered flip that is the same
> framebuffer:
>
> cleanup: prepare_count 1 -> 0
> prepare: prepare_count 0 -> 1, pins, stores iova[]
> cleanup: memset(iova, 0)
>
> leaving the plane programmed with a NULL base address:
>
> arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402,
> iova=0x00000100, fsynr=0x3e0023, cbfrsynra=0x1c00, cb=11
>
> The opposite order is broken too since commit 8ac37c88f991 ("drm/msm:
> Refcount framebuffer pins"): a prepare which finds the count non-zero
> returns at once, assuming iova[] is populated.
The clanker thinks this is correct and was able to reproduce by doing
something similar with a test program
It also left the following comment:
The mutex approach in patch 1 has a direct precedent in omapdrm, so it's
a sound choice. msm is unusual in caching iova[] per framebuffer and
then zeroing it, which is why it needs the lock at all. If you want a
fix with less state, the amdgpu approach of not clearing the cached
address on cleanup would also avoid the NULL-base hazard. I'd keep
the mutex, though, since patch 2's count-leak fix sits on top of it.
any which way:
Acked-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 02/10] drm/msm: fix framebuffer pin refcount leak on prepare failure
2026-10-23 23:25 [PATCH v5 00/10] drm/msm: fix SMMU fault dumps Dmitry Baryshkov
2026-10-23 23:25 ` [PATCH v5 01/10] drm/msm: serialise framebuffer pin state Dmitry Baryshkov
@ 2026-10-23 23:25 ` Dmitry Baryshkov
2026-10-26 8:59 ` Konrad Dybcio
2026-10-23 23:25 ` [PATCH v5 03/10] drm/msm: release scanout framebuffers only after a vblank Dmitry Baryshkov
` (7 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Dmitry Baryshkov @ 2026-10-23 23:25 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Antonino Maniscalco, Kalyan Thota, Federico Amedeo Izzo,
Helen Koike, Vignesh Raman, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
msm_framebuffer_prepare() bumps prepare_count before pinning, but returns
straight out of the pin loop on error, leaking the count, the
msm_gem_vma_get() reference and any planes already pinned.
drm_atomic_helper_prepare_planes() does not call cleanup_fb() for the
plane whose prepare_fb() failed, so nothing ever drops it.
Since commit 8ac37c88f991 ("drm/msm: Refcount framebuffer pins") a
prepare which finds the count already non-zero returns early, assuming
iova[] is populated. With the count stuck, every later prepare of that
framebuffer reports success while iova[] is still zero, and DPU scans out
from a NULL base address:
arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402,
iova=0x00000100, fsynr=0x3e0023, cbfrsynra=0x1c00, cb=11
Unwind properly on failure instead.
Fixes: 8ac37c88f991 ("drm/msm: Refcount framebuffer pins")
Assisted-by: LLM
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/msm_fb.c | 32 +++++++++++++++++++++++---------
1 file changed, 23 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c
index a38e5b3a1d1e..792d47b4acc4 100644
--- a/drivers/gpu/drm/msm/msm_fb.c
+++ b/drivers/gpu/drm/msm/msm_fb.c
@@ -80,6 +80,18 @@ void msm_framebuffer_describe(struct drm_framebuffer *fb, struct seq_file *m)
}
#endif
+static void msm_framebuffer_unpin_planes(struct drm_framebuffer *fb, int n)
+{
+ struct msm_drm_private *priv = fb->dev->dev_private;
+ struct drm_gpuvm *vm = priv->kms->vm;
+ int i;
+
+ for (i = 0; i < n; i++) {
+ msm_gem_unpin_iova(fb->obj[i], vm);
+ msm_gem_vma_put(fb->obj[i]);
+ }
+}
+
/* prepare/pin all the fb's bo's for scanout.
*/
int msm_framebuffer_prepare(struct drm_framebuffer *fb, bool needs_dirtyfb)
@@ -102,8 +114,17 @@ int msm_framebuffer_prepare(struct drm_framebuffer *fb, bool needs_dirtyfb)
ret = msm_gem_get_and_pin_iova(fb->obj[i], vm, &msm_fb->iova[i]);
drm_dbg_state(fb->dev, "FB[%u]: iova[%d]: %08llx (%d)\n",
fb->base.id, i, msm_fb->iova[i], ret);
- if (ret)
+ if (ret) {
+ msm_gem_vma_put(fb->obj[i]);
+ msm_framebuffer_unpin_planes(fb, i);
+ memset(msm_fb->iova, 0, sizeof(msm_fb->iova));
+ msm_fb->prepare_count--;
+
+ if (needs_dirtyfb)
+ refcount_dec(&msm_fb->dirtyfb);
+
return ret;
+ }
}
return 0;
@@ -111,10 +132,7 @@ int msm_framebuffer_prepare(struct drm_framebuffer *fb, bool needs_dirtyfb)
void msm_framebuffer_cleanup(struct drm_framebuffer *fb, bool needed_dirtyfb)
{
- struct msm_drm_private *priv = fb->dev->dev_private;
- struct drm_gpuvm *vm = priv->kms->vm;
struct msm_framebuffer *msm_fb = to_msm_framebuffer(fb);
- int i, n = fb->format->num_planes;
if (needed_dirtyfb)
refcount_dec(&msm_fb->dirtyfb);
@@ -125,11 +143,7 @@ void msm_framebuffer_cleanup(struct drm_framebuffer *fb, bool needed_dirtyfb)
return;
memset(msm_fb->iova, 0, sizeof(msm_fb->iova));
-
- for (i = 0; i < n; i++) {
- msm_gem_unpin_iova(fb->obj[i], vm);
- msm_gem_vma_put(fb->obj[i]);
- }
+ msm_framebuffer_unpin_planes(fb, fb->format->num_planes);
}
uint32_t msm_framebuffer_iova(struct drm_framebuffer *fb, int plane)
--
2.47.3
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 02/10] drm/msm: fix framebuffer pin refcount leak on prepare failure
2026-10-23 23:25 ` [PATCH v5 02/10] drm/msm: fix framebuffer pin refcount leak on prepare failure Dmitry Baryshkov
@ 2026-10-26 8:59 ` Konrad Dybcio
0 siblings, 0 replies; 20+ messages in thread
From: Konrad Dybcio @ 2026-10-26 8:59 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie,
Simona Vetter, Antonino Maniscalco, Kalyan Thota,
Federico Amedeo Izzo, Helen Koike, Vignesh Raman,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 10/24/26 1:25 AM, Dmitry Baryshkov wrote:
> msm_framebuffer_prepare() bumps prepare_count before pinning, but returns
> straight out of the pin loop on error, leaking the count, the
> msm_gem_vma_get() reference and any planes already pinned.
> drm_atomic_helper_prepare_planes() does not call cleanup_fb() for the
> plane whose prepare_fb() failed, so nothing ever drops it.
>
> Since commit 8ac37c88f991 ("drm/msm: Refcount framebuffer pins") a
> prepare which finds the count already non-zero returns early, assuming
> iova[] is populated. With the count stuck, every later prepare of that
> framebuffer reports success while iova[] is still zero, and DPU scans out
> from a NULL base address:
>
> arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402,
> iova=0x00000100, fsynr=0x3e0023, cbfrsynra=0x1c00, cb=11
>
> Unwind properly on failure instead.
>
> Fixes: 8ac37c88f991 ("drm/msm: Refcount framebuffer pins")
> Assisted-by: LLM
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
LLM:
Regression: writeback path double-decrements prepare_count
DPU writeback swallows the prepare error, so cleanup still runs for
a count that the error unwind has already dropped:
1. dpu_wb_conn_prepare_job() (dpu_writeback.c) calls
dpu_encoder_prepare_wb_job(), which returns void, then always
returns 0.
2. dpu_encoder_phys_wb_prepare_wb_job() calls
msm_framebuffer_prepare(). On failure it logs and returns, so the
error is lost.
3. drm_writeback_prepare_job() therefore sets job->prepared = true.
4. drm_writeback_cleanup_job() (drm_writeback.c:501) calls
dpu_encoder_phys_wb_cleanup_wb_job(), which calls
msm_framebuffer_cleanup().
With patch 2, the failed prepare has already dropped prepare_count
from 1 to 0. Cleanup decrements it again. Because prepare_count is
unsigned (patch 1), it wraps to UINT_MAX and never reaches zero, so
the planes are not unpinned. The next prepare of that framebuffer
does prepare_count++, sees a nonzero old value, and returns success
with iova[] still zero: the original NULL-base-address fault.
Before patch 2 the same path left the count at 1 and cleanup brought
it back to 0, so it was consistent.
Konrad:
I had a hunch to suggest using refcount_t in patch 1 and I think
we can go forward with that
Konrad
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 03/10] drm/msm: release scanout framebuffers only after a vblank
2026-10-23 23:25 [PATCH v5 00/10] drm/msm: fix SMMU fault dumps Dmitry Baryshkov
2026-10-23 23:25 ` [PATCH v5 01/10] drm/msm: serialise framebuffer pin state Dmitry Baryshkov
2026-10-23 23:25 ` [PATCH v5 02/10] drm/msm: fix framebuffer pin refcount leak on prepare failure Dmitry Baryshkov
@ 2026-10-23 23:25 ` Dmitry Baryshkov
2026-10-23 23:25 ` [PATCH v5 04/10] drm/msm/mdp5: flush the interface before starting the timing engine Dmitry Baryshkov
` (6 subsequent siblings)
9 siblings, 0 replies; 20+ messages in thread
From: Dmitry Baryshkov @ 2026-10-23 23:25 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Antonino Maniscalco, Kalyan Thota, Federico Amedeo Izzo,
Helen Koike, Vignesh Raman, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
msm_framebuffer_cleanup() releases a framebuffer as soon as
drm_atomic_helper_cleanup_planes() runs. msm_atomic_commit_tail() waits
only for ->wait_flush() before that, which for video mode waits for
CTL_FLUSH to read back zero, ie. for the new configuration to be latched;
the frame in flight with the old one is still being fetched. Since
commit 111fdd2198e6 ("drm/msm: drm_gpuvm conversion") the unpin also
detaches the vma, so the display is left reading unmapped memory:
arm-smmu 15000000.iommu: Unhandled context fault: fsr=0x402,
iova=0x007eb100, fsynr=0x3f0023, cbfrsynra=0xc20, cb=27
Hand the retired framebuffer to the crtc instead and drop the pin and the
vma reference from a drm_vblank_work, as i915 does for its cursor
framebuffers. The work holds a reference on the framebuffer, and
drm_vblank_work_schedule() holds a vblank reference until it runs.
The pin count is dropped by the deferred work rather than by
->cleanup_fb(), so a framebuffer scanned out by several crtcs stays pinned
until the last of them has passed a vblank, not until the last one has
retired it. Deferring here also covers the async plane update path, which
has no commit tail at all: drm_atomic_helper_async_commit() programs the
hardware and drm_atomic_helper_unprepare_planes() releases the old
framebuffer straight away.
A crtc which is being switched off stops fetching: its encoders are
disabled first, and both DPU and MDP5 turn the timing engine off there
and wait for a vsync to latch it. No further vblank arrives to run the
pending works then, so keep them on a list per crtc and release them by
hand from msm_crtc_vblank_off(). A framebuffer retired once that has
happened is released directly. Whether the vblank is off is tracked
next to the list rather than read from crtc->state, which a following
commit may already have swapped in, and a work is added to the list and
scheduled under the same lock, so that msm_crtc_vblank_off() cannot free
it in between.
Fixes: 111fdd2198e6 ("drm/msm: drm_gpuvm conversion")
Assisted-by: LLM
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 +-
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 3 +-
drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 4 +-
drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 2 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 4 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 2 +-
drivers/gpu/drm/msm/msm_drv.h | 4 +-
drivers/gpu/drm/msm/msm_fb.c | 19 ++-
drivers/gpu/drm/msm/msm_kms.c | 142 +++++++++++++++++++++
drivers/gpu/drm/msm/msm_kms.h | 19 +++
11 files changed, 190 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 42d0a529b4d5..1f8fce366a1d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1213,7 +1213,7 @@ static void dpu_crtc_disable(struct drm_crtc *crtc,
}
/* Disable/save vblank irq handling */
- drm_crtc_vblank_off(crtc);
+ msm_crtc_vblank_off(crtc);
drm_for_each_encoder_mask(encoder, crtc->dev,
old_crtc_state->encoder_mask) {
@@ -1301,7 +1301,7 @@ static void dpu_crtc_enable(struct drm_crtc *crtc,
}
/* Enable/restore vblank irq handling */
- drm_crtc_vblank_on(crtc);
+ msm_crtc_vblank_on(crtc);
}
static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state *cstate)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 22433bfbea1e..5db33e49c345 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -615,7 +615,7 @@ static void dpu_encoder_phys_wb_cleanup_wb_job(struct dpu_encoder_phys *phys_enc
if (!job->fb)
return;
- msm_framebuffer_cleanup(job->fb, false);
+ msm_framebuffer_cleanup(job->fb, NULL, false);
wb_enc->wb_job = NULL;
wb_enc->wb_conn = NULL;
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 7b92082d35a6..0e986b533bf0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -684,7 +684,8 @@ static void dpu_plane_cleanup_fb(struct drm_plane *plane,
DPU_DEBUG_PLANE(pdpu, "FB[%u]\n", old_state->fb->base.id);
- msm_framebuffer_cleanup(old_state->fb, old_pstate->needs_dirtyfb);
+ msm_framebuffer_cleanup(old_state->fb, old_state->crtc,
+ old_pstate->needs_dirtyfb);
}
static int dpu_plane_check_inline_rotation(struct dpu_plane *pdpu,
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
index 57dfce58450b..2d73ac3d85f2 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
@@ -267,7 +267,7 @@ static void mdp4_crtc_atomic_disable(struct drm_crtc *crtc,
return;
/* Disable/save vblank irq handling before power is disabled */
- drm_crtc_vblank_off(crtc);
+ msm_crtc_vblank_off(crtc);
mdp_irq_unregister(&mdp4_kms->base, &mdp4_crtc->err);
mdp4_disable(mdp4_kms);
@@ -297,7 +297,7 @@ static void mdp4_crtc_atomic_enable(struct drm_crtc *crtc,
mdp4_enable(mdp4_kms);
/* Restore vblank irq handling after power is enabled */
- drm_crtc_vblank_on(crtc);
+ msm_crtc_vblank_on(crtc);
mdp_irq_register(&mdp4_kms->base, &mdp4_crtc->err);
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
index 9459f70ce0ba..5f669a02d798 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
@@ -97,7 +97,7 @@ static void mdp4_plane_cleanup_fb(struct drm_plane *plane,
return;
DBG("%s: cleanup: FB[%u]", mdp4_plane->name, fb->base.id);
- msm_framebuffer_cleanup(fb, false);
+ msm_framebuffer_cleanup(fb, old_state->crtc, false);
}
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index 4c4a897fc1ee..e9e55927655e 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -499,7 +499,7 @@ static void mdp5_crtc_atomic_disable(struct drm_crtc *crtc,
return;
/* Disable/save vblank irq handling before power is disabled */
- drm_crtc_vblank_off(crtc);
+ msm_crtc_vblank_off(crtc);
if (mdp5_cstate->cmd_mode)
mdp_irq_unregister(&mdp5_kms->base, &mdp5_crtc->pp_done);
@@ -527,7 +527,7 @@ static void mdp5_crtc_vblank_on(struct drm_crtc *crtc)
count = intf->mode == MDP5_INTF_DSI_MODE_COMMAND ? 0 : 0xffffffff;
drm_crtc_set_max_vblank_count(crtc, count);
- drm_crtc_vblank_on(crtc);
+ msm_crtc_vblank_on(crtc);
}
static void mdp5_crtc_atomic_enable(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 841f444a8d68..dacb387d9bb6 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -155,7 +155,7 @@ static void mdp5_plane_cleanup_fb(struct drm_plane *plane,
return;
DBG("%s: cleanup: FB[%u]", plane->name, fb->base.id);
- msm_framebuffer_cleanup(fb, needed_dirtyfb);
+ msm_framebuffer_cleanup(fb, old_state->crtc, needed_dirtyfb);
}
static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state,
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 2840e8a5052d..9fad2c191ff2 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -254,7 +254,9 @@ int msm_gem_prime_pin(struct drm_gem_object *obj);
void msm_gem_prime_unpin(struct drm_gem_object *obj);
int msm_framebuffer_prepare(struct drm_framebuffer *fb, bool needs_dirtyfb);
-void msm_framebuffer_cleanup(struct drm_framebuffer *fb, bool needed_dirtyfb);
+void msm_framebuffer_cleanup(struct drm_framebuffer *fb, struct drm_crtc *crtc,
+ bool needed_dirtyfb);
+void msm_framebuffer_unpin(struct drm_framebuffer *fb);
uint32_t msm_framebuffer_iova(struct drm_framebuffer *fb, int plane);
struct drm_gem_object *msm_framebuffer_bo(struct drm_framebuffer *fb, int plane);
const struct msm_format *msm_framebuffer_format(struct drm_framebuffer *fb);
diff --git a/drivers/gpu/drm/msm/msm_fb.c b/drivers/gpu/drm/msm/msm_fb.c
index 792d47b4acc4..cc71c42d4e12 100644
--- a/drivers/gpu/drm/msm/msm_fb.c
+++ b/drivers/gpu/drm/msm/msm_fb.c
@@ -130,13 +130,10 @@ int msm_framebuffer_prepare(struct drm_framebuffer *fb, bool needs_dirtyfb)
return 0;
}
-void msm_framebuffer_cleanup(struct drm_framebuffer *fb, bool needed_dirtyfb)
+void msm_framebuffer_unpin(struct drm_framebuffer *fb)
{
struct msm_framebuffer *msm_fb = to_msm_framebuffer(fb);
- if (needed_dirtyfb)
- refcount_dec(&msm_fb->dirtyfb);
-
guard(mutex)(&msm_fb->lock);
if (--msm_fb->prepare_count)
@@ -146,6 +143,20 @@ void msm_framebuffer_cleanup(struct drm_framebuffer *fb, bool needed_dirtyfb)
msm_framebuffer_unpin_planes(fb, fb->format->num_planes);
}
+void msm_framebuffer_cleanup(struct drm_framebuffer *fb, struct drm_crtc *crtc,
+ bool needed_dirtyfb)
+{
+ struct msm_framebuffer *msm_fb = to_msm_framebuffer(fb);
+
+ if (needed_dirtyfb)
+ refcount_dec(&msm_fb->dirtyfb);
+
+ if (crtc && msm_crtc_queue_fb_unpin(crtc, fb))
+ return;
+
+ msm_framebuffer_unpin(fb);
+}
+
uint32_t msm_framebuffer_iova(struct drm_framebuffer *fb, int plane)
{
struct msm_framebuffer *msm_fb = to_msm_framebuffer(fb);
diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c
index 9a207c1e8583..d836d6dd7f3e 100644
--- a/drivers/gpu/drm/msm/msm_kms.c
+++ b/drivers/gpu/drm/msm/msm_kms.c
@@ -11,8 +11,10 @@
#include <uapi/linux/sched/types.h>
#include <drm/drm_drv.h>
+#include <drm/drm_framebuffer.h>
#include <drm/drm_mode_config.h>
#include <drm/drm_vblank.h>
+#include <drm/drm_vblank_work.h>
#include <drm/clients/drm_client_setup.h>
#include "disp/msm_disp_snapshot.h"
@@ -165,6 +167,146 @@ void msm_crtc_disable_vblank(struct drm_crtc *crtc)
vblank_ctrl_queue_work(priv, crtc, false);
}
+struct msm_fb_unpin_work {
+ struct drm_vblank_work base;
+ struct list_head node;
+ struct msm_kms_fb_unpin *pending;
+ struct drm_framebuffer *fb;
+};
+
+static void msm_kms_fb_unpin_release(struct msm_fb_unpin_work *unpin)
+{
+ msm_framebuffer_unpin(unpin->fb);
+ drm_framebuffer_put(unpin->fb);
+ kfree(unpin);
+}
+
+static void msm_kms_fb_unpin_work(struct kthread_work *work)
+{
+ struct msm_fb_unpin_work *unpin =
+ container_of(to_drm_vblank_work(work), struct msm_fb_unpin_work,
+ base);
+ struct msm_kms_fb_unpin *pending = unpin->pending;
+
+ scoped_guard(spinlock, &pending->lock) {
+ if (list_empty(&unpin->node))
+ return;
+ list_del_init(&unpin->node);
+ }
+
+ msm_kms_fb_unpin_release(unpin);
+}
+
+void msm_crtc_vblank_on(struct drm_crtc *crtc)
+{
+ struct msm_drm_private *priv = crtc->dev->dev_private;
+ struct msm_kms *kms = priv->kms;
+ unsigned int idx = drm_crtc_index(crtc);
+
+ drm_crtc_vblank_on(crtc);
+
+ if (!kms || idx >= ARRAY_SIZE(kms->fb_unpin))
+ return;
+
+ scoped_guard(spinlock, &kms->fb_unpin[idx].lock)
+ kms->fb_unpin[idx].vblank_off = false;
+}
+
+/*
+ * The crtc stops fetching here, and with it the vblanks the pending works are
+ * waiting for, so release the framebuffers directly.
+ */
+static void msm_kms_fb_unpin_release_all(struct msm_kms_fb_unpin *pending)
+{
+ struct msm_fb_unpin_work *unpin;
+
+ scoped_guard(spinlock, &pending->lock)
+ pending->vblank_off = true;
+
+ for (;;) {
+ scoped_guard(spinlock, &pending->lock) {
+ unpin = list_first_entry_or_null(&pending->fbs,
+ typeof(*unpin), node);
+ if (unpin)
+ list_del_init(&unpin->node);
+ }
+
+ if (!unpin)
+ break;
+
+ drm_vblank_work_cancel_sync(&unpin->base);
+ msm_kms_fb_unpin_release(unpin);
+ }
+}
+
+void msm_crtc_vblank_off(struct drm_crtc *crtc)
+{
+ struct msm_drm_private *priv = crtc->dev->dev_private;
+ struct msm_kms *kms = priv->kms;
+ unsigned int idx = drm_crtc_index(crtc);
+
+ if (kms && idx < ARRAY_SIZE(kms->fb_unpin))
+ msm_kms_fb_unpin_release_all(&kms->fb_unpin[idx]);
+
+ drm_crtc_vblank_off(crtc);
+}
+
+/*
+ * Publish and schedule the work in one go under the lock, as
+ * msm_crtc_vblank_off() frees whatever it finds on the list.
+ */
+static bool msm_kms_fb_unpin_schedule(struct msm_kms_fb_unpin *pending,
+ struct msm_fb_unpin_work *unpin,
+ struct drm_crtc *crtc)
+{
+ guard(spinlock)(&pending->lock);
+
+ if (pending->vblank_off)
+ return false;
+
+ list_add_tail(&unpin->node, &pending->fbs);
+
+ if (drm_vblank_work_schedule(&unpin->base,
+ drm_crtc_vblank_count(crtc) + 1, true) == 1)
+ return true;
+
+ list_del_init(&unpin->node);
+
+ return false;
+}
+
+bool msm_crtc_queue_fb_unpin(struct drm_crtc *crtc, struct drm_framebuffer *fb)
+{
+ struct msm_drm_private *priv = crtc->dev->dev_private;
+ struct msm_kms *kms = priv->kms;
+ unsigned int idx = drm_crtc_index(crtc);
+ struct msm_kms_fb_unpin *pending;
+ struct msm_fb_unpin_work *unpin;
+
+ if (!kms || idx >= ARRAY_SIZE(kms->fb_unpin))
+ return false;
+
+ pending = &kms->fb_unpin[idx];
+
+ unpin = kzalloc_obj(*unpin);
+ if (!unpin)
+ return false;
+
+ unpin->fb = fb;
+ unpin->pending = pending;
+ drm_framebuffer_get(fb);
+
+ drm_vblank_work_init(&unpin->base, crtc, msm_kms_fb_unpin_work);
+
+ if (msm_kms_fb_unpin_schedule(pending, unpin, crtc))
+ return true;
+
+ drm_framebuffer_put(fb);
+ kfree(unpin);
+
+ return false;
+}
+
static int msm_kms_fault_handler(void *arg, unsigned long iova, int flags, void *data)
{
struct msm_kms *kms = arg;
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index b5e1cd38cd8a..a33c2a0db206 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -133,6 +133,13 @@ struct msm_drm_thread {
struct kthread_worker *worker;
};
+struct msm_kms_fb_unpin {
+ /* protects the list of framebuffers waiting for a vblank: */
+ spinlock_t lock;
+ struct list_head fbs;
+ bool vblank_off;
+};
+
struct msm_kms {
const struct msm_kms_funcs *funcs;
struct drm_device *dev;
@@ -168,8 +175,14 @@ struct msm_kms {
struct workqueue_struct *wq;
struct msm_drm_thread event_thread[MAX_CRTCS];
+
+ struct msm_kms_fb_unpin fb_unpin[MAX_CRTCS];
};
+bool msm_crtc_queue_fb_unpin(struct drm_crtc *crtc, struct drm_framebuffer *fb);
+void msm_crtc_vblank_on(struct drm_crtc *crtc);
+void msm_crtc_vblank_off(struct drm_crtc *crtc);
+
static inline int msm_kms_init(struct msm_kms *kms,
const struct msm_kms_funcs *funcs)
{
@@ -179,6 +192,12 @@ static inline int msm_kms_init(struct msm_kms *kms,
for (i = 0; i < ARRAY_SIZE(kms->commit_lock); i++)
mutex_init(&kms->commit_lock[i]);
+ for (i = 0; i < ARRAY_SIZE(kms->fb_unpin); i++) {
+ spin_lock_init(&kms->fb_unpin[i].lock);
+ INIT_LIST_HEAD(&kms->fb_unpin[i].fbs);
+ kms->fb_unpin[i].vblank_off = true;
+ }
+
kms->funcs = funcs;
kms->wq = alloc_ordered_workqueue("msm", 0);
--
2.47.3
^ permalink raw reply [flat|nested] 20+ messages in thread* [PATCH v5 04/10] drm/msm/mdp5: flush the interface before starting the timing engine
2026-10-23 23:25 [PATCH v5 00/10] drm/msm: fix SMMU fault dumps Dmitry Baryshkov
` (2 preceding siblings ...)
2026-10-23 23:25 ` [PATCH v5 03/10] drm/msm: release scanout framebuffers only after a vblank Dmitry Baryshkov
@ 2026-10-23 23:25 ` Dmitry Baryshkov
2026-10-26 9:11 ` Konrad Dybcio
2026-10-23 23:25 ` [PATCH v5 05/10] drm/msm/dpu: clear the CWB blocks left by a previous reservation Dmitry Baryshkov
` (5 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Dmitry Baryshkov @ 2026-10-23 23:25 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Antonino Maniscalco, Kalyan Thota, Federico Amedeo Izzo,
Helen Koike, Vignesh Raman, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
Disabling a CRTC and then enabling it again can produce an SMMU fault on
MSM8996 as soon as the timing engine restarts, e.g. when fbcon takes over
after kms_atomic:
arm-smmu d00000.iommu: Unhandled context fault: fsr=0x402, iova=0x01fa6e00
The faulting address is inside the framebuffer that was scanned out
before the disable, which has been released since. The pipes keep that
address latched (CURRENT_SRC0_ADDR still points to the released buffer)
because the plane removal flushed after the timing engine stopped never
takes effect. On the next enable the pipes are programmed and flushed
before the encoder is enabled, but that flush doesn't include the
interface: the encoder enables the timing engine first and only then
flushes the INTF. The first frame is fetched with the latched pipe
configuration, from the released buffer.
Flush the interface before enabling the timing engine, the order used by
both the DPU driver and the downstream mdss driver. With the INTF flush
pending, the whole pending flush takes effect when the timing engine
starts and the first frame is fetched with the new configuration.
Fixes: 06c0dd96bfbb ("drm/msm: add mdp5/apq8x74")
Assisted-by: LLM
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
index eaba3b2d73b5..e9eb1db92775 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_encoder.c
@@ -164,10 +164,11 @@ static void mdp5_vid_encoder_enable(struct drm_encoder *encoder)
if (WARN_ON(mdp5_encoder->enabled))
return;
+ /* the pipes fetch the first frame with stale addresses otherwise */
+ mdp5_ctl_commit(ctl, pipeline, mdp_ctl_flush_mask_encoder(intf), true);
spin_lock_irqsave(&mdp5_encoder->intf_lock, flags);
mdp5_write(mdp5_kms, REG_MDP5_INTF_TIMING_ENGINE_EN(intfn), 1);
spin_unlock_irqrestore(&mdp5_encoder->intf_lock, flags);
- mdp5_ctl_commit(ctl, pipeline, mdp_ctl_flush_mask_encoder(intf), true);
mdp5_ctl_set_encoder_state(ctl, pipeline, true);
--
2.47.3
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 04/10] drm/msm/mdp5: flush the interface before starting the timing engine
2026-10-23 23:25 ` [PATCH v5 04/10] drm/msm/mdp5: flush the interface before starting the timing engine Dmitry Baryshkov
@ 2026-10-26 9:11 ` Konrad Dybcio
0 siblings, 0 replies; 20+ messages in thread
From: Konrad Dybcio @ 2026-10-26 9:11 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie,
Simona Vetter, Antonino Maniscalco, Kalyan Thota,
Federico Amedeo Izzo, Helen Koike, Vignesh Raman,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 10/24/26 1:25 AM, Dmitry Baryshkov wrote:
> Disabling a CRTC and then enabling it again can produce an SMMU fault on
> MSM8996 as soon as the timing engine restarts, e.g. when fbcon takes over
> after kms_atomic:
>
> arm-smmu d00000.iommu: Unhandled context fault: fsr=0x402, iova=0x01fa6e00
>
> The faulting address is inside the framebuffer that was scanned out
> before the disable, which has been released since. The pipes keep that
> address latched (CURRENT_SRC0_ADDR still points to the released buffer)
> because the plane removal flushed after the timing engine stopped never
> takes effect. On the next enable the pipes are programmed and flushed
> before the encoder is enabled, but that flush doesn't include the
> interface: the encoder enables the timing engine first and only then
> flushes the INTF. The first frame is fetched with the latched pipe
> configuration, from the released buffer.
>
> Flush the interface before enabling the timing engine, the order used by
> both the DPU driver and the downstream mdss driver. With the INTF flush
> pending, the whole pending flush takes effect when the timing engine
> starts and the first frame is fetched with the new configuration.
>
> Fixes: 06c0dd96bfbb ("drm/msm: add mdp5/apq8x74")
> Assisted-by: LLM
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 05/10] drm/msm/dpu: clear the CWB blocks left by a previous reservation
2026-10-23 23:25 [PATCH v5 00/10] drm/msm: fix SMMU fault dumps Dmitry Baryshkov
` (3 preceding siblings ...)
2026-10-23 23:25 ` [PATCH v5 04/10] drm/msm/mdp5: flush the interface before starting the timing engine Dmitry Baryshkov
@ 2026-10-23 23:25 ` Dmitry Baryshkov
2026-10-26 9:13 ` Konrad Dybcio
2026-10-23 23:25 ` [PATCH v5 06/10] drm/msm/dpu: clear the CDM block " Dmitry Baryshkov
` (4 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Dmitry Baryshkov @ 2026-10-23 23:25 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Antonino Maniscalco, Kalyan Thota, Federico Amedeo Izzo,
Helen Koike, Vignesh Raman, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
dpu_encoder_virt_atomic_mode_set() only writes the first num_cwb entries
of dpu_enc->hw_cwb[], and not at all when the encoder is not the
writeback of a clone, so a modeset which drops the clone, or reserves
fewer CWB blocks, keeps the pointers from the previous reservation.
dpu_encoder_helper_phys_setup_cwb() then walks every non-NULL entry: it
programs a CWB mux the crtc no longer owns, and looks its pingpong up past
the pingpongs actually assigned, in the uninitialised part of rt_pp_list[].
Store NULL for the entries which have no CWB block, as is done for the
pingpongs.
Fixes: 5008375443ed ("drm/msm/dpu: Reserve resources for CWB")
Assisted-by: LLM
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0a6be69a3e83..8fcd0b4e266c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1205,9 +1205,10 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
ARRAY_SIZE(hw_pp));
}
- for (i = 0; i < num_cwb; i++) {
- dpu_enc->hw_cwb[i] = to_dpu_hw_cwb(hw_cwb[i]);
- cwb_mask |= BIT(dpu_enc->hw_cwb[i]->idx - CWB_0);
+ for (i = 0; i < MAX_CWB_PER_ENC; i++) {
+ dpu_enc->hw_cwb[i] = i < num_cwb ? to_dpu_hw_cwb(hw_cwb[i]) : NULL;
+ if (dpu_enc->hw_cwb[i])
+ cwb_mask |= BIT(dpu_enc->hw_cwb[i]->idx - CWB_0);
}
dpu_enc->cwb_mask = cwb_mask;
--
2.47.3
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 05/10] drm/msm/dpu: clear the CWB blocks left by a previous reservation
2026-10-23 23:25 ` [PATCH v5 05/10] drm/msm/dpu: clear the CWB blocks left by a previous reservation Dmitry Baryshkov
@ 2026-10-26 9:13 ` Konrad Dybcio
0 siblings, 0 replies; 20+ messages in thread
From: Konrad Dybcio @ 2026-10-26 9:13 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie,
Simona Vetter, Antonino Maniscalco, Kalyan Thota,
Federico Amedeo Izzo, Helen Koike, Vignesh Raman,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 10/24/26 1:25 AM, Dmitry Baryshkov wrote:
> dpu_encoder_virt_atomic_mode_set() only writes the first num_cwb entries
> of dpu_enc->hw_cwb[], and not at all when the encoder is not the
> writeback of a clone, so a modeset which drops the clone, or reserves
> fewer CWB blocks, keeps the pointers from the previous reservation.
> dpu_encoder_helper_phys_setup_cwb() then walks every non-NULL entry: it
> programs a CWB mux the crtc no longer owns, and looks its pingpong up past
> the pingpongs actually assigned, in the uninitialised part of rt_pp_list[].
>
> Store NULL for the entries which have no CWB block, as is done for the
> pingpongs.
>
> Fixes: 5008375443ed ("drm/msm/dpu: Reserve resources for CWB")
> Assisted-by: LLM
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
Acked-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 06/10] drm/msm/dpu: clear the CDM block left by a previous reservation
2026-10-23 23:25 [PATCH v5 00/10] drm/msm: fix SMMU fault dumps Dmitry Baryshkov
` (4 preceding siblings ...)
2026-10-23 23:25 ` [PATCH v5 05/10] drm/msm/dpu: clear the CWB blocks left by a previous reservation Dmitry Baryshkov
@ 2026-10-23 23:25 ` Dmitry Baryshkov
2026-10-26 9:15 ` Konrad Dybcio
2026-10-23 23:25 ` [PATCH v5 07/10] drm/msm/dpu: clear the DSPP pointer when no DSPP is assigned Dmitry Baryshkov
` (3 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Dmitry Baryshkov @ 2026-10-23 23:25 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Antonino Maniscalco, Kalyan Thota, Federico Amedeo Izzo,
Helen Koike, Vignesh Raman, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
dpu_encoder_virt_atomic_mode_set() only updates cur_master->hw_cdm for a
writeback with a job or for DP, so a modeset of a writeback without a job
keeps the CDM of the previous reservation, which the crtc may no longer
own. dpu_encoder_helper_phys_cleanup() later unbinds that CDM from its
pingpong and flushes it, changing a block which may by then belong to
another display.
Store NULL when no CDM is assigned.
Fixes: 8b45a26f2ba9 ("drm/msm/dpu: reserve cdm blocks for writeback in case of YUV output")
Assisted-by: LLM
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 8fcd0b4e266c..b337f197d3f5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1160,6 +1160,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
struct dpu_hw_blk *hw_cwb[MAX_CHANNELS_PER_ENC];
+ struct dpu_hw_blk *hw_cdm = NULL;
int num_ctl, num_pp, num_dsc, num_pp_per_intf;
int num_cwb = 0;
bool is_cwb_encoder;
@@ -1231,14 +1232,12 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
dpu_enc->dsc_mask = dsc_mask;
if ((dpu_enc->disp_info.intf_type == INTF_WB && conn_state->writeback_job) ||
- dpu_enc->disp_info.intf_type == INTF_DP) {
- struct dpu_hw_blk *hw_cdm = NULL;
-
+ dpu_enc->disp_info.intf_type == INTF_DP)
dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
drm_enc->crtc, DPU_HW_BLK_CDM,
&hw_cdm, 1);
- dpu_enc->cur_master->hw_cdm = hw_cdm ? to_dpu_hw_cdm(hw_cdm) : NULL;
- }
+
+ dpu_enc->cur_master->hw_cdm = hw_cdm ? to_dpu_hw_cdm(hw_cdm) : NULL;
/*
* There may be 4 PP and 2 INTF for quad pipe case, so INTF is not
--
2.47.3
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 06/10] drm/msm/dpu: clear the CDM block left by a previous reservation
2026-10-23 23:25 ` [PATCH v5 06/10] drm/msm/dpu: clear the CDM block " Dmitry Baryshkov
@ 2026-10-26 9:15 ` Konrad Dybcio
0 siblings, 0 replies; 20+ messages in thread
From: Konrad Dybcio @ 2026-10-26 9:15 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie,
Simona Vetter, Antonino Maniscalco, Kalyan Thota,
Federico Amedeo Izzo, Helen Koike, Vignesh Raman,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 10/24/26 1:25 AM, Dmitry Baryshkov wrote:
> dpu_encoder_virt_atomic_mode_set() only updates cur_master->hw_cdm for a
> writeback with a job or for DP, so a modeset of a writeback without a job
> keeps the CDM of the previous reservation, which the crtc may no longer
> own. dpu_encoder_helper_phys_cleanup() later unbinds that CDM from its
> pingpong and flushes it, changing a block which may by then belong to
> another display.
>
> Store NULL when no CDM is assigned.
>
> Fixes: 8b45a26f2ba9 ("drm/msm/dpu: reserve cdm blocks for writeback in case of YUV output")
> Assisted-by: LLM
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 07/10] drm/msm/dpu: clear the DSPP pointer when no DSPP is assigned
2026-10-23 23:25 [PATCH v5 00/10] drm/msm: fix SMMU fault dumps Dmitry Baryshkov
` (5 preceding siblings ...)
2026-10-23 23:25 ` [PATCH v5 06/10] drm/msm/dpu: clear the CDM block " Dmitry Baryshkov
@ 2026-10-23 23:25 ` Dmitry Baryshkov
2026-10-26 9:15 ` Konrad Dybcio
2026-10-23 23:25 ` [PATCH v5 08/10] drm/msm/dpu: clear the DSC blocks left by a previous reservation Dmitry Baryshkov
` (2 subsequent siblings)
9 siblings, 1 reply; 20+ messages in thread
From: Dmitry Baryshkov @ 2026-10-23 23:25 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Antonino Maniscalco, Kalyan Thota, Federico Amedeo Izzo,
Helen Koike, Vignesh Raman, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
dpu_crtc_assign_resources() only stores mixers[i].hw_dspp when the new
topology has a DSPP for that mixer, and dpu_crtc_duplicate_state() copies
the state with kmemdup(), so a crtc which once used a DSPP keeps a pointer
to it for the rest of its life -- across reservations which no longer own
that block.
Assign NULL when there is no DSPP for the mixer.
Fixes: 4259ff7ae509 ("drm/msm/dpu: add support for pcc color block in dpu driver")
Assisted-by: LLM
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 1f8fce366a1d..2ff0dab33710 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1480,8 +1480,8 @@ static int dpu_crtc_assign_resources(struct drm_crtc *crtc,
cstate->mixers[i].hw_lm = to_dpu_hw_mixer(hw_lm[i]);
cstate->mixers[i].lm_ctl = to_dpu_hw_ctl(hw_ctl[ctl_idx]);
- if (i < num_dspp)
- cstate->mixers[i].hw_dspp = to_dpu_hw_dspp(hw_dspp[i]);
+ cstate->mixers[i].hw_dspp = i < num_dspp ?
+ to_dpu_hw_dspp(hw_dspp[i]) : NULL;
}
cstate->num_mixers = num_lm;
--
2.47.3
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 07/10] drm/msm/dpu: clear the DSPP pointer when no DSPP is assigned
2026-10-23 23:25 ` [PATCH v5 07/10] drm/msm/dpu: clear the DSPP pointer when no DSPP is assigned Dmitry Baryshkov
@ 2026-10-26 9:15 ` Konrad Dybcio
0 siblings, 0 replies; 20+ messages in thread
From: Konrad Dybcio @ 2026-10-26 9:15 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie,
Simona Vetter, Antonino Maniscalco, Kalyan Thota,
Federico Amedeo Izzo, Helen Koike, Vignesh Raman,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 10/24/26 1:25 AM, Dmitry Baryshkov wrote:
> dpu_crtc_assign_resources() only stores mixers[i].hw_dspp when the new
> topology has a DSPP for that mixer, and dpu_crtc_duplicate_state() copies
> the state with kmemdup(), so a crtc which once used a DSPP keeps a pointer
> to it for the rest of its life -- across reservations which no longer own
> that block.
>
> Assign NULL when there is no DSPP for the mixer.
>
> Fixes: 4259ff7ae509 ("drm/msm/dpu: add support for pcc color block in dpu driver")
> Assisted-by: LLM
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 08/10] drm/msm/dpu: clear the DSC blocks left by a previous reservation
2026-10-23 23:25 [PATCH v5 00/10] drm/msm: fix SMMU fault dumps Dmitry Baryshkov
` (6 preceding siblings ...)
2026-10-23 23:25 ` [PATCH v5 07/10] drm/msm/dpu: clear the DSPP pointer when no DSPP is assigned Dmitry Baryshkov
@ 2026-10-23 23:25 ` Dmitry Baryshkov
2026-10-26 9:16 ` Konrad Dybcio
2026-10-23 23:25 ` [PATCH v5 09/10] drm/msm/dpu: only reassign resources when the encoder is reprogrammed Dmitry Baryshkov
2026-10-23 23:25 ` [PATCH v5 10/10] drm/ci: mark pixel-format tests as passing on SC7180 Dmitry Baryshkov
9 siblings, 1 reply; 20+ messages in thread
From: Dmitry Baryshkov @ 2026-10-23 23:25 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Antonino Maniscalco, Kalyan Thota, Federico Amedeo Izzo,
Helen Koike, Vignesh Raman, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
dpu_encoder_virt_atomic_mode_set() fills dpu_enc->hw_dsc[] with the DSC
blocks of the new reservation, but only the first num_dsc entries. The
array is never cleared, so an encoder which once drove two DSC blocks
keeps pointing at the second one after it has been given a topology with
a single DSC.
dpu_encoder_use_dsc_merge() counts the non-NULL entries of the whole
array and compares them against the number of interfaces, so the leftover
pointer makes it report DSC merge for a topology which has none, and
dpu_encoder_prep_dsc() then programs DSC_MODE_MULTIPLEX into the only DSC
block that is really used.
Assign NULL when the new reservation has no block for the slot.
Fixes: 58dca9810749 ("drm/msm/disp/dpu1: Add support for DSC in encoder")
Assisted-by: LLM
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index b337f197d3f5..419e0c6e8ef1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1224,9 +1224,10 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
num_dsc = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
drm_enc->crtc, DPU_HW_BLK_DSC,
hw_dsc, ARRAY_SIZE(hw_dsc));
- for (i = 0; i < num_dsc; i++) {
- dpu_enc->hw_dsc[i] = to_dpu_hw_dsc(hw_dsc[i]);
- dsc_mask |= BIT(dpu_enc->hw_dsc[i]->idx - DSC_0);
+ for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
+ dpu_enc->hw_dsc[i] = i < num_dsc ? to_dpu_hw_dsc(hw_dsc[i]) : NULL;
+ if (dpu_enc->hw_dsc[i])
+ dsc_mask |= BIT(dpu_enc->hw_dsc[i]->idx - DSC_0);
}
dpu_enc->dsc_mask = dsc_mask;
--
2.47.3
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 08/10] drm/msm/dpu: clear the DSC blocks left by a previous reservation
2026-10-23 23:25 ` [PATCH v5 08/10] drm/msm/dpu: clear the DSC blocks left by a previous reservation Dmitry Baryshkov
@ 2026-10-26 9:16 ` Konrad Dybcio
0 siblings, 0 replies; 20+ messages in thread
From: Konrad Dybcio @ 2026-10-26 9:16 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie,
Simona Vetter, Antonino Maniscalco, Kalyan Thota,
Federico Amedeo Izzo, Helen Koike, Vignesh Raman,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 10/24/26 1:25 AM, Dmitry Baryshkov wrote:
> dpu_encoder_virt_atomic_mode_set() fills dpu_enc->hw_dsc[] with the DSC
> blocks of the new reservation, but only the first num_dsc entries. The
> array is never cleared, so an encoder which once drove two DSC blocks
> keeps pointing at the second one after it has been given a topology with
> a single DSC.
>
> dpu_encoder_use_dsc_merge() counts the non-NULL entries of the whole
> array and compares them against the number of interfaces, so the leftover
> pointer makes it report DSC merge for a topology which has none, and
> dpu_encoder_prep_dsc() then programs DSC_MODE_MULTIPLEX into the only DSC
> block that is really used.
>
> Assign NULL when the new reservation has no block for the slot.
>
> Fixes: 58dca9810749 ("drm/msm/disp/dpu1: Add support for DSC in encoder")
> Assisted-by: LLM
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 09/10] drm/msm/dpu: only reassign resources when the encoder is reprogrammed
2026-10-23 23:25 [PATCH v5 00/10] drm/msm: fix SMMU fault dumps Dmitry Baryshkov
` (7 preceding siblings ...)
2026-10-23 23:25 ` [PATCH v5 08/10] drm/msm/dpu: clear the DSC blocks left by a previous reservation Dmitry Baryshkov
@ 2026-10-23 23:25 ` Dmitry Baryshkov
2026-10-26 9:19 ` Konrad Dybcio
2026-10-23 23:25 ` [PATCH v5 10/10] drm/ci: mark pixel-format tests as passing on SC7180 Dmitry Baryshkov
9 siblings, 1 reply; 20+ messages in thread
From: Dmitry Baryshkov @ 2026-10-23 23:25 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Antonino Maniscalco, Kalyan Thota, Federico Amedeo Izzo,
Helen Koike, Vignesh Raman, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
Running kms_plane@pixel-format-source-clamping causes SMMU faults
(fsr=0x402, iova=0x0001fb00): it sets a CTM, after which the display stops
updating, so the test times out waiting for a CRC and the pipe is still
fetching the framebuffers released when the runner kills it.
dpu_crtc_atomic_check() reassigns resources whenever color_mgmt_changed is
set, but crtc_set_mode() only calls ->atomic_mode_set() for mode_changed or
connectors_changed. A colour management update therefore re-reserves and
can get a different CTL: cstate->mixers[].lm_ctl points at the new one
while phys->hw_ctl still points at the old, so the flush mask is
accumulated on one and triggered on the other. CTL_FLUSH is written with
zero, so the pipe is not reprogrammed again until the next modeset.
Reassign only when the encoder is reprogrammed too. Enabling colour
management still needs a modeset, as that is when DSPPs join the topology,
so ask for one in dpu_crtc_check_mode_changed() and only there. Drop the
msm_atomic_check() hack, which forced a modeset both ways, ignored
GAMMA_LUT and overrode allow_modeset behind userspace's back.
Fixes: 39a750ff5fc9 ("drm/msm/dpu: Add DSPP GC driver to provide GAMMA_LUT DRM property")
Assisted-by: LLM
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 28 ++++++++++++++++++++++++++--
drivers/gpu/drm/msm/msm_atomic.c | 17 +----------------
2 files changed, 27 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 2ff0dab33710..048800227ad3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1489,6 +1489,26 @@ static int dpu_crtc_assign_resources(struct drm_crtc *crtc,
return 0;
}
+static bool dpu_crtc_needs_dspp(const struct drm_crtc_state *crtc_state)
+{
+ return crtc_state->ctm || crtc_state->gamma_lut;
+}
+
+static bool dpu_crtc_has_dspp(const struct drm_crtc_state *crtc_state)
+{
+ const struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
+ unsigned int i;
+
+ if (!cstate->num_mixers)
+ return false;
+
+ for (i = 0; i < cstate->num_mixers; i++)
+ if (!cstate->mixers[i].hw_dspp)
+ return false;
+
+ return true;
+}
+
/**
* dpu_crtc_check_mode_changed: check if full modeset is required
* @old_crtc_state: Previous CRTC state
@@ -1506,6 +1526,11 @@ int dpu_crtc_check_mode_changed(struct drm_crtc_state *old_crtc_state,
DRM_DEBUG_ATOMIC("%d\n", crtc->base.id);
+ /* DSPPs are only reserved during a modeset */
+ if (dpu_crtc_needs_dspp(new_crtc_state) &&
+ !dpu_crtc_has_dspp(old_crtc_state))
+ new_crtc_state->mode_changed = true;
+
/* there might be cases where encoder needs a modeset too */
drm_for_each_encoder_mask(drm_enc, crtc->dev, new_crtc_state->encoder_mask) {
if (dpu_encoder_needs_modeset(drm_enc, new_crtc_state->state))
@@ -1535,8 +1560,7 @@ 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) {
+ if (crtc_state->mode_changed || crtc_state->connectors_changed) {
rc = dpu_crtc_assign_resources(crtc, crtc_state);
if (rc < 0)
return rc;
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index c26b0c7acdfa..db583cee7f98 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -186,22 +186,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.47.3
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 09/10] drm/msm/dpu: only reassign resources when the encoder is reprogrammed
2026-10-23 23:25 ` [PATCH v5 09/10] drm/msm/dpu: only reassign resources when the encoder is reprogrammed Dmitry Baryshkov
@ 2026-10-26 9:19 ` Konrad Dybcio
0 siblings, 0 replies; 20+ messages in thread
From: Konrad Dybcio @ 2026-10-26 9:19 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie,
Simona Vetter, Antonino Maniscalco, Kalyan Thota,
Federico Amedeo Izzo, Helen Koike, Vignesh Raman,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 10/24/26 1:25 AM, Dmitry Baryshkov wrote:
> Running kms_plane@pixel-format-source-clamping causes SMMU faults
> (fsr=0x402, iova=0x0001fb00): it sets a CTM, after which the display stops
> updating, so the test times out waiting for a CRC and the pipe is still
> fetching the framebuffers released when the runner kills it.
>
> dpu_crtc_atomic_check() reassigns resources whenever color_mgmt_changed is
> set, but crtc_set_mode() only calls ->atomic_mode_set() for mode_changed or
> connectors_changed. A colour management update therefore re-reserves and
> can get a different CTL: cstate->mixers[].lm_ctl points at the new one
> while phys->hw_ctl still points at the old, so the flush mask is
> accumulated on one and triggered on the other. CTL_FLUSH is written with
> zero, so the pipe is not reprogrammed again until the next modeset.
>
> Reassign only when the encoder is reprogrammed too. Enabling colour
> management still needs a modeset, as that is when DSPPs join the topology,
> so ask for one in dpu_crtc_check_mode_changed() and only there. Drop the
> msm_atomic_check() hack, which forced a modeset both ways, ignored
> GAMMA_LUT and overrode allow_modeset behind userspace's back.
>
> Fixes: 39a750ff5fc9 ("drm/msm/dpu: Add DSPP GC driver to provide GAMMA_LUT DRM property")
> Assisted-by: LLM
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
LLM is curious whether turning off CTM/GAMMA should trigger a modeset too.
My hunch is "yes"
Konrad
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v5 10/10] drm/ci: mark pixel-format tests as passing on SC7180
2026-10-23 23:25 [PATCH v5 00/10] drm/msm: fix SMMU fault dumps Dmitry Baryshkov
` (8 preceding siblings ...)
2026-10-23 23:25 ` [PATCH v5 09/10] drm/msm/dpu: only reassign resources when the encoder is reprogrammed Dmitry Baryshkov
@ 2026-10-23 23:25 ` Dmitry Baryshkov
2026-10-26 9:19 ` Konrad Dybcio
9 siblings, 1 reply; 20+ messages in thread
From: Dmitry Baryshkov @ 2026-10-23 23:25 UTC (permalink / raw)
To: Rob Clark, Dmitry Baryshkov, Abhinav Kumar, Jessica Zhang,
Sean Paul, Marijn Suijten, David Airlie, Simona Vetter,
Antonino Maniscalco, Kalyan Thota, Federico Amedeo Izzo,
Helen Koike, Vignesh Raman, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
kms_plane@pixel-format and kms_plane@pixel-format-source-clamping timed out
waiting for the first CRC once the test had set a legacy gamma LUT. They
pass since "drm/msm/dpu: only reassign resources when the encoder is
reprogrammed", so drop them from the expected failures.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
drivers/gpu/drm/ci/xfails/msm-sc7180-trogdor-kingoftown-fails.txt | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/ci/xfails/msm-sc7180-trogdor-kingoftown-fails.txt b/drivers/gpu/drm/ci/xfails/msm-sc7180-trogdor-kingoftown-fails.txt
index 2daf4ab879a9..ea3013faa37c 100644
--- a/drivers/gpu/drm/ci/xfails/msm-sc7180-trogdor-kingoftown-fails.txt
+++ b/drivers/gpu/drm/ci/xfails/msm-sc7180-trogdor-kingoftown-fails.txt
@@ -18,5 +18,3 @@ kms_pipe_crc_basic@compare-crc-sanitycheck-nv12,Fail
kms_plane_alpha_blend@alpha-7efc,Fail
kms_plane_alpha_blend@coverage-7efc,Fail
kms_plane_alpha_blend@coverage-vs-premult-vs-constant,Fail
-kms_plane@pixel-format,Fail
-kms_plane@pixel-format-source-clamping,Fail
--
2.47.3
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v5 10/10] drm/ci: mark pixel-format tests as passing on SC7180
2026-10-23 23:25 ` [PATCH v5 10/10] drm/ci: mark pixel-format tests as passing on SC7180 Dmitry Baryshkov
@ 2026-10-26 9:19 ` Konrad Dybcio
0 siblings, 0 replies; 20+ messages in thread
From: Konrad Dybcio @ 2026-10-26 9:19 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Sean Paul, Marijn Suijten, David Airlie,
Simona Vetter, Antonino Maniscalco, Kalyan Thota,
Federico Amedeo Izzo, Helen Koike, Vignesh Raman,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
On 10/24/26 1:25 AM, Dmitry Baryshkov wrote:
> kms_plane@pixel-format and kms_plane@pixel-format-source-clamping timed out
> waiting for the first CRC once the test had set a legacy gamma LUT. They
> pass since "drm/msm/dpu: only reassign resources when the encoder is
> reprogrammed", so drop them from the expected failures.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
Acked-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply [flat|nested] 20+ messages in thread