From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753193Ab3CCKyd (ORCPT ); Sun, 3 Mar 2013 05:54:33 -0500 Received: from mail-pa0-f45.google.com ([209.85.220.45]:34608 "EHLO mail-pa0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752842Ab3CCKyc convert rfc822-to-8bit (ORCPT ); Sun, 3 Mar 2013 05:54:32 -0500 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT To: Richard Zhao , Bill Huang From: Mike Turquette In-Reply-To: <20130302082216.GA4581@richard-laptop> Cc: "linaro-dev@lists.linaro.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "patches@linaro.org" References: <1362026969-11457-1-git-send-email-mturquette@linaro.org> <1362026969-11457-3-git-send-email-mturquette@linaro.org> <1362130891.19498.12.camel@bilhuang-vm1> <20130301182234.6210.63879@quantum> <20130301204832.6210.40653@quantum> <1362192954.2407.26.camel@bilhuang-vm1> <20130302082216.GA4581@richard-laptop> Message-ID: <20130303105424.6210.1196@quantum> User-Agent: alot/0.3.3+ Subject: Re: [PATCH 2/5] clk: notifier handler for dynamic voltage scaling Date: Sun, 03 Mar 2013 02:54:24 -0800 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Quoting Richard Zhao (2013-03-02 00:22:19) > On Fri, Mar 01, 2013 at 06:55:54PM -0800, Bill Huang wrote: > > On Sat, 2013-03-02 at 04:48 +0800, Mike Turquette wrote: > > > Quoting Mike Turquette (2013-03-01 10:22:34) > > > > Quoting Bill Huang (2013-03-01 01:41:31) > > > > > On Thu, 2013-02-28 at 12:49 +0800, Mike Turquette wrote: > > > > > > Dynamic voltage and frequency scaling (dvfs) is a common power saving > > > > > > technique in many of today's modern processors. This patch introduces a > > > > > > common clk rate-change notifier handler which scales voltage > > > > > > appropriately whenever clk_set_rate is called on an affected clock. > > > > > > > > > > I really think clk_enable and clk_disable should also be triggering > > > > > notifier call and DVFS should act accordingly since there are cases > > > > > drivers won't set clock rate but instead disable its clock directly, do > > > > > you agree? > > > > > > > > > > > > > > Hi Bill, > > > > > > > > I'll think about this. Perhaps a better solution would be to adapt > > > > these drivers to runtime PM. Then a call to runtime_pm_put() would > > > > result in a call to clk_disable(...) and regulator_set_voltage(...). > > > > > > > > There is no performance-based equivalent to runtime PM, which is one > > > > reason why clk_set_rate is a likely entry point into dvfs. But for > > > > operations that have nice api's like runtime PM it would be better to > > > > use those interfaces and not overload the clk.h api unnecessarily. > > > > > > > > > > Bill, > > > > > > I wasn't thinking at all when I wrote this. Trying to rush to the > > > airport I guess... > > > > > > clk_enable() and clk_disable() must not sleep and all operations are > > > done under a spinlock. So this rules out most use of notifiers. It is > > > expected for some drivers to very aggressively enable/disable clocks in > > > interrupt handlers so scaling voltage as a function of clk_{en|dis}able > > > calls is also likely out of the question. > > > > Yeah for those existing drivers to call enable/disable clocks in > > interrupt have ruled out this, I didn't think through when I was asking. > > > > > > Some platforms have chosen to implement voltage scaling in their > > > .prepare callbacks. I personally do not like this and still prefer > > > drivers be adapted to runtime pm and let those callbacks handle voltage > > > scaling along with clock handling. > Voltage scaling in clock notifiers seems similar. Clock and regulater > embedded code into each other will cause things complicated. Hi Richard, Sorry, I do not follow the above statement. Can you clarify what you mean? > > > > I think different SoC have different mechanisms or constraints on doing > > their DVFS, such as Tegra VDD_CORE rail, it supplies power to many > > devices and as a consequence each device do not have their own power > > rail to control, instead a central driver to handle/control this power > > rail is needed (to set voltage at the maximum of the requested voltage > > from all its sub-devices), so I'm wondering even if every drivers are > > doing DVFS through runtime pm, we're still having hole on knowing > > whether or not clocks of the interested devices are enabled/disabled at > > runtime, I'm not familiar with runtime pm and hence do not know if there > > is a mechanism to handle this, I'll study a bit. Thanks. > > > > > > Regards, > > > Mike > > > > > > > > > There are three prerequisites to using this feature: > > > > > > > > > > > > 1) the affected clocks must be using the common clk framework > > > > > > 2) voltage must be scaled using the regulator framework > > > > > > 3) clock frequency and regulator voltage values must be paired via the > > > > > > OPP library > > > > > > > > > > Just a note, Tegra Core won't meet prerequisite #3 since each regulator > > > > > voltage values is associated with clocks driving those many sub-HW > > > > > blocks in it. > > > > > > > > This patch isn't the one and only way to perform dvfs. It is just a > > > > helper function for registering notifier handlers for systems that meet > > > > the above three requirements. Even if you do not use the OPP library > > > > there is no reason why you could not register your own rate-change > > > > notifier handler to implement dvfs using whatever lookup-table you use > > > > today. > > > > > > > > And patches are welcome to extend the usefulness of this helper. I'd > > > > like as many people to benefit from this mechanism as possible. > > > > The extension is not so easy for us though since OPP library is assuming > > each device has a 1-1 mapping on its operating frequency and voltage. > Is there someone working on OPP clock/regulator sets support? > No, but I'm going to bring this up at LCA on Wednesday. It would be nice to have some DT bindings for declaring operating points that tie clocks & regulators together. Regards, Mike > Thanks > Richard > > > > > > > > At some point we should think hard about DT bindings for these operating > > > > points... > > > > > > > > Regards, > > > > Mike > > > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel