Re: [PATCH v2] drm/panel: samsung-s6d16d0: Power off on prepare failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Early returns on DSI command failures in teardown functions (`s6d16d0_unprepare` and `s6d16d0_disable`) bypass hardware power-off and cause DRM state machine desynchronization.
--

commit 5d36b39e4bbd28714a2875efbe7aa78f7026a69d
Author: Laxman Acharya Padhya <acharyalaxman8848@gmail.com>

drm/panel: samsung-s6d16d0: Power off on prepare failure

If enabling tearing mode or exiting sleep mode fails after the
regulator is enabled, s6d16d0_prepare() returns without asserting
reset or disabling the supply. Since the DRM panel core leaves the
panel unprepared, a later unprepare call skips the driver callback
and the supply remains enabled.

Assert reset and disable the supply before returning the DSI command error.

> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
> index ba1a02000bb9d..04e19b5f22f8b 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
> @@ -89,16 +89,22 @@ static int s6d16d0_prepare(struct drm_panel *panel)
>  				       MIPI_DSI_DCS_TEAR_MODE_VBLANK);
>  	if (ret) {
>  		dev_err(s6->dev, "failed to enable vblank TE (%d)\n", ret);
> -		return ret;
> +		goto err_power_off;
>  	}
>  	/* Exit sleep mode and power on */
>  	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
>  	if (ret) {
>  		dev_err(s6->dev, "failed to exit sleep mode (%d)\n", ret);
> -		return ret;
> +		goto err_power_off;
>  	}
>  
>  	return 0;
> +
> +err_power_off:
> +	gpiod_set_value_cansleep(s6->reset_gpio, 1);
> +	regulator_disable(s6->supply);
> +
> +	return ret;
>  }

[Severity: High]
This is a pre-existing issue, but since this patch fixes the error path in
s6d16d0_prepare(), does the same early return problem exist in
s6d16d0_unprepare() and s6d16d0_disable()?

Looking at s6d16d0_unprepare(), if mipi_dsi_dcs_enter_sleep_mode() fails,
it returns early:

drivers/gpu/drm/panel/panel-samsung-s6d16d0.c:s6d16d0_unprepare() {
    ...
	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
	if (ret) {
		dev_err(s6->dev, "failed to enter sleep mode (%d)\n", ret);
		return ret;
	}

	/* Assert RESET */
	gpiod_set_value_cansleep(s6->reset_gpio, 1);
	regulator_disable(s6->supply);

	return 0;
}

Can this early return bypass regulator_disable(), leaving the regulator
permanently enabled?

Furthermore, because these functions return an error, the DRM core skips
updating the panel state. 

Similarly, if mipi_dsi_dcs_set_display_off() fails in s6d16d0_disable():

drivers/gpu/drm/panel/panel-samsung-s6d16d0.c:s6d16d0_disable() {
    ...
	ret = mipi_dsi_dcs_set_display_off(dsi);
	if (ret) {
		dev_err(s6->dev, "failed to turn display off (%d)\n", ret);
		return ret;
	}
    ...
}

Will the early error return in both of these teardown functions cause a
permanent desynchronization between the panel and the DRM state machine,
where subsequent attempts to enable the panel skip the prepare and enable
callbacks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703165632.27027-1-acharyalaxman8848@gmail.com?part=1