Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] Conflicting MIPI DCS brightness functions send payload with mismatched endianness - [High] Incorrect devres registration order causes DSI to detach before panel teardown - [High] Unconditional DSI hardware access in backlight update callback while panel is unpowered - [Medium] Hardcoded brightness in initialization sequence desynchronizes physical and logical state --
commit 225e11adf3557eed6e400ee38411cadcbb543985 Author: Arpit Saini <[email protected]> Subject: drm/panel: add Ilitek ILI7807S panel driver Add a DRM panel driver for the DLC DLC0697 1080x1920@60Hz MIPI DSI panel based on the Ilitek ILI7807S display controller. > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili7807s.c > b/drivers/gpu/drm/panel/panel-ilitek-ili7807s.c > new file mode 100644 > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili7807s.c [ ... ] > +static void dlc0697_init_sequence(struct mipi_dsi_multi_context *dsi_ctx) > +{ [ ... ] > + mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0xff, 0x78, 0x07, 0x00); > + mipi_dsi_dcs_set_tear_on_multi(dsi_ctx, MIPI_DSI_DCS_TEAR_MODE_VBLANK); > + mipi_dsi_dcs_write_seq_multi(dsi_ctx, MIPI_DCS_SET_CABC_MIN_BRIGHTNESS, > 0x09, 0x99); > + mipi_dsi_dcs_write_seq_multi(dsi_ctx, MIPI_DCS_WRITE_CONTROL_DISPLAY, > 0x24); > + mipi_dsi_dcs_write_seq_multi(dsi_ctx, MIPI_DCS_WRITE_POWER_SAVE, 0x01); > + mipi_dsi_dcs_set_display_brightness_multi(dsi_ctx, 0x1fff); [Severity: Medium] Will hardcoding the brightness to 0x1fff during initialization cause the screen to flash at maximum brightness on system resume? The brightness will remain maxed out until the subsequent enable path triggers a backlight update. Does it make sense to restore the user's configured brightness level from the backlight device here instead? [Severity: High] Does the panel hardware expect the 16-bit brightness payload in Little Endian or Big Endian byte order? This initialization sequence uses mipi_dsi_dcs_set_display_brightness_multi(), which sends a Little Endian payload. However, the backlight update callback ili7807s_bl_update_status() uses mipi_dsi_dcs_set_display_brightness_large(), which sends a Big Endian payload: ili7807s_bl_update_status() { ... ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness); } Since these serialize the value with opposite endianness, one of these paths might send a corrupted value to the hardware, potentially causing incorrect backlight levels or initialization failures. [ ... ] > +static int ili7807s_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); [Severity: High] Can a userspace process writing to the backlight sysfs brightness attribute cause a DSI host timeout if the panel is currently asleep? The backlight device is registered in ili7807s_probe() and exposed to userspace asynchronously from the display pipeline state. Because ili7807s_bl_update_status() transmits DCS commands without checking if the panel is actually powered on (for example, by checking a prepared flag), attempting to communicate over the DSI bus while unpowered can cause the host controller to wait indefinitely. [ ... ] > +static int ili7807s_probe(struct mipi_dsi_device *dsi) > +{ [ ... ] > + ret = devm_drm_panel_add(dev, &ctx->panel); > + if (ret < 0) > + return dev_err_probe(dev, ret, "failed to add panel\n"); > + > + return devm_mipi_dsi_attach(dev, dsi); > +} [Severity: High] Could the devres registration order here cause DSI transmission errors during driver unbind? Since devres actions execute in LIFO order, devm_mipi_dsi_attach() will be undone (detaching the DSI device) before devm_drm_panel_add() is undone. When the DRM subsystem subsequently tears down the display pipeline and invokes the panel's unprepare callback, the DCS commands sent to power down the panel will fail because the DSI device is no longer attached to the host. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
