Re: [PATCH v2 21/37] drm/bridge: tda998x: Switch to atomic bridge callbacks

[sashiko-bot@kernel.org]

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 <mripard@kernel.org>

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/20260707-drm-all-atomic-bridges-v2-0-21d03cbca446@kernel.org?part=21