diff mbox series

[v2] drm/panel: hx83112a: Transition to wrapped mipi_dsi functions

Message ID 20240903173130.41784-1-abhishektamboli9@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/panel: hx83112a: Transition to wrapped mipi_dsi functions | expand

Commit Message

Abhishek Tamboli Sept. 3, 2024, 5:31 p.m. UTC 
Transition to mipi_dsi_dcs_write_seq_multi() macros for initialization
sequences. The previous mipi_dsi_dcs_write_seq() macros were
non-intuitive and use other wrapped MIPI DSI functions in the
driver code to simplify the code pattern.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202409040049.2hf8jrZG-lkp@intel.com/
Signed-off-by: Abhishek Tamboli <abhishektamboli9@gmail.com>
---
Changes in v2:
- Update the commit message to explain the reason for the change.
- Correct the code by changing 'dsi->mode_flags' to 'dsi_ctx.dsi->mode_flags'
This change addresses a build error in v1 reported by kernel test robot
caused by using an undeclared variable 'dsi'.
[v1] : https://lore.kernel.org/all/20240902170153.34512-1-abhishektamboli9@gmail.com/

 drivers/gpu/drm/panel/panel-himax-hx83112a.c | 140 ++++++++-----------
 1 file changed, 60 insertions(+), 80 deletions(-)

--
2.34.1

Comments

Javier Martinez Canillas Sept. 4, 2024, 12:18 p.m. UTC | #1 
Abhishek Tamboli <abhishektamboli9@gmail.com> writes:

Hello Abhishek,

> Transition to mipi_dsi_dcs_write_seq_multi() macros for initialization
> sequences. The previous mipi_dsi_dcs_write_seq() macros were
> non-intuitive and use other wrapped MIPI DSI functions in the
> driver code to simplify the code pattern.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202409040049.2hf8jrZG-lkp@intel.com/
> Signed-off-by: Abhishek Tamboli <abhishektamboli9@gmail.com>
> ---
> Changes in v2:
> - Update the commit message to explain the reason for the change.

Thanks for improving the commit message. The change looks good to me.

Acked-by: Javier Martinez Canillas <javierm@redhat.com>
Jessica Zhang Sept. 6, 2024, 10:26 p.m. UTC | #2 
On 9/3/2024 10:31 AM, Abhishek Tamboli wrote:
> Transition to mipi_dsi_dcs_write_seq_multi() macros for initialization
> sequences. The previous mipi_dsi_dcs_write_seq() macros were
> non-intuitive and use other wrapped MIPI DSI functions in the
> driver code to simplify the code pattern.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202409040049.2hf8jrZG-lkp@intel.com/
> Signed-off-by: Abhishek Tamboli <abhishektamboli9@gmail.com>

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

