linux-kernel - Re: [PATCH 2/2] drm/panel: Add driver for Visionox r66451 panel

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: <a5b7d094-3e2a-f5e3-0c1e-90e95a3b7715@quicinc.com>
Date:   Tue, 30 May 2023 10:43:44 -0700
From:   Jessica Zhang <quic_jesszhan@...cinc.com>
To:     Marijn Suijten <marijn.suijten@...ainline.org>,
        Neil Armstrong <neil.armstrong@...aro.org>
CC:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        <devicetree@...r.kernel.org>, Sam Ravnborg <sam@...nborg.org>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>,
        <dri-devel@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>,
        Rob Herring <robh+dt@...nel.org>,
        "Krzysztof Kozlowski" <krzysztof.kozlowski+dt@...aro.org>
Subject: Re: [PATCH 2/2] drm/panel: Add driver for Visionox r66451 panel



On 5/29/2023 5:09 AM, Marijn Suijten wrote:
> On 2023-05-26 09:32:45, Neil Armstrong wrote:
> <snip>
>>>>>>> +static int visionox_r66451_bl_update_status(struct backlight_device *bl)
>>>>>>> +{
>>>>>>> +    struct mipi_dsi_device *dsi = bl_get_data(bl);
>>>>>>> +    u16 brightness = backlight_get_brightness(bl);
>>>>>>> +
>>>>>>> +    return mipi_dsi_dcs_set_display_brightness(dsi, cpu_to_le16(brightness));
>>>>>>
>>>>>> mipi_dsi_dcs_set_display_brightness() already converts the brightness,
>>>>>> so you don't need cpu_to_le16 here.
>>>>>
>>>>> Tread carefully here: we've had the same issue and conversation on our
>>>>> Sony panels where this extra inversion is required.
>>>>> set_display_brightness() sends the bytes as little-endian to the panel
>>>>> (and it even assumes little-endian in get_display_brightness()) but the
>>>>> spec for 16-bit brightness values states that they have to be sent in
>>>>> big-endian.  This is why c9d27c6be518b ("drm/mipi-dsi: Fix byte order of
>>>>> 16-bit DCS set/get brightness") added
>>>>> mipi_dsi_dcs_set_display_brightness_large().
>>>>>
>>>>> Jessica, if you need to have the endian swap here (should be very easy
>>>>> to test with a real panel, but it should be given the max_brightness
>>>>> value being over 8 bits) please switch to the _large() variant.
>>>>
>>>> Got it, thanks for the heads up!
>>>
>>> Hi Marijn,
>>>
>>> Just wanted to update this thread -- I've checked the backlight brightness values in the sysfs and it matches the value being given in the panel driver (255), so I think it should be fine to use *_set_display_brightness() without the _large() variant.
>>
>> Sure, I was also misleaded by you using cpu_to_le16() but as Dmitry said it's already
>> done in mipi_dsi_dcs_set_display_brightness() and a no-op on LE arm64 platforms anyway.
> 
> Yuck, right, it's cpu_to_le16 here and not cpu_to_be16.  @Jessica, can
> you please remove this misleading conversion?
> mipi_dsi_dcs_set_display_brightness() takes a native u16, not a specific
> __le16.

Hi Marijn,

Acked, will drop the conversion.

Thanks,

Jessica Zhang

> 
> - Marijn