Re: [PATCH v8 2/2] drm: panel: add support for the Renesas R63419 based dual-DSI video mode Display Panels

[sashiko-bot@kernel.org]

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 <kancy2333@outlook.com>
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-8570e692143e@linaro.org?part=2