2018-07-02 14:25:36

by Akshay Adiga

[permalink] [raw]
Subject: [PATCH v2 0/2] powernv/cpuidle Device-tree parsing cleanup

Device-tree parsed multiple time in powernv cpuidle and powernv
hotplug code.

First to identify supported flags. Second time, to identify deepest_state
and first deep state. Third time, during cpuidle init to find the available
idle states. Any change in device-tree format will lead to make changes in
these 3 places. Errors in device-tree can be handled in a better manner.

This series adds code to parse device tree once and save in global structure.

Changes from v1 :
- folded first 2 patches into 1
- rename pm_ctrl_reg_* as psscr_*
- added comment stating removal of pmicr parsing code
- removed parsing code for pmicr
- add member valid in pnv_idle_states_t to indicate if the psscr-mask/val
are valid combination,
- Change function description of pnv_parse_cpuidle_dt
- Added error handling code.

Akshay Adiga (2):
powernv/cpuidle: Parse dt idle properties into global structure
powernv/cpuidle: Use parsed device tree values for cpuidle_init

arch/powerpc/include/asm/cpuidle.h | 13 ++
arch/powerpc/platforms/powernv/idle.c | 216 ++++++++++++++++----------
drivers/cpuidle/cpuidle-powernv.c | 156 ++++---------------
3 files changed, 178 insertions(+), 207 deletions(-)

--
2.18.0.rc2.85.g1fb9df7



2018-07-02 14:25:47

by Akshay Adiga

[permalink] [raw]
Subject: [PATCH v2 1/2] powernv/cpuidle: Parse dt idle properties into global structure

Device-tree parsing happens twice, once while deciding idle state to be
used for hotplug and once during cpuidle init. Hence, parsing the device
tree and caching it will reduce code duplication. Parsing code has been
moved to pnv_parse_cpuidle_dt() from pnv_probe_idle_states(). In addition
to the properties in the device tree the number of available states is
also required.

Signed-off-by: Akshay Adiga <[email protected]>
---
arch/powerpc/include/asm/cpuidle.h | 11 ++
arch/powerpc/platforms/powernv/idle.c | 216 ++++++++++++++++----------
drivers/cpuidle/cpuidle-powernv.c | 11 +-
3 files changed, 151 insertions(+), 87 deletions(-)

diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
index e210a83eb196..574b0ce1d671 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -79,6 +79,17 @@ struct stop_sprs {
u64 mmcra;
};

+#define PNV_IDLE_NAME_LEN 16
+struct pnv_idle_states_t {
+ char name[PNV_IDLE_NAME_LEN];
+ u32 latency_ns;
+ u32 residency_ns;
+ u64 psscr_val;
+ u64 psscr_mask;
+ u32 flags;
+ bool valid;
+};
+
extern u32 pnv_fastsleep_workaround_at_entry[];
extern u32 pnv_fastsleep_workaround_at_exit[];

diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
index 1c5d0675b43c..7cf71b3e03a1 100644
--- a/arch/powerpc/platforms/powernv/idle.c
+++ b/arch/powerpc/platforms/powernv/idle.c
@@ -36,6 +36,8 @@
#define P9_STOP_SPR_PSSCR 855

static u32 supported_cpuidle_states;
+struct pnv_idle_states_t *pnv_idle_states;
+int nr_pnv_idle_states;

