[PATCH v4 2/4] drm/panel: jdi-lpm102a188a: Fix bug and clean up driver

Brigham Campbell posted 4 patches 2 months, 3 weeks ago
There is a newer version of this series
[PATCH v4 2/4] drm/panel: jdi-lpm102a188a: Fix bug and clean up driver
Posted by Brigham Campbell 2 months, 3 weeks ago
Fix bug in unprepare() which causes the function's return value to be
that of the last mipi "enter sleep mode" command.

Update driver to use the "multi" variant of MIPI functions in order to
facilitate improved error handling and remove the panel's dependency on
deprecated MIPI functions.

Use the new mipi_dsi_dual macro to reduce code duplication.

Signed-off-by: Brigham Campbell <me@brighamcampbell.com>
---
 drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c | 197 ++++++------------
 1 file changed, 60 insertions(+), 137 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c b/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c
index 5b5082efb282..9df67facdc47 100644
--- a/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c
+++ b/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c
@@ -81,25 +81,25 @@ static int jdi_panel_disable(struct drm_panel *panel)
 static int jdi_panel_unprepare(struct drm_panel *panel)
 {
 	struct jdi_panel *jdi = to_panel_jdi(panel);
-	int ret;
 
-	ret = mipi_dsi_dcs_set_display_off(jdi->link1);
-	if (ret < 0)
-		dev_err(panel->dev, "failed to set display off: %d\n", ret);
+	/*
+	 * One context per panel since we'll continue trying to shut down the
+	 * other panel even if one isn't responding.
+	 */
+	struct mipi_dsi_multi_context dsi_ctx1 = { .dsi = jdi->link1 };
+	struct mipi_dsi_multi_context dsi_ctx2 = { .dsi = jdi->link2 };
 
-	ret = mipi_dsi_dcs_set_display_off(jdi->link2);
-	if (ret < 0)
-		dev_err(panel->dev, "failed to set display off: %d\n", ret);
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx1);
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx2);
 
 	/* Specified by JDI @ 50ms, subject to change */
 	msleep(50);
 
-	ret = mipi_dsi_dcs_enter_sleep_mode(jdi->link1);
-	if (ret < 0)
-		dev_err(panel->dev, "failed to enter sleep mode: %d\n", ret);
-	ret = mipi_dsi_dcs_enter_sleep_mode(jdi->link2);
-	if (ret < 0)
-		dev_err(panel->dev, "failed to enter sleep mode: %d\n", ret);
+	/* Doesn't hurt to try sleep mode even if display off fails */
+	dsi_ctx1.accum_err = 0;
+	dsi_ctx2.accum_err = 0;
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx1);
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx2);
 
 	/* Specified by JDI @ 150ms, subject to change */
 	msleep(150);
@@ -123,72 +123,47 @@ static int jdi_panel_unprepare(struct drm_panel *panel)
 	/* Specified by JDI @ 20ms, subject to change */
 	msleep(20);
 
-	return ret;
-}
-
-static int jdi_setup_symmetrical_split(struct mipi_dsi_device *left,
-				       struct mipi_dsi_device *right,
-				       const struct drm_display_mode *mode)
-{
-	int err;
-
-	err = mipi_dsi_dcs_set_column_address(left, 0, mode->hdisplay / 2 - 1);
-	if (err < 0) {
-		dev_err(&left->dev, "failed to set column address: %d\n", err);
-		return err;
-	}
-
-	err = mipi_dsi_dcs_set_column_address(right, 0, mode->hdisplay / 2 - 1);
-	if (err < 0) {
-		dev_err(&right->dev, "failed to set column address: %d\n", err);
-		return err;
-	}
-
-	err = mipi_dsi_dcs_set_page_address(left, 0, mode->vdisplay - 1);
-	if (err < 0) {
-		dev_err(&left->dev, "failed to set page address: %d\n", err);
-		return err;
-	}
-
-	err = mipi_dsi_dcs_set_page_address(right, 0, mode->vdisplay - 1);
-	if (err < 0) {
-		dev_err(&right->dev, "failed to set page address: %d\n", err);
-		return err;
-	}
-
 	return 0;
 }
 
