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

Tue Apr 11 05:00:11 UTC 2017

Hi Lucas,

On Monday 10 Apr 2017 11:58:38 Lucas Stach wrote:
> 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 at 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.

I don't think anyone will review the timings, and my hopes are not much higher 
for the rest of the panel properties, regardless of whether they're specified 
in DT or C code.

> 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.

Aren't you essentially advocating for going back from DT to C board files 
because firmware is often buggy ? :-) I agree that boot loaders get things 
wrong more often than not, but that's not specific to panels.

> > 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.

I'm afraid I still don't agree. I don't see the panel-lvds driver as a one-
size-fits-them-all solution to support all LVDS panels without ever having to 
write a line of C code. Things as power sequences need to be implemented in 
drivers, there's no doubt about that. Any oddity that will make a panel not 
compatible with the panel-lvds DT bindings will require a specific driver (or 
specific code in an existing driver). I believe we agree on that.

The point I don't agree with is that timings have to be specified in C code. 
As pointed in in a comment to https://sietch-tagr.blogspot.fi/2016/04/display-panels-are-not-special.html, there's a huge number of display panels on the 
marked, and the Linux kernel supports only a very tiny fraction of them. I 
still haven't seen the scalability issue of a C mode DB being addressed, and 
that's a major blocker for me. The other comment about EMC and EMI (Electro-
Magnetic Compatibility and Electro-Magnetic Interference) is also valid in my 
opinion, and is why we had to specify clock frequencies in DT for cameras as 
they need to be fine-tuned for each particular system. The same applies to 
panels, where timings for the same panel integrated in different systems may 
need to differ.

I'm certainly willing to reconsider my position if someone can propose a way 
to solve those two issues without involving DT (and, of course, without too 
many dirty hacks either :-)).

-- 
Regards,

Laurent Pinchart