Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] The error path in `sw43408_prepare()` still asserts the reset GPIO, reintroducing the unrecoverable DDIC state that this patch intends to fix. - [High] Missing `drm_panel_remove()` on the `mipi_dsi_attach()` error path leaves a dangling panel in the global registry. -- --- Patch [1]: [PATCH 1/2] drm/panel: lg-sw43408: do not assert reset across unprepare --- commit 943efe5fda9e672780d0b298511b40b26e4fb79c Author: David Heidelberg <[email protected]> drm/panel: lg-sw43408: do not assert reset across unprepare This commit stops asserting the reset GPIO in sw43408_unprepare(). Leaving it asserted while the shared logic rail remains powered puts the DDIC into an unrecoverable state where it fails to light up again. > diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c > b/drivers/gpu/drm/panel/panel-lg-sw43408.c > index 293826c5006b9..0bf8e08f10b9c 100644 > --- a/drivers/gpu/drm/panel/panel-lg-sw43408.c > +++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c > @@ -55,8 +55,14 @@ static int sw43408_unprepare(struct drm_panel *panel) > > mipi_dsi_msleep(&ctx, 100); > > - gpiod_set_value(sw43408->reset_gpio, 1); > - > + /* > + * Keep reset deasserted: vddi may stay powered (it is shared with > + * the touchscreen on Pixel 3) and holding the DDIC in reset while > + * its logic rail is up puts it into a state that no init sequence > + * recovers from - the panel keeps answering DCS commands and > + * reports display-on, but never lights up again. The vendor stack > + * only ever toggles reset as part of a powered-up init sequence. > + */ > ret = regulator_bulk_disable(ARRAY_SIZE(sw43408_supplies), > sw43408->supplies); > > return ret ? : ctx.accum_err; [Severity: High] This isn't a bug introduced by this patch, but does the error path in sw43408_prepare() reintroduce the exact problem this patch fixes? If sw43408_program() fails, the code jumps to the poweroff label: drivers/gpu/drm/panel/panel-lg-sw43408.c:sw43408_prepare() { ... ret = sw43408_program(panel); if (ret) goto poweroff; return 0; poweroff: gpiod_set_value(ctx->reset_gpio, 1); regulator_bulk_disable(ARRAY_SIZE(sw43408_supplies), ctx->supplies); return ret; } This explicitly asserts the reset GPIO while regulators are disabled. Since vddi is shared and remains powered, doesn't this strand the DDIC in the unrecoverable state upon initialization failure? [Severity: High] This is another pre-existing issue, but does sw43408_probe() leak the panel and leave it dangling in the global DRM panel list on attachment failure? drivers/gpu/drm/panel/panel-lg-sw43408.c:sw43408_probe() { ... ret = sw43408_add(ctx); if (ret < 0) return ret; ... return mipi_dsi_attach(dsi); } The sw43408_add() function registers the panel with drm_panel_add(). If mipi_dsi_attach() subsequently fails, it returns the error without calling drm_panel_remove(). When a DRM display driver later finds this panel and attempts to use it via the unattached DSI device, could this cause a kernel oops? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
