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 <[email protected]>
> ---
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
