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

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

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

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@gmail.com>
---
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);
 

---
base-commit: 0d33d21e47d9dc66f91e44da3fc9220c74d93df7
change-id: 20260727-icna35xx-pps-bdd353e188cc

Best regards,
-- 
Aaron Kling <webgeek1234@gmail.com>

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

[Neil Armstrong]

On 7/28/26 03:02, Aaron Kling via B4 Relay wrote:
> From: Aaron Kling <webgeek1234@gmail.com>
> 
> 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@gmail.com>

Any Fixes tag ?

> ---
> 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,

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@linaro.org> wrote:
>
> On 7/28/26 03:02, Aaron Kling via B4 Relay wrote:
> > From: Aaron Kling <webgeek1234@gmail.com>
> >
> > 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@gmail.com>
>
> 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