Re: [PATCH 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: Unsynchronized sysfs backlight triggers DSI DCS writes
--

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

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

This commit adds support for the Samsung AMB630QY01 panel used in the
Google Pixel 3 XL, driven by the S6E3HA8 DDIC. It introduces the panel
initialization sequence, standard DCS brightness control via a backlight
device, and corresponding DSC settings.

> 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 = {
>  	.num_supplies = ARRAY_SIZE(s6e3ha8_vddr_supplies),
>  };
>  
> +static int s6e3ha8_amb630qy01_prepare(struct drm_panel *panel)
> +{
[ ... ]
> +	drm_dsc_pps_payload_pack(&pps, &priv->dsc);
> +	mipi_dsi_picture_parameter_set_multi(&ctx, &pps);

[Severity: High]
This is a pre-existing issue, but does this result in sending an uninitialized
DSC PPS payload to the panel?

In s6e3ha8_amb630qy01_prepare(), the PPS payload is created using priv->dsc
and transmitted to the hardware. However, the DRM DSI host typically
populates the display timings and rate control parameters into priv->dsc
later in the encoder enable sequence.

If the panel relies on this PPS for decompression, could DSC decoding fail
and result in a corrupted display since pic_width, pic_height, and rate
control parameters are initialized to 0 at this point?

[ ... ]
> +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);
> +}
[ ... ]
> @@ -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);
> +		if (IS_ERR(priv->panel.backlight))

[Severity: High]
Does registering the backlight device here create a race condition where
sysfs writes can trigger DSI DCS writes while the panel is unpowered?

The backlight device is exposed to userspace before the DSI device is
attached or the panel is powered. If a local user or process writes to the
sysfs backlight brightness file, s6e3ha8_bl_update_status() unconditionally
invokes mipi_dsi_dcs_set_display_brightness_large() without verifying the
panel's power state (for example, by checking priv->prepared).

Can this attempt to drive unpowered DSI lanes, wedging the command engine or
causing DSI host timeouts?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920-crosshatch-panel-v1-0-de6e9512da96@ixit.cz?part=8