> ---
> Changes in v2:
> - Update the commit message to explain the reason for the change.
> - Correct the code by changing 'dsi->mode_flags' to 'dsi_ctx.dsi->mode_flags'
> This change addresses a build error in v1 reported by kernel test robot
> caused by using an undeclared variable 'dsi'.
> [v1] : https://lore.kernel.org/all/20240902170153.34512-1-abhishektamboli9@gmail.com/
> 
>   drivers/gpu/drm/panel/panel-himax-hx83112a.c | 140 ++++++++-----------
>   1 file changed, 60 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-himax-hx83112a.c b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
> index 466c27012abf..6c457d17fd11 100644
> --- a/drivers/gpu/drm/panel/panel-himax-hx83112a.c
> +++ b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
> @@ -58,30 +58,28 @@ static void hx83112a_reset(struct hx83112a_panel *ctx)
> 
>   static int hx83112a_on(struct hx83112a_panel *ctx)
>   {
> -	struct mipi_dsi_device *dsi = ctx->dsi;
> -	struct device *dev = &dsi->dev;
> -	int ret;
> -
> -	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
> +	struct mipi_dsi_multi_context dsi_ctx = {.dsi = ctx->dsi};
> +
> +	dsi_ctx.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,
> +	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(dsi, HX83112A_SETDISP,
> +	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(dsi, HX83112A_SETDRV,
> +	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(dsi, HX83112A_SETBANK, 0x02);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
> +	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(dsi, HX83112A_SETBANK, 0x00);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
> +	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,
> @@ -90,8 +88,8 @@ static int hx83112a_on(struct hx83112a_panel *ctx)
>   			       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,
> +	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,
> @@ -100,8 +98,8 @@ static int hx83112a_on(struct hx83112a_panel *ctx)
>   			       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,
> +	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,
> @@ -110,13 +108,13 @@ static int hx83112a_on(struct hx83112a_panel *ctx)
>   			       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,
> +	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(dsi, HX83112A_SETPANEL, 0x08);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER2, 0x2b, 0x2b);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
> +	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,
> @@ -124,105 +122,87 @@ static int hx83112a_on(struct hx83112a_panel *ctx)
>   			       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,
> +	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(dsi, HX83112A_SETBANK, 0x00);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP1,
> +	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(dsi, HX83112A_SETGIP2,
> +	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(dsi, HX83112A_SETGIP3,
> +	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(dsi, HX83112A_SETBANK, 0x01);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
> +	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(dsi, HX83112A_SETBANK, 0x02);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
> +	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(dsi, HX83112A_SETBANK, 0x03);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
> +	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(dsi, HX83112A_SETBANK, 0x00);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
> +	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(dsi, HX83112A_SETBANK, 0x01);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
> +	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(dsi, HX83112A_SETBANK, 0x02);
> -	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
> +	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(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);
> +	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);
> 
> -	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);
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, 150);
> 
> -	return 0;
> +	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 = ctx->dsi};
> 
> -	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
> +	dsi_ctx.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);
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +	mipi_dsi_msleep(&dsi_ctx, 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_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)
> --
> 2.34.1
>
Doug Anderson Sept. 10, 2024, 9:22 p.m. UTC | #3 
Hi,

On Tue, Sep 3, 2024 at 10:32 AM Abhishek Tamboli
<abhishektamboli9@gmail.com> wrote:
>
> Transition to mipi_dsi_dcs_write_seq_multi() macros for initialization
> sequences. The previous mipi_dsi_dcs_write_seq() macros were
> non-intuitive and use other wrapped MIPI DSI functions in the
> driver code to simplify the code pattern.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202409040049.2hf8jrZG-lkp@intel.com/

You'd only include the above two tags if the original problematic
commit had actually landed. Since it didn't you leave them off and the
Robot gets no credit (even though it is awesome).


> Signed-off-by: Abhishek Tamboli <abhishektamboli9@gmail.com>
> ---
> Changes in v2:
> - Update the commit message to explain the reason for the change.
> - Correct the code by changing 'dsi->mode_flags' to 'dsi_ctx.dsi->mode_flags'
> This change addresses a build error in v1 reported by kernel test robot
> caused by using an undeclared variable 'dsi'.
> [v1] : https://lore.kernel.org/all/20240902170153.34512-1-abhishektamboli9@gmail.com/
>
>  drivers/gpu/drm/panel/panel-himax-hx83112a.c | 140 ++++++++-----------
>  1 file changed, 60 insertions(+), 80 deletions(-)

Just adding a note that there's nearly the same patch in
https://lore.kernel.org/r/20240904141521.554451-1-tejasvipin76@gmail.com
and we're discussing what to do about it there.

-Doug
Abhishek Tamboli Sept. 12, 2024, 12:12 a.m. UTC | #4 
Hi Doug,
Thanks for the feedback.

