* [PATCH V2] thermal: qcom-spmi-temp-alarm: add support for GEN2 PMIC peripherals
From: Kiran Gunda @ 2018-05-24 7:19 UTC (permalink / raw)
To: rui.zhang, sboyd, Eduardo Valentin, linux-pm, linux-kernel
Cc: linux-arm-msm, linux-arm-msm-owner, bjorn.andersson,
David Collins, Kiran Gunda
From: David Collins <collinsd@codeaurora.org>
Add support for the TEMP_ALARM GEN2 PMIC peripheral subtype. The
GEN2 subtype defines an over temperature state with hysteresis
instead of stage in the status register. There are two GEN2
states corresponding to stages 1 and 2.
Signed-off-by: David Collins <collinsd@codeaurora.org>
Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
---
drivers/thermal/qcom-spmi-temp-alarm.c | 92 ++++++++++++++++++++++++++--------
1 file changed, 71 insertions(+), 21 deletions(-)
Changes from [V1]:
Rebased on top of 4.17-rc6
diff --git a/drivers/thermal/qcom-spmi-temp-alarm.c b/drivers/thermal/qcom-spmi-temp-alarm.c
index 95f987d..ad4f3a8 100644
--- a/drivers/thermal/qcom-spmi-temp-alarm.c
+++ b/drivers/thermal/qcom-spmi-temp-alarm.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2011-2015, 2017, The Linux Foundation. All rights reserved.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 and
@@ -11,6 +11,7 @@
* GNU General Public License for more details.
*/
+#include <linux/bitops.h>
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/iio/consumer.h>
@@ -29,13 +30,17 @@
#define QPNP_TM_REG_ALARM_CTRL 0x46
#define QPNP_TM_TYPE 0x09
-#define QPNP_TM_SUBTYPE 0x08
+#define QPNP_TM_SUBTYPE_GEN1 0x08
+#define QPNP_TM_SUBTYPE_GEN2 0x09
-#define STATUS_STAGE_MASK 0x03
+#define STATUS_GEN1_STAGE_MASK GENMASK(1, 0)
+#define STATUS_GEN2_STATE_MASK GENMASK(6, 4)
+#define STATUS_GEN2_STATE_SHIFT 4
-#define SHUTDOWN_CTRL1_THRESHOLD_MASK 0x03
+#define SHUTDOWN_CTRL1_OVERRIDE_MASK GENMASK(7, 6)
+#define SHUTDOWN_CTRL1_THRESHOLD_MASK GENMASK(1, 0)
-#define ALARM_CTRL_FORCE_ENABLE 0x80
+#define ALARM_CTRL_FORCE_ENABLE BIT(7)
/*
* Trip point values based on threshold control
@@ -58,6 +63,7 @@
struct qpnp_tm_chip {
struct regmap *map;
struct thermal_zone_device *tz_dev;
+ unsigned int subtype;
long temp;
unsigned int thresh;
unsigned int stage;
@@ -66,6 +72,9 @@ struct qpnp_tm_chip {
struct iio_channel *adc;
};
+/* This array maps from GEN2 alarm state to GEN1 alarm stage */
+static const unsigned int alarm_state_map[8] = {0, 1, 1, 2, 2, 3, 3, 3};
+
static int qpnp_tm_read(struct qpnp_tm_chip *chip, u16 addr, u8 *data)
{
unsigned int val;
@@ -84,30 +93,59 @@ static int qpnp_tm_write(struct qpnp_tm_chip *chip, u16 addr, u8 data)
return regmap_write(chip->map, chip->base + addr, data);
}
+/**
+ * qpnp_tm_get_temp_stage() - return over-temperature stage
+ * @chip: Pointer to the qpnp_tm chip
+ *
+ * Return: stage (GEN1) or state (GEN2) on success, or errno on failure.
+ */
+static int qpnp_tm_get_temp_stage(struct qpnp_tm_chip *chip)
+{
+ int ret;
+ u8 reg = 0;
+
+ ret = qpnp_tm_read(chip, QPNP_TM_REG_STATUS, ®);
+ if (ret < 0)
+ return ret;
+
+ if (chip->subtype == QPNP_TM_SUBTYPE_GEN1)
+ ret = reg & STATUS_GEN1_STAGE_MASK;
+ else
+ ret = (reg & STATUS_GEN2_STATE_MASK) >> STATUS_GEN2_STATE_SHIFT;
+
+ return ret;
+}
+
/*
* This function updates the internal temp value based on the
* current thermal stage and threshold as well as the previous stage
*/
static int qpnp_tm_update_temp_no_adc(struct qpnp_tm_chip *chip)
{
- unsigned int stage;
+ unsigned int stage, stage_new, stage_old;
int ret;
- u8 reg = 0;
- ret = qpnp_tm_read(chip, QPNP_TM_REG_STATUS, ®);
+ ret = qpnp_tm_get_temp_stage(chip);
if (ret < 0)
return ret;
+ stage = ret;
- stage = reg & STATUS_STAGE_MASK;
+ if (chip->subtype == QPNP_TM_SUBTYPE_GEN1) {
+ stage_new = stage;
+ stage_old = chip->stage;
+ } else {
+ stage_new = alarm_state_map[stage];
+ stage_old = alarm_state_map[chip->stage];
+ }
- if (stage > chip->stage) {
+ if (stage_new > stage_old) {
/* increasing stage, use lower bound */
- chip->temp = (stage - 1) * TEMP_STAGE_STEP +
+ chip->temp = (stage_new - 1) * TEMP_STAGE_STEP +
chip->thresh * TEMP_THRESH_STEP +
TEMP_STAGE_HYSTERESIS + TEMP_THRESH_MIN;
- } else if (stage < chip->stage) {
+ } else if (stage_new < stage_old) {
/* decreasing stage, use upper bound */
- chip->temp = stage * TEMP_STAGE_STEP +
+ chip->temp = stage_new * TEMP_STAGE_STEP +
chip->thresh * TEMP_THRESH_STEP -
TEMP_STAGE_HYSTERESIS + TEMP_THRESH_MIN;
}
@@ -162,28 +200,37 @@ static irqreturn_t qpnp_tm_isr(int irq, void *data)
*/
static int qpnp_tm_init(struct qpnp_tm_chip *chip)
{
+ unsigned int stage;
int ret;
- u8 reg;
+ u8 reg = 0;
- chip->thresh = THRESH_MIN;
+ ret = qpnp_tm_read(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, ®);
+ if (ret < 0)
+ return ret;
+
+ chip->thresh = reg & SHUTDOWN_CTRL1_THRESHOLD_MASK;
chip->temp = DEFAULT_TEMP;
- ret = qpnp_tm_read(chip, QPNP_TM_REG_STATUS, ®);
+ ret = qpnp_tm_get_temp_stage(chip);
if (ret < 0)
return ret;
+ chip->stage = ret;
- chip->stage = reg & STATUS_STAGE_MASK;
+ stage = chip->subtype == QPNP_TM_SUBTYPE_GEN1
+ ? chip->stage : alarm_state_map[chip->stage];
- if (chip->stage)
+ if (stage)
chip->temp = chip->thresh * TEMP_THRESH_STEP +
- (chip->stage - 1) * TEMP_STAGE_STEP +
+ (stage - 1) * TEMP_STAGE_STEP +
TEMP_THRESH_MIN;
/*
* Set threshold and disable software override of stage 2 and 3
* shutdowns.
*/
- reg = chip->thresh & SHUTDOWN_CTRL1_THRESHOLD_MASK;
+ chip->thresh = THRESH_MIN;
+ reg &= ~(SHUTDOWN_CTRL1_OVERRIDE_MASK | SHUTDOWN_CTRL1_THRESHOLD_MASK);
+ reg |= chip->thresh & SHUTDOWN_CTRL1_THRESHOLD_MASK;
ret = qpnp_tm_write(chip, QPNP_TM_REG_SHUTDOWN_CTRL1, reg);
if (ret < 0)
return ret;
@@ -246,12 +293,15 @@ static int qpnp_tm_probe(struct platform_device *pdev)
return ret;
}
- if (type != QPNP_TM_TYPE || subtype != QPNP_TM_SUBTYPE) {
+ if (type != QPNP_TM_TYPE || (subtype != QPNP_TM_SUBTYPE_GEN1
+ && subtype != QPNP_TM_SUBTYPE_GEN2)) {
dev_err(&pdev->dev, "invalid type 0x%02x or subtype 0x%02x\n",
type, subtype);
return -ENODEV;
}
+ chip->subtype = subtype;
+
ret = qpnp_tm_init(chip);
if (ret < 0) {
dev_err(&pdev->dev, "init failed\n");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* Re: [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Ulf Hansson @ 2018-05-24 7:04 UTC (permalink / raw)
To: Jon Hunter
Cc: Rajendra Nayak, Geert Uytterhoeven, Linux PM, Greg Kroah-Hartman,
Kevin Hilman, Rafael J . Wysocki, Linux Kernel Mailing List,
Todor Tomov, Viresh Kumar, linux-tegra, Vincent Guittot,
Linux ARM
In-Reply-To: <3838f17a-2ac8-bf3f-f0b1-f69bbe17629c@nvidia.com>
On 23 May 2018 at 11:07, Jon Hunter <jonathanh@nvidia.com> wrote:
>
> On 23/05/18 07:12, Ulf Hansson wrote:
>
> ...
>
>
>>>>>> Thanks for sending this. Believe it or not this has still been on my
>>>>>> to-do list
>>>>>> and so we definitely need a solution for Tegra.
>>>>>>
>>>>>> Looking at the above it appears that additional power-domains exposed
>>>>>> as devices
>>>>>> to the client device. So I assume that this means that the drivers for
>>>>>> devices
>>>>>> with multiple power-domains will need to call RPM APIs for each of
>>>>>> these
>>>>>> additional power-domains. Is that correct?
>>>>>
>>>>>
>>>>> They can, but should not!
>>>>>
>>>>> Instead, the driver shall use device_link_add() and device_link_del(),
>>>>> dynamically, depending on what PM domain that their original device
>>>>> needs for the current running use case.
>>>>>
>>>>> In that way, they keep existing runtime PM deployment, operating on
>>>>> its original device.
>>>>
>>>>
>>>> OK, sounds good. Any reason why the linking cannot be handled by the
>>>> above API? Is there a use-case where you would not want it linked?
>>>
>>>
>>> I am guessing the linking is what would give the driver the ability to
>>> decide which subset of powerdomains it actually wants to control
>>> at any point using runtime PM. If we have cases wherein the driver would
>>> want to turn on/off _all_ its associated powerdomains _always_
>>> then a default linking of all would help.
>>
>>
>> First, I think we need to decide on *where* the linking should be
>> done, not at both places, as that would just mess up synchronization
>> of who is responsible for calling the device_link_del() at detach.
>>
>> Second, It would in principle be fine to call device_link_add() and
>> device_link_del() as a part of the attach/detach APIs. However, there
>> is a downside to such solution, which would be that the driver then
>> needs call the detach API, just to do device_link_del(). Of course
>> then it would also needs to call the attach API later if/when needed.
>> Doing this adds unnecessary overhead - comparing to just let the
>> driver call device_link_add|del() when needed. On the upside, yes, it
>> would put less burden on the drivers as it then only needs to care
>> about using one set of functions.
>>
>> Which solution do you prefer?
>
>
> Any reason why we could not add a 'boolean' argument to the API to indicate
> whether the new device should be linked? I think that I prefer the API
> handles it, but I can see there could be instances where drivers may wish to
> handle it themselves.
Coming back to this question. Both Tegra XUSB and Qcom Camera use
case, would benefit from doing the linking themselves, as it needs
different PM domains to be powered on depending on the current use
case - as to avoid wasting power.
However, I can understand that you prefer some simplicity over
optimizations, as you told us. Then, does it mean that you are
insisting on extending the APIs with a boolean for linking, or are you
fine with the driver to call device_link_add()?
[...]
Kind regards
Uffe
^ permalink raw reply
* [PATCH] cpufreq: reinitialize new policy min/max when writing scaling_(max|min)_freq
From: Kevin Wangtao @ 2018-05-24 6:43 UTC (permalink / raw)
To: rjw, viresh.kumar
Cc: linux-pm, linux-kernel, gengyanping, sunzhaosheng, Kevin Wangtao
consider such situation, current user_policy.min is 1000000,
current user_policy.max is 1200000, in cpufreq_set_policy,
other driver may update policy.min to 1200000, policy.max to
1300000. After that, If we input "echo 1300000 > scaling_min_freq",
then user_policy.min will be 1300000, and user_policy.max is
still 1200000, because the input value is checked with policy.max
not user_policy.max. if we get all related cpus offline and
online again, it will cause cpufreq_init_policy fail because
user_policy.min is higher than user_policy.max.
The solution is when user space tries to write scaling_(max|min)_freq,
the min/max of new_policy should be reinitialized with min/max
of user_policy, like what cpufreq_update_policy does.
Signed-off-by: Kevin Wangtao <kevin.wangtao@hisilicon.com>
---
drivers/cpufreq/cpufreq.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index b79c532..8b33e08 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -697,6 +697,8 @@ static ssize_t store_##file_name \
struct cpufreq_policy new_policy; \
\
memcpy(&new_policy, policy, sizeof(*policy)); \
+ new_policy->min = policy->user_policy.min; \
+ new_policy->max = policy->user_policy.max; \
\
ret = sscanf(buf, "%u", &new_policy.object); \
if (ret != 1) \
--
2.8.1
^ permalink raw reply related
* Re: [PATCH v2 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings
From: Taniya Das @ 2018-05-24 5:46 UTC (permalink / raw)
To: Sudeep Holla, linux-kernel, linux-pm, Stephen Boyd, robh
Cc: Rafael J. Wysocki, Viresh Kumar, Rajendra Nayak, Amit Nischal,
devicetree, skannan, amit.kucheria
In-Reply-To: <34b510f5-fd0b-2e28-45b0-f200f9f8d2c5@arm.com>
On 5/23/2018 8:43 PM, Sudeep Holla wrote:
>
>
> On 19/05/18 18:34, Taniya Das wrote:
>> Add QCOM cpufreq firmware device bindings for Qualcomm Technology Inc's
>> SoCs. This is required for managing the cpu frequency transitions which are
>> controlled by firmware.
>>
>> Signed-off-by: Taniya Das <tdas@codeaurora.org>
>> ---
>> .../bindings/cpufreq/cpufreq-qcom-fw.txt | 68 ++++++++++++++++++++++
>> 1 file changed, 68 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
>>
>> diff --git a/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
>> new file mode 100644
>> index 0000000..bc912f4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/cpufreq/cpufreq-qcom-fw.txt
>> @@ -0,0 +1,68 @@
>> +Qualcomm Technologies, Inc. CPUFREQ Bindings
>> +
>> +CPUFREQ FW is a hardware engine used by some Qualcomm Technologies, Inc. (QTI)
>> +SoCs to manage frequency in hardware. It is capable of controlling frequency
>> +for multiple clusters.
>> +
>> +Properties:
>> +- compatible
>> + Usage: required
>> + Value type: <string>
>> + Definition: must be "qcom,cpufreq-fw".
>> +
>
> If the firmware is referred with some other name, better to use that
> than cpufreq
>> +Note that #address-cells, #size-cells, and ranges shall be present to ensure
>> +the cpufreq can address a freq-domain registers.
>> +
>> +A freq-domain sub-node would be defined for the cpus with the following
>> +properties:
>> +
>> +- compatible:
>> + Usage: required
>> + Value type: <string>
>> + Definition: must be "cpufreq".
>> +
>> +- reg
>> + Usage: required
>> + Value type: <prop-encoded-array>
>> + Definition: Addresses and sizes for the memory of the perf_base
>> + , lut_base and en_base.
>
> Can you explicitly define each one of them ? Either here or in reg-names.
>
Sure will define each of them in the next series.
>> +- reg-names
>> + Usage: required
>> + Value type: <stringlist>
>> + Definition: Address names. Must be "perf_base", "lut_base",
>> + "en_base".
>> + Must be specified in the same order as the
>> + corresponding addresses are specified in the reg
>> + property.
>> +
>> +- qcom,cpulist
>> + Usage: required
>> + Value type: <phandles of CPU>
>> + Definition: List of related cpu handles which are under a cluster.
>> +
>
> As already mentioned by Rob and Viresh, better to align with OPP style
> to avoid phandle list of CPUs.
>
I have put down an example device tree node for Rob & Viresh to review.
> Also I see similar bindings for devfreq, can't they be aligned ?
> E.g. lut_base here while it's ftbl_base in devfreq.
>
It is actually the look up table, thus was the name given as "lut". Will
check with Saravana for devfreq and align accordingly.
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.
--
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings
From: Viresh Kumar @ 2018-05-24 5:43 UTC (permalink / raw)
To: Taniya Das
Cc: Rob Herring, Rafael J. Wysocki, linux-kernel@vger.kernel.org,
linux-pm, Stephen Boyd, Rajendra Nayak, Amit Nischal, devicetree,
Saravana Kannan, Amit Kucheria
In-Reply-To: <cba1616d-ad12-599b-4351-3c7240de852a@codeaurora.org>
On 24-05-18, 10:48, Taniya Das wrote:
> Hello Rob, Viresh,
>
> Thank you for the comments. If I understand correctly, the device tree nodes
> should look something like the below.
>
> Though I am not sure if any vendor name could be associated in the cpu
> nodes.
Well Rob said Yes to that I think.
> Please suggest if my understanding is wrong.
>
> cpu@0 {
> qcom,freq-domain = <&freq_domain_table0>;
> …
> };
>
> same follows for cpu 1/2/3
>
> cpu@400 {
> qcom,freq-domain = <&freq_domain_table1>;
> …
> };
> same follows for cpu 5/6/7
>
> freq_domain_table0 : freq_table {
> reg = < >, < >, < > ;
> reg-names = "perf_base", "lut_base", "en_base";
> };
>
> freq_domain_table1 : freq_table {
> reg = < >, < >, < > ;
> reg-names = "perf_base", "lut_base", "en_base";
> };
Mostly yes.
--
viresh
^ permalink raw reply
* Re: [PATCH v1 2/2] cpufreq: tegra20: Use PLL_C as intermediate clock source
From: Dmitry Osipenko @ 2018-05-24 5:37 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael J. Wysocki, Thierry Reding, Jonathan Hunter,
Peter De Schrijver, linux-tegra, linux-pm, linux-kernel
In-Reply-To: <20180524043040.25pld3ezs4lpabro@vireshk-i7>
On 24.05.2018 07:30, Viresh Kumar wrote:
> On 23-05-18, 19:00, Dmitry Osipenko wrote:
>> PLL_C is running at 600MHz which is significantly higher than the 216MHz
>> of the PLL_P and it is known that PLL_C is always-ON because AHB BUS is
>> running on that PLL. Let's use PLL_C as intermediate clock source, making
>> CPU snappier a tad during of the frequency transition.
>>
>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>> ---
>> drivers/cpufreq/tegra20-cpufreq.c | 25 +++++++++++++++++++++----
>> 1 file changed, 21 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/cpufreq/tegra20-cpufreq.c b/drivers/cpufreq/tegra20-cpufreq.c
>> index 3ad6bded6efc..4bf5ba7da40b 100644
>> --- a/drivers/cpufreq/tegra20-cpufreq.c
>> +++ b/drivers/cpufreq/tegra20-cpufreq.c
>> @@ -25,12 +25,13 @@
>> #include <linux/types.h>
>>
>> #define PLL_P_FREQ 216000
>> +#define PLL_C_FREQ 600000
>>
>> static struct cpufreq_frequency_table freq_table[] = {
>> { .frequency = 216000 },
>> { .frequency = 312000 },
>> { .frequency = 456000 },
>> - { .frequency = 608000 },
>> + { .frequency = 600000 },
>> { .frequency = 760000 },
>> { .frequency = 816000 },
>> { .frequency = 912000 },
>> @@ -44,6 +45,7 @@ struct tegra20_cpufreq {
>> struct clk *cpu_clk;
>> struct clk *pll_x_clk;
>> struct clk *pll_p_clk;
>> + struct clk *pll_c_clk;
>> bool pll_x_prepared;
>> };
>>
>> @@ -58,7 +60,10 @@ static unsigned int tegra_get_intermediate(struct cpufreq_policy *policy,
>> if (index == 0 || policy->cur == PLL_P_FREQ)
>> return 0;
>>
>> - return PLL_P_FREQ;
>> + if (index == 3 || policy->cur == PLL_C_FREQ)
>> + return 0;
>
> So we can choose between two different intermediate frequencies ? And
> I didn't like the way magic number 3 is used here. Its prone to errors
> and we better use a macro or something else here.
>
> Like instead of doing index == 3, what about freq_table[index].freq ==
> PLL_C_FREQ ? Same for the previous patch as well.
The frequency is determined by the parent clock of CCLK (CPU clock), we can
choose between different parents for the CCLK. PLL_C as PLL_P and PLL_X are
among the available parents for the CCLK to choose from and there some others.
I don't mind to use freq_table[index].freq, though I'd like to keep compiled
assembly minimal where possible. Hence the freq_table should be made constant to
tell compiler that it doesn't need to emit data fetches for the table values and
could embed the constants into the code where appropriate.
Could we constify the "struct cpufreq_frequency_table" within the cpufreq core?
Seems nothing prevents this (I already tried to constify - there are no
obstacles), unless some cpufreq driver would try to modify
policy->freq_table->... within the cpufreq callback implementation.
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: cpufreq: Introduce QCOM CPUFREQ FW bindings
From: Taniya Das @ 2018-05-24 5:18 UTC (permalink / raw)
To: Rob Herring, Viresh Kumar
Cc: Rafael J. Wysocki, linux-kernel@vger.kernel.org, linux-pm,
Stephen Boyd, Rajendra Nayak, Amit Nischal, devicetree,
Saravana Kannan, Amit Kucheria
In-Reply-To: <CAL_JsqL7VdSjerOioc5cSOyKAAXJuVnPJqp_WbTtwr11UUxq=A@mail.gmail.com>
Hello Rob, Viresh,
Thank you for the comments. If I understand correctly, the device tree
nodes should look something like the below.
Though I am not sure if any vendor name could be associated in the cpu
nodes.
Please suggest if my understanding is wrong.
cpu@0 {
qcom,freq-domain = <&freq_domain_table0>;
…
};
same follows for cpu 1/2/3
cpu@400 {
qcom,freq-domain = <&freq_domain_table1>;
…
};
same follows for cpu 5/6/7
freq_domain_table0 : freq_table {
reg = < >, < >, < > ;
reg-names = "perf_base", "lut_base", "en_base";
};
freq_domain_table1 : freq_table {
reg = < >, < >, < > ;
reg-names = "perf_base", "lut_base", "en_base";
};
On 5/23/2018 7:48 PM, Rob Herring wrote:
> On Wed, May 23, 2018 at 12:48 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> On 22-05-18, 14:31, Rob Herring wrote:
>>> On Sat, May 19, 2018 at 11:04:50PM +0530, Taniya Das wrote:
>>>> + freq-domain-0 {
>>>> + compatible = "cpufreq";
>>>> + reg = <0x17d43920 0x4>,
>>>> + <0x17d43110 0x500>,
>>>> + <0x17d41000 0x4>;
>>>> + reg-names = "perf_base", "lut_base", "en_base";
>>>> + qcom,cpulist = <&CPU0 &CPU1 &CPU2 &CPU3>;
>>
>> I was thinking, can't we add platform specific properties in the CPU
>> nodes ? If yes, then we can point the phandle of fw node from the CPUs
>> and this awkward list can go away.
>
> Yes, that's fine. That would be more like OPP binding in that the CPU
> points to the OPP table rather than the OPP pointing to the CPUs.
>
> With that, you can get rid of the child nodes completely. Just make
> the parent reg property N sets of 3 addresses for N domains.
>
> Rob
>
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.
--
^ permalink raw reply
* Re: [PATCH v11 1/2] cpufreq: Add Kryo CPU scaling driver
From: Viresh Kumar @ 2018-05-24 4:37 UTC (permalink / raw)
To: Sudeep Holla
Cc: Ilia Lin, vireshk, nm, sboyd, robh, mark.rutland, rjw, linux-pm,
devicetree, linux-kernel
In-Reply-To: <1ec7645d-72b6-5a1a-48c3-831a3c484a0e@arm.com>
On 23-05-18, 14:25, Sudeep Holla wrote:
> On 23/05/18 13:38, Ilia Lin wrote:
> > +static int __init qcom_cpufreq_kryo_init(void)
> > +{
> > + /*
> > + * Since the driver depends on smem and nvmem drivers, which may
> > + * return EPROBE_DEFER, all the real activity is done in the probe,
> > + * which may be defered as well. The init here is only registering
> > + * a platform device.
> > + */
> > + platform_device_register_simple("qcom-cpufreq-kryo", -1, NULL, 0);
> > + return 0;
> > +}
> > +module_init(qcom_cpufreq_kryo_init);
>
> Do you need this at all ? See below on how to eliminate the need for this.
>
> > +
> > +static struct platform_driver qcom_cpufreq_kryo_driver = {
> > + .probe = qcom_cpufreq_kryo_probe,
> > + .driver = {
> > + .name = "qcom-cpufreq-kryo",
> > + },
> > +};
> > +builtin_platform_driver(qcom_cpufreq_kryo_driver);
>
> Use builtin_platform_driver_probe and remove qcom_cpufreq_kryo_init
> or use module_platform_driver_probe if it can be module.
I agree with this, just use a single init routine to register both the driver
and device.
--
viresh
^ permalink raw reply
* Re: [PATCH v11 1/2] cpufreq: Add Kryo CPU scaling driver
From: Viresh Kumar @ 2018-05-24 4:34 UTC (permalink / raw)
To: Sudeep Holla
Cc: Ilia Lin, vireshk, nm, sboyd, robh, mark.rutland, rjw, linux-pm,
devicetree, linux-kernel
In-Reply-To: <1ec7645d-72b6-5a1a-48c3-831a3c484a0e@arm.com>
On 23-05-18, 14:25, Sudeep Holla wrote:
> On 23/05/18 13:38, Ilia Lin wrote:
> > +config ARM_QCOM_CPUFREQ_KRYO
> > + bool "Qualcomm Kryo based CPUFreq"
> > + depends on QCOM_QFPROM
> > + depends on QCOM_SMEM
> > + select PM_OPP
> > + help
> > + This adds the CPUFreq driver for Qualcomm Kryo SoC based boards.
> > +
> > + If in doubt, say N.
> > +
>
> Sorry but just noticed now, any reason why this can't be module. I can't
> imagine any.
Actually I asked him to do that as cpufreq-dt itself can be compiled
in as module and this driver wasn't doing much and isn't big enough
(size wise) as well.
--
viresh
^ permalink raw reply
* Re: [PATCH v1 2/2] cpufreq: tegra20: Use PLL_C as intermediate clock source
From: Viresh Kumar @ 2018-05-24 4:30 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Rafael J. Wysocki, Thierry Reding, Jonathan Hunter,
Peter De Schrijver, linux-tegra, linux-pm, linux-kernel
In-Reply-To: <20180523160020.15291-2-digetx@gmail.com>
On 23-05-18, 19:00, Dmitry Osipenko wrote:
> PLL_C is running at 600MHz which is significantly higher than the 216MHz
> of the PLL_P and it is known that PLL_C is always-ON because AHB BUS is
> running on that PLL. Let's use PLL_C as intermediate clock source, making
> CPU snappier a tad during of the frequency transition.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> drivers/cpufreq/tegra20-cpufreq.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/tegra20-cpufreq.c b/drivers/cpufreq/tegra20-cpufreq.c
> index 3ad6bded6efc..4bf5ba7da40b 100644
> --- a/drivers/cpufreq/tegra20-cpufreq.c
> +++ b/drivers/cpufreq/tegra20-cpufreq.c
> @@ -25,12 +25,13 @@
> #include <linux/types.h>
>
> #define PLL_P_FREQ 216000
> +#define PLL_C_FREQ 600000
>
> static struct cpufreq_frequency_table freq_table[] = {
> { .frequency = 216000 },
> { .frequency = 312000 },
> { .frequency = 456000 },
> - { .frequency = 608000 },
> + { .frequency = 600000 },
> { .frequency = 760000 },
> { .frequency = 816000 },
> { .frequency = 912000 },
> @@ -44,6 +45,7 @@ struct tegra20_cpufreq {
> struct clk *cpu_clk;
> struct clk *pll_x_clk;
> struct clk *pll_p_clk;
> + struct clk *pll_c_clk;
> bool pll_x_prepared;
> };
>
> @@ -58,7 +60,10 @@ static unsigned int tegra_get_intermediate(struct cpufreq_policy *policy,
> if (index == 0 || policy->cur == PLL_P_FREQ)
> return 0;
>
> - return PLL_P_FREQ;
> + if (index == 3 || policy->cur == PLL_C_FREQ)
> + return 0;
So we can choose between two different intermediate frequencies ? And
I didn't like the way magic number 3 is used here. Its prone to errors
and we better use a macro or something else here.
Like instead of doing index == 3, what about freq_table[index].freq ==
PLL_C_FREQ ? Same for the previous patch as well.
--
viresh
^ permalink raw reply
* Re: [PATCH v1 1/2] cpufreq: tegra20: Constify rate value of the intermediate clk
From: Viresh Kumar @ 2018-05-24 4:27 UTC (permalink / raw)
To: Dmitry Osipenko
Cc: Rafael J. Wysocki, Thierry Reding, Jonathan Hunter,
Peter De Schrijver, linux-tegra, linux-pm, linux-kernel
In-Reply-To: <20180523160020.15291-1-digetx@gmail.com>
On 23-05-18, 19:00, Dmitry Osipenko wrote:
> PLL_P is known to be always running at 216MHz, hence there is no need to
> query its rate.
>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
> drivers/cpufreq/tegra20-cpufreq.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/cpufreq/tegra20-cpufreq.c b/drivers/cpufreq/tegra20-cpufreq.c
> index 05f57dcd5215..3ad6bded6efc 100644
> --- a/drivers/cpufreq/tegra20-cpufreq.c
> +++ b/drivers/cpufreq/tegra20-cpufreq.c
> @@ -24,6 +24,8 @@
> #include <linux/platform_device.h>
> #include <linux/types.h>
>
> +#define PLL_P_FREQ 216000
> +
> static struct cpufreq_frequency_table freq_table[] = {
> { .frequency = 216000 },
> { .frequency = 312000 },
> @@ -48,18 +50,15 @@ struct tegra20_cpufreq {
> static unsigned int tegra_get_intermediate(struct cpufreq_policy *policy,
> unsigned int index)
> {
> - struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
> - unsigned int ifreq = clk_get_rate(cpufreq->pll_p_clk) / 1000;
> -
> /*
> * Don't switch to intermediate freq if:
> * - we are already at it, i.e. policy->cur == ifreq
> * - index corresponds to ifreq
> */
> - if (freq_table[index].frequency == ifreq || policy->cur == ifreq)
> + if (index == 0 || policy->cur == PLL_P_FREQ)
> return 0;
>
> - return ifreq;
> + return PLL_P_FREQ;
> }
>
> static int tegra_target_intermediate(struct cpufreq_policy *policy,
> @@ -93,14 +92,13 @@ static int tegra_target(struct cpufreq_policy *policy, unsigned int index)
> {
> struct tegra20_cpufreq *cpufreq = cpufreq_get_driver_data();
> unsigned long rate = freq_table[index].frequency;
> - unsigned int ifreq = clk_get_rate(cpufreq->pll_p_clk) / 1000;
> int ret;
>
> /*
> * target freq == pll_p, don't need to take extra reference to pll_x_clk
> * as it isn't used anymore.
> */
> - if (rate == ifreq)
> + if (index == 0)
> return clk_set_parent(cpufreq->cpu_clk, cpufreq->pll_p_clk);
>
> ret = clk_set_rate(cpufreq->pll_x_clk, rate * 1000);
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply
* [RFC/RFT] [PATCH v2 6/6] cpufreq: intel_pstate: enable boost for SKX
From: Srinivas Pandruvada @ 2018-05-24 1:47 UTC (permalink / raw)
To: lenb, rjw, peterz, mgorman
Cc: linux-pm, linux-kernel, juri.lelli, viresh.kumar,
Srinivas Pandruvada
In-Reply-To: <20180524014738.52924-1-srinivas.pandruvada@linux.intel.com>
Enable HWP boost on Skylake server platform.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/cpufreq/intel_pstate.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index b2613c9775d5..6f8214a40764 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1801,6 +1801,11 @@ static const struct x86_cpu_id intel_pstate_cpu_ee_disable_ids[] = {
{}
};
+static const struct x86_cpu_id intel_pstate_hwp_boost_ids[] __initconst = {
+ ICPU(INTEL_FAM6_SKYLAKE_X, core_funcs),
+ {}
+};
+
static int intel_pstate_init_cpu(unsigned int cpunum)
{
struct cpudata *cpu;
@@ -1831,6 +1836,10 @@ static int intel_pstate_init_cpu(unsigned int cpunum)
intel_pstate_disable_ee(cpunum);
intel_pstate_hwp_enable(cpu);
+
+ id = x86_match_cpu(intel_pstate_hwp_boost_ids);
+ if (id)
+ hwp_boost = true;
}
intel_pstate_get_cpu_pstates(cpu);
--
2.13.6
^ permalink raw reply related
* [RFC/RFT] [PATCH v2 5/6] cpufreq: intel_pstate: New sysfs entry to control HWP boost
From: Srinivas Pandruvada @ 2018-05-24 1:47 UTC (permalink / raw)
To: lenb, rjw, peterz, mgorman
Cc: linux-pm, linux-kernel, juri.lelli, viresh.kumar,
Srinivas Pandruvada
In-Reply-To: <20180524014738.52924-1-srinivas.pandruvada@linux.intel.com>
A new attribute is added to intel_pstate sysfs to enable/disable
HWP dynamic performance boost.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/cpufreq/intel_pstate.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 6d0ebe4fe1c7..b2613c9775d5 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -1033,6 +1033,30 @@ static ssize_t store_min_perf_pct(struct kobject *a, struct attribute *b,
return count;
}
+static ssize_t show_hwp_dynamic_boost(struct kobject *kobj,
+ struct attribute *attr, char *buf)
+{
+ return sprintf(buf, "%u\n", hwp_boost);
+}
+
+static ssize_t store_hwp_dynamic_boost(struct kobject *a, struct attribute *b,
+ const char *buf, size_t count)
+{
+ unsigned int input;
+ int ret;
+
+ ret = kstrtouint(buf, 10, &input);
+ if (ret)
+ return ret;
+
+ mutex_lock(&intel_pstate_driver_lock);
+ hwp_boost = !!input;
+ intel_pstate_update_policies();
+ mutex_unlock(&intel_pstate_driver_lock);
+
+ return count;
+}
+
show_one(max_perf_pct, max_perf_pct);
show_one(min_perf_pct, min_perf_pct);
@@ -1042,6 +1066,7 @@ define_one_global_rw(max_perf_pct);
define_one_global_rw(min_perf_pct);
define_one_global_ro(turbo_pct);
define_one_global_ro(num_pstates);
+define_one_global_rw(hwp_dynamic_boost);
static struct attribute *intel_pstate_attributes[] = {
&status.attr,
@@ -1082,6 +1107,11 @@ static void __init intel_pstate_sysfs_expose_params(void)
rc = sysfs_create_file(intel_pstate_kobject, &min_perf_pct.attr);
WARN_ON(rc);
+ if (hwp_active) {
+ rc = sysfs_create_file(intel_pstate_kobject,
+ &hwp_dynamic_boost.attr);
+ WARN_ON(rc);
+ }
}
/************************** sysfs end ************************/
--
2.13.6
^ permalink raw reply related
* [RFC/RFT] [PATCH v2 4/6] cpufreq: intel_pstate: HWP boost performance on IO wakeup
From: Srinivas Pandruvada @ 2018-05-24 1:47 UTC (permalink / raw)
To: lenb, rjw, peterz, mgorman
Cc: linux-pm, linux-kernel, juri.lelli, viresh.kumar,
Srinivas Pandruvada
In-Reply-To: <20180524014738.52924-1-srinivas.pandruvada@linux.intel.com>
This change uses SCHED_CPUFREQ_IOWAIT flag to boost HWP performance.
Since SCHED_CPUFREQ_IOWAIT flag is set frequently, we don't start
boosting steps unless we see two consecutive flags in two ticks. This
avoids boosting due to IO because of regular system activities.
To avoid synchronization issues, the actual processing of the flag is
done on the local CPU callback.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/cpufreq/intel_pstate.c | 44 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 382160570b5f..6d0ebe4fe1c7 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -223,6 +223,8 @@ struct global_params {
* operation
* @hwp_req_cached: Cached value of the last HWP Request MSR
* @hwp_cap_cached: Cached value of the last HWP Capabilities MSR
+ * @last_io_update: Last time when IO wake flag was set
+ * @sched_flags: Store scheduler flags for possible cross CPU update
* @hwp_boost_min: Last HWP boosted min performance
*
* This structure stores per CPU instance data for all CPUs.
@@ -258,6 +260,8 @@ struct cpudata {
s16 epp_saved;
u64 hwp_req_cached;
u64 hwp_cap_cached;
+ u64 last_io_update;
+ unsigned long sched_flags;
int hwp_boost_min;
};
@@ -1462,9 +1466,49 @@ static inline bool intel_pstate_hwp_boost_down(struct cpudata *cpu)
return false;
}
+static inline void intel_pstate_update_util_hwp_local(struct cpudata *cpu,
+ u64 time)
+{
+ bool io_flag;
+
+ cpu->sample.time = time;
+ io_flag = test_and_clear_bit(SCHED_CPUFREQ_IOWAIT, &cpu->sched_flags);
+ if (io_flag) {
+ bool do_io = false;
+
+ /*
+ * Set iowait_boost flag and update time. Since IO WAIT flag
+ * is set all the time, we can't just conclude that there is
+ * some IO bound activity is scheduled on this CPU with just
+ * one occurrence. If we receive at least two in two
+ * consecutive ticks, then we treat as boost candidate.
+ */
+ if (time_before64(time, cpu->last_io_update + 2 * TICK_NSEC))
+ do_io = true;
+
+ cpu->last_io_update = time;
+
+ if (do_io)
+ intel_pstate_hwp_boost_up(cpu);
+
+ } else {
+ if (intel_pstate_hwp_boost_down(cpu))
+ return;
+ }
+
+ cpu->last_update = time;
+}
+
static inline void intel_pstate_update_util_hwp(struct update_util_data *data,
u64 time, unsigned int flags)
{
+ struct cpudata *cpu = container_of(data, struct cpudata, update_util);
+
+ if (flags & SCHED_CPUFREQ_IOWAIT)
+ set_bit(SCHED_CPUFREQ_IOWAIT, &cpu->sched_flags);
+
+ if (smp_processor_id() == cpu->cpu)
+ intel_pstate_update_util_hwp_local(cpu, time);
}
static inline void intel_pstate_calc_avg_perf(struct cpudata *cpu)
--
2.13.6
^ permalink raw reply related
* [RFC/RFT] [PATCH v2 3/6] cpufreq: intel_pstate: Add update_util_hook for HWP
From: Srinivas Pandruvada @ 2018-05-24 1:47 UTC (permalink / raw)
To: lenb, rjw, peterz, mgorman
Cc: linux-pm, linux-kernel, juri.lelli, viresh.kumar,
Srinivas Pandruvada
In-Reply-To: <20180524014738.52924-1-srinivas.pandruvada@linux.intel.com>
When HWP dynamic boost is active then set the HWP specific update util
hook.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/cpufreq/intel_pstate.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 6ad46e07cad6..382160570b5f 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -291,6 +291,7 @@ static struct pstate_funcs pstate_funcs __read_mostly;
static int hwp_active __read_mostly;
static bool per_cpu_limits __read_mostly;
+static bool hwp_boost __read_mostly;
static struct cpufreq_driver *intel_pstate_driver __read_mostly;
@@ -1461,6 +1462,11 @@ static inline bool intel_pstate_hwp_boost_down(struct cpudata *cpu)
return false;
}
+static inline void intel_pstate_update_util_hwp(struct update_util_data *data,
+ u64 time, unsigned int flags)
+{
+}
+
static inline void intel_pstate_calc_avg_perf(struct cpudata *cpu)
{
struct sample *sample = &cpu->sample;
@@ -1764,7 +1770,7 @@ static void intel_pstate_set_update_util_hook(unsigned int cpu_num)
{
struct cpudata *cpu = all_cpu_data[cpu_num];
- if (hwp_active)
+ if (hwp_active && !hwp_boost)
return;
if (cpu->update_util_set)
@@ -1772,8 +1778,12 @@ static void intel_pstate_set_update_util_hook(unsigned int cpu_num)
/* Prevent intel_pstate_update_util() from using stale data. */
cpu->sample.time = 0;
- cpufreq_add_update_util_hook(cpu_num, &cpu->update_util,
- intel_pstate_update_util);
+ if (hwp_active)
+ cpufreq_add_update_util_hook(cpu_num, &cpu->update_util,
+ intel_pstate_update_util_hwp);
+ else
+ cpufreq_add_update_util_hook(cpu_num, &cpu->update_util,
+ intel_pstate_update_util);
cpu->update_util_set = true;
}
@@ -1885,8 +1895,11 @@ static int intel_pstate_set_policy(struct cpufreq_policy *policy)
intel_pstate_set_update_util_hook(policy->cpu);
}
- if (hwp_active)
+ if (hwp_active) {
+ if (!hwp_boost)
+ intel_pstate_clear_update_util_hook(policy->cpu);
intel_pstate_hwp_set(policy->cpu);
+ }
mutex_unlock(&intel_pstate_limits_lock);
--
2.13.6
^ permalink raw reply related
* [RFC/RFT] [PATCH v2 2/6] cpufreq: intel_pstate: Add HWP boost utility functions
From: Srinivas Pandruvada @ 2018-05-24 1:47 UTC (permalink / raw)
To: lenb, rjw, peterz, mgorman
Cc: linux-pm, linux-kernel, juri.lelli, viresh.kumar,
Srinivas Pandruvada
In-Reply-To: <20180524014738.52924-1-srinivas.pandruvada@linux.intel.com>
Added two utility functions to HWP boost up gradually and boost down to
the default cached HWP request values.
Boost up:
Boost up updates HWP request minimum value in steps. This minimum value
can reach upto at HWP request maximum values depends on how frequently,
the IOWAIT flag is set. At max, boost up will take three steps to reach
the maximum, depending on the current HWP request levels and HWP
capabilities. For example, if the current settings are:
If P0 (Turbo max) = P1 (Guaranteed max) = min
No boost at all.
If P0 (Turbo max) > P1 (Guaranteed max) = min
Should result in one level boost only for P0.
If P0 (Turbo max) = P1 (Guaranteed max) > min
Should result in two level boost:
(min + p1)/2 and P1.
If P0 (Turbo max) > P1 (Guaranteed max) > min
Should result in three level boost:
(min + p1)/2, P1 and P0.
We don't set any level between P0 and P1 as there is no guarantee that
they will be honored.
Boost down:
After the system is idle for hold time of 3ms, the HWP request is reset
to the default cached value from HWP init or user modified one via sysfs.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/cpufreq/intel_pstate.c | 74 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index baed29c768e7..6ad46e07cad6 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -223,6 +223,7 @@ struct global_params {
* operation
* @hwp_req_cached: Cached value of the last HWP Request MSR
* @hwp_cap_cached: Cached value of the last HWP Capabilities MSR
+ * @hwp_boost_min: Last HWP boosted min performance
*
* This structure stores per CPU instance data for all CPUs.
*/
@@ -257,6 +258,7 @@ struct cpudata {
s16 epp_saved;
u64 hwp_req_cached;
u64 hwp_cap_cached;
+ int hwp_boost_min;
};
static struct cpudata **all_cpu_data;
@@ -1387,6 +1389,78 @@ static void intel_pstate_get_cpu_pstates(struct cpudata *cpu)
intel_pstate_set_min_pstate(cpu);
}
+/*
+ * Long hold time will keep high perf limits for long time,
+ * which negatively impacts perf/watt for some workloads,
+ * like specpower. 3ms is based on experiements on some
+ * workoads.
+ */
+static int hwp_boost_hold_time_ms = 3;
+
+static inline void intel_pstate_hwp_boost_up(struct cpudata *cpu)
+{
+ u64 hwp_req = READ_ONCE(cpu->hwp_req_cached);
+ int max_limit = (hwp_req & 0xff00) >> 8;
+ int min_limit = (hwp_req & 0xff);
+ int boost_level1;
+
+ /*
+ * Cases to consider (User changes via sysfs or boot time):
+ * If, P0 (Turbo max) = P1 (Guaranteed max) = min:
+ * No boost, return.
+ * If, P0 (Turbo max) > P1 (Guaranteed max) = min:
+ * Should result in one level boost only for P0.
+ * If, P0 (Turbo max) = P1 (Guaranteed max) > min:
+ * Should result in two level boost:
+ * (min + p1)/2 and P1.
+ * If, P0 (Turbo max) > P1 (Guaranteed max) > min:
+ * Should result in three level boost:
+ * (min + p1)/2, P1 and P0.
+ */
+
+ /* If max and min are equal or already at max, nothing to boost */
+ if (max_limit == min_limit || cpu->hwp_boost_min >= max_limit)
+ return;
+
+ if (!cpu->hwp_boost_min)
+ cpu->hwp_boost_min = min_limit;
+
+ /* level at half way mark between min and guranteed */
+ boost_level1 = (HWP_GUARANTEED_PERF(cpu->hwp_cap_cached) + min_limit) >> 1;
+
+ if (cpu->hwp_boost_min < boost_level1)
+ cpu->hwp_boost_min = boost_level1;
+ else if (cpu->hwp_boost_min < HWP_GUARANTEED_PERF(cpu->hwp_cap_cached))
+ cpu->hwp_boost_min = HWP_GUARANTEED_PERF(cpu->hwp_cap_cached);
+ else if (cpu->hwp_boost_min == HWP_GUARANTEED_PERF(cpu->hwp_cap_cached) &&
+ max_limit != HWP_GUARANTEED_PERF(cpu->hwp_cap_cached))
+ cpu->hwp_boost_min = max_limit;
+ else
+ return;
+
+ hwp_req = (hwp_req & ~GENMASK_ULL(7, 0)) | cpu->hwp_boost_min;
+ wrmsrl(MSR_HWP_REQUEST, hwp_req);
+ cpu->last_update = cpu->sample.time;
+}
+
+static inline bool intel_pstate_hwp_boost_down(struct cpudata *cpu)
+{
+ if (cpu->hwp_boost_min) {
+ bool expired;
+
+ /* Check if we are idle for hold time to boost down */
+ expired = time_after64(cpu->sample.time, cpu->last_update +
+ (hwp_boost_hold_time_ms * NSEC_PER_MSEC));
+ if (expired) {
+ wrmsrl(MSR_HWP_REQUEST, cpu->hwp_req_cached);
+ cpu->hwp_boost_min = 0;
+ return true;
+ }
+ }
+
+ return false;
+}
+
static inline void intel_pstate_calc_avg_perf(struct cpudata *cpu)
{
struct sample *sample = &cpu->sample;
--
2.13.6
^ permalink raw reply related
* [RFC/RFT] [PATCH v2 1/6] cpufreq: intel_pstate: Cache last HWP capability/request value
From: Srinivas Pandruvada @ 2018-05-24 1:47 UTC (permalink / raw)
To: lenb, rjw, peterz, mgorman
Cc: linux-pm, linux-kernel, juri.lelli, viresh.kumar,
Srinivas Pandruvada
In-Reply-To: <20180524014738.52924-1-srinivas.pandruvada@linux.intel.com>
Store the HWP request value last set using MSR_HWP_REQUEST. This will
allow us to save cycles used for reading last HWP request value for
dynamic update of MSR_HWP_REQUEST.
Also store HWP capability MSR value in local memory, to avoid reading
MSR later for calculating limits for dynamic update of MSR_HWP_REQUEST.
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
drivers/cpufreq/intel_pstate.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 17e566afbb41..baed29c768e7 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -221,6 +221,8 @@ struct global_params {
* preference/bias
* @epp_saved: Saved EPP/EPB during system suspend or CPU offline
* operation
+ * @hwp_req_cached: Cached value of the last HWP Request MSR
+ * @hwp_cap_cached: Cached value of the last HWP Capabilities MSR
*
* This structure stores per CPU instance data for all CPUs.
*/
@@ -253,6 +255,8 @@ struct cpudata {
s16 epp_policy;
s16 epp_default;
s16 epp_saved;
+ u64 hwp_req_cached;
+ u64 hwp_cap_cached;
};
static struct cpudata **all_cpu_data;
@@ -689,6 +693,7 @@ static void intel_pstate_get_hwp_max(unsigned int cpu, int *phy_max,
u64 cap;
rdmsrl_on_cpu(cpu, MSR_HWP_CAPABILITIES, &cap);
+ WRITE_ONCE(all_cpu_data[cpu]->hwp_cap_cached, cap);
if (global.no_turbo)
*current_max = HWP_GUARANTEED_PERF(cap);
else
@@ -763,6 +768,7 @@ static void intel_pstate_hwp_set(unsigned int cpu)
intel_pstate_set_epb(cpu, epp);
}
skip_epp:
+ WRITE_ONCE(cpu_data->hwp_req_cached, value);
wrmsrl_on_cpu(cpu, MSR_HWP_REQUEST, value);
}
--
2.13.6
^ permalink raw reply related
* [RFC/RFT] [PATCH v2 0/6] Intel_pstate: HWP Dynamic performance boost
From: Srinivas Pandruvada @ 2018-05-24 1:47 UTC (permalink / raw)
To: lenb, rjw, peterz, mgorman
Cc: linux-pm, linux-kernel, juri.lelli, viresh.kumar,
Srinivas Pandruvada
v2
This is a much simpler version than the previous one and only consider IO
boost, using the existing mechanism. There is no change in this series
beyond intel_pstate driver.
Once PeterZ finishes his work on frequency invariant, I will revisit
thread migration optimization in HWP mode.
Other changes:
- Gradual boost instead of single step as suggested by PeterZ.
- Cross CPU synchronization concerns identified by Rafael.
- Split the patch for HWP MSR value caching as suggested by PeterZ.
Not changed as suggested:
There is no architecture way to identify platform with Per-core
P-states, so still have to enable feature based on CPU model.
-----------
v1
This series tries to address some concern in performance particularly with IO
workloads (Reported by Mel Gorman), when HWP is using intel_pstate powersave
policy.
Background
HWP performance can be controlled by user space using sysfs interface for
max/min frequency limits and energy performance preference settings. Based on
workload characteristics these can be adjusted from user space. These limits
are not changed dynamically by kernel based on workload.
By default HWP defaults to energy performance preference value of 0x80 on
majority of platforms(Scale is 0-255, 0 is max performance and 255 is min).
This value offers best performance/watt and for majority of server workloads
performance doesn't suffer. Also users always have option to use performance
policy of intel_pstate, to get best performance. But user tend to run with
out of box configuration, which is powersave policy on most of the distros.
In some case it is possible to dynamically adjust performance, for example,
when a CPU is woken up due to IO completion or thread migrate to a new CPU. In
this case HWP algorithm will take some time to build utilization and ramp up
P-states. So this may results in lower performance for some IO workloads and
workloads which tend to migrate. The idea of this patch series is to
temporarily boost performance dynamically in these cases. This is only
applicable only when user is using powersave policy, not in performance policy.
Results on a Skylake server:
Benchmark Improvement %
----------------------------------------------------------------------
dbench 50.36
thread IO bench (tiobench) 10.35
File IO 9.81
sqlite 15.76
X264 -104 cores 9.75
Spec Power (Negligible impact 7382 Vs. 7378)
Idle Power No change observed
-----------------------------------------------------------------------
HWP brings in best performace/watt at EPP=0x80. Since we are boosting
EPP here to 0, the performance/watt drops upto 10%. So there is a power
penalty of these changes.
Also Mel Gorman provided test results on a prior patchset, which shows
benifits of this series.
Srinivas Pandruvada (6):
cpufreq: intel_pstate: Cache last HWP capability/request value
cpufreq: intel_pstate: Add HWP boost utility functions
cpufreq: intel_pstate: Add update_util_hook for HWP
cpufreq: intel_pstate: HWP boost performance on IO wakeup
cpufreq: intel_pstate: New sysfs entry to control HWP boost
cpufreq: intel_pstate: enable boost for SKX
drivers/cpufreq/intel_pstate.c | 183 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 179 insertions(+), 4 deletions(-)
--
2.13.6
^ permalink raw reply
* Re: [PATCH] cpufreq: schedutil: Avoid missing updates for one-CPU policies
From: Joel Fernandes @ 2018-05-24 0:56 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM, LKML, Peter Zijlstra, Viresh Kumar, Juri Lelli,
Patrick Bellasi, claudio, Todd Kjos
In-Reply-To: <1672734.JYOlA1IWnU@aspire.rjw.lan>
On Wed, May 23, 2018 at 11:47:45AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Commit 152db033d775 (schedutil: Allow cpufreq requests to be made
> even when kthread kicked) made changes to prevent utilization updates
> from being discarded during processing a previous request, but it
> left a small window in which that still can happen in the one-CPU
> policy case. Namely, updates coming in after setting work_in_progress
> in sugov_update_commit() and clearing it in sugov_work() will still
> be dropped due to the work_in_progress check in sugov_update_single().
>
> To close that window, rearrange the code so as to acquire the update
> lock around the deferred update branch in sugov_update_single()
> and drop the work_in_progress check from it.
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>
thanks,
- Joel
^ permalink raw reply
* Re: [PATCH v3 23/27] x86/modules: Adapt module loading for PIE support
From: Randy Dunlap @ 2018-05-23 23:07 UTC (permalink / raw)
To: Thomas Garnier
Cc: Herbert Xu, David S . Miller, Thomas Gleixner, Ingo Molnar,
H . Peter Anvin, Peter Zijlstra, Josh Poimboeuf, Greg KH,
Philippe Ombredanne, Kate Stewart, Arnaldo Carvalho de Melo,
Yonghong Song, Andrey Ryabinin, Kees Cook, Tom Lendacky,
Kirill A. Shutemov, Andy Lutomirski, Dominik Brodowski,
Borislav Petkov, Borislav Petkov, Rafael J. Wysocki, Len Brown,
Pavel Machek
In-Reply-To: <CAJcbSZHPc2w7g8Q4UZV3sx=s6LVL_+dsomro7to6RNhd-4F4Ag@mail.gmail.com>
On 05/23/2018 03:01 PM, Thomas Garnier wrote:
> On Wed, May 23, 2018 at 2:27 PM Randy Dunlap <rdunlap@infradead.org> wrote:
>
>> Hi,
>
>> (for several patches in this series:)
>> The commit message is confusing. See below.
>
> Thanks for the edits, I will change the different commit messages.
>
>
>
>> On 05/23/2018 12:54 PM, Thomas Garnier wrote:
>>> Adapt module loading to support PIE relocations. Generate dynamic GOT if
>>> a symbol requires it but no entry exist in the kernel GOT.
>
>> exists
>
>>>
>>> Position Independent Executable (PIE) support will allow to extended the
>
>> will allow us to extend
> the
>
>>> KASLR randomization range below the -2G memory limit.
>
>> Does that say "below th negative 2G memory limit"?
>> I don't get it.
>
> Yes, below 0xffffffff80000000 basically. I think I will just say that.
Yes, please, that's much better.
>
>
>>>
>>> Signed-off-by: Thomas Garnier <thgarnie@google.com>
>>> ---
>>> arch/x86/Makefile | 4 +
>>> arch/x86/include/asm/module.h | 11 ++
>>> arch/x86/include/asm/sections.h | 4 +
>>> arch/x86/kernel/module.c | 181 +++++++++++++++++++++++++++++++-
>>> arch/x86/kernel/module.lds | 3 +
>>> 5 files changed, 198 insertions(+), 5 deletions(-)
>>> create mode 100644 arch/x86/kernel/module.lds
>
>
>> Thanks,
>> --
>> ~Randy
>
>
>
--
~Randy
^ permalink raw reply
* Re: [PATCH v3 23/27] x86/modules: Adapt module loading for PIE support
From: Thomas Garnier @ 2018-05-23 22:01 UTC (permalink / raw)
To: Randy Dunlap
Cc: Herbert Xu, David S . Miller, Thomas Gleixner, Ingo Molnar,
H . Peter Anvin, Peter Zijlstra, Josh Poimboeuf, Greg KH,
Philippe Ombredanne, Kate Stewart, Arnaldo Carvalho de Melo,
Yonghong Song, Andrey Ryabinin, Kees Cook, Tom Lendacky,
Kirill A. Shutemov, Andy Lutomirski, Dominik Brodowski,
Borislav Petkov, Borislav Petkov, Rafael J. Wysocki, Len Brown,
Pavel Machek <pavel@
In-Reply-To: <168ebedb-7c27-d1f3-c2f9-223d44186a52@infradead.org>
On Wed, May 23, 2018 at 2:27 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> Hi,
> (for several patches in this series:)
> The commit message is confusing. See below.
Thanks for the edits, I will change the different commit messages.
> On 05/23/2018 12:54 PM, Thomas Garnier wrote:
> > Adapt module loading to support PIE relocations. Generate dynamic GOT if
> > a symbol requires it but no entry exist in the kernel GOT.
> exists
> >
> > Position Independent Executable (PIE) support will allow to extended the
> will allow us to extend
the
> > KASLR randomization range below the -2G memory limit.
> Does that say "below th negative 2G memory limit"?
> I don't get it.
Yes, below 0xffffffff80000000 basically. I think I will just say that.
> >
> > Signed-off-by: Thomas Garnier <thgarnie@google.com>
> > ---
> > arch/x86/Makefile | 4 +
> > arch/x86/include/asm/module.h | 11 ++
> > arch/x86/include/asm/sections.h | 4 +
> > arch/x86/kernel/module.c | 181 +++++++++++++++++++++++++++++++-
> > arch/x86/kernel/module.lds | 3 +
> > 5 files changed, 198 insertions(+), 5 deletions(-)
> > create mode 100644 arch/x86/kernel/module.lds
> Thanks,
> --
> ~Randy
--
Thomas
^ permalink raw reply
* Re: [PATCH v3 23/27] x86/modules: Adapt module loading for PIE support
From: Randy Dunlap @ 2018-05-23 21:26 UTC (permalink / raw)
To: Thomas Garnier, Herbert Xu, David S . Miller, Thomas Gleixner,
Ingo Molnar, H . Peter Anvin, Peter Zijlstra, Josh Poimboeuf,
Greg Kroah-Hartman, Philippe Ombredanne, Kate Stewart,
Arnaldo Carvalho de Melo, Yonghong Song, Andrey Ryabinin,
Kees Cook, Tom Lendacky, Kirill A . Shutemov, Andy Lutomirski,
Dominik Brodowski, Borislav Petkov, Borislav Petkov,
Rafael J . Wysocki
Cc: linux-arch, kvm, linux-pm, x86, linux-doc, linux-kernel,
virtualization, linux-sparse, linux-crypto, kernel-hardening,
xen-devel
In-Reply-To: <20180523195421.180248-24-thgarnie@google.com>
Hi,
(for several patches in this series:)
The commit message is confusing. See below.
On 05/23/2018 12:54 PM, Thomas Garnier wrote:
> Adapt module loading to support PIE relocations. Generate dynamic GOT if
> a symbol requires it but no entry exist in the kernel GOT.
exists
>
> Position Independent Executable (PIE) support will allow to extended the
will allow us to extend the
> KASLR randomization range below the -2G memory limit.
Does that say "below th negative 2G memory limit"?
I don't get it.
>
> Signed-off-by: Thomas Garnier <thgarnie@google.com>
> ---
> arch/x86/Makefile | 4 +
> arch/x86/include/asm/module.h | 11 ++
> arch/x86/include/asm/sections.h | 4 +
> arch/x86/kernel/module.c | 181 +++++++++++++++++++++++++++++++-
> arch/x86/kernel/module.lds | 3 +
> 5 files changed, 198 insertions(+), 5 deletions(-)
> create mode 100644 arch/x86/kernel/module.lds
Thanks,
--
~Randy
^ permalink raw reply
* Re: [PATCH v3 16/27] compiler: Option to add PROVIDE_HIDDEN replacement for weak symbols
From: Randy Dunlap @ 2018-05-23 21:16 UTC (permalink / raw)
To: Thomas Garnier, Herbert Xu, David S . Miller, Thomas Gleixner,
Ingo Molnar, H . Peter Anvin, Peter Zijlstra, Josh Poimboeuf,
Greg Kroah-Hartman, Philippe Ombredanne, Kate Stewart,
Arnaldo Carvalho de Melo, Yonghong Song, Andrey Ryabinin,
Kees Cook, Tom Lendacky, Kirill A . Shutemov, Andy Lutomirski,
Dominik Brodowski, Borislav Petkov, Borislav Petkov,
Rafael J . Wysocki
Cc: x86, linux-crypto, linux-kernel, linux-pm, virtualization,
xen-devel, linux-arch, linux-sparse, kvm, linux-doc,
kernel-hardening
In-Reply-To: <20180523195421.180248-17-thgarnie@google.com>
On 05/23/2018 12:54 PM, Thomas Garnier wrote:
> Provide an option to have a PROVIDE_HIDDEN (linker script) entry for
> each weak symbol. This option solve an error in x86_64 where the linker
solves
> optimizes pie generate code to be non-pie because --emit-relocs was used
generated
> instead of -pie (to reduce dynamic relocations).
>
> Signed-off-by: Thomas Garnier <thgarnie@google.com>
> ---
> init/Kconfig | 7 +++++++
> scripts/link-vmlinux.sh | 14 ++++++++++++++
> 2 files changed, 21 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 0fc3a58d9f2f..2866cca86b4a 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -1954,6 +1954,13 @@ config ASN1
> inform it as to what tags are to be expected in a stream and what
> functions to call on what tags.
>
> +config WEAK_PROVIDE_HIDDEN
> + bool
> + help
> + Generate linker script PROVIDE_HIDDEN entries for all weak symbols. It
> + allows to prevent non-pie code being replaced by the linker if the
non-PIE
> + emit-relocs option is used instead of pie (useful for x86_64 pie).
PIE PIE).
> +
--
~Randy
^ permalink raw reply
* Re: [PATCH v3 13/13] soc: rockchip: power-domain: add power domain support for px30
From: Heiko Stübner @ 2018-05-23 20:10 UTC (permalink / raw)
To: Elaine Zhang
Cc: robh+dt, mark.rutland, devicetree, rjw, khilman, ulf.hansson,
linux-pm, linux-arm-kernel, linux-rockchip, linux-kernel, wxt,
xxx, xf, huangtao, Finley Xiao
In-Reply-To: <1527058412-10754-1-git-send-email-zhangqing@rock-chips.com>
Am Mittwoch, 23. Mai 2018, 08:53:32 CEST schrieb Elaine Zhang:
> From: Finley Xiao <finley.xiao@rock-chips.com>
>
> This driver is modified to support PX30 SoC.
>
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
applied for 4.18 (or later)
Thanks
Heiko
^ permalink raw reply
* Re: [PATCH v3 12/13] dt-bindings: add binding for px30 power domains
From: Heiko Stübner @ 2018-05-23 20:10 UTC (permalink / raw)
To: Elaine Zhang
Cc: robh+dt, mark.rutland, devicetree, rjw, khilman, ulf.hansson,
linux-pm, linux-arm-kernel, linux-rockchip, linux-kernel, wxt,
xxx, xf, huangtao, Finley Xiao
In-Reply-To: <1527058371-10706-1-git-send-email-zhangqing@rock-chips.com>
Am Mittwoch, 23. Mai 2018, 08:52:51 CEST schrieb Elaine Zhang:
> From: Finley Xiao <finley.xiao@rock-chips.com>
>
> Add binding documentation for the power domains
> found on Rockchip PX30 SoCs.
>
> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
applied for 4.18 (or later)
Thanks
Heiko
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox