Hi,

On Fri, Dec 12, 2025 at 10:37 AM Amin GATTOUT <[email protected]> wrote:
>
> Update the driver to use the non-deprecated mipi_dsi_*_multi()
> helpers, as recommended in Documentation/gpu/todo.rst. The multi
> variants provide proper error accumulation and handle the required
> DCS NOP insertions, which suits the OTM8009A command sequences.
>
> Refactor otm8009a_dcs_write_buf() and the dcs_write_seq/dcs_write_cmd_at
> macros to take a mipi_dsi_multi_context pointer, passing it through
> from callers. This ensures consistent error handling throughout the
> driver.
>
> Replace all mdelay() and msleep() calls within DSI command sequences
> with mipi_dsi_msleep() for proper error accumulation.
>
> The init, disable, and backlight update paths now return dsi_ctx.accum_err,
> ensuring errors are propagated to callers.
>
> Signed-off-by: Amin GATTOUT <[email protected]>
> ---
>  .../gpu/drm/panel/panel-orisetech-otm8009a.c  | 171 ++++++++----------
>  1 file changed, 74 insertions(+), 97 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c 
> b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> index a0f58c3b73f6..1388e292fb60 100644
> --- a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> +++ b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> @@ -109,13 +109,10 @@ static inline struct otm8009a *panel_to_otm8009a(struct 
> drm_panel *panel)
>         return container_of(panel, struct otm8009a, panel);
>  }
>
> -static void otm8009a_dcs_write_buf(struct otm8009a *ctx, const void *data,
> +static void otm8009a_dcs_write_buf(struct mipi_dsi_multi_context *dsi_ctx, 
> const void *data,
>                                    size_t len)
>  {
> -       struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> -
> -       if (mipi_dsi_dcs_write_buffer(dsi, data, len) < 0)
> -               dev_warn(ctx->dev, "mipi dsi dcs write buffer failed\n");
> +       mipi_dsi_dcs_write_buffer_multi(dsi_ctx, data, len);
>  }

This looks really good now, though a nit. The function
otm8009a_dcs_write_buf() provides no value now. Delete it and have
callers just call mipi_dsi_dcs_write_buffer_multi() directly.

Oh, and I guess dcs_write_seq() also becomes pointless and can be
removed? dcs_write_cmd_at() would just become this, I think:

#define dcs_write_cmd_at(ctx, cmd, seq...) \
({ \
  mipi_dsi_dcs_write_seq_multi(ctx, MCS_ADRSFT, (cmd) & 0xFF); \
  mipi_dsi_dcs_write_seq_multi(ctx, (cmd) >> 8, seq); \
})

-Doug

On Fri, Dec 12, 2025 at 10:37 AM Amin GATTOUT <[email protected]> wrote:
>
> Update the driver to use the non-deprecated mipi_dsi_*_multi()
> helpers, as recommended in Documentation/gpu/todo.rst. The multi
> variants provide proper error accumulation and handle the required
> DCS NOP insertions, which suits the OTM8009A command sequences.
>
> Refactor otm8009a_dcs_write_buf() and the dcs_write_seq/dcs_write_cmd_at
> macros to take a mipi_dsi_multi_context pointer, passing it through
> from callers. This ensures consistent error handling throughout the
> driver.
>
> Replace all mdelay() and msleep() calls within DSI command sequences
> with mipi_dsi_msleep() for proper error accumulation.
>
> The init, disable, and backlight update paths now return dsi_ctx.accum_err,
> ensuring errors are propagated to callers.
>
> Signed-off-by: Amin GATTOUT <[email protected]>
> ---
>  .../gpu/drm/panel/panel-orisetech-otm8009a.c  | 171 ++++++++----------
>  1 file changed, 74 insertions(+), 97 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c 
> b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> index a0f58c3b73f6..1388e292fb60 100644
> --- a/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> +++ b/drivers/gpu/drm/panel/panel-orisetech-otm8009a.c
> @@ -109,13 +109,10 @@ static inline struct otm8009a *panel_to_otm8009a(struct 
> drm_panel *panel)
>         return container_of(panel, struct otm8009a, panel);
>  }
>
> -static void otm8009a_dcs_write_buf(struct otm8009a *ctx, const void *data,
> +static void otm8009a_dcs_write_buf(struct mipi_dsi_multi_context *dsi_ctx, 
> const void *data,
>                                    size_t len)
>  {
> -       struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> -
> -       if (mipi_dsi_dcs_write_buffer(dsi, data, len) < 0)
> -               dev_warn(ctx->dev, "mipi dsi dcs write buffer failed\n");
> +       mipi_dsi_dcs_write_buffer_multi(dsi_ctx, data, len);
>  }
>
>  #define dcs_write_seq(ctx, seq...)                     \
> @@ -133,153 +130,131 @@ static void otm8009a_dcs_write_buf(struct otm8009a 
> *ctx, const void *data,
>  static int otm8009a_init_sequence(struct otm8009a *ctx)
>  {
>         struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> -       int ret;
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>
>         /* Enter CMD2 */
> -       dcs_write_cmd_at(ctx, MCS_CMD2_ENA1, 0x80, 0x09, 0x01);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_CMD2_ENA1, 0x80, 0x09, 0x01);
>
>         /* Enter Orise Command2 */
> -       dcs_write_cmd_at(ctx, MCS_CMD2_ENA2, 0x80, 0x09);
> -
> -       dcs_write_cmd_at(ctx, MCS_SD_PCH_CTRL, 0x30);
> -       mdelay(10);
> -
> -       dcs_write_cmd_at(ctx, MCS_NO_DOC1, 0x40);
> -       mdelay(10);
> -
> -       dcs_write_cmd_at(ctx, MCS_PWR_CTRL4 + 1, 0xA9);
> -       dcs_write_cmd_at(ctx, MCS_PWR_CTRL2 + 1, 0x34);
> -       dcs_write_cmd_at(ctx, MCS_P_DRV_M, 0x50);
> -       dcs_write_cmd_at(ctx, MCS_VCOMDC, 0x4E);
> -       dcs_write_cmd_at(ctx, MCS_OSC_ADJ, 0x66); /* 65Hz */
> -       dcs_write_cmd_at(ctx, MCS_PWR_CTRL2 + 2, 0x01);
> -       dcs_write_cmd_at(ctx, MCS_PWR_CTRL2 + 5, 0x34);
> -       dcs_write_cmd_at(ctx, MCS_PWR_CTRL2 + 4, 0x33);
> -       dcs_write_cmd_at(ctx, MCS_GVDDSET, 0x79, 0x79);
> -       dcs_write_cmd_at(ctx, MCS_SD_CTRL + 1, 0x1B);
> -       dcs_write_cmd_at(ctx, MCS_PWR_CTRL1 + 2, 0x83);
> -       dcs_write_cmd_at(ctx, MCS_SD_PCH_CTRL + 1, 0x83);
> -       dcs_write_cmd_at(ctx, MCS_RGB_VID_SET, 0x0E);
> -       dcs_write_cmd_at(ctx, MCS_PANSET, 0x00, 0x01);
> -
> -       dcs_write_cmd_at(ctx, MCS_GOAVST, 0x85, 0x01, 0x00, 0x84, 0x01, 0x00);
> -       dcs_write_cmd_at(ctx, MCS_GOACLKA1, 0x18, 0x04, 0x03, 0x39, 0x00, 
> 0x00,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_CMD2_ENA2, 0x80, 0x09);
> +
> +       dcs_write_cmd_at(&dsi_ctx, MCS_SD_PCH_CTRL, 0x30);
> +       mipi_dsi_msleep(&dsi_ctx, 10);
> +
> +       dcs_write_cmd_at(&dsi_ctx, MCS_NO_DOC1, 0x40);
> +       mipi_dsi_msleep(&dsi_ctx, 10);
> +
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PWR_CTRL4 + 1, 0xA9);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PWR_CTRL2 + 1, 0x34);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_P_DRV_M, 0x50);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_VCOMDC, 0x4E);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_OSC_ADJ, 0x66); /* 65Hz */
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PWR_CTRL2 + 2, 0x01);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PWR_CTRL2 + 5, 0x34);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PWR_CTRL2 + 4, 0x33);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_GVDDSET, 0x79, 0x79);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_SD_CTRL + 1, 0x1B);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PWR_CTRL1 + 2, 0x83);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_SD_PCH_CTRL + 1, 0x83);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_RGB_VID_SET, 0x0E);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PANSET, 0x00, 0x01);
> +
> +       dcs_write_cmd_at(&dsi_ctx, MCS_GOAVST, 0x85, 0x01, 0x00, 0x84, 0x01, 
> 0x00);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_GOACLKA1, 0x18, 0x04, 0x03, 0x39, 
> 0x00, 0x00,
>                          0x00, 0x18, 0x03, 0x03, 0x3A, 0x00, 0x00, 0x00);
> -       dcs_write_cmd_at(ctx, MCS_GOACLKA3, 0x18, 0x02, 0x03, 0x3B, 0x00, 
> 0x00,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_GOACLKA3, 0x18, 0x02, 0x03, 0x3B, 
> 0x00, 0x00,
>                          0x00, 0x18, 0x01, 0x03, 0x3C, 0x00, 0x00, 0x00);
> -       dcs_write_cmd_at(ctx, MCS_GOAECLK, 0x01, 0x01, 0x20, 0x20, 0x00, 0x00,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_GOAECLK, 0x01, 0x01, 0x20, 0x20, 0x00, 
> 0x00,
>                          0x01, 0x02, 0x00, 0x00);
>
> -       dcs_write_cmd_at(ctx, MCS_NO_DOC2, 0x00);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_NO_DOC2, 0x00);
>
> -       dcs_write_cmd_at(ctx, MCS_PANCTRLSET1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
> -       dcs_write_cmd_at(ctx, MCS_PANCTRLSET2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PANCTRLSET1, 0, 0, 0, 0, 0, 0, 0, 0, 
> 0, 0);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PANCTRLSET2, 0, 0, 0, 0, 0, 0, 0, 0, 
> 0, 0,
>                          0, 0, 0, 0, 0);
> -       dcs_write_cmd_at(ctx, MCS_PANCTRLSET3, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PANCTRLSET3, 0, 0, 0, 0, 0, 0, 0, 0, 
> 0, 0,
>                          0, 0, 0, 0, 0);
> -       dcs_write_cmd_at(ctx, MCS_PANCTRLSET4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
> -       dcs_write_cmd_at(ctx, MCS_PANCTRLSET5, 0, 4, 4, 4, 4, 4, 0, 0, 0, 0,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PANCTRLSET4, 0, 0, 0, 0, 0, 0, 0, 0, 
> 0, 0);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PANCTRLSET5, 0, 4, 4, 4, 4, 4, 0, 0, 
> 0, 0,
>                          0, 0, 0, 0, 0);
> -       dcs_write_cmd_at(ctx, MCS_PANCTRLSET6, 0, 0, 0, 0, 0, 0, 4, 4, 4, 4,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PANCTRLSET6, 0, 0, 0, 0, 0, 0, 4, 4, 
> 4, 4,
>                          4, 0, 0, 0, 0);
> -       dcs_write_cmd_at(ctx, MCS_PANCTRLSET7, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
> -       dcs_write_cmd_at(ctx, MCS_PANCTRLSET8, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PANCTRLSET7, 0, 0, 0, 0, 0, 0, 0, 0, 
> 0, 0);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PANCTRLSET8, 0xFF, 0xFF, 0xFF, 0xFF, 
> 0xFF,
>                          0xFF, 0xFF, 0xFF, 0xFF, 0xFF);
>
> -       dcs_write_cmd_at(ctx, MCS_PANU2D1, 0x00, 0x26, 0x09, 0x0B, 0x01, 0x25,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PANU2D1, 0x00, 0x26, 0x09, 0x0B, 0x01, 
> 0x25,
>                          0x00, 0x00, 0x00, 0x00);
> -       dcs_write_cmd_at(ctx, MCS_PANU2D2, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PANU2D2, 0x00, 0x00, 0x00, 0x00, 0x00, 
> 0x00,
>                          0x00, 0x00, 0x00, 0x00, 0x00, 0x26, 0x0A, 0x0C, 
> 0x02);
> -       dcs_write_cmd_at(ctx, MCS_PANU2D3, 0x25, 0x00, 0x00, 0x00, 0x00, 0x00,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PANU2D3, 0x25, 0x00, 0x00, 0x00, 0x00, 
> 0x00,
>                          0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
> 0x00);
> -       dcs_write_cmd_at(ctx, MCS_PAND2U1, 0x00, 0x25, 0x0C, 0x0A, 0x02, 0x26,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PAND2U1, 0x00, 0x25, 0x0C, 0x0A, 0x02, 
> 0x26,
>                          0x00, 0x00, 0x00, 0x00);
> -       dcs_write_cmd_at(ctx, MCS_PAND2U2, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PAND2U2, 0x00, 0x00, 0x00, 0x00, 0x00, 
> 0x00,
>                          0x00, 0x00, 0x00, 0x00, 0x00, 0x25, 0x0B, 0x09, 
> 0x01);
> -       dcs_write_cmd_at(ctx, MCS_PAND2U3, 0x26, 0x00, 0x00, 0x00, 0x00, 0x00,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PAND2U3, 0x26, 0x00, 0x00, 0x00, 0x00, 
> 0x00,
>                          0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
> 0x00);
>
> -       dcs_write_cmd_at(ctx, MCS_PWR_CTRL1 + 1, 0x66);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_PWR_CTRL1 + 1, 0x66);
>
> -       dcs_write_cmd_at(ctx, MCS_NO_DOC3, 0x06);
> +       dcs_write_cmd_at(&dsi_ctx, MCS_NO_DOC3, 0x06);
>
> -       dcs_write_cmd_at(ctx, MCS_GMCT2_2P, 0x00, 0x09, 0x0F, 0x0E, 0x07, 
> 0x10,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_GMCT2_2P, 0x00, 0x09, 0x0F, 0x0E, 
> 0x07, 0x10,
>                          0x0B, 0x0A, 0x04, 0x07, 0x0B, 0x08, 0x0F, 0x10, 0x0A,
>                          0x01);
> -       dcs_write_cmd_at(ctx, MCS_GMCT2_2N, 0x00, 0x09, 0x0F, 0x0E, 0x07, 
> 0x10,
> +       dcs_write_cmd_at(&dsi_ctx, MCS_GMCT2_2N, 0x00, 0x09, 0x0F, 0x0E, 
> 0x07, 0x10,
>                          0x0B, 0x0A, 0x04, 0x07, 0x0B, 0x08, 0x0F, 0x10, 0x0A,
>                          0x01);
>
>         /* Exit CMD2 */
> -       dcs_write_cmd_at(ctx, MCS_CMD2_ENA1, 0xFF, 0xFF, 0xFF);
> -
> -       ret = mipi_dsi_dcs_nop(dsi);
> -       if (ret)
> -               return ret;
> +       dcs_write_cmd_at(&dsi_ctx, MCS_CMD2_ENA1, 0xFF, 0xFF, 0xFF);
>
> -       ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> -       if (ret)
> -               return ret;
> +       mipi_dsi_dcs_nop_multi(&dsi_ctx);
>
> -       /* Wait for sleep out exit */
> -       mdelay(120);
> +       mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +       mipi_dsi_msleep(&dsi_ctx, 120);
>
>         /* Default portrait 480x800 rgb24 */
> -       dcs_write_seq(ctx, MIPI_DCS_SET_ADDRESS_MODE, 0x00);
> +       dcs_write_seq(&dsi_ctx, MIPI_DCS_SET_ADDRESS_MODE, 0x00);
>
> -       ret = mipi_dsi_dcs_set_column_address(dsi, 0, OTM8009A_HDISPLAY - 1);
> -       if (ret)
> -               return ret;
> +       mipi_dsi_dcs_set_column_address_multi(&dsi_ctx, 0, OTM8009A_HDISPLAY 
> - 1);
>
> -       ret = mipi_dsi_dcs_set_page_address(dsi, 0, OTM8009A_VDISPLAY - 1);
> -       if (ret)
> -               return ret;
> +       mipi_dsi_dcs_set_page_address_multi(&dsi_ctx, 0, OTM8009A_VDISPLAY - 
> 1);
>
>         /* See otm8009a driver documentation for pixel format descriptions */
> -       ret = mipi_dsi_dcs_set_pixel_format(dsi, MIPI_DCS_PIXEL_FMT_24BIT |
> +       mipi_dsi_dcs_set_pixel_format_multi(&dsi_ctx, 
> MIPI_DCS_PIXEL_FMT_24BIT |
>                                             MIPI_DCS_PIXEL_FMT_24BIT << 4);
> -       if (ret)
> -               return ret;
>
>         /* Disable CABC feature */
> -       dcs_write_seq(ctx, MIPI_DCS_WRITE_POWER_SAVE, 0x00);
> +       dcs_write_seq(&dsi_ctx, MIPI_DCS_WRITE_POWER_SAVE, 0x00);
>
> -       ret = mipi_dsi_dcs_set_display_on(dsi);
> -       if (ret)
> -               return ret;
> +       mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
>
> -       ret = mipi_dsi_dcs_nop(dsi);
> -       if (ret)
> -               return ret;
> +       mipi_dsi_dcs_nop_multi(&dsi_ctx);
>
>         /* Send Command GRAM memory write (no parameters) */
> -       dcs_write_seq(ctx, MIPI_DCS_WRITE_MEMORY_START);
> +       dcs_write_seq(&dsi_ctx, MIPI_DCS_WRITE_MEMORY_START);
>
>         /* Wait a short while to let the panel be ready before the 1st frame 
> */
> -       mdelay(10);
> +       mipi_dsi_msleep(&dsi_ctx, 10);
>
> -       return 0;
> +       return dsi_ctx.accum_err;
>  }
>
>  static int otm8009a_disable(struct drm_panel *panel)
>  {
>         struct otm8009a *ctx = panel_to_otm8009a(panel);
>         struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> -       int ret;
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>
>         backlight_disable(ctx->bl_dev);
>
> -       ret = mipi_dsi_dcs_set_display_off(dsi);
> -       if (ret)
> -               return ret;
> -
> -       ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> -       if (ret)
> -               return ret;
> -
> -       msleep(120);
> +       mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> +       mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> +       mipi_dsi_msleep(&dsi_ctx, 120);
>
> -       return 0;
> +       return dsi_ctx.accum_err;
>  }
>
>  static int otm8009a_unprepare(struct drm_panel *panel)
> @@ -383,6 +358,8 @@ static const struct drm_panel_funcs otm8009a_drm_funcs = {
>  static int otm8009a_backlight_update_status(struct backlight_device *bd)
>  {
>         struct otm8009a *ctx = bl_get_data(bd);
> +       struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>         u8 data[2];
>
>         if (!ctx->prepared) {
> @@ -397,7 +374,7 @@ static int otm8009a_backlight_update_status(struct 
> backlight_device *bd)
>                  */
>                 data[0] = MIPI_DCS_SET_DISPLAY_BRIGHTNESS;
>                 data[1] = bd->props.brightness;
> -               otm8009a_dcs_write_buf(ctx, data, ARRAY_SIZE(data));
> +               otm8009a_dcs_write_buf(&dsi_ctx, data, ARRAY_SIZE(data));
>
>                 /* set Brightness Control & Backlight on */
>                 data[1] = 0x24;
> @@ -409,9 +386,9 @@ static int otm8009a_backlight_update_status(struct 
> backlight_device *bd)
>
>         /* Update Brightness Control & Backlight */
>         data[0] = MIPI_DCS_WRITE_CONTROL_DISPLAY;
> -       otm8009a_dcs_write_buf(ctx, data, ARRAY_SIZE(data));
> +       otm8009a_dcs_write_buf(&dsi_ctx, data, ARRAY_SIZE(data));
>
> -       return 0;
> +       return dsi_ctx.accum_err;
>  }
>
>  static const struct backlight_ops otm8009a_backlight_ops = {
> --
> 2.43.0
>

Reply via email to