Re: [v4 4/4] drm/panel: Support for Starry-ili9882t TDDI MIPI-DSI panel

From: Doug Anderson
Date: Thu Jul 06 2023 - 17:25:37 EST


Hi,

On Tue, Jul 4, 2023 at 12:47 AM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> On Thu, Jun 1, 2023 at 5:55 PM Doug Anderson <dianders@xxxxxxxxxx> wrote:
> > On Thu, May 25, 2023 at 2:32 AM Cong Yang
> > <yangcong5@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > The Starry-ili9882 is a 10.51" WUXGA TFT panel. which fits in nicely with
> > > the existing panel-boe-tv101wum-nl6 driver. From the datasheet,MIPI need
> > > to keep the LP11 state before the lcm_reset pin is pulled high. So add
> > > lp11_before_reset flag.
> > >
> > > Signed-off-by: Cong Yang <yangcong5@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> > > Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> > > ---
> > > .../gpu/drm/panel/panel-boe-tv101wum-nl6.c | 371 ++++++++++++++++++
> > > 1 file changed, 371 insertions(+)
> >
> > Applied to drm-misc-next:
> >
> > 8716a6473e6c drm/panel: Support for Starry-ili9882t TDDI MIPI-DSI panel
>
> Sorry for noticing too late and coming after the fact and complaining.
>
> We must stop using the panel-boe-tv101wum-nl6.c driver as a
> one-stop-shop for Chromium panels. The Starry panel in particular
> hardware-wise has nothing in common with the other panels in this
> driver and I'm suspicious about patch 3/4 as well.
>
> Please check my patch breaking it out to a separate driver, and
> if you could check internally if you have a datasheet for Ilitek
> ILI9882t or can use your vendor leverage to get one to improve
> on the driver (such as define the DCS commands...) that would
> be great.
>
> There are good reasons for grouping the panel drivers into
> respective display controller such as fixing bugs in one place
> and if we ever want to properly support things such as
> gamma correction it will provide the proper per-display-controller
> approach.

I mentioned in response to your patch #3 also [1], but closing the
loop here as well. The original reason several panels all ended up in
one driver was in response to Sam's feedback [2]. That was even
documented when the first of the "Chromium" panels landed in commit
93ee1a2c0f08 ("drm/panel: support for BOE and INX video mode panel").

In my mind it's really a tradeoff and I'm happy to go with whatever
consensus that others agree with. What I'm never super happy with,
though, is changing the bikeshed color too often, so I'd be really
curious to hear Sam's thoughts on the issue and whether he'd like to
see this driver broken out into a dozen drivers.

[1] https://lore.kernel.org/r/CAD=FV=Xkr3Qpd8m_6Xta_2jL_ezbxsmMyarbKXTXL+UJLG9xNw@xxxxxxxxxxxxxx
[2] https://lore.kernel.org/all/YSPAseE6WD8dDRuz@xxxxxxxxxxxx/