Re: [PATCH v1 6/6] drm/panel: Add Hitachi TX10D07VM0BAA and LG LH400WV3-SD04 MIPI DBI panel driver

[Thierry Reding <thierry.reding@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4239 bytes --]

On Wed, Sep 30, 2026 at 01:48:02PM +0300, Svyatoslav Ryhel wrote:
> ср, 30 вер. 2026 р. о 13:43 Thierry Reding <thierry.reding@kernel.org> пише:
> >
> > On Wed, Sep 30, 2026 at 01:34:19PM +0300, Svyatoslav Ryhel wrote:
> > > ср, 30 вер. 2026 р. о 13:23 Thierry Reding <thierry.reding@kernel.org> пише:
> > > >
> > > > On Wed, Sep 30, 2026 at 12:08:41PM +0300, Svyatoslav Ryhel wrote:
> > > > > ср, 30 вер. 2026 р. о 12:02 Thierry Reding <thierry.reding@kernel.org> пише:
> > > > > > On Wed, Sep 30, 2026 at 10:05:35AM +0300, Svyatoslav Ryhel wrote:
> > > > [...]
> > > > > > > +static const struct of_device_id panel_dbi_of_match[] = {
> > > > > > > +     { .compatible = "hit,tx10d07vm0baa", .data = (void *)PANEL_DBI_TX10D07VM0BAA },
> > > > > > > +     { .compatible = "lg,lh400wv3-sd04", .data = (void *)PANEL_DBI_LH400WV3 },
> > > > > >
> > > > > > Why the detour through that PANEL_DB_* enum? You could just pass the
> > > > > > panel funcs pointers directly via .data here.
> > > > > >
> > > > >
> > > > > Passing API/OPS via .data is discouraged.
> > > >
> > > > No it's not. We do it all the time.
> > > >
> > >
> > > I have had some controversial experience with MFD, with passing cell
> > > composition. If DRM subsystem allows this, I am more then happy to
> > > pass panel ops directly.
> > >
> > > > > > Also, looking at the enable/disable sequences these are in fact two
> > > > > > different drivers, with the only commonality being that they happen to
> > > > > > be used in the same device. Rolling them both into one driver seems a
> > > > > > bit odd.
> > > > >
> > > > > I did this to simplify maintainance. Both panels are used in the LG
> > > > > Optimus 2X. My assumption is that LG switched one to another at some
> > > > > point, hence they share same timings, controls and supplies, but
> > > > > differ in en/disable sequence. Additionally, these are the the only
> > > > > DBI Type B-only panels in the kernel, from what I can see.
> > > >
> > > > That's just one more reason to put them into more of a generic driver,
> > > > which would allow people to find it and extend/improve it as needed.
> > > >
> > >
> > > This driver cannot be "generic". Both panels don't fall into any
> > > category of being "generic". They are grouped solely cause they share
> > > same timings, controls and supplies (only these 2 panels, other will
> > > definitely differ) and are used in the same device. I have no problems
> > > in splitting them into 2 distinct drivers.
> >
> > The driver can still be mostly generic. And I suspect that the panels
> > might have different names for the supplies and controls as well, they
> > just happen to match in schematics or sources that you used as reference
> > because they are for the same device.
> >
> > My point is, you can keep a lot of boilerplate in a generic DBI driver
> > and then parameterize the things that aren't the same across them. That
> > gives you the best of both worlds.
> >
> > The reason why I objected is that you have a very specific name for the
> > driver file and the second panel doesn't match that at all, so it's
> > confusing.
> >
> 
> Noted. I will split them to remove confusion. I assume that schema can
> remain common.

Works for me.

> IMHO creating a generic DBI Type B panel driver does not wort it,
> especially since these panels are more like non-simple DSI panels and
> always require a dedicated en/disable sequence.

But that's not a great reason. Note that you can have a generic driver
that describes radically different panels. It's all a matter of
parameterization. You can have very different *-supply property names
and enable/disable sequences, all in the same driver. What matters at
the driver level is the general structure (i.e. you're device has a
backlight, one or more power supplies, a DBI command sequence, a reset
GPIO, ...).

You managed to make them work in one driver, whether that's called
panel-dbi.c or panel-hitachi-tx10d07vm0baa.c doesn't matter. If there's
ever a DBI driver that doesn't match the structure of whatever you
introduce, we can improve or add another driver.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]