/*
* The default stop state that will be used by ppc_md.power_save
@@ -622,48 +624,10 @@ int validate_psscr_val_mask(u64 *psscr_val, u64 *psscr_mask, u32 flags)
* @dt_idle_states: Number of idle state entries
* Returns 0 on success
*/
-static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
- int dt_idle_states)
+static int __init pnv_power9_idle_init(void)
{
- u64 *psscr_val = NULL;
- u64 *psscr_mask = NULL;
- u32 *residency_ns = NULL;
u64 max_residency_ns = 0;
- int rc = 0, i;
-
- psscr_val = kcalloc(dt_idle_states, sizeof(*psscr_val), GFP_KERNEL);
- psscr_mask = kcalloc(dt_idle_states, sizeof(*psscr_mask), GFP_KERNEL);
- residency_ns = kcalloc(dt_idle_states, sizeof(*residency_ns),
- GFP_KERNEL);
-
- if (!psscr_val || !psscr_mask || !residency_ns) {
- rc = -1;
- goto out;
- }
-
- if (of_property_read_u64_array(np,
- "ibm,cpu-idle-state-psscr",
- psscr_val, dt_idle_states)) {
- pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr in DT\n");
- rc = -1;
- goto out;
- }
-
- if (of_property_read_u64_array(np,
- "ibm,cpu-idle-state-psscr-mask",
- psscr_mask, dt_idle_states)) {
- pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr-mask in DT\n");
- rc = -1;
- goto out;
- }
-
- if (of_property_read_u32_array(np,
- "ibm,cpu-idle-state-residency-ns",
- residency_ns, dt_idle_states)) {
- pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-residency-ns in DT\n");
- rc = -1;
- goto out;
- }
+ int i;

/*
* Set pnv_first_deep_stop_state, pnv_deepest_stop_psscr_{val,mask},
@@ -679,33 +643,36 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
* the shallowest (OPAL_PM_STOP_INST_FAST) loss-less stop state.
*/
pnv_first_deep_stop_state = MAX_STOP_STATE;
- for (i = 0; i < dt_idle_states; i++) {
+ for (i = 0; i < nr_pnv_idle_states; i++) {
int err;
- u64 psscr_rl = psscr_val[i] & PSSCR_RL_MASK;
+ struct pnv_idle_states_t *state = &pnv_idle_states[i];
+ u64 psscr_rl = state->psscr_val & PSSCR_RL_MASK;

- if ((flags[i] & OPAL_PM_LOSE_FULL_CONTEXT) &&
- (pnv_first_deep_stop_state > psscr_rl))
+ if ((state->flags & OPAL_PM_LOSE_FULL_CONTEXT) &&
+ pnv_first_deep_stop_state > psscr_rl)
pnv_first_deep_stop_state = psscr_rl;

- err = validate_psscr_val_mask(&psscr_val[i], &psscr_mask[i],
- flags[i]);
+ err = validate_psscr_val_mask(&state->psscr_val,
+ &state->psscr_mask,
+ state->flags);
if (err) {
- report_invalid_psscr_val(psscr_val[i], err);
+ state->valid = false;
+ report_invalid_psscr_val(state->psscr_val, err);
continue;
}

- if (max_residency_ns < residency_ns[i]) {
- max_residency_ns = residency_ns[i];
- pnv_deepest_stop_psscr_val = psscr_val[i];
- pnv_deepest_stop_psscr_mask = psscr_mask[i];
- pnv_deepest_stop_flag = flags[i];
+ if (max_residency_ns < state->residency_ns) {
+ max_residency_ns = state->residency_ns;
+ pnv_deepest_stop_psscr_val = state->psscr_val;
+ pnv_deepest_stop_psscr_mask = state->psscr_mask;
+ pnv_deepest_stop_flag = state->flags;
deepest_stop_found = true;
}

if (!default_stop_found &&
- (flags[i] & OPAL_PM_STOP_INST_FAST)) {
- pnv_default_stop_val = psscr_val[i];
- pnv_default_stop_mask = psscr_mask[i];
+ (state->flags & OPAL_PM_STOP_INST_FAST)) {
+ pnv_default_stop_val = state->psscr_val;
+ pnv_default_stop_mask = state->psscr_mask;
default_stop_found = true;
}
}
@@ -728,11 +695,8 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,

pr_info("cpuidle-powernv: Requested Level (RL) value of first deep stop = 0x%llx\n",
pnv_first_deep_stop_state);
-out:
- kfree(psscr_val);
- kfree(psscr_mask);
- kfree(residency_ns);
- return rc;
+
+ return 0;
}

/*
@@ -740,50 +704,146 @@ static int __init pnv_power9_idle_init(struct device_node *np, u32 *flags,
*/
static void __init pnv_probe_idle_states(void)
{
- struct device_node *np;
- int dt_idle_states;
- u32 *flags = NULL;
int i;

+ if (nr_pnv_idle_states < 0) {
+ pr_warn("cpuidle-powernv: no idle states found in the DT\n");
+ return;
+ }
+
+ if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+ if (pnv_power9_idle_init())
+ return;
+ }
+
+ for (i = 0; i < nr_pnv_idle_states; i++)
+ supported_cpuidle_states |= pnv_idle_states[i].flags;
+}
+
+/*
+ * This function parses device-tree and populates all the information
+ * into pnv_idle_states structure. It also sets up nr_pnv_idle_states
+ * which is the number of cpuidle states discovered through device-tree.
+ */
+
+static int pnv_parse_cpuidle_dt(void)
+{
+ struct device_node *np;
+ int nr_idle_states, i;
+ int rc = 0;
+ u32 *temp_u32;
+ u64 *temp_u64;
+ const char **temp_string;
+
np = of_find_node_by_path("/ibm,opal/power-mgt");
if (!np) {
pr_warn("opal: PowerMgmt Node not found\n");
- goto out;
+ return -ENODEV;
}
- dt_idle_states = of_property_count_u32_elems(np,
- "ibm,cpu-idle-state-flags");
- if (dt_idle_states < 0) {
- pr_warn("cpuidle-powernv: no idle states found in the DT\n");
+ nr_idle_states = of_property_count_u32_elems(np,
+ "ibm,cpu-idle-state-flags");
+
+ pnv_idle_states = kcalloc(nr_idle_states, sizeof(*pnv_idle_states),
+ GFP_KERNEL);
+ temp_u32 = kcalloc(nr_idle_states, sizeof(u32), GFP_KERNEL);
+ temp_u64 = kcalloc(nr_idle_states, sizeof(u64), GFP_KERNEL);
+ temp_string = kcalloc(nr_idle_states, sizeof(char *), GFP_KERNEL);
+
+ if (!(pnv_idle_states && temp_u32 && temp_u64 && temp_string)) {
+ pr_err("Could not allocate memory for dt parsing\n");
+ rc = -ENOMEM;
goto out;
}

- flags = kcalloc(dt_idle_states, sizeof(*flags), GFP_KERNEL);
-
- if (of_property_read_u32_array(np,
- "ibm,cpu-idle-state-flags", flags, dt_idle_states)) {
+ /* Read flags */
+ if (of_property_read_u32_array(np, "ibm,cpu-idle-state-flags",
+ temp_u32, nr_idle_states)) {
pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-flags in DT\n");
+ rc = -EINVAL;
goto out;
}
+ for (i = 0; i < nr_idle_states; i++)
+ pnv_idle_states[i].flags = temp_u32[i];
+
+ /* Read latencies */
+ if (of_property_read_u32_array(np, "ibm,cpu-idle-state-latencies-ns",
+ temp_u32, nr_idle_states)) {
+ pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-latencies-ns in DT\n");
+ rc = -EINVAL;
+ goto out;
+ }
+ for (i = 0; i < nr_idle_states; i++)
+ pnv_idle_states[i].latency_ns = temp_u32[i];
+
+ /* Read residencies */
+ if (of_property_read_u32_array(np, "ibm,cpu-idle-state-residency-ns",
+ temp_u32, nr_idle_states)) {
+ pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-latencies-ns in DT\n");
+ rc = -EINVAL;
+ goto out;
+ }
+ for (i = 0; i < nr_idle_states; i++)
+ pnv_idle_states[i].residency_ns = temp_u32[i];

+ /* For power9 */
if (cpu_has_feature(CPU_FTR_ARCH_300)) {
- if (pnv_power9_idle_init(np, flags, dt_idle_states))
+ /* Read pm_crtl_val */
+ if (of_property_read_u64_array(np, "ibm,cpu-idle-state-psscr",
+ temp_u64, nr_idle_states)) {
+ pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr in DT\n");
+ rc = -EINVAL;
+ goto out;
+ }
+ for (i = 0; i < nr_idle_states; i++)
+ pnv_idle_states[i].psscr_val = temp_u64[i];
+
+ /* Read pm_crtl_mask */
+ if (of_property_read_u64_array(np, "ibm,cpu-idle-state-psscr-mask",
+ temp_u64, nr_idle_states)) {
+ pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr-mask in DT\n");
+ rc = -EINVAL;
goto out;
+ }
+ for (i = 0; i < nr_idle_states; i++)
+ pnv_idle_states[i].psscr_mask = temp_u64[i];
}

- for (i = 0; i < dt_idle_states; i++)
- supported_cpuidle_states |= flags[i];
+ /*
+ * power8 specific properties ibm,cpu-idle-state-pmicr-mask and
+ * ibm,cpu-idle-state-pmicr-val were never used and there is no
+ * plan to use it in near future. Hence, not parsing these properties
+ */

+ if (of_property_read_string_array(np, "ibm,cpu-idle-state-names",
+ temp_string, nr_idle_states) < 0) {
+ pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-names in DT\n");
+ rc = -EINVAL;
+ goto out;
+ }
+ for (i = 0; i < nr_idle_states; i++)
+ strncpy(pnv_idle_states[i].name, temp_string[i],
+ PNV_IDLE_NAME_LEN);
+ nr_pnv_idle_states = nr_idle_states;
+ rc = 0;
out:
- kfree(flags);
+ kfree(temp_u32);
+ kfree(temp_u64);
+ kfree(temp_string);
+ return rc;
}
+
static int __init pnv_init_idle_states(void)
{
-
+ int rc = 0;
supported_cpuidle_states = 0;

+ /* In case we error out nr_pnv_idle_states will be zero */
+ nr_pnv_idle_states = 0;
if (cpuidle_disable != IDLE_NO_OVERRIDE)
goto out;
-
+ rc = pnv_parse_cpuidle_dt();
+ if (rc)
+ return rc;
pnv_probe_idle_states();

if (!(supported_cpuidle_states & OPAL_PM_SLEEP_ENABLED_ER1)) {
diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index d29e4f041efe..7ab613d4dca1 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -414,15 +414,8 @@ static int powernv_add_idle_states(void)
else
target_residency = 0;

- if (has_stop_states) {
- int err = validate_psscr_val_mask(&psscr_val[i],
- &psscr_mask[i],
- flags[i]);
- if (err) {
- report_invalid_psscr_val(psscr_val[i], err);
- continue;
- }
- }
+ if (has_stop_states && !(state->valid))
+ continue;

if (flags[i] & OPAL_PM_TIMEBASE_STOP)
stops_timebase = true;
--
2.18.0.rc2.85.g1fb9df7


2018-07-02 14:26:24

by Akshay Adiga

[permalink] [raw]
Subject: [PATCH v2 2/2] powernv/cpuidle: Use parsed device tree values for cpuidle_init

Export pnv_idle_states and nr_pnv_idle_states so that its accessible to
cpuidle driver. Use properties from pnv_idle_states structure for powernv
cpuidle_init.

Signed-off-by: Akshay Adiga <[email protected]>
---
arch/powerpc/include/asm/cpuidle.h | 2 +
drivers/cpuidle/cpuidle-powernv.c | 143 +++++------------------------
2 files changed, 26 insertions(+), 119 deletions(-)

diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
index 574b0ce1d671..43e5f31fe64d 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -90,6 +90,8 @@ struct pnv_idle_states_t {
bool valid;
};

+extern struct pnv_idle_states_t *pnv_idle_states;
+extern int nr_pnv_idle_states;
extern u32 pnv_fastsleep_workaround_at_entry[];
extern u32 pnv_fastsleep_workaround_at_exit[];

diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index 7ab613d4dca1..ec93b2ae7b17 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -242,6 +242,7 @@ static inline void add_powernv_state(int index, const char *name,
powernv_states[index].target_residency = target_residency;
powernv_states[index].exit_latency = exit_latency;
powernv_states[index].enter = idle_fn;
+ /* For power8 and below psscr_* will be 0 */
stop_psscr_table[index].val = psscr_val;
stop_psscr_table[index].mask = psscr_mask;
}
@@ -263,179 +264,84 @@ static inline int validate_dt_prop_sizes(const char *prop1, int prop1_len,
extern u32 pnv_get_supported_cpuidle_states(void);
static int powernv_add_idle_states(void)
{
- struct device_node *power_mgt;
int nr_idle_states = 1; /* Snooze */
- int dt_idle_states, count;
- u32 latency_ns[CPUIDLE_STATE_MAX];
- u32 residency_ns[CPUIDLE_STATE_MAX];
- u32 flags[CPUIDLE_STATE_MAX];
- u64 psscr_val[CPUIDLE_STATE_MAX];
- u64 psscr_mask[CPUIDLE_STATE_MAX];
- const char *names[CPUIDLE_STATE_MAX];
+ int dt_idle_states;
u32 has_stop_states = 0;
- int i, rc;
+ int i;
u32 supported_flags = pnv_get_supported_cpuidle_states();


/* Currently we have snooze statically defined */
-
- power_mgt = of_find_node_by_path("/ibm,opal/power-mgt");
- if (!power_mgt) {
- pr_warn("opal: PowerMgmt Node not found\n");
+ if (nr_pnv_idle_states <= 0) {
+ pr_warn("cpuidle-powernv : Only Snooze is available\n");
goto out;
}

- /* Read values of any property to determine the num of idle states */
- dt_idle_states = of_property_count_u32_elems(power_mgt, "ibm,cpu-idle-state-flags");
- if (dt_idle_states < 0) {
- pr_warn("cpuidle-powernv: no idle states found in the DT\n");
- goto out;
- }
-
- count = of_property_count_u32_elems(power_mgt,
- "ibm,cpu-idle-state-latencies-ns");
-
- if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags", dt_idle_states,
- "ibm,cpu-idle-state-latencies-ns",
- count) != 0)
- goto out;
-
- count = of_property_count_strings(power_mgt,
- "ibm,cpu-idle-state-names");
- if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags", dt_idle_states,
- "ibm,cpu-idle-state-names",
- count) != 0)
- goto out;
+ /* TODO: Count only states which are eligible for cpuidle */
+ dt_idle_states = nr_pnv_idle_states;

/*
* Since snooze is used as first idle state, max idle states allowed is
* CPUIDLE_STATE_MAX -1
*/
- if (dt_idle_states > CPUIDLE_STATE_MAX - 1) {
+ if (nr_pnv_idle_states > CPUIDLE_STATE_MAX - 1) {
pr_warn("cpuidle-powernv: discovered idle states more than allowed");
dt_idle_states = CPUIDLE_STATE_MAX - 1;
}

- if (of_property_read_u32_array(power_mgt,
- "ibm,cpu-idle-state-flags", flags, dt_idle_states)) {
- pr_warn("cpuidle-powernv : missing ibm,cpu-idle-state-flags in DT\n");
- goto out;
- }
-
- if (of_property_read_u32_array(power_mgt,
- "ibm,cpu-idle-state-latencies-ns", latency_ns,
- dt_idle_states)) {
- pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-latencies-ns in DT\n");
- goto out;
- }
- if (of_property_read_string_array(power_mgt,
- "ibm,cpu-idle-state-names", names, dt_idle_states) < 0) {
- pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-names in DT\n");
- goto out;
- }
-
/*
* If the idle states use stop instruction, probe for psscr values
* and psscr mask which are necessary to specify required stop level.
*/
- has_stop_states = (flags[0] &
+ has_stop_states = (pnv_idle_states[0].flags &
(OPAL_PM_STOP_INST_FAST | OPAL_PM_STOP_INST_DEEP));
- if (has_stop_states) {
- count = of_property_count_u64_elems(power_mgt,
- "ibm,cpu-idle-state-psscr");
- if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags",
- dt_idle_states,
- "ibm,cpu-idle-state-psscr",
- count) != 0)
- goto out;
-
- count = of_property_count_u64_elems(power_mgt,
- "ibm,cpu-idle-state-psscr-mask");
- if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags",
- dt_idle_states,
- "ibm,cpu-idle-state-psscr-mask",
- count) != 0)
- goto out;
-
- if (of_property_read_u64_array(power_mgt,
- "ibm,cpu-idle-state-psscr", psscr_val, dt_idle_states)) {
- pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr in DT\n");
- goto out;
- }
-
- if (of_property_read_u64_array(power_mgt,
- "ibm,cpu-idle-state-psscr-mask",
- psscr_mask, dt_idle_states)) {
- pr_warn("cpuidle-powernv:Missing ibm,cpu-idle-state-psscr-mask in DT\n");
- goto out;
- }
- }
-
- count = of_property_count_u32_elems(power_mgt,
- "ibm,cpu-idle-state-residency-ns");
-
- if (count < 0) {
- rc = count;
- } else if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags",
- dt_idle_states,
- "ibm,cpu-idle-state-residency-ns",
- count) != 0) {
- goto out;
- } else {
- rc = of_property_read_u32_array(power_mgt,
- "ibm,cpu-idle-state-residency-ns",
- residency_ns, dt_idle_states);
- }

