Re: [PATCH RFC 7/9] drm/panel: add atomic variants of the enable and disable callbacks

[sashiko-bot@kernel.org]

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 <dmitry.baryshkov@oss.qualcomm.com>
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/20261008-panel-atomic-state-v1-0-b157fddb8de1@oss.qualcomm.com?part=7