Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Bridge atomic callbacks use sleeping functions (`msleep`, `usleep_range`) while running in an atomic context path. -- --- Patch [16]: [PATCH v2 16/37] drm/bridge: parade-ps8622: Switch to atomic bridge callbacks --- commit 6103d7b41fd50ff49b7002eef1519ecf77486219 Author: Maxime Ripard <[email protected]> drm/bridge: parade-ps8622: Switch to atomic bridge callbacks This commit updates the parade-ps8622 bridge to use atomic bridge callbacks instead of the legacy ones. The functions have been updated to accept the drm_atomic_commit state handlers. > diff --git a/drivers/gpu/drm/bridge/parade-ps8622.c > b/drivers/gpu/drm/bridge/parade-ps8622.c > index f879a1df077d2..33844db753aa5 100644 > --- a/drivers/gpu/drm/bridge/parade-ps8622.c > +++ b/drivers/gpu/drm/bridge/parade-ps8622.c > @@ -336,7 +336,8 @@ static const struct backlight_ops ps8622_backlight_ops = { > .update_status = ps8622_backlight_update, > }; > > -static void ps8622_pre_enable(struct drm_bridge *bridge) > +static void ps8622_pre_enable(struct drm_bridge *bridge, > + struct drm_atomic_commit *commit) > { > struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge); > int ret; [Severity: High] This is a pre-existing issue, but this function calls a sleeping function further down in the body: usleep_range(PS8622_RST_HIGH_T2_MIN_US + PS8622_POWER_RISE_T1_MAX_US, PS8622_RST_HIGH_T2_MAX_US + PS8622_POWER_RISE_T1_MIN_US); Could this cause kernel warnings or deadlocks? The drm_atomic_helper_commit_tail() path directly invokes these bridge atomic callbacks, and sleeping is forbidden in this atomic context. [ ... ] > @@ -381,13 +382,15 @@ static void ps8622_pre_enable(struct drm_bridge *bridge) > ps8622->enabled = true; > } > > -static void ps8622_disable(struct drm_bridge *bridge) > +static void ps8622_disable(struct drm_bridge *bridge, > + struct drm_atomic_commit *commit) > { > /* Delay after panel is disabled */ > msleep(PS8622_PWMO_END_T12_MS); [Severity: High] This isn't a bug introduced by this patch, but does calling msleep() here violate the atomic context requirements? This callback can be executed from drm_atomic_helper_commit_tail() where sleeping is forbidden. > } > > -static void ps8622_post_disable(struct drm_bridge *bridge) > +static void ps8622_post_disable(struct drm_bridge *bridge, > + struct drm_atomic_commit *commit) > { > struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge); [Severity: High] This is a pre-existing issue, but similar to the other callbacks, this path also contains calls to sleeping functions later in the body: usleep_range(PS8622_POWER_FALL_T16_MAX_US, 2 * PS8622_POWER_FALL_T16_MAX_US); ... msleep(PS8622_POWER_OFF_T17_MS); Is it safe to use sleeping functions here since this bridge atomic callback also runs in an atomic context path? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=16
