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=-6.9 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,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 C5A6DECE564 for ; Tue, 18 Sep 2018 18:14:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6ECB221508 for ; Tue, 18 Sep 2018 18:14:28 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=agner.ch header.i=@agner.ch header.b="rAzYxoxl" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6ECB221508 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=agner.ch 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 S1730168AbeIRXsN (ORCPT ); Tue, 18 Sep 2018 19:48:13 -0400 Received: from mail.kmu-office.ch ([178.209.48.109]:43008 "EHLO mail.kmu-office.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729695AbeIRXsN (ORCPT ); Tue, 18 Sep 2018 19:48:13 -0400 Received: from webmail.kmu-office.ch (unknown [IPv6:2a02:418:6a02::a3]) by mail.kmu-office.ch (Postfix) with ESMTPSA id 1478A5C05C9; Tue, 18 Sep 2018 20:14:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=agner.ch; s=dkim; t=1537294463; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=B7ag1FhSxkNI87yAqNi9KT2aYCun3TuVvrnJjgCKbu0=; b=rAzYxoxlQw+/EvgUHhm21Gdv9fak2HOVB/2nJl1S+/9WK6/whS165k3ptJhxTEhDzTfmqR ds0uGhjOB8YScs+1IYyORyPk9LXFqkCR4dFQMt8JETnWUq2Uffh4EXX6h+wyy6zhl2rfRz p3nr/6FIH56XFCvx8fBZUcL0xAf0Jms= MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Date: Tue, 18 Sep 2018 11:14:22 -0700 From: Stefan Agner To: Laurent Pinchart Cc: linus.walleij@linaro.org, airlied@linux.ie, robh+dt@kernel.org, mark.rutland@arm.com, shawnguo@kernel.org, s.hauer@pengutronix.de, p.zabel@pengutronix.de, kernel@pengutronix.de, fabio.estevam@nxp.com, linux-imx@nxp.com, architt@codeaurora.org, a.hajda@samsung.com, gustavo@padovan.org, maarten.lankhorst@linux.intel.com, sean@poorly.run, marcel.ziswiler@toradex.com, max.krummenacher@toradex.com, dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/8] drm/bridge: use bus flags in bridge timings In-Reply-To: <6714863.eJMplaj6yU@avalon> References: <20180912183222.25414-1-stefan@agner.ch> <20180912183222.25414-2-stefan@agner.ch> <6714863.eJMplaj6yU@avalon> Message-ID: X-Sender: stefan@agner.ch User-Agent: Roundcube Webmail/1.3.4 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 14.09.2018 02:23, Laurent Pinchart wrote: > Hi Stefan, > > Thankk you for the patch. > > On Wednesday, 12 September 2018 21:32:15 EEST Stefan Agner wrote: >> The DRM bus flags conveys additional information on pixel data on >> the bus. All currently available bus flags might be of interest for >> a bridge. In the case at hand a dumb VGA bridge needs a specific >> data enable polarity (DRM_BUS_FLAG_DE_LOW). >> >> Replace the sampling_edge field with input_bus_flags and allow all >> currently documented bus flags. >> >> This changes the perspective from sampling side to the driving >> side for the currently supported flags. We assume that the sampling >> edge is always the opposite of the driving edge (hence we need to >> invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an >> assumption we already make for displays. For all we know it is a >> safe assumption for bridges too. > > Please don't, that will get confusing very quickly. Flag names such as > DRM_BUS_FLAG_PIXDATA_NEGEDGE don't mention sampling or driving. There's only a > small comment next to their definition, which will easily be overlooked. I'd > rather update the definition to cover both sampling and driving, and document > the input_bus_flags field to explain that all flags refer to sampling. > There is history to that, and I'd really rather prefer to keep that similar across the kernel. There is already precedence in the kernel, the display timings (which is a consumer) defines it from the driving perspective too (see DISPLAY_FLAGS_PIXDATA_POSEDGE). That is why I introduced the bus flags using the same perspective. If we _really_ want it from sampling side, we should create new defines which are explicit about that, e.g.: DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE. But again, since even the display flags use the driving perspective, I'd rather prefer to have it that way also for bridges too. -- Stefan >> Signed-off-by: Stefan Agner >> --- >> drivers/gpu/drm/bridge/dumb-vga-dac.c | 6 +++--- >> include/drm/drm_bridge.h | 11 +++++------ >> 2 files changed, 8 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c >> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index 9b706789a341..d5aa0f931ef2 >> 100644 >> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c >> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c >> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev) >> */ >> static const struct drm_bridge_timings default_dac_timings = { >> /* Timing specifications, datasheet page 7 */ >> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, >> + .input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE, >> .setup_time_ps = 500, >> .hold_time_ps = 1500, >> }; >> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings >> default_dac_timings = { */ >> static const struct drm_bridge_timings ti_ths8134_dac_timings = { >> /* From timing diagram, datasheet page 9 */ >> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, >> + .input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE, >> /* From datasheet, page 12 */ >> .setup_time_ps = 3000, >> /* I guess this means latched input */ >> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings >> ti_ths8134_dac_timings = { */ >> static const struct drm_bridge_timings ti_ths8135_dac_timings = { >> /* From timing diagram, datasheet page 14 */ >> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, >> + .input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE, >> /* From datasheet, page 16 */ >> .setup_time_ps = 2000, >> .hold_time_ps = 500, >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h >> index bd850747ce54..45e90f4b46c3 100644 >> --- a/include/drm/drm_bridge.h >> +++ b/include/drm/drm_bridge.h >> @@ -244,14 +244,13 @@ struct drm_bridge_funcs { >> */ >> struct drm_bridge_timings { >> /** >> - * @sampling_edge: >> + * @input_bus_flags: >> * >> - * Tells whether the bridge samples the digital input signal >> - * from the display engine on the positive or negative edge of the >> - * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE >> - * bitwise flags from the DRM connector (bit 2 and 3 valid). >> + * Additional settings this bridge requires for the pixel data on >> + * the input bus (e.g. pixel signal polarity). See also >> + * &drm_display_info->bus_flags. >> */ >> - u32 sampling_edge; >> + u32 input_bus_flags; >> /** >> * @setup_time_ps: >> * From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Agner Subject: Re: [PATCH v2 1/8] drm/bridge: use bus flags in bridge timings Date: Tue, 18 Sep 2018 11:14:22 -0700 Message-ID: References: <20180912183222.25414-1-stefan@agner.ch> <20180912183222.25414-2-stefan@agner.ch> <6714863.eJMplaj6yU@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <6714863.eJMplaj6yU@avalon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Laurent Pinchart Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, max.krummenacher@toradex.com, kernel@pengutronix.de, marcel.ziswiler@toradex.com, airlied@linux.ie, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, robh+dt@kernel.org, linux-imx@nxp.com, fabio.estevam@nxp.com, sean@poorly.run, shawnguo@kernel.org, linux-arm-kernel@lists.infradead.org List-Id: devicetree@vger.kernel.org T24gMTQuMDkuMjAxOCAwMjoyMywgTGF1cmVudCBQaW5jaGFydCB3cm90ZToKPiBIaSBTdGVmYW4s Cj4gCj4gVGhhbmtrIHlvdSBmb3IgdGhlIHBhdGNoLgo+IAo+IE9uIFdlZG5lc2RheSwgMTIgU2Vw dGVtYmVyIDIwMTggMjE6MzI6MTUgRUVTVCBTdGVmYW4gQWduZXIgd3JvdGU6Cj4+IFRoZSBEUk0g YnVzIGZsYWdzIGNvbnZleXMgYWRkaXRpb25hbCBpbmZvcm1hdGlvbiBvbiBwaXhlbCBkYXRhIG9u Cj4+IHRoZSBidXMuIEFsbCBjdXJyZW50bHkgYXZhaWxhYmxlIGJ1cyBmbGFncyBtaWdodCBiZSBv ZiBpbnRlcmVzdCBmb3IKPj4gYSBicmlkZ2UuIEluIHRoZSBjYXNlIGF0IGhhbmQgYSBkdW1iIFZH QSBicmlkZ2UgbmVlZHMgYSBzcGVjaWZpYwo+PiBkYXRhIGVuYWJsZSBwb2xhcml0eSAoRFJNX0JV U19GTEFHX0RFX0xPVykuCj4+Cj4+IFJlcGxhY2UgdGhlIHNhbXBsaW5nX2VkZ2UgZmllbGQgd2l0 aCBpbnB1dF9idXNfZmxhZ3MgYW5kIGFsbG93IGFsbAo+PiBjdXJyZW50bHkgZG9jdW1lbnRlZCBi dXMgZmxhZ3MuCj4+Cj4+IFRoaXMgY2hhbmdlcyB0aGUgcGVyc3BlY3RpdmUgZnJvbSBzYW1wbGlu ZyBzaWRlIHRvIHRoZSBkcml2aW5nCj4+IHNpZGUgZm9yIHRoZSBjdXJyZW50bHkgc3VwcG9ydGVk IGZsYWdzLiBXZSBhc3N1bWUgdGhhdCB0aGUgc2FtcGxpbmcKPj4gZWRnZSBpcyBhbHdheXMgdGhl IG9wcG9zaXRlIG9mIHRoZSBkcml2aW5nIGVkZ2UgKGhlbmNlIHdlIG5lZWQgdG8KPj4gaW52ZXJ0 IHRoZSBEUk1fQlVTX0ZMQUdfUElYREFUQV9bUE9TfE5FR11FREdFIGZsYWdzKS4gVGhpcyBpcyBh bgo+PiBhc3N1bXB0aW9uIHdlIGFscmVhZHkgbWFrZSBmb3IgZGlzcGxheXMuIEZvciBhbGwgd2Ug a25vdyBpdCBpcyBhCj4+IHNhZmUgYXNzdW1wdGlvbiBmb3IgYnJpZGdlcyB0b28uCj4gCj4gUGxl YXNlIGRvbid0LCB0aGF0IHdpbGwgZ2V0IGNvbmZ1c2luZyB2ZXJ5IHF1aWNrbHkuIEZsYWcgbmFt ZXMgc3VjaCBhcyAKPiBEUk1fQlVTX0ZMQUdfUElYREFUQV9ORUdFREdFIGRvbid0IG1lbnRpb24g c2FtcGxpbmcgb3IgZHJpdmluZy4gVGhlcmUncyBvbmx5IGEgCj4gc21hbGwgY29tbWVudCBuZXh0 IHRvIHRoZWlyIGRlZmluaXRpb24sIHdoaWNoIHdpbGwgZWFzaWx5IGJlIG92ZXJsb29rZWQuIEkn ZCAKPiByYXRoZXIgdXBkYXRlIHRoZSBkZWZpbml0aW9uIHRvIGNvdmVyIGJvdGggc2FtcGxpbmcg YW5kIGRyaXZpbmcsIGFuZCBkb2N1bWVudCAKPiB0aGUgaW5wdXRfYnVzX2ZsYWdzIGZpZWxkIHRv IGV4cGxhaW4gdGhhdCBhbGwgZmxhZ3MgcmVmZXIgdG8gc2FtcGxpbmcuCj4gCgpUaGVyZSBpcyBo aXN0b3J5IHRvIHRoYXQsIGFuZCBJJ2QgcmVhbGx5IHJhdGhlciBwcmVmZXIgdG8ga2VlcCB0aGF0 IHNpbWlsYXIgYWNyb3NzIHRoZSBrZXJuZWwuIFRoZXJlIGlzIGFscmVhZHkgcHJlY2VkZW5jZSBp biB0aGUga2VybmVsLCB0aGUgZGlzcGxheSB0aW1pbmdzICh3aGljaCBpcyBhIGNvbnN1bWVyKSBk ZWZpbmVzIGl0IGZyb20gdGhlIGRyaXZpbmcgcGVyc3BlY3RpdmUgdG9vIChzZWUgRElTUExBWV9G TEFHU19QSVhEQVRBX1BPU0VER0UpLgoKVGhhdCBpcyB3aHkgSSBpbnRyb2R1Y2VkIHRoZSBidXMg ZmxhZ3MgdXNpbmcgdGhlIHNhbWUgcGVyc3BlY3RpdmUuCgpJZiB3ZSBfcmVhbGx5XyB3YW50IGl0 IGZyb20gc2FtcGxpbmcgc2lkZSwgd2Ugc2hvdWxkIGNyZWF0ZSBuZXcgZGVmaW5lcyB3aGljaCBh cmUgZXhwbGljaXQgYWJvdXQgdGhhdCwgZS5nLjogRFJNX0JVU19GTEFHX1BJWERBVEFfU0FNUExF X05FR0VER0UuCgpCdXQgYWdhaW4sIHNpbmNlIGV2ZW4gdGhlIGRpc3BsYXkgZmxhZ3MgdXNlIHRo ZSBkcml2aW5nIHBlcnNwZWN0aXZlLCBJJ2QgcmF0aGVyIHByZWZlciB0byBoYXZlIGl0IHRoYXQg d2F5IGFsc28gZm9yIGJyaWRnZXMgdG9vLgoKLS0KU3RlZmFuCgoKPj4gU2lnbmVkLW9mZi1ieTog U3RlZmFuIEFnbmVyIDxzdGVmYW5AYWduZXIuY2g+Cj4+IC0tLQo+PiAgZHJpdmVycy9ncHUvZHJt L2JyaWRnZS9kdW1iLXZnYS1kYWMuYyB8ICA2ICsrKy0tLQo+PiAgaW5jbHVkZS9kcm0vZHJtX2Jy aWRnZS5oICAgICAgICAgICAgICB8IDExICsrKysrLS0tLS0tCj4+ICAyIGZpbGVzIGNoYW5nZWQs IDggaW5zZXJ0aW9ucygrKSwgOSBkZWxldGlvbnMoLSkKPj4KPj4gZGlmZiAtLWdpdCBhL2RyaXZl cnMvZ3B1L2RybS9icmlkZ2UvZHVtYi12Z2EtZGFjLmMKPj4gYi9kcml2ZXJzL2dwdS9kcm0vYnJp ZGdlL2R1bWItdmdhLWRhYy5jIGluZGV4IDliNzA2Nzg5YTM0MS4uZDVhYTBmOTMxZWYyCj4+IDEw MDY0NAo+PiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL2R1bWItdmdhLWRhYy5jCj4+ICsr KyBiL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2UvZHVtYi12Z2EtZGFjLmMKPj4gQEAgLTIzNCw3ICsy MzQsNyBAQCBzdGF0aWMgaW50IGR1bWJfdmdhX3JlbW92ZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNl ICpwZGV2KQo+PiAqLwo+PiAgc3RhdGljIGNvbnN0IHN0cnVjdCBkcm1fYnJpZGdlX3RpbWluZ3Mg ZGVmYXVsdF9kYWNfdGltaW5ncyA9IHsKPj4gIAkvKiBUaW1pbmcgc3BlY2lmaWNhdGlvbnMsIGRh dGFzaGVldCBwYWdlIDcgKi8KPj4gLQkuc2FtcGxpbmdfZWRnZSA9IERSTV9CVVNfRkxBR19QSVhE QVRBX1BPU0VER0UsCj4+ICsJLmlucHV0X2J1c19mbGFncyA9IERSTV9CVVNfRkxBR19QSVhEQVRB X05FR0VER0UsCj4+ICAJLnNldHVwX3RpbWVfcHMgPSA1MDAsCj4+ICAJLmhvbGRfdGltZV9wcyA9 IDE1MDAsCj4+ICB9Owo+PiBAQCAtMjQ1LDcgKzI0NSw3IEBAIHN0YXRpYyBjb25zdCBzdHJ1Y3Qg ZHJtX2JyaWRnZV90aW1pbmdzCj4+IGRlZmF1bHRfZGFjX3RpbWluZ3MgPSB7ICovCj4+ICBzdGF0 aWMgY29uc3Qgc3RydWN0IGRybV9icmlkZ2VfdGltaW5ncyB0aV90aHM4MTM0X2RhY190aW1pbmdz ID0gewo+PiAgCS8qIEZyb20gdGltaW5nIGRpYWdyYW0sIGRhdGFzaGVldCBwYWdlIDkgKi8KPj4g LQkuc2FtcGxpbmdfZWRnZSA9IERSTV9CVVNfRkxBR19QSVhEQVRBX1BPU0VER0UsCj4+ICsJLmlu cHV0X2J1c19mbGFncyA9IERSTV9CVVNfRkxBR19QSVhEQVRBX05FR0VER0UsCj4+ICAJLyogRnJv bSBkYXRhc2hlZXQsIHBhZ2UgMTIgKi8KPj4gIAkuc2V0dXBfdGltZV9wcyA9IDMwMDAsCj4+ICAJ LyogSSBndWVzcyB0aGlzIG1lYW5zIGxhdGNoZWQgaW5wdXQgKi8KPj4gQEAgLTI1OCw3ICsyNTgs NyBAQCBzdGF0aWMgY29uc3Qgc3RydWN0IGRybV9icmlkZ2VfdGltaW5ncwo+PiB0aV90aHM4MTM0 X2RhY190aW1pbmdzID0geyAqLwo+PiAgc3RhdGljIGNvbnN0IHN0cnVjdCBkcm1fYnJpZGdlX3Rp bWluZ3MgdGlfdGhzODEzNV9kYWNfdGltaW5ncyA9IHsKPj4gIAkvKiBGcm9tIHRpbWluZyBkaWFn cmFtLCBkYXRhc2hlZXQgcGFnZSAxNCAqLwo+PiAtCS5zYW1wbGluZ19lZGdlID0gRFJNX0JVU19G TEFHX1BJWERBVEFfUE9TRURHRSwKPj4gKwkuaW5wdXRfYnVzX2ZsYWdzID0gRFJNX0JVU19GTEFH X1BJWERBVEFfTkVHRURHRSwKPj4gIAkvKiBGcm9tIGRhdGFzaGVldCwgcGFnZSAxNiAqLwo+PiAg CS5zZXR1cF90aW1lX3BzID0gMjAwMCwKPj4gIAkuaG9sZF90aW1lX3BzID0gNTAwLAo+PiBkaWZm IC0tZ2l0IGEvaW5jbHVkZS9kcm0vZHJtX2JyaWRnZS5oIGIvaW5jbHVkZS9kcm0vZHJtX2JyaWRn ZS5oCj4+IGluZGV4IGJkODUwNzQ3Y2U1NC4uNDVlOTBmNGI0NmMzIDEwMDY0NAo+PiAtLS0gYS9p bmNsdWRlL2RybS9kcm1fYnJpZGdlLmgKPj4gKysrIGIvaW5jbHVkZS9kcm0vZHJtX2JyaWRnZS5o Cj4+IEBAIC0yNDQsMTQgKzI0NCwxMyBAQCBzdHJ1Y3QgZHJtX2JyaWRnZV9mdW5jcyB7Cj4+ICAg Ki8KPj4gIHN0cnVjdCBkcm1fYnJpZGdlX3RpbWluZ3Mgewo+PiAgCS8qKgo+PiAtCSAqIEBzYW1w bGluZ19lZGdlOgo+PiArCSAqIEBpbnB1dF9idXNfZmxhZ3M6Cj4+ICAJICoKPj4gLQkgKiBUZWxs cyB3aGV0aGVyIHRoZSBicmlkZ2Ugc2FtcGxlcyB0aGUgZGlnaXRhbCBpbnB1dCBzaWduYWwKPj4g LQkgKiBmcm9tIHRoZSBkaXNwbGF5IGVuZ2luZSBvbiB0aGUgcG9zaXRpdmUgb3IgbmVnYXRpdmUg ZWRnZSBvZiB0aGUKPj4gLQkgKiBjbG9jaywgdGhpcyBzaG91bGQgcmV1c2UgdGhlIERSTV9CVVNf RkxBR19QSVhEQVRBX1tQT1N8TkVHXUVER0UKPj4gLQkgKiBiaXR3aXNlIGZsYWdzIGZyb20gdGhl IERSTSBjb25uZWN0b3IgKGJpdCAyIGFuZCAzIHZhbGlkKS4KPj4gKwkgKiBBZGRpdGlvbmFsIHNl dHRpbmdzIHRoaXMgYnJpZGdlIHJlcXVpcmVzIGZvciB0aGUgcGl4ZWwgZGF0YSBvbgo+PiArCSAq IHRoZSBpbnB1dCBidXMgKGUuZy4gcGl4ZWwgc2lnbmFsIHBvbGFyaXR5KS4gU2VlIGFsc28KPj4g KwkgKiAmZHJtX2Rpc3BsYXlfaW5mby0+YnVzX2ZsYWdzLgo+PiAgCSAqLwo+PiAtCXUzMiBzYW1w bGluZ19lZGdlOwo+PiArCXUzMiBpbnB1dF9idXNfZmxhZ3M7Cj4+ICAJLyoqCj4+ICAJICogQHNl dHVwX3RpbWVfcHM6Cj4+ICAJICoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRl c2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8v ZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: stefan@agner.ch (Stefan Agner) Date: Tue, 18 Sep 2018 11:14:22 -0700 Subject: [PATCH v2 1/8] drm/bridge: use bus flags in bridge timings In-Reply-To: <6714863.eJMplaj6yU@avalon> References: <20180912183222.25414-1-stefan@agner.ch> <20180912183222.25414-2-stefan@agner.ch> <6714863.eJMplaj6yU@avalon> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 14.09.2018 02:23, Laurent Pinchart wrote: > Hi Stefan, > > Thankk you for the patch. > > On Wednesday, 12 September 2018 21:32:15 EEST Stefan Agner wrote: >> The DRM bus flags conveys additional information on pixel data on >> the bus. All currently available bus flags might be of interest for >> a bridge. In the case at hand a dumb VGA bridge needs a specific >> data enable polarity (DRM_BUS_FLAG_DE_LOW). >> >> Replace the sampling_edge field with input_bus_flags and allow all >> currently documented bus flags. >> >> This changes the perspective from sampling side to the driving >> side for the currently supported flags. We assume that the sampling >> edge is always the opposite of the driving edge (hence we need to >> invert the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE flags). This is an >> assumption we already make for displays. For all we know it is a >> safe assumption for bridges too. > > Please don't, that will get confusing very quickly. Flag names such as > DRM_BUS_FLAG_PIXDATA_NEGEDGE don't mention sampling or driving. There's only a > small comment next to their definition, which will easily be overlooked. I'd > rather update the definition to cover both sampling and driving, and document > the input_bus_flags field to explain that all flags refer to sampling. > There is history to that, and I'd really rather prefer to keep that similar across the kernel. There is already precedence in the kernel, the display timings (which is a consumer) defines it from the driving perspective too (see DISPLAY_FLAGS_PIXDATA_POSEDGE). That is why I introduced the bus flags using the same perspective. If we _really_ want it from sampling side, we should create new defines which are explicit about that, e.g.: DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE. But again, since even the display flags use the driving perspective, I'd rather prefer to have it that way also for bridges too. -- Stefan >> Signed-off-by: Stefan Agner >> --- >> drivers/gpu/drm/bridge/dumb-vga-dac.c | 6 +++--- >> include/drm/drm_bridge.h | 11 +++++------ >> 2 files changed, 8 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/dumb-vga-dac.c >> b/drivers/gpu/drm/bridge/dumb-vga-dac.c index 9b706789a341..d5aa0f931ef2 >> 100644 >> --- a/drivers/gpu/drm/bridge/dumb-vga-dac.c >> +++ b/drivers/gpu/drm/bridge/dumb-vga-dac.c >> @@ -234,7 +234,7 @@ static int dumb_vga_remove(struct platform_device *pdev) >> */ >> static const struct drm_bridge_timings default_dac_timings = { >> /* Timing specifications, datasheet page 7 */ >> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, >> + .input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE, >> .setup_time_ps = 500, >> .hold_time_ps = 1500, >> }; >> @@ -245,7 +245,7 @@ static const struct drm_bridge_timings >> default_dac_timings = { */ >> static const struct drm_bridge_timings ti_ths8134_dac_timings = { >> /* From timing diagram, datasheet page 9 */ >> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, >> + .input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE, >> /* From datasheet, page 12 */ >> .setup_time_ps = 3000, >> /* I guess this means latched input */ >> @@ -258,7 +258,7 @@ static const struct drm_bridge_timings >> ti_ths8134_dac_timings = { */ >> static const struct drm_bridge_timings ti_ths8135_dac_timings = { >> /* From timing diagram, datasheet page 14 */ >> - .sampling_edge = DRM_BUS_FLAG_PIXDATA_POSEDGE, >> + .input_bus_flags = DRM_BUS_FLAG_PIXDATA_NEGEDGE, >> /* From datasheet, page 16 */ >> .setup_time_ps = 2000, >> .hold_time_ps = 500, >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h >> index bd850747ce54..45e90f4b46c3 100644 >> --- a/include/drm/drm_bridge.h >> +++ b/include/drm/drm_bridge.h >> @@ -244,14 +244,13 @@ struct drm_bridge_funcs { >> */ >> struct drm_bridge_timings { >> /** >> - * @sampling_edge: >> + * @input_bus_flags: >> * >> - * Tells whether the bridge samples the digital input signal >> - * from the display engine on the positive or negative edge of the >> - * clock, this should reuse the DRM_BUS_FLAG_PIXDATA_[POS|NEG]EDGE >> - * bitwise flags from the DRM connector (bit 2 and 3 valid). >> + * Additional settings this bridge requires for the pixel data on >> + * the input bus (e.g. pixel signal polarity). See also >> + * &drm_display_info->bus_flags. >> */ >> - u32 sampling_edge; >> + u32 input_bus_flags; >> /** >> * @setup_time_ps: >> *