Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Early returns on DSI command failures in teardown functions (`s6d16d0_unprepare` and `s6d16d0_disable`) bypass hardware power-off and cause DRM state machine desynchronization. -- commit 5d36b39e4bbd28714a2875efbe7aa78f7026a69d Author: Laxman Acharya Padhya <[email protected]> drm/panel: samsung-s6d16d0: Power off on prepare failure If enabling tearing mode or exiting sleep mode fails after the regulator is enabled, s6d16d0_prepare() returns without asserting reset or disabling the supply. Since the DRM panel core leaves the panel unprepared, a later unprepare call skips the driver callback and the supply remains enabled. Assert reset and disable the supply before returning the DSI command error. > diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > index ba1a02000bb9d..04e19b5f22f8b 100644 > --- a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > +++ b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c > @@ -89,16 +89,22 @@ static int s6d16d0_prepare(struct drm_panel *panel) > MIPI_DSI_DCS_TEAR_MODE_VBLANK); > if (ret) { > dev_err(s6->dev, "failed to enable vblank TE (%d)\n", ret); > - return ret; > + goto err_power_off; > } > /* Exit sleep mode and power on */ > ret = mipi_dsi_dcs_exit_sleep_mode(dsi); > if (ret) { > dev_err(s6->dev, "failed to exit sleep mode (%d)\n", ret); > - return ret; > + goto err_power_off; > } > > return 0; > + > +err_power_off: > + gpiod_set_value_cansleep(s6->reset_gpio, 1); > + regulator_disable(s6->supply); > + > + return ret; > } [Severity: High] This is a pre-existing issue, but since this patch fixes the error path in s6d16d0_prepare(), does the same early return problem exist in s6d16d0_unprepare() and s6d16d0_disable()? Looking at s6d16d0_unprepare(), if mipi_dsi_dcs_enter_sleep_mode() fails, it returns early: drivers/gpu/drm/panel/panel-samsung-s6d16d0.c:s6d16d0_unprepare() { ... ret = mipi_dsi_dcs_enter_sleep_mode(dsi); if (ret) { dev_err(s6->dev, "failed to enter sleep mode (%d)\n", ret); return ret; } /* Assert RESET */ gpiod_set_value_cansleep(s6->reset_gpio, 1); regulator_disable(s6->supply); return 0; } Can this early return bypass regulator_disable(), leaving the regulator permanently enabled? Furthermore, because these functions return an error, the DRM core skips updating the panel state. Similarly, if mipi_dsi_dcs_set_display_off() fails in s6d16d0_disable(): drivers/gpu/drm/panel/panel-samsung-s6d16d0.c:s6d16d0_disable() { ... ret = mipi_dsi_dcs_set_display_off(dsi); if (ret) { dev_err(s6->dev, "failed to turn display off (%d)\n", ret); return ret; } ... } Will the early error return in both of these teardown functions cause a permanent desynchronization between the panel and the DRM state machine, where subsequent attempts to enable the panel skip the prepare and enable callbacks? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