-static int jdi_write_dcdc_registers(struct jdi_panel *jdi)
+static void jdi_setup_symmetrical_split(struct mipi_dsi_multi_context *dsi_ctx,
+					struct mipi_dsi_device *left,
+					struct mipi_dsi_device *right,
+					const struct drm_display_mode *mode)
+{
+	mipi_dsi_dual(mipi_dsi_dcs_set_column_address_multi,
+		      left, right, dsi_ctx,
+		      0, mode->hdisplay / 2 - 1);
+	mipi_dsi_dual(mipi_dsi_dcs_set_page_address_multi,
+		      left, right, dsi_ctx,
+		      0, mode->vdisplay - 1);
+}
+
+static void jdi_write_dcdc_registers(struct mipi_dsi_multi_context *dsi_ctx,
+				     struct jdi_panel *jdi)
 {
 	/* Clear the manufacturer command access protection */
-	mipi_dsi_generic_write_seq(jdi->link1, MCS_CMD_ACS_PROT,
-				   MCS_CMD_ACS_PROT_OFF);
-	mipi_dsi_generic_write_seq(jdi->link2, MCS_CMD_ACS_PROT,
-				   MCS_CMD_ACS_PROT_OFF);
+	mipi_dsi_dual(mipi_dsi_generic_write_seq_multi,
+		      jdi->link1, jdi->link2, dsi_ctx,
+		      MCS_CMD_ACS_PROT, MCS_CMD_ACS_PROT_OFF);
 	/*
-	 * Change the VGH/VGL divide rations to move the noise generated by the
+	 * Change the VGH/VGL divide ratios to move the noise generated by the
 	 * TCONN. This should hopefully avoid interaction with the backlight
 	 * controller.
 	 */
-	mipi_dsi_generic_write_seq(jdi->link1, MCS_PWR_CTRL_FUNC,
-				   MCS_PWR_CTRL_PARAM1_VGH_330_DIV |
-				   MCS_PWR_CTRL_PARAM1_DEFAULT,
-				   MCS_PWR_CTRL_PARAM2_VGL_410_DIV |
-				   MCS_PWR_CTRL_PARAM2_DEFAULT);
-
-	mipi_dsi_generic_write_seq(jdi->link2, MCS_PWR_CTRL_FUNC,
-				   MCS_PWR_CTRL_PARAM1_VGH_330_DIV |
-				   MCS_PWR_CTRL_PARAM1_DEFAULT,
-				   MCS_PWR_CTRL_PARAM2_VGL_410_DIV |
-				   MCS_PWR_CTRL_PARAM2_DEFAULT);
-
-	return 0;
+	mipi_dsi_dual(mipi_dsi_generic_write_seq_multi,
+		      jdi->link1, jdi->link2, dsi_ctx,
+		      MCS_PWR_CTRL_FUNC,
+		      MCS_PWR_CTRL_PARAM1_VGH_330_DIV |
+		      MCS_PWR_CTRL_PARAM1_DEFAULT,
+		      MCS_PWR_CTRL_PARAM2_VGL_410_DIV |
+		      MCS_PWR_CTRL_PARAM2_DEFAULT);
 }
 
 static int jdi_panel_prepare(struct drm_panel *panel)
 {
 	struct jdi_panel *jdi = to_panel_jdi(panel);
+	struct mipi_dsi_multi_context dsi_ctx = { .accum_err = 0 };
 	int err;
 
 	/* Disable backlight to avoid showing random pixels
@@ -231,88 +206,36 @@ static int jdi_panel_prepare(struct drm_panel *panel)
 	 * put in place to communicate the configuration back to the DSI host
 	 * controller.
 	 */
-	err = jdi_setup_symmetrical_split(jdi->link1, jdi->link2,
-					  jdi->mode);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to set up symmetrical split: %d\n",
-			err);
-		goto poweroff;
-	}
+	jdi_setup_symmetrical_split(&dsi_ctx, jdi->link1, jdi->link2,
+				    jdi->mode);
 
