From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Subject: Re: [PATCH v2 3/8] drm/bridge: dw-hdmi: allow overriding of phy-type reading Date: Mon, 19 Feb 2018 21:06:40 +0200 Message-ID: <16512170.VqSpgYmDPS@avalon> References: <20180216204158.29839-1-heiko@sntech.de> <3905607.btOD7ERYi5@avalon> <3104429.3dFhngZnWQ@phil> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <3104429.3dFhngZnWQ@phil> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Heiko Stuebner 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 SGkgSGVpa28sCgpPbiBNb25kYXksIDE5IEZlYnJ1YXJ5IDIwMTggMjA6NDY6NDYgRUVUIEhlaWtv IFN0dWVibmVyIHdyb3RlOgo+IEFtIE1vbnRhZywgMTkuIEZlYnJ1YXIgMjAxOCwgMTc6NTk6MDIg Q0VUIHNjaHJpZWIgTGF1cmVudCBQaW5jaGFydDoKPiA+IE9uIEZyaWRheSwgMTYgRmVicnVhcnkg MjAxOCAyMjo0MTo1MyBFRVQgSGVpa28gU3R1ZWJuZXIgd3JvdGU6Cj4gPj4gSW4gc29tZSBJUCBp bXBsZW1lbnRhdGlvbnMgdGhlIHJlYWRpbmcgb2YgdGhlIHBoeS10eXBlIG1heSBiZSBicm9rZW4u Cj4gPj4gT25lIGV4YW1wbGUgYXJlIHRoZSBSb2NrY2hpcCByazMyMjggYW5kIHJrMzMyOCBzb2Nz IHRoYXQgdXNlIGEgc2VwYXJhdGUKPiA+PiBwaHkgZnJvbSBJbm5vc2lsaWNvbiBidXQgc3RpbGwg cmVwb3J0IHRoZSBIRE1JMjBfVFggdHlwZS4KPiA+PiAKPiA+PiBTbyBhbGxvdyB0aGUgZ2x1ZSBk cml2ZXIgdG8gZm9yY2UgYSBzcGVjaWZpYyB0eXBlLCBsaWtlIHRoZSB2ZW5kb3ItcGh5Cj4gPj4g Zm9yIHRoZXNlIGNhc2VzLgo+ID4+IAo+ID4+IFNpZ25lZC1vZmYtYnk6IEhlaWtvIFN0dWVibmVy IDxoZWlrb0BzbnRlY2guZGU+Cj4gPiAKPiA+IFRlc3RlZC1ieTogTGF1cmVudCBQaW5jaGFydCA8 bGF1cmVudC5waW5jaGFydEBpZGVhc29uYm9hcmQuY29tPgo+IAo+IHRoYW5rcyBmb3IgdGVzdGlu ZyA6LSkKPiAKPiA+PiAtLS0KPiA+PiAKPiA+PiAgZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9zeW5v cHN5cy9kdy1oZG1pLmMgfCA0ICsrKy0KPiA+PiAgaW5jbHVkZS9kcm0vYnJpZGdlL2R3X2hkbWku aCAgICAgICAgICAgICAgfCAxICsKPiA+PiAgMiBmaWxlcyBjaGFuZ2VkLCA0IGluc2VydGlvbnMo KyksIDEgZGVsZXRpb24oLSkKPiA+PiAKPiA+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJt L2JyaWRnZS9zeW5vcHN5cy9kdy1oZG1pLmMKPiA+PiBiL2RyaXZlcnMvZ3B1L2RybS9icmlkZ2Uv c3lub3BzeXMvZHctaGRtaS5jIGluZGV4Cj4gPj4gZjk4MDIzOTljYzBkLi41MGQyMzE2MjZjNGQg MTAwNjQ0Cj4gPj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2JyaWRnZS9zeW5vcHN5cy9kdy1oZG1p LmMKPiA+PiArKysgYi9kcml2ZXJzL2dwdS9kcm0vYnJpZGdlL3N5bm9wc3lzL2R3LWhkbWkuYwo+ ID4+IEBAIC0yMjE4LDcgKzIyMTgsOSBAQCBzdGF0aWMgaW50IGR3X2hkbWlfZGV0ZWN0X3BoeShz dHJ1Y3QgZHdfaGRtaQo+ID4+ICpoZG1pKQo+ID4+ICAJdW5zaWduZWQgaW50IGk7Cj4gPj4gIAl1 OCBwaHlfdHlwZTsKPiA+PiAKPiA+PiAtCXBoeV90eXBlID0gaGRtaV9yZWFkYihoZG1pLCBIRE1J X0NPTkZJRzJfSUQpOwo+ID4+ICsJcGh5X3R5cGUgPSAoaGRtaS0+cGxhdF9kYXRhLT5waHlfZm9y Y2VfdHlwZSkgPwo+ID4+ICsJCQkJaGRtaS0+cGxhdF9kYXRhLT5waHlfZm9yY2VfdHlwZSA6Cj4g Pj4gKwkJCQloZG1pX3JlYWRiKGhkbWksIEhETUlfQ09ORklHMl9JRCk7Cj4gPiAKPiA+IE5vIG5l ZWQgZm9yIHBhcmVudGhlc2VzLiBZb3UgY291bGQgZXZlbiB3cml0ZSB0aGlzCj4gPiAKPiA+ICAg ICAgICAgcGh5X3R5cGUgPSBoZG1pLT5wbGF0X2RhdGEtPnBoeV9mb3JjZV90eXBlID8KPiA+ICAg ICAgICAgICAgICAgICAgOiBoZG1pX3JlYWRiKGhkbWksIEhETUlfQ09ORklHMl9JRCk7Cj4gPiAK PiA+IGJ1dCB0aGF0J3MgdXAgdG8geW91Lgo+ID4gCj4gPiBXaGF0IGlmIGEgZHJpdmVyIHdhbnRz IHRvIGZvcmNlIHRoZSBQSFkgdHlwZSB0bwo+ID4gRFdfSERNSV9QSFlfRFdDX0hETUlfVFhfUEhZ ID8gT3IgZG8geW91IGV4cGVjdCBvbmx5IHRoZQo+ID4gRFdfSERNSV9QSFlfVkVORE9SX1BIWSB0 eXBlIHRvIGJlIGZvcmNlZCA/IElmIHNvLCB3ZSBjb3VsZCBhbHNvIHVzZSBhCj4gPiBmb3JjZV92 ZW5kb3JfcGh5IGJvb2xlYW4gZmllbGQgaW5zdGVhZCBvZiBwaHlfZm9yY2VfdHlwZS4KPiAKPiBJ bml0aWFsbHkgSSB0aG91Z2h0IG9mIGltcGxpY2l0bHkgb3ZlcnJpZGluZyB0aGUgcGh5LXR5cGUg d2hlbiB0aGUgZXh0ZXJuYWwKPiBwaHktb3BzIGFyZSBzZXQsIGJ1dCBkZWNpZGVkIGFnYWluc3Qg aXQgYmVjYXVzZSB0aGF0IG1pZ2h0IGJyZWFrCj4gKHRoZW9yZXRpY2FsKSBjYXNlcyB3aGVyZSBw aHktb3BzIG1heSBiZSBhbHdheXMgc2V0IGJ1dCBvbmx5IHVzZWQgd2hlbgo+IHRoZSBpcCByZXR1 cm5zIGEgbWF0Y2hpbmcgcGh5LXR5cGUgYW5kIHRodXMgY2FtZSB0byBqdXN0IGFsbG93IG92ZXJy aWRpbmcKPiB0aGF0IHR5cGUgcmVhZGluZy4KPiAKPiBBcyBmb3IgbGltaXRpbmcgdG8gb25seSBh bGxvdyBmb3JjaW5nIHRoZSB2ZW5kb3IgdHlwZSwgbXkgcGVyc29uYWwgZmVlbGluZwo+IHdvdWxk IGJlIHRvIGFsbG93IGdsdWUgZHJpdmVycyB0byBqdXN0IG92ZXJyaWRlIHRoZSB0eXBlIGFzIG5l ZWRlZAo+IGxpa2UgZG9uZSBpbiB0aGUgcGF0Y2guIEFzIGNhbiBiZSBzZWVuIG9uIHRoZSByazMz MjgsIHRoZSBkdy1oZG1pCj4gcmVwb3J0cyBvbmUgb2YgdGhlIHJlZ3VsYXIgcGh5cyAoZm9yZ290 IHdoaWNoIG9uZSkgYnV0IGluc3RlYWQgaGFzIGEKPiBjb21wbGV0ZWx5IHNlcGFyYXRlIGlwIGZv ciBpdCwgc28gSSdkIGd1ZXNzIHdlIG1heSB2ZXJ5IHdlbGwgc2VlCj4gaW1wbGVtZW50YXRpb25z IHRoYXQganVzdCByZXBvcnQgYSB3cm9uZyB0eXBlIChubyB2ZW5kb3ItdHlwZSkuCj4gCj4gU28g SSBkb24ndCBzZWUgYW4gaXNzdWUgd2l0aCBkcml2ZXJzIGJlaW5nIGFsbG93ZWQgdG8gc2V0IHRo ZSB0eXBlIHRvCj4gRFdfSERNSV9QSFlfRFdDX0hETUlfVFhfUEhZLCBiZWNhdXNlIHN1Y2ggY2Fz ZXMgbWF5IGV4aXN0IGluIHRoZQo+IGZ1dHVyZSBhbmQgSSdkIGV4cGVjdCBkcml2ZXIgYXV0aG9y cyB0byBzb21ld2hhdCBrbm93IHdoYXQgdGhleSdyZSBkb2luZy4KCk15IHBvaW50IHdhcyB0aGF0 IERXX0hETUlfUEhZX0RXQ19IRE1JX1RYX1BIWSA9PSAwLCBzbyB0cnlpbmcgdG8gZm9yY2UgCkRX X0hETUlfUEhZX0RXQ19IRE1JX1RYX1BIWSB0aHJvdWdoIHBoeV9mb3JjZV90eXBlIHdvbid0IHdv cmsuCgo+IEJ1dCBJJ20gbm90IHRpZWQgdG8gdGhhdCwgc28gd2lsbCBnbyB3aXRoIHRoZSBtYWpv cml0eSBvcGluaW9uIDotRCAuCgotLSAKUmVnYXJkcywKCkxhdXJlbnQgUGluY2hhcnQKCl9fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZlbCBtYWls aW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZy ZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: laurent.pinchart@ideasonboard.com (Laurent Pinchart) Date: Mon, 19 Feb 2018 21:06:40 +0200 Subject: [PATCH v2 3/8] drm/bridge: dw-hdmi: allow overriding of phy-type reading In-Reply-To: <3104429.3dFhngZnWQ@phil> References: <20180216204158.29839-1-heiko@sntech.de> <3905607.btOD7ERYi5@avalon> <3104429.3dFhngZnWQ@phil> Message-ID: <16512170.VqSpgYmDPS@avalon> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Heiko, On Monday, 19 February 2018 20:46:46 EET Heiko Stuebner wrote: > 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. My point was that DW_HDMI_PHY_DWC_HDMI_TX_PHY == 0, so trying to force DW_HDMI_PHY_DWC_HDMI_TX_PHY through phy_force_type won't work. > But I'm not tied to that, so will go with the majority opinion :-D . -- Regards, Laurent Pinchart