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

[Doug Anderson]

Hi,

On Thu, Jul 6, 2023 at 2:36 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> On Thu, Jul 6, 2023 at 11:25 PM Doug Anderson <dianders@xxxxxxxxxx> wrote:
>
> > 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.
>
> This is not question about a dozen drivers, to be clear.
>
> I just want to break out the drivers that have an identifiable
> display controller that differs from the others, especially this one.
>
> The rest of the drivers inside of this boe driver I can't really tell,
> they seem related? We don't know?
>
> So the Ilitek ILI9882t is an obvious break-out.

I guess. To me it feels like the concept of breaking the driver into
multiple sub-drivers and the idea of supporting ILI9882t more cleanly
are orthogonal. You could still do your patch #4 and break out the
page switching function without breaking up the driver.

It feels to me fairly likely that many of the panels here are just as
different from each other as the ILI9882t is from them. I guess it's
not a dozen, but it feels like using the same "how different are they
from each other" metric we'd end up with at least 5-6 new drivers. It
seems clear to me that the panel that Sam first commented on is as
different from the others in the BOE driver as the ILI9882t is.
Certainly it has a pretty darn big unique command sequence for init...


> For the rest, I guess I would be happier if the Chromium people
> could use their leverage with vendors to muscle out the details
> about display controller vendors and provide #defines for all
> magic commands, we all dislike these opaque firmware-looking
> jam tables.

Yeah, I've grumbled about this with each new blob added. For instance:

https://lore.kernel.org/r/CAD=FV=Uo-7rFWGiJG0oJ0ydosA4DxhFqiWGrab1zoZyxyPsOBg@xxxxxxxxxxxxxx/

Where I said:

> I'm not really an expert on
> MIPI panels but the convention of a big stream of binary commands
> seems to match what other panels in this driver do, even if their
> table of binary data isn't quite as long as yours (are all of yours
> actually needed?)

The problem is that it's hard for me to make a strong argument here
when there is prior art of panels being supported with blob-sequences.
In this case, I think you as an upstream developer have more leverage.
I can help put pressure to make sure that upstream concerns are
addressed, but I think it's on upstream to put their foot down and say
that these blob sequences are not OK for new panels. In each case I
landed a patch with a new blob sequence I tried to give the community
time to respond and I tried to telegraph what I was going to do to
make sure nobody was surprised...

-Doug