-	err = mipi_dsi_dcs_set_tear_scanline(jdi->link1,
-					     jdi->mode->vdisplay - 16);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to set tear scanline: %d\n", err);
-		goto poweroff;
-	}
+	mipi_dsi_dual(mipi_dsi_dcs_set_tear_scanline_multi,
+		      jdi->link1, jdi->link2, &dsi_ctx,
+		      jdi->mode->vdisplay - 16);
 
-	err = mipi_dsi_dcs_set_tear_scanline(jdi->link2,
-					     jdi->mode->vdisplay - 16);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to set tear scanline: %d\n", err);
-		goto poweroff;
-	}
+	mipi_dsi_dual(mipi_dsi_dcs_set_tear_on_multi,
+		      jdi->link1, jdi->link2, &dsi_ctx,
+		      MIPI_DSI_DCS_TEAR_MODE_VBLANK);
 
-	err = mipi_dsi_dcs_set_tear_on(jdi->link1,
-				       MIPI_DSI_DCS_TEAR_MODE_VBLANK);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to set tear on: %d\n", err);
-		goto poweroff;
-	}
+	mipi_dsi_dual(mipi_dsi_dcs_set_pixel_format_multi,
+		      jdi->link1, jdi->link2, &dsi_ctx,
+		      MIPI_DCS_PIXEL_FMT_24BIT);
 
-	err = mipi_dsi_dcs_set_tear_on(jdi->link2,
-				       MIPI_DSI_DCS_TEAR_MODE_VBLANK);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to set tear on: %d\n", err);
-		goto poweroff;
-	}
+	mipi_dsi_dual(mipi_dsi_dcs_exit_sleep_mode_multi,
+		      jdi->link1, jdi->link2, &dsi_ctx);
 
-	err = mipi_dsi_dcs_set_pixel_format(jdi->link1, MIPI_DCS_PIXEL_FMT_24BIT);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to set pixel format: %d\n", err);
-		goto poweroff;
-	}
-
-	err = mipi_dsi_dcs_set_pixel_format(jdi->link2, MIPI_DCS_PIXEL_FMT_24BIT);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to set pixel format: %d\n", err);
-		goto poweroff;
-	}
-
-	err = mipi_dsi_dcs_exit_sleep_mode(jdi->link1);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to exit sleep mode: %d\n", err);
-		goto poweroff;
-	}
-
-	err = mipi_dsi_dcs_exit_sleep_mode(jdi->link2);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to exit sleep mode: %d\n", err);
-		goto poweroff;
-	}
-
-	err = jdi_write_dcdc_registers(jdi);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to write dcdc registers: %d\n", err);
-		goto poweroff;
-	}
+	jdi_write_dcdc_registers(&dsi_ctx, jdi);
 	/*
-	 * We need to wait 150ms between mipi_dsi_dcs_exit_sleep_mode() and
-	 * mipi_dsi_dcs_set_display_on().
+	 * We need to wait 150ms between mipi_dsi_dcs_exit_sleep_mode_multi()
+	 * and mipi_dsi_dcs_set_display_on_multi().
 	 */
-	msleep(150);
+	mipi_dsi_msleep(&dsi_ctx, 150);
 
-	err = mipi_dsi_dcs_set_display_on(jdi->link1);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to set display on: %d\n", err);
-		goto poweroff;
-	}
+	mipi_dsi_dual(mipi_dsi_dcs_set_display_on_multi,
+		      jdi->link1, jdi->link2, &dsi_ctx);
 
-	err = mipi_dsi_dcs_set_display_on(jdi->link2);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to set display on: %d\n", err);
+	if (dsi_ctx.accum_err < 0)
 		goto poweroff;
-	}
 
 	jdi->link1->mode_flags &= ~MIPI_DSI_MODE_LPM;
 	jdi->link2->mode_flags &= ~MIPI_DSI_MODE_LPM;
-- 
2.50.1
Re: [PATCH v4 2/4] drm/panel: jdi-lpm102a188a: Fix bug and clean up driver
Posted by Doug Anderson 2 months, 2 weeks ago
Hi,

