Re: [PATCH 2/2] drm/panel: Add Samsung AMS662ZS01 panel driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Panel reset GPIO is initialized to physical HIGH while regulators are off, causing hardware back-powering.
- [Medium] Data race on shared `dsi->mode_flags` due to non-atomic modification in sysfs backlight update callback.
- [Medium] The backlight sysfs interface issues DSI transfers to an unpowered panel.
--

commit df8bf199977cc597d9cc38caadd2457c9f44d341
Author: Vsevolod Nevorotov <sevanevorotov29@gmail.com>

    drm/panel: Add Samsung AMS662ZS01 panel driver
    
    Add support for the Samsung AMS662ZS01 AMOLED display panel used in the
    OnePlus 9RT (martini) smartphone.
    
    The panel operates in Command Mode with DSC (Display Stream Compression)
    enabled at 1080x2400 resolution at 60Hz.

> diff --git a/drivers/gpu/drm/panel/panel-samsung-ams662zs01.c b/drivers/gpu/drm/panel/panel-samsung-ams662zs01.c
> new file mode 100644
> index 0000000000000..52e7e75c08b13
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-samsung-ams662zs01.c
> [ ... ]
> @@ -267,11 +267,10 @@ static int ams662zs01_bl_update_status(struct backlight_device *bl)
>  {
>  	struct mipi_dsi_device *dsi = bl_get_data(bl);
>  	u16 brightness = backlight_get_brightness(bl);
>  	int ret;
>  
>  	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
>  	ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness);
>  	dsi->mode_flags |= MIPI_DSI_MODE_LPM;

[Severity: Medium]
Could this create a data race on dsi->mode_flags?

The backlight subsystem uses bl->update_lock while DRM uses
drm_modeset_lock, providing no mutual exclusion between these paths. A
concurrent sysfs write to the backlight brightness node during
ams662zs01_prepare() could race on dsi->mode_flags. This might cause
display initialization commands to be sent in the wrong power mode.

[Severity: Medium]
Does this function lack a check to ensure the panel is prepared and powered
on before attempting DSI communication?

If userspace writes to the backlight sysfs node while the panel is
unprepared (screen off or system suspended), this unconditionally sends DCS
commands. Attempting a DSI transfer while the panel's regulators are
disabled could cause hardware timeouts, bus errors, or host driver failures.

>  
>  	return ret;
>  }
> [ ... ]
> @@ -311,9 +310,10 @@ static int ams662zs01_probe(struct mipi_dsi_device *dsi)
>  	if (IS_ERR(ctx))
>  		return PTR_ERR(ctx);
>  
>  	ret = devm_regulator_bulk_get_const(dev, ARRAY_SIZE(ams662zs01_supplies),
>  					    ams662zs01_supplies, &ctx->supplies);
>  	if (ret < 0)
>  		return ret;
>  
>  	ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);

[Severity: High]
Does this initialize the active-low reset GPIO to physical HIGH before the
panel's regulators are turned on?

Calling devm_gpiod_get() with GPIOD_OUT_LOW on an active-low GPIO
immediately drives the pin physically HIGH. Driving the reset GPIO high
while the panel's main power rails (VDDIO/VDD) are off could forward-bias
the ESD protection diodes inside the panel driver IC. This back-powers the
IC logic domain, which might cause permanent hardware damage or latch-up
preventing panel initialization.

>  	if (IS_ERR(ctx->reset_gpio))
>  		return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio),
>  				     "Failed to get reset-gpios\n");
> [ ... ]
> @@ -330,9 +330,10 @@ static int ams662zs01_probe(struct mipi_dsi_device *dsi)
>  	dsi->format = MIPI_DSI_FMT_RGB888;
>  	dsi->mode_flags = MIPI_DSI_MODE_NO_EOT_PACKET |
>  			  MIPI_DSI_CLOCK_NON_CONTINUOUS |
>  			  MIPI_DSI_MODE_LPM;
>  
>  	ctx->panel.prepare_prev_first = true;
>  
>  	ctx->panel.backlight = ams662zs01_create_backlight(dsi);
>  	if (IS_ERR(ctx->panel.backlight))
>  		return dev_err_probe(dev, PTR_ERR(ctx->panel.backlight),
>  				     "Failed to create backlight\n");

[Severity: Medium]
Is it safe to expose the backlight sysfs interface here before the DSI
device is attached to the host?

Because the backlight device is registered here, it is exposed to userspace
while the panel is unprepared and before mipi_dsi_attach() is called. This
makes the unprotected DSI transfers in ams662zs01_bl_update_status()
reachable immediately, even while the panel is suspended.

>  
>  	drm_panel_add(&ctx->panel);

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