Message-ID: <CAD=FV=V=K9L=bJiNvFJ+K_DHUTPxA4WtukXA+E_VW6uihE8kdQ@mail.gmail.com>
Date:   Thu, 7 Dec 2023 11:14:34 -0800
From:   Doug Anderson <dianders@...omium.org>
To:     Maxime Ripard <mripard@...nel.org>
Cc:     Pin-yen Lin <treapking@...omium.org>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Jessica Zhang <quic_jesszhan@...cinc.com>,
        Sam Ravnborg <sam@...nborg.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>, linux-kernel@...r.kernel.org,
        Guenter Roeck <groeck@...omium.org>,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH v2 4/4] drm/panel-edp: Add some panels with conservative timings

Hi,

On Thu, Dec 7, 2023 at 10:58 AM Maxime Ripard <mripard@...nel.org> wrote:
>
> On Thu, Dec 07, 2023 at 10:23:53AM -0800, Doug Anderson wrote:
> > Hi,
> >
> > On Thu, Dec 7, 2023 at 12:18 AM Pin-yen Lin <treapking@...omium.org> wrote:
> > >
> > > These panels are used by Mediatek MT8173 Chromebooks but we can't find
> > > the corresponding data sheets, and these panels used to work on v4.19
> > > kernel without any specified delays.
> > >
> > > Therefore, instead of having them use the default conservative timings,
> > > update them with less-conservative timings from other panels of the same
> > > vendor. The panels should still work under those timings, and we can
> > > save some delays and suppress the warnings.
> > >
> > > Signed-off-by: Pin-yen Lin <treapking@...omium.org>
> > >
> > > ---
> > >
> > > (no changes since v1)
> > >
> > >  drivers/gpu/drm/panel/panel-edp.c | 31 +++++++++++++++++++++++++++++++
> > >  1 file changed, 31 insertions(+)
> >
> > Reviewed-by: Douglas Anderson <dianders@...omium.org>
> >
> > Repeating my comments from v1 here too, since I expect this patch to
> > sit on the lists for a little while:
> >
> >
> > This is OK w/ me, but it will need time on the mailing lists before
> > landing in case anyone else has opinions.
>
> Generally speaking, I'm not really a fan of big patches that dump
> whatever ChromeOS is doing ...
>
> > Specifical thoughts:
> >
> > * I at least feel fairly confident that this is OK since these panels
> > essentially booted without _any_ delays back on the old downstream
> > v4.19 kernel. Presumably the panels just had fairly robust timing
> > controllers and so worked OK, but it's better to get the timing more
> > correct.
>
> ... especially since you have to rely on the recollection of engineers
> involved at the time and you have no real way to test and make things
> clearer anymore, and we have to take patches in that are handwavy "trust
> us, it's doing the right thing".
>
> I'd really prefer to have these patches sent as they are found out.

It's probably not clear enough from the commit message, but this isn't
just a dump from downstream 4.19. What happened was:

1. Downstream chromeos-4.19 used the "little white lie" approach. They
all claimed a specific panel's compatible string even though there
were a whole pile of panels out there actually being used. Personally,
this was not something I was ever a fan of (and I wasn't personally
involved in this project), but it was the "state of the art" before
the generic panel-edp. Getting out of the "little white lie" situation
was why I spent so much time on the generic edp-panel solution
upstream.

2. These devices have now been uprevved to a newer kernel and I
believe that there were issues seen that necessitated a move to the
proper generic panel-edp code.

3. We are now getting field reports from our warning collector about a
whole pile of panels that are falling back to the "conservative"
timings, which means that they turn on/off much more slowly than they
should.

Pin-yen made an attempt to search for panels data sheets that matched
up with the IDs that came in from the field reports but there were
some panels that he just couldn't find.

So basically we're stuck. Options:

1. Leave customers who have these panels stuck with the hardware
behaving worse than it used to. This is not acceptable to me.

2. Land Pin-yen's patch as a downstream-only patch in ChromeOS. This
would solve the problem, but it would make me sad. If anyone ever
wants to take these old laptops and run some other Linux distribution
on them (and there are several that target old Chromebooks) then
they'd be again stuck with old timings.

3. Land a patch like this one that at least gets us into not such a bad shape.

While I don't love this patch (and that's why I made it clear that it
needs to spend time on the list), it seems better than the
alternatives. Do you have a proposal for something else? If not, can
you confirm you're OK with #3 given this explanation? ...and perhaps
more details in the commit message?


I would also note that, hopefully, patches like this shouldn't be a
recurring pattern. Any new Chromebooks using panel-edp will get
flagged much earlier and we should be able to get real/proper timings
added. I believe that we've also added a factory test so that
(assuming it doesn't get ignored by someone) devices that aren't
supported don't even make it out of the factory.

> Also, the fact that the 4.19 kernel mentionned in the commit log is
> actually a downstream tree needs to be made much clearer.

Yeah, that would help too.

Powered by blists - more mailing lists