On Thu, Jul 17, 2025 at 9:41 AM Brigham Campbell <me@brighamcampbell.com> wrote:
>
>  static int jdi_panel_prepare(struct drm_panel *panel)
>  {
>         struct jdi_panel *jdi = to_panel_jdi(panel);
> +       struct mipi_dsi_multi_context dsi_ctx = { .accum_err = 0 };

nit: can just be this:

struct mipi_dsi_multi_context dsi_ctx = {};

This looks so nice and clean now! :-) I'd bet that the compiled size
of the code is also quite a bit smaller as well...

Reviewed-by: Douglas Anderson <dianders@chromium.org>
Re: [PATCH v4 2/4] drm/panel: jdi-lpm102a188a: Fix bug and clean up driver
Posted by Diogo Ivo 2 months, 2 weeks ago

On 7/18/25 5:11 PM, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jul 17, 2025 at 9:41 AM Brigham Campbell <me@brighamcampbell.com> wrote:
>>
>>   static int jdi_panel_prepare(struct drm_panel *panel)
>>   {
>>          struct jdi_panel *jdi = to_panel_jdi(panel);
>> +       struct mipi_dsi_multi_context dsi_ctx = { .accum_err = 0 };
> 
> nit: can just be this:
> 
> struct mipi_dsi_multi_context dsi_ctx = {};

I am not an expert here but I was under the impression that this is only
valid with C23 while the kernel is written in C11. Is there something I
am missing?

Diogo
Re: [PATCH v4 2/4] drm/panel: jdi-lpm102a188a: Fix bug and clean up driver
Posted by Brigham Campbell 2 months, 2 weeks ago
On Sat Jul 19, 2025 at 11:10 AM MDT, Diogo Ivo wrote:
>> nit: can just be this:
>> 
>> struct mipi_dsi_multi_context dsi_ctx = {};
>
> I am not an expert here but I was under the impression that this is only
> valid with C23 while the kernel is written in C11. Is there something I
> am missing?
>
> Diogo

You're right, C23 was the first standard to bless the usage of the empty
initializer, ` = {};`, but if I'm right, it's been a GNU extension long
before C11. At risk of being pedantic, I'll draw attention to line 580
of the kernel's root Makefile:

KBUILD_CFLAGS += -std=gnu11

The kernel is technically written in the GNU variant of C11, extensions
and all. In fact, the first patch of this series uses optional variadic
macro arguments, which aren't a part of any official C standard as far
as I'm aware.

In any case, a simple grep for some forms of the empty initializer shows
usages all over the drm subsystem.

That said, I don't know if GNU extensions are formally documented or
where one would look for that information. Importantly, I am by far the
junior as far as kernel coding is concerned. I yield to your experience
and I'm happy to change this initialization in v6 if that's best.

Cheers,
Brigham
Re: [PATCH v4 2/4] drm/panel: jdi-lpm102a188a: Fix bug and clean up driver
Posted by Diogo Ivo 2 months, 2 weeks ago

On 7/20/25 8:50 AM, Brigham Campbell wrote:
> On Sat Jul 19, 2025 at 11:10 AM MDT, Diogo Ivo wrote:
>>> nit: can just be this:
>>>
>>> struct mipi_dsi_multi_context dsi_ctx = {};
>>
>> I am not an expert here but I was under the impression that this is only
>> valid with C23 while the kernel is written in C11. Is there something I
>> am missing?
>>
>> Diogo
> 
> You're right, C23 was the first standard to bless the usage of the empty
> initializer, ` = {};`, but if I'm right, it's been a GNU extension long
> before C11. At risk of being pedantic, I'll draw attention to line 580
> of the kernel's root Makefile:
> 
> KBUILD_CFLAGS += -std=gnu11
> 
> The kernel is technically written in the GNU variant of C11, extensions
> and all. In fact, the first patch of this series uses optional variadic
> macro arguments, which aren't a part of any official C standard as far
> as I'm aware.
> 
> In any case, a simple grep for some forms of the empty initializer shows
> usages all over the drm subsystem.
> 
> That said, I don't know if GNU extensions are formally documented or
> where one would look for that information. Importantly, I am by far the
> junior as far as kernel coding is concerned. I yield to your experience
> and I'm happy to change this initialization in v6 if that's best.

I found the documentation here [1], and it does state regarding designated
initializers that "Omitted fields are implicitly initialized the same as for
objects that have static storage duration." so I take it that no v6 is 
needed :)

