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 18A87C433F5 for ; Wed, 5 Sep 2018 07:44:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B6F712075E for ; Wed, 5 Sep 2018 07:44:52 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="t/Gu8jjN" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B6F712075E 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 S1727723AbeIEMNq (ORCPT ); Wed, 5 Sep 2018 08:13:46 -0400 Received: from perceval.ideasonboard.com ([213.167.242.64]:52882 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726401AbeIEMNp (ORCPT ); Wed, 5 Sep 2018 08:13:45 -0400 Received: from avalon.localnet (unknown [IPv6:2a02:a03f:4407:cd00:1953:677d:5909:a7be]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 41E8C994; Wed, 5 Sep 2018 09:44:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1536133487; bh=6212t6QYD5SdOwQEG/oMgUDisjxFgUOFgMeFh1Tam6U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=t/Gu8jjNJsWD11ab121+iteMuNh+L2blN8umXDnKHKkkJ8FZ1u1X9xBzV1QQ/vE6d zGq4tpGR6Bpqs/NTCC6k8xjYTGA3JPDtCH6onaR9FFiMdgDTMGako/y0myfwsGz2Hf 2EZPpcB5REhbwxY8hyQtODhVhhNWUbqrN1Dd8ldc= From: Laurent Pinchart To: Stefan Agner 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 1/6] drm/bridge: use bus flags in bridge timings Date: Wed, 05 Sep 2018 10:44:53 +0300 Message-ID: <1569297.pdEFdpi3HS@avalon> Organization: Ideas on Board Oy In-Reply-To: <4035252.QuWadVx7pr@avalon> References: <20180905052113.21262-1-stefan@agner.ch> <4035252.QuWadVx7pr@avalon> 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 Wednesday, 5 September 2018 10:06:28 EEST Laurent Pinchart wrote: > On Wednesday, 5 September 2018 08:21:08 EEST Stefan Agner wrote: > > The DRM bus flags convey additional information on pixel data on > > the bus. All current available bus flags might be of interest for > > a bridge. Remove the sampling_edge field and use bus_flags. > > > > In the case at hand a dumb VGA bridge needs a specific data enable > > polarity (DRM_BUS_FLAG_DE_LOW). > > > > 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..7a5c24967115 > > 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, > > + .bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE, > > > > .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, > > + .bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE, > > > > /* 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, > > + .bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE, > > > > /* 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..85d4b51eae19 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: > > > > + * @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). > > + * Tells what additional settings for the pixel data on the bus > > + * this bridge requires (like pixel signal polarity). See also > > + * &drm_display_info->bus_flags. > > > > */ > > > > - u32 sampling_edge; > > + u32 bus_flags; > > While I'm not opposed to extending the bridge structure to allow specifying > other flags, I think we're losing information here. The sampling_edge field > makes it clear that the DRM_BUS_FLAG_PIXDATA_(NEG|POS)EDGE flags specified > on which clock edge signals are sampled. bus_flags could be interpreted > differently, for instance as specifying on which clock edge signals are > output on the other side of the bridge. > > We could easily fix this by specifying that the bus_flags field refers to > the input side of the bridge. We could also rename the field to > input_bus_flags. The rename could be delayed until needed, but that would > result in yet another change to all bridge drivers, so we may want to do it > now as your patch touches all the drivers already. Other options might also > be possible. And I should of course make sure to wake up before reviewing patches. Obviously only one driver is currently affected by the rename. More will use the flags later though, so the argument could still hold. > > /** > > > > * @setup_time_ps: > > * -- 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: Wed, 05 Sep 2018 10:44:53 +0300 Message-ID: <1569297.pdEFdpi3HS@avalon> References: <20180905052113.21262-1-stefan@agner.ch> <4035252.QuWadVx7pr@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <4035252.QuWadVx7pr@avalon> 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@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 SGkgU3RlZmFuLAoKT24gV2VkbmVzZGF5LCA1IFNlcHRlbWJlciAyMDE4IDEwOjA2OjI4IEVFU1Qg TGF1cmVudCBQaW5jaGFydCB3cm90ZToKPiBPbiBXZWRuZXNkYXksIDUgU2VwdGVtYmVyIDIwMTgg MDg6MjE6MDggRUVTVCBTdGVmYW4gQWduZXIgd3JvdGU6Cj4gPiBUaGUgRFJNIGJ1cyBmbGFncyBj b252ZXkgYWRkaXRpb25hbCBpbmZvcm1hdGlvbiBvbiBwaXhlbCBkYXRhIG9uCj4gPiB0aGUgYnVz LiBBbGwgY3VycmVudCBhdmFpbGFibGUgYnVzIGZsYWdzIG1pZ2h0IGJlIG9mIGludGVyZXN0IGZv cgo+ID4gYSBicmlkZ2UuIFJlbW92ZSB0aGUgc2FtcGxpbmdfZWRnZSBmaWVsZCBhbmQgdXNlIGJ1 c19mbGFncy4KPiA+IAo+ID4gSW4gdGhlIGNhc2UgYXQgaGFuZCBhIGR1bWIgVkdBIGJyaWRnZSBu ZWVkcyBhIHNwZWNpZmljIGRhdGEgZW5hYmxlCj4gPiBwb2xhcml0eSAoRFJNX0JVU19GTEFHX0RF X0xPVykuCj4gPiAKPiA+IFNpZ25lZC1vZmYtYnk6IFN0ZWZhbiBBZ25lciA8c3RlZmFuQGFnbmVy LmNoPgo+ID4gLS0tCj4gPiAKPiA+ICBkcml2ZXJzL2dwdS9kcm0vYnJpZGdlL2R1bWItdmdhLWRh Yy5jIHwgIDYgKysrLS0tCj4gPiAgaW5jbHVkZS9kcm0vZHJtX2JyaWRnZS5oICAgICAgICAgICAg ICB8IDExICsrKysrLS0tLS0tCj4gPiAgMiBmaWxlcyBjaGFuZ2VkLCA4IGluc2VydGlvbnMoKyks IDkgZGVsZXRpb25zKC0pCj4gPiAKPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vYnJp ZGdlL2R1bWItdmdhLWRhYy5jCj4gPiBiL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2UvZHVtYi12Z2Et ZGFjLmMgaW5kZXggOWI3MDY3ODlhMzQxLi43YTVjMjQ5NjcxMTUKPiA+IDEwMDY0NAo+ID4gLS0t IGEvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9kdW1iLXZnYS1kYWMuYwo+ID4gKysrIGIvZHJpdmVy cy9ncHUvZHJtL2JyaWRnZS9kdW1iLXZnYS1kYWMuYwo+ID4gQEAgLTIzNCw3ICsyMzQsNyBAQCBz dGF0aWMgaW50IGR1bWJfdmdhX3JlbW92ZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlCj4gPiAqcGRl dikgKi8KPiA+IAo+ID4gIHN0YXRpYyBjb25zdCBzdHJ1Y3QgZHJtX2JyaWRnZV90aW1pbmdzIGRl ZmF1bHRfZGFjX3RpbWluZ3MgPSB7Cj4gPiAgCj4gPiAgCS8qIFRpbWluZyBzcGVjaWZpY2F0aW9u cywgZGF0YXNoZWV0IHBhZ2UgNyAqLwo+ID4gCj4gPiAtCS5zYW1wbGluZ19lZGdlID0gRFJNX0JV U19GTEFHX1BJWERBVEFfUE9TRURHRSwKPiA+ICsJLmJ1c19mbGFncyA9IERSTV9CVVNfRkxBR19Q SVhEQVRBX1BPU0VER0UsCj4gPiAKPiA+ICAJLnNldHVwX3RpbWVfcHMgPSA1MDAsCj4gPiAgCS5o b2xkX3RpbWVfcHMgPSAxNTAwLAo+ID4gIAo+ID4gIH07Cj4gPiAKPiA+IEBAIC0yNDUsNyArMjQ1 LDcgQEAgc3RhdGljIGNvbnN0IHN0cnVjdCBkcm1fYnJpZGdlX3RpbWluZ3MKPiA+IGRlZmF1bHRf ZGFjX3RpbWluZ3MgPSB7ICovCj4gPiAKPiA+ICBzdGF0aWMgY29uc3Qgc3RydWN0IGRybV9icmlk Z2VfdGltaW5ncyB0aV90aHM4MTM0X2RhY190aW1pbmdzID0gewo+ID4gIAo+ID4gIAkvKiBGcm9t IHRpbWluZyBkaWFncmFtLCBkYXRhc2hlZXQgcGFnZSA5ICovCj4gPiAKPiA+IC0JLnNhbXBsaW5n X2VkZ2UgPSBEUk1fQlVTX0ZMQUdfUElYREFUQV9QT1NFREdFLAo+ID4gKwkuYnVzX2ZsYWdzID0g RFJNX0JVU19GTEFHX1BJWERBVEFfUE9TRURHRSwKPiA+IAo+ID4gIAkvKiBGcm9tIGRhdGFzaGVl dCwgcGFnZSAxMiAqLwo+ID4gIAkuc2V0dXBfdGltZV9wcyA9IDMwMDAsCj4gPiAgCS8qIEkgZ3Vl c3MgdGhpcyBtZWFucyBsYXRjaGVkIGlucHV0ICovCj4gPiAKPiA+IEBAIC0yNTgsNyArMjU4LDcg QEAgc3RhdGljIGNvbnN0IHN0cnVjdCBkcm1fYnJpZGdlX3RpbWluZ3MKPiA+IHRpX3RoczgxMzRf ZGFjX3RpbWluZ3MgPSB7ICovCj4gPiAKPiA+ICBzdGF0aWMgY29uc3Qgc3RydWN0IGRybV9icmlk Z2VfdGltaW5ncyB0aV90aHM4MTM1X2RhY190aW1pbmdzID0gewo+ID4gIAo+ID4gIAkvKiBGcm9t IHRpbWluZyBkaWFncmFtLCBkYXRhc2hlZXQgcGFnZSAxNCAqLwo+ID4gCj4gPiAtCS5zYW1wbGlu Z19lZGdlID0gRFJNX0JVU19GTEFHX1BJWERBVEFfUE9TRURHRSwKPiA+ICsJLmJ1c19mbGFncyA9 IERSTV9CVVNfRkxBR19QSVhEQVRBX1BPU0VER0UsCj4gPiAKPiA+ICAJLyogRnJvbSBkYXRhc2hl ZXQsIHBhZ2UgMTYgKi8KPiA+ICAJLnNldHVwX3RpbWVfcHMgPSAyMDAwLAo+ID4gIAkuaG9sZF90 aW1lX3BzID0gNTAwLAo+ID4gCj4gPiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9kcm0vZHJtX2JyaWRn ZS5oIGIvaW5jbHVkZS9kcm0vZHJtX2JyaWRnZS5oCj4gPiBpbmRleCBiZDg1MDc0N2NlNTQuLjg1 ZDRiNTFlYWUxOSAxMDA2NDQKPiA+IC0tLSBhL2luY2x1ZGUvZHJtL2RybV9icmlkZ2UuaAo+ID4g KysrIGIvaW5jbHVkZS9kcm0vZHJtX2JyaWRnZS5oCj4gPiBAQCAtMjQ0LDE0ICsyNDQsMTMgQEAg c3RydWN0IGRybV9icmlkZ2VfZnVuY3Mgewo+ID4gCj4gPiAgICovCj4gPiAgCj4gPiAgc3RydWN0 IGRybV9icmlkZ2VfdGltaW5ncyB7Cj4gPiAgCj4gPiAgCS8qKgo+ID4gCj4gPiAtCSAqIEBzYW1w bGluZ19lZGdlOgo+ID4gCj4gPiArCSAqIEBidXNfZmxhZ3M6Cj4gPiAgCSAqCj4gPiAKPiA+IC0J ICogVGVsbHMgd2hldGhlciB0aGUgYnJpZGdlIHNhbXBsZXMgdGhlIGRpZ2l0YWwgaW5wdXQgc2ln bmFsCj4gPiAtCSAqIGZyb20gdGhlIGRpc3BsYXkgZW5naW5lIG9uIHRoZSBwb3NpdGl2ZSBvciBu ZWdhdGl2ZSBlZGdlIG9mIHRoZQo+ID4gLQkgKiBjbG9jaywgdGhpcyBzaG91bGQgcmV1c2UgdGhl IERSTV9CVVNfRkxBR19QSVhEQVRBX1tQT1N8TkVHXUVER0UKPiA+IC0JICogYml0d2lzZSBmbGFn cyBmcm9tIHRoZSBEUk0gY29ubmVjdG9yIChiaXQgMiBhbmQgMyB2YWxpZCkuCj4gPiArCSAqIFRl bGxzIHdoYXQgYWRkaXRpb25hbCBzZXR0aW5ncyBmb3IgdGhlIHBpeGVsIGRhdGEgb24gdGhlIGJ1 cwo+ID4gKwkgKiB0aGlzIGJyaWRnZSByZXF1aXJlcyAobGlrZSBwaXhlbCBzaWduYWwgcG9sYXJp dHkpLiBTZWUgYWxzbwo+ID4gKwkgKiAmZHJtX2Rpc3BsYXlfaW5mby0+YnVzX2ZsYWdzLgo+ID4g Cj4gPiAgCSAqLwo+ID4gCj4gPiAtCXUzMiBzYW1wbGluZ19lZGdlOwo+ID4gKwl1MzIgYnVzX2Zs YWdzOwo+IAo+IFdoaWxlIEknbSBub3Qgb3Bwb3NlZCB0byBleHRlbmRpbmcgdGhlIGJyaWRnZSBz dHJ1Y3R1cmUgdG8gYWxsb3cgc3BlY2lmeWluZwo+IG90aGVyIGZsYWdzLCBJIHRoaW5rIHdlJ3Jl IGxvc2luZyBpbmZvcm1hdGlvbiBoZXJlLiBUaGUgc2FtcGxpbmdfZWRnZSBmaWVsZAo+IG1ha2Vz IGl0IGNsZWFyIHRoYXQgdGhlIERSTV9CVVNfRkxBR19QSVhEQVRBXyhORUd8UE9TKUVER0UgZmxh Z3Mgc3BlY2lmaWVkCj4gb24gd2hpY2ggY2xvY2sgZWRnZSBzaWduYWxzIGFyZSBzYW1wbGVkLiBi dXNfZmxhZ3MgY291bGQgYmUgaW50ZXJwcmV0ZWQKPiBkaWZmZXJlbnRseSwgZm9yIGluc3RhbmNl IGFzIHNwZWNpZnlpbmcgb24gd2hpY2ggY2xvY2sgZWRnZSBzaWduYWxzIGFyZQo+IG91dHB1dCBv biB0aGUgb3RoZXIgc2lkZSBvZiB0aGUgYnJpZGdlLgo+IAo+IFdlIGNvdWxkIGVhc2lseSBmaXgg dGhpcyBieSBzcGVjaWZ5aW5nIHRoYXQgdGhlIGJ1c19mbGFncyBmaWVsZCByZWZlcnMgdG8KPiB0 aGUgaW5wdXQgc2lkZSBvZiB0aGUgYnJpZGdlLiBXZSBjb3VsZCBhbHNvIHJlbmFtZSB0aGUgZmll bGQgdG8KPiBpbnB1dF9idXNfZmxhZ3MuIFRoZSByZW5hbWUgY291bGQgYmUgZGVsYXllZCB1bnRp bCBuZWVkZWQsIGJ1dCB0aGF0IHdvdWxkCj4gcmVzdWx0IGluIHlldCBhbm90aGVyIGNoYW5nZSB0 byBhbGwgYnJpZGdlIGRyaXZlcnMsIHNvIHdlIG1heSB3YW50IHRvIGRvIGl0Cj4gbm93IGFzIHlv dXIgcGF0Y2ggdG91Y2hlcyBhbGwgdGhlIGRyaXZlcnMgYWxyZWFkeS4gT3RoZXIgb3B0aW9ucyBt aWdodCBhbHNvCj4gYmUgcG9zc2libGUuCgpBbmQgSSBzaG91bGQgb2YgY291cnNlIG1ha2Ugc3Vy ZSB0byB3YWtlIHVwIGJlZm9yZSByZXZpZXdpbmcgcGF0Y2hlcy4gCk9idmlvdXNseSBvbmx5IG9u ZSBkcml2ZXIgaXMgY3VycmVudGx5IGFmZmVjdGVkIGJ5IHRoZSByZW5hbWUuIE1vcmUgd2lsbCB1 c2UgCnRoZSBmbGFncyBsYXRlciB0aG91Z2gsIHNvIHRoZSBhcmd1bWVudCBjb3VsZCBzdGlsbCBo b2xkLgoKPiA+ICAJLyoqCj4gPiAgCQo+ID4gIAkgKiBAc2V0dXBfdGltZV9wczoKPiA+ICAJICoK Ci0tIApSZWdhcmRzLAoKTGF1cmVudCBQaW5jaGFydAoKCgpfX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZl bEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFp bG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 From: laurent.pinchart@ideasonboard.com (Laurent Pinchart) Date: Wed, 05 Sep 2018 10:44:53 +0300 Subject: [PATCH 1/6] drm/bridge: use bus flags in bridge timings In-Reply-To: <4035252.QuWadVx7pr@avalon> References: <20180905052113.21262-1-stefan@agner.ch> <4035252.QuWadVx7pr@avalon> Message-ID: <1569297.pdEFdpi3HS@avalon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Stefan, On Wednesday, 5 September 2018 10:06:28 EEST Laurent Pinchart wrote: > On Wednesday, 5 September 2018 08:21:08 EEST Stefan Agner wrote: > > The DRM bus flags convey additional information on pixel data on > > the bus. All current available bus flags might be of interest for > > a bridge. Remove the sampling_edge field and use bus_flags. > > > > In the case at hand a dumb VGA bridge needs a specific data enable > > polarity (DRM_BUS_FLAG_DE_LOW). > > > > 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..7a5c24967115 > > 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, > > + .bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE, > > > > .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, > > + .bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE, > > > > /* 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, > > + .bus_flags = DRM_BUS_FLAG_PIXDATA_POSEDGE, > > > > /* 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..85d4b51eae19 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: > > > > + * @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). > > + * Tells what additional settings for the pixel data on the bus > > + * this bridge requires (like pixel signal polarity). See also > > + * &drm_display_info->bus_flags. > > > > */ > > > > - u32 sampling_edge; > > + u32 bus_flags; > > While I'm not opposed to extending the bridge structure to allow specifying > other flags, I think we're losing information here. The sampling_edge field > makes it clear that the DRM_BUS_FLAG_PIXDATA_(NEG|POS)EDGE flags specified > on which clock edge signals are sampled. bus_flags could be interpreted > differently, for instance as specifying on which clock edge signals are > output on the other side of the bridge. > > We could easily fix this by specifying that the bus_flags field refers to > the input side of the bridge. We could also rename the field to > input_bus_flags. The rename could be delayed until needed, but that would > result in yet another change to all bridge drivers, so we may want to do it > now as your patch touches all the drivers already. Other options might also > be possible. And I should of course make sure to wake up before reviewing patches. Obviously only one driver is currently affected by the rename. More will use the flags later though, so the argument could still hold. > > /** > > > > * @setup_time_ps: > > * -- Regards, Laurent Pinchart