From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from galahad.ideasonboard.com ([185.26.127.97]:33138 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753551AbdADNH7 (ORCPT ); Wed, 4 Jan 2017 08:07:59 -0500 From: Laurent Pinchart To: Daniel Vetter Cc: Laurent Pinchart , dri-devel@lists.freedesktop.org, linux-renesas-soc@vger.kernel.org Subject: Re: [PATCH v3 06/13] drm: bridge: Add LVDS encoder driver Date: Wed, 04 Jan 2017 15:08:02 +0200 Message-ID: <5172242.1xI6ry2aS1@avalon> In-Reply-To: <20170104081818.xhj6xqp5elwe3szf@phenom.ffwll.local> References: <1480410283-28698-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com> <4516476.dJ1bJMDj4s@avalon> <20170104081818.xhj6xqp5elwe3szf@phenom.ffwll.local> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-renesas-soc-owner@vger.kernel.org List-ID: Hi Daniel, On Wednesday 04 Jan 2017 09:18:18 Daniel Vetter wrote: > On Tue, Nov 29, 2016 at 10:57:07PM +0200, Laurent Pinchart wrote: > > On Tuesday 29 Nov 2016 10:54:09 Daniel Vetter wrote: > >> On Tue, Nov 29, 2016 at 11:04:36AM +0200, Laurent Pinchart wrote: > >>> The LVDS encoder driver is a DRM bridge driver that supports the > >>> parallel to LVDS encoders that don't require any configuration. The > >>> driver thus doesn't interact with the device, but creates an LVDS > >>> connector for the panel and exposes its size and timing based on > >>> information retrieved from DT. > >>> > >>> Signed-off-by: Laurent Pinchart > >>> > >> > >> Since it's 100% dummy, why put LVDS into the name? This little thing > >> here could be our generic "wrap drm_panel and attach it to a chain" > >> helper. So what about calling this _The_ drm_panel_bridge, and also > >> linking it into docs to feature it a bit more prominently. > > > > I'm not opposed to that, except that this driver should not be considered > > as just a helper to link a panel. It should only be used to support a > > real hardware bridge that requires no control. > > > >> I came up with this because I spotted some refactoring belows for > >> building this helper, until I realized that this driver _is_ the helper I > >> think we want ;-) Only thing missing is an exported function to > >> instantiate a bridge with just a drm_panel as the parameter. And putting > >> it into the drm_kms_helper.ko module. > > > > What would that be used for ? The bridge should be instantiated by this > > bridge driver, bound to a bridge device instantiated from DT (or the > > platform firmware of your choice). > > Atm every driver using drm_panel needs a bit of glue to bind it into it's > display chain. With this code, and bridge chaining, you'd get that for > free, and I think that would be rather useful. You would trade the bit of panel glue for a bit of bridge glue. Bridge chaining could perhaps help slightly at runtime there, but at init time the amount of glue would be similar. I've noticed one piece of code that is LVDS-specific in this driver, and that's connector creation. The connector type is hardcoded to LVDS. To make the driver more generic, we would need a way to find the connector type at runtime. I'm wondering whether I should do this now, or keep the driver LVDS- specific until someone comes up with a similar need. Without several potential users it's often hard to design a properly generic API. > And from a software point of view I'd say if it quacks like a bridge, and > walks like a bridge, it probably _is_ a bridge. Even if no one calls that, > or if physical the only thing on the board at that spot are a bunch of dumb > wires. I dream of moving all encoders to DRM bridge, and then unifying drm_bridge and drm_panel with a common set of operations that could be invoked on both objects. That way the core could chain bridges and panels quite easily. I plan to experiment with this when moving the omapdrm driver to DRM bridge and DRM panel (it currently includes a bunch of custom encoder and panel drivers). -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v3 06/13] drm: bridge: Add LVDS encoder driver Date: Wed, 04 Jan 2017 15:08:02 +0200 Message-ID: <5172242.1xI6ry2aS1@avalon> References: <1480410283-28698-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com> <4516476.dJ1bJMDj4s@avalon> <20170104081818.xhj6xqp5elwe3szf@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from galahad.ideasonboard.com (galahad.ideasonboard.com [IPv6:2001:4b98:dc2:45:216:3eff:febb:480d]) by gabe.freedesktop.org (Postfix) with ESMTPS id 013DF6E252 for ; Wed, 4 Jan 2017 13:07:58 +0000 (UTC) In-Reply-To: <20170104081818.xhj6xqp5elwe3szf@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: linux-renesas-soc@vger.kernel.org, Laurent Pinchart , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org SGkgRGFuaWVsLAoKT24gV2VkbmVzZGF5IDA0IEphbiAyMDE3IDA5OjE4OjE4IERhbmllbCBWZXR0 ZXIgd3JvdGU6Cj4gT24gVHVlLCBOb3YgMjksIDIwMTYgYXQgMTA6NTc6MDdQTSArMDIwMCwgTGF1 cmVudCBQaW5jaGFydCB3cm90ZToKPiA+IE9uIFR1ZXNkYXkgMjkgTm92IDIwMTYgMTA6NTQ6MDkg RGFuaWVsIFZldHRlciB3cm90ZToKPiA+PiBPbiBUdWUsIE5vdiAyOSwgMjAxNiBhdCAxMTowNDoz NkFNICswMjAwLCBMYXVyZW50IFBpbmNoYXJ0IHdyb3RlOgo+ID4+PiBUaGUgTFZEUyBlbmNvZGVy IGRyaXZlciBpcyBhIERSTSBicmlkZ2UgZHJpdmVyIHRoYXQgc3VwcG9ydHMgdGhlCj4gPj4+IHBh cmFsbGVsIHRvIExWRFMgZW5jb2RlcnMgdGhhdCBkb24ndCByZXF1aXJlIGFueSBjb25maWd1cmF0 aW9uLiBUaGUKPiA+Pj4gZHJpdmVyIHRodXMgZG9lc24ndCBpbnRlcmFjdCB3aXRoIHRoZSBkZXZp Y2UsIGJ1dCBjcmVhdGVzIGFuIExWRFMKPiA+Pj4gY29ubmVjdG9yIGZvciB0aGUgcGFuZWwgYW5k IGV4cG9zZXMgaXRzIHNpemUgYW5kIHRpbWluZyBiYXNlZCBvbgo+ID4+PiBpbmZvcm1hdGlvbiBy ZXRyaWV2ZWQgZnJvbSBEVC4KPiA+Pj4gCj4gPj4+IFNpZ25lZC1vZmYtYnk6IExhdXJlbnQgUGlu Y2hhcnQKPiA+Pj4gPGxhdXJlbnQucGluY2hhcnQrcmVuZXNhc0BpZGVhc29uYm9hcmQuY29tPgo+ ID4+IAo+ID4+IFNpbmNlIGl0J3MgMTAwJSBkdW1teSwgd2h5IHB1dCBMVkRTIGludG8gdGhlIG5h bWU/IFRoaXMgbGl0dGxlIHRoaW5nCj4gPj4gaGVyZSBjb3VsZCBiZSBvdXIgZ2VuZXJpYyAid3Jh cCBkcm1fcGFuZWwgYW5kIGF0dGFjaCBpdCB0byBhIGNoYWluIgo+ID4+IGhlbHBlci4gU28gd2hh dCBhYm91dCBjYWxsaW5nIHRoaXMgX1RoZV8gZHJtX3BhbmVsX2JyaWRnZSwgYW5kIGFsc28KPiA+ PiBsaW5raW5nIGl0IGludG8gZG9jcyB0byBmZWF0dXJlIGl0IGEgYml0IG1vcmUgcHJvbWluZW50 bHkuCj4gPiAKPiA+IEknbSBub3Qgb3Bwb3NlZCB0byB0aGF0LCBleGNlcHQgdGhhdCB0aGlzIGRy aXZlciBzaG91bGQgbm90IGJlIGNvbnNpZGVyZWQKPiA+IGFzIGp1c3QgYSBoZWxwZXIgdG8gbGlu ayBhIHBhbmVsLiBJdCBzaG91bGQgb25seSBiZSB1c2VkIHRvIHN1cHBvcnQgYQo+ID4gcmVhbCBo YXJkd2FyZSBicmlkZ2UgdGhhdCByZXF1aXJlcyBubyBjb250cm9sLgo+ID4gCj4gPj4gSSBjYW1l IHVwIHdpdGggdGhpcyBiZWNhdXNlIEkgc3BvdHRlZCBzb21lIHJlZmFjdG9yaW5nIGJlbG93cyBm b3IKPiA+PiBidWlsZGluZyB0aGlzIGhlbHBlciwgdW50aWwgSSByZWFsaXplZCB0aGF0IHRoaXMg ZHJpdmVyIF9pc18gdGhlIGhlbHBlciBJCj4gPj4gdGhpbmsgd2Ugd2FudCA7LSkgT25seSB0aGlu ZyBtaXNzaW5nIGlzIGFuIGV4cG9ydGVkIGZ1bmN0aW9uIHRvCj4gPj4gaW5zdGFudGlhdGUgYSBi cmlkZ2Ugd2l0aCBqdXN0IGEgZHJtX3BhbmVsIGFzIHRoZSBwYXJhbWV0ZXIuIEFuZCBwdXR0aW5n Cj4gPj4gaXQgaW50byB0aGUgZHJtX2ttc19oZWxwZXIua28gbW9kdWxlLgo+ID4gCj4gPiBXaGF0 IHdvdWxkIHRoYXQgYmUgdXNlZCBmb3IgPyBUaGUgYnJpZGdlIHNob3VsZCBiZSBpbnN0YW50aWF0 ZWQgYnkgdGhpcwo+ID4gYnJpZGdlIGRyaXZlciwgYm91bmQgdG8gYSBicmlkZ2UgZGV2aWNlIGlu c3RhbnRpYXRlZCBmcm9tIERUIChvciB0aGUKPiA+IHBsYXRmb3JtIGZpcm13YXJlIG9mIHlvdXIg Y2hvaWNlKS4KPiAKPiBBdG0gZXZlcnkgZHJpdmVyIHVzaW5nIGRybV9wYW5lbCBuZWVkcyBhIGJp dCBvZiBnbHVlIHRvIGJpbmQgaXQgaW50byBpdCdzCj4gZGlzcGxheSBjaGFpbi4gV2l0aCB0aGlz IGNvZGUsIGFuZCBicmlkZ2UgY2hhaW5pbmcsIHlvdSdkIGdldCB0aGF0IGZvcgo+IGZyZWUsIGFu ZCBJIHRoaW5rIHRoYXQgd291bGQgYmUgcmF0aGVyIHVzZWZ1bC4KCllvdSB3b3VsZCB0cmFkZSB0 aGUgYml0IG9mIHBhbmVsIGdsdWUgZm9yIGEgYml0IG9mIGJyaWRnZSBnbHVlLiBCcmlkZ2UgCmNo YWluaW5nIGNvdWxkIHBlcmhhcHMgaGVscCBzbGlnaHRseSBhdCBydW50aW1lIHRoZXJlLCBidXQg YXQgaW5pdCB0aW1lIHRoZSAKYW1vdW50IG9mIGdsdWUgd291bGQgYmUgc2ltaWxhci4KCkkndmUg bm90aWNlZCBvbmUgcGllY2Ugb2YgY29kZSB0aGF0IGlzIExWRFMtc3BlY2lmaWMgaW4gdGhpcyBk cml2ZXIsIGFuZCAKdGhhdCdzIGNvbm5lY3RvciBjcmVhdGlvbi4gVGhlIGNvbm5lY3RvciB0eXBl IGlzIGhhcmRjb2RlZCB0byBMVkRTLiBUbyBtYWtlIAp0aGUgZHJpdmVyIG1vcmUgZ2VuZXJpYywg d2Ugd291bGQgbmVlZCBhIHdheSB0byBmaW5kIHRoZSBjb25uZWN0b3IgdHlwZSBhdCAKcnVudGlt ZS4gSSdtIHdvbmRlcmluZyB3aGV0aGVyIEkgc2hvdWxkIGRvIHRoaXMgbm93LCBvciBrZWVwIHRo ZSBkcml2ZXIgTFZEUy0Kc3BlY2lmaWMgdW50aWwgc29tZW9uZSBjb21lcyB1cCB3aXRoIGEgc2lt aWxhciBuZWVkLiBXaXRob3V0IHNldmVyYWwgcG90ZW50aWFsIAp1c2VycyBpdCdzIG9mdGVuIGhh cmQgdG8gZGVzaWduIGEgcHJvcGVybHkgZ2VuZXJpYyBBUEkuCgo+IEFuZCBmcm9tIGEgc29mdHdh cmUgcG9pbnQgb2YgdmlldyBJJ2Qgc2F5IGlmIGl0IHF1YWNrcyBsaWtlIGEgYnJpZGdlLCBhbmQK PiB3YWxrcyBsaWtlIGEgYnJpZGdlLCBpdCBwcm9iYWJseSBfaXNfIGEgYnJpZGdlLiBFdmVuIGlm IG5vIG9uZSBjYWxscyB0aGF0LAo+IG9yIGlmIHBoeXNpY2FsIHRoZSBvbmx5IHRoaW5nIG9uIHRo ZSBib2FyZCBhdCB0aGF0IHNwb3QgYXJlIGEgYnVuY2ggb2YgZHVtYgo+IHdpcmVzLgoKSSBkcmVh bSBvZiBtb3ZpbmcgYWxsIGVuY29kZXJzIHRvIERSTSBicmlkZ2UsIGFuZCB0aGVuIHVuaWZ5aW5n IGRybV9icmlkZ2UgYW5kIApkcm1fcGFuZWwgd2l0aCBhIGNvbW1vbiBzZXQgb2Ygb3BlcmF0aW9u cyB0aGF0IGNvdWxkIGJlIGludm9rZWQgb24gYm90aCAKb2JqZWN0cy4gVGhhdCB3YXkgdGhlIGNv cmUgY291bGQgY2hhaW4gYnJpZGdlcyBhbmQgcGFuZWxzIHF1aXRlIGVhc2lseS4gSSBwbGFuIAp0 byBleHBlcmltZW50IHdpdGggdGhpcyB3aGVuIG1vdmluZyB0aGUgb21hcGRybSBkcml2ZXIgdG8g RFJNIGJyaWRnZSBhbmQgRFJNIApwYW5lbCAoaXQgY3VycmVudGx5IGluY2x1ZGVzIGEgYnVuY2gg b2YgY3VzdG9tIGVuY29kZXIgYW5kIHBhbmVsIGRyaXZlcnMpLgoKLS0gClJlZ2FyZHMsCgpMYXVy ZW50IFBpbmNoYXJ0CgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5v cmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2 ZWwK