Diogo

[1]: 
https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html#Designated-Inits
Re: [PATCH v4 2/4] drm/panel: jdi-lpm102a188a: Fix bug and clean up driver
Posted by Doug Anderson 2 months, 2 weeks ago
Hi,

On Sun, Jul 20, 2025 at 4:19 AM Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt> wrote:
>
> On 7/20/25 8:50 AM, Brigham Campbell wrote:
> > On Sat Jul 19, 2025 at 11:10 AM MDT, Diogo Ivo wrote:
> >>> nit: can just be this:
> >>>
> >>> struct mipi_dsi_multi_context dsi_ctx = {};
> >>
> >> I am not an expert here but I was under the impression that this is only
> >> valid with C23 while the kernel is written in C11. Is there something I
> >> am missing?
> >>
> >> Diogo
> >
> > You're right, C23 was the first standard to bless the usage of the empty
> > initializer, ` = {};`, but if I'm right, it's been a GNU extension long
> > before C11. At risk of being pedantic, I'll draw attention to line 580
> > of the kernel's root Makefile:
> >
> > KBUILD_CFLAGS += -std=gnu11
> >
> > The kernel is technically written in the GNU variant of C11, extensions
> > and all. In fact, the first patch of this series uses optional variadic
> > macro arguments, which aren't a part of any official C standard as far
> > as I'm aware.
> >
> > In any case, a simple grep for some forms of the empty initializer shows
> > usages all over the drm subsystem.
> >
> > That said, I don't know if GNU extensions are formally documented or
> > where one would look for that information. Importantly, I am by far the
> > junior as far as kernel coding is concerned. I yield to your experience
> > and I'm happy to change this initialization in v6 if that's best.
>
> I found the documentation here [1], and it does state regarding designated
> initializers that "Omitted fields are implicitly initialized the same as for
> objects that have static storage duration." so I take it that no v6 is
> needed :)
>
> Diogo
>
> [1]:
> https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html#Designated-Inits

Right. I think the key here is (as Brigham said) `git grep '= {}'`
shows countless hits in the kernel. :-) ...so we're not introducing
any new compiler requirement here with your patch. ;-)

-Doug
Re: [PATCH v4 2/4] drm/panel: jdi-lpm102a188a: Fix bug and clean up driver
Posted by Diogo Ivo 2 months, 2 weeks ago

On 7/17/25 5:40 PM, Brigham Campbell wrote:
> Fix bug in unprepare() which causes the function's return value to be
> that of the last mipi "enter sleep mode" command.
> 
> Update driver to use the "multi" variant of MIPI functions in order to
> facilitate improved error handling and remove the panel's dependency on
> deprecated MIPI functions.
> 
> Use the new mipi_dsi_dual macro to reduce code duplication.
> 
> Signed-off-by: Brigham Campbell <me@brighamcampbell.com>

Reviewed-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>
Tested-by: Diogo Ivo <diogo.ivo@tecnico.ulisboa.pt>

Thanks for the patch! Just the smallest of nits in the review but it's
fine by me if this goes in as is.

