From mboxrd@z Thu Jan 1 00:00:00 1970 From: laurent.pinchart@ideasonboard.com (Laurent Pinchart) Date: Fri, 14 Sep 2018 12:49:40 +0300 Subject: [PATCH 1/6] drm/bridge: use bus flags in bridge timings In-Reply-To: References: <20180905052113.21262-1-stefan@agner.ch> Message-ID: <2015108.bT6A7b8iLH@avalon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Stefan, On Thursday, 6 September 2018 23:25:56 EEST Stefan Agner wrote: > On 06.09.2018 04:07, Linus Walleij wrote: > > On Wed, Sep 5, 2018 at 8:32 PM Stefan Agner wrote: > >> On 05.09.2018 00:44, Laurent Pinchart wrote: > >> > >> Good point! I actually really don't like that we use the same flags here > >> but from a different perspective. Especially since the flags defines > >> document things differently: > >> > >> /* drive data on pos. edge */ > >> #define DRM_BUS_FLAG_PIXDATA_POSEDGE (1<<2) > >> /* drive data on neg. edge */ > >> #define DRM_BUS_FLAG_PIXDATA_NEGEDGE (1<<3) > > > > Maybe a stupid comment from my side, but can't we just change the > > documentation to match the usecases? > > > > /* Trigger pixel data latch on positive edge */ > > #define DRM_BUS_FLAG_PIXDATA_POSEDGE (1<<2) > > > >> Using the opposite perspective would also need translation in crtc > >> drivers... So far no driver uses sampling_edge. > >> > >> I would prefer if we always use the meaning as documented by the flags. > >> > >> I guess we would need to convert DRM_BUS_FLAG_PIXDATA_POSEDGE -> > >> DRM_BUS_FLAG_PIXDATA_NEGEDGE. > >> > >> Linus Walleij, you added sampling edge, any thoughts? > > > > I just thought it was generally useful to have triggering edge encoded > > into the bridge as it makes it clear that this edge is something > > that is a delayed version of the driving edge which is subject to > > clock skew caused by the speed of electrons in silicon and > > copper and slew rate caused by parasitic capacitance. > > Ok, I read a bit up on the history of bridge timing, especially: > https://www.spinics.net/lists/dri-devel/msg155618.html > > IMHO, this got overengineered. For displays we do not need all that > setup/sample delay timing information, and much longer cables are in > use. So why is all that needed for bridges? > > For Linus case, the THS8134(A/B) data sheet I found (revised March 2010) > clearly states: > Clock input. A rising edge on CLK latches RPr0-7, GY0-7, BPb0-7. > > So we need to drive on negative edge, hence DRM_BUS_FLAG_PIXDATA_NEGEDGE > should be used, which makes the pl111 driver setting TIM2_IPC: > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0121d/index.h > tml > > > DRM_BUS_FLAG_PIXDATA_POSEDGE is the right value for my use cases, but it > > doesn't match how the ADV7123 operates. Using DRM_BUS_FLAG_PIXDATA_NEGEDGE > > would match the hardware, but would break display for some modes, > > depending on the display clock frequency as the internal 8.5ns output > > delay applied to a falling clock edge would fall right into the 1.7ns > > setup + hold time window of the ADV7123 around the rising edge. I can't > > test this right now as I don't have local access to boards using the > > ADV7123, but from a quick calculation that ignores the PCB transmission > > delay modes with frequencies between 57MHz and 71MHz could break if the > > data was output on the falling edge of the clock. > > If clocks vs. data signal are really that much off on R-Car DU, then > parallel displays must have the very same issue... > > Are you sure that only the clock signal has an output delay? And that > this output delay is a fixed value, clock independent? > > Typically, delays apply to all signals equally, and often are clock > frequency dependent... > > Without really looking at the signals, I would not jump to conclusions > here! I am pretty sure that driving on negative edge works just as well. I've tested Linus' original patch and it broke display on R-Car, so, no, it doesn't work :-) The R-Car display engine delays the clock internally (in some cases that delay is even configurable, and that's not uncommon in display controllers). We really need all this information, and I believe we need it for panels too, not just for bridges. The fact that we managed to get away without adding it to panels is likely due to the large number of panels out there, which makes it less likely that the same panel gets used by different systems in mainline with different clock delays. I expect that some panel drivers report the wrong clock edge to make things work on the board they were tested with, and I expect we'll eventually need to add the same information for panels too. So please don't remove this useful API, otherwise you'll break my board, and I won't be happy. -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH 1/6] drm/bridge: use bus flags in bridge timings Date: Fri, 14 Sep 2018 12:49:40 +0300 Message-ID: <2015108.bT6A7b8iLH@avalon> References: <20180905052113.21262-1-stefan@agner.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: 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: Stefan Agner Cc: Mark Rutland , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , max.krummenacher@toradex.com, Sascha Hauer , Marcel Ziswiler , Dave Airlie , linux-kernel@vger.kernel.org, "open list:DRM PANEL DRIVERS" , Rob Herring , NXP Linux Team , Fabio Estevam , sean@poorly.run, Shawn Guo , Linux ARM List-Id: devicetree@vger.kernel.org SGkgU3RlZmFuLAoKT24gVGh1cnNkYXksIDYgU2VwdGVtYmVyIDIwMTggMjM6MjU6NTYgRUVTVCBT dGVmYW4gQWduZXIgd3JvdGU6Cj4gT24gMDYuMDkuMjAxOCAwNDowNywgTGludXMgV2FsbGVpaiB3 cm90ZToKPiA+IE9uIFdlZCwgU2VwIDUsIDIwMTggYXQgODozMiBQTSBTdGVmYW4gQWduZXIgPHN0 ZWZhbkBhZ25lci5jaD4gd3JvdGU6Cj4gPj4gT24gMDUuMDkuMjAxOCAwMDo0NCwgTGF1cmVudCBQ aW5jaGFydCB3cm90ZToKPiA+PiAKPiA+PiBHb29kIHBvaW50ISBJIGFjdHVhbGx5IHJlYWxseSBk b24ndCBsaWtlIHRoYXQgd2UgdXNlIHRoZSBzYW1lIGZsYWdzIGhlcmUKPiA+PiBidXQgZnJvbSBh IGRpZmZlcmVudCBwZXJzcGVjdGl2ZS4gRXNwZWNpYWxseSBzaW5jZSB0aGUgZmxhZ3MgZGVmaW5l cwo+ID4+IGRvY3VtZW50IHRoaW5ncyBkaWZmZXJlbnRseToKPiA+PiAKPiA+PiAvKiBkcml2ZSBk YXRhIG9uIHBvcy4gZWRnZSAqLwo+ID4+ICNkZWZpbmUgRFJNX0JVU19GTEFHX1BJWERBVEFfUE9T RURHRSAgICAoMTw8MikKPiA+PiAvKiBkcml2ZSBkYXRhIG9uIG5lZy4gZWRnZSAqLwo+ID4+ICNk ZWZpbmUgRFJNX0JVU19GTEFHX1BJWERBVEFfTkVHRURHRSAgICAoMTw8MykKPiA+IAo+ID4gTWF5 YmUgYSBzdHVwaWQgY29tbWVudCBmcm9tIG15IHNpZGUsIGJ1dCBjYW4ndCB3ZSBqdXN0IGNoYW5n ZSB0aGUKPiA+IGRvY3VtZW50YXRpb24gdG8gbWF0Y2ggdGhlIHVzZWNhc2VzPwo+ID4gCj4gPiAv KiBUcmlnZ2VyIHBpeGVsIGRhdGEgbGF0Y2ggb24gcG9zaXRpdmUgZWRnZSAqLwo+ID4gI2RlZmlu ZSBEUk1fQlVTX0ZMQUdfUElYREFUQV9QT1NFREdFICAgICgxPDwyKQo+ID4gCj4gPj4gVXNpbmcg dGhlIG9wcG9zaXRlIHBlcnNwZWN0aXZlIHdvdWxkIGFsc28gbmVlZCB0cmFuc2xhdGlvbiBpbiBj cnRjCj4gPj4gZHJpdmVycy4uLiBTbyBmYXIgbm8gZHJpdmVyIHVzZXMgc2FtcGxpbmdfZWRnZS4K PiA+PiAKPiA+PiBJIHdvdWxkIHByZWZlciBpZiB3ZSBhbHdheXMgdXNlIHRoZSBtZWFuaW5nIGFz IGRvY3VtZW50ZWQgYnkgdGhlIGZsYWdzLgo+ID4+IAo+ID4+IEkgZ3Vlc3Mgd2Ugd291bGQgbmVl ZCB0byBjb252ZXJ0IERSTV9CVVNfRkxBR19QSVhEQVRBX1BPU0VER0UgLT4KPiA+PiBEUk1fQlVT X0ZMQUdfUElYREFUQV9ORUdFREdFLgo+ID4+IAo+ID4+IExpbnVzIFdhbGxlaWosIHlvdSBhZGRl ZCBzYW1wbGluZyBlZGdlLCBhbnkgdGhvdWdodHM/Cj4gPiAKPiA+IEkganVzdCB0aG91Z2h0IGl0 IHdhcyBnZW5lcmFsbHkgdXNlZnVsIHRvIGhhdmUgdHJpZ2dlcmluZyBlZGdlIGVuY29kZWQKPiA+ IGludG8gdGhlIGJyaWRnZSBhcyBpdCBtYWtlcyBpdCBjbGVhciB0aGF0IHRoaXMgZWRnZSBpcyBz b21ldGhpbmcKPiA+IHRoYXQgaXMgYSBkZWxheWVkIHZlcnNpb24gb2YgdGhlIGRyaXZpbmcgZWRn ZSB3aGljaCBpcyBzdWJqZWN0IHRvCj4gPiBjbG9jayBza2V3IGNhdXNlZCBieSB0aGUgc3BlZWQg b2YgZWxlY3Ryb25zIGluIHNpbGljb24gYW5kCj4gPiBjb3BwZXIgYW5kIHNsZXcgcmF0ZSBjYXVz ZWQgYnkgcGFyYXNpdGljIGNhcGFjaXRhbmNlLgo+IAo+IE9rLCBJIHJlYWQgYSBiaXQgdXAgb24g dGhlIGhpc3Rvcnkgb2YgYnJpZGdlIHRpbWluZywgZXNwZWNpYWxseToKPiBodHRwczovL3d3dy5z cGluaWNzLm5ldC9saXN0cy9kcmktZGV2ZWwvbXNnMTU1NjE4Lmh0bWwKPiAKPiBJTUhPLCB0aGlz IGdvdCBvdmVyZW5naW5lZXJlZC4gRm9yIGRpc3BsYXlzIHdlIGRvIG5vdCBuZWVkIGFsbCB0aGF0 Cj4gc2V0dXAvc2FtcGxlIGRlbGF5IHRpbWluZyBpbmZvcm1hdGlvbiwgYW5kIG11Y2ggbG9uZ2Vy IGNhYmxlcyBhcmUgaW4KPiB1c2UuIFNvIHdoeSBpcyBhbGwgdGhhdCBuZWVkZWQgZm9yIGJyaWRn ZXM/Cj4gCj4gRm9yIExpbnVzIGNhc2UsIHRoZSBUSFM4MTM0KEEvQikgZGF0YSBzaGVldCBJIGZv dW5kIChyZXZpc2VkIE1hcmNoIDIwMTApCj4gY2xlYXJseSBzdGF0ZXM6Cj4gQ2xvY2sgaW5wdXQu IEEgcmlzaW5nIGVkZ2Ugb24gQ0xLIGxhdGNoZXMgUlByMC03LCBHWTAtNywgQlBiMC03Lgo+IAo+ IFNvIHdlIG5lZWQgdG8gZHJpdmUgb24gbmVnYXRpdmUgZWRnZSwgaGVuY2UgRFJNX0JVU19GTEFH X1BJWERBVEFfTkVHRURHRQo+IHNob3VsZCBiZSB1c2VkLCB3aGljaCBtYWtlcyB0aGUgcGwxMTEg ZHJpdmVyIHNldHRpbmcgVElNMl9JUEM6Cj4gaHR0cDovL2luZm9jZW50ZXIuYXJtLmNvbS9oZWxw L2luZGV4LmpzcD90b3BpYz0vY29tLmFybS5kb2MuZGRpMDEyMWQvaW5kZXguaAo+IHRtbAo+IAo+ ID4gRFJNX0JVU19GTEFHX1BJWERBVEFfUE9TRURHRSBpcyB0aGUgcmlnaHQgdmFsdWUgZm9yIG15 IHVzZSBjYXNlcywgYnV0IGl0Cj4gPiBkb2Vzbid0IG1hdGNoIGhvdyB0aGUgQURWNzEyMyBvcGVy YXRlcy4gVXNpbmcgRFJNX0JVU19GTEFHX1BJWERBVEFfTkVHRURHRQo+ID4gd291bGQgbWF0Y2gg dGhlIGhhcmR3YXJlLCBidXQgd291bGQgYnJlYWsgZGlzcGxheSBmb3Igc29tZSBtb2RlcywKPiA+ IGRlcGVuZGluZyBvbiB0aGUgZGlzcGxheSBjbG9jayBmcmVxdWVuY3kgYXMgdGhlIGludGVybmFs IDguNW5zIG91dHB1dAo+ID4gZGVsYXkgYXBwbGllZCB0byBhIGZhbGxpbmcgY2xvY2sgZWRnZSB3 b3VsZCBmYWxsIHJpZ2h0IGludG8gdGhlIDEuN25zCj4gPiBzZXR1cCArIGhvbGQgdGltZSB3aW5k b3cgb2YgdGhlIEFEVjcxMjMgYXJvdW5kIHRoZSByaXNpbmcgZWRnZS4gSSBjYW4ndAo+ID4gdGVz dCB0aGlzIHJpZ2h0IG5vdyBhcyBJIGRvbid0IGhhdmUgbG9jYWwgYWNjZXNzIHRvIGJvYXJkcyB1 c2luZyB0aGUKPiA+IEFEVjcxMjMsIGJ1dCBmcm9tIGEgcXVpY2sgY2FsY3VsYXRpb24gdGhhdCBp Z25vcmVzIHRoZSBQQ0IgdHJhbnNtaXNzaW9uCj4gPiBkZWxheSBtb2RlcyB3aXRoIGZyZXF1ZW5j aWVzIGJldHdlZW4gNTdNSHogYW5kIDcxTUh6IGNvdWxkIGJyZWFrIGlmIHRoZQo+ID4gZGF0YSB3 YXMgb3V0cHV0IG9uIHRoZSBmYWxsaW5nIGVkZ2Ugb2YgdGhlIGNsb2NrLgo+IAo+IElmIGNsb2Nr cyB2cy4gZGF0YSBzaWduYWwgYXJlIHJlYWxseSB0aGF0IG11Y2ggb2ZmIG9uIFItQ2FyIERVLCB0 aGVuCj4gcGFyYWxsZWwgZGlzcGxheXMgbXVzdCBoYXZlIHRoZSB2ZXJ5IHNhbWUgaXNzdWUuLi4K PiAKPiBBcmUgeW91IHN1cmUgdGhhdCBvbmx5IHRoZSBjbG9jayBzaWduYWwgaGFzIGFuIG91dHB1 dCBkZWxheT8gQW5kIHRoYXQKPiB0aGlzIG91dHB1dCBkZWxheSBpcyBhIGZpeGVkIHZhbHVlLCBj bG9jayBpbmRlcGVuZGVudD8KPiAKPiBUeXBpY2FsbHksIGRlbGF5cyBhcHBseSB0byBhbGwgc2ln bmFscyBlcXVhbGx5LCBhbmQgb2Z0ZW4gYXJlIGNsb2NrCj4gZnJlcXVlbmN5IGRlcGVuZGVudC4u Lgo+IAo+IFdpdGhvdXQgcmVhbGx5IGxvb2tpbmcgYXQgdGhlIHNpZ25hbHMsIEkgd291bGQgbm90 IGp1bXAgdG8gY29uY2x1c2lvbnMKPiBoZXJlISBJIGFtIHByZXR0eSBzdXJlIHRoYXQgZHJpdmlu ZyBvbiBuZWdhdGl2ZSBlZGdlIHdvcmtzIGp1c3QgYXMgd2VsbC4KCkkndmUgdGVzdGVkIExpbnVz JyBvcmlnaW5hbCBwYXRjaCBhbmQgaXQgYnJva2UgZGlzcGxheSBvbiBSLUNhciwgc28sIG5vLCBp dCAKZG9lc24ndCB3b3JrIDotKQoKVGhlIFItQ2FyIGRpc3BsYXkgZW5naW5lIGRlbGF5cyB0aGUg Y2xvY2sgaW50ZXJuYWxseSAoaW4gc29tZSBjYXNlcyB0aGF0IGRlbGF5IAppcyBldmVuIGNvbmZp Z3VyYWJsZSwgYW5kIHRoYXQncyBub3QgdW5jb21tb24gaW4gZGlzcGxheSBjb250cm9sbGVycyku IFdlIApyZWFsbHkgbmVlZCBhbGwgdGhpcyBpbmZvcm1hdGlvbiwgYW5kIEkgYmVsaWV2ZSB3ZSBu ZWVkIGl0IGZvciBwYW5lbHMgdG9vLCBub3QgCmp1c3QgZm9yIGJyaWRnZXMuIFRoZSBmYWN0IHRo YXQgd2UgbWFuYWdlZCB0byBnZXQgYXdheSB3aXRob3V0IGFkZGluZyBpdCB0byAKcGFuZWxzIGlz IGxpa2VseSBkdWUgdG8gdGhlIGxhcmdlIG51bWJlciBvZiBwYW5lbHMgb3V0IHRoZXJlLCB3aGlj aCBtYWtlcyBpdCAKbGVzcyBsaWtlbHkgdGhhdCB0aGUgc2FtZSBwYW5lbCBnZXRzIHVzZWQgYnkg ZGlmZmVyZW50IHN5c3RlbXMgaW4gbWFpbmxpbmUgCndpdGggZGlmZmVyZW50IGNsb2NrIGRlbGF5 cy4gSSBleHBlY3QgdGhhdCBzb21lIHBhbmVsIGRyaXZlcnMgcmVwb3J0IHRoZSB3cm9uZyAKY2xv Y2sgZWRnZSB0byBtYWtlIHRoaW5ncyB3b3JrIG9uIHRoZSBib2FyZCB0aGV5IHdlcmUgdGVzdGVk IHdpdGgsIGFuZCBJIApleHBlY3Qgd2UnbGwgZXZlbnR1YWxseSBuZWVkIHRvIGFkZCB0aGUgc2Ft ZSBpbmZvcm1hdGlvbiBmb3IgcGFuZWxzIHRvby4KClNvIHBsZWFzZSBkb24ndCByZW1vdmUgdGhp cyB1c2VmdWwgQVBJLCBvdGhlcndpc2UgeW91J2xsIGJyZWFrIG15IGJvYXJkLCBhbmQgSSAKd29u J3QgYmUgaGFwcHkuCgotLSAKUmVnYXJkcywKCkxhdXJlbnQgUGluY2hhcnQKCgoKX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcg bGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRl c2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BA9E5ECDFD0 for ; Fri, 14 Sep 2018 09:49:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5F6392146E for ; Fri, 14 Sep 2018 09:49:32 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="qBtin8CX" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5F6392146E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=ideasonboard.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728193AbeINPDN (ORCPT ); Fri, 14 Sep 2018 11:03:13 -0400 Received: from perceval.ideasonboard.com ([213.167.242.64]:32946 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727730AbeINPDN (ORCPT ); Fri, 14 Sep 2018 11:03:13 -0400 Received: from avalon.localnet (unknown [IPv6:2a02:a03f:44f6:3500:d929:375b:d608:66c7]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8EC52CE; Fri, 14 Sep 2018 11:49:27 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1536918568; bh=scpBvkzq0DQHJDNSVM2/GkCp30rE90qcPvnOqt13UVs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=qBtin8CXtptIpUZHzswFJEpo+w8Cvt2EQU0cvGYGmQFsA9dLqcQKj3P/tqD1X5Yu9 2Jmg7EFLw6n6naJ9ojhxFxU7t3FR1PlDnyIzGCIHRVq7HPYP9eZHFvnbpF1fbHu6ZK FLpaQIZHOx238i7ro59NtWSHQrB03PUPG2+SQeBI= From: Laurent Pinchart To: Stefan Agner Cc: Linus Walleij , Dave Airlie , Rob Herring , Mark Rutland , Shawn Guo , Sascha Hauer , Philipp Zabel , Sascha Hauer , Fabio Estevam , NXP Linux Team , Archit Taneja , Andrzej Hajda , Gustavo Padovan , Maarten Lankhorst , sean@poorly.run, Marcel Ziswiler , max.krummenacher@toradex.com, "open list:DRM PANEL DRIVERS" , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Linux ARM , linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/6] drm/bridge: use bus flags in bridge timings Date: Fri, 14 Sep 2018 12:49:40 +0300 Message-ID: <2015108.bT6A7b8iLH@avalon> Organization: Ideas on Board Oy In-Reply-To: References: <20180905052113.21262-1-stefan@agner.ch> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Stefan, On Thursday, 6 September 2018 23:25:56 EEST Stefan Agner wrote: > On 06.09.2018 04:07, Linus Walleij wrote: > > On Wed, Sep 5, 2018 at 8:32 PM Stefan Agner wrote: > >> On 05.09.2018 00:44, Laurent Pinchart wrote: > >> > >> Good point! I actually really don't like that we use the same flags here > >> but from a different perspective. Especially since the flags defines > >> document things differently: > >> > >> /* drive data on pos. edge */ > >> #define DRM_BUS_FLAG_PIXDATA_POSEDGE (1<<2) > >> /* drive data on neg. edge */ > >> #define DRM_BUS_FLAG_PIXDATA_NEGEDGE (1<<3) > > > > Maybe a stupid comment from my side, but can't we just change the > > documentation to match the usecases? > > > > /* Trigger pixel data latch on positive edge */ > > #define DRM_BUS_FLAG_PIXDATA_POSEDGE (1<<2) > > > >> Using the opposite perspective would also need translation in crtc > >> drivers... So far no driver uses sampling_edge. > >> > >> I would prefer if we always use the meaning as documented by the flags. > >> > >> I guess we would need to convert DRM_BUS_FLAG_PIXDATA_POSEDGE -> > >> DRM_BUS_FLAG_PIXDATA_NEGEDGE. > >> > >> Linus Walleij, you added sampling edge, any thoughts? > > > > I just thought it was generally useful to have triggering edge encoded > > into the bridge as it makes it clear that this edge is something > > that is a delayed version of the driving edge which is subject to > > clock skew caused by the speed of electrons in silicon and > > copper and slew rate caused by parasitic capacitance. > > Ok, I read a bit up on the history of bridge timing, especially: > https://www.spinics.net/lists/dri-devel/msg155618.html > > IMHO, this got overengineered. For displays we do not need all that > setup/sample delay timing information, and much longer cables are in > use. So why is all that needed for bridges? > > For Linus case, the THS8134(A/B) data sheet I found (revised March 2010) > clearly states: > Clock input. A rising edge on CLK latches RPr0-7, GY0-7, BPb0-7. > > So we need to drive on negative edge, hence DRM_BUS_FLAG_PIXDATA_NEGEDGE > should be used, which makes the pl111 driver setting TIM2_IPC: > http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0121d/index.h > tml > > > DRM_BUS_FLAG_PIXDATA_POSEDGE is the right value for my use cases, but it > > doesn't match how the ADV7123 operates. Using DRM_BUS_FLAG_PIXDATA_NEGEDGE > > would match the hardware, but would break display for some modes, > > depending on the display clock frequency as the internal 8.5ns output > > delay applied to a falling clock edge would fall right into the 1.7ns > > setup + hold time window of the ADV7123 around the rising edge. I can't > > test this right now as I don't have local access to boards using the > > ADV7123, but from a quick calculation that ignores the PCB transmission > > delay modes with frequencies between 57MHz and 71MHz could break if the > > data was output on the falling edge of the clock. > > If clocks vs. data signal are really that much off on R-Car DU, then > parallel displays must have the very same issue... > > Are you sure that only the clock signal has an output delay? And that > this output delay is a fixed value, clock independent? > > Typically, delays apply to all signals equally, and often are clock > frequency dependent... > > Without really looking at the signals, I would not jump to conclusions > here! I am pretty sure that driving on negative edge works just as well. I've tested Linus' original patch and it broke display on R-Car, so, no, it doesn't work :-) The R-Car display engine delays the clock internally (in some cases that delay is even configurable, and that's not uncommon in display controllers). We really need all this information, and I believe we need it for panels too, not just for bridges. The fact that we managed to get away without adding it to panels is likely due to the large number of panels out there, which makes it less likely that the same panel gets used by different systems in mainline with different clock delays. I expect that some panel drivers report the wrong clock edge to make things work on the board they were tested with, and I expect we'll eventually need to add the same information for panels too. So please don't remove this useful API, otherwise you'll break my board, and I won't be happy. -- Regards, Laurent Pinchart