On Tue, Sep 10, 2024 at 02:22:37PM -0700, Doug Anderson wrote:
> Hi,
> 
> On Tue, Sep 3, 2024 at 10:32 AM Abhishek Tamboli
> <abhishektamboli9@gmail.com> wrote:
> >
> > Transition to mipi_dsi_dcs_write_seq_multi() macros for initialization
> > sequences. The previous mipi_dsi_dcs_write_seq() macros were
> > non-intuitive and use other wrapped MIPI DSI functions in the
> > driver code to simplify the code pattern.
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202409040049.2hf8jrZG-lkp@intel.com/
> 
> You'd only include the above two tags if the original problematic
> commit had actually landed. Since it didn't you leave them off and the
> Robot gets no credit (even though it is awesome).
Sure, I will keep this in mind before sending the next patch.
> 
> > Signed-off-by: Abhishek Tamboli <abhishektamboli9@gmail.com>
> > ---
> > Changes in v2:
> > - Update the commit message to explain the reason for the change.
> > - Correct the code by changing 'dsi->mode_flags' to 'dsi_ctx.dsi->mode_flags'
> > This change addresses a build error in v1 reported by kernel test robot
> > caused by using an undeclared variable 'dsi'.
> > [v1] : https://lore.kernel.org/all/20240902170153.34512-1-abhishektamboli9@gmail.com/
> >
> >  drivers/gpu/drm/panel/panel-himax-hx83112a.c | 140 ++++++++-----------
> >  1 file changed, 60 insertions(+), 80 deletions(-)
> 
> Just adding a note that there's nearly the same patch in
> https://lore.kernel.org/r/20240904141521.554451-1-tejasvipin76@gmail.com
> and we're discussing what to do about it there.
> 
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..6c457d17fd11 100644
--- a/drivers/gpu/drm/panel/panel-himax-hx83112a.c
+++ b/drivers/gpu/drm/panel/panel-himax-hx83112a.c
@@ -58,30 +58,28 @@  static void hx83112a_reset(struct hx83112a_panel *ctx)

 static int hx83112a_on(struct hx83112a_panel *ctx)
 {
-	struct mipi_dsi_device *dsi = ctx->dsi;
-	struct device *dev = &dsi->dev;
-	int ret;
-
-	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+	struct mipi_dsi_multi_context dsi_ctx = {.dsi = ctx->dsi};
+
+	dsi_ctx.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,
+	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(dsi, HX83112A_SETDISP,
+	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(dsi, HX83112A_SETDRV,
+	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(dsi, HX83112A_SETBANK, 0x02);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDRV,
+	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(dsi, HX83112A_SETBANK, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETBANK, 0x03);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETDGCLUT,
+	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,
@@ -90,8 +88,8 @@  static int hx83112a_on(struct hx83112a_panel *ctx)
 			       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,
+	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,
@@ -100,8 +98,8 @@  static int hx83112a_on(struct hx83112a_panel *ctx)
 			       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,
+	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,
@@ -110,13 +108,13 @@  static int hx83112a_on(struct hx83112a_panel *ctx)
 			       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,
+	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(dsi, HX83112A_SETPANEL, 0x08);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETPOWER2, 0x2b, 0x2b);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP0,
+	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,
@@ -124,105 +122,87 @@  static int hx83112a_on(struct hx83112a_panel *ctx)
 			       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,
+	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(dsi, HX83112A_SETBANK, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP1,
+	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(dsi, HX83112A_SETGIP2,
+	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(dsi, HX83112A_SETGIP3,
+	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(dsi, HX83112A_SETBANK, 0x01);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
+	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(dsi, HX83112A_SETBANK, 0x02);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
+	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(dsi, HX83112A_SETBANK, 0x03);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETGIP3,
+	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(dsi, HX83112A_SETBANK, 0x00);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
+	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(dsi, HX83112A_SETBANK, 0x01);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
+	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(dsi, HX83112A_SETBANK, 0x02);
-	mipi_dsi_dcs_write_seq(dsi, HX83112A_SETTP1,
+	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(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);
+	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);

-	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);
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 150);

-	return 0;
+	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 = ctx->dsi};

-	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+	dsi_ctx.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);
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+	mipi_dsi_msleep(&dsi_ctx, 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_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)