linux-kernel - Re: [PATCH v6 12/14] drm/edp-panel: Add LG Display panel model LP079QX1-SP0V

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: <7ef0454b-fddd-44c0-be4a-c81c443f08f6@rock-chips.com>
Date: Sat, 25 Jan 2025 12:18:09 +0800
From: Damon Ding <damon.ding@...k-chips.com>
To: Doug Anderson <dianders@...omium.org>,
 Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: heiko@...ech.de, robh@...nel.org, krzk+dt@...nel.org,
 conor+dt@...nel.org, rfoss@...nel.org, vkoul@...nel.org,
 sebastian.reichel@...labora.com, cristian.ciocaltea@...labora.com,
 l.stach@...gutronix.de, andy.yan@...k-chips.com, hjc@...k-chips.com,
 algea.cao@...k-chips.com, kever.yang@...k-chips.com,
 dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org
Subject: Re: [PATCH v6 12/14] drm/edp-panel: Add LG Display panel model
 LP079QX1-SP0V

Hi Doug,

On 2025/1/24 8:30, Doug Anderson wrote:
> Hi,
> 
> On Thu, Jan 23, 2025 at 3:31 AM Dmitry Baryshkov
> <dmitry.baryshkov@...aro.org> wrote:
>>
>> On Thu, Jan 23, 2025 at 06:07:45PM +0800, Damon Ding wrote:
>>> The raw edid for LP079QX1-SP0V panel model is:
>>>
>>> 00 ff ff ff ff ff ff 00 16 83 00 00 00 00 00 00
>>> 04 17 01 00 a5 10 0c 78 06 ef 05 a3 54 4c 99 26
>>> 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01
>>> 01 01 01 01 01 01 ea 4e 00 4c 60 00 14 80 0c 10
>>> 84 00 78 a0 00 00 00 18 00 00 00 10 00 00 00 00
>>> 00 00 00 00 00 00 00 00 00 00 00 00 00 fe 00 4c
>>> 50 30 37 39 51 58 31 2d 53 50 30 56 00 00 00 fc
>>> 00 43 6f 6c 6f 72 20 4c 43 44 0a 20 20 20 00 3f
>>>
>>> Signed-off-by: Damon Ding <damon.ding@...k-chips.com>
>>> ---
>>>   drivers/gpu/drm/panel/panel-edp.c | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>
>> Please use get_maintainers.pl when compiling To/Cc lists. Added Douglas
>> to CC manually.
>>
>> Or, better, split irrelevant patches to separate series.
>>
>>>
>>> diff --git a/drivers/gpu/drm/panel/panel-edp.c b/drivers/gpu/drm/panel/panel-edp.c
>>> index f8511fe5fb0d..77e3fd3ed160 100644
>>> --- a/drivers/gpu/drm/panel/panel-edp.c
>>> +++ b/drivers/gpu/drm/panel/panel-edp.c
>>> @@ -1808,6 +1808,12 @@ static const struct panel_delay delay_200_150_e50 = {
>>>        .enable = 50,
>>>   };
>>>
>>> +static const struct panel_delay delay_50_500_e200 = {
>>> +     .hpd_absent = 50,
>>> +     .unprepare = 500,
>>> +     .enable = 200,
>>> +};
> 
> Wow, hpd_absent is 50ms. That's excellent!
> 
> I was curious if this was really correct since it's the lowest I've
> seen, but I searched the internet and found a datasheet that confirms
> it. Crazy. Although my datasheet has that whole section grayed out and
> says "Measurement not available". How worried should be about that? I
> guess if "hpd_absent" of 50 eventually doesn't work it can always be
> increased.
> 

The datasheet I have should be the same as yours, and the T3 is "Min. 
2ms | Typ. 30ms | Max. 50ms" with the gray "Measurement not available".

> Looking at that datasheet (assuming you can find the same one), I
> wonder how you handle the T4 120ms requirement. It looks like that's
> the time from tcon reset (which is nearly power on) until you're
> allowed to put valid data on the panel. I _think_ you could meet the
> requirement via setting `powered_on_to_enable` to 320 though, right?
> ...or maybe 335 to handle the maximum value of "tcon_reset" (no idea
> how you control this signal). If I remember correctly (it's been a
> while), things will already be clocking but outputting black when the
> panel's "enable" function is called. After the function is called then
> we'll turn on the backlight (and still outputting black) and
> eventually we'll put valid data. So as long as you consider the
> continued output of black as "valid data" then the timing diagram is
> satisfied.
> 

Yes, it is indeed better to set the "powered_on_to_enable" to 335ms, as 
this will help prevent the display from showing invalid black screens. I 
will add it in the next version.

> BTW: don't you also need a 200 ms "disable"?
> 

According to the datasheet, it is a good idea to set "disable" to 200ms.

> 
>>>   #define EDP_PANEL_ENTRY(vend_chr_0, vend_chr_1, vend_chr_2, product_id, _delay, _name) \
>>>   { \
>>>        .ident = { \
>>> @@ -1955,6 +1961,8 @@ static const struct edp_panel_entry edp_panels[] = {
>>>        EDP_PANEL_ENTRY('C', 'S', 'W', 0x1100, &delay_200_500_e80_d50, "MNB601LS1-1"),
>>>        EDP_PANEL_ENTRY('C', 'S', 'W', 0x1104, &delay_200_500_e50, "MNB601LS1-4"),
>>>
>>> +     EDP_PANEL_ENTRY('E', 'T', 'C', 0x0000, &delay_50_500_e200, "LP079QX1-SP0V"),
> 
> I don't love that the ID is 0x0000. That makes me nervous that the
> panel vendor isn't setting the ID properly. ...but at least the string
> matches in the EDID so hopefully that will be enough to uniquely
> identify things.
> 
> 

The ID "0x0000" makes me nervous too, but the EDID obtained from this 
panel indeed show it, which is quite surprising. May I still set it to 
"0x0000" as it really is?

Best regards,
Damon