Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm: sleepable mutex lock in atomic context (plugged_lock) --
commit cb0347df94f2c11e58ecb3d98faa755fd4b526ef Author: Xilin Wu <[email protected]> drm/msm/dp: Serialize stream operations with HPD processing HPD PHY test requests stop and restart the mainlink under plugged_lock, but atomic stream enable and disable do not take that lock. Both paths can check a resource flag before either updates it and acquire duplicate clock or PHY references. They can also reset a link while the other path is training it or enabling video. > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index 32a2ac89d8ed0..787efbcb6c124 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c [ ... ] > @@ -1449,7 +1468,13 @@ void msm_dp_display_atomic_enable(struct msm_dp > *msm_dp_display, > > dp = container_of(msm_dp_display, struct msm_dp_display_private, > msm_dp_display); > > - rc = msm_dp_display_prepare_link(dp); > + rc = msm_dp_display_prepare(dp); > + if (rc) > + return; > + > + guard(mutex)(&dp->plugged_lock); [Severity: High] Does this introduce a sleeping lock into an atomic context? The DRM subsystem guidelines state that encoder atomic enable/disable callbacks run in atomic context during non-blocking commits. Calling sleeping functions, like acquiring a mutex, from an atomic context can cause kernel warnings, system instability, and potential deadlocks. > + > + rc = msm_dp_display_prepare_link_locked(dp); > if (rc) { > DRM_ERROR("DP display prepare failed, rc=%d\n", rc); > return; [ ... ] > @@ -1474,6 +1499,8 @@ void msm_dp_display_atomic_disable(struct msm_dp *dp) > > msm_dp_display = container_of(dp, struct msm_dp_display_private, > msm_dp_display); > > + guard(mutex)(&msm_dp_display->plugged_lock); [Severity: High] Does this also introduce a sleeping lock into an atomic disable callback? > + > /* > * If .atomic_enable() bailed out - link training failure is the common > * case - the mainlink was never brought up and ->power_on stayed false. [ ... ] > @@ -1511,7 +1538,8 @@ void msm_dp_display_atomic_post_disable(struct msm_dp > *dp) > > msm_dp_display_audio_notify_disable(msm_dp_display); > > - msm_dp_display_disable(msm_dp_display, msm_dp_display->panel); > + scoped_guard(mutex, &msm_dp_display->plugged_lock) > + msm_dp_display_disable(msm_dp_display, msm_dp_display->panel); [Severity: High] Could this scoped_guard() cause a sleep in atomic context during the post-disable phase of an atomic commit? > > msm_dp_display_unprepare(msm_dp_display); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
