Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] `lsdc_crtc1_soft_reset` calls `msleep(20)`, which can sleep, but it is reachable from the CRTC atomic enable callback (`lsdc_crtc_atomic_enable`), which runs in atomic context. -- commit 4cb085e0fbbbf2cb0c2550f2c69cc0175ba5ecf8 Author: Maxime Ripard <[email protected]> drm/loongson: Move hardware reset to CRTC creation This patch moves the hardware-level CRTC reset from the atomic state initialization hook to the CRTC creation path. It adds a new hw_reset callback to handle the register writes independently. > diff --git a/drivers/gpu/drm/loongson/lsdc_crtc.c > b/drivers/gpu/drm/loongson/lsdc_crtc.c > index 16b8f36a40710..7cca574e3b6af 100644 > --- a/drivers/gpu/drm/loongson/lsdc_crtc.c > +++ b/drivers/gpu/drm/loongson/lsdc_crtc.c [ ... ] > @@ -937,6 +932,15 @@ static bool lsdc_crtc_get_scanout_position(struct > drm_crtc *crtc, > return true; > } > > +static void lsdc_crtc_hw_reset(struct drm_crtc *crtc) > +{ > + struct lsdc_crtc *lcrtc = to_lsdc_crtc(crtc); > + > + /* Reset the CRTC hardware, this is required for S3 support */ > + if (lcrtc->hw_ops->reset) > + lcrtc->hw_ops->reset(lcrtc); > +} > + While reviewing the hardware reset paths in this file, I noticed a pre-existing issue nearby: drivers/gpu/drm/loongson/lsdc_crtc.c: > static void lsdc_crtc1_soft_reset(struct lsdc_crtc *lcrtc) > { [ ... ] > /* Wait about a vblank time */ > msleep(20); > } [Severity: High] This isn't a bug introduced by this patch, but can this msleep() be called during atomic context? The DRM core calls the CRTC .atomic_enable callback, which executes in atomic context: lsdc_crtc_atomic_enable() lcrtc->hw_ops->enable(lcrtc) // resolves to lsdc_crtc1_enable() If a hardware stall is detected, it attempts error recovery: lsdc_crtc1_enable() { ... if (val & CRTC_ANCHORED) { drm_warn(&ldev->base, "%s stall\n", lcrtc->base.name); return lsdc_crtc1_soft_reset(lcrtc); } } This calls lsdc_crtc1_soft_reset(), which erroneously calls msleep(20). Sleeping in atomic context violates core kernel locking rules. This triggers might_sleep() warnings and can lead to deadlocks or panics. (Note that lsdc_crtc0_soft_reset() correctly uses mdelay(20) instead). -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
