Message-ID: <CAD=FV=XG1yFfbvAh9t6XiMNKBc3kHNZ8FqVjayboQjuTfxt-6A@mail.gmail.com>
Date:   Fri, 29 Sep 2023 07:17:22 -0700
From:   Doug Anderson <dianders@...omium.org>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Ruihai Zhou <zhouruihai@...qin.corp-partner.google.com>,
        Stephen Boyd <swboyd@...omium.org>,
        Cong Yang <yangcong5@...qin.corp-partner.google.com>,
        Jitao Shi <jitao.shi@...iatek.com>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Sam Ravnborg <sam@...nborg.org>,
        David Airlie <airlied@...il.com>,
        Daniel Vetter <daniel@...ll.ch>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 0/4] Fix up the boe-tv101wum-nl6 panel driver

Hi,

On Thu, Sep 28, 2023 at 2:42 PM Linus Walleij <linus.walleij@...aro.org> wrote:
>
> On Tue, Sep 26, 2023 at 11:49 PM Doug Anderson <dianders@...omium.org> wrote:
>
> > > I'm curious what the latest on this patch series is. Is it abandoned,
> > > or is it still on your list to move forward with it? If it's
> > > abandoned, does that mean we've abandoned the idea of breaking
> > > ili9882t into a separate driver?
> > >
> > > From looking at things that have landed downstream in the ChromeOS
> > > kernel trees it looks as if additional fixes are getting blocked from
> > > being posted/landed because of the limbo state that this is in.
> >
> > I presume Linus is busy or otherwise indisposed.
>
> Sorry I was looking for the branch with my patches and I have it
> somewhere not ordinary :/
>
> Originally I shelved it because I got requests to do additional
> patches to the driver:
> https://lore.kernel.org/dri-devel/CAD=FV=Xkr3Qpd8m_6Xta_2jL_ezbxsmMyarbKXTXL+UJLG9xNw@mail.gmail.com/
>
> To do measurements about binary code size in object files, and if it does,
> then I need to invent new sequence macros (IIUC):
> https://lore.kernel.org/dri-devel/CAD=FV=Wju3WS45=EpXMUg7FjYDh3-=mvm_jS7TF1tsaAzbb4Uw@mail.gmail.com/
>
> So I just didn't have time for that extensive rework of the driver.
>
> It's good feedback, but I just wanted to make the situation a little
> bit better, and perfect is the enemy of good (TM).
>
> > So I guess we have two options here:
> >
> > a) Cong Yang can post any relevant fixes to the existing "monolithic"
> > panel driver so that we can get them landed and at least get things
> > fixed.
> >
> > - or -
> >
> > b) Cong Yang could take over all or some of Linus's series and post
> > new versions of it, addressing feedback.
>
> Either works for me, I would prefer b), Cong is welcome to adopt
> the patches if he/his employer want to. Go ahead!
>
> We can't really let this one-size-fits-all driver go on like this.
>
> My main concern with the "boe-tv101wum-nl6" driver is that it can
> be renamed "cromeos-hackfest" at this point because it becomes
> hard for any other system to reuse the panel drivers, the typical
> example would be a system using say ili9882t but with
> a different init sequence or something, why would they want
> support for 9 unrelated panels compiled in? The condition that
> these drivers should be related to the original panel that gave
> name to the file has seemingly been dropped long ago.
>
> It looks like the drivers only share the power lines (avdd, avee, pp3300,
> pp1800) then this can be broken out to a helper library. But I am
> sceptical about that too. I doubt that the vastly different panels
> actually have exactly these these supply line names, I think it is
> actually names of the rails on the chrome machine board. And that is
> not how these regulators should be named, they should be named after
> the input name on the component. This is really hard to catch in reviews when
> we don't have datasheets so I'm not blaming anyone, but is this something
> that even needs fixing in the device tree bindings? (By deprecation
> and addition...) can we look into this?
>
> I would say can we at least agree that before we merge one more
> driver into this file, break out to subdrivers those that clearly have
> an identifiable display controller and is thus reusable? From my
> point of view I can just see the ili9882t so that's a good start.

This sounds like a reasonable plan to me. What if Cong posted patches
that broke this up into a separate driver for the distinct controller
but otherwise didn't substantially reorganize it? In other words both
the old driver and the new one would keep the "struct panel_init_cmd"
until we get some resolution about the binary size issue. That would
at least let us move forward...

-Doug

Powered by blists - more mailing lists