linux-kernel - Re: [PATCH] drm/panel: jdi-lt070me05000: Use MIPI DSI multi functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAD=FV=W1SHQDSxfWLhiR3LyGZioqm8VVGOp+FBUp_+r53ejSxQ@mail.gmail.com>
Date: Sat, 31 Jan 2026 10:07:35 -0800
From: Doug Anderson <dianders@...omium.org>
To: Chintan Patel <chintanlike@...il.com>
Cc: neil.armstrong@...aro.org, jesszhan0024@...il.com, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/panel: jdi-lt070me05000: Use MIPI DSI multi functions

Hi,

On Fri, Jan 30, 2026 at 7:23 PM Chintan Patel <chintanlike@...il.com> wrote:
>
> Convert to the non-deprecated mipi_dsi_*_multi() helpers per the TODO
> list. These reduce boilerplate error checking while providing proper
> error accumulation via mipi_dsi_multi_context.
>
> Also use mipi_dsi_msleep()/mipi_dsi_usleep_range() macros for all
> delays, which automatically skip on error.
>
> Suggested-by: Doug Anderson <dianders@...omium.org>

I'd skip my Suggested-by here. I'm already on the TODO. ;-)

> -       msleep(120);
> -
> -       ret = mipi_dsi_generic_write(dsi, (u8[]){0xB0, 0x00}, 2);
> -       if (ret < 0) {
> -               dev_err(dev, "failed to set mcap: %d\n", ret);
> -               return ret;
> -       }
> +       mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +       mipi_dsi_msleep(&dsi_ctx, 120);
>
> -       mdelay(10);

It's worth noting that the old code used a "mdelay" instead of a
"msleep" here. ...but I recognize that there is no "mipi_dsi_mdelay".
It's _probably_ OK to change this and we've already got other sleeping
calls in this function, but it could change the amount of delay a bit.

People always have fun bikeshedding about when to use the various
delays, but IMO slightly better in this case would be to change this
to the "usleep_range" call instead of "msleep". For functions like
this that aren't run very often, you probably don't want a very large
slop, so the mdelay(10) could probably be mipi_dsi_usleep_range(10000,
11000) and the later mdelay(20) could probably be
mipi_dsi_usleep_range(20000, 21000). You'd probably want to at least
mention this change in the commit message.

I'd leave the preexisting usleep_range(10000, 20000) with the
preexisting 10ms slop just to be on the safe side.

> +       mipi_dsi_generic_write_multi(&dsi_ctx, (u8[]){0xB0, 0x00}, 2);

FWIW, instead of the above I believe it's cleaner to do this:

mipi_dsi_generic_write_seq_multi(&dsi_ctx, 0xb0, 0x00);

...it's probably worth changing to that while you're doing your cleanup?

I'll also note that lowercase hex (0xb0 instead of 0xB0) is the
preferred style of the Linux kernel, so maybe switch to that as you're
touching the code, too?

>  static void jdi_panel_off(struct jdi_panel *jdi)
>  {
>         struct mipi_dsi_device *dsi = jdi->dsi;
> -       struct device *dev = &jdi->dsi->dev;
> -       int ret;
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
>
>         dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
>
> -       ret = mipi_dsi_dcs_set_display_off(dsi);
> -       if (ret < 0)
> -               dev_err(dev, "failed to set display off: %d\n", ret);
> -
> -       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_set_display_off_multi(&dsi_ctx);
> +       mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);

The old code still runs the "enter sleep mode" even if the "set
display off" failed. This is probably on purpose. Power down code
often ignores errors (other than reporting them) and continues on
because there's not really better choices.

In this sense, you probably want a `dsi_ctx.accum_err = 0` between the
above two commands.

> -       msleep(100);
> +       mipi_dsi_msleep(&dsi_ctx, 100);

Let's keep it as "msleep(100)". Old code would have slept here even if
there were errors, and that could possibly make sense in power off.
Let's preserve the old behavior. That makes sure even if there some
errors we sleep enough before we start turning off regulators / GPIOs.