Re: [PATCH v2 08/11] drm/panel: s6e3ha8: add Samsung AMB630QY01 (Google Pixel 3 XL) panel

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/panel: s6e3ha8: unconditional DCS command in backlight update causes DSI host wedge
--

commit cc2d04d32c8c8a9c163b61b933026b6a253b1aca
Author: David Heidelberg <david@ixit.cz>

drm/panel: s6e3ha8: add Samsung AMB630QY01 (Google Pixel 3 XL) panel

The Google Pixel 3 XL (crosshatch) uses a Samsung AMB630QY01 6.3" 1440x2960
flexible AMOLED panel, driven by the same S6E3HA8 DDIC as the Galaxy S9 panel
this driver already supports. This commit adds the initialization sequences and
timings, along with a backlight device since brightness is controlled via
standard DCS commands.

> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c b/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
> index 2591b8c1a7753..e02f1d9f0dcd0 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c

[ ... ]

> @@ -271,6 +294,136 @@ static const struct s6e3ha8_desc s6e3ha8_amb577px01_wqhd_desc = {

[ ... ]

> +static int s6e3ha8_bl_update_status(struct backlight_device *bl)
> +{
> +	struct mipi_dsi_device *dsi = bl_get_data(bl);
> +	u16 brightness = backlight_get_brightness(bl);
> +
> +	return mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);

[Severity: High]
Could this transmission wedge the display pipeline? In
drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c within
s6e3ha8_bl_update_status(), this DSI transmission is executed unconditionally
without verifying if the panel's regulators and DSI link are active (e.g.
lacking a prepared state check).

Writing to the sysfs brightness attribute while the display is suspended or
disabled causes the driver to send DCS commands over the DSI bus when the DSI
host clocks and PHY may be gated or panel regulators disabled.

On many platforms (including the target Qualcomm sdm845), unclocked accesses
or DSI timeouts can result in a system crash (NOC error) or permanently wedge
the display pipeline.

> +}

[ ... ]

> @@ -313,6 +466,13 @@ static int s6e3ha8_probe(struct mipi_dsi_device *dsi)
>  
>  	priv->panel.prepare_prev_first = true;
>  
> +	if (priv->desc->has_backlight) {
> +		priv->panel.backlight = s6e3ha8_create_backlight(dsi);

[Severity: High]
Does this code create a race condition for early or late backlight updates?
In drivers/gpu/drm/panel/panel-samsung-s6e3ha8.c, this exposes the backlight
interface to userspace in s6e3ha8_probe(), creating the window for the race
condition where sysfs writes can trigger before the panel is prepared or after
it is unprepared.

A user-space process or udev rule could write to the sysfs brightness
attribute after the panel has been disabled via drm_panel_unprepare/disable.
The sysfs interface is typically accessible to unprivileged users, making this
easily reachable from user-space.

> +		if (IS_ERR(priv->panel.backlight))
> +			return dev_err_probe(dev, PTR_ERR(priv->panel.backlight),
> +					     "Failed to create backlight\n");
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-crosshatch-panel-v2-0-08d5c7bf72fc@ixit.cz?part=8