for (i = 0; i < dt_idle_states; i++) {
unsigned int exit_latency, target_residency;
bool stops_timebase = false;
+ struct pnv_idle_states_t *state = &pnv_idle_states[i];

/*
* Skip the platform idle state whose flag isn't in
* the supported_cpuidle_states flag mask.
*/
- if ((flags[i] & supported_flags) != flags[i])
+ if ((state->flags & supported_flags) != state->flags)
continue;
/*
* If an idle state has exit latency beyond
* POWERNV_THRESHOLD_LATENCY_NS then don't use it
* in cpu-idle.
*/
- if (latency_ns[i] > POWERNV_THRESHOLD_LATENCY_NS)
+ if (state->latency_ns > POWERNV_THRESHOLD_LATENCY_NS)
continue;
/*
* Firmware passes residency and latency values in ns.
* cpuidle expects it in us.
*/
- exit_latency = DIV_ROUND_UP(latency_ns[i], 1000);
- if (!rc)
- target_residency = DIV_ROUND_UP(residency_ns[i], 1000);
- else
- target_residency = 0;
+ exit_latency = DIV_ROUND_UP(state->latency_ns, 1000);
+ target_residency = DIV_ROUND_UP(state->residency_ns, 1000);

if (has_stop_states && !(state->valid))
continue;

- if (flags[i] & OPAL_PM_TIMEBASE_STOP)
+ if (state->flags & OPAL_PM_TIMEBASE_STOP)
stops_timebase = true;

/*
* For nap and fastsleep, use default target_residency
* values if f/w does not expose it.
*/
- if (flags[i] & OPAL_PM_NAP_ENABLED) {
- if (!rc)
- target_residency = 100;
+ if (state->flags & OPAL_PM_NAP_ENABLED) {
/* Add NAP state */
add_powernv_state(nr_idle_states, "Nap",
CPUIDLE_FLAG_NONE, nap_loop,
target_residency, exit_latency, 0, 0);
} else if (has_stop_states && !stops_timebase) {
- add_powernv_state(nr_idle_states, names[i],
+ add_powernv_state(nr_idle_states, state->name,
CPUIDLE_FLAG_NONE, stop_loop,
target_residency, exit_latency,
- psscr_val[i], psscr_mask[i]);
+ state->psscr_val,
+ state->psscr_mask);
}

/*
@@ -443,20 +349,19 @@ static int powernv_add_idle_states(void)
* within this config dependency check.
*/
#ifdef CONFIG_TICK_ONESHOT
- else if (flags[i] & OPAL_PM_SLEEP_ENABLED ||
- flags[i] & OPAL_PM_SLEEP_ENABLED_ER1) {
- if (!rc)
- target_residency = 300000;
+ else if (state->flags & OPAL_PM_SLEEP_ENABLED ||
+ state->flags & OPAL_PM_SLEEP_ENABLED_ER1) {
/* Add FASTSLEEP state */
add_powernv_state(nr_idle_states, "FastSleep",
CPUIDLE_FLAG_TIMER_STOP,
fastsleep_loop,
target_residency, exit_latency, 0, 0);
} else if (has_stop_states && stops_timebase) {
- add_powernv_state(nr_idle_states, names[i],
+ add_powernv_state(nr_idle_states, state->name,
CPUIDLE_FLAG_TIMER_STOP, stop_loop,
target_residency, exit_latency,
- psscr_val[i], psscr_mask[i]);
+ state->psscr_val,
+ state->psscr_mask);
}
#endif
else
--
2.18.0.rc2.85.g1fb9df7


2018-07-03 03:31:53

by Nicholas Piggin

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] powernv/cpuidle: Parse dt idle properties into global structure

On Mon, 2 Jul 2018 19:53:20 +0530
Akshay Adiga <[email protected]> wrote:

> Device-tree parsing happens twice, once while deciding idle state to be
> used for hotplug and once during cpuidle init. Hence, parsing the device
> tree and caching it will reduce code duplication. Parsing code has been
> moved to pnv_parse_cpuidle_dt() from pnv_probe_idle_states(). In addition
> to the properties in the device tree the number of available states is
> also required.
>
> Signed-off-by: Akshay Adiga <[email protected]>
> ---
> arch/powerpc/include/asm/cpuidle.h | 11 ++
> arch/powerpc/platforms/powernv/idle.c | 216 ++++++++++++++++----------
> drivers/cpuidle/cpuidle-powernv.c | 11 +-
> 3 files changed, 151 insertions(+), 87 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
> index e210a83eb196..574b0ce1d671 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -79,6 +79,17 @@ struct stop_sprs {
> u64 mmcra;
> };
>
> +#define PNV_IDLE_NAME_LEN 16
> +struct pnv_idle_states_t {
> + char name[PNV_IDLE_NAME_LEN];
> + u32 latency_ns;
> + u32 residency_ns;
> + u64 psscr_val;
> + u64 psscr_mask;
> + u32 flags;
> + bool valid;
> +};


This is a nice looking cleanup.

Reviewed-by: Nicholas Piggin <[email protected]>

2018-07-03 03:33:47

by Nicholas Piggin

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] powernv/cpuidle: Use parsed device tree values for cpuidle_init

On Mon, 2 Jul 2018 19:53:21 +0530
Akshay Adiga <[email protected]> wrote:

> Export pnv_idle_states and nr_pnv_idle_states so that its accessible to
> cpuidle driver. Use properties from pnv_idle_states structure for powernv
> cpuidle_init.
>
> Signed-off-by: Akshay Adiga <[email protected]>

Reviewed-by: Nicholas Piggin <[email protected]>

2018-07-03 04:21:33

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] powernv/cpuidle: Parse dt idle properties into global structure

