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