| 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
| ||
|
Message-ID: <CANMq1KALq+C2GD2uRohKpwvkDC05-fHyo=_WoHwnsKNjgcSfEQ@mail.gmail.com>
Date: Mon, 22 Feb 2021 13:31:17 +0800
From: Nicolas Boichat <drinkcat@...omium.org>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Andrzej Hajda <a.hajda@...sung.com>,
Robert Foss <robert.foss@...aro.org>,
Chun-Kuang Hu <chunkuang.hu@...nel.org>,
Daniel Vetter <daniel@...ll.ch>,
David Airlie <airlied@...ux.ie>,
Emil Velikov <emil.velikov@...labora.com>,
Inki Dae <inki.dae@...sung.com>,
Jernej Skrabec <jernej.skrabec@...l.net>,
Jonas Karlman <jonas@...boo.se>,
Joonyoung Shim <jy0922.shim@...sung.com>,
Jordan Crouse <jcrouse@...eaurora.org>,
Krzysztof Kozlowski <krzk@...nel.org>,
Kyungmin Park <kyungmin.park@...sung.com>,
Linus Walleij <linus.walleij@...aro.org>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Matthias Brugger <matthias.bgg@...il.com>,
Maxime Ripard <mripard@...nel.org>,
Neil Armstrong <narmstrong@...libre.com>,
Philipp Zabel <p.zabel@...gutronix.de>,
Rajendra Nayak <rnayak@...eaurora.org>,
Rikard Falkeborn <rikard.falkeborn@...il.com>,
Rob Clark <robdclark@...il.com>,
Sam Ravnborg <sam@...nborg.org>, Sean Paul <sean@...rly.run>,
Seung-Woo Kim <sw0312.kim@...sung.com>,
Thierry Reding <thierry.reding@...il.com>,
Thomas Zimmermann <tzimmermann@...e.de>,
Viresh Kumar <viresh.kumar@...aro.org>,
Xin Ji <xji@...logixsemi.com>,
dri-devel <dri-devel@...ts.freedesktop.org>,
freedreno@...ts.freedesktop.org,
linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
linux-arm-msm@...r.kernel.org, lkml <linux-kernel@...r.kernel.org>,
"moderated list:ARM/Mediatek SoC support"
<linux-mediatek@...ts.infradead.org>,
linux-samsung-soc@...r.kernel.org
Subject: Re: [PATCH] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling features
On Mon, Feb 22, 2021 at 3:08 AM Laurent Pinchart
<laurent.pinchart@...asonboard.com> wrote:
>
> Hi Nicolas,
>
> Thank you for the patch.
>
> On Thu, Feb 11, 2021 at 11:33:55AM +0800, Nicolas Boichat wrote:
> > Many of the DSI flags have names opposite to their actual effects,
> > e.g. MIPI_DSI_MODE_EOT_PACKET means that EoT packets will actually
> > be disabled. Fix this by including _NO_ in the flag names, e.g.
> > MIPI_DSI_MODE_NO_EOT_PACKET.
> >
> > Signed-off-by: Nicolas Boichat <drinkcat@...omium.org>
>
> This looks good to me, it increases readability.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
>
> Please however see the end of the mail for a comment.
>
> > ---
> > I considered adding _DISABLE_ instead, but that'd make the
> > flag names a big too long.
> >
> > Generated with:
> > flag=MIPI_DSI_MODE_VIDEO_HFP; git grep $flag | cut -f1 -d':' | \
> > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HFP/" {}
> > flag=MIPI_DSI_MODE_VIDEO_HBP; git grep $flag | cut -f1 -d':' | \
> > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HBP/" {}
> > flag=MIPI_DSI_MODE_VIDEO_HSA; git grep $flag | cut -f1 -d':' | \
> > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_VIDEO_NO_HSA/" {}
> > flag=MIPI_DSI_MODE_EOT_PACKET; git grep $flag | cut -f1 -d':' | \
> > xargs -I{} sed -i -e "s/$flag/MIPI_DSI_MODE_NO_EOT_PACKET/" {}
> > (then minor format changes)
>
> Ever tried coccinelle ? :-)
Fun project for next time ,-)
>
> > drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +-
> > drivers/gpu/drm/bridge/analogix/anx7625.c | 2 +-
> > drivers/gpu/drm/bridge/cdns-dsi.c | 4 ++--
> > drivers/gpu/drm/bridge/tc358768.c | 2 +-
> > drivers/gpu/drm/exynos/exynos_drm_dsi.c | 8 ++++----
> > drivers/gpu/drm/mcde/mcde_dsi.c | 2 +-
> > drivers/gpu/drm/mediatek/mtk_dsi.c | 2 +-
> > drivers/gpu/drm/msm/dsi/dsi_host.c | 8 ++++----
> > drivers/gpu/drm/panel/panel-asus-z00t-tm5p5-n35596.c | 2 +-
> > drivers/gpu/drm/panel/panel-dsi-cm.c | 2 +-
> > drivers/gpu/drm/panel/panel-elida-kd35t133.c | 2 +-
> > drivers/gpu/drm/panel/panel-khadas-ts050.c | 2 +-
> > drivers/gpu/drm/panel/panel-leadtek-ltk050h3146w.c | 2 +-
> > drivers/gpu/drm/panel/panel-leadtek-ltk500hd1829.c | 2 +-
> > drivers/gpu/drm/panel/panel-novatek-nt35510.c | 2 +-
> > drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 2 +-
> > drivers/gpu/drm/panel/panel-samsung-s6d16d0.c | 2 +-
> > drivers/gpu/drm/panel/panel-samsung-s6e63j0x03.c | 2 +-
> > drivers/gpu/drm/panel/panel-samsung-s6e63m0-dsi.c | 2 +-
> > drivers/gpu/drm/panel/panel-samsung-s6e8aa0.c | 4 ++--
> > drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 2 +-
> > drivers/gpu/drm/panel/panel-simple.c | 2 +-
> > drivers/gpu/drm/panel/panel-sony-acx424akp.c | 2 +-
> > drivers/gpu/drm/panel/panel-xinpeng-xpp055c272.c | 2 +-
> > include/drm/drm_mipi_dsi.h | 8 ++++----
> > 25 files changed, 36 insertions(+), 36 deletions(-)
> >
> > []
> > diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> > index 360e6377e84b..ba91cf22af51 100644
> > --- a/include/drm/drm_mipi_dsi.h
> > +++ b/include/drm/drm_mipi_dsi.h
> > @@ -119,15 +119,15 @@ struct mipi_dsi_host *of_find_mipi_dsi_host_by_node(struct device_node *node);
> > /* enable hsync-end packets in vsync-pulse and v-porch area */
> > #define MIPI_DSI_MODE_VIDEO_HSE BIT(4)
>
> We're mixing bits that enable a feature and bits that disable a feature.
> Are these bits defined in the DSI spec, or internal to DRM ? In the
> latter case, would it make sense to standardize on one "polarity" ? That
> would be a more intrusive change in drivers though.
Yes, that'd require auditing every single code path and reverse the
logic as needed. I'm not volunteering for that ,-P (hopefully the
current change is still an improvement).
Hopefully real DSI experts can comment (Andrzej?), I think the default
are sensible settings?
>
> > /* disable hfront-porch area */
> > -#define MIPI_DSI_MODE_VIDEO_HFP BIT(5)
> > +#define MIPI_DSI_MODE_VIDEO_NO_HFP BIT(5)
> > /* disable hback-porch area */
> > -#define MIPI_DSI_MODE_VIDEO_HBP BIT(6)
> > +#define MIPI_DSI_MODE_VIDEO_NO_HBP BIT(6)
> > /* disable hsync-active area */
> > -#define MIPI_DSI_MODE_VIDEO_HSA BIT(7)
> > +#define MIPI_DSI_MODE_VIDEO_NO_HSA BIT(7)
> > /* flush display FIFO on vsync pulse */
> > #define MIPI_DSI_MODE_VSYNC_FLUSH BIT(8)
> > /* disable EoT packets in HS mode */
> > -#define MIPI_DSI_MODE_EOT_PACKET BIT(9)
> > +#define MIPI_DSI_MODE_NO_EOT_PACKET BIT(9)
> > /* device supports non-continuous clock behavior (DSI spec 5.6.1) */
> > #define MIPI_DSI_CLOCK_NON_CONTINUOUS BIT(10)
> > /* transmit data in low power */
>
> --
> Regards,
>
> Laurent Pinchart
Powered by blists - more mailing lists