diff mbox series

drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions

Message ID 20240904141521.554451-1-tejasvipin76@gmail.com (mailing list archive)
State New, archived
Headers show
Series drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions | expand

Commit Message

Tejas Vipin Sept. 4, 2024, 2:15 p.m. UTC
Changes the himax-hx83112a panel to use multi style functions for
improved error handling.

Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
---
 drivers/gpu/drm/panel/panel-himax-hx83112a.c | 297 +++++++++----------
 1 file changed, 136 insertions(+), 161 deletions(-)

Comments

Doug Anderson Sept. 4, 2024, 5:52 p.m. UTC | #1
Hi,

On Wed, Sep 4, 2024 at 7:15 AM Tejas Vipin <tejasvipin76@gmail.com> wrote:
>
> Changes the himax-hx83112a panel to use multi style functions for
> improved error handling.
>
> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> ---
>  drivers/gpu/drm/panel/panel-himax-hx83112a.c | 297 +++++++++----------
>  1 file changed, 136 insertions(+), 161 deletions(-)

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Jessica Zhang Sept. 6, 2024, 10:14 p.m. UTC | #2
On 9/4/2024 7:15 AM, Tejas Vipin wrote:
> Changes the himax-hx83112a panel to use multi style functions for
> improved error handling.
> 
> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>

Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>

> ---
>   drivers/gpu/drm/panel/panel-himax-hx83112a.c | 297 +++++++++----------
>   1 file changed, 136 insertions(+), 161 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-himax-hx83112a.c b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
> index 466c27012abf..47bce087e339 100644
> --- a/drivers/gpu/drm/panel/panel-himax-hx83112a.c
> +++ b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
> @@ -56,198 +56,173 @@ static void hx83112a_reset(struct hx83112a_panel *ctx)
>   	msleep(50);
>   }
>   
> -static int hx83112a_on(struct hx83112a_panel *ctx)
> +static int hx83112a_on(struct mipi_dsi_device *dsi)
>   {
> -	struct mipi_dsi_device *dsi = ctx->dsi;
> -	struct device *dev = &dsi->dev;
> -	int ret;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>   
>   	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
>   
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETEXTC, 0x83, 0x11, 0x2a);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER1,
> -			       0x08, 0x28, 0x28, 0x83, 0x83, 0x4c, 0x4f, 0x33);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDISP,
> -			       0x00, 0x02, 0x00, 0x90, 0x24, 0x00, 0x08, 0x19,
> -			       0xea, 0x11, 0x11, 0x00, 0x11, 0xa3);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
> -			       0x58, 0x68, 0x58, 0x68, 0x0f, 0xef, 0x0b, 0xc0,
> -			       0x0b, 0xc0, 0x0b, 0xc0, 0x00, 0xff, 0x00, 0xff,
> -			       0x00, 0x00, 0x14, 0x15, 0x00, 0x29, 0x11, 0x07,
> -			       0x12, 0x00, 0x29);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
> -			       0x00, 0x12, 0x12, 0x11, 0x88, 0x12, 0x12, 0x00,
> -			       0x53);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
> -			       0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
> -			       0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
> -			       0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
> -			       0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
> -			       0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
> -			       0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
> -			       0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
> -			       0x40);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
> -			       0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
> -			       0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
> -			       0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
> -			       0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
> -			       0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
> -			       0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
> -			       0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
> -			       0x40);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
> -			       0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
> -			       0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
> -			       0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
> -			       0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
> -			       0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
> -			       0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
> -			       0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
> -			       0x40);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT, 0x01);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTCON,
> -			       0x70, 0x00, 0x04, 0xe0, 0x33, 0x00);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPANEL, 0x08);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER2, 0x2b, 0x2b);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
> -			       0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08,
> -			       0x08, 0x03, 0x03, 0x22, 0x18, 0x07, 0x07, 0x07,
> -			       0x07, 0x32, 0x10, 0x06, 0x00, 0x06, 0x32, 0x10,
> -			       0x07, 0x00, 0x07, 0x32, 0x19, 0x31, 0x09, 0x31,
> -			       0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x08,
> -			       0x09, 0x30, 0x00, 0x00, 0x00, 0x06, 0x0d, 0x00,
> -			       0x0f);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
> -			       0x00, 0x00, 0x19, 0x10, 0x00, 0x0a, 0x00, 0x81);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP1,
> -			       0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
> -			       0xc0, 0xc0, 0x18, 0x18, 0x19, 0x19, 0x18, 0x18,
> -			       0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
> -			       0x28, 0x28, 0x24, 0x24, 0x02, 0x03, 0x02, 0x03,
> -			       0x00, 0x01, 0x00, 0x01, 0x31, 0x31, 0x31, 0x31,
> -			       0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP2,
> -			       0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
> -			       0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x19, 0x19,
> -			       0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
> -			       0x24, 0x24, 0x28, 0x28, 0x01, 0x00, 0x01, 0x00,
> -			       0x03, 0x02, 0x03, 0x02, 0x31, 0x31, 0x31, 0x31,
> -			       0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
> -			       0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea,
> -			       0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa,
> -			       0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa, 0xaa, 0xaa);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
> -			       0xaa, 0x2e, 0x28, 0x00, 0x00, 0x00, 0xaa, 0x2e,
> -			       0x28, 0x00, 0x00, 0x00, 0xaa, 0xee, 0xaa, 0xaa,
> -			       0xaa, 0xaa, 0xaa, 0xee, 0xaa, 0xaa, 0xaa, 0xaa);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
> -			       0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
> -			       0xff, 0xff, 0xff, 0xff);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
> -			       0xaa, 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
> -			       0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xff, 0xff, 0xff,
> -			       0xff, 0xff, 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
> -			       0x0e, 0x0e, 0x1e, 0x65, 0x1c, 0x65, 0x00, 0x50,
> -			       0x20, 0x20, 0x00, 0x00, 0x02, 0x02, 0x02, 0x05,
> -			       0x14, 0x14, 0x32, 0xb9, 0x23, 0xb9, 0x08);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
> -			       0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
> -			       0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x00, 0x00,
> -			       0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> -			       0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
> -			       0x00, 0x00, 0x00, 0x02, 0x00);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0xc3);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETCLOCK, 0xd1, 0xd6);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0x3f);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0xc6);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPTBA, 0x37);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0x3f);
> -
> -	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
> -		return ret;
> -	}
> -	msleep(150);
> -
> -	ret = mipi_dsi_dcs_set_display_on(dsi);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed to set display on: %d\n", ret);
> -		return ret;
> -	}
> -	msleep(50);
> -
> -	return 0;
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETEXTC, 0x83, 0x11, 0x2a);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPOWER1,
> +				     0x08, 0x28, 0x28, 0x83, 0x83, 0x4c, 0x4f, 0x33);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDISP,
> +				     0x00, 0x02, 0x00, 0x90, 0x24, 0x00, 0x08, 0x19,
> +				     0xea, 0x11, 0x11, 0x00, 0x11, 0xa3);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDRV,
> +				     0x58, 0x68, 0x58, 0x68, 0x0f, 0xef, 0x0b, 0xc0,
> +				     0x0b, 0xc0, 0x0b, 0xc0, 0x00, 0xff, 0x00, 0xff,
> +				     0x00, 0x00, 0x14, 0x15, 0x00, 0x29, 0x11, 0x07,
> +				     0x12, 0x00, 0x29);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDRV,
> +				     0x00, 0x12, 0x12, 0x11, 0x88, 0x12, 0x12, 0x00,
> +				     0x53);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x03);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
> +				     0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
> +				     0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
> +				     0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
> +				     0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
> +				     0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
> +				     0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
> +				     0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
> +				     0x40);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
> +				     0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
> +				     0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
> +				     0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
> +				     0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
> +				     0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
> +				     0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
> +				     0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
> +				     0x40);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
> +				     0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
> +				     0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
> +				     0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
> +				     0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
> +				     0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
> +				     0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
> +				     0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
> +				     0x40);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTCON,
> +				     0x70, 0x00, 0x04, 0xe0, 0x33, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPANEL, 0x08);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPOWER2, 0x2b, 0x2b);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP0,
> +				     0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08,
> +				     0x08, 0x03, 0x03, 0x22, 0x18, 0x07, 0x07, 0x07,
> +				     0x07, 0x32, 0x10, 0x06, 0x00, 0x06, 0x32, 0x10,
> +				     0x07, 0x00, 0x07, 0x32, 0x19, 0x31, 0x09, 0x31,
> +				     0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x08,
> +				     0x09, 0x30, 0x00, 0x00, 0x00, 0x06, 0x0d, 0x00,
> +				     0x0f);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP0,
> +				     0x00, 0x00, 0x19, 0x10, 0x00, 0x0a, 0x00, 0x81);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP1,
> +				     0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
> +				     0xc0, 0xc0, 0x18, 0x18, 0x19, 0x19, 0x18, 0x18,
> +				     0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
> +				     0x28, 0x28, 0x24, 0x24, 0x02, 0x03, 0x02, 0x03,
> +				     0x00, 0x01, 0x00, 0x01, 0x31, 0x31, 0x31, 0x31,
> +				     0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP2,
> +				     0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
> +				     0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x19, 0x19,
> +				     0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
> +				     0x24, 0x24, 0x28, 0x28, 0x01, 0x00, 0x01, 0x00,
> +				     0x03, 0x02, 0x03, 0x02, 0x31, 0x31, 0x31, 0x31,
> +				     0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
> +				     0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea,
> +				     0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa,
> +				     0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa, 0xaa, 0xaa);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
> +				     0xaa, 0x2e, 0x28, 0x00, 0x00, 0x00, 0xaa, 0x2e,
> +				     0x28, 0x00, 0x00, 0x00, 0xaa, 0xee, 0xaa, 0xaa,
> +				     0xaa, 0xaa, 0xaa, 0xee, 0xaa, 0xaa, 0xaa, 0xaa);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
> +				     0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
> +				     0xff, 0xff, 0xff, 0xff);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x03);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
> +				     0xaa, 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
> +				     0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xff, 0xff, 0xff,
> +				     0xff, 0xff, 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> +				     0x0e, 0x0e, 0x1e, 0x65, 0x1c, 0x65, 0x00, 0x50,
> +				     0x20, 0x20, 0x00, 0x00, 0x02, 0x02, 0x02, 0x05,
> +				     0x14, 0x14, 0x32, 0xb9, 0x23, 0xb9, 0x08);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> +				     0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> +				     0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x00, 0x00,
> +				     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +				     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
> +				     0x00, 0x00, 0x00, 0x02, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0xc3);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETCLOCK, 0xd1, 0xd6);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0x3f);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0xc6);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPTBA, 0x37);
> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0x3f);
> +
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, 150);
> +
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, 50);
> +
> +	return dsi_ctx.accum_err;
>   }
>   
>   static int hx83112a_disable(struct drm_panel *panel)
>   {
>   	struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
>   	struct mipi_dsi_device *dsi = ctx->dsi;
> -	struct device *dev = &dsi->dev;
> -	int ret;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>   
>   	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
>   
> -	ret = mipi_dsi_dcs_set_display_off(dsi);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed to set display off: %d\n", ret);
> -		return ret;
> -	}
> -	msleep(20);
> -
> -	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
> -		return ret;
> -	}
> -	msleep(120);
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, 20);
> +	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, 120);
>   
> -	return 0;
> +	return dsi_ctx.accum_err;
>   }
>   
>   static int hx83112a_prepare(struct drm_panel *panel)
>   {
>   	struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
> -	struct device *dev = &ctx->dsi->dev;
>   	int ret;
>   
>   	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed to enable regulators: %d\n", ret);
> +	if (ret < 0)
>   		return ret;
> -	}
>   
>   	hx83112a_reset(ctx);
>   
> -	ret = hx83112a_on(ctx);
> +	ret = hx83112a_on(ctx->dsi);
>   	if (ret < 0) {
> -		dev_err(dev, "Failed to initialize panel: %d\n", ret);
>   		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
>   		regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> -		return ret;
>   	}
>   
> -	return 0;
> +	return ret;
>   }
>   
>   static int hx83112a_unprepare(struct drm_panel *panel)
> -- 
> 2.46.0
>
Jessica Zhang Sept. 6, 2024, 10:23 p.m. UTC | #3
On 9/6/2024 3:14 PM, Jessica Zhang wrote:
> 
> 
> On 9/4/2024 7:15 AM, Tejas Vipin wrote:
>> Changes the himax-hx83112a panel to use multi style functions for
>> improved error handling.
>>
>> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> 
> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>