Hi Akshay,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.18-rc3 next-20180702]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Akshay-Adiga/powernv-cpuidle-Device-tree-parsing-cleanup/20180703-024607
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=powerpc

Note: the linux-review/Akshay-Adiga/powernv-cpuidle-Device-tree-parsing-cleanup/20180703-024607 HEAD 4beae9263d77036dae7f43905823867c6d982690 builds fine.
It only hurts bisectibility.

All errors (new ones prefixed by >>):

drivers/cpuidle/cpuidle-powernv.c: In function 'powernv_add_idle_states':
>> drivers/cpuidle/cpuidle-powernv.c:417:28: error: 'state' undeclared (first use in this function); did you mean 'statx'?
if (has_stop_states && !(state->valid))
^~~~~
statx
drivers/cpuidle/cpuidle-powernv.c:417:28: note: each undeclared identifier is reported only once for each function it appears in

vim +417 drivers/cpuidle/cpuidle-powernv.c

262
263 extern u32 pnv_get_supported_cpuidle_states(void);
264 static int powernv_add_idle_states(void)
265 {
266 struct device_node *power_mgt;
267 int nr_idle_states = 1; /* Snooze */
268 int dt_idle_states, count;
269 u32 latency_ns[CPUIDLE_STATE_MAX];
270 u32 residency_ns[CPUIDLE_STATE_MAX];
271 u32 flags[CPUIDLE_STATE_MAX];
272 u64 psscr_val[CPUIDLE_STATE_MAX];
273 u64 psscr_mask[CPUIDLE_STATE_MAX];
274 const char *names[CPUIDLE_STATE_MAX];
275 u32 has_stop_states = 0;
276 int i, rc;
277 u32 supported_flags = pnv_get_supported_cpuidle_states();
278
279
280 /* Currently we have snooze statically defined */
281
282 power_mgt = of_find_node_by_path("/ibm,opal/power-mgt");
283 if (!power_mgt) {
284 pr_warn("opal: PowerMgmt Node not found\n");
285 goto out;
286 }
287
288 /* Read values of any property to determine the num of idle states */
289 dt_idle_states = of_property_count_u32_elems(power_mgt, "ibm,cpu-idle-state-flags");
290 if (dt_idle_states < 0) {
291 pr_warn("cpuidle-powernv: no idle states found in the DT\n");
292 goto out;
293 }
294
295 count = of_property_count_u32_elems(power_mgt,
296 "ibm,cpu-idle-state-latencies-ns");
297
298 if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags", dt_idle_states,
299 "ibm,cpu-idle-state-latencies-ns",
300 count) != 0)
301 goto out;
302
303 count = of_property_count_strings(power_mgt,
304 "ibm,cpu-idle-state-names");
305 if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags", dt_idle_states,
306 "ibm,cpu-idle-state-names",
307 count) != 0)
308 goto out;
309
310 /*
311 * Since snooze is used as first idle state, max idle states allowed is
312 * CPUIDLE_STATE_MAX -1
313 */
314 if (dt_idle_states > CPUIDLE_STATE_MAX - 1) {
315 pr_warn("cpuidle-powernv: discovered idle states more than allowed");
316 dt_idle_states = CPUIDLE_STATE_MAX - 1;
317 }
318
319 if (of_property_read_u32_array(power_mgt,
320 "ibm,cpu-idle-state-flags", flags, dt_idle_states)) {
321 pr_warn("cpuidle-powernv : missing ibm,cpu-idle-state-flags in DT\n");
322 goto out;
323 }
324
325 if (of_property_read_u32_array(power_mgt,
326 "ibm,cpu-idle-state-latencies-ns", latency_ns,
327 dt_idle_states)) {
328 pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-latencies-ns in DT\n");
329 goto out;
330 }
331 if (of_property_read_string_array(power_mgt,
332 "ibm,cpu-idle-state-names", names, dt_idle_states) < 0) {
333 pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-names in DT\n");
334 goto out;
335 }
336
337 /*
338 * If the idle states use stop instruction, probe for psscr values
339 * and psscr mask which are necessary to specify required stop level.
340 */
341 has_stop_states = (flags[0] &
342 (OPAL_PM_STOP_INST_FAST | OPAL_PM_STOP_INST_DEEP));
343 if (has_stop_states) {
344 count = of_property_count_u64_elems(power_mgt,
345 "ibm,cpu-idle-state-psscr");
346 if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags",
347 dt_idle_states,
348 "ibm,cpu-idle-state-psscr",
349 count) != 0)
350 goto out;
351
352 count = of_property_count_u64_elems(power_mgt,
353 "ibm,cpu-idle-state-psscr-mask");
354 if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags",
355 dt_idle_states,
356 "ibm,cpu-idle-state-psscr-mask",
357 count) != 0)
358 goto out;
359
360 if (of_property_read_u64_array(power_mgt,
361 "ibm,cpu-idle-state-psscr", psscr_val, dt_idle_states)) {
362 pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-psscr in DT\n");
363 goto out;
364 }
365
366 if (of_property_read_u64_array(power_mgt,
367 "ibm,cpu-idle-state-psscr-mask",
368 psscr_mask, dt_idle_states)) {
369 pr_warn("cpuidle-powernv:Missing ibm,cpu-idle-state-psscr-mask in DT\n");
370 goto out;
371 }
372 }
373
374 count = of_property_count_u32_elems(power_mgt,
375 "ibm,cpu-idle-state-residency-ns");
376
377 if (count < 0) {
378 rc = count;
379 } else if (validate_dt_prop_sizes("ibm,cpu-idle-state-flags",
380 dt_idle_states,
381 "ibm,cpu-idle-state-residency-ns",
382 count) != 0) {
383 goto out;
384 } else {
385 rc = of_property_read_u32_array(power_mgt,
386 "ibm,cpu-idle-state-residency-ns",
387 residency_ns, dt_idle_states);
388 }
389
390 for (i = 0; i < dt_idle_states; i++) {
391 unsigned int exit_latency, target_residency;
392 bool stops_timebase = false;
393
394 /*
395 * Skip the platform idle state whose flag isn't in
396 * the supported_cpuidle_states flag mask.
397 */
398 if ((flags[i] & supported_flags) != flags[i])
399 continue;
400 /*
401 * If an idle state has exit latency beyond
402 * POWERNV_THRESHOLD_LATENCY_NS then don't use it
403 * in cpu-idle.
404 */
405 if (latency_ns[i] > POWERNV_THRESHOLD_LATENCY_NS)
406 continue;
407 /*
408 * Firmware passes residency and latency values in ns.
409 * cpuidle expects it in us.
410 */
411 exit_latency = DIV_ROUND_UP(latency_ns[i], 1000);
412 if (!rc)
413 target_residency = DIV_ROUND_UP(residency_ns[i], 1000);
414 else
415 target_residency = 0;
416
> 417 if (has_stop_states && !(state->valid))
418 continue;
419
420 if (flags[i] & OPAL_PM_TIMEBASE_STOP)
421 stops_timebase = true;
422
423 /*
424 * For nap and fastsleep, use default target_residency
425 * values if f/w does not expose it.
426 */
427 if (flags[i] & OPAL_PM_NAP_ENABLED) {
428 if (!rc)
429 target_residency = 100;
430 /* Add NAP state */
431 add_powernv_state(nr_idle_states, "Nap",
432 CPUIDLE_FLAG_NONE, nap_loop,
433 target_residency, exit_latency, 0, 0);
434 } else if (has_stop_states && !stops_timebase) {
435 add_powernv_state(nr_idle_states, names[i],
436 CPUIDLE_FLAG_NONE, stop_loop,
437 target_residency, exit_latency,
438 psscr_val[i], psscr_mask[i]);
439 }
440
441 /*
442 * All cpuidle states with CPUIDLE_FLAG_TIMER_STOP set must come
443 * within this config dependency check.
444 */
445 #ifdef CONFIG_TICK_ONESHOT
446 else if (flags[i] & OPAL_PM_SLEEP_ENABLED ||
447 flags[i] & OPAL_PM_SLEEP_ENABLED_ER1) {
448 if (!rc)
449 target_residency = 300000;
450 /* Add FASTSLEEP state */
451 add_powernv_state(nr_idle_states, "FastSleep",
452 CPUIDLE_FLAG_TIMER_STOP,
453 fastsleep_loop,
454 target_residency, exit_latency, 0, 0);
455 } else if (has_stop_states && stops_timebase) {
456 add_powernv_state(nr_idle_states, names[i],
457 CPUIDLE_FLAG_TIMER_STOP, stop_loop,
458 target_residency, exit_latency,
459 psscr_val[i], psscr_mask[i]);
460 }
461 #endif
462 else
463 continue;
464 nr_idle_states++;
465 }
466 out:
467 return nr_idle_states;
468 }
469

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (9.25 kB)
.config.gz (22.83 kB)
Download all attachments