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 <[email protected]> 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/[email protected]?part=1