> ---
>   drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c | 197 ++++++------------
>   1 file changed, 60 insertions(+), 137 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c b/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c
> index 5b5082efb282..9df67facdc47 100644
> --- a/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c
> +++ b/drivers/gpu/drm/panel/panel-jdi-lpm102a188a.c
> @@ -81,25 +81,25 @@ static int jdi_panel_disable(struct drm_panel *panel)
>   static int jdi_panel_unprepare(struct drm_panel *panel)
>   {
>   	struct jdi_panel *jdi = to_panel_jdi(panel);
> -	int ret;
>   
> -	ret = mipi_dsi_dcs_set_display_off(jdi->link1);
> -	if (ret < 0)
> -		dev_err(panel->dev, "failed to set display off: %d\n", ret);
> +	/*
> +	 * One context per panel since we'll continue trying to shut down the
> +	 * other panel even if one isn't responding.
> +	 */
> +	struct mipi_dsi_multi_context dsi_ctx1 = { .dsi = jdi->link1 };
> +	struct mipi_dsi_multi_context dsi_ctx2 = { .dsi = jdi->link2 };
>   
> -	ret = mipi_dsi_dcs_set_display_off(jdi->link2);
> -	if (ret < 0)
> -		dev_err(panel->dev, "failed to set display off: %d\n", ret);
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx1);
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx2);
>   
>   	/* Specified by JDI @ 50ms, subject to change */
>   	msleep(50);
>   
> -	ret = mipi_dsi_dcs_enter_sleep_mode(jdi->link1);
> -	if (ret < 0)
> -		dev_err(panel->dev, "failed to enter sleep mode: %d\n", ret);
> -	ret = mipi_dsi_dcs_enter_sleep_mode(jdi->link2);
> -	if (ret < 0)
> -		dev_err(panel->dev, "failed to enter sleep mode: %d\n", ret);
> +	/* Doesn't hurt to try sleep mode even if display off fails */
> +	dsi_ctx1.accum_err = 0;
> +	dsi_ctx2.accum_err = 0;
> +	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx1);
> +	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx2);
>   
>   	/* Specified by JDI @ 150ms, subject to change */
>   	msleep(150);
> @@ -123,72 +123,47 @@ static int jdi_panel_unprepare(struct drm_panel *panel)
>   	/* Specified by JDI @ 20ms, subject to change */
>   	msleep(20);
>   
> -	return ret;
> -}
> -
> -static int jdi_setup_symmetrical_split(struct mipi_dsi_device *left,
> -				       struct mipi_dsi_device *right,
> -				       const struct drm_display_mode *mode)
> -{
> -	int err;
> -
> -	err = mipi_dsi_dcs_set_column_address(left, 0, mode->hdisplay / 2 - 1);
> -	if (err < 0) {
> -		dev_err(&left->dev, "failed to set column address: %d\n", err);
> -		return err;
> -	}
> -
> -	err = mipi_dsi_dcs_set_column_address(right, 0, mode->hdisplay / 2 - 1);
> -	if (err < 0) {
> -		dev_err(&right->dev, "failed to set column address: %d\n", err);
> -		return err;
> -	}
> -
> -	err = mipi_dsi_dcs_set_page_address(left, 0, mode->vdisplay - 1);
> -	if (err < 0) {
> -		dev_err(&left->dev, "failed to set page address: %d\n", err);
> -		return err;
> -	}
> -
> -	err = mipi_dsi_dcs_set_page_address(right, 0, mode->vdisplay - 1);
> -	if (err < 0) {
> -		dev_err(&right->dev, "failed to set page address: %d\n", err);
> -		return err;
> -	}
> -
>   	return 0;
>   }
>   
> -static int jdi_write_dcdc_registers(struct jdi_panel *jdi)
> +static void jdi_setup_symmetrical_split(struct mipi_dsi_multi_context *dsi_ctx,
> +					struct mipi_dsi_device *left,
> +					struct mipi_dsi_device *right,
> +					const struct drm_display_mode *mode)
> +{
> +	mipi_dsi_dual(mipi_dsi_dcs_set_column_address_multi,
> +		      left, right, dsi_ctx,
> +		      0, mode->hdisplay / 2 - 1);
> +	mipi_dsi_dual(mipi_dsi_dcs_set_page_address_multi,
> +		      left, right, dsi_ctx,
> +		      0, mode->vdisplay - 1);
> +}
> +
> +static void jdi_write_dcdc_registers(struct mipi_dsi_multi_context *dsi_ctx,
> +				     struct jdi_panel *jdi)
>   {
>   	/* Clear the manufacturer command access protection */
> -	mipi_dsi_generic_write_seq(jdi->link1, MCS_CMD_ACS_PROT,
> -				   MCS_CMD_ACS_PROT_OFF);
> -	mipi_dsi_generic_write_seq(jdi->link2, MCS_CMD_ACS_PROT,
> -				   MCS_CMD_ACS_PROT_OFF);
> +	mipi_dsi_dual(mipi_dsi_generic_write_seq_multi,
> +		      jdi->link1, jdi->link2, dsi_ctx,
> +		      MCS_CMD_ACS_PROT, MCS_CMD_ACS_PROT_OFF);
>   	/*
> -	 * Change the VGH/VGL divide rations to move the noise generated by the
> +	 * Change the VGH/VGL divide ratios to move the noise generated by the
>   	 * TCONN. This should hopefully avoid interaction with the backlight
>   	 * controller.
>   	 */
> -	mipi_dsi_generic_write_seq(jdi->link1, MCS_PWR_CTRL_FUNC,
> -				   MCS_PWR_CTRL_PARAM1_VGH_330_DIV |
> -				   MCS_PWR_CTRL_PARAM1_DEFAULT,
> -				   MCS_PWR_CTRL_PARAM2_VGL_410_DIV |
> -				   MCS_PWR_CTRL_PARAM2_DEFAULT);
> -
> -	mipi_dsi_generic_write_seq(jdi->link2, MCS_PWR_CTRL_FUNC,
> -				   MCS_PWR_CTRL_PARAM1_VGH_330_DIV |
> -				   MCS_PWR_CTRL_PARAM1_DEFAULT,
> -				   MCS_PWR_CTRL_PARAM2_VGL_410_DIV |
> -				   MCS_PWR_CTRL_PARAM2_DEFAULT);
> -
> -	return 0;
> +	mipi_dsi_dual(mipi_dsi_generic_write_seq_multi,
> +		      jdi->link1, jdi->link2, dsi_ctx,
> +		      MCS_PWR_CTRL_FUNC,
> +		      MCS_PWR_CTRL_PARAM1_VGH_330_DIV |
> +		      MCS_PWR_CTRL_PARAM1_DEFAULT,
> +		      MCS_PWR_CTRL_PARAM2_VGL_410_DIV |
> +		      MCS_PWR_CTRL_PARAM2_DEFAULT);
>   }
>   
>   static int jdi_panel_prepare(struct drm_panel *panel)
>   {
>   	struct jdi_panel *jdi = to_panel_jdi(panel);
> +	struct mipi_dsi_multi_context dsi_ctx = { .accum_err = 0 };

Technically this should be in reverse christmas tree order but it is
just a tiny nitpick.

>   	int err;
>   
>   	/* Disable backlight to avoid showing random pixels
> @@ -231,88 +206,36 @@ static int jdi_panel_prepare(struct drm_panel *panel)
>   	 * put in place to communicate the configuration back to the DSI host
>   	 * controller.
>   	 */
> -	err = jdi_setup_symmetrical_split(jdi->link1, jdi->link2,
> -					  jdi->mode);
> -	if (err < 0) {
> -		dev_err(panel->dev, "failed to set up symmetrical split: %d\n",
> -			err);
> -		goto poweroff;
> -	}
> +	jdi_setup_symmetrical_split(&dsi_ctx, jdi->link1, jdi->link2,
> +				    jdi->mode);
>   
> -	err = mipi_dsi_dcs_set_tear_scanline(jdi->link1,
> -					     jdi->mode->vdisplay - 16);
> -	if (err < 0) {
> -		dev_err(panel->dev, "failed to set tear scanline: %d\n", err);
> -		goto poweroff;
> -	}
> +	mipi_dsi_dual(mipi_dsi_dcs_set_tear_scanline_multi,
> +		      jdi->link1, jdi->link2, &dsi_ctx,
> +		      jdi->mode->vdisplay - 16);
>   
> -	err = mipi_dsi_dcs_set_tear_scanline(jdi->link2,
> -					     jdi->mode->vdisplay - 16);
> -	if (err < 0) {
> -		dev_err(panel->dev, "failed to set tear scanline: %d\n", err);
> -		goto poweroff;
> -	}
> +	mipi_dsi_dual(mipi_dsi_dcs_set_tear_on_multi,
> +		      jdi->link1, jdi->link2, &dsi_ctx,
> +		      MIPI_DSI_DCS_TEAR_MODE_VBLANK);
>   
> -	err = mipi_dsi_dcs_set_tear_on(jdi->link1,
> -				       MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> -	if (err < 0) {
> -		dev_err(panel->dev, "failed to set tear on: %d\n", err);
> -		goto poweroff;
> -	}
> +	mipi_dsi_dual(mipi_dsi_dcs_set_pixel_format_multi,
> +		      jdi->link1, jdi->link2, &dsi_ctx,
> +		      MIPI_DCS_PIXEL_FMT_24BIT);
>   
> -	err = mipi_dsi_dcs_set_tear_on(jdi->link2,
> -				       MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> -	if (err < 0) {
> -		dev_err(panel->dev, "failed to set tear on: %d\n", err);
> -		goto poweroff;
> -	}
> +	mipi_dsi_dual(mipi_dsi_dcs_exit_sleep_mode_multi,
> +		      jdi->link1, jdi->link2, &dsi_ctx);
>   
> -	err = mipi_dsi_dcs_set_pixel_format(jdi->link1, MIPI_DCS_PIXEL_FMT_24BIT);
> -	if (err < 0) {
> -		dev_err(panel->dev, "failed to set pixel format: %d\n", err);
> -		goto poweroff;
> -	}
> -
> -	err = mipi_dsi_dcs_set_pixel_format(jdi->link2, MIPI_DCS_PIXEL_FMT_24BIT);
> -	if (err < 0) {
> -		dev_err(panel->dev, "failed to set pixel format: %d\n", err);
> -		goto poweroff;
> -	}
> -
> -	err = mipi_dsi_dcs_exit_sleep_mode(jdi->link1);
> -	if (err < 0) {
> -		dev_err(panel->dev, "failed to exit sleep mode: %d\n", err);
> -		goto poweroff;
> -	}
> -
> -	err = mipi_dsi_dcs_exit_sleep_mode(jdi->link2);
> -	if (err < 0) {
> -		dev_err(panel->dev, "failed to exit sleep mode: %d\n", err);
> -		goto poweroff;
> -	}
> -
> -	err = jdi_write_dcdc_registers(jdi);
> -	if (err < 0) {
> -		dev_err(panel->dev, "failed to write dcdc registers: %d\n", err);
> -		goto poweroff;
> -	}
> +	jdi_write_dcdc_registers(&dsi_ctx, jdi);
>   	/*
> -	 * We need to wait 150ms between mipi_dsi_dcs_exit_sleep_mode() and
> -	 * mipi_dsi_dcs_set_display_on().
> +	 * We need to wait 150ms between mipi_dsi_dcs_exit_sleep_mode_multi()
> +	 * and mipi_dsi_dcs_set_display_on_multi().
>   	 */
> -	msleep(150);
> +	mipi_dsi_msleep(&dsi_ctx, 150);
>   
> -	err = mipi_dsi_dcs_set_display_on(jdi->link1);
> -	if (err < 0) {
> -		dev_err(panel->dev, "failed to set display on: %d\n", err);
> -		goto poweroff;
> -	}
> +	mipi_dsi_dual(mipi_dsi_dcs_set_display_on_multi,
> +		      jdi->link1, jdi->link2, &dsi_ctx);
>   
> -	err = mipi_dsi_dcs_set_display_on(jdi->link2);
> -	if (err < 0) {
> -		dev_err(panel->dev, "failed to set display on: %d\n", err);
> +	if (dsi_ctx.accum_err < 0)
>   		goto poweroff;
> -	}
>   
>   	jdi->link1->mode_flags &= ~MIPI_DSI_MODE_LPM;
>   	jdi->link2->mode_flags &= ~MIPI_DSI_MODE_LPM;