Move away from using deprecated API and use _multi
variants if available. Use mipi_dsi_msleep()
and mipi_dsi_usleep_range() instead of msleep()
and usleep_range() respectively.

Used Coccinelle to find the multiple occurences.
SmPl patch:
@rule@
identifier dsi_var;
identifier r;
identifier func;
type t;
position p;
expression dsi_device;
expression list es;
@@
t func(...) {
...
struct mipi_dsi_device *dsi_var = dsi_device;
+struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi_var };
<+...
(
-mipi_dsi_dcs_write_seq(dsi_var,es)@p;
+mipi_dsi_dcs_write_seq_multi(&dsi_ctx,es);
|
-mipi_dsi_generic_write_seq(dsi_var,es)@p;
+mipi_dsi_generic_write_seq_multi(&dsi_ctx,es);
|
-mipi_dsi_generic_write(dsi_var,es)@p;
+mipi_dsi_generic_write_multi(&dsi_ctx,es);
|
-r = mipi_dsi_dcs_nop(dsi_var)@p;
+mipi_dsi_dcs_nop_multi(&dsi_ctx);
|
....rest of API
..
)
-if(r < 0) {
-...
-}
...+>

v2: Do not skip the reset in case of error during
panel activate (Dmitry)
- Convert all usleep_range()

Cc: Maxime Ripard <maxime.ripard@bootlin.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Tejas Vipin <tejasvipin76@gmail.com>
Cc: Doug Anderson <dianders@chromium.org>
Signed-off-by: Anusha Srivatsa <asrivats@redhat.com>
---
Previous attempt for this change was addressed in:[1]
The series did not handle returns properly and still
used msleep() and usleep_range() API.
It also collided with an Tejas's similar efforts.

Will be sending the patches per driver instead of
major haul of changes.

Following [2] for reference.

[1] -> https://patchwork.freedesktop.org/series/144824/
[2] -> https://lore.kernel.org/dri-devel/20250220045721.145905-1-tejasvipin76@gmail.com/#iZ31drivers:gpu:drm:panel:panel-sony-td4353-jdi.c
---
Changes in v2:
- Handle the reset case properly 
- Handle msleep() and  usleep_range()

- Link to v1: https://lore.kernel.org/r/20250305-mipi-synaptic-1-v1-1-92017cd19ef6@redhat.com
---
 drivers/gpu/drm/panel/panel-synaptics-r63353.c | 69 ++++++++++----------------
 1 file changed, 27 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-synaptics-r63353.c b/drivers/gpu/drm/panel/panel-synaptics-r63353.c
index 17349825543fe6a117bbfd9cb92a564ce433d13a..991723e5545725ac1de8e1f1968e3eea8254e2b2 100644
--- a/drivers/gpu/drm/panel/panel-synaptics-r63353.c
+++ b/drivers/gpu/drm/panel/panel-synaptics-r63353.c
@@ -70,6 +70,7 @@ static int r63353_panel_power_on(struct r63353_panel *rpanel)
 {
 	struct mipi_dsi_device *dsi = rpanel->dsi;
 	struct device *dev = &dsi->dev;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
 	int ret;
 
 	ret = regulator_enable(rpanel->avdd);
@@ -78,7 +79,7 @@ static int r63353_panel_power_on(struct r63353_panel *rpanel)
 		return ret;
 	}
 
-	usleep_range(15000, 25000);
+	mipi_dsi_usleep_range(&dsi_ctx, 15000, 25000);
 
 	ret = regulator_enable(rpanel->dvdd);
 	if (ret) {
@@ -87,9 +88,9 @@ static int r63353_panel_power_on(struct r63353_panel *rpanel)
 		return ret;
 	}
 
-	usleep_range(300000, 350000);
+	mipi_dsi_usleep_range(&dsi_ctx, 300000, 350000);
 	gpiod_set_value(rpanel->reset_gpio, 1);
