From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [RFC PATCH 2/3] drm: bridge: panel: allow override of the bus format Date: Mon, 26 Mar 2018 22:03:31 +0300 Message-ID: <1986009.iWteaM7My3@avalon> References: <20180317221525.18534-1-peda@axentia.se> <7015227.p44CVkfgjM@avalon> 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: Peter Rosin Cc: Mark Rutland , devicetree@vger.kernel.org, David Airlie , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Rob Herring , Jacopo Mondi , Daniel Vetter List-Id: devicetree@vger.kernel.org SGkgUGV0ZXIsCgooQ0MnaW5nIEphY29wbyBNb25kaSkKCk9uIFN1bmRheSwgMjUgTWFyY2ggMjAx OCAxNTowMToxMSBFRVNUIFBldGVyIFJvc2luIHdyb3RlOgo+IE9uIDIwMTgtMDMtMjAgMTQ6NTYs IExhdXJlbnQgUGluY2hhcnQgd3JvdGU6Cj4gPiBIaSBQZXRlciwKPiA+IAo+ID4gVGhhbmsgeW91 IGZvciB0aGUgcGF0Y2guCj4gPiAKPiA+IE9uIFN1bmRheSwgMTggTWFyY2ggMjAxOCAwMDoxNToy NCBFRVQgUGV0ZXIgUm9zaW4gd3JvdGU6Cj4gPj4gVXNlZnVsIGlmIHRoZSBicmlkZ2UgZG9lcyBz b21lIGtpbmQgb2YgY29udmVyc2lvbiBvZiB0aGUgYnVzIGZvcm1hdC4KPiA+PiAKPiA+PiBTaWdu ZWQtb2ZmLWJ5OiBQZXRlciBSb3NpbiA8cGVkYUBheGVudGlhLnNlPgo+ID4+IC0tLQo+ID4+IAo+ ID4+ICBkcml2ZXJzL2dwdS9kcm0vYnJpZGdlL3BhbmVsLmMgfCAyMiArKysrKysrKysrKysrKysr KysrKystCj4gPj4gIGluY2x1ZGUvZHJtL2RybV9icmlkZ2UuaCAgICAgICB8ICAxICsKPiA+PiAg MiBmaWxlcyBjaGFuZ2VkLCAyMiBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pCj4gPj4gCj4g Pj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2UvcGFuZWwuYwo+ID4+IGIvZHJp dmVycy9ncHUvZHJtL2JyaWRnZS9wYW5lbC5jIGluZGV4IDZkOTlkNGEzYmViMy4uY2NlZjAyODNm ZjQxIDEwMDY0NAo+ID4+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2UvcGFuZWwuYwo+ID4+ ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2UvcGFuZWwuYwo+ID4+IEBAIC0yMiw2ICsyMiw3 IEBAIHN0cnVjdCBwYW5lbF9icmlkZ2Ugewo+ID4+ICAJc3RydWN0IGRybV9jb25uZWN0b3IgY29u bmVjdG9yOwo+ID4+ICAJc3RydWN0IGRybV9wYW5lbCAqcGFuZWw7Cj4gPj4gIAl1MzIgY29ubmVj dG9yX3R5cGU7Cj4gPj4gKwl1MzIgYnVzX2Zvcm1hdDsKPiA+PiAgfTsKPiA+PiAgCj4gPj4gIHN0 YXRpYyBpbmxpbmUgc3RydWN0IHBhbmVsX2JyaWRnZSAqCj4gPj4gQEAgLTQwLDggKzQxLDE1IEBA IHN0YXRpYyBpbnQgcGFuZWxfYnJpZGdlX2Nvbm5lY3Rvcl9nZXRfbW9kZXMoc3RydWN0Cj4gPj4g ZHJtX2Nvbm5lY3RvciAqY29ubmVjdG9yKSB7Cj4gPj4gIAlzdHJ1Y3QgcGFuZWxfYnJpZGdlICpw YW5lbF9icmlkZ2UgPQo+ID4+ICAJCWRybV9jb25uZWN0b3JfdG9fcGFuZWxfYnJpZGdlKGNvbm5l Y3Rvcik7Cj4gPj4gCj4gPj4gKwlpbnQgcmV0Owo+ID4+ICsKPiA+PiArCXJldCA9IGRybV9wYW5l bF9nZXRfbW9kZXMocGFuZWxfYnJpZGdlLT5wYW5lbCk7Cj4gPj4gKwo+ID4+ICsJaWYgKHBhbmVs X2JyaWRnZS0+YnVzX2Zvcm1hdCkKPiA+PiArCQlkcm1fZGlzcGxheV9pbmZvX3NldF9idXNfZm9y bWF0cygmY29ubmVjdG9yLT5kaXNwbGF5X2luZm8sCj4gPj4gKwkJCQkJCSAmcGFuZWxfYnJpZGdl LT5idXNfZm9ybWF0LCAxKTsKPiA+IAo+ID4gV2hpbGUgSSBhZ3JlZSB3aXRoIHRoZSBwcm9ibGVt IHN0YXRlbWVudCBhbmQsIHRvIHNvbWUgZXh0ZW50LCB0aGUgRFQKPiA+IGJpbmRpbmdzLCBJIGRv bid0IHRoaW5rIHRoaXMgaXMgdGhlIHJpZ2h0IGltcGxlbWVudGF0aW9uLiBZb3UndmUKPiA+IGNv cnJlY3RseSBub3RlZCB0aGF0IGRpc3BsYXkgY29udHJvbGxlciBzaG91bGRuJ3QgYmxpbmRseSB1 c2UgdGhlIGZvcm1hdHMKPiA+IHJlcG9ydGVkIGJ5IHRoZSBwYW5lbCB0aHJvdWdoIHRoZSBjb25u ZWN0b3IgZm9ybWF0cywgYW5kIHRoYXQgaGFja2luZyB0aGUKPiA+IHBhbmVsIGRyaXZlciB0byBv dmVycmlkZSB0aGUgZm9ybWF0cyBpc24ndCBhIGdvb2QgaWRlYSwgc28gSSB3b3VsZG4ndAo+ID4g b3ZlcnJpZGUgdGhlIGZvcm1hdHMgcmVwb3J0ZWQgYnkgdGhlIGNvbm5lY3Rvci4gSSB3b3VsZCBp bnN0ZWFkIGV4dGVuZAo+ID4gdGhlIGRybV9icmlkZ2UgQVBJIHRvIHJlcG9ydCBmb3JtYXRzIGF0 IGJyaWRnZSBpbnB1dHMuIFRoaXMgd291bGQgYmUgbW9yZQo+ID4gZ2VuZXJpYyBhbmQgYWxsb3cg ZWFjaCBicmlkZ2UgdG8gY29uZmlndXJlIGl0c2VsZiBhY2NvcmRpbmcgdG8gdGhlIG5leHQKPiA+ IGJyaWRnZSBpbiB0aGUgY2hhaW4uCj4gPiAKPiA+IEknbSBub3Qgc3VyZSB3aGV0aGVyIHRoaXMg QVBJIGV4dGVuc2lvbiBzaG91bGQgYmUgaW4gdGhlIGZvcm0gb2YgYSBuZXcKPiA+IGJyaWRnZSBm dW5jdGlvbiwgb3IgaWYgdGhlIGZvcm1hdHMgc2hvdWxkIGJlIHN0b3JlZCBpbiB0aGUgZHJtX2Jy aWRnZQo+ID4gc3RydWN0dXJlIGRpcmVjdGx5IGFzIGRvbmUgZm9yIGNvbm5lY3RvcnMgaW4gdGhl IGRpc3BsYXkgaW5mbyBzdHJ1Y3R1cmUuCj4gPiBJJ20gdGVtcHRlZCBieSB0aGUgZm9ybWVyLCBi dXQgSSdtIG9wZW4gdG8gZGlzY3Vzc2lvbnMuCj4gCj4gT2ssIEkgY2FuIGxvb2sgaW50byB0aGF0 LCBidXQgbGV0IG1lIGNoZWNrIGlmIEkgZ290IHRoaXMgcmlnaHQuIEZyb20gdGhlCj4gdmVyeSBs aXR0bGUgb2YgdGhlIGNvZGUgdGhhdCBJIGhhdmUgbG9va2VkIGF0LCBJIGhhdmUgZ2F0aGVyZWQg dGhhdCBkaXNwbGF5Cj4gY29udHJvbGxlcnMgaGFuZGxlIGJyaWRnZXMgZXhwbGljaXRseSwgcmln aHQ/CgpUaGF0J3MgY29ycmVjdCwgeWVzLiBPciwgcmF0aGVyLCB0aGV5IGhhbmRsZSB0aGUgZmly c3QgYnJpZGdlIGluIHRoZSBjaGFpbiwgCmFuZCB0aGVuIG90aGVyIGJyaWRnZXMgYXJlIGhhbmRs ZWQgcmVjdXJzaXZlbHkuCgo+IElmIHNvLCBieSBleHRlbmRpbmcgdGhlIGJyaWRnZSAod2l0aCBl aXRoZXIgYSBuZXcgZnVuY3Rpb24gb3IgbmV3IGRhdGEpIHlvdQo+IGltcG9zZSBjaGFuZ2VzIHRv IGFsbCBkaXNwbGF5IGNvbnRyb2xsZXJzIHdhbnRpbmcgdG8gaGFuZGxlIHRoaXMgbmV3IGJyaWRn ZQo+IGlucHV0LWZvcm1hdC4gSWYgc28sIEkgYXNzdW1lIEkgY2FuIGxlYXZlIG91dCB0aGUgY2hh bmdlcyB0byBhbGwgZGlzcGxheQo+IGNvbnRyb2xsZXJzIHRoYXQgSSBkbyBub3QgY2FyZSBhYm91 dC4gQ29ycmVjdD8KClRoYXQncyBjb3JyZWN0LgoKPiBBbHNvLCBkb24ndCBob2xkIHlvdXIgYnJl YXRoIHdhaXRpbmcgZm9yIGEgdjIsIGJ1dCBJJ2xsIHRyeSB0byBnZXQgdG8gaXQgOi0pCgpJIHdv bid0IGhvbGQgbXkgYnJlYXRoLCBidXQgSmFjb3BvIG1pZ2h0IDotKSBIZSBoYXMgYSBzaW1pbGFy IGlzc3VlIHRvIHNvbHZlIAoocmVwb3J0aW5nIHRoZSBMVkRTIG1vZGVzIHN1cHBvcnRlZCBieSB0 aGUgYnJpZGdlKS4KCj4gPj4gLQlyZXR1cm4gZHJtX3BhbmVsX2dldF9tb2RlcyhwYW5lbF9icmlk Z2UtPnBhbmVsKTsKPiA+PiArCXJldHVybiByZXQ7Cj4gPj4gIH0KPiA+PiAgCj4gPj4gIHN0YXRp YyBjb25zdCBzdHJ1Y3QgZHJtX2Nvbm5lY3Rvcl9oZWxwZXJfZnVuY3MKPiA+PiBAQCAtMjAzLDYg KzIxMSwxOCBAQCB2b2lkIGRybV9wYW5lbF9icmlkZ2VfcmVtb3ZlKHN0cnVjdCBkcm1fYnJpZGdl Cj4gPj4gKmJyaWRnZSkKPiA+PiAgfQo+ID4+ICBFWFBPUlRfU1lNQk9MKGRybV9wYW5lbF9icmlk Z2VfcmVtb3ZlKTsKPiA+PiAKPiA+PiArdm9pZCBkcm1fcGFuZWxfYnJpZGdlX3NldF9idXNfZm9y bWF0KHN0cnVjdCBkcm1fYnJpZGdlICpicmlkZ2UsIHUzMgo+ID4+IGJ1c19mb3JtYXQpCj4gPj4g K3sKPiA+PiArCXN0cnVjdCBwYW5lbF9icmlkZ2UgKnBhbmVsX2JyaWRnZTsKPiA+PiArCj4gPj4g KwlpZiAoIWJyaWRnZSkKPiA+PiArCQlyZXR1cm47Cj4gPj4gKwo+ID4+ICsJcGFuZWxfYnJpZGdl ID0gZHJtX2JyaWRnZV90b19wYW5lbF9icmlkZ2UoYnJpZGdlKTsKPiA+PiArCXBhbmVsX2JyaWRn ZS0+YnVzX2Zvcm1hdCA9IGJ1c19mb3JtYXQ7Cj4gPj4gK30KPiA+PiArRVhQT1JUX1NZTUJPTChk cm1fcGFuZWxfYnJpZGdlX3NldF9idXNfZm9ybWF0KTsKPiA+PiArCj4gPj4gIHN0YXRpYyB2b2lk IGRldm1fZHJtX3BhbmVsX2JyaWRnZV9yZWxlYXNlKHN0cnVjdCBkZXZpY2UgKmRldiwgdm9pZCAq cmVzKQo+ID4+ICB7Cj4gPj4gIAlzdHJ1Y3QgZHJtX2JyaWRnZSAqKmJyaWRnZSA9IHJlczsKPiA+ PiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9kcm0vZHJtX2JyaWRnZS5oIGIvaW5jbHVkZS9kcm0vZHJt X2JyaWRnZS5oCj4gPj4gaW5kZXggNjgyZDAxYmE5MjBjLi44MTkwM2I5MmYxODcgMTAwNjQ0Cj4g Pj4gLS0tIGEvaW5jbHVkZS9kcm0vZHJtX2JyaWRnZS5oCj4gPj4gKysrIGIvaW5jbHVkZS9kcm0v ZHJtX2JyaWRnZS5oCj4gPj4gQEAgLTI2OCw2ICsyNjgsNyBAQCB2b2lkIGRybV9icmlkZ2VfZW5h YmxlKHN0cnVjdCBkcm1fYnJpZGdlICpicmlkZ2UpOwo+ID4+ICBzdHJ1Y3QgZHJtX2JyaWRnZSAq ZHJtX3BhbmVsX2JyaWRnZV9hZGQoc3RydWN0IGRybV9wYW5lbCAqcGFuZWwsCj4gPj4gIAkJCQkJ dTMyIGNvbm5lY3Rvcl90eXBlKTsKPiA+PiAgdm9pZCBkcm1fcGFuZWxfYnJpZGdlX3JlbW92ZShz dHJ1Y3QgZHJtX2JyaWRnZSAqYnJpZGdlKTsKPiA+PiArdm9pZCBkcm1fcGFuZWxfYnJpZGdlX3Nl dF9idXNfZm9ybWF0KHN0cnVjdCBkcm1fYnJpZGdlICpicmlkZ2UsIHUzMgo+ID4+IGJ1c19mb3Jt YXQpOwo+ID4+IHN0cnVjdCBkcm1fYnJpZGdlICpkZXZtX2RybV9wYW5lbF9icmlkZ2VfYWRkKHN0 cnVjdCBkZXZpY2UgKmRldiwKPiA+PiAgCQkJCQkgICAgIHN0cnVjdCBkcm1fcGFuZWwgKnBhbmVs LAo+ID4+ICAJCQkJCSAgICAgdTMyIGNvbm5lY3Rvcl90eXBlKTsKCi0tIApSZWdhcmRzLAoKTGF1 cmVudCBQaW5jaGFydAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Au b3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRl dmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752159AbeCZTDg (ORCPT ); Mon, 26 Mar 2018 15:03:36 -0400 Received: from galahad.ideasonboard.com ([185.26.127.97]:44866 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750983AbeCZTDe (ORCPT ); Mon, 26 Mar 2018 15:03:34 -0400 From: Laurent Pinchart To: Peter Rosin Cc: linux-kernel@vger.kernel.org, David Airlie , Rob Herring , Mark Rutland , Archit Taneja , Andrzej Hajda , Daniel Vetter , Gustavo Padovan , Sean Paul , dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, Jacopo Mondi Subject: Re: [RFC PATCH 2/3] drm: bridge: panel: allow override of the bus format Date: Mon, 26 Mar 2018 22:03:31 +0300 Message-ID: <1986009.iWteaM7My3@avalon> Organization: Ideas on Board Oy In-Reply-To: References: <20180317221525.18534-1-peda@axentia.se> <7015227.p44CVkfgjM@avalon> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Peter, (CC'ing Jacopo Mondi) On Sunday, 25 March 2018 15:01:11 EEST Peter Rosin wrote: > On 2018-03-20 14:56, Laurent Pinchart wrote: > > Hi Peter, > > > > Thank you for the patch. > > > > On Sunday, 18 March 2018 00:15:24 EET Peter Rosin wrote: > >> Useful if the bridge does some kind of conversion of the bus format. > >> > >> Signed-off-by: Peter Rosin > >> --- > >> > >> drivers/gpu/drm/bridge/panel.c | 22 +++++++++++++++++++++- > >> include/drm/drm_bridge.h | 1 + > >> 2 files changed, 22 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/bridge/panel.c > >> b/drivers/gpu/drm/bridge/panel.c index 6d99d4a3beb3..ccef0283ff41 100644 > >> --- a/drivers/gpu/drm/bridge/panel.c > >> +++ b/drivers/gpu/drm/bridge/panel.c > >> @@ -22,6 +22,7 @@ struct panel_bridge { > >> struct drm_connector connector; > >> struct drm_panel *panel; > >> u32 connector_type; > >> + u32 bus_format; > >> }; > >> > >> static inline struct panel_bridge * > >> @@ -40,8 +41,15 @@ static int panel_bridge_connector_get_modes(struct > >> drm_connector *connector) { > >> struct panel_bridge *panel_bridge = > >> drm_connector_to_panel_bridge(connector); > >> > >> + int ret; > >> + > >> + ret = drm_panel_get_modes(panel_bridge->panel); > >> + > >> + if (panel_bridge->bus_format) > >> + drm_display_info_set_bus_formats(&connector->display_info, > >> + &panel_bridge->bus_format, 1); > > > > While I agree with the problem statement and, to some extent, the DT > > bindings, I don't think this is the right implementation. You've > > correctly noted that display controller shouldn't blindly use the formats > > reported by the panel through the connector formats, and that hacking the > > panel driver to override the formats isn't a good idea, so I wouldn't > > override the formats reported by the connector. I would instead extend > > the drm_bridge API to report formats at bridge inputs. This would be more > > generic and allow each bridge to configure itself according to the next > > bridge in the chain. > > > > I'm not sure whether this API extension should be in the form of a new > > bridge function, or if the formats should be stored in the drm_bridge > > structure directly as done for connectors in the display info structure. > > I'm tempted by the former, but I'm open to discussions. > > Ok, I can look into that, but let me check if I got this right. From the > very little of the code that I have looked at, I have gathered that display > controllers handle bridges explicitly, right? That's correct, yes. Or, rather, they handle the first bridge in the chain, and then other bridges are handled recursively. > If so, by extending the bridge (with either a new function or new data) you > impose changes to all display controllers wanting to handle this new bridge > input-format. If so, I assume I can leave out the changes to all display > controllers that I do not care about. Correct? That's correct. > Also, don't hold your breath waiting for a v2, but I'll try to get to it :-) I won't hold my breath, but Jacopo might :-) He has a similar issue to solve (reporting the LVDS modes supported by the bridge). > >> - return drm_panel_get_modes(panel_bridge->panel); > >> + return ret; > >> } > >> > >> static const struct drm_connector_helper_funcs > >> @@ -203,6 +211,18 @@ void drm_panel_bridge_remove(struct drm_bridge > >> *bridge) > >> } > >> EXPORT_SYMBOL(drm_panel_bridge_remove); > >> > >> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32 > >> bus_format) > >> +{ > >> + struct panel_bridge *panel_bridge; > >> + > >> + if (!bridge) > >> + return; > >> + > >> + panel_bridge = drm_bridge_to_panel_bridge(bridge); > >> + panel_bridge->bus_format = bus_format; > >> +} > >> +EXPORT_SYMBOL(drm_panel_bridge_set_bus_format); > >> + > >> static void devm_drm_panel_bridge_release(struct device *dev, void *res) > >> { > >> struct drm_bridge **bridge = res; > >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > >> index 682d01ba920c..81903b92f187 100644 > >> --- a/include/drm/drm_bridge.h > >> +++ b/include/drm/drm_bridge.h > >> @@ -268,6 +268,7 @@ void drm_bridge_enable(struct drm_bridge *bridge); > >> struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel, > >> u32 connector_type); > >> void drm_panel_bridge_remove(struct drm_bridge *bridge); > >> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32 > >> bus_format); > >> struct drm_bridge *devm_drm_panel_bridge_add(struct device *dev, > >> struct drm_panel *panel, > >> u32 connector_type); -- Regards, Laurent Pinchart