Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751336AbeAEKp7 (ORCPT + 1 other); Fri, 5 Jan 2018 05:45:59 -0500 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:43310 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750927AbeAEKp4 (ORCPT ); Fri, 5 Jan 2018 05:45:56 -0500 Cc: Sudeep Holla , ALKML , LKML , DTML , Roy Franz , Harb Abdulhamid , Nishanth Menon , Arnd Bergmann , Loc Ho , Ryan Harkin , Jassi Brar , linux-pm@vger.kernel.org Subject: Re: [PATCH v5 20/20] cpufreq: scmi: add support for fast frequency switching To: Alexey Klimov References: <1514904162-11201-1-git-send-email-sudeep.holla@arm.com> <1514904162-11201-21-git-send-email-sudeep.holla@arm.com> From: Sudeep Holla Organization: ARM Message-ID: Date: Fri, 5 Jan 2018 10:45:50 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On 04/01/18 22:10, Alexey Klimov wrote: > Hi Sudeep, > > On Tue, Jan 2, 2018 at 2:42 PM, Sudeep Holla wrote: >> The cpufreq core provides option for drivers to implement fast_switch >> callback which is invoked for frequency switching from interrupt context. >> >> This patch adds support for fast_switch callback in SCMI cpufreq driver >> by making use of polling based SCMI transfer. It also sets the flag >> fast_switch_possible. >> >> Cc: linux-pm@vger.kernel.org >> Acked-by: Rafael J. Wysocki >> Acked-by: Viresh Kumar >> Signed-off-by: Sudeep Holla >> --- >> drivers/cpufreq/scmi-cpufreq.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c >> index 0ee9335d0063..d0a82d7c6fd4 100644 >> --- a/drivers/cpufreq/scmi-cpufreq.c >> +++ b/drivers/cpufreq/scmi-cpufreq.c >> @@ -64,6 +64,19 @@ scmi_cpufreq_set_target(struct cpufreq_policy *policy, unsigned int index) >> return perf_ops->freq_set(handle, priv->domain_id, freq, false); >> } >> >> +static unsigned int scmi_cpufreq_fast_switch(struct cpufreq_policy *policy, >> + unsigned int target_freq) >> +{ >> + struct scmi_data *priv = policy->driver_data; >> + struct scmi_perf_ops *perf_ops = handle->perf_ops; >> + >> + if (!perf_ops->freq_set(handle, priv->domain_id, >> + target_freq * 1000, true)) >> + return target_freq; >> + >> + return 0; >> +} > > Could you please explain how it's supposed to work for purpose of fast > frequency switching? > > I am trying to track down ->freq_set. > So it looks like this will fire an scmi perf level set command and > will poll for this command to complete without asking for firmware to > send command completion irq. > > scmi_perf_level_set() will call the following functions: > > scmi_one_xfer_init(); > scmi_do_xfer(handle, t); > scmi_one_xfer_put(handle, t); > > > The first function in the list calls scmi_one_xfer_get() which has > this in the description (I guess because of down_timeout()): > "This function can sleep depending on pending requests already in the system > for the SCMI entity. Further, this also holds a spinlock to maintain > integrity of internal data structures." > > So it can sleep. > Indeed, I can drop the whole semaphore story and expect the caller to retry in case buffer is full which is very rare condition. > As far as I see description of fast frequency switching it's required > for fast_switch to not sleep: > (file Documentation/cpu-freq/cpu-drivers.txt) > > "This function is used for frequency switching from scheduler's context. > Not all drivers are expected to implement it, as sleeping from within > this callback isn't allowed. This callback must be highly optimized to > do switching as fast as possible." > > > The other questions to this implementation of fast switching: > > 1) Fast switching callback must be highly optimized. Is it now? I see > few spinlocks (in scmi mbox client and in the mailbox framework) there > and polling functionality with udelay(5) inside that will timeout (if > my calculations are correct) after 0.5 ms. Do you have any alternate ideas to avoid that and still achieve fast switching ? > 2) Is it highly dependent on transport? If mailbox transport > ->send_data() may sleep or hrtimer-based polling in mailbox framework > will be used, then this fast switch won't work, right? > Yes. > I am still looking into that: I can be wrong and just trying to > understand if it is all okay. > Thanks for taking a look at this. -- Regards, Sudeep