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
[off-list ref] wrote:

quoted

On Thu, Jan 23, 2025 at 06:07:45PM +0800, Damon Ding wrote:

quoted

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 <redacted>
---
  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.

quoted

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.

quoted

quoted

  #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