Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The atomic_enable and atomic_disable bridge callbacks call sleeping functions but execute in an atomic context. -- --- Patch [21]: [PATCH v2 21/37] drm/bridge: tda998x: Switch to atomic bridge callbacks --- commit 3c3197b45d3a1fb57e72b15de2a23cbccfa9ab6d Author: Maxime Ripard <[email protected]> drm/bridge: tda998x: Switch to atomic bridge callbacks This patch switches the tda998x bridge driver from legacy non-atomic enable and disable callbacks to their atomic counterparts. It updates the callback signatures and the drm_bridge_funcs structure accordingly. > diff --git a/drivers/gpu/drm/bridge/tda998x_drv.c > b/drivers/gpu/drm/bridge/tda998x_drv.c > index 6c427bc75896b..16c7455911758 100644 > --- a/drivers/gpu/drm/bridge/tda998x_drv.c > +++ b/drivers/gpu/drm/bridge/tda998x_drv.c [ ... ] > @@ -1410,7 +1410,8 @@ static enum drm_mode_status > tda998x_bridge_mode_valid(struct drm_bridge *bridge, > return MODE_OK; > } > > -static void tda998x_bridge_enable(struct drm_bridge *bridge) > +static void tda998x_bridge_enable(struct drm_bridge *bridge, > + struct drm_atomic_commit *commit) > { [Severity: High] This isn't a bug introduced by this patch, but will this cause sleeping in atomic context warnings or deadlocks? The tda998x_bridge_enable() callback executes in an atomic context under drm_atomic_helper_commit_tail(), where sleeping is forbidden. However, it calls reg_write(): static void tda998x_bridge_enable(...) { ... if (!priv->is_on) { reg_write(priv, REG_ENA_VP_0, 0xff); which takes a mutex (mutex_lock(&priv->mutex)) and initiates an I2C transfer, both of which can sleep. Migrating to the atomic_enable hook retains this behavior in the atomic commit path. > struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge); > [ ... ] > @@ -1428,7 +1429,8 @@ static void tda998x_bridge_enable(struct drm_bridge > *bridge) > } > } > > -static void tda998x_bridge_disable(struct drm_bridge *bridge) > +static void tda998x_bridge_disable(struct drm_bridge *bridge, > + struct drm_atomic_commit *commit) > { [Severity: High] This is a pre-existing issue, but does this face the same atomic context problem as tda998x_bridge_enable()? Similar to the enable path, tda998x_bridge_disable() runs in an atomic context but invokes reg_write() which can sleep: static void tda998x_bridge_disable(...) { ... if (priv->is_on) { reg_write(priv, REG_ENA_VP_0, 0x00); > struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge); > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=21
