linux-kernel - Re: [PATCH 1/2] drm/mipi-dsi: Replace mipi_dsi_dcs_set_tear

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=URp_SJ8RBhwgX1sW22EvscMA9y1OZUSu_F-79DrTFRXg@mail.gmail.com>
Date: Fri, 14 Feb 2025 16:42:14 -0800
From: Doug Anderson <dianders@...omium.org>
To: Tejas Vipin <tejasvipin76@...il.com>
Cc: neil.armstrong@...aro.org, maarten.lankhorst@...ux.intel.com, 
	mripard@...nel.org, tzimmermann@...e.de, airlied@...il.com, simona@...ll.ch, 
	quic_jesszhan@...cinc.com, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] drm/mipi-dsi: Replace mipi_dsi_dcs_set_tear_off with
 its multi version

Hi,

On Fri, Feb 14, 2025 at 9:30 AM Tejas Vipin <tejasvipin76@...il.com> wrote:
>
> mipi_dsi_dcs_set_tear_off can heavily benefit from being converted
> to a multi style function as it is often called in the context of
> similar functions.

Given that it has one caller, the wording "heavily benefit" and "often
called" is a bit of a stretch.

> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -346,7 +346,6 @@ int mipi_dsi_dcs_set_column_address(struct mipi_dsi_device *dsi, u16 start,
>                                     u16 end);
>  int mipi_dsi_dcs_set_page_address(struct mipi_dsi_device *dsi, u16 start,
>                                   u16 end);
> -int mipi_dsi_dcs_set_tear_off(struct mipi_dsi_device *dsi);
>  int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi,
>                              enum mipi_dsi_dcs_tear_mode mode);
>  int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format);
> @@ -379,6 +378,7 @@ void mipi_dsi_dcs_set_page_address_multi(struct mipi_dsi_multi_context *ctx,
>                                          u16 start, u16 end);
>  void mipi_dsi_dcs_set_tear_scanline_multi(struct mipi_dsi_multi_context *ctx,
>                                           u16 scanline);
> +void mipi_dsi_dcs_set_tear_off_multi(struct mipi_dsi_multi_context *ctx);

This patch can't land as-is since it breaks bisection. In other words,
if someone has the first patch but not the second then it won't
compile because there will still be a user of
mipi_dsi_dcs_set_tear_off() but you've removed it. If they have the
second patch but not the first then it won't compile because
mipi_dsi_dcs_set_tear_off_multi() hasn't been introduced yet. You have
two options:

1. Turn your 2 patches into 3 patches. The first patch would need to
still provide the old function, the second patch would stay as-is, and
the third patch would remove the wrapper.

2. Just squash the two patches together.

If I were picking, I'd pick #2. While it's nice to separate out
patches, this is not a very complex case and adding code just to
delete it two patches later is a bit silly. That being said, it's a
tradeoff so if someone else has strong opinions I wouldn't object to
taking path #1.

-Doug