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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a804b8eb-3c79-4753-670c-cd5c2dfbc3a4@samsung.com>
Date:   Mon, 22 Feb 2021 08:20:33 +0100
From:   Andrzej Hajda <a.hajda@...sung.com>
To:     Nicolas Boichat <drinkcat@...omium.org>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:     Neil Armstrong <narmstrong@...libre.com>,
        David Airlie <airlied@...ux.ie>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Sam Ravnborg <sam@...nborg.org>,
        Emil Velikov <emil.velikov@...labora.com>,
        linux-samsung-soc@...r.kernel.org,
        Joonyoung Shim <jy0922.shim@...sung.com>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        Chun-Kuang Hu <chunkuang.hu@...nel.org>,
        Jonas Karlman <jonas@...boo.se>, linux-arm-msm@...r.kernel.org,
        "moderated list:ARM/Mediatek SoC support" 
        <linux-mediatek@...ts.infradead.org>,
        Rikard Falkeborn <rikard.falkeborn@...il.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Sean Paul <sean@...rly.run>, Xin Ji <xji@...logixsemi.com>,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        Seung-Woo Kim <sw0312.kim@...sung.com>,
        lkml <linux-kernel@...r.kernel.org>,
        Robert Foss <robert.foss@...aro.org>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        freedreno@...ts.freedesktop.org
Subject: Re: [PATCH] drm/dsi: Add _NO_ to MIPI_DSI_* flags disabling
 features

Hi Nicolas,

W dniu 22.02.2021 o 06:31, Nicolas Boichat pisze:
> 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.


Reviewed-by: Andrzej Hajda <andrzej.hajda@...sung.com>

And comment at the end.

>>
>>> ---
>>> 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?

Hehe, "real DSI expert" :), ok I've read spec few times :)

If I remember correctly the spec did not prioritizes these modes, specs 
are publicly available if somebody want to check it feel free.

These values were taken from defaults for Exynos DSI, as nobody at the 
time has better idea.

We could try to optimize it by looking for example in different dsi 
hosts defaults, or maybe dsi devices, but I am not sure if it is worth time.

This solution is good for me.


Regards

Andrzej


>
>
>>>   /* 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
> _______________________________________________
> dri-devel mailing list
> dri-devel@...ts.freedesktop.org
> https://protect2.fireeye.com/v1/url?k=e6f0d6d2-b96befef-e6f15d9d-0cc47a31309a-f4be6a0935319c2d&q=1&e=5e175166-1972-4f28-a483-e9a65c07e25f&u=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