From mboxrd@z Thu Jan 1 00:00:00 1970 From: laurent.pinchart@ideasonboard.com (Laurent Pinchart) Date: Wed, 04 Apr 2018 01:28:29 +0300 Subject: [PATCH v2 0/5] allow override of bus format in bridges In-Reply-To: <20180328070826.GY14155@phenom.ffwll.local> References: <20180326212447.7380-1-peda@axentia.se> <20180328070826.GY14155@phenom.ffwll.local> Message-ID: <2233231.my6BbD3pcT@avalon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Daniel, On Wednesday, 28 March 2018 10:08:26 EEST Daniel Vetter wrote: > On Mon, Mar 26, 2018 at 11:24:42PM +0200, Peter Rosin wrote: > > Hi! > > > > [I got to v2 sooner than expected] > > > > I have an Atmel sama5d31 hooked up to an lvds encoder and then > > on to an lvds panel. Which seems like something that has been > > done one or two times before... > > > > The problem is that the bus_format of the SoC and the panel do > > not agree. The SoC driver (atmel-hlcdc) can handle the > > rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is > > wired for the rgb565 case. The lvds encoder supports rgb888 on > > its input side with the LSB wires for each color simply pulled > > down internally in the encoder in my case which means that the > > rgb565 bus_format is the format that works best. And the panel > > is expecting lvds (vesa-24), which is what the encoder outputs. > > > > The reason I "blame" the bus_format of the drm_connector is that > > with the below DT snippet, things do not work *exactly* due to > > that. At least, it starts to work if I hack the panel-lvds driver > > to report the rgb565 bus_format instead of vesa-24. > > > > panel: panel { > > compatible = "panel-lvds"; > > > > width-mm = <304>; > > height-mm = <228; > > > > data-mapping = "vesa-24"; > > > > panel-timing { > > // 1024x768 @ 60Hz (typical) > > clock-frequency = <52140000 65000000 71100000>; > > hactive = <1024>; > > vactive = <768>; > > hfront-porch = <48 88 88>; > > hback-porch = <96 168 168>; > > hsync-len = <32 64 64>; > > vfront-porch = <8 13 14>; > > vback-porch = <8 13 14>; > > vsync-len = <8 12 14>; > > }; > > > > port { > > panel_input: endpoint { > > remote-endpoint = <&lvds_encoder_output>; > > }; > > }; > > }; > > > > lvds-encoder { > > compatible = "ti,ds90c185", "lvds-encoder"; > > > > ports { > > #address-cells = <1>; > > #size-cells = <0>; > > > > port at 0 { > > reg = <0>; > > > > lvds_encoder_input: endpoint { > > remote-endpoint = <&hlcdc_output>; > > }; > > }; > > > > port at 1 { > > reg = <1>; > > > > lvds_encoder_output: endpoint { > > remote-endpoint = <&panel_input>; > > }; > > }; > > }; > > }; > > > > But, instead of perverting the panel-lvds driver with support > > for a totally fake non-lvds bus_format, I intruduce an API that allows > > display controller drivers to query the required bus_format of any > > intermediate bridges, and match up with that instead of the formats > > given by the drm_connector. I trigger this with this addition to the > > > > lvds-encoder DT node: > > interface-pix-fmt = "rgb565"; > > > > Naming is hard though, so I'm not sure if that's good? > > > > I threw in the first patch, since that is the actual lvds encoder > > I have in this case. > > > > Suggestions welcome. > > Took a quick look, feels rather un-atomic. And there's beend discussing > for other bridge related state that we might want to track (like the full > adjusted_mode that might need to be adjusted at each stage in the chain). > So here's my suggestions: > > - Add an optional per-bridge internal state struct using the support in > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#handling-driver-privat > e-state > > Yes it says "driver private", but since bridge is just helper stuff > that's all included. "driver private" == "not exposed as uapi" here. > Include all the usual convenience wrappers to get at the state for a > bridge. > > - Then stuff your bus_format into that new drm_bridge_state struct. > > - Add a new bridge callback atomic_check, which gets that bridge state as > parameter (similar to all the other atomic_check functions). > > This way we can even handle the bus_format dynamically, through the atomic > framework your bridge's atomic_check callback can look at the entire > atomic state (both up and down the chain if needed), it all neatly fits > into atomic overall and it's much easier to extend. While I think we'll eventually need bridge states, I don't think that's need yet. The bus formats reported by this patch series are static. We're not talking about the currently configured format for a bridge, but about the list of supported formats. This is similar to the bus_formats field present in the drm_display_info structure. There is thus in my opinion no need to interface this with atomic until we need to track the current format (and I think that will indeed happen at some point, but I don't think Peter needs this feature for now). That's why I've told Peter that I would like a bridge API to report the information and haven't requested a state-based implementation. > Please also cc Laurent Pinchart on this. > > > Changes since v1 https://lkml.org/lkml/2018/3/17/221 > > - Add a proper bridge API to query the bus_format instead of abusing > > the ->get_modes part of the code. This is cleaner but requires > > changes to all display controller drivers wishing to participate. > > - Add patch to adjust the atmel-hlcdc driver according to the above. > > - Hook the new info into the bridge local to the lvds-encoder instead > > of messing about with new interfaces for the panel-bridge driver. > > - Add patch with a DT parsing function of bus_formats in a central place. > > - Rephrase the addition of ti,ds90c185 to the lvds-transmitter binding. > > > > Peter Rosin (5): > > dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c185 > > drm: bridge: add API to query the expected input formats of bridges > > drm: of: add display bus-format parser > > drm: bridge: lvds-encoder: allow specifying the input bus format > > drm/atmel-hlcdc: take bridges into account when selecting output > > format > > > > .../bindings/display/bridge/lvds-transmitter.txt | 14 ++++- > > .../devicetree/bindings/display/bus-format.txt | 35 +++++++++++++ > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 49 +++++++++++++++-- > > drivers/gpu/drm/bridge/lvds-encoder.c | 25 +++++++++ > > drivers/gpu/drm/drm_bridge.c | 32 ++++++++++++ > > drivers/gpu/drm/drm_of.c | 59 +++++++++++++++++ > > include/drm/drm_bridge.h | 18 +++++++ > > include/drm/drm_of.h | 9 ++++ > > 8 files changed, 237 insertions(+), 4 deletions(-) > > create mode 100644 > > Documentation/devicetree/bindings/display/bus-format.txt -- Regards, Laurent Pinchart From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v2 0/5] allow override of bus format in bridges Date: Wed, 04 Apr 2018 01:28:29 +0300 Message-ID: <2233231.my6BbD3pcT@avalon> References: <20180326212447.7380-1-peda@axentia.se> <20180328070826.GY14155@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20180328070826.GY14155@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: Mark Rutland , Boris Brezillon , Alexandre Belloni , devicetree@vger.kernel.org, David Airlie , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Nicolas Ferre , Rob Herring , Jacopo Mondi , Daniel Vetter , Peter Rosin , linux-arm-kernel@lists.infradead.org List-Id: devicetree@vger.kernel.org SGkgRGFuaWVsLAoKT24gV2VkbmVzZGF5LCAyOCBNYXJjaCAyMDE4IDEwOjA4OjI2IEVFU1QgRGFu aWVsIFZldHRlciB3cm90ZToKPiBPbiBNb24sIE1hciAyNiwgMjAxOCBhdCAxMToyNDo0MlBNICsw MjAwLCBQZXRlciBSb3NpbiB3cm90ZToKPiA+IEhpIQo+ID4gCj4gPiBbSSBnb3QgdG8gdjIgc29v bmVyIHRoYW4gZXhwZWN0ZWRdCj4gPiAKPiA+IEkgaGF2ZSBhbiBBdG1lbCBzYW1hNWQzMSBob29r ZWQgdXAgdG8gYW4gbHZkcyBlbmNvZGVyIGFuZCB0aGVuCj4gPiBvbiB0byBhbiBsdmRzIHBhbmVs LiBXaGljaCBzZWVtcyBsaWtlIHNvbWV0aGluZyB0aGF0IGhhcyBiZWVuCj4gPiBkb25lIG9uZSBv ciB0d28gdGltZXMgYmVmb3JlLi4uCj4gPiAKPiA+IFRoZSBwcm9ibGVtIGlzIHRoYXQgdGhlIGJ1 c19mb3JtYXQgb2YgdGhlIFNvQyBhbmQgdGhlIHBhbmVsIGRvCj4gPiBub3QgYWdyZWUuIFRoZSBT b0MgZHJpdmVyIChhdG1lbC1obGNkYykgY2FuIGhhbmRsZSB0aGUKPiA+IHJnYjQ0NCwgcmdiNTY1 LCByZ2I2NjYgYW5kIHJnYjg4OCBidXMgZm9ybWF0cy4gVGhlIGhhcmR3YXJlIGlzCj4gPiB3aXJl ZCBmb3IgdGhlIHJnYjU2NSBjYXNlLiBUaGUgbHZkcyBlbmNvZGVyIHN1cHBvcnRzIHJnYjg4OCBv bgo+ID4gaXRzIGlucHV0IHNpZGUgd2l0aCB0aGUgTFNCIHdpcmVzIGZvciBlYWNoIGNvbG9yIHNp bXBseSBwdWxsZWQKPiA+IGRvd24gaW50ZXJuYWxseSBpbiB0aGUgZW5jb2RlciBpbiBteSBjYXNl IHdoaWNoIG1lYW5zIHRoYXQgdGhlCj4gPiByZ2I1NjUgYnVzX2Zvcm1hdCBpcyB0aGUgZm9ybWF0 IHRoYXQgd29ya3MgYmVzdC4gQW5kIHRoZSBwYW5lbAo+ID4gaXMgZXhwZWN0aW5nIGx2ZHMgKHZl c2EtMjQpLCB3aGljaCBpcyB3aGF0IHRoZSBlbmNvZGVyIG91dHB1dHMuCj4gPiAKPiA+IFRoZSBy ZWFzb24gSSAiYmxhbWUiIHRoZSBidXNfZm9ybWF0IG9mIHRoZSBkcm1fY29ubmVjdG9yIGlzIHRo YXQKPiA+IHdpdGggdGhlIGJlbG93IERUIHNuaXBwZXQsIHRoaW5ncyBkbyBub3Qgd29yayAqZXhh Y3RseSogZHVlIHRvCj4gPiB0aGF0LiBBdCBsZWFzdCwgaXQgc3RhcnRzIHRvIHdvcmsgaWYgSSBo YWNrIHRoZSBwYW5lbC1sdmRzIGRyaXZlcgo+ID4gdG8gcmVwb3J0IHRoZSByZ2I1NjUgYnVzX2Zv cm1hdCBpbnN0ZWFkIG9mIHZlc2EtMjQuCj4gPiAKPiA+IAlwYW5lbDogcGFuZWwgewo+ID4gCQlj b21wYXRpYmxlID0gInBhbmVsLWx2ZHMiOwo+ID4gCQkKPiA+IAkJd2lkdGgtbW0gPSA8MzA0PjsK PiA+IAkJaGVpZ2h0LW1tID0gPDIyODsKPiA+IAkJCj4gPiAJCWRhdGEtbWFwcGluZyA9ICJ2ZXNh LTI0IjsKPiA+IAkJCj4gPiAJCXBhbmVsLXRpbWluZyB7Cj4gPiAJCQkvLyAxMDI0eDc2OCBAIDYw SHogKHR5cGljYWwpCj4gPiAJCQljbG9jay1mcmVxdWVuY3kgPSA8NTIxNDAwMDAgNjUwMDAwMDAg NzExMDAwMDA+Owo+ID4gCQkJaGFjdGl2ZSA9IDwxMDI0PjsKPiA+IAkJCXZhY3RpdmUgPSA8NzY4 PjsKPiA+IAkJCWhmcm9udC1wb3JjaCA9IDw0OCA4OCA4OD47Cj4gPiAJCQloYmFjay1wb3JjaCA9 IDw5NiAxNjggMTY4PjsKPiA+IAkJCWhzeW5jLWxlbiA9IDwzMiA2NCA2ND47Cj4gPiAJCQl2ZnJv bnQtcG9yY2ggPSA8OCAxMyAxND47Cj4gPiAJCQl2YmFjay1wb3JjaCA9IDw4IDEzIDE0PjsKPiA+ IAkJCXZzeW5jLWxlbiA9IDw4IDEyIDE0PjsKPiA+IAkJfTsKPiA+IAkJCj4gPiAJCXBvcnQgewo+ ID4gCQkJcGFuZWxfaW5wdXQ6IGVuZHBvaW50IHsKPiA+IAkJCQlyZW1vdGUtZW5kcG9pbnQgPSA8 Jmx2ZHNfZW5jb2Rlcl9vdXRwdXQ+Owo+ID4gCQkJfTsKPiA+IAkJfTsKPiA+IAl9Owo+ID4gCQo+ ID4gCWx2ZHMtZW5jb2RlciB7Cj4gPiAJCWNvbXBhdGlibGUgPSAidGksZHM5MGMxODUiLCAibHZk cy1lbmNvZGVyIjsKPiA+IAkJCj4gPiAJCXBvcnRzIHsKPiA+IAkJCSNhZGRyZXNzLWNlbGxzID0g PDE+Owo+ID4gCQkJI3NpemUtY2VsbHMgPSA8MD47Cj4gPiAJCQkKPiA+IAkJCXBvcnRAMCB7Cj4g PiAJCQkJcmVnID0gPDA+Owo+ID4gCQkJCQo+ID4gCQkJCWx2ZHNfZW5jb2Rlcl9pbnB1dDogZW5k cG9pbnQgewo+ID4gCQkJCQlyZW1vdGUtZW5kcG9pbnQgPSA8JmhsY2RjX291dHB1dD47Cj4gPiAJ CQkJfTsKPiA+IAkJCX07Cj4gPiAJCQkKPiA+IAkJCXBvcnRAMSB7Cj4gPiAJCQkJcmVnID0gPDE+ Owo+ID4gCQkJCQo+ID4gCQkJCWx2ZHNfZW5jb2Rlcl9vdXRwdXQ6IGVuZHBvaW50IHsKPiA+IAkJ CQkJcmVtb3RlLWVuZHBvaW50ID0gPCZwYW5lbF9pbnB1dD47Cj4gPiAJCQkJfTsKPiA+IAkJCX07 Cj4gPiAJCX07Cj4gPiAJfTsKPiA+IAo+ID4gQnV0LCBpbnN0ZWFkIG9mIHBlcnZlcnRpbmcgdGhl IHBhbmVsLWx2ZHMgZHJpdmVyIHdpdGggc3VwcG9ydAo+ID4gZm9yIGEgdG90YWxseSBmYWtlIG5v bi1sdmRzIGJ1c19mb3JtYXQsIEkgaW50cnVkdWNlIGFuIEFQSSB0aGF0IGFsbG93cwo+ID4gZGlz cGxheSBjb250cm9sbGVyIGRyaXZlcnMgdG8gcXVlcnkgdGhlIHJlcXVpcmVkIGJ1c19mb3JtYXQg b2YgYW55Cj4gPiBpbnRlcm1lZGlhdGUgYnJpZGdlcywgYW5kIG1hdGNoIHVwIHdpdGggdGhhdCBp bnN0ZWFkIG9mIHRoZSBmb3JtYXRzCj4gPiBnaXZlbiBieSB0aGUgZHJtX2Nvbm5lY3Rvci4gSSB0 cmlnZ2VyIHRoaXMgd2l0aCB0aGlzIGFkZGl0aW9uIHRvIHRoZQo+ID4gCj4gPiBsdmRzLWVuY29k ZXIgRFQgbm9kZToKPiA+IAkJaW50ZXJmYWNlLXBpeC1mbXQgPSAicmdiNTY1IjsKPiA+IAo+ID4g TmFtaW5nIGlzIGhhcmQgdGhvdWdoLCBzbyBJJ20gbm90IHN1cmUgaWYgdGhhdCdzIGdvb2Q/Cj4g PiAKPiA+IEkgdGhyZXcgaW4gdGhlIGZpcnN0IHBhdGNoLCBzaW5jZSB0aGF0IGlzIHRoZSBhY3R1 YWwgbHZkcyBlbmNvZGVyCj4gPiBJIGhhdmUgaW4gdGhpcyBjYXNlLgo+ID4gCj4gPiBTdWdnZXN0 aW9ucyB3ZWxjb21lLgo+IAo+IFRvb2sgYSBxdWljayBsb29rLCBmZWVscyByYXRoZXIgdW4tYXRv bWljLiBBbmQgdGhlcmUncyBiZWVuZCBkaXNjdXNzaW5nCj4gZm9yIG90aGVyIGJyaWRnZSByZWxh dGVkIHN0YXRlIHRoYXQgd2UgbWlnaHQgd2FudCB0byB0cmFjayAobGlrZSB0aGUgZnVsbAo+IGFk anVzdGVkX21vZGUgdGhhdCBtaWdodCBuZWVkIHRvIGJlIGFkanVzdGVkIGF0IGVhY2ggc3RhZ2Ug aW4gdGhlIGNoYWluKS4KPiBTbyBoZXJlJ3MgbXkgc3VnZ2VzdGlvbnM6Cj4gCj4gLSBBZGQgYW4g b3B0aW9uYWwgcGVyLWJyaWRnZSBpbnRlcm5hbCBzdGF0ZSBzdHJ1Y3QgdXNpbmcgdGhlIHN1cHBv cnQgaW4KPiAKPiBodHRwczovL2RyaS5mcmVlZGVza3RvcC5vcmcvZG9jcy9kcm0vZ3B1L2RybS1r bXMuaHRtbCNoYW5kbGluZy1kcml2ZXItcHJpdmF0Cj4gZS1zdGF0ZQo+IAo+ICAgWWVzIGl0IHNh eXMgImRyaXZlciBwcml2YXRlIiwgYnV0IHNpbmNlIGJyaWRnZSBpcyBqdXN0IGhlbHBlciBzdHVm Zgo+ICAgdGhhdCdzIGFsbCBpbmNsdWRlZC4gImRyaXZlciBwcml2YXRlIiA9PSAibm90IGV4cG9z ZWQgYXMgdWFwaSIgaGVyZS4KPiAgIEluY2x1ZGUgYWxsIHRoZSB1c3VhbCBjb252ZW5pZW5jZSB3 cmFwcGVycyB0byBnZXQgYXQgdGhlIHN0YXRlIGZvciBhCj4gICBicmlkZ2UuCj4gCj4gLSBUaGVu IHN0dWZmIHlvdXIgYnVzX2Zvcm1hdCBpbnRvIHRoYXQgbmV3IGRybV9icmlkZ2Vfc3RhdGUgc3Ry dWN0Lgo+IAo+IC0gQWRkIGEgbmV3IGJyaWRnZSBjYWxsYmFjayBhdG9taWNfY2hlY2ssIHdoaWNo IGdldHMgdGhhdCBicmlkZ2Ugc3RhdGUgYXMKPiAgIHBhcmFtZXRlciAoc2ltaWxhciB0byBhbGwg dGhlIG90aGVyIGF0b21pY19jaGVjayBmdW5jdGlvbnMpLgo+IAo+IFRoaXMgd2F5IHdlIGNhbiBl dmVuIGhhbmRsZSB0aGUgYnVzX2Zvcm1hdCBkeW5hbWljYWxseSwgdGhyb3VnaCB0aGUgYXRvbWlj Cj4gZnJhbWV3b3JrIHlvdXIgYnJpZGdlJ3MgYXRvbWljX2NoZWNrIGNhbGxiYWNrIGNhbiBsb29r IGF0IHRoZSBlbnRpcmUKPiBhdG9taWMgc3RhdGUgKGJvdGggdXAgYW5kIGRvd24gdGhlIGNoYWlu IGlmIG5lZWRlZCksIGl0IGFsbCBuZWF0bHkgZml0cwo+IGludG8gYXRvbWljIG92ZXJhbGwgYW5k IGl0J3MgbXVjaCBlYXNpZXIgdG8gZXh0ZW5kLgoKV2hpbGUgSSB0aGluayB3ZSdsbCBldmVudHVh bGx5IG5lZWQgYnJpZGdlIHN0YXRlcywgSSBkb24ndCB0aGluayB0aGF0J3MgbmVlZCAKeWV0LiBU aGUgYnVzIGZvcm1hdHMgcmVwb3J0ZWQgYnkgdGhpcyBwYXRjaCBzZXJpZXMgYXJlIHN0YXRpYy4g V2UncmUgbm90IAp0YWxraW5nIGFib3V0IHRoZSBjdXJyZW50bHkgY29uZmlndXJlZCBmb3JtYXQg Zm9yIGEgYnJpZGdlLCBidXQgYWJvdXQgdGhlIGxpc3QgCm9mIHN1cHBvcnRlZCBmb3JtYXRzLiBU aGlzIGlzIHNpbWlsYXIgdG8gdGhlIGJ1c19mb3JtYXRzIGZpZWxkIHByZXNlbnQgaW4gdGhlIApk cm1fZGlzcGxheV9pbmZvIHN0cnVjdHVyZS4gVGhlcmUgaXMgdGh1cyBpbiBteSBvcGluaW9uIG5v IG5lZWQgdG8gaW50ZXJmYWNlIAp0aGlzIHdpdGggYXRvbWljIHVudGlsIHdlIG5lZWQgdG8gdHJh Y2sgdGhlIGN1cnJlbnQgZm9ybWF0IChhbmQgSSB0aGluayB0aGF0IAp3aWxsIGluZGVlZCBoYXBw ZW4gYXQgc29tZSBwb2ludCwgYnV0IEkgZG9uJ3QgdGhpbmsgUGV0ZXIgbmVlZHMgdGhpcyBmZWF0 dXJlIApmb3Igbm93KS4gVGhhdCdzIHdoeSBJJ3ZlIHRvbGQgUGV0ZXIgdGhhdCBJIHdvdWxkIGxp a2UgYSBicmlkZ2UgQVBJIHRvIHJlcG9ydCAKdGhlIGluZm9ybWF0aW9uIGFuZCBoYXZlbid0IHJl cXVlc3RlZCBhIHN0YXRlLWJhc2VkIGltcGxlbWVudGF0aW9uLgoKPiBQbGVhc2UgYWxzbyBjYyBM YXVyZW50IFBpbmNoYXJ0IG9uIHRoaXMuCj4gCj4gPiBDaGFuZ2VzIHNpbmNlIHYxIGh0dHBzOi8v bGttbC5vcmcvbGttbC8yMDE4LzMvMTcvMjIxCj4gPiAtIEFkZCBhIHByb3BlciBicmlkZ2UgQVBJ IHRvIHF1ZXJ5IHRoZSBidXNfZm9ybWF0IGluc3RlYWQgb2YgYWJ1c2luZwo+ID4gICB0aGUgLT5n ZXRfbW9kZXMgcGFydCBvZiB0aGUgY29kZS4gVGhpcyBpcyBjbGVhbmVyIGJ1dCByZXF1aXJlcwo+ ID4gICBjaGFuZ2VzIHRvIGFsbCBkaXNwbGF5IGNvbnRyb2xsZXIgZHJpdmVycyB3aXNoaW5nIHRv IHBhcnRpY2lwYXRlLgo+ID4gLSBBZGQgcGF0Y2ggdG8gYWRqdXN0IHRoZSBhdG1lbC1obGNkYyBk cml2ZXIgYWNjb3JkaW5nIHRvIHRoZSBhYm92ZS4KPiA+IC0gSG9vayB0aGUgbmV3IGluZm8gaW50 byB0aGUgYnJpZGdlIGxvY2FsIHRvIHRoZSBsdmRzLWVuY29kZXIgaW5zdGVhZAo+ID4gICBvZiBt ZXNzaW5nIGFib3V0IHdpdGggbmV3IGludGVyZmFjZXMgZm9yIHRoZSBwYW5lbC1icmlkZ2UgZHJp dmVyLgo+ID4gLSBBZGQgcGF0Y2ggd2l0aCBhIERUIHBhcnNpbmcgZnVuY3Rpb24gb2YgYnVzX2Zv cm1hdHMgaW4gYSBjZW50cmFsIHBsYWNlLgo+ID4gLSBSZXBocmFzZSB0aGUgYWRkaXRpb24gb2Yg dGksZHM5MGMxODUgdG8gdGhlIGx2ZHMtdHJhbnNtaXR0ZXIgYmluZGluZy4KPiA+IAo+ID4gUGV0 ZXIgUm9zaW4gKDUpOgo+ID4gICBkdC1iaW5kaW5nczogZGlzcGxheTogYnJpZGdlOiBsdmRzLXRy YW5zbWl0dGVyOiBhZGQgdGksZHM5MGMxODUKPiA+ICAgZHJtOiBicmlkZ2U6IGFkZCBBUEkgdG8g cXVlcnkgdGhlIGV4cGVjdGVkIGlucHV0IGZvcm1hdHMgb2YgYnJpZGdlcwo+ID4gICBkcm06IG9m OiBhZGQgZGlzcGxheSBidXMtZm9ybWF0IHBhcnNlcgo+ID4gICBkcm06IGJyaWRnZTogbHZkcy1l bmNvZGVyOiBhbGxvdyBzcGVjaWZ5aW5nIHRoZSBpbnB1dCBidXMgZm9ybWF0Cj4gPiAgIGRybS9h dG1lbC1obGNkYzogdGFrZSBicmlkZ2VzIGludG8gYWNjb3VudCB3aGVuIHNlbGVjdGluZyBvdXRw dXQKPiA+ICAgICBmb3JtYXQKPiA+ICAKPiA+ICAuLi4vYmluZGluZ3MvZGlzcGxheS9icmlkZ2Uv bHZkcy10cmFuc21pdHRlci50eHQgICB8IDE0ICsrKystCj4gPiAgLi4uL2RldmljZXRyZWUvYmlu ZGluZ3MvZGlzcGxheS9idXMtZm9ybWF0LnR4dCAgICAgfCAzNSArKysrKysrKysrKysrCj4gPiAg ZHJpdmVycy9ncHUvZHJtL2F0bWVsLWhsY2RjL2F0bWVsX2hsY2RjX2NydGMuYyAgICAgfCA0OSAr KysrKysrKysrKysrKystLQo+ID4gIGRyaXZlcnMvZ3B1L2RybS9icmlkZ2UvbHZkcy1lbmNvZGVy LmMgICAgICAgICAgICAgIHwgMjUgKysrKysrKysrCj4gPiAgZHJpdmVycy9ncHUvZHJtL2RybV9i cmlkZ2UuYyAgICAgICAgICAgICAgICAgICAgICAgfCAzMiArKysrKysrKysrKysKPiA+ICBkcml2 ZXJzL2dwdS9kcm0vZHJtX29mLmMgICAgICAgICAgICAgICAgICAgICAgICAgICB8IDU5ICsrKysr KysrKysrKysrKysrCj4gPiAgaW5jbHVkZS9kcm0vZHJtX2JyaWRnZS5oICAgICAgICAgICAgICAg ICAgICAgICAgICAgfCAxOCArKysrKysrCj4gPiAgaW5jbHVkZS9kcm0vZHJtX29mLmggICAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgfCAgOSArKysrCj4gPiAgOCBmaWxlcyBjaGFuZ2VkLCAy MzcgaW5zZXJ0aW9ucygrKSwgNCBkZWxldGlvbnMoLSkKPiA+ICBjcmVhdGUgbW9kZSAxMDA2NDQK PiA+ICBEb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvZGlzcGxheS9idXMtZm9ybWF0 LnR4dAoKLS0gClJlZ2FyZHMsCgpMYXVyZW50IFBpbmNoYXJ0CgoKCl9fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKZHJp LWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756719AbeDCWfL (ORCPT ); Tue, 3 Apr 2018 18:35:11 -0400 Received: from galahad.ideasonboard.com ([185.26.127.97]:42823 "EHLO galahad.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755582AbeDCW2V (ORCPT ); Tue, 3 Apr 2018 18:28:21 -0400 From: Laurent Pinchart To: Daniel Vetter Cc: Peter Rosin , linux-kernel@vger.kernel.org, Mark Rutland , Boris Brezillon , Alexandre Belloni , devicetree@vger.kernel.org, David Airlie , Nicolas Ferre , dri-devel@lists.freedesktop.org, Rob Herring , Jacopo Mondi , Daniel Vetter , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 0/5] allow override of bus format in bridges Date: Wed, 04 Apr 2018 01:28:29 +0300 Message-ID: <2233231.my6BbD3pcT@avalon> Organization: Ideas on Board Oy In-Reply-To: <20180328070826.GY14155@phenom.ffwll.local> References: <20180326212447.7380-1-peda@axentia.se> <20180328070826.GY14155@phenom.ffwll.local> 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 Daniel, On Wednesday, 28 March 2018 10:08:26 EEST Daniel Vetter wrote: > On Mon, Mar 26, 2018 at 11:24:42PM +0200, Peter Rosin wrote: > > Hi! > > > > [I got to v2 sooner than expected] > > > > I have an Atmel sama5d31 hooked up to an lvds encoder and then > > on to an lvds panel. Which seems like something that has been > > done one or two times before... > > > > The problem is that the bus_format of the SoC and the panel do > > not agree. The SoC driver (atmel-hlcdc) can handle the > > rgb444, rgb565, rgb666 and rgb888 bus formats. The hardware is > > wired for the rgb565 case. The lvds encoder supports rgb888 on > > its input side with the LSB wires for each color simply pulled > > down internally in the encoder in my case which means that the > > rgb565 bus_format is the format that works best. And the panel > > is expecting lvds (vesa-24), which is what the encoder outputs. > > > > The reason I "blame" the bus_format of the drm_connector is that > > with the below DT snippet, things do not work *exactly* due to > > that. At least, it starts to work if I hack the panel-lvds driver > > to report the rgb565 bus_format instead of vesa-24. > > > > panel: panel { > > compatible = "panel-lvds"; > > > > width-mm = <304>; > > height-mm = <228; > > > > data-mapping = "vesa-24"; > > > > panel-timing { > > // 1024x768 @ 60Hz (typical) > > clock-frequency = <52140000 65000000 71100000>; > > hactive = <1024>; > > vactive = <768>; > > hfront-porch = <48 88 88>; > > hback-porch = <96 168 168>; > > hsync-len = <32 64 64>; > > vfront-porch = <8 13 14>; > > vback-porch = <8 13 14>; > > vsync-len = <8 12 14>; > > }; > > > > port { > > panel_input: endpoint { > > remote-endpoint = <&lvds_encoder_output>; > > }; > > }; > > }; > > > > lvds-encoder { > > compatible = "ti,ds90c185", "lvds-encoder"; > > > > ports { > > #address-cells = <1>; > > #size-cells = <0>; > > > > port@0 { > > reg = <0>; > > > > lvds_encoder_input: endpoint { > > remote-endpoint = <&hlcdc_output>; > > }; > > }; > > > > port@1 { > > reg = <1>; > > > > lvds_encoder_output: endpoint { > > remote-endpoint = <&panel_input>; > > }; > > }; > > }; > > }; > > > > But, instead of perverting the panel-lvds driver with support > > for a totally fake non-lvds bus_format, I intruduce an API that allows > > display controller drivers to query the required bus_format of any > > intermediate bridges, and match up with that instead of the formats > > given by the drm_connector. I trigger this with this addition to the > > > > lvds-encoder DT node: > > interface-pix-fmt = "rgb565"; > > > > Naming is hard though, so I'm not sure if that's good? > > > > I threw in the first patch, since that is the actual lvds encoder > > I have in this case. > > > > Suggestions welcome. > > Took a quick look, feels rather un-atomic. And there's beend discussing > for other bridge related state that we might want to track (like the full > adjusted_mode that might need to be adjusted at each stage in the chain). > So here's my suggestions: > > - Add an optional per-bridge internal state struct using the support in > > https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#handling-driver-privat > e-state > > Yes it says "driver private", but since bridge is just helper stuff > that's all included. "driver private" == "not exposed as uapi" here. > Include all the usual convenience wrappers to get at the state for a > bridge. > > - Then stuff your bus_format into that new drm_bridge_state struct. > > - Add a new bridge callback atomic_check, which gets that bridge state as > parameter (similar to all the other atomic_check functions). > > This way we can even handle the bus_format dynamically, through the atomic > framework your bridge's atomic_check callback can look at the entire > atomic state (both up and down the chain if needed), it all neatly fits > into atomic overall and it's much easier to extend. While I think we'll eventually need bridge states, I don't think that's need yet. The bus formats reported by this patch series are static. We're not talking about the currently configured format for a bridge, but about the list of supported formats. This is similar to the bus_formats field present in the drm_display_info structure. There is thus in my opinion no need to interface this with atomic until we need to track the current format (and I think that will indeed happen at some point, but I don't think Peter needs this feature for now). That's why I've told Peter that I would like a bridge API to report the information and haven't requested a state-based implementation. > Please also cc Laurent Pinchart on this. > > > Changes since v1 https://lkml.org/lkml/2018/3/17/221 > > - Add a proper bridge API to query the bus_format instead of abusing > > the ->get_modes part of the code. This is cleaner but requires > > changes to all display controller drivers wishing to participate. > > - Add patch to adjust the atmel-hlcdc driver according to the above. > > - Hook the new info into the bridge local to the lvds-encoder instead > > of messing about with new interfaces for the panel-bridge driver. > > - Add patch with a DT parsing function of bus_formats in a central place. > > - Rephrase the addition of ti,ds90c185 to the lvds-transmitter binding. > > > > Peter Rosin (5): > > dt-bindings: display: bridge: lvds-transmitter: add ti,ds90c185 > > drm: bridge: add API to query the expected input formats of bridges > > drm: of: add display bus-format parser > > drm: bridge: lvds-encoder: allow specifying the input bus format > > drm/atmel-hlcdc: take bridges into account when selecting output > > format > > > > .../bindings/display/bridge/lvds-transmitter.txt | 14 ++++- > > .../devicetree/bindings/display/bus-format.txt | 35 +++++++++++++ > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 49 +++++++++++++++-- > > drivers/gpu/drm/bridge/lvds-encoder.c | 25 +++++++++ > > drivers/gpu/drm/drm_bridge.c | 32 ++++++++++++ > > drivers/gpu/drm/drm_of.c | 59 +++++++++++++++++ > > include/drm/drm_bridge.h | 18 +++++++ > > include/drm/drm_of.h | 9 ++++ > > 8 files changed, 237 insertions(+), 4 deletions(-) > > create mode 100644 > > Documentation/devicetree/bindings/display/bus-format.txt -- Regards, Laurent Pinchart