Hi Tejas,

Just a heads up, it seems that this might be a duplicate of this change [1]?

Thanks,

Jessica Zhang

[1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1

> 
>> ---
>>   drivers/gpu/drm/panel/panel-himax-hx83112a.c | 297 +++++++++----------
>>   1 file changed, 136 insertions(+), 161 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/panel/panel-himax-hx83112a.c 
>> b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
>> index 466c27012abf..47bce087e339 100644
>> --- a/drivers/gpu/drm/panel/panel-himax-hx83112a.c
>> +++ b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
>> @@ -56,198 +56,173 @@ static void hx83112a_reset(struct hx83112a_panel 
>> *ctx)
>>       msleep(50);
>>   }
>> -static int hx83112a_on(struct hx83112a_panel *ctx)
>> +static int hx83112a_on(struct mipi_dsi_device *dsi)
>>   {
>> -    struct mipi_dsi_device *dsi = ctx->dsi;
>> -    struct device *dev = &dsi->dev;
>> -    int ret;
>> +    struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>>       dsi->mode_flags |= MIPI_DSI_MODE_LPM;
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETEXTC, 0x83, 0x11, 0x2a);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER1,
>> -                   0x08, 0x28, 0x28, 0x83, 0x83, 0x4c, 0x4f, 0x33);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDISP,
>> -                   0x00, 0x02, 0x00, 0x90, 0x24, 0x00, 0x08, 0x19,
>> -                   0xea, 0x11, 0x11, 0x00, 0x11, 0xa3);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
>> -                   0x58, 0x68, 0x58, 0x68, 0x0f, 0xef, 0x0b, 0xc0,
>> -                   0x0b, 0xc0, 0x0b, 0xc0, 0x00, 0xff, 0x00, 0xff,
>> -                   0x00, 0x00, 0x14, 0x15, 0x00, 0x29, 0x11, 0x07,
>> -                   0x12, 0x00, 0x29);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
>> -                   0x00, 0x12, 0x12, 0x11, 0x88, 0x12, 0x12, 0x00,
>> -                   0x53);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
>> -                   0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
>> -                   0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
>> -                   0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
>> -                   0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
>> -                   0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
>> -                   0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
>> -                   0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
>> -                   0x40);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
>> -                   0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
>> -                   0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
>> -                   0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
>> -                   0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
>> -                   0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
>> -                   0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
>> -                   0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
>> -                   0x40);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
>> -                   0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
>> -                   0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
>> -                   0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
>> -                   0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
>> -                   0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
>> -                   0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
>> -                   0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
>> -                   0x40);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT, 0x01);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTCON,
>> -                   0x70, 0x00, 0x04, 0xe0, 0x33, 0x00);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPANEL, 0x08);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER2, 0x2b, 0x2b);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
>> -                   0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08,
>> -                   0x08, 0x03, 0x03, 0x22, 0x18, 0x07, 0x07, 0x07,
>> -                   0x07, 0x32, 0x10, 0x06, 0x00, 0x06, 0x32, 0x10,
>> -                   0x07, 0x00, 0x07, 0x32, 0x19, 0x31, 0x09, 0x31,
>> -                   0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x08,
>> -                   0x09, 0x30, 0x00, 0x00, 0x00, 0x06, 0x0d, 0x00,
>> -                   0x0f);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
>> -                   0x00, 0x00, 0x19, 0x10, 0x00, 0x0a, 0x00, 0x81);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP1,
>> -                   0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
>> -                   0xc0, 0xc0, 0x18, 0x18, 0x19, 0x19, 0x18, 0x18,
>> -                   0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
>> -                   0x28, 0x28, 0x24, 0x24, 0x02, 0x03, 0x02, 0x03,
>> -                   0x00, 0x01, 0x00, 0x01, 0x31, 0x31, 0x31, 0x31,
>> -                   0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP2,
>> -                   0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
>> -                   0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x19, 0x19,
>> -                   0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
>> -                   0x24, 0x24, 0x28, 0x28, 0x01, 0x00, 0x01, 0x00,
>> -                   0x03, 0x02, 0x03, 0x02, 0x31, 0x31, 0x31, 0x31,
>> -                   0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
>> -                   0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea,
>> -                   0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa,
>> -                   0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa, 0xaa, 0xaa);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
>> -                   0xaa, 0x2e, 0x28, 0x00, 0x00, 0x00, 0xaa, 0x2e,
>> -                   0x28, 0x00, 0x00, 0x00, 0xaa, 0xee, 0xaa, 0xaa,
>> -                   0xaa, 0xaa, 0xaa, 0xee, 0xaa, 0xaa, 0xaa, 0xaa);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
>> -                   0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
>> -                   0xff, 0xff, 0xff, 0xff);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
>> -                   0xaa, 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
>> -                   0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xff, 0xff, 0xff,
>> -                   0xff, 0xff, 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
>> -                   0x0e, 0x0e, 0x1e, 0x65, 0x1c, 0x65, 0x00, 0x50,
>> -                   0x20, 0x20, 0x00, 0x00, 0x02, 0x02, 0x02, 0x05,
>> -                   0x14, 0x14, 0x32, 0xb9, 0x23, 0xb9, 0x08);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
>> -                   0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
>> -                   0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x00, 0x00,
>> -                   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> -                   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
>> -                   0x00, 0x00, 0x00, 0x02, 0x00);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0xc3);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETCLOCK, 0xd1, 0xd6);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0x3f);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0xc6);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPTBA, 0x37);
>> -    mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0x3f);
>> -
>> -    ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
>> -    if (ret < 0) {
>> -        dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
>> -        return ret;
>> -    }
>> -    msleep(150);
>> -
>> -    ret = mipi_dsi_dcs_set_display_on(dsi);
>> -    if (ret < 0) {
>> -        dev_err(dev, "Failed to set display on: %d\n", ret);
>> -        return ret;
>> -    }
>> -    msleep(50);
>> -
>> -    return 0;
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETEXTC, 0x83, 
>> 0x11, 0x2a);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPOWER1,
>> +                     0x08, 0x28, 0x28, 0x83, 0x83, 0x4c, 0x4f, 0x33);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDISP,
>> +                     0x00, 0x02, 0x00, 0x90, 0x24, 0x00, 0x08, 0x19,
>> +                     0xea, 0x11, 0x11, 0x00, 0x11, 0xa3);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDRV,
>> +                     0x58, 0x68, 0x58, 0x68, 0x0f, 0xef, 0x0b, 0xc0,
>> +                     0x0b, 0xc0, 0x0b, 0xc0, 0x00, 0xff, 0x00, 0xff,
>> +                     0x00, 0x00, 0x14, 0x15, 0x00, 0x29, 0x11, 0x07,
>> +                     0x12, 0x00, 0x29);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDRV,
>> +                     0x00, 0x12, 0x12, 0x11, 0x88, 0x12, 0x12, 0x00,
>> +                     0x53);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x03);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
>> +                     0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
>> +                     0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
>> +                     0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
>> +                     0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
>> +                     0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
>> +                     0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
>> +                     0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
>> +                     0x40);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
>> +                     0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
>> +                     0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
>> +                     0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
>> +                     0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
>> +                     0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
>> +                     0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
>> +                     0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
>> +                     0x40);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
>> +                     0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
>> +                     0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
>> +                     0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
>> +                     0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
>> +                     0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
>> +                     0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
>> +                     0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
>> +                     0x40);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT, 0x01);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTCON,
>> +                     0x70, 0x00, 0x04, 0xe0, 0x33, 0x00);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPANEL, 0x08);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPOWER2, 0x2b, 
>> 0x2b);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP0,
>> +                     0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08,
>> +                     0x08, 0x03, 0x03, 0x22, 0x18, 0x07, 0x07, 0x07,
>> +                     0x07, 0x32, 0x10, 0x06, 0x00, 0x06, 0x32, 0x10,
>> +                     0x07, 0x00, 0x07, 0x32, 0x19, 0x31, 0x09, 0x31,
>> +                     0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x08,
>> +                     0x09, 0x30, 0x00, 0x00, 0x00, 0x06, 0x0d, 0x00,
>> +                     0x0f);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP0,
>> +                     0x00, 0x00, 0x19, 0x10, 0x00, 0x0a, 0x00, 0x81);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP1,
>> +                     0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
>> +                     0xc0, 0xc0, 0x18, 0x18, 0x19, 0x19, 0x18, 0x18,
>> +                     0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
>> +                     0x28, 0x28, 0x24, 0x24, 0x02, 0x03, 0x02, 0x03,
>> +                     0x00, 0x01, 0x00, 0x01, 0x31, 0x31, 0x31, 0x31,
>> +                     0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP2,
>> +                     0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
>> +                     0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x19, 0x19,
>> +                     0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
>> +                     0x24, 0x24, 0x28, 0x28, 0x01, 0x00, 0x01, 0x00,
>> +                     0x03, 0x02, 0x03, 0x02, 0x31, 0x31, 0x31, 0x31,
>> +                     0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
>> +                     0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea,
>> +                     0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa,
>> +                     0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa, 0xaa, 0xaa);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
>> +                     0xaa, 0x2e, 0x28, 0x00, 0x00, 0x00, 0xaa, 0x2e,
>> +                     0x28, 0x00, 0x00, 0x00, 0xaa, 0xee, 0xaa, 0xaa,
>> +                     0xaa, 0xaa, 0xaa, 0xee, 0xaa, 0xaa, 0xaa, 0xaa);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
>> +                     0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
>> +                     0xff, 0xff, 0xff, 0xff);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x03);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
>> +                     0xaa, 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
>> +                     0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xff, 0xff, 0xff,
>> +                     0xff, 0xff, 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
>> +                     0x0e, 0x0e, 0x1e, 0x65, 0x1c, 0x65, 0x00, 0x50,
>> +                     0x20, 0x20, 0x00, 0x00, 0x02, 0x02, 0x02, 0x05,
>> +                     0x14, 0x14, 0x32, 0xb9, 0x23, 0xb9, 0x08);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
>> +                     0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
>> +                     0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x00, 0x00,
>> +                     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> +                     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
>> +                     0x00, 0x00, 0x00, 0x02, 0x00);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0xc3);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETCLOCK, 0xd1, 
>> 0xd6);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0x3f);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0xc6);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPTBA, 0x37);
>> +    mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0x3f);
>> +
>> +    mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
>> +    mipi_dsi_msleep(&dsi_ctx, 150);
>> +
>> +    mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
>> +    mipi_dsi_msleep(&dsi_ctx, 50);
>> +
>> +    return dsi_ctx.accum_err;
>>   }
>>   static int hx83112a_disable(struct drm_panel *panel)
>>   {
>>       struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
>>       struct mipi_dsi_device *dsi = ctx->dsi;
>> -    struct device *dev = &dsi->dev;
>> -    int ret;
>> +    struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>>       dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
>> -    ret = mipi_dsi_dcs_set_display_off(dsi);
>> -    if (ret < 0) {
>> -        dev_err(dev, "Failed to set display off: %d\n", ret);
>> -        return ret;
>> -    }
>> -    msleep(20);
>> -
>> -    ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
>> -    if (ret < 0) {
>> -        dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
>> -        return ret;
>> -    }
>> -    msleep(120);
>> +    mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
>> +    mipi_dsi_msleep(&dsi_ctx, 20);
>> +    mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>> +    mipi_dsi_msleep(&dsi_ctx, 120);
>> -    return 0;
>> +    return dsi_ctx.accum_err;
>>   }
>>   static int hx83112a_prepare(struct drm_panel *panel)
>>   {
>>       struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
>> -    struct device *dev = &ctx->dsi->dev;
>>       int ret;
>>       ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), 
>> ctx->supplies);
>> -    if (ret < 0) {
>> -        dev_err(dev, "Failed to enable regulators: %d\n", ret);
>> +    if (ret < 0)
>>           return ret;
>> -    }
>>       hx83112a_reset(ctx);
>> -    ret = hx83112a_on(ctx);
>> +    ret = hx83112a_on(ctx->dsi);
>>       if (ret < 0) {
>> -        dev_err(dev, "Failed to initialize panel: %d\n", ret);
>>           gpiod_set_value_cansleep(ctx->reset_gpio, 1);
>>           regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), 
>> ctx->supplies);
>> -        return ret;
>>       }
>> -    return 0;
>> +    return ret;
>>   }
>>   static int hx83112a_unprepare(struct drm_panel *panel)
>> -- 
>> 2.46.0
>>
Tejas Vipin Sept. 7, 2024, 8:32 a.m. UTC | #4
On 9/7/24 3:53 AM, Jessica Zhang wrote:
> 
> 
> On 9/6/2024 3:14 PM, Jessica Zhang wrote:
>>
>>
>> On 9/4/2024 7:15 AM, Tejas Vipin wrote:
>>> Changes the himax-hx83112a panel to use multi style functions for
>>> improved error handling.
>>>
>>> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
>>
>> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> 
> Hi Tejas,
> 
> Just a heads up, it seems that this might be a duplicate of this change [1]?
> 
> Thanks,
> 
> Jessica Zhang
> 
> [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1

Ah, thanks for letting me know. I hadn't realized someone else had
started working on this too. 

However, I would argue that my patch [2] is a better candidate for merging
because of the following reasons:

1) Removes unnecessary error printing:
The mipi_dsi_*_multi() functions all have inbuilt error printing which
makes printing errors after hx83112a_on unnecessary as is addressed in
[2] like so:

> -	ret = hx83112a_on(ctx);
> +	ret = hx83112a_on(ctx->dsi);
>  	if (ret < 0) {
> -		dev_err(dev, "Failed to initialize panel: %d\n", ret);
>  		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
>  		regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> -		return ret;
>  	}

[2] also removes the unnecessary dev_err after regulator_bulk_enable as was
addressed in [3] like so:

>  	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed to enable regulators: %d\n", ret);
> +	if (ret < 0)
>  		return ret;
> -	}

2) Better formatting

The mipi_dsi_dcs_write_seq_multi statements in [1] aren't formatted
quite right according to what has been done so far. They are written as
such in [1]:

> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
>  			       0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);

Where they should be written as such in [2]:

> +	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> +				     0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);

All in all, the module generated using my patch ends up being a teensy
bit smaller (1% smaller). But if chronology is what is important, then
it would at least be nice to see the above changes as part of Abhishek's
patch too. And I'll be sure to look at the mail in the drm inbox now
onwards :p

[1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
[2] https://lore.kernel.org/all/20240904141521.554451-1-tejasvipin76@gmail.com/
[3] https://lore.kernel.org/all/CAD=FV=XRZKL_ppjUKDK61fQkWhHiQCJLfmVBS7wSo4sUux2g8Q@mail.gmail.com/
Doug Anderson Sept. 10, 2024, 9:19 p.m. UTC | #5
Hi,

On Sat, Sep 7, 2024 at 1:32 AM Tejas Vipin <tejasvipin76@gmail.com> wrote:
>
> On 9/7/24 3:53 AM, Jessica Zhang wrote:
> >
> >
> > On 9/6/2024 3:14 PM, Jessica Zhang wrote:
> >>
> >>
> >> On 9/4/2024 7:15 AM, Tejas Vipin wrote:
> >>> Changes the himax-hx83112a panel to use multi style functions for
> >>> improved error handling.
> >>>
> >>> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> >>
> >> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> >
> > Hi Tejas,
> >
> > Just a heads up, it seems that this might be a duplicate of this change [1]?
> >
> > Thanks,
> >
> > Jessica Zhang
> >
> > [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
>
> Ah, thanks for letting me know. I hadn't realized someone else had
> started working on this too.
>
> However, I would argue that my patch [2] is a better candidate for merging
> because of the following reasons:
>
> 1) Removes unnecessary error printing:
> The mipi_dsi_*_multi() functions all have inbuilt error printing which
> makes printing errors after hx83112a_on unnecessary as is addressed in
> [2] like so:
>
> > -     ret = hx83112a_on(ctx);
> > +     ret = hx83112a_on(ctx->dsi);
> >       if (ret < 0) {
> > -             dev_err(dev, "Failed to initialize panel: %d\n", ret);
> >               gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> >               regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > -             return ret;
> >       }
>
> [2] also removes the unnecessary dev_err after regulator_bulk_enable as was
> addressed in [3] like so:
>
> >       ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > -     if (ret < 0) {
> > -             dev_err(dev, "Failed to enable regulators: %d\n", ret);
> > +     if (ret < 0)
> >               return ret;
> > -     }
>
> 2) Better formatting
>
> The mipi_dsi_dcs_write_seq_multi statements in [1] aren't formatted
> quite right according to what has been done so far. They are written as
> such in [1]:
>
> > +     mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> >                              0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
>
> Where they should be written as such in [2]:
>
> > +     mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> > +                                  0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
>
> All in all, the module generated using my patch ends up being a teensy
> bit smaller (1% smaller). But if chronology is what is important, then
> it would at least be nice to see the above changes as part of Abhishek's
> patch too. And I'll be sure to look at the mail in the drm inbox now
> onwards :p
>
> [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> [2] https://lore.kernel.org/all/20240904141521.554451-1-tejasvipin76@gmail.com/
> [3] https://lore.kernel.org/all/CAD=FV=XRZKL_ppjUKDK61fQkWhHiQCJLfmVBS7wSo4sUux2g8Q@mail.gmail.com/

I would tend to agree that Tejas's patch looks slightly better, but
Abhishek's patch appears to have been posted first.

Neil: any idea what to do here? Maybe a Co-Developed-by or something?
...or we could land Abhishek and Tejas could post a followup for the
extra cleanup?

Abhishek: are you planning to post more _multi cleanups? If so, please
make sure to CC Tejas (who has been posting a bunch of them) and
perhaps me since I've been helping to review them a bit.

-Doug
Neil Armstrong Sept. 11, 2024, 7:41 a.m. UTC | #6
On 10/09/2024 23:19, Doug Anderson wrote:
> Hi,
> 
> On Sat, Sep 7, 2024 at 1:32 AM Tejas Vipin <tejasvipin76@gmail.com> wrote:
>>
>> On 9/7/24 3:53 AM, Jessica Zhang wrote:
>>>
>>>
>>> On 9/6/2024 3:14 PM, Jessica Zhang wrote:
>>>>
>>>>
>>>> On 9/4/2024 7:15 AM, Tejas Vipin wrote:
>>>>> Changes the himax-hx83112a panel to use multi style functions for
>>>>> improved error handling.
>>>>>
>>>>> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
>>>>
>>>> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>>>
>>> Hi Tejas,
>>>
>>> Just a heads up, it seems that this might be a duplicate of this change [1]?
>>>
>>> Thanks,
>>>
>>> Jessica Zhang
>>>
>>> [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
>>
>> Ah, thanks for letting me know. I hadn't realized someone else had
>> started working on this too.
>>
>> However, I would argue that my patch [2] is a better candidate for merging
>> because of the following reasons:
>>
>> 1) Removes unnecessary error printing:
>> The mipi_dsi_*_multi() functions all have inbuilt error printing which
>> makes printing errors after hx83112a_on unnecessary as is addressed in
>> [2] like so:
>>
>>> -     ret = hx83112a_on(ctx);
>>> +     ret = hx83112a_on(ctx->dsi);
>>>        if (ret < 0) {
>>> -             dev_err(dev, "Failed to initialize panel: %d\n", ret);
>>>                gpiod_set_value_cansleep(ctx->reset_gpio, 1);
>>>                regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
>>> -             return ret;
>>>        }
>>
>> [2] also removes the unnecessary dev_err after regulator_bulk_enable as was
>> addressed in [3] like so:
>>
>>>        ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
>>> -     if (ret < 0) {
>>> -             dev_err(dev, "Failed to enable regulators: %d\n", ret);
>>> +     if (ret < 0)
>>>                return ret;
>>> -     }
>>
>> 2) Better formatting
>>
>> The mipi_dsi_dcs_write_seq_multi statements in [1] aren't formatted
>> quite right according to what has been done so far. They are written as
>> such in [1]:
>>
>>> +     mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
>>>                               0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
>>
>> Where they should be written as such in [2]:
>>
>>> +     mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
>>> +                                  0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
>>
>> All in all, the module generated using my patch ends up being a teensy
>> bit smaller (1% smaller). But if chronology is what is important, then
>> it would at least be nice to see the above changes as part of Abhishek's
>> patch too. And I'll be sure to look at the mail in the drm inbox now
>> onwards :p
>>
>> [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
>> [2] https://lore.kernel.org/all/20240904141521.554451-1-tejasvipin76@gmail.com/
>> [3] https://lore.kernel.org/all/CAD=FV=XRZKL_ppjUKDK61fQkWhHiQCJLfmVBS7wSo4sUux2g8Q@mail.gmail.com/
> 
> I would tend to agree that Tejas's patch looks slightly better, but
> Abhishek's patch appears to have been posted first.
> 
> Neil: any idea what to do here? Maybe a Co-Developed-by or something?
> ...or we could land Abhishek and Tejas could post a followup for the
> extra cleanup?

Yeah usually I take the first one when they are equal, but indeed Tejas
cleanup up a little further and better aligned the parameters so I think
Tejas's one is a better looking version.

In this case we should apply Teja's one, nothing personal Abhishek!

> 
> Abhishek: are you planning to post more _multi cleanups? If so, please
> make sure to CC Tejas (who has been posting a bunch of them) and
> perhaps me since I've been helping to review them a bit.
> 
> -Doug
Doug Anderson Sept. 11, 2024, 8:07 p.m. UTC | #7
Hi,

On Wed, Sep 11, 2024 at 12:41 AM <neil.armstrong@linaro.org> wrote:
>
> On 10/09/2024 23:19, Doug Anderson wrote:
> > Hi,
> >
> > On Sat, Sep 7, 2024 at 1:32 AM Tejas Vipin <tejasvipin76@gmail.com> wrote:
> >>
> >> On 9/7/24 3:53 AM, Jessica Zhang wrote:
> >>>
> >>>
> >>> On 9/6/2024 3:14 PM, Jessica Zhang wrote:
> >>>>
> >>>>
> >>>> On 9/4/2024 7:15 AM, Tejas Vipin wrote:
> >>>>> Changes the himax-hx83112a panel to use multi style functions for
> >>>>> improved error handling.
> >>>>>
> >>>>> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> >>>>
> >>>> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> >>>
> >>> Hi Tejas,
> >>>
> >>> Just a heads up, it seems that this might be a duplicate of this change [1]?
> >>>
> >>> Thanks,
> >>>
> >>> Jessica Zhang
> >>>
> >>> [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> >>
> >> Ah, thanks for letting me know. I hadn't realized someone else had
> >> started working on this too.
> >>
> >> However, I would argue that my patch [2] is a better candidate for merging
> >> because of the following reasons:
> >>
> >> 1) Removes unnecessary error printing:
> >> The mipi_dsi_*_multi() functions all have inbuilt error printing which
> >> makes printing errors after hx83112a_on unnecessary as is addressed in
> >> [2] like so:
> >>
> >>> -     ret = hx83112a_on(ctx);
> >>> +     ret = hx83112a_on(ctx->dsi);
> >>>        if (ret < 0) {
> >>> -             dev_err(dev, "Failed to initialize panel: %d\n", ret);
> >>>                gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> >>>                regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> >>> -             return ret;
> >>>        }
> >>
> >> [2] also removes the unnecessary dev_err after regulator_bulk_enable as was
> >> addressed in [3] like so:
> >>
> >>>        ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> >>> -     if (ret < 0) {
> >>> -             dev_err(dev, "Failed to enable regulators: %d\n", ret);
> >>> +     if (ret < 0)
> >>>                return ret;
> >>> -     }
> >>
> >> 2) Better formatting
> >>
> >> The mipi_dsi_dcs_write_seq_multi statements in [1] aren't formatted
> >> quite right according to what has been done so far. They are written as
> >> such in [1]:
> >>
> >>> +     mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> >>>                               0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> >>
> >> Where they should be written as such in [2]:
> >>
> >>> +     mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> >>> +                                  0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> >>
> >> All in all, the module generated using my patch ends up being a teensy
> >> bit smaller (1% smaller). But if chronology is what is important, then
> >> it would at least be nice to see the above changes as part of Abhishek's
> >> patch too. And I'll be sure to look at the mail in the drm inbox now
> >> onwards :p
> >>
> >> [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> >> [2] https://lore.kernel.org/all/20240904141521.554451-1-tejasvipin76@gmail.com/
> >> [3] https://lore.kernel.org/all/CAD=FV=XRZKL_ppjUKDK61fQkWhHiQCJLfmVBS7wSo4sUux2g8Q@mail.gmail.com/
> >
> > I would tend to agree that Tejas's patch looks slightly better, but
> > Abhishek's patch appears to have been posted first.
> >
> > Neil: any idea what to do here? Maybe a Co-Developed-by or something?
> > ...or we could land Abhishek and Tejas could post a followup for the
> > extra cleanup?
>
> Yeah usually I take the first one when they are equal, but indeed Tejas
> cleanup up a little further and better aligned the parameters so I think
> Tejas's one is a better looking version.
>
> In this case we should apply Teja's one, nothing personal Abhishek!

Pushed to drm-misc-next:

[1/1] drm/panel: himax-hx83112a: transition to mipi_dsi wrapped functions
      commit: 32e5666b8a4d0f2aee39a0b2f8386cf9f86a8225
Abhishek Tamboli Sept. 12, 2024, 12:26 a.m. UTC | #8
On Tue, Sep 10, 2024 at 02:19:53PM -0700, Doug Anderson wrote:
> Hi,
> 
> On Sat, Sep 7, 2024 at 1:32 AM Tejas Vipin <tejasvipin76@gmail.com> wrote:
> >
> > On 9/7/24 3:53 AM, Jessica Zhang wrote:
> > >
> > >
> > > On 9/6/2024 3:14 PM, Jessica Zhang wrote:
> > >>
> > >>
> > >> On 9/4/2024 7:15 AM, Tejas Vipin wrote:
> > >>> Changes the himax-hx83112a panel to use multi style functions for
> > >>> improved error handling.
> > >>>
> > >>> Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> > >>
> > >> Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> > >
> > > Hi Tejas,
> > >
> > > Just a heads up, it seems that this might be a duplicate of this change [1]?
> > >
> > > Thanks,
> > >
> > > Jessica Zhang
> > >
> > > [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> >
> > Ah, thanks for letting me know. I hadn't realized someone else had
> > started working on this too.
> >
> > However, I would argue that my patch [2] is a better candidate for merging
> > because of the following reasons:
> >
> > 1) Removes unnecessary error printing:
> > The mipi_dsi_*_multi() functions all have inbuilt error printing which
> > makes printing errors after hx83112a_on unnecessary as is addressed in
> > [2] like so:
> >
> > > -     ret = hx83112a_on(ctx);
> > > +     ret = hx83112a_on(ctx->dsi);
> > >       if (ret < 0) {
> > > -             dev_err(dev, "Failed to initialize panel: %d\n", ret);
> > >               gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> > >               regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > > -             return ret;
> > >       }
> >
> > [2] also removes the unnecessary dev_err after regulator_bulk_enable as was
> > addressed in [3] like so:
> >
> > >       ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > > -     if (ret < 0) {
> > > -             dev_err(dev, "Failed to enable regulators: %d\n", ret);
> > > +     if (ret < 0)
> > >               return ret;
> > > -     }
> >
> > 2) Better formatting
> >
> > The mipi_dsi_dcs_write_seq_multi statements in [1] aren't formatted
> > quite right according to what has been done so far. They are written as
> > such in [1]:
> >
> > > +     mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> > >                              0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> >
> > Where they should be written as such in [2]:
> >
> > > +     mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> > > +                                  0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> >
> > All in all, the module generated using my patch ends up being a teensy
> > bit smaller (1% smaller). But if chronology is what is important, then
> > it would at least be nice to see the above changes as part of Abhishek's
> > patch too. And I'll be sure to look at the mail in the drm inbox now
> > onwards :p
> >
> > [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> > [2] https://lore.kernel.org/all/20240904141521.554451-1-tejasvipin76@gmail.com/
> > [3] https://lore.kernel.org/all/CAD=FV=XRZKL_ppjUKDK61fQkWhHiQCJLfmVBS7wSo4sUux2g8Q@mail.gmail.com/
> 
> I would tend to agree that Tejas's patch looks slightly better, but
> Abhishek's patch appears to have been posted first.
> 
> Neil: any idea what to do here? Maybe a Co-Developed-by or something?
> ...or we could land Abhishek and Tejas could post a followup for the
> extra cleanup?
> 
> Abhishek: are you planning to post more _multi cleanups? 
I’ve just started learning about the DRM subsystem, I don’t have specific 
plans for _multi cleanups at the moment. However, I would be happy 
to contribute further.
>If so, please
> make sure to CC Tejas (who has been posting a bunch of them) and
> perhaps me since I've been helping to review them a bit.
Yes, I’ll make sure to CC Tejas and you.

Regards,
ABhishek
Abhishek Tamboli Sept. 12, 2024, 12:32 a.m. UTC | #9
On Wed, Sep 11, 2024 at 09:41:30AM +0200, neil.armstrong@linaro.org wrote:
> On 10/09/2024 23:19, Doug Anderson wrote:
> > Hi,
> > 
> > On Sat, Sep 7, 2024 at 1:32 AM Tejas Vipin <tejasvipin76@gmail.com> wrote:
> > > 
> > > On 9/7/24 3:53 AM, Jessica Zhang wrote:
> > > > 
> > > > 
> > > > On 9/6/2024 3:14 PM, Jessica Zhang wrote:
> > > > > 
> > > > > 
> > > > > On 9/4/2024 7:15 AM, Tejas Vipin wrote:
> > > > > > Changes the himax-hx83112a panel to use multi style functions for
> > > > > > improved error handling.
> > > > > > 
> > > > > > Signed-off-by: Tejas Vipin <tejasvipin76@gmail.com>
> > > > > 
> > > > > Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> > > > 
> > > > Hi Tejas,
> > > > 
> > > > Just a heads up, it seems that this might be a duplicate of this change [1]?
> > > > 
> > > > Thanks,
> > > > 
> > > > Jessica Zhang
> > > > 
> > > > [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> > > 
> > > Ah, thanks for letting me know. I hadn't realized someone else had
> > > started working on this too.
> > > 
> > > However, I would argue that my patch [2] is a better candidate for merging
> > > because of the following reasons:
> > > 
> > > 1) Removes unnecessary error printing:
> > > The mipi_dsi_*_multi() functions all have inbuilt error printing which
> > > makes printing errors after hx83112a_on unnecessary as is addressed in
> > > [2] like so:
> > > 
> > > > -     ret = hx83112a_on(ctx);
> > > > +     ret = hx83112a_on(ctx->dsi);
> > > >        if (ret < 0) {
> > > > -             dev_err(dev, "Failed to initialize panel: %d\n", ret);
> > > >                gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> > > >                regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > > > -             return ret;
> > > >        }
> > > 
> > > [2] also removes the unnecessary dev_err after regulator_bulk_enable as was
> > > addressed in [3] like so:
> > > 
> > > >        ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
> > > > -     if (ret < 0) {
> > > > -             dev_err(dev, "Failed to enable regulators: %d\n", ret);
> > > > +     if (ret < 0)
> > > >                return ret;
> > > > -     }
> > > 
> > > 2) Better formatting
> > > 
> > > The mipi_dsi_dcs_write_seq_multi statements in [1] aren't formatted
> > > quite right according to what has been done so far. They are written as
> > > such in [1]:
> > > 
> > > > +     mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> > > >                               0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> > > 
> > > Where they should be written as such in [2]:
> > > 
> > > > +     mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
> > > > +                                  0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
> > > 
> > > All in all, the module generated using my patch ends up being a teensy
> > > bit smaller (1% smaller). But if chronology is what is important, then
> > > it would at least be nice to see the above changes as part of Abhishek's
> > > patch too. And I'll be sure to look at the mail in the drm inbox now
> > > onwards :p
> > > 
> > > [1] https://patchwork.freedesktop.org/patch/612367/?series=138155&rev=1
> > > [2] https://lore.kernel.org/all/20240904141521.554451-1-tejasvipin76@gmail.com/
> > > [3] https://lore.kernel.org/all/CAD=FV=XRZKL_ppjUKDK61fQkWhHiQCJLfmVBS7wSo4sUux2g8Q@mail.gmail.com/
> > 
> > I would tend to agree that Tejas's patch looks slightly better, but
> > Abhishek's patch appears to have been posted first.
> > 
> > Neil: any idea what to do here? Maybe a Co-Developed-by or something?
> > ...or we could land Abhishek and Tejas could post a followup for the
> > extra cleanup?
> 
> Yeah usually I take the first one when they are equal, but indeed Tejas
> cleanup up a little further and better aligned the parameters so I think
> Tejas's one is a better looking version.
> 
> In this case we should apply Teja's one, nothing personal Abhishek!
No problem at all, I completely understand. It makes sense to go with 
Tejas's version.

