Re: [PATCH v2 16/37] drm/bridge: parade-ps8622: 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] 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 <mripard@kernel.org>

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