-	usleep_range(15000, 25000);
+	mipi_dsi_usleep_range(&dsi_ctx, 15000, 25000);
 
 	return 0;
 }
@@ -106,53 +107,46 @@ static int r63353_panel_power_off(struct r63353_panel *rpanel)
 static int r63353_panel_activate(struct r63353_panel *rpanel)
 {
 	struct mipi_dsi_device *dsi = rpanel->dsi;
-	struct device *dev = &dsi->dev;
-	int i, ret;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
+	int i;
 
-	ret = mipi_dsi_dcs_soft_reset(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to do Software Reset (%d)\n", ret);
+	mipi_dsi_dcs_soft_reset_multi(&dsi_ctx);
+	if (dsi_ctx.accum_err)
 		goto fail;
-	}
 
-	usleep_range(15000, 17000);
+	mipi_dsi_usleep_range(&dsi_ctx, 15000, 17000);
 
-	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to enter sleep mode (%d)\n", ret);
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
+	if (dsi_ctx.accum_err)
 		goto fail;
-	}
 
 	for (i = 0; i < rpanel->pdata->init_length; i++) {
 		const struct r63353_instr *instr = &rpanel->pdata->init[i];
 
-		ret = mipi_dsi_dcs_write_buffer(dsi, instr->data, instr->len);
-		if (ret < 0)
+		mipi_dsi_dcs_write_buffer_multi(&dsi_ctx, instr->data,
+						instr->len);
+		if (dsi_ctx.accum_err)
 			goto fail;
 	}
 
-	msleep(120);
+	mipi_dsi_msleep(&dsi_ctx, 120);
 
-	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to exit sleep mode (%d)\n", ret);
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+	if (dsi_ctx.accum_err)
 		goto fail;
-	}
 
-	usleep_range(5000, 10000);
+	mipi_dsi_usleep_range(&dsi_ctx, 5000, 10000);
 
-	ret = mipi_dsi_dcs_set_display_on(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to set display ON (%d)\n", ret);
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+	if (dsi_ctx.accum_err)
 		goto fail;
-	}
 
-	return 0;
+	return dsi_ctx.accum_err;
 
 fail:
 	gpiod_set_value(rpanel->reset_gpio, 0);
 
-	return ret;
+	return dsi_ctx.accum_err;
 }
 
 static int r63353_panel_prepare(struct drm_panel *panel)
@@ -181,24 +175,15 @@ static int r63353_panel_prepare(struct drm_panel *panel)
 static int r63353_panel_deactivate(struct r63353_panel *rpanel)
 {
 	struct mipi_dsi_device *dsi = rpanel->dsi;
-	struct device *dev = &dsi->dev;
-	int ret;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
 
-	ret = mipi_dsi_dcs_set_display_off(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to set display OFF (%d)\n", ret);
-		return ret;
-	}
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
 
-	usleep_range(5000, 10000);
+	mipi_dsi_usleep_range(&dsi_ctx, 5000, 10000);
 
-	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to enter sleep mode (%d)\n", ret);
-		return ret;
-	}
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
 
-	return 0;
+	return dsi_ctx.accum_err;
 }
 
 static int r63353_panel_unprepare(struct drm_panel *panel)

---
base-commit: ced7486468ac3b38d59a69fca5d97998499c936b
change-id: 20250305-mipi-synaptic-1-a2043543cd3a

Best regards,
-- 
Anusha Srivatsa <asrivats@redhat.com>

Hi,

On Mon, Mar 10, 2025 at 1:58 PM Anusha Srivatsa <asrivats@redhat.com> wrote:
>
> @@ -70,6 +70,7 @@ static int r63353_panel_power_on(struct r63353_panel *rpanel)
>  {
>         struct mipi_dsi_device *dsi = rpanel->dsi;
>         struct device *dev = &dsi->dev;
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>         int ret;
>
>         ret = regulator_enable(rpanel->avdd);
> @@ -78,7 +79,7 @@ static int r63353_panel_power_on(struct r63353_panel *rpanel)
>                 return ret;
>         }
>
> -       usleep_range(15000, 25000);
> +       mipi_dsi_usleep_range(&dsi_ctx, 15000, 25000);

