From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH] drm: panels: Add MAINTAINERS entry for LVS panel driver Date: Wed, 12 Apr 2017 17:44:41 +0200 Message-ID: <20170412154440.GD22031@ulmo.ba.sec> References: <10367276.mxSL6V0lE5@avalon> <20170410071758.GA7819@ulmo.ba.sec> <1594881.cjtMSijjde@avalon> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1497784663==" Return-path: Received: from hqemgate14.nvidia.com (hqemgate14.nvidia.com [216.228.121.143]) by gabe.freedesktop.org (Postfix) with ESMTPS id E47D06E740 for ; Wed, 12 Apr 2017 15:44:50 +0000 (UTC) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Rob Herring Cc: Emil Velikov , Laurent Pinchart , Laurent Pinchart , dri-devel List-Id: dri-devel@lists.freedesktop.org --===============1497784663== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="5p8PegU4iirBW1oA" Content-Disposition: inline --5p8PegU4iirBW1oA Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Apr 11, 2017 at 01:56:35PM -0500, Rob Herring wrote: > On Mon, Apr 10, 2017 at 2:27 PM, Dave Airlie wrote: > > On 10 April 2017 at 19:03, Laurent Pinchart > > wrote: > >> Hi Thierry, > >> > >> On Monday 10 Apr 2017 09:17:59 Thierry Reding wrote: > >>> 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. > >>> > > >>> > On 7 April 2017 at 18:33, Thierry Reding 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 informati= on* > >>> > 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 h= ave > >>> any desire to repeat that discussion. > >>> > >>> We've had these differences ever since the very beginning. So we're n= ow > >>> 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 no= n- > >>> 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. > >>> > >>> > 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. > >>> > >>> 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. With= out > >>> 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. > >> > >> It will *if* the panel doesn't need any device-specific handling. In a= ll other > >> cases I agree with you, panel-specific code will be needed in the kern= el (to > >> handle power sequences for instance). > >> > >>> So if you're already going to have to support a panel in a driver, wh= y 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. > >> > >> That we agree on, panels are not special. They're not the only devices= that > >> store in DT information that could be hardcoded in the driver based on= the > >> compatible string. We have many devices whose compatible string contai= ns the > >> SoC version. Driver could then hardcode interrupts or clocks without a= ny need > >> to specific them in DT. We don't do that as it would be more complex to > >> handle. > >> > >> Regarding timings, I've long hesitated (albeit I confess I was more on= the > >> side of specifying them in DT) until Rob Herring convinced me with the= generic > >> rule that adding information in DT that are generally exposed by devic= es > >> themselves makes sense. Displays traditionally expose video mode infor= mation > >> through EDID, which is effectively device firmware. When a panel is in= tegrated > >> in a system, and the system designer decides to save money by removing= the > >> EDID I2C EEPROM, moving that piece of device firmware data to system f= irmware > >> makes sense to me. > >> > >> I certainly won't try to revive the power sequence discussions, I don't > >> believe it belongs to DT. > >> > >>> > 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. > >>> > >>> Would you mind pointing out which ones these are? I'm aware of only a > >>> couple that seemed to have sneeked in because people were trying to > >>> side-step adding drm/panel support for their boards, so I don't think > >>> that qualifies as a reason to rethink how drm/panel works. > > > > Okay I'm afraid I agree with Thierry here. > > > > I don't want mode timings or EDID in DT files, I'm pretty sure I was on= e of the > > people who helped decide that just having a compatible string and modes= in > > a driver makes sense. So if we have imported code to be the opposite of= this > > please work on removing it. > > > > If you move EDID from over the wire into DT it's not really tied to the= panel > > anymore, and is now just a complicated way of specifying the modes from= the > > panel documentation. > > > > EDID and DDC are a way for a panel to communicate to the host system, > > timing constraints and mode info, if you remove the i2c link, why bother > > encoding stuff in an EDID? >=20 > EDID is a way to make the h/w discoverable as is DT. Both are just > data. Both are OS independent data. Only the mechanism of providing > the data differs. I can envision putting DT overlays into EEPROMs on > add-on boards which then would even have the same mechanism. >=20 > This is not an either or decision. DT can provide data and the OS can > decide whether it uses it or not as long as we continue with specific > compatible strings. How does the OS decide not to use the data provided by DT? What criteria should be used to determine that the DT can't be trusted? Where do we get fallback data from? What we did with the panel-simple driver was to mandate that there is a mode (later display timing ranges) hard-coded in the driver. This means that for each supported panel we have trusted data that we know works with it. But we also made provisions to get data from an EDID if it is present. However there are also cases where the EDID can't be accessed, or where it is corrupt. For such cases we can always fallback to the hard-coded data in the driver. There are no such mechanisms for panel-lvds. On the other hand I don't see anything in panel-lvds that couldn't be implemented as part of the panel-simple driver. We already have ways to describe everything it does and more. Thierry --5p8PegU4iirBW1oA Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAljuS2YACgkQ3SOs138+ s6EQ/BAAg4+IV1Ra2xkLdmPq+InTSgqkm5nXBnRKJAl8bajtzEypNNtRevp9R3/M bOFuNUl6TIkIRkx7+JukeeWH8JoV1MUmKNusmPm0fBTZznN4a1G8DM0V+D8yWs6m 6yyamgD0HopUyaGgTQdmYRWs1kpe2Hra7807EUtfHsA7NECBzxh7ei0ZvqYa/6gh r0XwsMlH469UrixvBSzM/O1zp48yYBkFdsdH+YSaEjG2rv4u6BdEn2ncWA5nZLuv lNxaQBbaJUa1fWlKc/VY1+GhzpmVlxy65Q7CMzM4wdzS5Gr14Da7uXrpFJtax1QD 6InS5j2gGLTVefP0CY1ICTk8U2W73FwMR8AYERyIFQJlSdFhgCfoe/rSfYVFbkua A36l34Q9yyOoJzdNOfHMG6pkWcvFM6GZcprlYrssnpImOVEyJEGWrjKHz0VYn8KO DtRF8osbNtCEL1weunuQ022jz6tcnRXFK6FmAdN+4hG+Od5ucB5wZx6jif5Rkp2H 9XSYPlmOrYi3A+S9WbC/w0bsIt3OBgcqagZg5rL3g6+NmavWkdAwTi7tbY+7pqwu 9G8+17Yq75o5rqTAG/N/nrUEJILiguPQRznY2G4dwu2tL6nsvDfqpIpI/RZNbUJ3 a04V1B4pZxyoG3cCE6N/CC6HclTfawE2Qh4dTayfmAV3IHdlqk4= =fKyI -----END PGP SIGNATURE----- --5p8PegU4iirBW1oA-- --===============1497784663== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============1497784663==--