Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver

[Lucas Stach <l.stach@pengutronix.de>]

Am Montag, den 10.04.2017, 09:17 +0200 schrieb Thierry Reding:
> On Sun, Apr 09, 2017 at 01:31:40PM +0100, Emil Velikov wrote:
> > Hi Thierry,
> > 
> > I don't mean to stir up anything, just voicing "my 2c" as they say.

I normally try to keep away from this "political" discussions,
especially with regard to the panel stuff, but as this seems to pop up
again and again, I feel the need to speak up.

> > On 7 April 2017 at 18:33, Thierry Reding <treding@nvidia.com> wrote:
> > 
> > > Ever since the simple-panel binding was introduced, which is now about
> > > 3 1/2 years ago,
> > 
> > Do you have a link to these discussions? Your blog article does not
> > have any links and I only found the "Runtime Interpreted Power
> > Sequences" thread.
> > That in itself does not cover the pros/cons of storing HW information*
> > within DT.
> 
> There's some discussion here:
> 
> 	https://lists.freedesktop.org/archives/dri-devel/2013-November/049509.html
> 
> which continues here:
> 
> 	https://lists.freedesktop.org/archives/dri-devel/2013-December/050082.html
> 
> There are a couple of earlier threads, though, that discuss similar
> issues. This one seems to be the earliest I can find that is publicly
> archived:
> 
> 	http://www.spinics.net/lists/devicetree/msg08497.html
> 
> Going over all these threads again wasn't a very pleasant experience. I
> realize how much time I already spent discussing these, and I don't have
> any desire to repeat that discussion.
> 
> We've had these differences ever since the very beginning. So we're now
> again going in circles.
> 
> The main concern back at the time was that having to specify timings in
> the driver would result in a complete mess because we have zillions of
> panels that we need to support. That's turned out to be a complete non-
> issue. We've got something on the order of 50 or 60 drivers supported in
> the simple-panel driver, and for everything that's more complicated we
> have a handful of separate drivers, all fairly simple as well.
> 
> So while I understand why people want to put all this information into
> DT, we've repeatedly discussed the disadvantages that this would have.
> And while we were never able to get everyone to agree, the current
> solution has had enough agreement that we merged it. And it turned out
> to be good enough. There's nothing in panel-lvds that I can see that
> fundamentally changes this.

I was personally opposed to the idea of specifying panels only by
compatible and carrying around a "modedb" in the form of the
simple-panel driver. Since then I've come to appreciate the flexibility
that having a real driver for a panel provides.

A panel is _much_ more than a simple mode. For LVDS panels it's in many
cases a complete range of timings that will work with the panel, it's
the enable sequencing with regulators and GPIOs, it's the size of the
panel, so we get proper DPI information, it's a bus format describing
all the oddities that vendors choose to implement when designing the
physical interface. It provides proper sequencing of backlight
enable/disable.

I very much support Thierrys view, that all this should be encoded by
the compatible. This allows us to add required properties to the panel
if needed. It certainly saved us a lot of backward compat issues in
imx-drm, that we could just add all the required information to the
panel, so everything is consistent within one kernel version.

> > Personally, the idea of having hardware information* in DT does not
> > sound all that bad. The simple panel driver(s) can use those
> > properties and any panels that require anything more complex will
> > still need their own driver.

Although some people seems to feel different on that matter, but DT is
supposed to be ABI. Newer kernels _must_ work on older DTs. Which means
it needs to encode _all_ information on the panel.

Personally I don't trust people, who are just enabling their one board
to go through and validate that all the panel information is correct. As
soon as a picture shows up they are going to ship the DT.

With having to go through a panel driver, there is at least some basic
review of the panel properties.

Also speaking from experience with a board with different display
configurations, where only the bootloader knows the specific attached
panel, it's very convenient to have the bootloader only patch in the
correct compatible, instead of all the panel properties. If any of those
properties turns out to be wrong, I can fix it with a simple kernel
update, instead of having to update the bootloader.

> Again, the point is that you're going to have to modify the driver in
> any case, because you need to support the new compatible string. Without
> that compatible string you have zero information about the panel, and
> matching on a generic one isn't going to give you a working panel. So if
> you're already going to have to support a panel in a driver, why not go
> all the way and fully describe its capabilities and properties? We do it
> for all other devices. Panels are not at all special.
> 
> > For better or for worse, there's already a handful of drivers and
> > bindings that rely on/provide these. Using that information
> > consistently across the board, would be of a benefit, IMHO.

Yes, some old DTs still provide panel properties. That's no reason to
not use the agreed upon way to describe panels in newer DTs. imx-drm
still provides some support for the old way as this predates the panel
stuff and we take DT backward compat serious, but newer kernels probably
will start to complain about this. Please don't advocate this for new
stuff.

So to sum up, I completely support Thierrys way of handling panels and
IMHO we can already see the brick wall the the "generic" (which is not
generic at all, but very specific in that it doesn't require any of the
handling a panel driver could provide) LVDS panel stuff is going to hit.
Requiring properties "data-mirror" to describe uncommon bit orderings
should be a sufficient warning sign.

Regards,
Lucas

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel