The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] drm/panel: ilitek-ili9805: Use _multi variants
@ 2026-07-16 22:09 Michail Tatas
  2026-07-16 23:31 ` Doug Anderson
  0 siblings, 1 reply; 2+ messages in thread
From: Michail Tatas @ 2026-07-16 22:09 UTC (permalink / raw)
  To: michael, neil.armstrong, maarten.lankhorst, mripard, tzimmermann,
	airlied, simona, dianders, jesszhan0024
  Cc: dri-devel, linux-kernel

Convert functions mipi_dsi_* to mipi_dsi_*_multi as per the
gpu/todo file.
The multi variant of these functions include improved error 
handling.

Signed-off-by: Michail Tatas <michail.tatas@gmail.com>
---
 drivers/gpu/drm/panel/panel-ilitek-ili9805.c | 51 +++++---------------
 1 file changed, 13 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9805.c b/drivers/gpu/drm/panel/panel-ilitek-ili9805.c
index e6c483851f1f..7e9587afebbe 100644
--- a/drivers/gpu/drm/panel/panel-ilitek-ili9805.c
+++ b/drivers/gpu/drm/panel/panel-ilitek-ili9805.c
@@ -159,36 +159,23 @@ static int ili9805_power_off(struct ili9805 *ctx)
 
 static int ili9805_activate(struct ili9805 *ctx)
 {
-	struct mipi_dsi_device *dsi = ctx->dsi;
-	struct device *dev = &dsi->dev;
-	int i, ret;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
+	int i;
 
 	for (i = 0; i < ctx->desc->init_length; i++) {
 		const struct ili9805_instr *instr = &ctx->desc->init[i];
 
-		ret = mipi_dsi_dcs_write_buffer(ctx->dsi, instr->data, instr->len);
-		if (ret < 0)
-			return ret;
+		mipi_dsi_dcs_write_buffer_multi(&dsi_ctx, instr->data, instr->len);
 
 		if (instr->delay > 0)
-			msleep(instr->delay);
-	}
-
-	ret = mipi_dsi_dcs_exit_sleep_mode(ctx->dsi);
-	if (ret) {
-		dev_err(dev, "Failed to exit sleep mode (%d)\n", ret);
-		return ret;
+			mipi_dsi_msleep(&dsi_ctx, instr->delay);
 	}
 
-	usleep_range(5000, 6000);
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+	mipi_dsi_usleep_range(&dsi_ctx, 5000, 6000);
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
 
-	ret = mipi_dsi_dcs_set_display_on(ctx->dsi);
-	if (ret) {
-		dev_err(dev, "Failed to set display ON (%d)\n", ret);
-		return ret;
-	}
-
-	return 0;
+	return dsi_ctx.accum_err;
 }
 
 static int ili9805_prepare(struct drm_panel *panel)
@@ -211,25 +198,13 @@ static int ili9805_prepare(struct drm_panel *panel)
 
 static int ili9805_deactivate(struct ili9805 *ctx)
 {
-	struct mipi_dsi_device *dsi = ctx->dsi;
-	struct device *dev = &dsi->dev;
-	int ret;
-
-	ret = mipi_dsi_dcs_set_display_off(ctx->dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to set display OFF (%d)\n", ret);
-		return ret;
-	}
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
 
-	usleep_range(5000, 10000);
-
-	ret = mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
-	if (ret < 0) {
-		dev_err(dev, "Failed to enter sleep mode (%d)\n", ret);
-		return ret;
-	}
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+	mipi_dsi_usleep_range(&dsi_ctx, 5000, 10000);
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
 
-	return 0;
+	return dsi_ctx.accum_err;
 }
 
 static int ili9805_unprepare(struct drm_panel *panel)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] drm/panel: ilitek-ili9805: Use _multi variants
  2026-07-16 22:09 [PATCH] drm/panel: ilitek-ili9805: Use _multi variants Michail Tatas
@ 2026-07-16 23:31 ` Doug Anderson
  0 siblings, 0 replies; 2+ messages in thread
From: Doug Anderson @ 2026-07-16 23:31 UTC (permalink / raw)
  To: Michail Tatas
  Cc: michael, neil.armstrong, maarten.lankhorst, mripard, tzimmermann,
	airlied, simona, jesszhan0024, dri-devel, linux-kernel

Hi,

On Thu, Jul 16, 2026 at 3:09 PM Michail Tatas <michail.tatas@gmail.com> wrote:
>
> @@ -159,36 +159,23 @@ static int ili9805_power_off(struct ili9805 *ctx)
>
>  static int ili9805_activate(struct ili9805 *ctx)
>  {
> -       struct mipi_dsi_device *dsi = ctx->dsi;
> -       struct device *dev = &dsi->dev;
> -       int i, ret;
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
> +       int i;
>
>         for (i = 0; i < ctx->desc->init_length; i++) {
>                 const struct ili9805_instr *instr = &ctx->desc->init[i];
>
> -               ret = mipi_dsi_dcs_write_buffer(ctx->dsi, instr->data, instr->len);
> -               if (ret < 0)
> -                       return ret;
> +               mipi_dsi_dcs_write_buffer_multi(&dsi_ctx, instr->data, instr->len);
>
>                 if (instr->delay > 0)
> -                       msleep(instr->delay);
> -       }

What you've done is an improvement, but it's not all the way there.
Specifically, we'd really want to get rid of the whole "struct
ili9805_instr" type and instead each panel should have an init
function. The "struct ili9805_desc" should have a pointer to the init
function instead of a pointer to the init data.

For details, you can see a preivous email about this [1], which then
has further links if you want to dig into details. You can see that
Chintan eventually implemented this in commit a89d9a327d06
("drm/panel: novatek-nt36672a: Inline panel init sequences").

If landing this patch without cleaning up the init sequences is really
important to you, I could be convinced. However, the init sequences
aren't all that big and it would be nice if you could clean it up all
at once. It could be one patch or two.

[1] http://lore.kernel.org/r/CAD=FV=UCyfjiqcpYCM5ePz-auX4g=i_+i78ivvvyA8R1XtAKzg@mail.gmail.com


> @@ -211,25 +198,13 @@ static int ili9805_prepare(struct drm_panel *panel)
>
>  static int ili9805_deactivate(struct ili9805 *ctx)
>  {
> -       struct mipi_dsi_device *dsi = ctx->dsi;
> -       struct device *dev = &dsi->dev;
> -       int ret;
> -
> -       ret = mipi_dsi_dcs_set_display_off(ctx->dsi);
> -       if (ret < 0) {
> -               dev_err(dev, "Failed to set display OFF (%d)\n", ret);
> -               return ret;
> -       }
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
>
> -       usleep_range(5000, 10000);
> -
> -       ret = mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
> -       if (ret < 0) {
> -               dev_err(dev, "Failed to enter sleep mode (%d)\n", ret);
> -               return ret;
> -       }
> +       mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +       mipi_dsi_usleep_range(&dsi_ctx, 5000, 10000);
> +       mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>
> -       return 0;
> +       return dsi_ctx.accum_err;

Nobody looks at the return code of this function. Since you're
touching it anyway, can you change ili9805_deactivate() to return
"void"? I wouldn't object if you made ili9805_power_off() return
"void" in the same patch too, even though it's a bit unrelated to the
rest of the patch.

-Doug

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-16 23:32 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16 22:09 [PATCH] drm/panel: ilitek-ili9805: Use _multi variants Michail Tatas
2026-07-16 23:31 ` Doug Anderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox