From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yakir Yang Subject: Re: [PATCH v1 5/6] drm/rockchip: dw_hdmi: introduce the VPLL clock setting Date: Wed, 13 Jul 2016 15:45:06 +0800 Message-ID: <5785F182.5070901@rock-chips.com> References: <1468235079-29152-1-git-send-email-ykk@rock-chips.com> <1468235149-29625-1-git-send-email-ykk@rock-chips.com> <3636683.EuFECY2EQB@diego> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <3636683.EuFECY2EQB@diego> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: =?UTF-8?Q?Heiko_St=c3=bcbner?= Cc: devicetree@vger.kernel.org, xhc@rock-chips.com, Daniel Vetter , Kumar Gala , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-rockchip@lists.infradead.org, Rob Herring , Andy Yan , Russell King , Zheng Yang , linux-arm-kernel@lists.infradead.org List-Id: linux-rockchip.vger.kernel.org SGVpa28sCgpPbiAwNy8xMi8yMDE2IDEwOjI3IFBNLCBIZWlrbyBTdMO8Ym5lciB3cm90ZToKPiBI aSBZYWtpciwKPgo+IEFtIE1vbnRhZywgMTEuIEp1bGkgMjAxNiwgMTk6MDU6NDkgc2NocmllYiBZ YWtpciBZYW5nOgo+PiBGb3IgUkszMzk5IEhETUksIHRoZXJlIGlzIGFuIGV4dGVybmFsIGNsb2Nr IG5lZWQgZm9yIEhETUkgUEhZLAo+PiBhbmQgaXQgc2hvdWxkIGtlZXAgdGhlIHNhbWUgY2xvY2sg cmF0ZSB3aXRoIFZPUCBEQ0xLLgo+Pgo+PiBWUExMIGhhdmUgc3VwcG9ydGVkIHRoZSBjbG9jayBm b3IgSERNSSBQSFksIGJ1dCB0aGVyZSBpcyBubwo+PiBjbG9jayBkaXZpZGVyIGJld3RlZW4gVlBM TCBhbmQgSERNSSBQSFkuIFNvIHdlIG5lZWQgdG8gc2V0IHRoZQo+PiBWUExMIHJhdGUgbWFudWFs bHkgaW4gSERNSSBkcml2ZXIuCj4gSSBkb24ndCB0aGluayByZXNlcnZpbmcgdGhlIHZwbGwgZm9y IHRoZSBoZG1pIGF0IGFsbCB0aW1lcyBpcyB0aGUgcmlnaHQgd2F5IHRvCj4gZ28uCj4KPiBXaGls ZSBJIGRvIGFncmVlIHRoYXQgcmVzZXJ2aW5nIHRoZSBWUExMIChhbmQgTlBMTCBvbiB0aGUgcmsz Mjg4KSBmb3IgZ3JhcGhpY3MKPiB1c2UgbG9va3MgbGlrZSB0aGUgcmlnaHQgd2F5LCBJIHRoaW5r IHRoZSBjb3JlIFJvY2tjaGlwIGRybSBkcml2ZXIgc2hvdWxkIGJlCj4gcmVzcG9uc2libGUgZm9y IG1hbmFnaW5nIGl0IGFuZCBhbHNvIGZvciBkZWNpZGluZyB3aGljaCBvdXRwdXQgZW5jb2RlciBn ZXRzIHRvCj4gdXNlIGl0Lgo+IFdoaWxlIHRydWUgdGhhdCBvbiB0aGUgQ2hyb21lYm9vayBkZXZp Y2UtdHlwZXMgdGhlIGVkcCBpcyBzdGF0aWMgYW5kIGhkbWkKPiBuZWVkcyB0aGUgYnJvYWQgcmFu Z2Ugb2YgZHluYW1pYyBmcmVxdWVuY2llcywgdGhpcyBpcyBub3QgbmVjZXNzYXJpbHkgdGhlIGNh c2UKPiBmb3IgYWxsIGZ1dHVyZSBkZXZpY2UgdHlwZXMgYW5kL29yIHNvY3MuCj4KPiBJbiB0aGUg b3RoZXIgdGhyZWFkIHdlIGRpc2N1c3NlZCBhZGRpbmcgdGhhdCBhcyByb2NrY2hpcCxkY2xrLXBs bCA9IDwmLi4uPjsgdG8KPiB0aGUgYmFzZSBkaXNwbGF5LXN1YnN5c3RlbSBub2RlLCBEb3VnIGRp ZG4ndCBtYW5hZ2UgdG8gZmluZCB0aW1lIHRvIHJlc3BvbmQgeWV0Cj4gdGhvdWdoIC0gYW5kIGlz IG9uIHZhY2F0aW9uIHJpZ2h0IG5vdy4KCkdyZWF0IGlkZWEuIExldCBhIHNlcGFyYXRlIGRybS1w bGwgZHJpdmVyIHRvIG1haW50YWluIGFsbCBjb25uZWN0b3IncyAKY2xvY2sgcmVxdWlyZW1lbnQu Cgo+IEkgc3RpbGwgYmVsaWV2ZSB0aGF0IHdvdWxkIGJlIHRoZSBiZXN0IHNvbHV0aW9uIDotKSAu Cj4KPgo+IFRoYXQgcHJvcGVydHkgY291bGQgbGlzdCAxIG9yIGV2ZW4gMiBwbGxzLCBkZXBlbmRp bmcgb24gdGhlIHNvYyBvciBib2FyZAo+IGxheW91dCAtIG1heWJlIHNvbWVvbmUgZnJlZXMgdXAg dGhlIGNwbGwgaW4gc29tZSBzcGVjaWFsIGxheW91dCBvciBzb21ldGhpbmcuCj4gSWYgbm9uZSBh cmUgbGlzdGVkLCB0aGVuIHRoZSBkcm0gZHJpdmVyIHdvdWxkIG5lZWQgdG8gY29wZSB3aXRoIGl0 cyBhdmFpbGFibGUKPiBjbG9ja3MuCj4KPiBJbXBsZW1lbnRhdGlvbi13aXNlIHlvdSBjb3VsZCBl dmVuIHN0aWxsIGtlZXAgeW91ciBzaG9ydGN1dCB1bnRpbCB3ZSBlbmNvdW50ZXIKPiBhIGRldmlj ZSB3aXRoIGRpZmZlcmVudCAsIGxldCB0aGUgZHJtIHVzZSB0aGUgcGxsIGZvciBoZG1pIG9ubHkg dW50aWwgc29tZW9uZQo+IGNvbWVzIHVwIHdpdGggYSBiZXR0ZXIgY29uY2VwdCwgYnV0IHN0aWxs IHRoZSBiaW5kaW5nIHNob3VsZCBiZSBjb3JyZWN0IGFuZAo+IHZlcnNhdGlsZSBmcm9tIHRoZSBz dGFydC4KClNvbWVvbmUgc2hvdWxkIHN0YXJ0IHRvIHByZXBhcmUgdGhlIGRybSBjb3JlIHBsbCBk cml2ZXIsIGl0J3MgZ3JlYXQgaWRlYS4KCkJSLAotIFlha2lyCgo+Cj4gSGVpa28KPgo+PiAtLS0K Pj4gICAuLi4vYmluZGluZ3MvZGlzcGxheS9yb2NrY2hpcC9kd19oZG1pLXJvY2tjaGlwLnR4dCB8 ICAzICsrLQo+PiAgIGRyaXZlcnMvZ3B1L2RybS9yb2NrY2hpcC9kd19oZG1pLXJvY2tjaGlwLmMg ICAgICAgIHwgMjUKPj4gKysrKysrKysrKysrKysrKysrKysrLSAyIGZpbGVzIGNoYW5nZWQsIDI2 IGluc2VydGlvbnMoKyksIDIgZGVsZXRpb25zKC0pCj4+Cj4+IGRpZmYgLS1naXQKPj4gYS9Eb2N1 bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvZGlzcGxheS9yb2NrY2hpcC9kd19oZG1pLXJv Y2tjaGlwLnR4dAo+PiBiL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9kaXNwbGF5 L3JvY2tjaGlwL2R3X2hkbWktcm9ja2NoaXAudHh0Cj4+IGluZGV4IDRlNTczZDIuLjRlMjNjYTQg MTAwNjQ0Cj4+IC0tLQo+PiBhL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy9kaXNw bGF5L3JvY2tjaGlwL2R3X2hkbWktcm9ja2NoaXAudHh0Cj4+ICsrKwo+PiBiL0RvY3VtZW50YXRp b24vZGV2aWNldHJlZS9iaW5kaW5ncy9kaXNwbGF5L3JvY2tjaGlwL2R3X2hkbWktcm9ja2NoaXAu dHh0Cj4+IEBAIC0xNyw3ICsxNyw4IEBAIFJlcXVpcmVkIHByb3BlcnRpZXM6Cj4+Cj4+ICAgT3B0 aW9uYWwgcHJvcGVydGllcwo+PiAgIC0gZGRjLWkyYy1idXM6IHBoYW5kbGUgb2YgYW4gSTJDIGNv bnRyb2xsZXIgdXNlZCBmb3IgRERDIEVESUQgcHJvYmluZwo+PiAtLSBjbG9ja3MsIGNsb2NrLW5h bWVzOiBwaGFuZGxlIHRvIHRoZSBIRE1JIENFQyBjbG9jaywgbmFtZSBzaG91bGQgYmUgImNlYyIK Pj4gKy0gY2xvY2tzLCBjbG9jay1uYW1lczogcGhhbmRsZSB0byB0aGUgSERNSSBDRUMgY2xvY2ss IG5hbWUgc2hvdWxkIGJlICJjZWMiLAo+PiArCQkgICAgICAgcGhhbmRsZSB0byB0aGUgVlBMTCBj bG9jaywgbmFtZSBzaG91bGQgYmUgInZwbGwiLgo+Pgo+PiAgIEV4YW1wbGU6Cj4+ICAgaGRtaTog aGRtaUBmZjk4MDAwMCB7Cj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vcm9ja2NoaXAv ZHdfaGRtaS1yb2NrY2hpcC5jCj4+IGIvZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlwL2R3X2hkbWkt cm9ja2NoaXAuYyBpbmRleCAzMjkwOTliLi43MDFiYjczIDEwMDY0NAo+PiAtLS0gYS9kcml2ZXJz L2dwdS9kcm0vcm9ja2NoaXAvZHdfaGRtaS1yb2NrY2hpcC5jCj4+ICsrKyBiL2RyaXZlcnMvZ3B1 L2RybS9yb2NrY2hpcC9kd19oZG1pLXJvY2tjaGlwLmMKPj4gQEAgLTcsMTAgKzcsMTIgQEAKPj4g ICAgKiAoYXQgeW91ciBvcHRpb24pIGFueSBsYXRlciB2ZXJzaW9uLgo+PiAgICAqLwo+Pgo+PiAr I2luY2x1ZGUgPGxpbnV4L2Nsay5oPgo+PiArI2luY2x1ZGUgPGxpbnV4L21mZC9zeXNjb24uaD4K Pj4gICAjaW5jbHVkZSA8bGludXgvbW9kdWxlLmg+Cj4+ICAgI2luY2x1ZGUgPGxpbnV4L3BsYXRm b3JtX2RldmljZS5oPgo+PiAtI2luY2x1ZGUgPGxpbnV4L21mZC9zeXNjb24uaD4KPj4gICAjaW5j bHVkZSA8bGludXgvcmVnbWFwLmg+Cj4+ICsKPj4gICAjaW5jbHVkZSA8ZHJtL2RybV9vZi5oPgo+ PiAgICNpbmNsdWRlIDxkcm0vZHJtUC5oPgo+PiAgICNpbmNsdWRlIDxkcm0vZHJtX2NydGNfaGVs cGVyLmg+Cj4+IEBAIC0zMyw2ICszNSw3IEBAIHN0cnVjdCByb2NrY2hpcF9oZG1pIHsKPj4gICAJ c3RydWN0IHJlZ21hcCAqcmVnbWFwOwo+PiAgIAlzdHJ1Y3QgZHJtX2VuY29kZXIgZW5jb2RlcjsK Pj4gICAJZW51bSBkd19oZG1pX2RldnR5cGUgZGV2X3R5cGU7Cj4+ICsJc3RydWN0IGNsayAqdnBs bF9jbGs7Cj4+ICAgfTsKPj4KPj4gICAjZGVmaW5lIHRvX3JvY2tjaGlwX2hkbWkoeCkJY29udGFp bmVyX29mKHgsIHN0cnVjdCByb2NrY2hpcF9oZG1pLCB4KQo+PiBAQCAtMTQ1LDYgKzE0OCw3IEBA IHN0YXRpYyBjb25zdCBzdHJ1Y3QgZHdfaGRtaV9waHlfY29uZmlnCj4+IHJvY2tjaGlwX3BoeV9j b25maWdbXSA9IHsgc3RhdGljIGludCByb2NrY2hpcF9oZG1pX3BhcnNlX2R0KHN0cnVjdAo+PiBy b2NrY2hpcF9oZG1pICpoZG1pKQo+PiAgIHsKPj4gICAJc3RydWN0IGRldmljZV9ub2RlICpucCA9 IGhkbWktPmRldi0+b2Zfbm9kZTsKPj4gKwlpbnQgcmV0Owo+Pgo+PiAgIAloZG1pLT5yZWdtYXAg PSBzeXNjb25fcmVnbWFwX2xvb2t1cF9ieV9waGFuZGxlKG5wLCAicm9ja2NoaXAsZ3JmIik7Cj4+ ICAgCWlmIChJU19FUlIoaGRtaS0+cmVnbWFwKSkgewo+PiBAQCAtMTUyLDYgKzE1NiwyMiBAQCBz dGF0aWMgaW50IHJvY2tjaGlwX2hkbWlfcGFyc2VfZHQoc3RydWN0IHJvY2tjaGlwX2hkbWkKPj4g KmhkbWkpIHJldHVybiBQVFJfRVJSKGhkbWktPnJlZ21hcCk7Cj4+ICAgCX0KPj4KPj4gKwloZG1p LT52cGxsX2NsayA9IGRldm1fY2xrX2dldChoZG1pLT5kZXYsICJ2cGxsIik7Cj4+ICsJaWYgKFBU Ul9FUlIoaGRtaS0+dnBsbF9jbGspID09IC1FTk9FTlQpIHsKPj4gKwkJaGRtaS0+dnBsbF9jbGsg PSBOVUxMOwo+PiArCX0gZWxzZSBpZiAoUFRSX0VSUihoZG1pLT52cGxsX2NsaykgPT0gLUVQUk9C RV9ERUZFUikgewo+PiArCQlyZXR1cm4gLUVQUk9CRV9ERUZFUjsKPj4gKwl9IGVsc2UgaWYgKElT X0VSUihoZG1pLT52cGxsX2NsaykpIHsKPj4gKwkJZGV2X2VycihoZG1pLT5kZXYsICJmYWlsZWQg dG8gZ2V0IGdyZiBjbG9ja1xuIik7Cj4+ICsJCXJldHVybiBQVFJfRVJSKGhkbWktPnZwbGxfY2xr KTsKPj4gKwl9Cj4+ICsKPj4gKwlyZXQgPSBjbGtfcHJlcGFyZV9lbmFibGUoaGRtaS0+dnBsbF9j bGspOwo+PiArCWlmIChyZXQpIHsKPj4gKwkJZGV2X2VycihoZG1pLT5kZXYsICJGYWlsZWQgdG8g ZW5hYmxlIEhETUkgdnBsbDogJWRcbiIsIHJldCk7Cj4+ICsJCXJldHVybiByZXQ7Cj4+ICsJfQo+ PiArCj4+ICAgCXJldHVybiAwOwo+PiAgIH0KPj4KPj4gQEAgLTE5NCw2ICsyMTQsOSBAQCBzdGF0 aWMgdm9pZCBkd19oZG1pX3JvY2tjaGlwX2VuY29kZXJfbW9kZV9zZXQoc3RydWN0Cj4+IGRybV9l bmNvZGVyICplbmNvZGVyLCBzdHJ1Y3QgZHJtX2Rpc3BsYXlfbW9kZSAqbW9kZSwKPj4gICAJCQkJ CSAgICAgIHN0cnVjdCBkcm1fZGlzcGxheV9tb2RlICphZGpfbW9kZSkKPj4gICB7Cj4+ICsJc3Ry dWN0IHJvY2tjaGlwX2hkbWkgKmhkbWkgPSB0b19yb2NrY2hpcF9oZG1pKGVuY29kZXIpOwo+PiAr Cj4+ICsJY2xrX3NldF9yYXRlKGhkbWktPnZwbGxfY2xrLCBhZGpfbW9kZS0+Y2xvY2sgKiAxMDAw KTsKPj4gICB9Cj4+Cj4+ICAgc3RhdGljIHZvaWQgZHdfaGRtaV9yb2NrY2hpcF9lbmNvZGVyX2Vu YWJsZShzdHJ1Y3QgZHJtX2VuY29kZXIgKmVuY29kZXIpCj4KPgo+CgoKX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApk cmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Au b3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: ykk@rock-chips.com (Yakir Yang) Date: Wed, 13 Jul 2016 15:45:06 +0800 Subject: [PATCH v1 5/6] drm/rockchip: dw_hdmi: introduce the VPLL clock setting In-Reply-To: <3636683.EuFECY2EQB@diego> References: <1468235079-29152-1-git-send-email-ykk@rock-chips.com> <1468235149-29625-1-git-send-email-ykk@rock-chips.com> <3636683.EuFECY2EQB@diego> Message-ID: <5785F182.5070901@rock-chips.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Heiko, On 07/12/2016 10:27 PM, Heiko St?bner wrote: > Hi Yakir, > > Am Montag, 11. Juli 2016, 19:05:49 schrieb Yakir Yang: >> For RK3399 HDMI, there is an external clock need for HDMI PHY, >> and it should keep the same clock rate with VOP DCLK. >> >> VPLL have supported the clock for HDMI PHY, but there is no >> clock divider bewteen VPLL and HDMI PHY. So we need to set the >> VPLL rate manually in HDMI driver. > I don't think reserving the vpll for the hdmi at all times is the right way to > go. > > While I do agree that reserving the VPLL (and NPLL on the rk3288) for graphics > use looks like the right way, I think the core Rockchip drm driver should be > responsible for managing it and also for deciding which output encoder gets to > use it. > While true that on the Chromebook device-types the edp is static and hdmi > needs the broad range of dynamic frequencies, this is not necessarily the case > for all future device types and/or socs. > > In the other thread we discussed adding that as rockchip,dclk-pll = <&...>; to > the base display-subsystem node, Doug didn't manage to find time to respond yet > though - and is on vacation right now. Great idea. Let a separate drm-pll driver to maintain all connector's clock requirement. > I still believe that would be the best solution :-) . > > > That property could list 1 or even 2 plls, depending on the soc or board > layout - maybe someone frees up the cpll in some special layout or something. > If none are listed, then the drm driver would need to cope with its available > clocks. > > Implementation-wise you could even still keep your shortcut until we encounter > a device with different , let the drm use the pll for hdmi only until someone > comes up with a better concept, but still the binding should be correct and > versatile from the start. Someone should start to prepare the drm core pll driver, it's great idea. BR, - Yakir > > Heiko > >> --- >> .../bindings/display/rockchip/dw_hdmi-rockchip.txt | 3 ++- >> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 25 >> +++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) >> >> diff --git >> a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt >> b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt >> index 4e573d2..4e23ca4 100644 >> --- >> a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt >> +++ >> b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt >> @@ -17,7 +17,8 @@ Required properties: >> >> Optional properties >> - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing >> -- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec" >> +- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec", >> + phandle to the VPLL clock, name should be "vpll". >> >> Example: >> hdmi: hdmi at ff980000 { >> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 329099b..701bb73 100644 >> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >> @@ -7,10 +7,12 @@ >> * (at your option) any later version. >> */ >> >> +#include >> +#include >> #include >> #include >> -#include >> #include >> + >> #include >> #include >> #include >> @@ -33,6 +35,7 @@ struct rockchip_hdmi { >> struct regmap *regmap; >> struct drm_encoder encoder; >> enum dw_hdmi_devtype dev_type; >> + struct clk *vpll_clk; >> }; >> >> #define to_rockchip_hdmi(x) container_of(x, struct rockchip_hdmi, x) >> @@ -145,6 +148,7 @@ static const struct dw_hdmi_phy_config >> rockchip_phy_config[] = { static int rockchip_hdmi_parse_dt(struct >> rockchip_hdmi *hdmi) >> { >> struct device_node *np = hdmi->dev->of_node; >> + int ret; >> >> hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf"); >> if (IS_ERR(hdmi->regmap)) { >> @@ -152,6 +156,22 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi >> *hdmi) return PTR_ERR(hdmi->regmap); >> } >> >> + hdmi->vpll_clk = devm_clk_get(hdmi->dev, "vpll"); >> + if (PTR_ERR(hdmi->vpll_clk) == -ENOENT) { >> + hdmi->vpll_clk = NULL; >> + } else if (PTR_ERR(hdmi->vpll_clk) == -EPROBE_DEFER) { >> + return -EPROBE_DEFER; >> + } else if (IS_ERR(hdmi->vpll_clk)) { >> + dev_err(hdmi->dev, "failed to get grf clock\n"); >> + return PTR_ERR(hdmi->vpll_clk); >> + } >> + >> + ret = clk_prepare_enable(hdmi->vpll_clk); >> + if (ret) { >> + dev_err(hdmi->dev, "Failed to enable HDMI vpll: %d\n", ret); >> + return ret; >> + } >> + >> return 0; >> } >> >> @@ -194,6 +214,9 @@ static void dw_hdmi_rockchip_encoder_mode_set(struct >> drm_encoder *encoder, struct drm_display_mode *mode, >> struct drm_display_mode *adj_mode) >> { >> + struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder); >> + >> + clk_set_rate(hdmi->vpll_clk, adj_mode->clock * 1000); >> } >> >> static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder) > > > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752105AbcGMHqM (ORCPT ); Wed, 13 Jul 2016 03:46:12 -0400 Received: from lucky1.263xmail.com ([211.157.147.133]:60176 "EHLO lucky1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750854AbcGMHqC (ORCPT ); Wed, 13 Jul 2016 03:46:02 -0400 X-263anti-spam: KSV:0; X-MAIL-GRAY: 1 X-MAIL-DELIVERY: 0 X-KSVirus-check: 0 X-ABS-CHECKED: 4 X-ADDR-CHECKED: 0 X-RL-SENDER: ykk@rock-chips.com X-FST-TO: linux-arm-kernel@lists.infradead.org X-SENDER-IP: 103.29.142.67 X-LOGIN-NAME: ykk@rock-chips.com X-UNIQUE-TAG: <489f55a49e3b2db573aa8208296f2139> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Subject: Re: [PATCH v1 5/6] drm/rockchip: dw_hdmi: introduce the VPLL clock setting To: =?UTF-8?Q?Heiko_St=c3=bcbner?= References: <1468235079-29152-1-git-send-email-ykk@rock-chips.com> <1468235149-29625-1-git-send-email-ykk@rock-chips.com> <3636683.EuFECY2EQB@diego> Cc: Mark Yao , Rob Herring , Russell King , Philipp Zabel , Andy Yan , David Airlie , Daniel Vetter , Kumar Gala , Zheng Yang , xhc@rock-chips.com, dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-rockchip@lists.infradead.org, linux-arm-kernel@lists.infradead.org From: Yakir Yang Message-ID: <5785F182.5070901@rock-chips.com> Date: Wed, 13 Jul 2016 15:45:06 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <3636683.EuFECY2EQB@diego> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Heiko, On 07/12/2016 10:27 PM, Heiko Stübner wrote: > Hi Yakir, > > Am Montag, 11. Juli 2016, 19:05:49 schrieb Yakir Yang: >> For RK3399 HDMI, there is an external clock need for HDMI PHY, >> and it should keep the same clock rate with VOP DCLK. >> >> VPLL have supported the clock for HDMI PHY, but there is no >> clock divider bewteen VPLL and HDMI PHY. So we need to set the >> VPLL rate manually in HDMI driver. > I don't think reserving the vpll for the hdmi at all times is the right way to > go. > > While I do agree that reserving the VPLL (and NPLL on the rk3288) for graphics > use looks like the right way, I think the core Rockchip drm driver should be > responsible for managing it and also for deciding which output encoder gets to > use it. > While true that on the Chromebook device-types the edp is static and hdmi > needs the broad range of dynamic frequencies, this is not necessarily the case > for all future device types and/or socs. > > In the other thread we discussed adding that as rockchip,dclk-pll = <&...>; to > the base display-subsystem node, Doug didn't manage to find time to respond yet > though - and is on vacation right now. Great idea. Let a separate drm-pll driver to maintain all connector's clock requirement. > I still believe that would be the best solution :-) . > > > That property could list 1 or even 2 plls, depending on the soc or board > layout - maybe someone frees up the cpll in some special layout or something. > If none are listed, then the drm driver would need to cope with its available > clocks. > > Implementation-wise you could even still keep your shortcut until we encounter > a device with different , let the drm use the pll for hdmi only until someone > comes up with a better concept, but still the binding should be correct and > versatile from the start. Someone should start to prepare the drm core pll driver, it's great idea. BR, - Yakir > > Heiko > >> --- >> .../bindings/display/rockchip/dw_hdmi-rockchip.txt | 3 ++- >> drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 25 >> +++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) >> >> diff --git >> a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt >> b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt >> index 4e573d2..4e23ca4 100644 >> --- >> a/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt >> +++ >> b/Documentation/devicetree/bindings/display/rockchip/dw_hdmi-rockchip.txt >> @@ -17,7 +17,8 @@ Required properties: >> >> Optional properties >> - ddc-i2c-bus: phandle of an I2C controller used for DDC EDID probing >> -- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec" >> +- clocks, clock-names: phandle to the HDMI CEC clock, name should be "cec", >> + phandle to the VPLL clock, name should be "vpll". >> >> Example: >> hdmi: hdmi@ff980000 { >> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >> b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c index 329099b..701bb73 100644 >> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c >> @@ -7,10 +7,12 @@ >> * (at your option) any later version. >> */ >> >> +#include >> +#include >> #include >> #include >> -#include >> #include >> + >> #include >> #include >> #include >> @@ -33,6 +35,7 @@ struct rockchip_hdmi { >> struct regmap *regmap; >> struct drm_encoder encoder; >> enum dw_hdmi_devtype dev_type; >> + struct clk *vpll_clk; >> }; >> >> #define to_rockchip_hdmi(x) container_of(x, struct rockchip_hdmi, x) >> @@ -145,6 +148,7 @@ static const struct dw_hdmi_phy_config >> rockchip_phy_config[] = { static int rockchip_hdmi_parse_dt(struct >> rockchip_hdmi *hdmi) >> { >> struct device_node *np = hdmi->dev->of_node; >> + int ret; >> >> hdmi->regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf"); >> if (IS_ERR(hdmi->regmap)) { >> @@ -152,6 +156,22 @@ static int rockchip_hdmi_parse_dt(struct rockchip_hdmi >> *hdmi) return PTR_ERR(hdmi->regmap); >> } >> >> + hdmi->vpll_clk = devm_clk_get(hdmi->dev, "vpll"); >> + if (PTR_ERR(hdmi->vpll_clk) == -ENOENT) { >> + hdmi->vpll_clk = NULL; >> + } else if (PTR_ERR(hdmi->vpll_clk) == -EPROBE_DEFER) { >> + return -EPROBE_DEFER; >> + } else if (IS_ERR(hdmi->vpll_clk)) { >> + dev_err(hdmi->dev, "failed to get grf clock\n"); >> + return PTR_ERR(hdmi->vpll_clk); >> + } >> + >> + ret = clk_prepare_enable(hdmi->vpll_clk); >> + if (ret) { >> + dev_err(hdmi->dev, "Failed to enable HDMI vpll: %d\n", ret); >> + return ret; >> + } >> + >> return 0; >> } >> >> @@ -194,6 +214,9 @@ static void dw_hdmi_rockchip_encoder_mode_set(struct >> drm_encoder *encoder, struct drm_display_mode *mode, >> struct drm_display_mode *adj_mode) >> { >> + struct rockchip_hdmi *hdmi = to_rockchip_hdmi(encoder); >> + >> + clk_set_rate(hdmi->vpll_clk, adj_mode->clock * 1000); >> } >> >> static void dw_hdmi_rockchip_encoder_enable(struct drm_encoder *encoder) > > >