Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Asymmetric panel callbacks for DCS commands (display on/off) causes state mismatch and permanent blanking. - [High] The panel initialization sequence sends the `set_display_on` command before the `exit_sleep_mode` command. - [High] Missing device link between the panel and the secondary DSI host leads to Use-After-Free and Double-Free. --
commit bd7c7e227b10119b2ca8cfde561c22808d05f4f2 Author: KancyJoe <[email protected]> drm: panel: add support for the Renesas R63419 based dual-DSI video mode Display Panels This commit implements support for the Renesas 63419 based dual-DSI video mode Display Panels used in the Ayaneo handheld gaming devices. > diff --git a/drivers/gpu/drm/panel/panel-renesas-r63419.c > b/drivers/gpu/drm/panel/panel-renesas-r63419.c > new file mode 100644 > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-renesas-r63419.c [ ... ] > +static int renesas_r63419_on(struct renesas_r63419_panel *ctx) > +{ > + struct mipi_dsi_multi_context dsi_ctx = { 0 }; > + > + /* > + * Panel registers are loaded from DDIC Non Volatile Memory > + * > + * The DDIC expects this sequence to get out of sleep and enable display > + */ > + > + mipi_dsi_dual(mipi_dsi_dcs_set_display_on_multi, > + &dsi_ctx, ctx->dsi[0], ctx->dsi[1]); > + mipi_dsi_msleep(&dsi_ctx, 50); > + > + mipi_dsi_dual(mipi_dsi_dcs_exit_sleep_mode_multi, > + &dsi_ctx, ctx->dsi[0], ctx->dsi[1]); [Severity: High] Are these MIPI DCS commands sent in the correct order? According to the standard, a display must exit sleep mode and wait for internal initialization before processing the set_display_on command. Sending set_display_on first might cause the sleeping DDIC to ignore it. > + mipi_dsi_msleep(&dsi_ctx, 150); > + > + return dsi_ctx.accum_err; > +} [ ... ] > +static int renesas_r63419_disable(struct drm_panel *panel) > +{ > + struct renesas_r63419_panel *ctx = to_renesas_r63419_panel(panel); > + struct mipi_dsi_multi_context dsi_ctx = { 0 }; > + > + mipi_dsi_dual(mipi_dsi_dcs_set_display_off_multi, > + &dsi_ctx, ctx->dsi[0], ctx->dsi[1]); > + mipi_dsi_msleep(&dsi_ctx, 50); > + > + mipi_dsi_dual(mipi_dsi_dcs_enter_sleep_mode_multi, > + &dsi_ctx, ctx->dsi[0], ctx->dsi[1]); > + mipi_dsi_msleep(&dsi_ctx, 120); > + > + return 0; > +} [ ... ] > +static const struct drm_panel_funcs renesas_r63419_panel_funcs = { > + .disable = renesas_r63419_disable, > + .prepare = renesas_r63419_prepare, > + .unprepare = renesas_r63419_unprepare, [Severity: High] Should there be an enable callback to match the disable callback? If the display pipeline is disabled and re-enabled during DPMS transitions, the bridge framework cycles disable and enable. Since the panel receives set_display_off and enter_sleep_mode during disable, but the enable callback is missing, won't the panel remain permanently asleep when waking up? > + .get_modes = renesas_r63419_get_modes, > + .get_orientation = renesas_r63419_get_orientation, > +}; [ ... ] > +static int renesas_r63419_probe(struct mipi_dsi_device *dsi) > +{ [ ... ] > + /* Register the second DSI device */ > + ctx->dsi[1] = devm_mipi_dsi_device_register_full(dev, dsi1_host, &info); > + if (IS_ERR(ctx->dsi[1])) > + return dev_err_probe(dev, PTR_ERR(ctx->dsi[1]), > + "Failed to register second DSI device\n"); [Severity: High] Does the secondary DSI device registration require a device link to protect its lifetime? If the secondary host is unbound via sysfs, the DSI core unregisters all child devices, freeing this pointer. Without a device link to force the panel to unbind first, could devm teardown or subsequent panel accesses lead to a use-after-free or double-free? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260625-topic-sm8650-ayaneo-pocket-s2-r63419-v8-0-8570e6921...@linaro.org?part=2
