.../gpu/drm/panel/panel-novatek-nt36672a.c | 83 ++++++------------- 1 file changed, 26 insertions(+), 57 deletions(-)
Convert the driver to use the non-deprecated mipi_dsi_*_multi()
helpers and mipi_dsi_msleep().
This removes open-coded error handling and relies on
mipi_dsi_multi_context for error accumulation.
In unprepare(), reset the accumulated error between
set_display_off and enter_sleep_mode to preserve the
existing power-down sequencing semantics.
Signed-off-by: Chintan Patel <chintanlike@gmail.com>
---
.../gpu/drm/panel/panel-novatek-nt36672a.c | 83 ++++++-------------
1 file changed, 26 insertions(+), 57 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
index 29e1f6aea480..2c8d67a69c7e 100644
--- a/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
+++ b/drivers/gpu/drm/panel/panel-novatek-nt36672a.c
@@ -79,23 +79,17 @@ static inline struct nt36672a_panel *to_nt36672a_panel(struct drm_panel *panel)
return container_of(panel, struct nt36672a_panel, base);
}
-static int nt36672a_send_cmds(struct drm_panel *panel, const struct nt36672a_panel_cmd *cmds,
- int num)
+static void nt36672a_send_cmds(struct mipi_dsi_multi_context *dsi_ctx,
+ const struct nt36672a_panel_cmd *cmds, int num)
{
- struct nt36672a_panel *pinfo = to_nt36672a_panel(panel);
unsigned int i;
- int err;
for (i = 0; i < num; i++) {
const struct nt36672a_panel_cmd *cmd = &cmds[i];
- err = mipi_dsi_dcs_write(pinfo->link, cmd->data[0], cmd->data + 1, 1);
-
- if (err < 0)
- return err;
+ /* cmd->data[0] is the DCS command, cmd->data[1] is the parameter */
+ mipi_dsi_dcs_write_buffer_multi(dsi_ctx, cmd->data, sizeof(cmd->data));
}
-
- return 0;
}
static int nt36672a_panel_power_off(struct drm_panel *panel)
@@ -115,34 +109,26 @@ static int nt36672a_panel_power_off(struct drm_panel *panel)
static int nt36672a_panel_unprepare(struct drm_panel *panel)
{
struct nt36672a_panel *pinfo = to_nt36672a_panel(panel);
- int ret;
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = pinfo->link };
/* send off cmds */
- ret = nt36672a_send_cmds(panel, pinfo->desc->off_cmds,
- pinfo->desc->num_off_cmds);
+ nt36672a_send_cmds(&dsi_ctx, pinfo->desc->off_cmds,
+ pinfo->desc->num_off_cmds);
- if (ret < 0)
- dev_err(panel->dev, "failed to send DCS off cmds: %d\n", ret);
-
- ret = mipi_dsi_dcs_set_display_off(pinfo->link);
- if (ret < 0)
- dev_err(panel->dev, "set_display_off cmd failed ret = %d\n", ret);
+ mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+ /* Reset error to continue power-down even if display off failed */
+ dsi_ctx.accum_err = 0;
+ mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
/* 120ms delay required here as per DCS spec */
msleep(120);
- ret = mipi_dsi_dcs_enter_sleep_mode(pinfo->link);
- if (ret < 0)
- dev_err(panel->dev, "enter_sleep cmd failed ret = %d\n", ret);
-
/* 0x3C = 60ms delay */
msleep(60);
- ret = nt36672a_panel_power_off(panel);
- if (ret < 0)
- dev_err(panel->dev, "power_off failed ret = %d\n", ret);
+ nt36672a_panel_power_off(panel);
- return ret;
+ return 0;
}
static int nt36672a_panel_power_on(struct nt36672a_panel *pinfo)
@@ -170,51 +156,34 @@ static int nt36672a_panel_power_on(struct nt36672a_panel *pinfo)
static int nt36672a_panel_prepare(struct drm_panel *panel)
{
struct nt36672a_panel *pinfo = to_nt36672a_panel(panel);
+ struct mipi_dsi_multi_context dsi_ctx = { .dsi = pinfo->link };
int err;
err = nt36672a_panel_power_on(pinfo);
if (err < 0)
- goto poweroff;
+ return err;
/* send first part of init cmds */
- err = nt36672a_send_cmds(panel, pinfo->desc->on_cmds_1,
- pinfo->desc->num_on_cmds_1);
+ nt36672a_send_cmds(&dsi_ctx, pinfo->desc->on_cmds_1,
+ pinfo->desc->num_on_cmds_1);
- if (err < 0) {
- dev_err(panel->dev, "failed to send DCS Init 1st Code: %d\n", err);
- goto poweroff;
- }
-
- err = mipi_dsi_dcs_exit_sleep_mode(pinfo->link);
- if (err < 0) {
- dev_err(panel->dev, "failed to exit sleep mode: %d\n", err);
- goto poweroff;
- }
+ mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
/* 0x46 = 70 ms delay */
- msleep(70);
+ mipi_dsi_msleep(&dsi_ctx, 70);
- err = mipi_dsi_dcs_set_display_on(pinfo->link);
- if (err < 0) {
- dev_err(panel->dev, "failed to Set Display ON: %d\n", err);
- goto poweroff;
- }
+ mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
/* Send rest of the init cmds */
- err = nt36672a_send_cmds(panel, pinfo->desc->on_cmds_2,
- pinfo->desc->num_on_cmds_2);
+ nt36672a_send_cmds(&dsi_ctx, pinfo->desc->on_cmds_2,
+ pinfo->desc->num_on_cmds_2);
- if (err < 0) {
- dev_err(panel->dev, "failed to send DCS Init 2nd Code: %d\n", err);
- goto poweroff;
- }
-
- msleep(120);
+ mipi_dsi_msleep(&dsi_ctx, 120);
- return 0;
+ err = dsi_ctx.accum_err;
+ if (err < 0)
+ gpiod_set_value(pinfo->reset_gpio, 0);
-poweroff:
- gpiod_set_value(pinfo->reset_gpio, 0);
return err;
}
--
2.43.0Hi,
On Sun, Feb 22, 2026 at 8:35 PM Chintan Patel <chintanlike@gmail.com> wrote:
>
> @@ -79,23 +79,17 @@ static inline struct nt36672a_panel *to_nt36672a_panel(struct drm_panel *panel)
> return container_of(panel, struct nt36672a_panel, base);
> }
>
> -static int nt36672a_send_cmds(struct drm_panel *panel, const struct nt36672a_panel_cmd *cmds,
> - int num)
> +static void nt36672a_send_cmds(struct mipi_dsi_multi_context *dsi_ctx,
> + const struct nt36672a_panel_cmd *cmds, int num)
nit: checkpatch --strict yells:
CHECK: Alignment should match open parenthesis
#37: FILE: drivers/gpu/drm/panel/panel-novatek-nt36672a.c:83:
+static void nt36672a_send_cmds(struct mipi_dsi_multi_context *dsi_ctx,
+ const struct nt36672a_panel_cmd *cmds, int num)
Also: FWIW having a function like nt36672a_send_cmds() is discouraged
these days. One of the reasons for creating the "_multi" functions and
making them efficient was that init functions were preferred. For
instance, see the commit d6ddb6624a7f ("drm/panel: boe-tv101wum-nl6:
Don't use a table for initting panels") or commit 6f6fd690de1a
("drm/panel: innolux-p079zca: Don't use a table for initting panels").
I won't insist that you make the conversion here since I think you're
still providing a valuable cleanup with the patch you've already
written, but in case you want to go above-and-beyond you could try it.
You could even try running bloat-o-meter to see how the size changes.
> @@ -115,34 +109,26 @@ static int nt36672a_panel_power_off(struct drm_panel *panel)
> static int nt36672a_panel_unprepare(struct drm_panel *panel)
> {
> struct nt36672a_panel *pinfo = to_nt36672a_panel(panel);
> - int ret;
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = pinfo->link };
>
> /* send off cmds */
> - ret = nt36672a_send_cmds(panel, pinfo->desc->off_cmds,
> - pinfo->desc->num_off_cmds);
> + nt36672a_send_cmds(&dsi_ctx, pinfo->desc->off_cmds,
> + pinfo->desc->num_off_cmds);
>
> - if (ret < 0)
> - dev_err(panel->dev, "failed to send DCS off cmds: %d\n", ret);
> -
> - ret = mipi_dsi_dcs_set_display_off(pinfo->link);
> - if (ret < 0)
> - dev_err(panel->dev, "set_display_off cmd failed ret = %d\n", ret);
> + mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
> + /* Reset error to continue power-down even if display off failed */
> + dsi_ctx.accum_err = 0;
> + mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>
> /* 120ms delay required here as per DCS spec */
> msleep(120);
>
> - ret = mipi_dsi_dcs_enter_sleep_mode(pinfo->link);
> - if (ret < 0)
> - dev_err(panel->dev, "enter_sleep cmd failed ret = %d\n", ret);
You've changed the order. Enter sleep mode used to be after the 120ms
sleep. Now it's before.
> -
> /* 0x3C = 60ms delay */
> msleep(60);
>
> - ret = nt36672a_panel_power_off(panel);
> - if (ret < 0)
> - dev_err(panel->dev, "power_off failed ret = %d\n", ret);
> + nt36672a_panel_power_off(panel);
You got rid of the extra print here, which is fine/right from my
perspective (since nt36672a_panel_power_off() already printed). ...but
nt36672a_panel_power_off()'s return code is never checked now. Just
make nt36672a_panel_power_off() return "void".
> @@ -170,51 +156,34 @@ static int nt36672a_panel_power_on(struct nt36672a_panel *pinfo)
> static int nt36672a_panel_prepare(struct drm_panel *panel)
> {
> struct nt36672a_panel *pinfo = to_nt36672a_panel(panel);
> + struct mipi_dsi_multi_context dsi_ctx = { .dsi = pinfo->link };
> int err;
>
> err = nt36672a_panel_power_on(pinfo);
> if (err < 0)
> - goto poweroff;
> + return err;
I won't insist since it's a matter of opinion, but IMO get rid of the
`err` local variable and consistently use `dsi_ctx.accum_err` to store
the error code in this function. Yeah, it's a bit awkward, but having
the separate `err` variable just makes it too easy to mess up and use
it. As one example of where that happened, see commit 61ce50fd8196
("drm/panel: jdi-lpm102a188a: Fix error code in jdi_panel_prepare()").
Also: instead of returning right away, I think you want to make sure
that the GPIO gets set to 0 like the old code did, don't you? I'd be a
little hesitant changing this unless we can truly prove that the GPIO
set is not needed or unless we're certain that the GPIO set was
incorrect (in which case we should fix it in a separate patch).
...nicely, if you use `dsi_ctx.accum_err` you don't even need a
"goto". You can just let the "_multi" functions all run and be
no-ops...
-Doug
>
> I won't insist since it's a matter of opinion, but IMO get rid of the
> `err` local variable and consistently use `dsi_ctx.accum_err` to store
> the error code in this function. Yeah, it's a bit awkward, but having
> the separate `err` variable just makes it too easy to mess up and use
> it. As one example of where that happened, see commit 61ce50fd8196
> ("drm/panel: jdi-lpm102a188a: Fix error code in jdi_panel_prepare()").
>
> Also: instead of returning right away, I think you want to make sure
> that the GPIO gets set to 0 like the old code did, don't you? I'd be a
> little hesitant changing this unless we can truly prove that the GPIO
> set is not needed or unless we're certain that the GPIO set was
> incorrect (in which case we should fix it in a separate patch).
>
> ...nicely, if you use `dsi_ctx.accum_err` you don't even need a
> "goto". You can just let the "_multi" functions all run and be
> no-ops...
>
> -Doug
Thank you Doug! Those were good points! Will work on v2.
© 2016 - 2026 Red Hat, Inc.