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

[Aaron Kling]

On Tue, Jul 28, 2026 at 10:33 AM Neil Armstrong
<neil.armstrong@xxxxxxxxxx> wrote:
>
> On 7/28/26 03:02, Aaron Kling via B4 Relay wrote:
> > From: Aaron Kling <webgeek1234@xxxxxxxxx>
> >
> > These ddic's do not handle the standard mipi dsi command for pps.
> > Instead, there are per chip vendor commands to write the packed struct
> > directly to manufacturer registers.
> >
> > Signed-off-by: Aaron Kling <webgeek1234@xxxxxxxxx>
>
> Any Fixes tag ?

Ah oops, I did forget that.

Fixes: f747473a838ec ("drm/panel: Add panel driver for Chipone
ICNA35XX based panels")

>
> > ---
> > This patch results in no change at runtime because the ddic defaults to
> > the config being set by the existing mode. The issue was found while
> > doing testing running the panel in a 10-bit mode. But since it is a real
> > problem, better to fix it early.
> > ---
> >   drivers/gpu/drm/panel/panel-chipone-icna35xx.c | 20 ++++++++++++++++++--
> >   1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > 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
> > @@ -69,6 +69,7 @@ static int icna3512_init_sequence(struct panel_info *pinfo)
> >   {
> >       struct mipi_dsi_multi_context dsi_ctx = { .dsi = pinfo->dsi };
> >       struct drm_dsc_picture_parameter_set pps;
> > +     u8 *pps_ptr = (u8 *)&pps;
> >
> >       pinfo->dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> >
> > @@ -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);
> > +
> > +     mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xFD, 0xA5, 0xA5);
> >
> >       mipi_dsi_msleep(&dsi_ctx, 20);
> >
> > @@ -104,6 +112,7 @@ static int icna3520_init_sequence(struct panel_info *pinfo)
> >   {
> >       struct mipi_dsi_multi_context dsi_ctx = { .dsi = pinfo->dsi };
> >       struct drm_dsc_picture_parameter_set pps;
> > +     u8 *pps_ptr = (u8 *)&pps;
> >
> >       pinfo->dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> >
> > @@ -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);
> > +
> > +     mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xFD, 0xA5, 0xA5);
> >
> >       mipi_dsi_msleep(&dsi_ctx, 20);
> >
>
> Otherwise it looks good.
>
> Thanks,
> Neil
>
> >
> > ---
> > base-commit: 0d33d21e47d9dc66f91e44da3fc9220c74d93df7
> > change-id: 20260727-icna35xx-pps-bdd353e188cc
> >
> > Best regards,
>

Aaron