Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
MIME-Version: 1.0
References: <20230703-fix-boe-tv101wum-nl6-v3-0-bd6e9432c755@linaro.org>
 <CAD=FV=W2AKQSnWh02Lxbqi47M325JNCaEn1_B0xAW3PKPKKF7Q@mail.gmail.com>
 <CAD=FV=WZ+zX9jrwOhN_ZboSYSBomx2s5vifQEB6MzNFgj=g4Ow@mail.gmail.com> <CACRpkdZutdTDQkCSQoA0_0U=Qdf+xDWTu3PzvYRRz=cT18wHRA@mail.gmail.com>
In-Reply-To: <CACRpkdZutdTDQkCSQoA0_0U=Qdf+xDWTu3PzvYRRz=cT18wHRA@mail.gmail.com>
From:   Doug Anderson <dianders@chromium.org>
Date:   Fri, 29 Sep 2023 07:17:22 -0700
Message-ID: <CAD=FV=XG1yFfbvAh9t6XiMNKBc3kHNZ8FqVjayboQjuTfxt-6A@mail.gmail.com>
Subject: Re: [PATCH v3 0/4] Fix up the boe-tv101wum-nl6 panel driver
To:     Linus Walleij <linus.walleij@linaro.org>
Cc:     Ruihai Zhou <zhouruihai@huaqin.corp-partner.google.com>,
        Stephen Boyd <swboyd@chromium.org>,
        Cong Yang <yangcong5@huaqin.corp-partner.google.com>,
        Jitao Shi <jitao.shi@mediatek.com>,
        Neil Armstrong <neil.armstrong@linaro.org>,
        Sam Ravnborg <sam@ravnborg.org>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi,

On Thu, Sep 28, 2023 at 2:42=E2=80=AFPM Linus Walleij <linus.walleij@linaro=
.org> wrote:
>
> On Tue, Sep 26, 2023 at 11:49=E2=80=AFPM Doug Anderson <dianders@chromium=
.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=3DFV=3DXkr3Qpd8m_6Xta_2jL_ezbxsmMya=
rbKXTXL+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=3DFV=3DWju3WS45=3DEpXMUg7FjYDh3-=3D=
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