Thanks for letting me know, and I appreciate the feedback!

Regards,
Abhishek
diff mbox series

Patch

diff --git a/drivers/gpu/drm/panel/panel-himax-hx83112a.c b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
index 466c27012abf..47bce087e339 100644
--- a/drivers/gpu/drm/panel/panel-himax-hx83112a.c
+++ b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
@@ -56,198 +56,173 @@  static void hx83112a_reset(struct hx83112a_panel *ctx)
 	msleep(50);
 }
 
-static int hx83112a_on(struct hx83112a_panel *ctx)
+static int hx83112a_on(struct mipi_dsi_device *dsi)
 {
-	struct mipi_dsi_device *dsi = ctx->dsi;
-	struct device *dev = &dsi->dev;
-	int ret;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
 
 	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
 
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETEXTC, 0x83, 0x11, 0x2a);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER1,
-			       0x08, 0x28, 0x28, 0x83, 0x83, 0x4c, 0x4f, 0x33);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDISP,
-			       0x00, 0x02, 0x00, 0x90, 0x24, 0x00, 0x08, 0x19,
-			       0xea, 0x11, 0x11, 0x00, 0x11, 0xa3);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
-			       0x58, 0x68, 0x58, 0x68, 0x0f, 0xef, 0x0b, 0xc0,
-			       0x0b, 0xc0, 0x0b, 0xc0, 0x00, 0xff, 0x00, 0xff,
-			       0x00, 0x00, 0x14, 0x15, 0x00, 0x29, 0x11, 0x07,
-			       0x12, 0x00, 0x29);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
-			       0x00, 0x12, 0x12, 0x11, 0x88, 0x12, 0x12, 0x00,
-			       0x53);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
-			       0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
-			       0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
-			       0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
-			       0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
-			       0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
-			       0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
-			       0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
-			       0x40);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
-			       0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
-			       0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
-			       0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
-			       0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
-			       0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
-			       0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
-			       0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
-			       0x40);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
-			       0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
-			       0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
-			       0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
-			       0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
-			       0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
-			       0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
-			       0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
-			       0x40);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT, 0x01);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTCON,
-			       0x70, 0x00, 0x04, 0xe0, 0x33, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPANEL, 0x08);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER2, 0x2b, 0x2b);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
-			       0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08,
-			       0x08, 0x03, 0x03, 0x22, 0x18, 0x07, 0x07, 0x07,
-			       0x07, 0x32, 0x10, 0x06, 0x00, 0x06, 0x32, 0x10,
-			       0x07, 0x00, 0x07, 0x32, 0x19, 0x31, 0x09, 0x31,
-			       0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x08,
-			       0x09, 0x30, 0x00, 0x00, 0x00, 0x06, 0x0d, 0x00,
-			       0x0f);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
-			       0x00, 0x00, 0x19, 0x10, 0x00, 0x0a, 0x00, 0x81);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP1,
-			       0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
-			       0xc0, 0xc0, 0x18, 0x18, 0x19, 0x19, 0x18, 0x18,
-			       0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
-			       0x28, 0x28, 0x24, 0x24, 0x02, 0x03, 0x02, 0x03,
-			       0x00, 0x01, 0x00, 0x01, 0x31, 0x31, 0x31, 0x31,
-			       0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP2,
-			       0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
-			       0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x19, 0x19,
-			       0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
-			       0x24, 0x24, 0x28, 0x28, 0x01, 0x00, 0x01, 0x00,
-			       0x03, 0x02, 0x03, 0x02, 0x31, 0x31, 0x31, 0x31,
-			       0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
-			       0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea,
-			       0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa,
-			       0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa, 0xaa, 0xaa);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
-			       0xaa, 0x2e, 0x28, 0x00, 0x00, 0x00, 0xaa, 0x2e,
-			       0x28, 0x00, 0x00, 0x00, 0xaa, 0xee, 0xaa, 0xaa,
-			       0xaa, 0xaa, 0xaa, 0xee, 0xaa, 0xaa, 0xaa, 0xaa);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
-			       0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
-			       0xff, 0xff, 0xff, 0xff);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
-			       0xaa, 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
-			       0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xff, 0xff, 0xff,
-			       0xff, 0xff, 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
-			       0x0e, 0x0e, 0x1e, 0x65, 0x1c, 0x65, 0x00, 0x50,
-			       0x20, 0x20, 0x00, 0x00, 0x02, 0x02, 0x02, 0x05,
-			       0x14, 0x14, 0x32, 0xb9, 0x23, 0xb9, 0x08);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x01);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
-			       0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x02);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
-			       0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x00, 0x00,
-			       0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-			       0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
-			       0x00, 0x00, 0x00, 0x02, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0xc3);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETCLOCK, 0xd1, 0xd6);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0x3f);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0xc6);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPTBA, 0x37);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_UNKNOWN1, 0x3f);
-
-	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to exit sleep mode: %d\n", ret);
-		return ret;
-	}
-	msleep(150);
-
-	ret = mipi_dsi_dcs_set_display_on(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to set display on: %d\n", ret);
-		return ret;
-	}
-	msleep(50);
-
-	return 0;
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETEXTC, 0x83, 0x11, 0x2a);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPOWER1,
+				     0x08, 0x28, 0x28, 0x83, 0x83, 0x4c, 0x4f, 0x33);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDISP,
+				     0x00, 0x02, 0x00, 0x90, 0x24, 0x00, 0x08, 0x19,
+				     0xea, 0x11, 0x11, 0x00, 0x11, 0xa3);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDRV,
+				     0x58, 0x68, 0x58, 0x68, 0x0f, 0xef, 0x0b, 0xc0,
+				     0x0b, 0xc0, 0x0b, 0xc0, 0x00, 0xff, 0x00, 0xff,
+				     0x00, 0x00, 0x14, 0x15, 0x00, 0x29, 0x11, 0x07,
+				     0x12, 0x00, 0x29);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDRV,
+				     0x00, 0x12, 0x12, 0x11, 0x88, 0x12, 0x12, 0x00,
+				     0x53);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x03);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
+				     0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
+				     0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
+				     0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
+				     0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
+				     0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
+				     0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
+				     0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
+				     0x40);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
+				     0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
+				     0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
+				     0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
+				     0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
+				     0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
+				     0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
+				     0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
+				     0x40);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT,
+				     0xff, 0xfe, 0xfb, 0xf8, 0xf4, 0xf1, 0xed, 0xe6,
+				     0xe2, 0xde, 0xdb, 0xd6, 0xd3, 0xcf, 0xca, 0xc6,
+				     0xc2, 0xbe, 0xb9, 0xb0, 0xa7, 0x9e, 0x96, 0x8d,
+				     0x84, 0x7c, 0x74, 0x6b, 0x62, 0x5a, 0x51, 0x49,
+				     0x41, 0x39, 0x31, 0x29, 0x21, 0x19, 0x12, 0x0a,
+				     0x06, 0x05, 0x02, 0x01, 0x00, 0x00, 0xc9, 0xb3,
+				     0x08, 0x0e, 0xf2, 0xe1, 0x59, 0xf4, 0x22, 0xad,
+				     0x40);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETDGCLUT, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTCON,
+				     0x70, 0x00, 0x04, 0xe0, 0x33, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPANEL, 0x08);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPOWER2, 0x2b, 0x2b);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP0,
+				     0x80, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x08,
+				     0x08, 0x03, 0x03, 0x22, 0x18, 0x07, 0x07, 0x07,
+				     0x07, 0x32, 0x10, 0x06, 0x00, 0x06, 0x32, 0x10,
+				     0x07, 0x00, 0x07, 0x32, 0x19, 0x31, 0x09, 0x31,
+				     0x00, 0x00, 0x05, 0x00, 0x00, 0x00, 0x00, 0x08,
+				     0x09, 0x30, 0x00, 0x00, 0x00, 0x06, 0x0d, 0x00,
+				     0x0f);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP0,
+				     0x00, 0x00, 0x19, 0x10, 0x00, 0x0a, 0x00, 0x81);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP1,
+				     0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
+				     0xc0, 0xc0, 0x18, 0x18, 0x19, 0x19, 0x18, 0x18,
+				     0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
+				     0x28, 0x28, 0x24, 0x24, 0x02, 0x03, 0x02, 0x03,
+				     0x00, 0x01, 0x00, 0x01, 0x31, 0x31, 0x31, 0x31,
+				     0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP2,
+				     0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18, 0x18,
+				     0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x19, 0x19,
+				     0x40, 0x40, 0x18, 0x18, 0x18, 0x18, 0x3f, 0x3f,
+				     0x24, 0x24, 0x28, 0x28, 0x01, 0x00, 0x01, 0x00,
+				     0x03, 0x02, 0x03, 0x02, 0x31, 0x31, 0x31, 0x31,
+				     0x30, 0x30, 0x30, 0x30, 0x2f, 0x2f, 0x2f, 0x2f);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
+				     0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea,
+				     0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa,
+				     0xaa, 0xaa, 0xaa, 0xea, 0xab, 0xaa, 0xaa, 0xaa);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
+				     0xaa, 0x2e, 0x28, 0x00, 0x00, 0x00, 0xaa, 0x2e,
+				     0x28, 0x00, 0x00, 0x00, 0xaa, 0xee, 0xaa, 0xaa,
+				     0xaa, 0xaa, 0xaa, 0xee, 0xaa, 0xaa, 0xaa, 0xaa);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
+				     0xaa, 0xff, 0xff, 0xff, 0xff, 0xff, 0xaa, 0xff,
+				     0xff, 0xff, 0xff, 0xff);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x03);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETGIP3,
+				     0xaa, 0xaa, 0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
+				     0xea, 0xaa, 0xaa, 0xaa, 0xaa, 0xff, 0xff, 0xff,
+				     0xff, 0xff, 0xaa, 0xff, 0xff, 0xff, 0xff, 0xff);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
+				     0x0e, 0x0e, 0x1e, 0x65, 0x1c, 0x65, 0x00, 0x50,
+				     0x20, 0x20, 0x00, 0x00, 0x02, 0x02, 0x02, 0x05,
+				     0x14, 0x14, 0x32, 0xb9, 0x23, 0xb9, 0x08);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x01);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
+				     0x02, 0x00, 0xa8, 0x01, 0xa8, 0x0d, 0xa4, 0x0e);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x02);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETTP1,
+				     0x00, 0x00, 0x08, 0x00, 0x01, 0x00, 0x00, 0x00,
+				     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+				     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00,
+				     0x00, 0x00, 0x00, 0x02, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETBANK, 0x00);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0xc3);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETCLOCK, 0xd1, 0xd6);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0x3f);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0xc6);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_SETPTBA, 0x37);
+	mipi_dsi_dcs_write_seq_multi(&dsi_ctx, HX83112A_UNKNOWN1, 0x3f);
+
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 150);
+
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 50);
+
+	return dsi_ctx.accum_err;
 }
 
 static int hx83112a_disable(struct drm_panel *panel)
 {
 	struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
 	struct mipi_dsi_device *dsi = ctx->dsi;
-	struct device *dev = &dsi->dev;
-	int ret;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
 
 	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
 
-	ret = mipi_dsi_dcs_set_display_off(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to set display off: %d\n", ret);
-		return ret;
-	}
-	msleep(20);
-
-	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to enter sleep mode: %d\n", ret);
-		return ret;
-	}
-	msleep(120);
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 20);
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 120);
 
-	return 0;
+	return dsi_ctx.accum_err;
 }
 
 static int hx83112a_prepare(struct drm_panel *panel)
 {
 	struct hx83112a_panel *ctx = to_hx83112a_panel(panel);
-	struct device *dev = &ctx->dsi->dev;
 	int ret;
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
-	if (ret < 0) {
-		dev_err(dev, "Failed to enable regulators: %d\n", ret);
+	if (ret < 0)
 		return ret;
-	}
 
 	hx83112a_reset(ctx);
 
-	ret = hx83112a_on(ctx);
+	ret = hx83112a_on(ctx->dsi);
 	if (ret < 0) {
-		dev_err(dev, "Failed to initialize panel: %d\n", ret);
 		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
 		regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
-		return ret;
 	}
 
-	return 0;
+	return ret;
 }
 
 static int hx83112a_unprepare(struct drm_panel *panel)