linux-kernel - Re: [PATCH v4 1/4] drm: Create mipi_dsi

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD=FV=WYBFrm-J55BTEJ7s=Jk4EFuMVAkahVZfdzW6V8mxE7Tg@mail.gmail.com>
Date: Fri, 18 Jul 2025 09:10:21 -0700
From: Doug Anderson <dianders@...omium.org>
To: Brigham Campbell <me@...ghamcampbell.com>
Cc: tejasvipin76@...il.com, diogo.ivo@...nico.ulisboa.pt, 
	skhan@...uxfoundation.org, linux-kernel-mentees@...ts.linux.dev, 
	dri-devel@...ts.freedesktop.org, linux-doc@...r.kernel.org, 
	linux-kernel@...r.kernel.org, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>
Subject: Re: [PATCH v4 1/4] drm: Create mipi_dsi_dual macro

Hi,

On Thu, Jul 17, 2025 at 9:41 AM Brigham Campbell <me@...ghamcampbell.com> wrote:
>
> Create mipi_dsi_dual macro for panels which are driven by two parallel
> serial interfaces. This allows for the reduction of code duplication in
> drivers for these panels.
>
> Signed-off-by: Brigham Campbell <me@...ghamcampbell.com>
> ---
>  include/drm/drm_mipi_dsi.h | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 369b0d8830c3..03199c966ea5 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -431,6 +431,30 @@ void mipi_dsi_dcs_set_tear_off_multi(struct mipi_dsi_multi_context *ctx);
>                 mipi_dsi_dcs_write_buffer_multi(ctx, d, ARRAY_SIZE(d)); \
>         } while (0)
>
> +/**
> + * mipi_dsi_dual - send the same MIPI DSI command to two interfaces
> + *
> + * This macro will send the specified MIPI DSI command twice, once per each of
> + * the two interfaces supplied. This is useful for reducing duplication of code
> + * in panel drivers which use two parallel serial interfaces.
> + *
> + * @_func: MIPI DSI function or macro to pass context and arguments into
> + * @_dsi1: First DSI interface to act as recipient of the MIPI DSI command
> + * @_dsi2: Second DSI interface to act as recipient of the MIPI DSI command
> + * @_ctx: Context for multiple DSI transactions
> + * @...: Arguments to pass to MIPI DSI function or macro
> + */
> +#define mipi_dsi_dual(_func, _dsi1, _dsi2, _ctx, ...)           \
> +       _mipi_dsi_dual(_func, _dsi1, _dsi2, _ctx, ##__VA_ARGS__)
> +
> +#define _mipi_dsi_dual(_func, _dsi1, _dsi2, _ctx, ...) \
> +       do {                                           \
> +               (_ctx)->dsi = (_dsi1);                 \
> +               _func((_ctx), ##__VA_ARGS__);          \

nit: shouldn't func be in parenthesis for safety? It's unlikely to
really matter, but just in case it's somehow a calculated value that
would make it safe from an order-of-operations point of view.


> +               (_ctx)->dsi = (_dsi2);                 \
> +               _func((_ctx), ##__VA_ARGS__);          \
> +       } while (0)

Can you explain why you need the extra level of indirection here (in
other words, why do you need to define _mipi_dsi_dual() and then use
it in mipi_dsi_dual())? I don't see it buying anything, but maybe it's
performing some magic trick I'm not aware of?

Reading this with a fresh set of eyes, I also realize that this macro
is probably vulnerable to issues where arguments have side effects
since we have to repeat them. I don't think there's a way around this
and I think the macro is still worthwhile, but something to go into
with open eyes. Possibly worth noting in the macro description? We
could probably at least eliminate the need to reference "_ctx" more
than once by assigning it to a local variable. I think referencing
"_func" and "__VA_ARGS__" more than once is unavoidable...

Maybe this is what you were trying to solve with the extra level of
indirection, but (at least with my knowledge of the C preprocessor), I
don't think it does.


-Doug