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: 2843 bytes --]

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.

Thierry

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