Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/msm: premature unpin of framebuffers causing IOMMU fault --
commit 86394032733ab2293f0e387866c3cbd603bc3285 Author: Dmitry Baryshkov <[email protected]> Subject: drm/msm: release scanout framebuffers only after a vblank This commit defers scanout framebuffer release until a vblank by handing them to a drm_vblank_work per CRTC. On CRTC disable, pending works are flushed and framebuffers are unpinned directly from msm_crtc_vblank_off(). > diff --git a/drivers/gpu/drm/msm/msm_kms.c b/drivers/gpu/drm/msm/msm_kms.c > index 7e1df472592d3..cb497d419ab83 100644 > --- a/drivers/gpu/drm/msm/msm_kms.c > +++ b/drivers/gpu/drm/msm/msm_kms.c [ ... ] > +/* > + * 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) > +{ [ ... ] > + 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); > +} [ ... ] > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 77a2d1f6700d0..35daf46fc4257 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); [Severity: High] Can this lead to an IOMMU context fault if the display hardware is still fetching the frame? When a userspace application issues an async plane update followed closely by a CRTC disable, the ongoing hardware fetch might hit an IOMMU fault because the memory is unmapped immediately during the disable sequence. Because msm_crtc_vblank_off() now synchronously unpins and unmaps all pending framebuffers via msm_kms_fb_unpin_release_all(), executing this before the hardware is done fetching creates a race window. Looking at dpu_crtc_disable(), we wait for the hardware to finish its current frame fetch much later in the function: drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:dpu_crtc_disable() { ... msm_crtc_vblank_off(crtc); ... if (_dpu_crtc_wait_for_frame_done(crtc)) DPU_ERROR(...); ... } Does this sequence leave a race window where the display is left reading unmapped memory? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
