Re: [PATCH] drm/panel: Replace deprecated DCS display-on calls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/panel: uninitialized return value in k101_im2ba02_enable() on DSI error
- [Low] drm/panel: single-call mipi_dsi_multi_context misuse and duplicate dev_err()
--

commit 98ea9fe2e57e340101561581a07219ed561f36d0
Author: amit <amittibi123@gmail.com>

drm/panel: Replace deprecated DCS display-on calls

This commit replaces calls to mipi_dsi_dcs_set_display_on() across 14 DRM
panel drivers with mipi_dsi_dcs_set_display_on_multi() using a local
mipi_dsi_multi_context structure.

> diff --git a/drivers/gpu/drm/panel/panel-boe-himax8279d.c b/drivers/gpu/drm/panel/panel-boe-himax8279d.c
> index 4a8560b4b899a..a4b35e042f315 100644
> --- a/drivers/gpu/drm/panel/panel-boe-himax8279d.c
> +++ b/drivers/gpu/drm/panel/panel-boe-himax8279d.c
> @@ -155,7 +155,12 @@ static int boe_panel_prepare(struct drm_panel *panel)
>  	/* T6: 120ms - 121ms */
>  	usleep_range(120000, 121000);
>  
> -	err = mipi_dsi_dcs_set_display_on(pinfo->link);
> +	struct mipi_dsi_multi_context dsi_ctx = {
> +		.dsi = pinfo->link,
> +	};
> +
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +	err = dsi_ctx.accum_err;
>  	if (err < 0) {
>  		dev_err(panel->dev, "failed to set display on: %d\n", err);
>  		goto poweroff;

[Severity: Low]
This isn't a bug, but is it intended to instantiate struct
mipi_dsi_multi_context mid-function for just a single
mipi_dsi_dcs_set_display_on_multi() call while leaving the surrounding DSI
calls (such as mipi_dsi_dcs_exit_sleep_mode(), mipi_dsi_dcs_set_tear_on(),
mipi_dsi_dcs_set_pixel_format(), and mipi_dsi_dcs_write_buffer()) unconverted?

In addition, mipi_dsi_dcs_set_display_on_multi() in
drivers/gpu/drm/drm_mipi_dsi.c already logs an error on failure:

	ret = mipi_dsi_dcs_set_display_on(dsi);
	if (ret < 0) {
		ctx->accum_err = ret;
		dev_err(dev, "sending DCS SET_DISPLAY_ON failed: %d\n",
			ctx->accum_err);
	}

Keeping the caller's dev_err() here in boe_panel_prepare() (as well as in
boe_panel_enable(), innolux_panel_prepare(), khadas_ts050_panel_prepare(),
kingdisplay_panel_prepare(), ltk500hd1829_prepare(), panel_nv3051d_prepare(),
nt35510_prepare(), rad_panel_enable(), and sharp_panel_prepare()) will print
duplicate error messages when setting the display on fails. Declaring
dsi_ctx in the middle of the function after executable statements also
introduces mixed declarations across 12 of the modified drivers (including
dsicm_power_on(), rm68200_prepare(), and s6e63j0x03_enable()).

Should the full DSI command sequences in these functions be converted to use
a single mipi_dsi_multi_context declared at the top of each function with the
redundant dev_err() calls removed?

[ ... ]

> diff --git a/drivers/gpu/drm/panel/panel-feixin-k101-im2ba02.c b/drivers/gpu/drm/panel/panel-feixin-k101-im2ba02.c
> index 8c3a231c147d5..baa64b7aeb0bd 100644
> --- a/drivers/gpu/drm/panel/panel-feixin-k101-im2ba02.c
> +++ b/drivers/gpu/drm/panel/panel-feixin-k101-im2ba02.c
> @@ -344,12 +344,15 @@ static int k101_im2ba02_enable(struct drm_panel *panel)
>  {
>  	struct k101_im2ba02 *ctx = panel_to_k101_im2ba02(panel);
>  	const struct k101_im2ba02_init_cmd *cmd = &timed_cmds[1];
> +	struct mipi_dsi_multi_context dsi_ctx = {
> +		.dsi = ctx->dsi,
> +	};
>  	int ret;
>  
>  	msleep(150);
>  
> -	ret = mipi_dsi_dcs_set_display_on(ctx->dsi);
> -	if (ret < 0)
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +	if (dsi_ctx.accum_err)
>  		return ret;

[Severity: High]
In k101_im2ba02_enable(), doesn't this return the uninitialized local
variable ret when mipi_dsi_dcs_set_display_on_multi() fails and sets
dsi_ctx.accum_err?

Since ret is never initialized or assigned, returning ret on error triggers
an uninitialized variable warning and can return a non-negative garbage value
to drm_panel_enable(), causing it to treat a failed panel enable as
successful while skipping the final mipi_dsi_dcs_write_buffer() command.

Should ret be removed and dsi_ctx.accum_err returned directly instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104073813.42429-1-amittibi123@gmail.com?part=1