Hi,
On Thu, Jan 23, 2025 at 3:31 AM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> > ---
> > 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.
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.
BTW: don't you also need a 200 ms "disable"?
> > #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.
