From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Stuebner Subject: Re: [PATCH v2 3/8] drm/bridge: dw-hdmi: allow overriding of phy-type reading Date: Mon, 19 Feb 2018 19:46:46 +0100 Message-ID: <3104429.3dFhngZnWQ@phil> References: <20180216204158.29839-1-heiko@sntech.de> <20180216204158.29839-4-heiko@sntech.de> <3905607.btOD7ERYi5@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <3905607.btOD7ERYi5@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, Jose.Abreu@synopsys.com, algea.cao@rock-chips.com, devicetree@vger.kernel.org, airlied@linux.ie, dri-devel@lists.freedesktop.org, kishon@ti.com, robh+dt@kernel.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org, zhengyang@rock-chips.com List-Id: linux-rockchip.vger.kernel.org SGkgTGF1cmVudCwKCkFtIE1vbnRhZywgMTkuIEZlYnJ1YXIgMjAxOCwgMTc6NTk6MDIgQ0VUIHNj aHJpZWIgTGF1cmVudCBQaW5jaGFydDoKPiBPbiBGcmlkYXksIDE2IEZlYnJ1YXJ5IDIwMTggMjI6 NDE6NTMgRUVUIEhlaWtvIFN0dWVibmVyIHdyb3RlOgo+ID4gSW4gc29tZSBJUCBpbXBsZW1lbnRh dGlvbnMgdGhlIHJlYWRpbmcgb2YgdGhlIHBoeS10eXBlIG1heSBiZSBicm9rZW4uCj4gPiBPbmUg ZXhhbXBsZSBhcmUgdGhlIFJvY2tjaGlwIHJrMzIyOCBhbmQgcmszMzI4IHNvY3MgdGhhdCB1c2Ug YSBzZXBhcmF0ZQo+ID4gcGh5IGZyb20gSW5ub3NpbGljb24gYnV0IHN0aWxsIHJlcG9ydCB0aGUg SERNSTIwX1RYIHR5cGUuCj4gPiAKPiA+IFNvIGFsbG93IHRoZSBnbHVlIGRyaXZlciB0byBmb3Jj ZSBhIHNwZWNpZmljIHR5cGUsIGxpa2UgdGhlIHZlbmRvci1waHkKPiA+IGZvciB0aGVzZSBjYXNl cy4KPiA+IAo+ID4gU2lnbmVkLW9mZi1ieTogSGVpa28gU3R1ZWJuZXIgPGhlaWtvQHNudGVjaC5k ZT4KPiAKPiBUZXN0ZWQtYnk6IExhdXJlbnQgUGluY2hhcnQgPGxhdXJlbnQucGluY2hhcnRAaWRl YXNvbmJvYXJkLmNvbT4KCnRoYW5rcyBmb3IgdGVzdGluZyA6LSkKCj4gPiAtLS0KPiA+ICBkcml2 ZXJzL2dwdS9kcm0vYnJpZGdlL3N5bm9wc3lzL2R3LWhkbWkuYyB8IDQgKysrLQo+ID4gIGluY2x1 ZGUvZHJtL2JyaWRnZS9kd19oZG1pLmggICAgICAgICAgICAgIHwgMSArCj4gPiAgMiBmaWxlcyBj aGFuZ2VkLCA0IGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkKPiA+IAo+ID4gZGlmZiAtLWdp dCBhL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2Uvc3lub3BzeXMvZHctaGRtaS5jCj4gPiBiL2RyaXZl cnMvZ3B1L2RybS9icmlkZ2Uvc3lub3BzeXMvZHctaGRtaS5jIGluZGV4Cj4gPiBmOTgwMjM5OWNj MGQuLjUwZDIzMTYyNmM0ZCAxMDA2NDQKPiA+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2Uv c3lub3BzeXMvZHctaGRtaS5jCj4gPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL3N5bm9w c3lzL2R3LWhkbWkuYwo+ID4gQEAgLTIyMTgsNyArMjIxOCw5IEBAIHN0YXRpYyBpbnQgZHdfaGRt aV9kZXRlY3RfcGh5KHN0cnVjdCBkd19oZG1pICpoZG1pKQo+ID4gIAl1bnNpZ25lZCBpbnQgaTsK PiA+ICAJdTggcGh5X3R5cGU7Cj4gPiAKPiA+IC0JcGh5X3R5cGUgPSBoZG1pX3JlYWRiKGhkbWks IEhETUlfQ09ORklHMl9JRCk7Cj4gPiArCXBoeV90eXBlID0gKGhkbWktPnBsYXRfZGF0YS0+cGh5 X2ZvcmNlX3R5cGUpID8KPiA+ICsJCQkJaGRtaS0+cGxhdF9kYXRhLT5waHlfZm9yY2VfdHlwZSA6 Cj4gPiArCQkJCWhkbWlfcmVhZGIoaGRtaSwgSERNSV9DT05GSUcyX0lEKTsKPiAKPiBObyBuZWVk IGZvciBwYXJlbnRoZXNlcy4gWW91IGNvdWxkIGV2ZW4gd3JpdGUgdGhpcwo+IAo+ICAgICAgICAg cGh5X3R5cGUgPSBoZG1pLT5wbGF0X2RhdGEtPnBoeV9mb3JjZV90eXBlID8gCj4gICAgICAgICAg ICAgICAgICA6IGhkbWlfcmVhZGIoaGRtaSwgSERNSV9DT05GSUcyX0lEKTsKPiAKPiBidXQgdGhh dCdzIHVwIHRvIHlvdS4KPiAKPiBXaGF0IGlmIGEgZHJpdmVyIHdhbnRzIHRvIGZvcmNlIHRoZSBQ SFkgdHlwZSB0byBEV19IRE1JX1BIWV9EV0NfSERNSV9UWF9QSFkgPyAKPiBPciBkbyB5b3UgZXhw ZWN0IG9ubHkgdGhlIERXX0hETUlfUEhZX1ZFTkRPUl9QSFkgdHlwZSB0byBiZSBmb3JjZWQgPyBJ ZiBzbywgd2UgCj4gY291bGQgYWxzbyB1c2UgYSBmb3JjZV92ZW5kb3JfcGh5IGJvb2xlYW4gZmll bGQgaW5zdGVhZCBvZiBwaHlfZm9yY2VfdHlwZS4KCkluaXRpYWxseSBJIHRob3VnaHQgb2YgaW1w bGljaXRseSBvdmVycmlkaW5nIHRoZSBwaHktdHlwZSB3aGVuIHRoZSBleHRlcm5hbApwaHktb3Bz IGFyZSBzZXQsIGJ1dCBkZWNpZGVkIGFnYWluc3QgaXQgYmVjYXVzZSB0aGF0IG1pZ2h0IGJyZWFr Cih0aGVvcmV0aWNhbCkgY2FzZXMgd2hlcmUgcGh5LW9wcyBtYXkgYmUgYWx3YXlzIHNldCBidXQg b25seSB1c2VkIHdoZW4KdGhlIGlwIHJldHVybnMgYSBtYXRjaGluZyBwaHktdHlwZSBhbmQgdGh1 cyBjYW1lIHRvIGp1c3QgYWxsb3cgb3ZlcnJpZGluZwp0aGF0IHR5cGUgcmVhZGluZy4KCkFzIGZv ciBsaW1pdGluZyB0byBvbmx5IGFsbG93IGZvcmNpbmcgdGhlIHZlbmRvciB0eXBlLCBteSBwZXJz b25hbCBmZWVsaW5nCndvdWxkIGJlIHRvIGFsbG93IGdsdWUgZHJpdmVycyB0byBqdXN0IG92ZXJy aWRlIHRoZSB0eXBlIGFzIG5lZWRlZApsaWtlIGRvbmUgaW4gdGhlIHBhdGNoLiBBcyBjYW4gYmUg c2VlbiBvbiB0aGUgcmszMzI4LCB0aGUgZHctaGRtaQpyZXBvcnRzIG9uZSBvZiB0aGUgcmVndWxh ciBwaHlzIChmb3Jnb3Qgd2hpY2ggb25lKSBidXQgaW5zdGVhZCBoYXMgYQpjb21wbGV0ZWx5IHNl cGFyYXRlIGlwIGZvciBpdCwgc28gSSdkIGd1ZXNzIHdlIG1heSB2ZXJ5IHdlbGwgc2VlCmltcGxl bWVudGF0aW9ucyB0aGF0IGp1c3QgcmVwb3J0IGEgd3JvbmcgdHlwZSAobm8gdmVuZG9yLXR5cGUp LgoKU28gSSBkb24ndCBzZWUgYW4gaXNzdWUgd2l0aCBkcml2ZXJzIGJlaW5nIGFsbG93ZWQgdG8g c2V0IHRoZSB0eXBlIHRvCkRXX0hETUlfUEhZX0RXQ19IRE1JX1RYX1BIWSwgYmVjYXVzZSBzdWNo IGNhc2VzIG1heSBleGlzdCBpbiB0aGUKZnV0dXJlIGFuZCBJJ2QgZXhwZWN0IGRyaXZlciBhdXRo b3JzIHRvIHNvbWV3aGF0IGtub3cgd2hhdCB0aGV5J3JlIGRvaW5nLgoKQnV0IEknbSBub3QgdGll ZCB0byB0aGF0LCBzbyB3aWxsIGdvIHdpdGggdGhlIG1ham9yaXR5IG9waW5pb24gOi1EIC4KCgpI ZWlrbwoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJp LWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBz Oi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: heiko@sntech.de (Heiko Stuebner) Date: Mon, 19 Feb 2018 19:46:46 +0100 Subject: [PATCH v2 3/8] drm/bridge: dw-hdmi: allow overriding of phy-type reading In-Reply-To: <3905607.btOD7ERYi5@avalon> References: <20180216204158.29839-1-heiko@sntech.de> <20180216204158.29839-4-heiko@sntech.de> <3905607.btOD7ERYi5@avalon> Message-ID: <3104429.3dFhngZnWQ@phil> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Laurent, Am Montag, 19. Februar 2018, 17:59:02 CET schrieb Laurent Pinchart: > On Friday, 16 February 2018 22:41:53 EET Heiko Stuebner wrote: > > In some IP implementations the reading of the phy-type may be broken. > > One example are the Rockchip rk3228 and rk3328 socs that use a separate > > phy from Innosilicon but still report the HDMI20_TX type. > > > > So allow the glue driver to force a specific type, like the vendor-phy > > for these cases. > > > > Signed-off-by: Heiko Stuebner > > Tested-by: Laurent Pinchart thanks for testing :-) > > --- > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 +++- > > include/drm/bridge/dw_hdmi.h | 1 + > > 2 files changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index > > f9802399cc0d..50d231626c4d 100644 > > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > > @@ -2218,7 +2218,9 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi) > > unsigned int i; > > u8 phy_type; > > > > - phy_type = hdmi_readb(hdmi, HDMI_CONFIG2_ID); > > + phy_type = (hdmi->plat_data->phy_force_type) ? > > + hdmi->plat_data->phy_force_type : > > + hdmi_readb(hdmi, HDMI_CONFIG2_ID); > > No need for parentheses. You could even write this > > phy_type = hdmi->plat_data->phy_force_type ? > : hdmi_readb(hdmi, HDMI_CONFIG2_ID); > > but that's up to you. > > What if a driver wants to force the PHY type to DW_HDMI_PHY_DWC_HDMI_TX_PHY ? > Or do you expect only the DW_HDMI_PHY_VENDOR_PHY type to be forced ? If so, we > could also use a force_vendor_phy boolean field instead of phy_force_type. Initially I thought of implicitly overriding the phy-type when the external phy-ops are set, but decided against it because that might break (theoretical) cases where phy-ops may be always set but only used when the ip returns a matching phy-type and thus came to just allow overriding that type reading. As for limiting to only allow forcing the vendor type, my personal feeling would be to allow glue drivers to just override the type as needed like done in the patch. As can be seen on the rk3328, the dw-hdmi reports one of the regular phys (forgot which one) but instead has a completely separate ip for it, so I'd guess we may very well see implementations that just report a wrong type (no vendor-type). So I don't see an issue with drivers being allowed to set the type to DW_HDMI_PHY_DWC_HDMI_TX_PHY, because such cases may exist in the future and I'd expect driver authors to somewhat know what they're doing. But I'm not tied to that, so will go with the majority opinion :-D . Heiko