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 <[email protected]> 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/[email protected]?part=8
