Re: [PATCH] drm/panel: chipone-icna35xx: Fix picture parameter set

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Direct calls to `mipi_dsi_dcs_write()` bypass the `mipi_dsi_multi_context` error accumulation, ignoring both previous errors and their own failures.
--

--- Patch [1]: [PATCH] drm/panel: chipone-icna35xx: Fix picture parameter set ---
commit 5c76e85884612ecbbcfd07cbe14b3a2d283f6c53
Author: Aaron Kling <webgeek1234@gmail.com>

drm/panel: chipone-icna35xx: Fix picture parameter set
    
This commit updates the chipone-icna35xx panel driver to send picture
parameter set (PPS) data using vendor-specific DCS writes rather than the
standard MIPI DSI command. This resolves an issue with configuring 10-bit
modes on these panels.

> diff --git a/drivers/gpu/drm/panel/panel-chipone-icna35xx.c b/drivers/gpu/drm/panel/panel-chipone-icna35xx.c
> index 2cc16494a4cef..b2422ffc1d1c5 100644
> --- a/drivers/gpu/drm/panel/panel-chipone-icna35xx.c
> +++ b/drivers/gpu/drm/panel/panel-chipone-icna35xx.c

[ ... ]

> @@ -91,7 +92,14 @@ static int icna3512_init_sequence(struct panel_info *pinfo)
>  	mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0x48, 0x20);
>  
>  	drm_dsc_pps_payload_pack(&pps, &pinfo->desc->dsc);
> -	mipi_dsi_picture_parameter_set_multi(&dsi_ctx, &pps);
> +	mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0x9F, 0x01);
> +	mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xC6, 0x11, 0x88);
> +
> +	mipi_dsi_dcs_write(dsi_ctx.dsi, 0xC7, pps_ptr, 32);
> +	mipi_dsi_dcs_write(dsi_ctx.dsi, 0xC8, pps_ptr + 32, 32);
> +	mipi_dsi_dcs_write(dsi_ctx.dsi, 0xC9, pps_ptr + 64, 24);

[Severity: Medium]
Do these direct mipi_dsi_dcs_write() calls bypass the multi-context error
handling?

Since icna3512_init_sequence() is using mipi_dsi_multi_context, the
_multi() variants will check and update dsi_ctx.accum_err. Calling
mipi_dsi_dcs_write() directly ignores any previous errors and discards
its own return values, which might lead to silent failures if the PPS data
transmission fails.

> +
> +	mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xFD, 0xA5, 0xA5);
>  
>  	mipi_dsi_msleep(&dsi_ctx, 20);

[ ... ]

> @@ -133,7 +142,14 @@ static int icna3520_init_sequence(struct panel_info *pinfo)
>  	mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xB2, 0x24);
>  
>  	drm_dsc_pps_payload_pack(&pps, &pinfo->desc->dsc);
> -	mipi_dsi_picture_parameter_set_multi(&dsi_ctx, &pps);
> +	mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0x9F, 0x01);
> +	mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xC0, 0x1A, 0x71);
> +
> +	mipi_dsi_dcs_write(dsi_ctx.dsi, 0xC1, pps_ptr, 32);
> +	mipi_dsi_dcs_write(dsi_ctx.dsi, 0xC2, pps_ptr + 32, 32);
> +	mipi_dsi_dcs_write(dsi_ctx.dsi, 0xC3, pps_ptr + 64, 24);

[Severity: Medium]
Does this sequence in icna3520_init_sequence() also bypass the error
accumulation?

Similar to the above, we might need a multi-aware wrapper or macro here
to ensure errors are caught and execution stops appropriately if a previous
command failed.

> +
> +	mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xFD, 0xA5, 0xA5);
>  
>  	mipi_dsi_msleep(&dsi_ctx, 20);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-icna35xx-pps-v1-1-691d79d13ee4@gmail.com?part=1