No. None of the conversions in this function are correct.
mipi_dsi_usleep_range() is only for use when you're in the middle of a
bunch of other "multi" calls and want the sleep to be conditional upon
there being no error. Here there is no chance of an error because no
_multi() are used. Go back to the normal usleep_range().

> @@ -106,53 +107,46 @@ static int r63353_panel_power_off(struct r63353_panel *rpanel)
>  static int r63353_panel_activate(struct r63353_panel *rpanel)
>  {
>         struct mipi_dsi_device *dsi = rpanel->dsi;
> -       struct device *dev = &dsi->dev;
> -       int i, ret;
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
> +       int i;
>
> -       ret = mipi_dsi_dcs_soft_reset(dsi);
> -       if (ret < 0) {
> -               dev_err(dev, "Failed to do Software Reset (%d)\n", ret);
> +       mipi_dsi_dcs_soft_reset_multi(&dsi_ctx);
> +       if (dsi_ctx.accum_err)
>                 goto fail;
> -       }

This isn't how the _multi() functions are intended to be used. The
whole idea is _not_ to have scattered "if" statements everywhere and
to just deal with errors at the appropriate places. You just trust
that the _multi() functions are no-ops if an error is set and so it
doesn't hurt to keep calling them. In this case you'd just have a pile
of _multi() functions with no "if" checks and then at the very end of
the function you check for the error. If the error is set then you set
the reset GPIO and return the error.

-Doug

On Mon, Mar 10, 2025 at 04:58:22PM -0400, Anusha Srivatsa wrote:
> Move away from using deprecated API and use _multi
> variants if available. Use mipi_dsi_msleep()
> and mipi_dsi_usleep_range() instead of msleep()
> and usleep_range() respectively.
> 
> Used Coccinelle to find the multiple occurences.
> SmPl patch:
> @rule@
> identifier dsi_var;
> identifier r;
> identifier func;
> type t;
> position p;
> expression dsi_device;
> expression list es;
> @@
> t func(...) {
> ...
> struct mipi_dsi_device *dsi_var = dsi_device;
> +struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi_var };
> <+...
> (
> -mipi_dsi_dcs_write_seq(dsi_var,es)@p;
> +mipi_dsi_dcs_write_seq_multi(&dsi_ctx,es);
> |
> -mipi_dsi_generic_write_seq(dsi_var,es)@p;
> +mipi_dsi_generic_write_seq_multi(&dsi_ctx,es);
> |
> -mipi_dsi_generic_write(dsi_var,es)@p;
> +mipi_dsi_generic_write_multi(&dsi_ctx,es);
> |
> -r = mipi_dsi_dcs_nop(dsi_var)@p;
> +mipi_dsi_dcs_nop_multi(&dsi_ctx);
> |
> ....rest of API
> ..
> )
> -if(r < 0) {
> -...
> -}
> ...+>

Again, you need to provide the full coccinelle script here otherwise
it's useless. And I have serious doubts that it's actually the script
you used, because ...

> @@ -106,53 +107,46 @@ static int r63353_panel_power_off(struct r63353_panel *rpanel)
>  static int r63353_panel_activate(struct r63353_panel *rpanel)
>  {
>  	struct mipi_dsi_device *dsi = rpanel->dsi;
> -	struct device *dev = &dsi->dev;
> -	int i, ret;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
> +	int i;
>  
> -	ret = mipi_dsi_dcs_soft_reset(dsi);
> -	if (ret < 0) {
> -		dev_err(dev, "Failed to do Software Reset (%d)\n", ret);
> +	mipi_dsi_dcs_soft_reset_multi(&dsi_ctx);
> +	if (dsi_ctx.accum_err)
>  		goto fail;
> -	}

This changes was definitely not what the script is doing.

Maxime