From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.codeaurora.org ([198.145.29.96]:48054 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752721AbdDLQhc (ORCPT ); Wed, 12 Apr 2017 12:37:32 -0400 Date: Wed, 12 Apr 2017 09:37:30 -0700 From: Stephen Boyd To: Neil Armstrong Cc: Linus Walleij , Eric Anholt , linux-clk , Mike Turquette , "linux-kernel@vger.kernel.org" , "open list:DRM PANEL DRIVERS" , Russell King Subject: Re: [PATCH v5] drm/pl111: Initial drm/kms driver for pl111 Message-ID: <20170412163730.GM7065@codeaurora.org> References: <20170411011801.15788-1-eric@anholt.net> <87lgr6ego2.fsf@eliezer.anholt.net> <3cba2a40-64fa-b73d-cc21-ebc27f4051bf@baylibre.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <3cba2a40-64fa-b73d-cc21-ebc27f4051bf@baylibre.com> Sender: linux-clk-owner@vger.kernel.org List-ID: On 04/12, Neil Armstrong wrote: > On 04/12/2017 09:40 AM, Linus Walleij wrote: > > On Wed, Apr 12, 2017 at 12:13 AM, Eric Anholt wrote: > > > >> Oh, one last thing I think we need to figure out: I'm using TIM2_CLKSEL, > >> which seems to be necessary on this platform. My understanding is that > >> this means that the pixel clock is divided from clcdclk instead of > >> apb_pclk. Do you agree? > > > > Yes the pixed clock is always derived from clcdclk. > > > > In most older ARM reference designs this is a VCO so that > > is why there is a clk_set_rate() on this in the fbdev code. > > (On some platforms that even has no effect I guess.) > > > >> The fbdev driver is using > >> clk_get(&fb->dev->dev, NULL) and not TIM2_CLKSEL, which I'm surprised by > >> because I would have thought that would give us the first clock from the > >> DT node (also clcdclk). > > > > So that thing is a 1-bit line that can select one of two clocks > > to be muxed into the PL111/CLCD. > > > > I guess that up until now all platforms just left that line dangling in > > the silicon. Congratulations, you came here first ;) > > > > Though when I look at the Nomadik it seems that it might be muxing > > the clock between 48 and 72 MHz, and I've been using 48MHz > > all along ooopsie. > > > > The current assumption in the bindings is that we have only > > one clock and TIM2_CLKSEL is N/A. > > > > If we want proper clcdclk handling with CLKSEL you should > > probably add some code to implement a real mux clock for > > this using and drivers/clk/clk-mux.c > > with select COMMON_CLK > > so that the driver still only sees clcdclk but that in turn is a > > mux that can select one of two sources and will react to > > the clk_set_rate() call by selecting the clock which is > > closest in frequency to what you want. > > > > This needs a small patch to alter the bindings too I guess. > > A small clock node inside the CLCD, just like PCI bridges have > > irqchips inside them etc: > > > > clcd@10120000 { > > compatible = "arm,pl110", "arm,primecell"; > > reg = <0x10120000 0x1000>; > > (...) > > clocks = <&clcdclk>, <&foo>; > > clock-names = "clcdclk", "apb_pclk"; > > > > clcdclk: clock-controller@0 { > > compatible = "arm,pl11x-clock-mux"; > > clocks = <&source_a>, <&source_b>; > > }; > > }; > > > > This can be set up easily in the OF probe path since that > > is what we're doing: just look for this subnode, if it is there > > create the clock controller. > > > > I do not think the clk maintainers would mind a small mux > > clock controller inside the CLCD driver to handle this mux > > if we need it. > > Hi Linus, > > Indeed it's the way of handling this use case, but no need to add > a clk node here, you can copy what we did in pwm-meson, meson-gx-mmc.c, > or dwmac-meson8b.c (we went further by also adding clk dividers). > > In the probe code, simply add a clock mux provider with the two parents > clock names (these can and should be platform specific) and only call > a clk_set_parent() with the clock specified in the node clocks cell. > +1 Avoiding a binding update is nice. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Boyd Subject: Re: [PATCH v5] drm/pl111: Initial drm/kms driver for pl111 Date: Wed, 12 Apr 2017 09:37:30 -0700 Message-ID: <20170412163730.GM7065@codeaurora.org> References: <20170411011801.15788-1-eric@anholt.net> <87lgr6ego2.fsf@eliezer.anholt.net> <3cba2a40-64fa-b73d-cc21-ebc27f4051bf@baylibre.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from smtp.codeaurora.org (smtp.codeaurora.org [198.145.29.96]) by gabe.freedesktop.org (Postfix) with ESMTPS id 97A336E75B for ; Wed, 12 Apr 2017 16:37:31 +0000 (UTC) Content-Disposition: inline In-Reply-To: <3cba2a40-64fa-b73d-cc21-ebc27f4051bf@baylibre.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Neil Armstrong Cc: Mike Turquette , "linux-kernel@vger.kernel.org" , "open list:DRM PANEL DRIVERS" , Russell King , linux-clk List-Id: dri-devel@lists.freedesktop.org T24gMDQvMTIsIE5laWwgQXJtc3Ryb25nIHdyb3RlOgo+IE9uIDA0LzEyLzIwMTcgMDk6NDAgQU0s IExpbnVzIFdhbGxlaWogd3JvdGU6Cj4gPiBPbiBXZWQsIEFwciAxMiwgMjAxNyBhdCAxMjoxMyBB TSwgRXJpYyBBbmhvbHQgPGVyaWNAYW5ob2x0Lm5ldD4gd3JvdGU6Cj4gPiAKPiA+PiBPaCwgb25l IGxhc3QgdGhpbmcgSSB0aGluayB3ZSBuZWVkIHRvIGZpZ3VyZSBvdXQ6IEknbSB1c2luZyBUSU0y X0NMS1NFTCwKPiA+PiB3aGljaCBzZWVtcyB0byBiZSBuZWNlc3Nhcnkgb24gdGhpcyBwbGF0Zm9y bS4gIE15IHVuZGVyc3RhbmRpbmcgaXMgdGhhdAo+ID4+IHRoaXMgbWVhbnMgdGhhdCB0aGUgcGl4 ZWwgY2xvY2sgaXMgZGl2aWRlZCBmcm9tIGNsY2RjbGsgaW5zdGVhZCBvZgo+ID4+IGFwYl9wY2xr LiAgRG8geW91IGFncmVlPwo+ID4gCj4gPiBZZXMgdGhlIHBpeGVkIGNsb2NrIGlzIGFsd2F5cyBk ZXJpdmVkIGZyb20gY2xjZGNsay4KPiA+IAo+ID4gSW4gbW9zdCBvbGRlciBBUk0gcmVmZXJlbmNl IGRlc2lnbnMgdGhpcyBpcyBhIFZDTyBzbyB0aGF0Cj4gPiBpcyB3aHkgdGhlcmUgaXMgYSBjbGtf c2V0X3JhdGUoKSBvbiB0aGlzIGluIHRoZSBmYmRldiBjb2RlLgo+ID4gKE9uIHNvbWUgcGxhdGZv cm1zIHRoYXQgZXZlbiBoYXMgbm8gZWZmZWN0IEkgZ3Vlc3MuKQo+ID4gCj4gPj4gVGhlIGZiZGV2 IGRyaXZlciBpcyB1c2luZwo+ID4+IGNsa19nZXQoJmZiLT5kZXYtPmRldiwgTlVMTCkgYW5kIG5v dCBUSU0yX0NMS1NFTCwgd2hpY2ggSSdtIHN1cnByaXNlZCBieQo+ID4+IGJlY2F1c2UgSSB3b3Vs ZCBoYXZlIHRob3VnaHQgdGhhdCB3b3VsZCBnaXZlIHVzIHRoZSBmaXJzdCBjbG9jayBmcm9tIHRo ZQo+ID4+IERUIG5vZGUgKGFsc28gY2xjZGNsaykuCj4gPiAKPiA+IFNvIHRoYXQgdGhpbmcgaXMg YSAxLWJpdCBsaW5lIHRoYXQgY2FuIHNlbGVjdCBvbmUgb2YgdHdvIGNsb2Nrcwo+ID4gdG8gYmUg bXV4ZWQgaW50byB0aGUgUEwxMTEvQ0xDRC4KPiA+IAo+ID4gSSBndWVzcyB0aGF0IHVwIHVudGls IG5vdyBhbGwgcGxhdGZvcm1zIGp1c3QgbGVmdCB0aGF0IGxpbmUgZGFuZ2xpbmcgaW4KPiA+IHRo ZSBzaWxpY29uLiBDb25ncmF0dWxhdGlvbnMsIHlvdSBjYW1lIGhlcmUgZmlyc3QgOykKPiA+IAo+ ID4gVGhvdWdoIHdoZW4gSSBsb29rIGF0IHRoZSBOb21hZGlrIGl0IHNlZW1zIHRoYXQgaXQgbWln aHQgYmUgbXV4aW5nCj4gPiB0aGUgY2xvY2sgYmV0d2VlbiA0OCBhbmQgNzIgTUh6LCBhbmQgSSd2 ZSBiZWVuIHVzaW5nIDQ4TUh6Cj4gPiBhbGwgYWxvbmcgb29vcHNpZS4KPiA+IAo+ID4gVGhlIGN1 cnJlbnQgYXNzdW1wdGlvbiBpbiB0aGUgYmluZGluZ3MgaXMgdGhhdCB3ZSBoYXZlIG9ubHkKPiA+ IG9uZSBjbG9jayBhbmQgVElNMl9DTEtTRUwgaXMgTi9BLgo+ID4gCj4gPiBJZiB3ZSB3YW50IHBy b3BlciBjbGNkY2xrIGhhbmRsaW5nIHdpdGggQ0xLU0VMIHlvdSBzaG91bGQKPiA+IHByb2JhYmx5 IGFkZCBzb21lIGNvZGUgdG8gaW1wbGVtZW50IGEgcmVhbCBtdXggY2xvY2sgZm9yCj4gPiB0aGlz IHVzaW5nIDxsaW51eC9jbG9jay1wcm92aWRlci5oPiBhbmQgZHJpdmVycy9jbGsvY2xrLW11eC5j Cj4gPiB3aXRoIHNlbGVjdCBDT01NT05fQ0xLCj4gPiBzbyB0aGF0IHRoZSBkcml2ZXIgc3RpbGwg b25seSBzZWVzIGNsY2RjbGsgYnV0IHRoYXQgaW4gdHVybiBpcyBhCj4gPiBtdXggdGhhdCBjYW4g c2VsZWN0IG9uZSBvZiB0d28gc291cmNlcyBhbmQgd2lsbCByZWFjdCB0bwo+ID4gdGhlIGNsa19z ZXRfcmF0ZSgpIGNhbGwgYnkgc2VsZWN0aW5nIHRoZSBjbG9jayB3aGljaCBpcwo+ID4gY2xvc2Vz dCBpbiBmcmVxdWVuY3kgdG8gd2hhdCB5b3Ugd2FudC4KPiA+IAo+ID4gVGhpcyBuZWVkcyBhIHNt YWxsIHBhdGNoIHRvIGFsdGVyIHRoZSBiaW5kaW5ncyB0b28gSSBndWVzcy4KPiA+IEEgc21hbGwg Y2xvY2sgbm9kZSBpbnNpZGUgdGhlIENMQ0QsIGp1c3QgbGlrZSBQQ0kgYnJpZGdlcyBoYXZlCj4g PiBpcnFjaGlwcyBpbnNpZGUgdGhlbSBldGM6Cj4gPiAKPiA+IGNsY2RAMTAxMjAwMDAgewo+ID4g ICAgICAgICBjb21wYXRpYmxlID0gImFybSxwbDExMCIsICJhcm0scHJpbWVjZWxsIjsKPiA+ICAg ICAgICAgcmVnID0gPDB4MTAxMjAwMDAgMHgxMDAwPjsKPiA+ICAgICAgICAgKC4uLikKPiA+ICAg ICAgICAgY2xvY2tzID0gPCZjbGNkY2xrPiwgPCZmb28+Owo+ID4gICAgICAgICBjbG9jay1uYW1l cyA9ICJjbGNkY2xrIiwgImFwYl9wY2xrIjsKPiA+IAo+ID4gICAgICAgICBjbGNkY2xrOiBjbG9j ay1jb250cm9sbGVyQDAgewo+ID4gICAgICAgICAgICAgICAgIGNvbXBhdGlibGUgPSAiYXJtLHBs MTF4LWNsb2NrLW11eCI7Cj4gPiAgICAgICAgICAgICAgICAgY2xvY2tzID0gPCZzb3VyY2VfYT4s IDwmc291cmNlX2I+Owo+ID4gICAgICAgICB9Owo+ID4gfTsKPiA+IAo+ID4gVGhpcyBjYW4gYmUg c2V0IHVwIGVhc2lseSBpbiB0aGUgT0YgcHJvYmUgcGF0aCBzaW5jZSB0aGF0Cj4gPiBpcyB3aGF0 IHdlJ3JlIGRvaW5nOiBqdXN0IGxvb2sgZm9yIHRoaXMgc3Vibm9kZSwgaWYgaXQgaXMgdGhlcmUK PiA+IGNyZWF0ZSB0aGUgY2xvY2sgY29udHJvbGxlci4KPiA+IAo+ID4gSSBkbyBub3QgdGhpbmsg dGhlIGNsayBtYWludGFpbmVycyB3b3VsZCBtaW5kIGEgc21hbGwgbXV4Cj4gPiBjbG9jayBjb250 cm9sbGVyIGluc2lkZSB0aGUgQ0xDRCBkcml2ZXIgdG8gaGFuZGxlIHRoaXMgbXV4Cj4gPiBpZiB3 ZSBuZWVkIGl0Lgo+IAo+IEhpIExpbnVzLAo+IAo+IEluZGVlZCBpdCdzIHRoZSB3YXkgb2YgaGFu ZGxpbmcgdGhpcyB1c2UgY2FzZSwgYnV0IG5vIG5lZWQgdG8gYWRkCj4gYSBjbGsgbm9kZSBoZXJl LCB5b3UgY2FuIGNvcHkgd2hhdCB3ZSBkaWQgaW4gcHdtLW1lc29uLCBtZXNvbi1neC1tbWMuYywK PiBvciBkd21hYy1tZXNvbjhiLmMgKHdlIHdlbnQgZnVydGhlciBieSBhbHNvIGFkZGluZyBjbGsg ZGl2aWRlcnMpLgo+IAo+IEluIHRoZSBwcm9iZSBjb2RlLCBzaW1wbHkgYWRkIGEgY2xvY2sgbXV4 IHByb3ZpZGVyIHdpdGggdGhlIHR3byBwYXJlbnRzCj4gY2xvY2sgbmFtZXMgKHRoZXNlIGNhbiBh bmQgc2hvdWxkIGJlIHBsYXRmb3JtIHNwZWNpZmljKSBhbmQgb25seSBjYWxsCj4gYSBjbGtfc2V0 X3BhcmVudCgpIHdpdGggdGhlIGNsb2NrIHNwZWNpZmllZCBpbiB0aGUgbm9kZSBjbG9ja3MgY2Vs bC4KPiAKCisxCgpBdm9pZGluZyBhIGJpbmRpbmcgdXBkYXRlIGlzIG5pY2UuCgotLSAKUXVhbGNv bW0gSW5ub3ZhdGlvbiBDZW50ZXIsIEluYy4gaXMgYSBtZW1iZXIgb2YgQ29kZSBBdXJvcmEgRm9y dW0sCmEgTGludXggRm91bmRhdGlvbiBDb2xsYWJvcmF0aXZlIFByb2plY3QKX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlz dApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0 b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg==