Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Calling the sleeping function `drm_edp_backlight_disable()` from within the `nv50_sor_atomic_disable` encoder atomic disable callback violates DRM atomic context rules. This problem wasn't introduced by this patch, but the logic was moved into the new `nv50_sor_atomic_disable_backlight()` inline function. -- --- Patch [1]: [PATCH 1/6] drm/nouveau/kms/nv50-: Move DPCD backlight disable into its own function --- commit a8a2b3da8fed6e6d5dafbcd1c4a9bb461b74def0 Author: Lyude Paul <[email protected]> drm/nouveau/kms/nv50-: Move DPCD backlight disable into its own function This commit moves the DPCD backlight disable logic into its own separate inline helper function. Aside from using the state device to access the drm device, no functional changes are introduced. Fixes: f575f2bdb6c3 ("drm/nouveau/kms/nv50-: Remove (nv_encoder->crtc) checks in ->disable callbacks") Cc: <[email protected]> # v5.12+ > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c > b/drivers/gpu/drm/nouveau/dispnv50/disp.c > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c [ ... ] > +#ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT > +static inline void > +nv50_sor_atomic_disable_backlight(struct nouveau_drm *drm, > + struct nouveau_encoder *nv_encoder, > + struct drm_atomic_commit *state) > +{ [ ... ] > + if (!backlight || !backlight->uses_dpcd) > + return; > + > + ret = drm_edp_backlight_disable(&nv_connector->aux, > &backlight->edp_info); [Severity: High] This is a pre-existing issue, but moving the logic into nv50_sor_atomic_disable_backlight() preserves a potentially problematic sleeping call in an atomic context. Is it safe to call drm_edp_backlight_disable() from this path? Because nv50_sor_atomic_disable_backlight() is invoked by the encoder atomic disable callback nv50_sor_atomic_disable(), it executes in an atomic context where sleeping is forbidden. The drm_edp_backlight_disable() helper performs DPCD transactions over the AUX channel. These transactions require taking a hardware mutex and can sleep or block while waiting for hardware responses. Could this lead to scheduling while atomic kernel warnings or potential deadlocks during display disables or modesets? > + if (ret < 0) > + NV_ERROR(drm, "Failed to disable backlight on > [CONNECTOR:%d:%s]: %d\n", > + nv_connector->base.base.id, nv_connector->base.name, > ret); > +} > +#endif -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
