Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm: backlight_disable error prevents panel->enabled = false update --
commit 00631cfc33b867ee09e15a877b7d601e80307028 Author: Dmitry Baryshkov <[email protected]> drm/panel: add atomic variants of the enable and disable callbacks This commit introduces atomic variants of panel prepare, enable, disable, and unprepare callbacks. These new callbacks receive the atomic commit state, allowing panels tracking configuration via atomic state to program themselves appropriately. > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c > index ed0d16e04eaec..a2cd043fe1467 100644 > --- a/drivers/gpu/drm/drm_panel.c > +++ b/drivers/gpu/drm/drm_panel.c [ ... ] > +static void drm_panel_atomic_disable(struct drm_panel *panel, > + struct drm_atomic_commit *commit) > +{ > + const struct drm_panel_funcs *funcs = panel->funcs; > + int ret = 0; > + > + if (!panel->enabled) { > + dev_warn(panel->dev, "Skipping disable of already disabled > panel\n"); > + return; > + } > + > + guard(mutex)(&panel->follower_lock); > + > + drm_panel_followers_disabling(panel); > + > + ret = backlight_disable(panel->backlight); > + if (ret < 0) > + DRM_DEV_INFO(panel->dev, "failed to disable backlight: %d\n", > + ret); > + > + if (funcs->atomic_disable) > + ret = funcs->atomic_disable(panel, commit); > + else if (funcs->disable) > + ret = funcs->disable(panel); > + if (ret < 0) > + return; [Severity: High] Does an error in backlight_disable() prevent the panel from being marked as disabled when the disable callbacks are missing? Since both funcs->atomic_disable and funcs->disable are optional callbacks, many drivers omit them. In that case, the if/else if block is skipped, and ret retains the negative value returned by backlight_disable(). The subsequent if (ret < 0) condition then evaluates to true, triggering an early return that skips setting panel->enabled = false. Because the state remains panel->enabled = true, future calls to drm_panel_atomic_enable() will immediately return without turning the screen back on, leaving the display permanently off. > + panel->enabled = false; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
