* [PATCH V3 08/17] intel_rapl: cleanup hardcoded MSR access
From: Zhang Rui @ 2019-07-10 13:44 UTC (permalink / raw)
To: rjw; +Cc: linux-pm, srinivas.pandruvada, rui.zhang
In-Reply-To: <1562766278-7099-1-git-send-email-rui.zhang@intel.com>
There are still some places in the common code that have hardcoded
MSR access, convert them to follow the abstracted register access.
Reviewed-and-tested-by: Pandruvada, Srinivas <srinivas.pandruvada@intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/powercap/intel_rapl.c | 38 ++++++++++++++++++++++----------------
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index 7a97d33..aa54c06 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -767,22 +767,24 @@ static int rapl_write_data_raw(struct rapl_domain *rd,
*/
static int rapl_check_unit_core(struct rapl_package *rp, int cpu)
{
- u64 msr_val;
+ struct reg_action ra;
u32 value;
- if (rdmsrl_safe_on_cpu(cpu, rp->priv->reg_unit, &msr_val)) {
- pr_err("Failed to read power unit MSR 0x%x on CPU %d, exit.\n",
+ ra.reg = rp->priv->reg_unit;
+ ra.mask = ~0;
+ if (rp->priv->read_raw(cpu, &ra)) {
+ pr_err("Failed to read power unit REG 0x%x on CPU %d, exit.\n",
rp->priv->reg_unit, cpu);
return -ENODEV;
}
- value = (msr_val & ENERGY_UNIT_MASK) >> ENERGY_UNIT_OFFSET;
+ value = (ra.value & ENERGY_UNIT_MASK) >> ENERGY_UNIT_OFFSET;
rp->energy_unit = ENERGY_UNIT_SCALE * 1000000 / (1 << value);
- value = (msr_val & POWER_UNIT_MASK) >> POWER_UNIT_OFFSET;
+ value = (ra.value & POWER_UNIT_MASK) >> POWER_UNIT_OFFSET;
rp->power_unit = 1000000 / (1 << value);
- value = (msr_val & TIME_UNIT_MASK) >> TIME_UNIT_OFFSET;
+ value = (ra.value & TIME_UNIT_MASK) >> TIME_UNIT_OFFSET;
rp->time_unit = 1000000 / (1 << value);
pr_debug("Core CPU %s energy=%dpJ, time=%dus, power=%duW\n",
@@ -793,21 +795,24 @@ static int rapl_check_unit_core(struct rapl_package *rp, int cpu)
static int rapl_check_unit_atom(struct rapl_package *rp, int cpu)
{
- u64 msr_val;
+ struct reg_action ra;
u32 value;
- if (rdmsrl_safe_on_cpu(cpu, rp->priv->reg_unit, &msr_val)) {
- pr_err("Failed to read power unit MSR 0x%x on CPU %d, exit.\n",
+ ra.reg = rp->priv->reg_unit;
+ ra.mask = ~0;
+ if (rp->priv->read_raw(cpu, &ra)) {
+ pr_err("Failed to read power unit REG 0x%x on CPU %d, exit.\n",
rp->priv->reg_unit, cpu);
return -ENODEV;
}
- value = (msr_val & ENERGY_UNIT_MASK) >> ENERGY_UNIT_OFFSET;
+
+ value = (ra.value & ENERGY_UNIT_MASK) >> ENERGY_UNIT_OFFSET;
rp->energy_unit = ENERGY_UNIT_SCALE * 1 << value;
- value = (msr_val & POWER_UNIT_MASK) >> POWER_UNIT_OFFSET;
+ value = (ra.value & POWER_UNIT_MASK) >> POWER_UNIT_OFFSET;
rp->power_unit = (1 << value) * 1000;
- value = (msr_val & TIME_UNIT_MASK) >> TIME_UNIT_OFFSET;
+ value = (ra.value & TIME_UNIT_MASK) >> TIME_UNIT_OFFSET;
rp->time_unit = 1000000 / (1 << value);
pr_debug("Atom %s energy=%dpJ, time=%dus, power=%duW\n",
@@ -1180,15 +1185,14 @@ static void rapl_remove_platform_domain(struct rapl_if_priv *priv)
static int rapl_check_domain(int cpu, int domain, struct rapl_package *rp)
{
- u32 reg;
- u64 val = 0;
+ struct reg_action ra;
switch (domain) {
case RAPL_DOMAIN_PACKAGE:
case RAPL_DOMAIN_PP0:
case RAPL_DOMAIN_PP1:
case RAPL_DOMAIN_DRAM:
- reg = rp->priv->regs[domain][RAPL_DOMAIN_REG_STATUS];
+ ra.reg = rp->priv->regs[domain][RAPL_DOMAIN_REG_STATUS];
break;
case RAPL_DOMAIN_PLATFORM:
/* PSYS(PLATFORM) is not a CPU domain, so avoid printng error */
@@ -1200,7 +1204,9 @@ static int rapl_check_domain(int cpu, int domain, struct rapl_package *rp)
/* make sure domain counters are available and contains non-zero
* values, otherwise skip it.
*/
- if (rdmsrl_safe_on_cpu(cpu, reg, &val) || !val)
+
+ ra.mask = ~0;
+ if (rp->priv->read_raw(cpu, &ra) || !ra.value)
return -ENODEV;
return 0;
--
2.7.4
^ permalink raw reply related
* [PATCH V3 04/17] intel_rapl: introduce struct rapl_if_private
From: Zhang Rui @ 2019-07-10 13:44 UTC (permalink / raw)
To: rjw; +Cc: linux-pm, srinivas.pandruvada, rui.zhang
In-Reply-To: <1562766278-7099-1-git-send-email-rui.zhang@intel.com>
Introduce a new structure, rapl_if_private, to save the private data
for different RAPL Interface.
Reviewed-and-tested-by: Pandruvada, Srinivas <srinivas.pandruvada@intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/powercap/intel_rapl.c | 59 +++++++++++++++++++++----------------------
include/linux/intel_rapl.h | 15 +++++++++++
2 files changed, 44 insertions(+), 30 deletions(-)
diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index adb35ec..e05d92d 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -75,6 +75,9 @@ enum unit_type {
TIME_UNIT,
};
+/* private data for RAPL MSR Interface */
+static struct rapl_if_priv rapl_msr_priv;
+
/* per domain data, some are optional */
#define NR_RAW_PRIMITIVES (NR_RAPL_PRIMITIVES - 2)
@@ -155,17 +158,14 @@ static const char * const rapl_domain_names[] = {
"psys",
};
-static struct powercap_control_type *control_type; /* PowerCap Controller */
-static struct rapl_domain *platform_rapl_domain; /* Platform (PSys) domain */
-
/* caller to ensure CPU hotplug lock is held */
-static struct rapl_package *rapl_find_package_domain(int cpu)
+static struct rapl_package *rapl_find_package_domain(int cpu, struct rapl_if_priv *priv)
{
int id = topology_logical_die_id(cpu);
struct rapl_package *rp;
list_for_each_entry(rp, &rapl_packages, plist) {
- if (rp->id == id)
+ if (rp->id == id && rp->priv->control_type == priv->control_type)
return rp;
}
@@ -1090,12 +1090,12 @@ static void rapl_update_domain_data(struct rapl_package *rp)
static void rapl_unregister_powercap(void)
{
- if (platform_rapl_domain) {
- powercap_unregister_zone(control_type,
- &platform_rapl_domain->power_zone);
- kfree(platform_rapl_domain);
+ if (&rapl_msr_priv.platform_rapl_domain) {
+ powercap_unregister_zone(rapl_msr_priv.control_type,
+ &rapl_msr_priv.platform_rapl_domain->power_zone);
+ kfree(rapl_msr_priv.platform_rapl_domain);
}
- powercap_unregister_control_type(control_type);
+ powercap_unregister_control_type(rapl_msr_priv.control_type);
}
static int rapl_package_register_powercap(struct rapl_package *rp)
@@ -1113,7 +1113,7 @@ static int rapl_package_register_powercap(struct rapl_package *rp)
nr_pl = find_nr_power_limit(rd);
pr_debug("register package domain %s\n", rp->name);
power_zone = powercap_register_zone(&rd->power_zone,
- control_type,
+ rp->priv->control_type,
rp->name, NULL,
&zone_ops[rd->id],
nr_pl,
@@ -1140,7 +1140,7 @@ static int rapl_package_register_powercap(struct rapl_package *rp)
/* number of power limits per domain varies */
nr_pl = find_nr_power_limit(rd);
power_zone = powercap_register_zone(&rd->power_zone,
- control_type, rd->name,
+ rp->priv->control_type, rd->name,
rp->power_zone,
&zone_ops[rd->id], nr_pl,
&constraint_ops);
@@ -1161,7 +1161,7 @@ static int rapl_package_register_powercap(struct rapl_package *rp)
*/
while (--rd >= rp->domains) {
pr_debug("unregister %s domain %s\n", rp->name, rd->name);
- powercap_unregister_zone(control_type, &rd->power_zone);
+ powercap_unregister_zone(rp->priv->control_type, &rd->power_zone);
}
return ret;
@@ -1191,9 +1191,9 @@ static int __init rapl_register_psys(void)
rd->rpl[0].name = pl1_name;
rd->rpl[1].prim_id = PL2_ENABLE;
rd->rpl[1].name = pl2_name;
- rd->rp = rapl_find_package_domain(0);
+ rd->rp = rapl_find_package_domain(0, &rapl_msr_priv);
- power_zone = powercap_register_zone(&rd->power_zone, control_type,
+ power_zone = powercap_register_zone(&rd->power_zone, rapl_msr_priv.control_type,
"psys", NULL,
&zone_ops[RAPL_DOMAIN_PLATFORM],
2, &constraint_ops);
@@ -1203,17 +1203,17 @@ static int __init rapl_register_psys(void)
return PTR_ERR(power_zone);
}
- platform_rapl_domain = rd;
+ rapl_msr_priv.platform_rapl_domain = rd;
return 0;
}
static int __init rapl_register_powercap(void)
{
- control_type = powercap_register_control_type(NULL, "intel-rapl", NULL);
- if (IS_ERR(control_type)) {
+ rapl_msr_priv.control_type = powercap_register_control_type(NULL, "intel-rapl", NULL);
+ if (IS_ERR(rapl_msr_priv.control_type)) {
pr_debug("failed to register powercap control_type.\n");
- return PTR_ERR(control_type);
+ return PTR_ERR(rapl_msr_priv.control_type);
}
return 0;
}
@@ -1338,16 +1338,16 @@ static void rapl_remove_package(struct rapl_package *rp)
}
pr_debug("remove package, undo power limit on %s: %s\n",
rp->name, rd->name);
- powercap_unregister_zone(control_type, &rd->power_zone);
+ powercap_unregister_zone(rp->priv->control_type, &rd->power_zone);
}
/* do parent zone last */
- powercap_unregister_zone(control_type, &rd_package->power_zone);
+ powercap_unregister_zone(rp->priv->control_type, &rd_package->power_zone);
list_del(&rp->plist);
kfree(rp);
}
/* called from CPU hotplug notifier, hotplug lock held */
-static struct rapl_package *rapl_add_package(int cpu)
+static struct rapl_package *rapl_add_package(int cpu, struct rapl_if_priv *priv)
{
int id = topology_logical_die_id(cpu);
struct rapl_package *rp;
@@ -1361,6 +1361,7 @@ static struct rapl_package *rapl_add_package(int cpu)
/* add the new package to the list */
rp->id = id;
rp->lead_cpu = cpu;
+ rp->priv = priv;
if (topology_max_die_per_package() > 1)
snprintf(rp->name, PACKAGE_DOMAIN_NAME_LENGTH,
@@ -1399,9 +1400,9 @@ static int rapl_cpu_online(unsigned int cpu)
{
struct rapl_package *rp;
- rp = rapl_find_package_domain(cpu);
+ rp = rapl_find_package_domain(cpu, &rapl_msr_priv);
if (!rp) {
- rp = rapl_add_package(cpu);
+ rp = rapl_add_package(cpu, &rapl_msr_priv);
if (IS_ERR(rp))
return PTR_ERR(rp);
}
@@ -1414,7 +1415,7 @@ static int rapl_cpu_down_prep(unsigned int cpu)
struct rapl_package *rp;
int lead_cpu;
- rp = rapl_find_package_domain(cpu);
+ rp = rapl_find_package_domain(cpu, &rapl_msr_priv);
if (!rp)
return 0;
@@ -1427,8 +1428,6 @@ static int rapl_cpu_down_prep(unsigned int cpu)
return 0;
}
-static enum cpuhp_state pcap_rapl_online;
-
static void power_limit_state_save(void)
{
struct rapl_package *rp;
@@ -1538,7 +1537,7 @@ static int __init rapl_init(void)
rapl_cpu_online, rapl_cpu_down_prep);
if (ret < 0)
goto err_unreg;
- pcap_rapl_online = ret;
+ rapl_msr_priv.pcap_rapl_online = ret;
/* Don't bail out if PSys is not supported */
rapl_register_psys();
@@ -1550,7 +1549,7 @@ static int __init rapl_init(void)
return 0;
err_unreg_all:
- cpuhp_remove_state(pcap_rapl_online);
+ cpuhp_remove_state(rapl_msr_priv.pcap_rapl_online);
err_unreg:
rapl_unregister_powercap();
@@ -1560,7 +1559,7 @@ static int __init rapl_init(void)
static void __exit rapl_exit(void)
{
unregister_pm_notifier(&rapl_pm_notifier);
- cpuhp_remove_state(pcap_rapl_online);
+ cpuhp_remove_state(rapl_msr_priv.pcap_rapl_online);
rapl_unregister_powercap();
}
diff --git a/include/linux/intel_rapl.h b/include/linux/intel_rapl.h
index 9471603..7bf1683e4 100644
--- a/include/linux/intel_rapl.h
+++ b/include/linux/intel_rapl.h
@@ -88,6 +88,20 @@ struct rapl_domain {
struct rapl_package *rp;
};
+/**
+ * struct rapl_if_priv: private data for different RAPL interfaces
+ * @control_type: Each RAPL interface must have its own powercap
+ * control type.
+ * @platform_rapl_domain: Optional. Some RAPL interface may have platform
+ * level RAPL control.
+ * @pcap_rapl_online: CPU hotplug state for each RAPL interface.
+ */
+struct rapl_if_priv {
+ struct powercap_control_type *control_type;
+ struct rapl_domain *platform_rapl_domain;
+ enum cpuhp_state pcap_rapl_online;
+};
+
/* maximum rapl package domain name: package-%d-die-%d */
#define PACKAGE_DOMAIN_NAME_LENGTH 30
@@ -108,6 +122,7 @@ struct rapl_package {
/* Track active cpus */
struct cpumask cpumask;
char name[PACKAGE_DOMAIN_NAME_LENGTH];
+ struct rapl_if_priv *priv;
};
#endif /* __INTEL_RAPL_H__ */
--
2.7.4
^ permalink raw reply related
* [PATCH V3 07/17] intel_rapl: cleanup some functions
From: Zhang Rui @ 2019-07-10 13:44 UTC (permalink / raw)
To: rjw; +Cc: linux-pm, srinivas.pandruvada, rui.zhang
In-Reply-To: <1562766278-7099-1-git-send-email-rui.zhang@intel.com>
Previously, there are three functions:
rapl_register_psys(), which registers platform rapl domain.
rapl_register_powercap(), which registers powercap control type.
rapl_unregsiter_powercap(), which unregisters platform rapl domain and
powercap control type.
This is confusing as the function name does not describe what it does
clearly.
With this patch, the three functions are removed, and two new functions
rapl_register_platform_domain()/rapl_unregister_platform_domain() are
introduced instead, and they do exactly what their function name describes.
Plus, as part of the common code, hardcoded MSR accesses in these functions
are converted to follow the abstracted register access.
Reviewed-and-tested-by: Pandruvada, Srinivas <srinivas.pandruvada@intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/powercap/intel_rapl.c | 62 +++++++++++++++++++++----------------------
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index d3b9d1c..7a97d33 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -1052,16 +1052,6 @@ static void rapl_update_domain_data(struct rapl_package *rp)
}
-static void rapl_unregister_powercap(void)
-{
- if (&rapl_msr_priv.platform_rapl_domain) {
- powercap_unregister_zone(rapl_msr_priv.control_type,
- &rapl_msr_priv.platform_rapl_domain->power_zone);
- kfree(rapl_msr_priv.platform_rapl_domain);
- }
- powercap_unregister_control_type(rapl_msr_priv.control_type);
-}
-
static int rapl_package_register_powercap(struct rapl_package *rp)
{
struct rapl_domain *rd;
@@ -1131,16 +1121,23 @@ static int rapl_package_register_powercap(struct rapl_package *rp)
return ret;
}
-static int __init rapl_register_psys(void)
+static int __init rapl_add_platform_domain(struct rapl_if_priv *priv)
{
struct rapl_domain *rd;
struct powercap_zone *power_zone;
- u64 val;
+ struct reg_action ra;
+ int ret;
- if (rdmsrl_safe_on_cpu(0, rapl_msr_priv.regs[RAPL_DOMAIN_PLATFORM][RAPL_DOMAIN_REG_STATUS], &val) || !val)
+ ra.reg = priv->regs[RAPL_DOMAIN_PLATFORM][RAPL_DOMAIN_REG_STATUS];
+ ra.mask = ~0;
+ ret = priv->read_raw(0, &ra);
+ if (ret || !ra.value)
return -ENODEV;
- if (rdmsrl_safe_on_cpu(0, rapl_msr_priv.regs[RAPL_DOMAIN_PLATFORM][RAPL_DOMAIN_REG_LIMIT], &val) || !val)
+ ra.reg = priv->regs[RAPL_DOMAIN_PLATFORM][RAPL_DOMAIN_REG_LIMIT];
+ ra.mask = ~0;
+ ret = priv->read_raw(0, &ra);
+ if (ret || !ra.value)
return -ENODEV;
rd = kzalloc(sizeof(*rd), GFP_KERNEL);
@@ -1149,15 +1146,15 @@ static int __init rapl_register_psys(void)
rd->name = rapl_domain_names[RAPL_DOMAIN_PLATFORM];
rd->id = RAPL_DOMAIN_PLATFORM;
- rd->regs[RAPL_DOMAIN_REG_LIMIT] = rapl_msr_priv.regs[RAPL_DOMAIN_PLATFORM][RAPL_DOMAIN_REG_LIMIT];
- rd->regs[RAPL_DOMAIN_REG_STATUS] = rapl_msr_priv.regs[RAPL_DOMAIN_PLATFORM][RAPL_DOMAIN_REG_STATUS];
+ rd->regs[RAPL_DOMAIN_REG_LIMIT] = priv->regs[RAPL_DOMAIN_PLATFORM][RAPL_DOMAIN_REG_LIMIT];
+ rd->regs[RAPL_DOMAIN_REG_STATUS] = priv->regs[RAPL_DOMAIN_PLATFORM][RAPL_DOMAIN_REG_STATUS];
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
rd->rpl[1].prim_id = PL2_ENABLE;
rd->rpl[1].name = pl2_name;
- rd->rp = rapl_find_package_domain(0, &rapl_msr_priv);
+ rd->rp = rapl_find_package_domain(0, priv);
- power_zone = powercap_register_zone(&rd->power_zone, rapl_msr_priv.control_type,
+ power_zone = powercap_register_zone(&rd->power_zone, priv->control_type,
"psys", NULL,
&zone_ops[RAPL_DOMAIN_PLATFORM],
2, &constraint_ops);
@@ -1167,19 +1164,18 @@ static int __init rapl_register_psys(void)
return PTR_ERR(power_zone);
}
- rapl_msr_priv.platform_rapl_domain = rd;
+ priv->platform_rapl_domain = rd;
return 0;
}
-static int __init rapl_register_powercap(void)
+static void rapl_remove_platform_domain(struct rapl_if_priv *priv)
{
- rapl_msr_priv.control_type = powercap_register_control_type(NULL, "intel-rapl", NULL);
- if (IS_ERR(rapl_msr_priv.control_type)) {
- pr_debug("failed to register powercap control_type.\n");
- return PTR_ERR(rapl_msr_priv.control_type);
+ if (priv->platform_rapl_domain) {
+ powercap_unregister_zone(priv->control_type,
+ &priv->platform_rapl_domain->power_zone);
+ kfree(priv->platform_rapl_domain);
}
- return 0;
}
static int rapl_check_domain(int cpu, int domain, struct rapl_package *rp)
@@ -1526,9 +1522,12 @@ static int __init rapl_init(void)
rapl_msr_priv.read_raw = rapl_msr_read_raw;
rapl_msr_priv.write_raw = rapl_msr_write_raw;
- ret = rapl_register_powercap();
- if (ret)
- return ret;
+
+ rapl_msr_priv.control_type = powercap_register_control_type(NULL, "intel-rapl", NULL);
+ if (IS_ERR(rapl_msr_priv.control_type)) {
+ pr_debug("failed to register powercap control_type.\n");
+ return PTR_ERR(rapl_msr_priv.control_type);
+ }
ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "powercap/rapl:online",
rapl_cpu_online, rapl_cpu_down_prep);
@@ -1537,7 +1536,7 @@ static int __init rapl_init(void)
rapl_msr_priv.pcap_rapl_online = ret;
/* Don't bail out if PSys is not supported */
- rapl_register_psys();
+ rapl_add_platform_domain(&rapl_msr_priv);
ret = register_pm_notifier(&rapl_pm_notifier);
if (ret)
@@ -1549,7 +1548,7 @@ static int __init rapl_init(void)
cpuhp_remove_state(rapl_msr_priv.pcap_rapl_online);
err_unreg:
- rapl_unregister_powercap();
+ powercap_unregister_control_type(rapl_msr_priv.control_type);
return ret;
}
@@ -1557,7 +1556,8 @@ static void __exit rapl_exit(void)
{
unregister_pm_notifier(&rapl_pm_notifier);
cpuhp_remove_state(rapl_msr_priv.pcap_rapl_online);
- rapl_unregister_powercap();
+ rapl_remove_platform_domain(&rapl_msr_priv);
+ powercap_unregister_control_type(rapl_msr_priv.control_type);
}
module_init(rapl_init);
--
2.7.4
^ permalink raw reply related
* [PATCH V3 06/17] intel_rapl: abstract register access operations
From: Zhang Rui @ 2019-07-10 13:44 UTC (permalink / raw)
To: rjw; +Cc: linux-pm, srinivas.pandruvada, rui.zhang
In-Reply-To: <1562766278-7099-1-git-send-email-rui.zhang@intel.com>
MSR and MMIO RAPL interfaces have different ways to access the registers,
thus in order to abstract the register access operations, two callbacks,
.read_raw()/.write_raw() are introduced, and they should be implemented by
MSR RAPL and MMIO RAPL interface driver respectly.
This patch implements them for the MSR I/F only.
Reviewed-and-tested-by: Pandruvada, Srinivas <srinivas.pandruvada@intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/powercap/intel_rapl.c | 110 ++++++++++++++++++++++--------------------
include/linux/intel_rapl.h | 13 +++++
2 files changed, 70 insertions(+), 53 deletions(-)
diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index 9f22aed..d3b9d1c 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -93,13 +93,6 @@ static struct rapl_if_priv rapl_msr_priv = {
/* per domain data, some are optional */
#define NR_RAW_PRIMITIVES (NR_RAPL_PRIMITIVES - 2)
-struct msrl_action {
- u32 msr_no;
- u64 clear_mask;
- u64 set_mask;
- int err;
-};
-
#define DOMAIN_STATE_INACTIVE BIT(0)
#define DOMAIN_STATE_POWER_LIMIT_SET BIT(1)
#define DOMAIN_STATE_BIOS_LOCKED BIT(2)
@@ -692,16 +685,16 @@ static int rapl_read_data_raw(struct rapl_domain *rd,
enum rapl_primitives prim,
bool xlate, u64 *data)
{
- u64 value, final;
- u32 msr;
+ u64 value;
struct rapl_primitive_info *rp = &rpi[prim];
+ struct reg_action ra;
int cpu;
if (!rp->name || rp->flag & RAPL_PRIMITIVE_DUMMY)
return -EINVAL;
- msr = rd->regs[rp->id];
- if (!msr)
+ ra.reg = rd->regs[rp->id];
+ if (!ra.reg)
return -EINVAL;
cpu = rd->rp->lead_cpu;
@@ -717,47 +710,23 @@ static int rapl_read_data_raw(struct rapl_domain *rd,
return 0;
}
- if (rdmsrl_safe_on_cpu(cpu, msr, &value)) {
- pr_debug("failed to read msr 0x%x on cpu %d\n", msr, cpu);
+ ra.mask = rp->mask;
+
+ if (rd->rp->priv->read_raw(cpu, &ra)) {
+ pr_debug("failed to read reg 0x%x on cpu %d\n", ra.reg, cpu);
return -EIO;
}
- final = value & rp->mask;
- final = final >> rp->shift;
+ value = ra.value >> rp->shift;
+
if (xlate)
- *data = rapl_unit_xlate(rd, rp->unit, final, 0);
+ *data = rapl_unit_xlate(rd, rp->unit, value, 0);
else
- *data = final;
+ *data = value;
return 0;
}
-
-static int msrl_update_safe(u32 msr_no, u64 clear_mask, u64 set_mask)
-{
- int err;
- u64 val;
-
- err = rdmsrl_safe(msr_no, &val);
- if (err)
- goto out;
-
- val &= ~clear_mask;
- val |= set_mask;
-
- err = wrmsrl_safe(msr_no, val);
-
-out:
- return err;
-}
-
-static void msrl_update_func(void *info)
-{
- struct msrl_action *ma = info;
-
- ma->err = msrl_update_safe(ma->msr_no, ma->clear_mask, ma->set_mask);
-}
-
/* Similar use of primitive info in the read counterpart */
static int rapl_write_data_raw(struct rapl_domain *rd,
enum rapl_primitives prim,
@@ -766,7 +735,7 @@ static int rapl_write_data_raw(struct rapl_domain *rd,
struct rapl_primitive_info *rp = &rpi[prim];
int cpu;
u64 bits;
- struct msrl_action ma;
+ struct reg_action ra;
int ret;
cpu = rd->rp->lead_cpu;
@@ -774,17 +743,13 @@ static int rapl_write_data_raw(struct rapl_domain *rd,
bits <<= rp->shift;
bits &= rp->mask;
- memset(&ma, 0, sizeof(ma));
+ memset(&ra, 0, sizeof(ra));
- ma.msr_no = rd->regs[rp->id];
- ma.clear_mask = rp->mask;
- ma.set_mask = bits;
+ ra.reg = rd->regs[rp->id];
+ ra.mask = rp->mask;
+ ra.value = bits;
- ret = smp_call_function_single(cpu, msrl_update_func, &ma, 1);
- if (ret)
- WARN_ON_ONCE(ret);
- else
- ret = ma.err;
+ ret = rd->rp->priv->write_raw(cpu, &ra);
return ret;
}
@@ -1507,6 +1472,43 @@ static struct notifier_block rapl_pm_notifier = {
.notifier_call = rapl_pm_callback,
};
+static int rapl_msr_read_raw(int cpu, struct reg_action *ra)
+{
+ if (rdmsrl_safe_on_cpu(cpu, ra->reg, &ra->value)) {
+ pr_debug("failed to read msr 0x%x on cpu %d\n", ra->reg, cpu);
+ return -EIO;
+ }
+ ra->value &= ra->mask;
+ return 0;
+}
+
+static void rapl_msr_update_func(void *info)
+{
+ struct reg_action *ra = info;
+ u64 val;
+
+ ra->err = rdmsrl_safe(ra->reg, &val);
+ if (ra->err)
+ return;
+
+ val &= ~ra->mask;
+ val |= ra->value;
+
+ ra->err = wrmsrl_safe(ra->reg, val);
+}
+
+
+static int rapl_msr_write_raw(int cpu, struct reg_action *ra)
+{
+ int ret;
+
+ ret = smp_call_function_single(cpu, rapl_msr_update_func, ra, 1);
+ if (WARN_ON_ONCE(ret))
+ return ret;
+
+ return ra->err;
+}
+
static int __init rapl_init(void)
{
const struct x86_cpu_id *id;
@@ -1522,6 +1524,8 @@ static int __init rapl_init(void)
rapl_defaults = (struct rapl_defaults *)id->driver_data;
+ rapl_msr_priv.read_raw = rapl_msr_read_raw;
+ rapl_msr_priv.write_raw = rapl_msr_write_raw;
ret = rapl_register_powercap();
if (ret)
return ret;
diff --git a/include/linux/intel_rapl.h b/include/linux/intel_rapl.h
index ec2c9e8..ff215d6 100644
--- a/include/linux/intel_rapl.h
+++ b/include/linux/intel_rapl.h
@@ -88,6 +88,13 @@ struct rapl_domain {
struct rapl_package *rp;
};
+struct reg_action {
+ u32 reg;
+ u64 mask;
+ u64 value;
+ int err;
+};
+
/**
* struct rapl_if_priv: private data for different RAPL interfaces
* @control_type: Each RAPL interface must have its own powercap
@@ -97,6 +104,10 @@ struct rapl_domain {
* @pcap_rapl_online: CPU hotplug state for each RAPL interface.
* @reg_unit: Register for getting energy/power/time unit.
* @regs: Register sets for different RAPL Domains.
+ * @read_raw: Callback for reading RAPL interface specific
+ * registers.
+ * @write_raw: Callback for writing RAPL interface specific
+ * registers.
*/
struct rapl_if_priv {
struct powercap_control_type *control_type;
@@ -104,6 +115,8 @@ struct rapl_if_priv {
enum cpuhp_state pcap_rapl_online;
u32 reg_unit;
u32 regs[RAPL_DOMAIN_MAX][RAPL_DOMAIN_REG_MAX];
+ int (*read_raw)(int cpu, struct reg_action *ra);
+ int (*write_raw)(int cpu, struct reg_action *ra);
};
/* maximum rapl package domain name: package-%d-die-%d */
--
2.7.4
^ permalink raw reply related
* [PATCH V3 05/17] intel_rapl: abstract register address
From: Zhang Rui @ 2019-07-10 13:44 UTC (permalink / raw)
To: rjw; +Cc: linux-pm, srinivas.pandruvada, rui.zhang
In-Reply-To: <1562766278-7099-1-git-send-email-rui.zhang@intel.com>
MSR and MMIO RAPL interface have different sets of registers, thus the
RAPL register address should be obtained from interface specific
structure, i.e. struct rapl_if_private, instead.
Reviewed-and-tested-by: Pandruvada, Srinivas <srinivas.pandruvada@intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/powercap/intel_rapl.c | 73 +++++++++++++++++++------------------------
include/linux/intel_rapl.h | 4 +++
2 files changed, 37 insertions(+), 40 deletions(-)
diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index e05d92d..9f22aed 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -76,7 +76,19 @@ enum unit_type {
};
/* private data for RAPL MSR Interface */
-static struct rapl_if_priv rapl_msr_priv;
+static struct rapl_if_priv rapl_msr_priv = {
+ .reg_unit = MSR_RAPL_POWER_UNIT,
+ .regs[RAPL_DOMAIN_PACKAGE] = {
+ MSR_PKG_POWER_LIMIT, MSR_PKG_ENERGY_STATUS, MSR_PKG_PERF_STATUS, 0, MSR_PKG_POWER_INFO },
+ .regs[RAPL_DOMAIN_PP0] = {
+ MSR_PP0_POWER_LIMIT, MSR_PP0_ENERGY_STATUS, 0, MSR_PP0_POLICY, 0 },
+ .regs[RAPL_DOMAIN_PP1] = {
+ MSR_PP1_POWER_LIMIT, MSR_PP1_ENERGY_STATUS, 0, MSR_PP1_POLICY, 0 },
+ .regs[RAPL_DOMAIN_DRAM] = {
+ MSR_DRAM_POWER_LIMIT, MSR_DRAM_ENERGY_STATUS, MSR_DRAM_PERF_STATUS, 0, MSR_DRAM_POWER_INFO },
+ .regs[RAPL_DOMAIN_PLATFORM] = {
+ MSR_PLATFORM_POWER_LIMIT, MSR_PLATFORM_ENERGY_STATUS, 0, 0, 0},
+};
/* per domain data, some are optional */
#define NR_RAW_PRIMITIVES (NR_RAPL_PRIMITIVES - 2)
@@ -541,15 +553,17 @@ static void rapl_init_domains(struct rapl_package *rp)
for (i = 0; i < RAPL_DOMAIN_MAX; i++) {
unsigned int mask = rp->domain_map & (1 << i);
+
+ rd->regs[RAPL_DOMAIN_REG_LIMIT] = rp->priv->regs[i][RAPL_DOMAIN_REG_LIMIT];
+ rd->regs[RAPL_DOMAIN_REG_STATUS] = rp->priv->regs[i][RAPL_DOMAIN_REG_STATUS];
+ rd->regs[RAPL_DOMAIN_REG_PERF] = rp->priv->regs[i][RAPL_DOMAIN_REG_PERF];
+ rd->regs[RAPL_DOMAIN_REG_POLICY] = rp->priv->regs[i][RAPL_DOMAIN_REG_POLICY];
+ rd->regs[RAPL_DOMAIN_REG_INFO] = rp->priv->regs[i][RAPL_DOMAIN_REG_INFO];
+
switch (mask) {
case BIT(RAPL_DOMAIN_PACKAGE):
rd->name = rapl_domain_names[RAPL_DOMAIN_PACKAGE];
rd->id = RAPL_DOMAIN_PACKAGE;
- rd->regs[RAPL_DOMAIN_REG_LIMIT] = MSR_PKG_POWER_LIMIT;
- rd->regs[RAPL_DOMAIN_REG_STATUS] = MSR_PKG_ENERGY_STATUS;
- rd->regs[RAPL_DOMAIN_REG_PERF] = MSR_PKG_PERF_STATUS;
- rd->regs[RAPL_DOMAIN_REG_POLICY] = 0;
- rd->regs[RAPL_DOMAIN_REG_INFO] = MSR_PKG_POWER_INFO;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
rd->rpl[1].prim_id = PL2_ENABLE;
@@ -558,33 +572,18 @@ static void rapl_init_domains(struct rapl_package *rp)
case BIT(RAPL_DOMAIN_PP0):
rd->name = rapl_domain_names[RAPL_DOMAIN_PP0];
rd->id = RAPL_DOMAIN_PP0;
- rd->regs[RAPL_DOMAIN_REG_LIMIT] = MSR_PP0_POWER_LIMIT;
- rd->regs[RAPL_DOMAIN_REG_STATUS] = MSR_PP0_ENERGY_STATUS;
- rd->regs[RAPL_DOMAIN_REG_PERF] = 0;
- rd->regs[RAPL_DOMAIN_REG_POLICY] = MSR_PP0_POLICY;
- rd->regs[RAPL_DOMAIN_REG_INFO] = 0;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
break;
case BIT(RAPL_DOMAIN_PP1):
rd->name = rapl_domain_names[RAPL_DOMAIN_PP1];
rd->id = RAPL_DOMAIN_PP1;
- rd->regs[RAPL_DOMAIN_REG_LIMIT] = MSR_PP1_POWER_LIMIT;
- rd->regs[RAPL_DOMAIN_REG_STATUS] = MSR_PP1_ENERGY_STATUS;
- rd->regs[RAPL_DOMAIN_REG_PERF] = 0;
- rd->regs[RAPL_DOMAIN_REG_POLICY] = MSR_PP1_POLICY;
- rd->regs[RAPL_DOMAIN_REG_INFO] = 0;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
break;
case BIT(RAPL_DOMAIN_DRAM):
rd->name = rapl_domain_names[RAPL_DOMAIN_DRAM];
rd->id = RAPL_DOMAIN_DRAM;
- rd->regs[RAPL_DOMAIN_REG_LIMIT] = MSR_DRAM_POWER_LIMIT;
- rd->regs[RAPL_DOMAIN_REG_STATUS] = MSR_DRAM_ENERGY_STATUS;
- rd->regs[RAPL_DOMAIN_REG_PERF] = MSR_DRAM_PERF_STATUS;
- rd->regs[RAPL_DOMAIN_REG_POLICY] = 0;
- rd->regs[RAPL_DOMAIN_REG_INFO] = MSR_DRAM_POWER_INFO;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
rd->domain_energy_unit =
@@ -806,9 +805,9 @@ static int rapl_check_unit_core(struct rapl_package *rp, int cpu)
u64 msr_val;
u32 value;
- if (rdmsrl_safe_on_cpu(cpu, MSR_RAPL_POWER_UNIT, &msr_val)) {
+ if (rdmsrl_safe_on_cpu(cpu, rp->priv->reg_unit, &msr_val)) {
pr_err("Failed to read power unit MSR 0x%x on CPU %d, exit.\n",
- MSR_RAPL_POWER_UNIT, cpu);
+ rp->priv->reg_unit, cpu);
return -ENODEV;
}
@@ -832,9 +831,9 @@ static int rapl_check_unit_atom(struct rapl_package *rp, int cpu)
u64 msr_val;
u32 value;
- if (rdmsrl_safe_on_cpu(cpu, MSR_RAPL_POWER_UNIT, &msr_val)) {
+ if (rdmsrl_safe_on_cpu(cpu, rp->priv->reg_unit, &msr_val)) {
pr_err("Failed to read power unit MSR 0x%x on CPU %d, exit.\n",
- MSR_RAPL_POWER_UNIT, cpu);
+ rp->priv->reg_unit, cpu);
return -ENODEV;
}
value = (msr_val & ENERGY_UNIT_MASK) >> ENERGY_UNIT_OFFSET;
@@ -1173,10 +1172,10 @@ static int __init rapl_register_psys(void)
struct powercap_zone *power_zone;
u64 val;
- if (rdmsrl_safe_on_cpu(0, MSR_PLATFORM_ENERGY_STATUS, &val) || !val)
+ if (rdmsrl_safe_on_cpu(0, rapl_msr_priv.regs[RAPL_DOMAIN_PLATFORM][RAPL_DOMAIN_REG_STATUS], &val) || !val)
return -ENODEV;
- if (rdmsrl_safe_on_cpu(0, MSR_PLATFORM_POWER_LIMIT, &val) || !val)
+ if (rdmsrl_safe_on_cpu(0, rapl_msr_priv.regs[RAPL_DOMAIN_PLATFORM][RAPL_DOMAIN_REG_LIMIT], &val) || !val)
return -ENODEV;
rd = kzalloc(sizeof(*rd), GFP_KERNEL);
@@ -1185,8 +1184,8 @@ static int __init rapl_register_psys(void)
rd->name = rapl_domain_names[RAPL_DOMAIN_PLATFORM];
rd->id = RAPL_DOMAIN_PLATFORM;
- rd->regs[RAPL_DOMAIN_REG_LIMIT] = MSR_PLATFORM_POWER_LIMIT;
- rd->regs[RAPL_DOMAIN_REG_STATUS] = MSR_PLATFORM_ENERGY_STATUS;
+ rd->regs[RAPL_DOMAIN_REG_LIMIT] = rapl_msr_priv.regs[RAPL_DOMAIN_PLATFORM][RAPL_DOMAIN_REG_LIMIT];
+ rd->regs[RAPL_DOMAIN_REG_STATUS] = rapl_msr_priv.regs[RAPL_DOMAIN_PLATFORM][RAPL_DOMAIN_REG_STATUS];
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
rd->rpl[1].prim_id = PL2_ENABLE;
@@ -1218,23 +1217,17 @@ static int __init rapl_register_powercap(void)
return 0;
}
-static int rapl_check_domain(int cpu, int domain)
+static int rapl_check_domain(int cpu, int domain, struct rapl_package *rp)
{
- unsigned msr;
+ u32 reg;
u64 val = 0;
switch (domain) {
case RAPL_DOMAIN_PACKAGE:
- msr = MSR_PKG_ENERGY_STATUS;
- break;
case RAPL_DOMAIN_PP0:
- msr = MSR_PP0_ENERGY_STATUS;
- break;
case RAPL_DOMAIN_PP1:
- msr = MSR_PP1_ENERGY_STATUS;
- break;
case RAPL_DOMAIN_DRAM:
- msr = MSR_DRAM_ENERGY_STATUS;
+ reg = rp->priv->regs[domain][RAPL_DOMAIN_REG_STATUS];
break;
case RAPL_DOMAIN_PLATFORM:
/* PSYS(PLATFORM) is not a CPU domain, so avoid printng error */
@@ -1246,7 +1239,7 @@ static int rapl_check_domain(int cpu, int domain)
/* make sure domain counters are available and contains non-zero
* values, otherwise skip it.
*/
- if (rdmsrl_safe_on_cpu(cpu, msr, &val) || !val)
+ if (rdmsrl_safe_on_cpu(cpu, reg, &val) || !val)
return -ENODEV;
return 0;
@@ -1293,7 +1286,7 @@ static int rapl_detect_domains(struct rapl_package *rp, int cpu)
for (i = 0; i < RAPL_DOMAIN_MAX; i++) {
/* use physical package id to read counters */
- if (!rapl_check_domain(cpu, i)) {
+ if (!rapl_check_domain(cpu, i, rp)) {
rp->domain_map |= 1 << i;
pr_info("Found RAPL domain %s\n", rapl_domain_names[i]);
}
diff --git a/include/linux/intel_rapl.h b/include/linux/intel_rapl.h
index 7bf1683e4..ec2c9e8 100644
--- a/include/linux/intel_rapl.h
+++ b/include/linux/intel_rapl.h
@@ -95,11 +95,15 @@ struct rapl_domain {
* @platform_rapl_domain: Optional. Some RAPL interface may have platform
* level RAPL control.
* @pcap_rapl_online: CPU hotplug state for each RAPL interface.
+ * @reg_unit: Register for getting energy/power/time unit.
+ * @regs: Register sets for different RAPL Domains.
*/
struct rapl_if_priv {
struct powercap_control_type *control_type;
struct rapl_domain *platform_rapl_domain;
enum cpuhp_state pcap_rapl_online;
+ u32 reg_unit;
+ u32 regs[RAPL_DOMAIN_MAX][RAPL_DOMAIN_REG_MAX];
};
/* maximum rapl package domain name: package-%d-die-%d */
--
2.7.4
^ permalink raw reply related
* [PATCH V3 03/17] intel_rapl: introduce intel_rapl.h
From: Zhang Rui @ 2019-07-10 13:44 UTC (permalink / raw)
To: rjw; +Cc: linux-pm, srinivas.pandruvada, rui.zhang
In-Reply-To: <1562766278-7099-1-git-send-email-rui.zhang@intel.com>
Create a new header file for the common definitions that might be used
by different RAPL Interface.
Reviewed-and-tested-by: Pandruvada, Srinivas <srinivas.pandruvada@intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
MAINTAINERS | 1 +
drivers/powercap/intel_rapl.c | 101 +------------------------------------
include/linux/intel_rapl.h | 113 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 116 insertions(+), 99 deletions(-)
create mode 100644 include/linux/intel_rapl.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 558acf2..a334feb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12638,6 +12638,7 @@ F: drivers/base/power/
F: include/linux/pm.h
F: include/linux/pm_*
F: include/linux/powercap.h
+F: include/linux/intel_rapl.h
F: drivers/powercap/
F: kernel/configs/nopm.config
diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index 9be9f20..adb35ec 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -18,8 +18,9 @@
#include <linux/cpu.h>
#include <linux/powercap.h>
#include <linux/suspend.h>
-#include <asm/iosf_mbi.h>
+#include <linux/intel_rapl.h>
+#include <asm/iosf_mbi.h>
#include <asm/processor.h>
#include <asm/cpu_device_id.h>
#include <asm/intel-family.h>
@@ -74,59 +75,9 @@ enum unit_type {
TIME_UNIT,
};
-enum rapl_domain_type {
- RAPL_DOMAIN_PACKAGE, /* entire package/socket */
- RAPL_DOMAIN_PP0, /* core power plane */
- RAPL_DOMAIN_PP1, /* graphics uncore */
- RAPL_DOMAIN_DRAM,/* DRAM control_type */
- RAPL_DOMAIN_PLATFORM, /* PSys control_type */
- RAPL_DOMAIN_MAX,
-};
-
-enum rapl_domain_reg_id {
- RAPL_DOMAIN_REG_LIMIT,
- RAPL_DOMAIN_REG_STATUS,
- RAPL_DOMAIN_REG_PERF,
- RAPL_DOMAIN_REG_POLICY,
- RAPL_DOMAIN_REG_INFO,
- RAPL_DOMAIN_REG_MAX,
-};
-
/* per domain data, some are optional */
-enum rapl_primitives {
- ENERGY_COUNTER,
- POWER_LIMIT1,
- POWER_LIMIT2,
- FW_LOCK,
-
- PL1_ENABLE, /* power limit 1, aka long term */
- PL1_CLAMP, /* allow frequency to go below OS request */
- PL2_ENABLE, /* power limit 2, aka short term, instantaneous */
- PL2_CLAMP,
-
- TIME_WINDOW1, /* long term */
- TIME_WINDOW2, /* short term */
- THERMAL_SPEC_POWER,
- MAX_POWER,
-
- MIN_POWER,
- MAX_TIME_WINDOW,
- THROTTLED_TIME,
- PRIORITY_LEVEL,
-
- /* below are not raw primitive data */
- AVERAGE_POWER,
- NR_RAPL_PRIMITIVES,
-};
-
#define NR_RAW_PRIMITIVES (NR_RAPL_PRIMITIVES - 2)
-/* Can be expanded to include events, etc.*/
-struct rapl_domain_data {
- u64 primitives[NR_RAPL_PRIMITIVES];
- unsigned long timestamp;
-};
-
struct msrl_action {
u32 msr_no;
u64 clear_mask;
@@ -138,60 +89,12 @@ struct msrl_action {
#define DOMAIN_STATE_POWER_LIMIT_SET BIT(1)
#define DOMAIN_STATE_BIOS_LOCKED BIT(2)
-#define NR_POWER_LIMITS (2)
-struct rapl_power_limit {
- struct powercap_zone_constraint *constraint;
- int prim_id; /* primitive ID used to enable */
- struct rapl_domain *domain;
- const char *name;
- u64 last_power_limit;
-};
-
static const char pl1_name[] = "long_term";
static const char pl2_name[] = "short_term";
-struct rapl_package;
-struct rapl_domain {
- const char *name;
- enum rapl_domain_type id;
- int regs[RAPL_DOMAIN_REG_MAX];
- struct powercap_zone power_zone;
- struct rapl_domain_data rdd;
- struct rapl_power_limit rpl[NR_POWER_LIMITS];
- u64 attr_map; /* track capabilities */
- unsigned int state;
- unsigned int domain_energy_unit;
- struct rapl_package *rp;
-};
#define power_zone_to_rapl_domain(_zone) \
container_of(_zone, struct rapl_domain, power_zone)
-/* maximum rapl package domain name: package-%d-die-%d */
-#define PACKAGE_DOMAIN_NAME_LENGTH 30
-
-
-/* Each rapl package contains multiple domains, these are the common
- * data across RAPL domains within a package.
- */
-struct rapl_package {
- unsigned int id; /* logical die id, equals physical 1-die systems */
- unsigned int nr_domains;
- unsigned long domain_map; /* bit map of active domains */
- unsigned int power_unit;
- unsigned int energy_unit;
- unsigned int time_unit;
- struct rapl_domain *domains; /* array of domains, sized at runtime */
- struct powercap_zone *power_zone; /* keep track of parent zone */
- unsigned long power_limit_irq; /* keep track of package power limit
- * notify interrupt enable status.
- */
- struct list_head plist;
- int lead_cpu; /* one active cpu per package for access */
- /* Track active cpus */
- struct cpumask cpumask;
- char name[PACKAGE_DOMAIN_NAME_LENGTH];
-};
-
struct rapl_defaults {
u8 floor_freq_reg_addr;
int (*check_unit)(struct rapl_package *rp, int cpu);
diff --git a/include/linux/intel_rapl.h b/include/linux/intel_rapl.h
new file mode 100644
index 0000000..9471603
--- /dev/null
+++ b/include/linux/intel_rapl.h
@@ -0,0 +1,113 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Data types and headers for RAPL support
+ *
+ * Copyright (C) 2019 Intel Corporation.
+ *
+ * Author: Zhang Rui <rui.zhang@intel.com>
+ */
+
+#ifndef __INTEL_RAPL_H__
+#define __INTEL_RAPL_H__
+
+#include <linux/types.h>
+#include <linux/powercap.h>
+
+enum rapl_domain_type {
+ RAPL_DOMAIN_PACKAGE, /* entire package/socket */
+ RAPL_DOMAIN_PP0, /* core power plane */
+ RAPL_DOMAIN_PP1, /* graphics uncore */
+ RAPL_DOMAIN_DRAM, /* DRAM control_type */
+ RAPL_DOMAIN_PLATFORM, /* PSys control_type */
+ RAPL_DOMAIN_MAX,
+};
+
+enum rapl_domain_reg_id {
+ RAPL_DOMAIN_REG_LIMIT,
+ RAPL_DOMAIN_REG_STATUS,
+ RAPL_DOMAIN_REG_PERF,
+ RAPL_DOMAIN_REG_POLICY,
+ RAPL_DOMAIN_REG_INFO,
+ RAPL_DOMAIN_REG_MAX,
+};
+
+struct rapl_package;
+
+enum rapl_primitives {
+ ENERGY_COUNTER,
+ POWER_LIMIT1,
+ POWER_LIMIT2,
+ FW_LOCK,
+
+ PL1_ENABLE, /* power limit 1, aka long term */
+ PL1_CLAMP, /* allow frequency to go below OS request */
+ PL2_ENABLE, /* power limit 2, aka short term, instantaneous */
+ PL2_CLAMP,
+
+ TIME_WINDOW1, /* long term */
+ TIME_WINDOW2, /* short term */
+ THERMAL_SPEC_POWER,
+ MAX_POWER,
+
+ MIN_POWER,
+ MAX_TIME_WINDOW,
+ THROTTLED_TIME,
+ PRIORITY_LEVEL,
+
+ /* below are not raw primitive data */
+ AVERAGE_POWER,
+ NR_RAPL_PRIMITIVES,
+};
+
+struct rapl_domain_data {
+ u64 primitives[NR_RAPL_PRIMITIVES];
+ unsigned long timestamp;
+};
+
+#define NR_POWER_LIMITS (2)
+struct rapl_power_limit {
+ struct powercap_zone_constraint *constraint;
+ int prim_id; /* primitive ID used to enable */
+ struct rapl_domain *domain;
+ const char *name;
+ u64 last_power_limit;
+};
+
+struct rapl_package;
+
+struct rapl_domain {
+ const char *name;
+ enum rapl_domain_type id;
+ int regs[RAPL_DOMAIN_REG_MAX];
+ struct powercap_zone power_zone;
+ struct rapl_domain_data rdd;
+ struct rapl_power_limit rpl[NR_POWER_LIMITS];
+ u64 attr_map; /* track capabilities */
+ unsigned int state;
+ unsigned int domain_energy_unit;
+ struct rapl_package *rp;
+};
+
+/* maximum rapl package domain name: package-%d-die-%d */
+#define PACKAGE_DOMAIN_NAME_LENGTH 30
+
+struct rapl_package {
+ unsigned int id; /* logical die id, equals physical 1-die systems */
+ unsigned int nr_domains;
+ unsigned long domain_map; /* bit map of active domains */
+ unsigned int power_unit;
+ unsigned int energy_unit;
+ unsigned int time_unit;
+ struct rapl_domain *domains; /* array of domains, sized at runtime */
+ struct powercap_zone *power_zone; /* keep track of parent zone */
+ unsigned long power_limit_irq; /* keep track of package power limit
+ * notify interrupt enable status.
+ */
+ struct list_head plist;
+ int lead_cpu; /* one active cpu per package for access */
+ /* Track active cpus */
+ struct cpumask cpumask;
+ char name[PACKAGE_DOMAIN_NAME_LENGTH];
+};
+
+#endif /* __INTEL_RAPL_H__ */
--
2.7.4
^ permalink raw reply related
* [PATCH V3 02/17] intel_rapl: remove hardcoded register index
From: Zhang Rui @ 2019-07-10 13:44 UTC (permalink / raw)
To: rjw; +Cc: linux-pm, srinivas.pandruvada, rui.zhang
In-Reply-To: <1562766278-7099-1-git-send-email-rui.zhang@intel.com>
enum rapl_domain_reg_id is defined for the RAPL registers for each RAPL
domain, thus use it whenever possible.
Reviewed-and-tested-by: Pandruvada, Srinivas <srinivas.pandruvada@intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/powercap/intel_rapl.c | 44 +++++++++++++++++++++----------------------
1 file changed, 22 insertions(+), 22 deletions(-)
diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index 45d5f22..9be9f20 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -642,11 +642,11 @@ static void rapl_init_domains(struct rapl_package *rp)
case BIT(RAPL_DOMAIN_PACKAGE):
rd->name = rapl_domain_names[RAPL_DOMAIN_PACKAGE];
rd->id = RAPL_DOMAIN_PACKAGE;
- rd->regs[0] = MSR_PKG_POWER_LIMIT;
- rd->regs[1] = MSR_PKG_ENERGY_STATUS;
- rd->regs[2] = MSR_PKG_PERF_STATUS;
- rd->regs[3] = 0;
- rd->regs[4] = MSR_PKG_POWER_INFO;
+ rd->regs[RAPL_DOMAIN_REG_LIMIT] = MSR_PKG_POWER_LIMIT;
+ rd->regs[RAPL_DOMAIN_REG_STATUS] = MSR_PKG_ENERGY_STATUS;
+ rd->regs[RAPL_DOMAIN_REG_PERF] = MSR_PKG_PERF_STATUS;
+ rd->regs[RAPL_DOMAIN_REG_POLICY] = 0;
+ rd->regs[RAPL_DOMAIN_REG_INFO] = MSR_PKG_POWER_INFO;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
rd->rpl[1].prim_id = PL2_ENABLE;
@@ -655,33 +655,33 @@ static void rapl_init_domains(struct rapl_package *rp)
case BIT(RAPL_DOMAIN_PP0):
rd->name = rapl_domain_names[RAPL_DOMAIN_PP0];
rd->id = RAPL_DOMAIN_PP0;
- rd->regs[0] = MSR_PP0_POWER_LIMIT;
- rd->regs[1] = MSR_PP0_ENERGY_STATUS;
- rd->regs[2] = 0;
- rd->regs[3] = MSR_PP0_POLICY;
- rd->regs[4] = 0;
+ rd->regs[RAPL_DOMAIN_REG_LIMIT] = MSR_PP0_POWER_LIMIT;
+ rd->regs[RAPL_DOMAIN_REG_STATUS] = MSR_PP0_ENERGY_STATUS;
+ rd->regs[RAPL_DOMAIN_REG_PERF] = 0;
+ rd->regs[RAPL_DOMAIN_REG_POLICY] = MSR_PP0_POLICY;
+ rd->regs[RAPL_DOMAIN_REG_INFO] = 0;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
break;
case BIT(RAPL_DOMAIN_PP1):
rd->name = rapl_domain_names[RAPL_DOMAIN_PP1];
rd->id = RAPL_DOMAIN_PP1;
- rd->regs[0] = MSR_PP1_POWER_LIMIT;
- rd->regs[1] = MSR_PP1_ENERGY_STATUS;
- rd->regs[2] = 0;
- rd->regs[3] = MSR_PP1_POLICY;
- rd->regs[4] = 0;
+ rd->regs[RAPL_DOMAIN_REG_LIMIT] = MSR_PP1_POWER_LIMIT;
+ rd->regs[RAPL_DOMAIN_REG_STATUS] = MSR_PP1_ENERGY_STATUS;
+ rd->regs[RAPL_DOMAIN_REG_PERF] = 0;
+ rd->regs[RAPL_DOMAIN_REG_POLICY] = MSR_PP1_POLICY;
+ rd->regs[RAPL_DOMAIN_REG_INFO] = 0;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
break;
case BIT(RAPL_DOMAIN_DRAM):
rd->name = rapl_domain_names[RAPL_DOMAIN_DRAM];
rd->id = RAPL_DOMAIN_DRAM;
- rd->regs[0] = MSR_DRAM_POWER_LIMIT;
- rd->regs[1] = MSR_DRAM_ENERGY_STATUS;
- rd->regs[2] = MSR_DRAM_PERF_STATUS;
- rd->regs[3] = 0;
- rd->regs[4] = MSR_DRAM_POWER_INFO;
+ rd->regs[RAPL_DOMAIN_REG_LIMIT] = MSR_DRAM_POWER_LIMIT;
+ rd->regs[RAPL_DOMAIN_REG_STATUS] = MSR_DRAM_ENERGY_STATUS;
+ rd->regs[RAPL_DOMAIN_REG_PERF] = MSR_DRAM_PERF_STATUS;
+ rd->regs[RAPL_DOMAIN_REG_POLICY] = 0;
+ rd->regs[RAPL_DOMAIN_REG_INFO] = MSR_DRAM_POWER_INFO;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
rd->domain_energy_unit =
@@ -1282,8 +1282,8 @@ static int __init rapl_register_psys(void)
rd->name = rapl_domain_names[RAPL_DOMAIN_PLATFORM];
rd->id = RAPL_DOMAIN_PLATFORM;
- rd->regs[0] = MSR_PLATFORM_POWER_LIMIT;
- rd->regs[1] = MSR_PLATFORM_ENERGY_STATUS;
+ rd->regs[RAPL_DOMAIN_REG_LIMIT] = MSR_PLATFORM_POWER_LIMIT;
+ rd->regs[RAPL_DOMAIN_REG_STATUS] = MSR_PLATFORM_ENERGY_STATUS;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
rd->rpl[1].prim_id = PL2_ENABLE;
--
2.7.4
^ permalink raw reply related
* [PATCH V3 01/17] intel_rapl: use reg instead of msr
From: Zhang Rui @ 2019-07-10 13:44 UTC (permalink / raw)
To: rjw; +Cc: linux-pm, srinivas.pandruvada, rui.zhang
In-Reply-To: <1562766278-7099-1-git-send-email-rui.zhang@intel.com>
To support both MSR and MMIO Interface, use 'reg' to discribe RAPL
registers instead of 'msr'.
Reviewed-and-tested-by: Pandruvada, Srinivas <srinivas.pandruvada@intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/powercap/intel_rapl.c | 98 +++++++++++++++++++++----------------------
1 file changed, 49 insertions(+), 49 deletions(-)
diff --git a/drivers/powercap/intel_rapl.c b/drivers/powercap/intel_rapl.c
index 8692f6b..45d5f22 100644
--- a/drivers/powercap/intel_rapl.c
+++ b/drivers/powercap/intel_rapl.c
@@ -83,13 +83,13 @@ enum rapl_domain_type {
RAPL_DOMAIN_MAX,
};
-enum rapl_domain_msr_id {
- RAPL_DOMAIN_MSR_LIMIT,
- RAPL_DOMAIN_MSR_STATUS,
- RAPL_DOMAIN_MSR_PERF,
- RAPL_DOMAIN_MSR_POLICY,
- RAPL_DOMAIN_MSR_INFO,
- RAPL_DOMAIN_MSR_MAX,
+enum rapl_domain_reg_id {
+ RAPL_DOMAIN_REG_LIMIT,
+ RAPL_DOMAIN_REG_STATUS,
+ RAPL_DOMAIN_REG_PERF,
+ RAPL_DOMAIN_REG_POLICY,
+ RAPL_DOMAIN_REG_INFO,
+ RAPL_DOMAIN_REG_MAX,
};
/* per domain data, some are optional */
@@ -154,7 +154,7 @@ struct rapl_package;
struct rapl_domain {
const char *name;
enum rapl_domain_type id;
- int msrs[RAPL_DOMAIN_MSR_MAX];
+ int regs[RAPL_DOMAIN_REG_MAX];
struct powercap_zone power_zone;
struct rapl_domain_data rdd;
struct rapl_power_limit rpl[NR_POWER_LIMITS];
@@ -216,7 +216,7 @@ struct rapl_primitive_info {
const char *name;
u64 mask;
int shift;
- enum rapl_domain_msr_id id;
+ enum rapl_domain_reg_id id;
enum unit_type unit;
u32 flag;
};
@@ -642,11 +642,11 @@ static void rapl_init_domains(struct rapl_package *rp)
case BIT(RAPL_DOMAIN_PACKAGE):
rd->name = rapl_domain_names[RAPL_DOMAIN_PACKAGE];
rd->id = RAPL_DOMAIN_PACKAGE;
- rd->msrs[0] = MSR_PKG_POWER_LIMIT;
- rd->msrs[1] = MSR_PKG_ENERGY_STATUS;
- rd->msrs[2] = MSR_PKG_PERF_STATUS;
- rd->msrs[3] = 0;
- rd->msrs[4] = MSR_PKG_POWER_INFO;
+ rd->regs[0] = MSR_PKG_POWER_LIMIT;
+ rd->regs[1] = MSR_PKG_ENERGY_STATUS;
+ rd->regs[2] = MSR_PKG_PERF_STATUS;
+ rd->regs[3] = 0;
+ rd->regs[4] = MSR_PKG_POWER_INFO;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
rd->rpl[1].prim_id = PL2_ENABLE;
@@ -655,33 +655,33 @@ static void rapl_init_domains(struct rapl_package *rp)
case BIT(RAPL_DOMAIN_PP0):
rd->name = rapl_domain_names[RAPL_DOMAIN_PP0];
rd->id = RAPL_DOMAIN_PP0;
- rd->msrs[0] = MSR_PP0_POWER_LIMIT;
- rd->msrs[1] = MSR_PP0_ENERGY_STATUS;
- rd->msrs[2] = 0;
- rd->msrs[3] = MSR_PP0_POLICY;
- rd->msrs[4] = 0;
+ rd->regs[0] = MSR_PP0_POWER_LIMIT;
+ rd->regs[1] = MSR_PP0_ENERGY_STATUS;
+ rd->regs[2] = 0;
+ rd->regs[3] = MSR_PP0_POLICY;
+ rd->regs[4] = 0;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
break;
case BIT(RAPL_DOMAIN_PP1):
rd->name = rapl_domain_names[RAPL_DOMAIN_PP1];
rd->id = RAPL_DOMAIN_PP1;
- rd->msrs[0] = MSR_PP1_POWER_LIMIT;
- rd->msrs[1] = MSR_PP1_ENERGY_STATUS;
- rd->msrs[2] = 0;
- rd->msrs[3] = MSR_PP1_POLICY;
- rd->msrs[4] = 0;
+ rd->regs[0] = MSR_PP1_POWER_LIMIT;
+ rd->regs[1] = MSR_PP1_ENERGY_STATUS;
+ rd->regs[2] = 0;
+ rd->regs[3] = MSR_PP1_POLICY;
+ rd->regs[4] = 0;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
break;
case BIT(RAPL_DOMAIN_DRAM):
rd->name = rapl_domain_names[RAPL_DOMAIN_DRAM];
rd->id = RAPL_DOMAIN_DRAM;
- rd->msrs[0] = MSR_DRAM_POWER_LIMIT;
- rd->msrs[1] = MSR_DRAM_ENERGY_STATUS;
- rd->msrs[2] = MSR_DRAM_PERF_STATUS;
- rd->msrs[3] = 0;
- rd->msrs[4] = MSR_DRAM_POWER_INFO;
+ rd->regs[0] = MSR_DRAM_POWER_LIMIT;
+ rd->regs[1] = MSR_DRAM_ENERGY_STATUS;
+ rd->regs[2] = MSR_DRAM_PERF_STATUS;
+ rd->regs[3] = 0;
+ rd->regs[4] = MSR_DRAM_POWER_INFO;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
rd->domain_energy_unit =
@@ -736,37 +736,37 @@ static u64 rapl_unit_xlate(struct rapl_domain *rd, enum unit_type type,
static struct rapl_primitive_info rpi[] = {
/* name, mask, shift, msr index, unit divisor */
PRIMITIVE_INFO_INIT(ENERGY_COUNTER, ENERGY_STATUS_MASK, 0,
- RAPL_DOMAIN_MSR_STATUS, ENERGY_UNIT, 0),
+ RAPL_DOMAIN_REG_STATUS, ENERGY_UNIT, 0),
PRIMITIVE_INFO_INIT(POWER_LIMIT1, POWER_LIMIT1_MASK, 0,
- RAPL_DOMAIN_MSR_LIMIT, POWER_UNIT, 0),
+ RAPL_DOMAIN_REG_LIMIT, POWER_UNIT, 0),
PRIMITIVE_INFO_INIT(POWER_LIMIT2, POWER_LIMIT2_MASK, 32,
- RAPL_DOMAIN_MSR_LIMIT, POWER_UNIT, 0),
+ RAPL_DOMAIN_REG_LIMIT, POWER_UNIT, 0),
PRIMITIVE_INFO_INIT(FW_LOCK, POWER_PP_LOCK, 31,
- RAPL_DOMAIN_MSR_LIMIT, ARBITRARY_UNIT, 0),
+ RAPL_DOMAIN_REG_LIMIT, ARBITRARY_UNIT, 0),
PRIMITIVE_INFO_INIT(PL1_ENABLE, POWER_LIMIT1_ENABLE, 15,
- RAPL_DOMAIN_MSR_LIMIT, ARBITRARY_UNIT, 0),
+ RAPL_DOMAIN_REG_LIMIT, ARBITRARY_UNIT, 0),
PRIMITIVE_INFO_INIT(PL1_CLAMP, POWER_LIMIT1_CLAMP, 16,
- RAPL_DOMAIN_MSR_LIMIT, ARBITRARY_UNIT, 0),
+ RAPL_DOMAIN_REG_LIMIT, ARBITRARY_UNIT, 0),
PRIMITIVE_INFO_INIT(PL2_ENABLE, POWER_LIMIT2_ENABLE, 47,
- RAPL_DOMAIN_MSR_LIMIT, ARBITRARY_UNIT, 0),
+ RAPL_DOMAIN_REG_LIMIT, ARBITRARY_UNIT, 0),
PRIMITIVE_INFO_INIT(PL2_CLAMP, POWER_LIMIT2_CLAMP, 48,
- RAPL_DOMAIN_MSR_LIMIT, ARBITRARY_UNIT, 0),
+ RAPL_DOMAIN_REG_LIMIT, ARBITRARY_UNIT, 0),
PRIMITIVE_INFO_INIT(TIME_WINDOW1, TIME_WINDOW1_MASK, 17,
- RAPL_DOMAIN_MSR_LIMIT, TIME_UNIT, 0),
+ RAPL_DOMAIN_REG_LIMIT, TIME_UNIT, 0),
PRIMITIVE_INFO_INIT(TIME_WINDOW2, TIME_WINDOW2_MASK, 49,
- RAPL_DOMAIN_MSR_LIMIT, TIME_UNIT, 0),
+ RAPL_DOMAIN_REG_LIMIT, TIME_UNIT, 0),
PRIMITIVE_INFO_INIT(THERMAL_SPEC_POWER, POWER_INFO_THERMAL_SPEC_MASK,
- 0, RAPL_DOMAIN_MSR_INFO, POWER_UNIT, 0),
+ 0, RAPL_DOMAIN_REG_INFO, POWER_UNIT, 0),
PRIMITIVE_INFO_INIT(MAX_POWER, POWER_INFO_MAX_MASK, 32,
- RAPL_DOMAIN_MSR_INFO, POWER_UNIT, 0),
+ RAPL_DOMAIN_REG_INFO, POWER_UNIT, 0),
PRIMITIVE_INFO_INIT(MIN_POWER, POWER_INFO_MIN_MASK, 16,
- RAPL_DOMAIN_MSR_INFO, POWER_UNIT, 0),
+ RAPL_DOMAIN_REG_INFO, POWER_UNIT, 0),
PRIMITIVE_INFO_INIT(MAX_TIME_WINDOW, POWER_INFO_MAX_TIME_WIN_MASK, 48,
- RAPL_DOMAIN_MSR_INFO, TIME_UNIT, 0),
+ RAPL_DOMAIN_REG_INFO, TIME_UNIT, 0),
PRIMITIVE_INFO_INIT(THROTTLED_TIME, PERF_STATUS_THROTTLE_TIME_MASK, 0,
- RAPL_DOMAIN_MSR_PERF, TIME_UNIT, 0),
+ RAPL_DOMAIN_REG_PERF, TIME_UNIT, 0),
PRIMITIVE_INFO_INIT(PRIORITY_LEVEL, PP_POLICY_MASK, 0,
- RAPL_DOMAIN_MSR_POLICY, ARBITRARY_UNIT, 0),
+ RAPL_DOMAIN_REG_POLICY, ARBITRARY_UNIT, 0),
/* non-hardware */
PRIMITIVE_INFO_INIT(AVERAGE_POWER, 0, 0, 0, POWER_UNIT,
RAPL_PRIMITIVE_DERIVED),
@@ -798,7 +798,7 @@ static int rapl_read_data_raw(struct rapl_domain *rd,
if (!rp->name || rp->flag & RAPL_PRIMITIVE_DUMMY)
return -EINVAL;
- msr = rd->msrs[rp->id];
+ msr = rd->regs[rp->id];
if (!msr)
return -EINVAL;
@@ -874,7 +874,7 @@ static int rapl_write_data_raw(struct rapl_domain *rd,
memset(&ma, 0, sizeof(ma));
- ma.msr_no = rd->msrs[rp->id];
+ ma.msr_no = rd->regs[rp->id];
ma.clear_mask = rp->mask;
ma.set_mask = bits;
@@ -1282,8 +1282,8 @@ static int __init rapl_register_psys(void)
rd->name = rapl_domain_names[RAPL_DOMAIN_PLATFORM];
rd->id = RAPL_DOMAIN_PLATFORM;
- rd->msrs[0] = MSR_PLATFORM_POWER_LIMIT;
- rd->msrs[1] = MSR_PLATFORM_ENERGY_STATUS;
+ rd->regs[0] = MSR_PLATFORM_POWER_LIMIT;
+ rd->regs[1] = MSR_PLATFORM_ENERGY_STATUS;
rd->rpl[0].prim_id = PL1_ENABLE;
rd->rpl[0].name = pl1_name;
rd->rpl[1].prim_id = PL2_ENABLE;
--
2.7.4
^ permalink raw reply related
* [PATCH V3 00/17] intel_rapl: RAPL changes for 5.3
From: Zhang Rui @ 2019-07-10 13:44 UTC (permalink / raw)
To: rjw; +Cc: linux-pm, srinivas.pandruvada, rui.zhang
This patch series contains two parts, a) RAPL abstraction and RAPL MMIO
interface support, b) RAPL support for latest ICL platforms.
All these patches are rebased on top of Linus' tree, which has the latest
RAPL changes for multi-die system.
Note that I have tested the RAPL abstractiona and MMIO RAPL patches on
Dell XPS 9360, a SKL platform, with 5.2-rc6 tip tree. But I have not tested
it with latest Linus' tree becase of a kernel boot crash regression.
Changes in V3:
- rebase RAPL MMIO support on top of latest Linus' tree.
- add RAPL support patches for ICL platforms on top to resolve the
conflicts.
Changes in V2:
- add kerneldoc for struct rapl_if_priv.
- use intel_rapl_msr.c for RAPL MSR I/F driver, instead of intel_rapl.c.
- changelog and coding style update.
thanks,
rui
^ permalink raw reply
* Re: [PATCH] drivers: thermal: processor_thermal: mark pm function __maybe_unused
From: Zhang Rui @ 2019-07-10 13:34 UTC (permalink / raw)
To: Arnd Bergmann, Eduardo Valentin
Cc: Srinivas Pandruvada, Daniel Lezcano, linux-pm, linux-kernel
In-Reply-To: <20190708124743.3585020-1-arnd@arndb.de>
Hi, Arnd,
thanks for the report.
On 一, 2019-07-08 at 14:47 +0200, Arnd Bergmann wrote:
> Without CONFIG_PM, we get a harmless warning:
>
> drivers/thermal/intel/int340x_thermal/processor_thermal_device.c:446:
> 12: error: 'proc_thermal_resume' defined but not used [-
> Werror=unused-function]
> static int proc_thermal_resume(struct device *dev)
>
> Mark it __maybe_unused to shut up the warning.
>
> Fixes: aaba9791fbb4 ("drivers: thermal: processor_thermal: Read PPCC
> on resume")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> .../thermal/intel/int340x_thermal/processor_thermal_device.c | 2
> +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git
> a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> index a3210f09f366..5ce639a99330 100644
> ---
> a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> +++
> b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
> @@ -443,7 +443,7 @@ static void proc_thermal_pci_remove(struct
> pci_dev *pdev)
> pci_disable_device(pdev);
> }
>
> -static int proc_thermal_resume(struct device *dev)
> +static int __maybe_unused proc_thermal_resume(struct device *dev)
> {
> struct proc_thermal_device *proc_dev;
>
I'd rather prefer to add #ifdef CONFIG_PM_SLEEP for
proc_thermal_resume().
Just like the patch below, what do you think?
thanks,
rui
From 6c395f66e98c895cf3ebf87c0b2fc63b6a57a196 Mon Sep 17 00:00:00 2001
From: Zhang Rui <rui.zhang@intel.com>
Date: Tue, 9 Jul 2019 21:19:12 +0800
Subject: [PATCH] drivers: thermal: processor_thermal_device: Fix build warning
As a system sleep callback, proc_thermal_resume() should be defined only
if CONFIG_PM_SLEEP is set.
This fixes a build warning when CONFIG_PM_SLEEP is not set,
drivers/thermal/intel/int340x_thermal/processor_thermal_device.c:446:12: error: 'proc_thermal_resume' defined but not used [-Werror=unused-function]
static int proc_thermal_resume(struct device *dev)
Fixes: aaba9791fbb4 ("drivers: thermal: processor_thermal: Read PPCC on resume")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/thermal/intel/int340x_thermal/processor_thermal_device.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
index a3210f0..77dae1e 100644
--- a/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
+++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_device.c
@@ -443,6 +443,7 @@ static void proc_thermal_pci_remove(struct pci_dev *pdev)
pci_disable_device(pdev);
}
+#ifdef CONFIG_PM_SLEEP
static int proc_thermal_resume(struct device *dev)
{
struct proc_thermal_device *proc_dev;
@@ -452,6 +453,9 @@ static int proc_thermal_resume(struct device *dev)
return 0;
}
+#else
+#define proc_thermal_resume NULL
+#endif
static SIMPLE_DEV_PM_OPS(proc_thermal_pm, NULL, proc_thermal_resume);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2] tools/power/x86/intel-speed-select: Add .gitignore file
From: Prarit Bhargava @ 2019-07-10 12:21 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Linux PM, Srinivas Pandruvada, David Arcari
In-Reply-To: <CAHp75VeyUK_ohxPwTg988gXugRh=y5QxT1zhSCmKoG5-CutRnQ@mail.gmail.com>
On 7/9/19 12:39 PM, Andy Shevchenko wrote:
> On Tue, Jul 9, 2019 at 2:17 AM Prarit Bhargava <prarit@redhat.com> wrote:
>>
>> Add a .gitignore file for build include/ and final binary.
>>
>
> It has improper Cc list (No PDx86, no its maintainers, no LKML, which
> is also requirement to send patches to PDx86).
> Please, gather the tags you got and send v2 with properly formed Cc list.
>
The problem is that get_maintainer.pl is throwing an error when I run it on this
patch so I had to make a guess at the cc list.
ie) Bad divisor in main::vcs_assign: 0
I'll backup a couple of versions of get_maintainer.pl and see if that works. If
not, should I just copy the cc list from Srinivas' original patchset?
P.
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
>> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Cc: David Arcari <darcari@redhat.com>
>> ---
>> tools/power/x86/intel-speed-select/.gitignore | 2 ++
>> 1 file changed, 2 insertions(+)
>> create mode 100644 tools/power/x86/intel-speed-select/.gitignore
>>
>> diff --git a/tools/power/x86/intel-speed-select/.gitignore b/tools/power/x86/intel-speed-select/.gitignore
>> new file mode 100644
>> index 000000000000..f61145925ce9
>> --- /dev/null
>> +++ b/tools/power/x86/intel-speed-select/.gitignore
>> @@ -0,0 +1,2 @@
>> +include/
>> +intel-speed-select
>> --
>> 2.21.0
>>
>
>
^ permalink raw reply
* [PATCH] thermal: Add some error messages
From: Amit Kucheria @ 2019-07-10 11:46 UTC (permalink / raw)
To: linux-kernel, Zhang Rui, Eduardo Valentin, Daniel Lezcano; +Cc: linux-pm
In-Reply-To: <cover.1562757659.git.amit.kucheria@linaro.org>
When registering a thermal zone device, we currently return -EINVAL in
four cases. This makes it a little hard to debug the real cause of the
failure.
Print some error messages to make it easier for developer to figure out
what happened.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
drivers/thermal/thermal_core.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 46cfb7de4eb28..95c3a4c649cf8 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -1238,17 +1238,26 @@ thermal_zone_device_register(const char *type, int trips, int mask,
int count;
struct thermal_governor *governor;
- if (!type || strlen(type) == 0)
+ if (!type || strlen(type) == 0) {
+ pr_err("Error: No thermal zone type defined");
return ERR_PTR(-EINVAL);
+ }
- if (type && strlen(type) >= THERMAL_NAME_LENGTH)
+ if (type && strlen(type) >= THERMAL_NAME_LENGTH) {
+ pr_err("Error: Thermal zone name (%s) too long, should be under %d chars",
+ type, THERMAL_NAME_LENGTH);
return ERR_PTR(-EINVAL);
+ }
- if (trips > THERMAL_MAX_TRIPS || trips < 0 || mask >> trips)
+ if (trips > THERMAL_MAX_TRIPS || trips < 0 || mask >> trips) {
+ pr_err("Error: Incorrect number of thermal trips");
return ERR_PTR(-EINVAL);
+ }
- if (!ops)
+ if (!ops) {
+ pr_err("Error: Thermal zone device ops not defined");
return ERR_PTR(-EINVAL);
+ }
if (trips > 0 && (!ops->get_trip_type || !ops->get_trip_temp))
return ERR_PTR(-EINVAL);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 1/3] opp: core: add regulators enable and disable
From: Kamil Konieczny @ 2019-07-10 10:43 UTC (permalink / raw)
To: Viresh Kumar
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
linux-pm, linux-samsung-soc
In-Reply-To: <20190709054014.o3g4e6gbovrq3vvn@vireshk-i7>
On 09.07.2019 07:40, Viresh Kumar wrote:
> On 08-07-19, 16:11, k.konieczny@partner.samsung.com wrote:
>> From: Kamil Konieczny <k.konieczny@partner.samsung.com>
>>
>> Add enable regulators to dev_pm_opp_set_regulators() and disable
>> regulators to dev_pm_opp_put_regulators(). This prepares for
>> converting exynos-bus devfreq driver to use dev_pm_opp_set_rate().
>>
>> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
>> ---
>> drivers/opp/core.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index 0e7703fe733f..947cac452854 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -1580,8 +1580,19 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
>> if (ret)
>> goto free_regulators;
>>
>> + for (i = 0; i < opp_table->regulator_count; i++) {
>> + ret = regulator_enable(opp_table->regulators[i]);
>> + if (ret < 0)
>> + goto disable;
>> + }
>
> I am wondering on why is this really required as this isn't done for
> any other platform, probably because the regulators are enabled by
> bootloader and are always on.
It is not ABI break, it should work with existing DTBs
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH 1/3] opp: core: add regulators enable and disable
From: Kamil Konieczny @ 2019-07-10 10:42 UTC (permalink / raw)
To: Viresh Kumar
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
linux-pm, linux-samsung-soc
In-Reply-To: <3d6eacb8-ed8d-d4ac-1c36-6224ab88a5dc@partner.samsung.com>
On 10.07.2019 12:16, Kamil Konieczny wrote:
>
>
> On 09.07.2019 07:40, Viresh Kumar wrote:
>> On 08-07-19, 16:11, k.konieczny@partner.samsung.com wrote:
>>> From: Kamil Konieczny <k.konieczny@partner.samsung.com>
>>>
>>> Add enable regulators to dev_pm_opp_set_regulators() and disable
>>> regulators to dev_pm_opp_put_regulators(). This prepares for
>>> converting exynos-bus devfreq driver to use dev_pm_opp_set_rate().
>>>
>>> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
>>> ---
>>> drivers/opp/core.c | 13 +++++++++++++
>>> 1 file changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>>> index 0e7703fe733f..947cac452854 100644
>>> --- a/drivers/opp/core.c
>>> +++ b/drivers/opp/core.c
>>> @@ -1580,8 +1580,19 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
>>> if (ret)
>>> goto free_regulators;
>>>
>>> + for (i = 0; i < opp_table->regulator_count; i++) {
>>> + ret = regulator_enable(opp_table->regulators[i]);
>>> + if (ret < 0)
>>> + goto disable;
>>> + }
>>
>> I am wondering on why is this really required as this isn't done for
>> any other platform, probably because the regulators are enabled by
>> bootloader and are always on.
>
> It was not enabled for historical reasons, from design point regualtors
> should be enabled before use.
On Exynos platform devfreq driver (exynos-bus) always enabled them,
so I wanted to preserve the current behaviour.
I've also checked the change with cpufreq-dt driver and it doesn't cause
issues.
Do you find this change acceptable?
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
^ permalink raw reply
* Re: The tick is active on idle adaptive-tick CPUs when /dev/cpu_dma_latency is used
From: Rafael J. Wysocki @ 2019-07-10 10:22 UTC (permalink / raw)
To: Thomas Lindroth
Cc: Linux PM, Linux Kernel Mailing List, Peter Zijlstra,
Frederic Weisbecker
In-Reply-To: <6ef6b96e-1772-6e80-60cf-eb57af618e99@gmail.com>
On Saturday, July 6, 2019 3:02:11 PM CEST Thomas Lindroth wrote:
> On 7/6/19 1:06 PM, Rafael J. Wysocki wrote:
> > The patch is below, but note that it adds the tick stopping overhead to the idle loop
> > for CPUs that are not adaptive-tick and when PM QoS latency constraints are used
> > which is not desirable in general.
> >
> > Please test it, but as I said above, the real solution appears to be to treat adaptive-tick
> > CPUs in a special way in the idle loop.
> >
> > ---
> > drivers/cpuidle/governors/menu.c | 16 +++++-----------
> > 1 file changed, 5 insertions(+), 11 deletions(-)
> >
> > Index: linux-pm/drivers/cpuidle/governors/menu.c
> > ===================================================================
> > --- linux-pm.orig/drivers/cpuidle/governors/menu.c
> > +++ linux-pm/drivers/cpuidle/governors/menu.c
> > @@ -302,9 +302,10 @@ static int menu_select(struct cpuidle_dr
> > !drv->states[0].disabled && !dev->states_usage[0].disable)) {
> > /*
> > * In this case state[0] will be used no matter what, so return
> > - * it right away and keep the tick running.
> > + * it right away and keep the tick running if state[0] is a
> > + * polling one.
> > */
> > - *stop_tick = false;
> > + *stop_tick = !!(drv->states[0].flags & CPUIDLE_FLAG_POLLING);
> > return 0;
> > }
> >
> > @@ -395,16 +396,9 @@ static int menu_select(struct cpuidle_dr
> >
> > return idx;
> > }
> > - if (s->exit_latency > latency_req) {
> > - /*
> > - * If we break out of the loop for latency reasons, use
> > - * the target residency of the selected state as the
> > - * expected idle duration so that the tick is retained
> > - * as long as that target residency is low enough.
> > - */
> > - predicted_us = drv->states[idx].target_residency;
> > + if (s->exit_latency > latency_req)
> > break;
> > - }
> > +
> > idx = i;
> > }
>
> I tested the patch and it appears to work. Idle CPUs now have ticks disabled even
> when /dev/cpu_dma_latency is used.
OK, thanks, but as I said previously, you'd see the problem again with the PM QoS
latency constraint set to 0, which is somewhat inconsistent. Also, this fix is
specific to the menu governor and the behavior should not depend on the
governor here IMO, so I have another patch to try (appended).
Please test it (instead of the previous one) and report back.
> I also want to thank you for your work on the idle loop redesign. Overall it works
> much better than before. I used to have a problem where idle CPUs would end up
> doing C0 polling for a long time resulting in a big performance drop on the HT
> sibling. When benchmarking I always had to offline siblings to get consistent
> results. That problem was fixed in the redesign.
>
Thank you, much appreciated.
---
kernel/sched/idle.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Index: linux-pm/kernel/sched/idle.c
===================================================================
--- linux-pm.orig/kernel/sched/idle.c
+++ linux-pm/kernel/sched/idle.c
@@ -191,7 +191,8 @@ static void cpuidle_idle_call(void)
*/
next_state = cpuidle_select(drv, dev, &stop_tick);
- if (stop_tick || tick_nohz_tick_stopped())
+ if (stop_tick || tick_nohz_tick_stopped() ||
+ !housekeeping_cpu(dev->cpu, HK_FLAG_TICK))
tick_nohz_idle_stop_tick();
else
tick_nohz_idle_retain_tick();
^ permalink raw reply
* [PATCH] driver core: Remove device link creation limitation
From: Rafael J. Wysocki @ 2019-07-10 10:19 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: LKML, Linux PM, Saravana Kannan, Lukas Wunner, Jon Hunter,
Ulf Hansson, Marek Szyprowski
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
If device_link_add() is called for a consumer/supplier pair with an
existing device link between them and the existing link's type is
not in agreement with the flags passed to that function by its
caller, NULL will be returned. That is seriously inconvenient,
because it forces the callers of device_link_add() to worry about
what others may or may not do even if that is not relevant to them
for any other reasons.
It turns out, however, that this limitation can be made go away
relatively easily.
The underlying observation is that if DL_FLAG_STATELESS has been
passed to device_link_add() in flags for the given consumer/supplier
pair at least once, calling either device_link_del() or
device_link_remove() to release the link returned by it should work,
but there are no other requirements associated with that flag. In
turn, if at least one of the callers of device_link_add() for the
given consumer/supplier pair has not passed DL_FLAG_STATELESS to it
in flags, the driver core should track the status of the link and act
on it as appropriate (ie. the link should be treated as "managed").
This means that DL_FLAG_STATELESS needs to be set for managed device
links and it should be valid to call device_link_del() or
device_link_remove() to drop references to them in certain
sutiations.
To allow that to happen, introduce a new (internal) device link flag
called DL_FLAG_MANAGED and make device_link_add() set it automatically
whenever DL_FLAG_STATELESS is not passed to it. Also make it take
additional references to existing device links that were previously
stateless (that is, with DL_FLAG_STATELESS set and DL_FLAG_MANAGED
unset) and will need to be managed going forward and initialize
their status (which has been DL_STATE_NONE so far).
Accordingly, when a managed device link is dropped automatically
by the driver core, make it clear DL_FLAG_MANAGED, reset the link's
status back to DL_STATE_NONE and drop the reference to it associated
with DL_FLAG_MANAGED instead of just deleting it right away (to
allow it to stay around in case it still needs to be released
explicitly by someone).
With that, since setting DL_FLAG_STATELESS doesn't mean that the
device link in question is not managed any more, replace all of the
status-tracking checks against DL_FLAG_STATELESS with analogous
checks against DL_FLAG_MANAGED and update the documentation to
reflect these changes.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
Documentation/driver-api/device_link.rst | 4
drivers/base/core.c | 169 +++++++++++++++++--------------
drivers/base/power/runtime.c | 4
include/linux/device.h | 4
4 files changed, 102 insertions(+), 79 deletions(-)
Index: linux-pm/drivers/base/core.c
===================================================================
--- linux-pm.orig/drivers/base/core.c
+++ linux-pm/drivers/base/core.c
@@ -124,6 +124,50 @@ static int device_is_dependent(struct de
return ret;
}
+static void device_link_init_status(struct device_link *link,
+ struct device *consumer,
+ struct device *supplier)
+{
+ switch (supplier->links.status) {
+ case DL_DEV_PROBING:
+ switch (consumer->links.status) {
+ case DL_DEV_PROBING:
+ /*
+ * A consumer driver can create a link to a supplier
+ * that has not completed its probing yet as long as it
+ * knows that the supplier is already functional (for
+ * example, it has just acquired some resources from the
+ * supplier).
+ */
+ link->status = DL_STATE_CONSUMER_PROBE;
+ break;
+ default:
+ link->status = DL_STATE_DORMANT;
+ break;
+ }
+ break;
+ case DL_DEV_DRIVER_BOUND:
+ switch (consumer->links.status) {
+ case DL_DEV_PROBING:
+ link->status = DL_STATE_CONSUMER_PROBE;
+ break;
+ case DL_DEV_DRIVER_BOUND:
+ link->status = DL_STATE_ACTIVE;
+ break;
+ default:
+ link->status = DL_STATE_AVAILABLE;
+ break;
+ }
+ break;
+ case DL_DEV_UNBINDING:
+ link->status = DL_STATE_SUPPLIER_UNBIND;
+ break;
+ default:
+ link->status = DL_STATE_DORMANT;
+ break;
+ }
+}
+
static int device_reorder_to_tail(struct device *dev, void *not_used)
{
struct device_link *link;
@@ -179,9 +223,9 @@ void device_pm_move_to_tail(struct devic
* of the link. If DL_FLAG_PM_RUNTIME is not set, DL_FLAG_RPM_ACTIVE will be
* ignored.
*
- * If DL_FLAG_STATELESS is set in @flags, the link is not going to be managed by
- * the driver core and, in particular, the caller of this function is expected
- * to drop the reference to the link acquired by it directly.
+ * If DL_FLAG_STATELESS is set in @flags, the caller of this function is
+ * expected to release the link returned by it directly with the help of either
+ * device_link_del() or device_link_remove().
*
* If that flag is not set, however, the caller of this function is handing the
* management of the link over to the driver core entirely and its return value
@@ -201,9 +245,16 @@ void device_pm_move_to_tail(struct devic
* be used to request the driver core to automaticall probe for a consmer
* driver after successfully binding a driver to the supplier device.
*
- * The combination of DL_FLAG_STATELESS and either DL_FLAG_AUTOREMOVE_CONSUMER
- * or DL_FLAG_AUTOREMOVE_SUPPLIER set in @flags at the same time is invalid and
- * will cause NULL to be returned upfront.
+ * The combination of DL_FLAG_STATELESS and one of DL_FLAG_AUTOREMOVE_CONSUMER,
+ * DL_FLAG_AUTOREMOVE_SUPPLIER, or DL_FLAG_AUTOPROBE_CONSUMER set in @flags at
+ * the same time is invalid and will cause NULL to be returned upfront.
+ * However, if a device link between the given @consumer and @supplier pair
+ * exists already when this function is called for them, the existing link will
+ * be returned regardless of its current type and status (the link's flags may
+ * be modified then). The caller of this function is then expected to treat
+ * the link as though it has just been created, so (in particular) if
+ * DL_FLAG_STATELESS was passed in @flags, the link needs to be released
+ * explicitly when not needed any more (as stated above).
*
* A side effect of the link creation is re-ordering of dpm_list and the
* devices_kset list by moving the consumer device and all devices depending
@@ -223,7 +274,8 @@ struct device_link *device_link_add(stru
(flags & DL_FLAG_STATELESS &&
flags & (DL_FLAG_AUTOREMOVE_CONSUMER |
DL_FLAG_AUTOREMOVE_SUPPLIER |
- DL_FLAG_AUTOPROBE_CONSUMER)) ||
+ DL_FLAG_AUTOPROBE_CONSUMER |
+ DL_FLAG_MANAGED)) ||
(flags & DL_FLAG_AUTOPROBE_CONSUMER &&
flags & (DL_FLAG_AUTOREMOVE_CONSUMER |
DL_FLAG_AUTOREMOVE_SUPPLIER)))
@@ -236,6 +288,9 @@ struct device_link *device_link_add(stru
}
}
+ if (!(flags & DL_FLAG_STATELESS))
+ flags |= DL_FLAG_MANAGED;
+
device_links_write_lock();
device_pm_lock();
@@ -262,15 +317,6 @@ struct device_link *device_link_add(stru
if (link->consumer != consumer)
continue;
- /*
- * Don't return a stateless link if the caller wants a stateful
- * one and vice versa.
- */
- if (WARN_ON((flags & DL_FLAG_STATELESS) != (link->flags & DL_FLAG_STATELESS))) {
- link = NULL;
- goto out;
- }
-
if (flags & DL_FLAG_PM_RUNTIME) {
if (!(link->flags & DL_FLAG_PM_RUNTIME)) {
pm_runtime_new_link(consumer);
@@ -281,6 +327,7 @@ struct device_link *device_link_add(stru
}
if (flags & DL_FLAG_STATELESS) {
+ link->flags |= DL_FLAG_STATELESS;
kref_get(&link->kref);
goto out;
}
@@ -299,6 +346,11 @@ struct device_link *device_link_add(stru
link->flags &= ~(DL_FLAG_AUTOREMOVE_CONSUMER |
DL_FLAG_AUTOREMOVE_SUPPLIER);
}
+ if (!(link->flags & DL_FLAG_MANAGED)) {
+ kref_get(&link->kref);
+ link->flags |= DL_FLAG_MANAGED;
+ device_link_init_status(link, consumer, supplier);
+ }
goto out;
}
@@ -325,48 +377,10 @@ struct device_link *device_link_add(stru
kref_init(&link->kref);
/* Determine the initial link state. */
- if (flags & DL_FLAG_STATELESS) {
+ if (flags & DL_FLAG_STATELESS)
link->status = DL_STATE_NONE;
- } else {
- switch (supplier->links.status) {
- case DL_DEV_PROBING:
- switch (consumer->links.status) {
- case DL_DEV_PROBING:
- /*
- * A consumer driver can create a link to a
- * supplier that has not completed its probing
- * yet as long as it knows that the supplier is
- * already functional (for example, it has just
- * acquired some resources from the supplier).
- */
- link->status = DL_STATE_CONSUMER_PROBE;
- break;
- default:
- link->status = DL_STATE_DORMANT;
- break;
- }
- break;
- case DL_DEV_DRIVER_BOUND:
- switch (consumer->links.status) {
- case DL_DEV_PROBING:
- link->status = DL_STATE_CONSUMER_PROBE;
- break;
- case DL_DEV_DRIVER_BOUND:
- link->status = DL_STATE_ACTIVE;
- break;
- default:
- link->status = DL_STATE_AVAILABLE;
- break;
- }
- break;
- case DL_DEV_UNBINDING:
- link->status = DL_STATE_SUPPLIER_UNBIND;
- break;
- default:
- link->status = DL_STATE_DORMANT;
- break;
- }
- }
+ else
+ device_link_init_status(link, consumer, supplier);
/*
* Some callers expect the link creation during consumer driver probe to
@@ -528,7 +542,7 @@ static void device_links_missing_supplie
* mark the link as "consumer probe in progress" to make the supplier removal
* wait for us to complete (or bad things may happen).
*
- * Links with the DL_FLAG_STATELESS flag set are ignored.
+ * Links with the DL_FLAG_MANAGED flag unset are ignored.
*/
int device_links_check_suppliers(struct device *dev)
{
@@ -538,7 +552,7 @@ int device_links_check_suppliers(struct
device_links_write_lock();
list_for_each_entry(link, &dev->links.suppliers, c_node) {
- if (link->flags & DL_FLAG_STATELESS)
+ if (!(link->flags & DL_FLAG_MANAGED))
continue;
if (link->status != DL_STATE_AVAILABLE) {
@@ -563,7 +577,7 @@ int device_links_check_suppliers(struct
*
* Also change the status of @dev's links to suppliers to "active".
*
- * Links with the DL_FLAG_STATELESS flag set are ignored.
+ * Links with the DL_FLAG_MANAGED flag unset are ignored.
*/
void device_links_driver_bound(struct device *dev)
{
@@ -572,7 +586,7 @@ void device_links_driver_bound(struct de
device_links_write_lock();
list_for_each_entry(link, &dev->links.consumers, s_node) {
- if (link->flags & DL_FLAG_STATELESS)
+ if (!(link->flags & DL_FLAG_MANAGED))
continue;
/*
@@ -593,7 +607,7 @@ void device_links_driver_bound(struct de
}
list_for_each_entry(link, &dev->links.suppliers, c_node) {
- if (link->flags & DL_FLAG_STATELESS)
+ if (!(link->flags & DL_FLAG_MANAGED))
continue;
WARN_ON(link->status != DL_STATE_CONSUMER_PROBE);
@@ -605,6 +619,13 @@ void device_links_driver_bound(struct de
device_links_write_unlock();
}
+static void device_link_drop_managed(struct device_link *link)
+{
+ link->flags &= ~DL_FLAG_MANAGED;
+ WRITE_ONCE(link->status, DL_STATE_NONE);
+ kref_put(&link->kref, __device_link_del);
+}
+
/**
* __device_links_no_driver - Update links of a device without a driver.
* @dev: Device without a drvier.
@@ -615,18 +636,18 @@ void device_links_driver_bound(struct de
* unless they already are in the "supplier unbind in progress" state in which
* case they need not be updated.
*
- * Links with the DL_FLAG_STATELESS flag set are ignored.
+ * Links with the DL_FLAG_MANAGED flag unset are ignored.
*/
static void __device_links_no_driver(struct device *dev)
{
struct device_link *link, *ln;
list_for_each_entry_safe_reverse(link, ln, &dev->links.suppliers, c_node) {
- if (link->flags & DL_FLAG_STATELESS)
+ if (!(link->flags & DL_FLAG_MANAGED))
continue;
if (link->flags & DL_FLAG_AUTOREMOVE_CONSUMER)
- __device_link_del(&link->kref);
+ device_link_drop_managed(link);
else if (link->status == DL_STATE_CONSUMER_PROBE ||
link->status == DL_STATE_ACTIVE)
WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
@@ -643,7 +664,7 @@ static void __device_links_no_driver(str
* %__device_links_no_driver() to update links to suppliers for it as
* appropriate.
*
- * Links with the DL_FLAG_STATELESS flag set are ignored.
+ * Links with the DL_FLAG_MANAGED flag unset are ignored.
*/
void device_links_no_driver(struct device *dev)
{
@@ -652,7 +673,7 @@ void device_links_no_driver(struct devic
device_links_write_lock();
list_for_each_entry(link, &dev->links.consumers, s_node) {
- if (link->flags & DL_FLAG_STATELESS)
+ if (!(link->flags & DL_FLAG_MANAGED))
continue;
/*
@@ -680,7 +701,7 @@ void device_links_no_driver(struct devic
* invoke %__device_links_no_driver() to update links to suppliers for it as
* appropriate.
*
- * Links with the DL_FLAG_STATELESS flag set are ignored.
+ * Links with the DL_FLAG_MANAGED flag unset are ignored.
*/
void device_links_driver_cleanup(struct device *dev)
{
@@ -689,7 +710,7 @@ void device_links_driver_cleanup(struct
device_links_write_lock();
list_for_each_entry_safe(link, ln, &dev->links.consumers, s_node) {
- if (link->flags & DL_FLAG_STATELESS)
+ if (!(link->flags & DL_FLAG_MANAGED))
continue;
WARN_ON(link->flags & DL_FLAG_AUTOREMOVE_CONSUMER);
@@ -702,7 +723,7 @@ void device_links_driver_cleanup(struct
*/
if (link->status == DL_STATE_SUPPLIER_UNBIND &&
link->flags & DL_FLAG_AUTOREMOVE_SUPPLIER)
- __device_link_del(&link->kref);
+ device_link_drop_managed(link);
WRITE_ONCE(link->status, DL_STATE_DORMANT);
}
@@ -724,7 +745,7 @@ void device_links_driver_cleanup(struct
*
* Return 'false' if there are no probing or active consumers.
*
- * Links with the DL_FLAG_STATELESS flag set are ignored.
+ * Links with the DL_FLAG_MANAGED flag unset are ignored.
*/
bool device_links_busy(struct device *dev)
{
@@ -734,7 +755,7 @@ bool device_links_busy(struct device *de
device_links_write_lock();
list_for_each_entry(link, &dev->links.consumers, s_node) {
- if (link->flags & DL_FLAG_STATELESS)
+ if (!(link->flags & DL_FLAG_MANAGED))
continue;
if (link->status == DL_STATE_CONSUMER_PROBE
@@ -764,7 +785,7 @@ bool device_links_busy(struct device *de
* driver to unbind and start over (the consumer will not re-probe as we have
* changed the state of the link already).
*
- * Links with the DL_FLAG_STATELESS flag set are ignored.
+ * Links with the DL_FLAG_MANAGED flag unset are ignored.
*/
void device_links_unbind_consumers(struct device *dev)
{
@@ -776,7 +797,7 @@ void device_links_unbind_consumers(struc
list_for_each_entry(link, &dev->links.consumers, s_node) {
enum device_link_state status;
- if (link->flags & DL_FLAG_STATELESS)
+ if (!(link->flags & DL_FLAG_MANAGED))
continue;
status = link->status;
Index: linux-pm/drivers/base/power/runtime.c
===================================================================
--- linux-pm.orig/drivers/base/power/runtime.c
+++ linux-pm/drivers/base/power/runtime.c
@@ -1624,7 +1624,7 @@ void pm_runtime_remove(struct device *de
* runtime PM references to the device, drop the usage counter of the device
* (as many times as needed).
*
- * Links with the DL_FLAG_STATELESS flag set are ignored.
+ * Links with the DL_FLAG_MANAGED flag unset are ignored.
*
* Since the device is guaranteed to be runtime-active at the point this is
* called, nothing else needs to be done here.
@@ -1641,7 +1641,7 @@ void pm_runtime_clean_up_links(struct de
idx = device_links_read_lock();
list_for_each_entry_rcu(link, &dev->links.consumers, s_node) {
- if (link->flags & DL_FLAG_STATELESS)
+ if (!(link->flags & DL_FLAG_MANAGED))
continue;
while (refcount_dec_not_one(&link->rpm_active))
Index: linux-pm/include/linux/device.h
===================================================================
--- linux-pm.orig/include/linux/device.h
+++ linux-pm/include/linux/device.h
@@ -829,12 +829,13 @@ enum device_link_state {
/*
* Device link flags.
*
- * STATELESS: The core won't track the presence of supplier/consumer drivers.
+ * STATELESS: The core will not remove this link automatically.
* AUTOREMOVE_CONSUMER: Remove the link automatically on consumer driver unbind.
* PM_RUNTIME: If set, the runtime PM framework will use this link.
* RPM_ACTIVE: Run pm_runtime_get_sync() on the supplier during link creation.
* AUTOREMOVE_SUPPLIER: Remove the link automatically on supplier driver unbind.
* AUTOPROBE_CONSUMER: Probe consumer driver automatically after supplier binds.
+ * MANAGED: The core tracks presence of supplier/consumer drivers (internal).
*/
#define DL_FLAG_STATELESS BIT(0)
#define DL_FLAG_AUTOREMOVE_CONSUMER BIT(1)
@@ -842,6 +843,7 @@ enum device_link_state {
#define DL_FLAG_RPM_ACTIVE BIT(3)
#define DL_FLAG_AUTOREMOVE_SUPPLIER BIT(4)
#define DL_FLAG_AUTOPROBE_CONSUMER BIT(5)
+#define DL_FLAG_MANAGED BIT(6)
/**
* struct device_link - Device link representation.
Index: linux-pm/Documentation/driver-api/device_link.rst
===================================================================
--- linux-pm.orig/Documentation/driver-api/device_link.rst
+++ linux-pm/Documentation/driver-api/device_link.rst
@@ -78,8 +78,8 @@ typically deleted in its ``->remove`` ca
driver is compiled as a module, the device link is added on module load and
orderly deleted on unload. The same restrictions that apply to device link
addition (e.g. exclusion of a parallel suspend/resume transition) apply equally
-to deletion. Device links with ``DL_FLAG_STATELESS`` unset (i.e. managed
-device links) are deleted automatically by the driver core.
+to deletion. Device links managed by the driver core are deleted automatically
+by it.
Several flags may be specified on device link addition, two of which
have already been mentioned above: ``DL_FLAG_STATELESS`` to express that no
^ permalink raw reply
* Re: [PATCH 1/3] opp: core: add regulators enable and disable
From: Kamil Konieczny @ 2019-07-10 10:16 UTC (permalink / raw)
To: Viresh Kumar
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
Krzysztof Kozlowski, Kukjin Kim, Kyungmin Park, Mark Rutland,
MyungJoo Ham, Nishanth Menon, Rob Herring, Stephen Boyd,
Viresh Kumar, devicetree, linux-arm-kernel, linux-kernel,
linux-pm, linux-samsung-soc
In-Reply-To: <20190709054014.o3g4e6gbovrq3vvn@vireshk-i7>
On 09.07.2019 07:40, Viresh Kumar wrote:
> On 08-07-19, 16:11, k.konieczny@partner.samsung.com wrote:
>> From: Kamil Konieczny <k.konieczny@partner.samsung.com>
>>
>> Add enable regulators to dev_pm_opp_set_regulators() and disable
>> regulators to dev_pm_opp_put_regulators(). This prepares for
>> converting exynos-bus devfreq driver to use dev_pm_opp_set_rate().
>>
>> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
>> ---
>> drivers/opp/core.c | 13 +++++++++++++
>> 1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index 0e7703fe733f..947cac452854 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -1580,8 +1580,19 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev,
>> if (ret)
>> goto free_regulators;
>>
>> + for (i = 0; i < opp_table->regulator_count; i++) {
>> + ret = regulator_enable(opp_table->regulators[i]);
>> + if (ret < 0)
>> + goto disable;
>> + }
>
> I am wondering on why is this really required as this isn't done for
> any other platform, probably because the regulators are enabled by
> bootloader and are always on.
It was not enabled for historical reasons, from design point regualtors
should be enabled before use.
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
^ permalink raw reply
* [PATCH] thermal: Fix use-after-free when unregistering thermal zone device
From: Ido Schimmel @ 2019-07-10 10:14 UTC (permalink / raw)
To: linux-pm
Cc: rui.zhang, edubezval, daniel.lezcano, jiri, vadimp, mlxsw,
Ido Schimmel
From: Ido Schimmel <idosch@mellanox.com>
thermal_zone_device_unregister() cancels the delayed work that polls the
thermal zone, but it does not wait for it to finish. This is racy with
respect to the freeing of the thermal zone device, which can result in a
use-after-free [1].
Fix this by waiting for the delayed work to finish before freeing the
thermal zone device. Note that thermal_zone_device_set_polling() is
never invoked from an atomic context, so it is safe to call
cancel_delayed_work_sync() that can block.
[1]
[ +0.002221] ==================================================================
[ +0.000064] BUG: KASAN: use-after-free in __mutex_lock+0x1076/0x11c0
[ +0.000016] Read of size 8 at addr ffff8881e48e0450 by task kworker/1:0/17
[ +0.000023] CPU: 1 PID: 17 Comm: kworker/1:0 Not tainted 5.2.0-rc6-custom-02495-g8e73ca3be4af #1701
[ +0.000010] Hardware name: Mellanox Technologies Ltd. MSN2100-CB2FO/SA001017, BIOS 5.6.5 06/07/2016
[ +0.000016] Workqueue: events_freezable_power_ thermal_zone_device_check
[ +0.000012] Call Trace:
[ +0.000021] dump_stack+0xa9/0x10e
[ +0.000020] print_address_description.cold.2+0x9/0x25e
[ +0.000018] __kasan_report.cold.3+0x78/0x9d
[ +0.000016] kasan_report+0xe/0x20
[ +0.000016] __mutex_lock+0x1076/0x11c0
[ +0.000014] step_wise_throttle+0x72/0x150
[ +0.000018] handle_thermal_trip+0x167/0x760
[ +0.000019] thermal_zone_device_update+0x19e/0x5f0
[ +0.000019] process_one_work+0x969/0x16f0
[ +0.000017] worker_thread+0x91/0xc40
[ +0.000014] kthread+0x33d/0x400
[ +0.000015] ret_from_fork+0x3a/0x50
[ +0.000020] Allocated by task 1:
[ +0.000015] save_stack+0x19/0x80
[ +0.000015] __kasan_kmalloc.constprop.4+0xc1/0xd0
[ +0.000014] kmem_cache_alloc_trace+0x152/0x320
[ +0.000015] thermal_zone_device_register+0x1b4/0x13a0
[ +0.000015] mlxsw_thermal_init+0xc92/0x23d0
[ +0.000014] __mlxsw_core_bus_device_register+0x659/0x11b0
[ +0.000013] mlxsw_core_bus_device_register+0x3d/0x90
[ +0.000013] mlxsw_pci_probe+0x355/0x4b0
[ +0.000014] local_pci_probe+0xc3/0x150
[ +0.000013] pci_device_probe+0x280/0x410
[ +0.000013] really_probe+0x26a/0xbb0
[ +0.000013] driver_probe_device+0x208/0x2e0
[ +0.000013] device_driver_attach+0xfe/0x140
[ +0.000013] __driver_attach+0x110/0x310
[ +0.000013] bus_for_each_dev+0x14b/0x1d0
[ +0.000013] driver_register+0x1c0/0x400
[ +0.000015] mlxsw_sp_module_init+0x5d/0xd3
[ +0.000014] do_one_initcall+0x239/0x4dd
[ +0.000013] kernel_init_freeable+0x42b/0x4e8
[ +0.000012] kernel_init+0x11/0x18b
[ +0.000013] ret_from_fork+0x3a/0x50
[ +0.000015] Freed by task 581:
[ +0.000013] save_stack+0x19/0x80
[ +0.000014] __kasan_slab_free+0x125/0x170
[ +0.000013] kfree+0xf3/0x310
[ +0.000013] thermal_release+0xc7/0xf0
[ +0.000014] device_release+0x77/0x200
[ +0.000014] kobject_put+0x1a8/0x4c0
[ +0.000014] device_unregister+0x38/0xc0
[ +0.000014] thermal_zone_device_unregister+0x54e/0x6a0
[ +0.000014] mlxsw_thermal_fini+0x184/0x35a
[ +0.000014] mlxsw_core_bus_device_unregister+0x10a/0x640
[ +0.000013] mlxsw_devlink_core_bus_device_reload+0x92/0x210
[ +0.000015] devlink_nl_cmd_reload+0x113/0x1f0
[ +0.000014] genl_family_rcv_msg+0x700/0xee0
[ +0.000013] genl_rcv_msg+0xca/0x170
[ +0.000013] netlink_rcv_skb+0x137/0x3a0
[ +0.000012] genl_rcv+0x29/0x40
[ +0.000013] netlink_unicast+0x49b/0x660
[ +0.000013] netlink_sendmsg+0x755/0xc90
[ +0.000013] __sys_sendto+0x3de/0x430
[ +0.000013] __x64_sys_sendto+0xe2/0x1b0
[ +0.000013] do_syscall_64+0xa4/0x4d0
[ +0.000013] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ +0.000017] The buggy address belongs to the object at ffff8881e48e0008
which belongs to the cache kmalloc-2k of size 2048
[ +0.000012] The buggy address is located 1096 bytes inside of
2048-byte region [ffff8881e48e0008, ffff8881e48e0808)
[ +0.000007] The buggy address belongs to the page:
[ +0.000012] page:ffffea0007923800 refcount:1 mapcount:0 mapping:ffff88823680d0c0 index:0x0 compound_mapcount: 0
[ +0.000020] flags: 0x200000000010200(slab|head)
[ +0.000019] raw: 0200000000010200 ffffea0007682008 ffffea00076ab808 ffff88823680d0c0
[ +0.000016] raw: 0000000000000000 00000000000d000d 00000001ffffffff 0000000000000000
[ +0.000007] page dumped because: kasan: bad access detected
[ +0.000012] Memory state around the buggy address:
[ +0.000012] ffff8881e48e0300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000012] ffff8881e48e0380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000012] >ffff8881e48e0400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000008] ^
[ +0.000012] ffff8881e48e0480: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000012] ffff8881e48e0500: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ +0.000007] ==================================================================
Fixes: b1569e99c795 ("ACPI: move thermal trip handling to generic thermal layer")
Reported-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/thermal/thermal_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 46cfb7de4eb2..f87f462c60c1 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -298,7 +298,7 @@ static void thermal_zone_device_set_polling(struct thermal_zone_device *tz,
&tz->poll_queue,
msecs_to_jiffies(delay));
else
- cancel_delayed_work(&tz->poll_queue);
+ cancel_delayed_work_sync(&tz->poll_queue);
}
static void monitor_thermal_zone(struct thermal_zone_device *tz)
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 0/3] add coupled regulators for Exynos5422/5800
From: Krzysztof Kozlowski @ 2019-07-10 10:14 UTC (permalink / raw)
To: Kamil Konieczny
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
Kukjin Kim, Kyungmin Park, Mark Rutland, MyungJoo Ham,
Nishanth Menon, Rob Herring, Stephen Boyd, Viresh Kumar,
devicetree, linux-arm-kernel, linux-kernel, linux-pm,
linux-samsung-soc@vger.kernel.org
In-Reply-To: <91f65527-3440-90fd-4096-5824fba1df78@partner.samsung.com>
On Wed, 10 Jul 2019 at 12:03, Kamil Konieczny
<k.konieczny@partner.samsung.com> wrote:
>
> On 10.07.2019 11:00, Krzysztof Kozlowski wrote:
> > On Mon, 8 Jul 2019 at 16:12, <k.konieczny@partner.samsung.com> wrote:
> >>
> >> From: Kamil Konieczny <k.konieczny@partner.samsung.com>
> >>
> >> Hi,
> >>
> >> The main purpose of this patch series is to add coupled regulators for
> >> Exynos5422/5800 to keep constrain on voltage difference between vdd_arm
> >> and vdd_int to be at most 300mV. In exynos-bus instead of using
> >> regulator_set_voltage_tol() with default voltage tolerance it should be
> >> used regulator_set_voltage_triplet() with volatege range, and this is
> >> already present in opp/core.c code, so it can be reused. While at this,
> >> move setting regulators into opp/core.
> >>
> >> This patchset was tested on Odroid XU3.
> >>
> >> The last patch depends on two previous.
> >
> > So you break the ABI... I assume that patchset maintains
> > bisectability. However there is no explanation why ABI break is needed
> > so this does not look good...
>
> Patchset is bisectable, first one is simple and do not depends on others,
> second depends on first, last depends on first and second.
>
> What do you mean by breaking ABI ?
I mean, that Linux kernel stops working with existing DTBs... or am I
mistaken and there is no problem? Maybe I confused the order...
Best regards,
Krzysztof
^ permalink raw reply
* [RFC PATCH] PM: QoS: Get rid of unused flags
From: Amit Kucheria @ 2019-07-10 10:12 UTC (permalink / raw)
To: linux-kernel, Rafael J. Wysocki, Len Brown, Pavel Machek; +Cc: linux-pm
In-Reply-To: <cover.1562751312.git.amit.kucheria@linaro.org>
The network_latency and network_throughput flags for PM-QoS have not
found much use in drivers or in userspace since they were introduced.
Commit 4a733ef1bea7 ("mac80211: remove PM-QoS listener") removed the
only user PM_QOS_NETWORK_LATENCY in the kernel a while ago and there
don't seem to be any userspace tools using the character device files
either.
PM_QOS_MEMORY_BANDWIDTH was never even added to the trace events.
Remove all the flags except cpu_dma_latency.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
---
I've looked around for use of /dev/network_throughput and
/dev/network_bandwidth) and not found any userspace programs that seem to
use this currently. So this shouldn't be breaking our ABI contract with
userspace.
Documentation/power/pm_qos_interface.txt | 5 +--
include/linux/pm_qos.h | 6 ---
include/trace/events/power.h | 8 +---
kernel/power/qos.c | 48 ------------------------
4 files changed, 4 insertions(+), 63 deletions(-)
diff --git a/Documentation/power/pm_qos_interface.txt b/Documentation/power/pm_qos_interface.txt
index 19c5f7b1a7bab..ac53ca4ae24e1 100644
--- a/Documentation/power/pm_qos_interface.txt
+++ b/Documentation/power/pm_qos_interface.txt
@@ -5,8 +5,7 @@ performance expectations by drivers, subsystems and user space applications on
one of the parameters.
Two different PM QoS frameworks are available:
-1. PM QoS classes for cpu_dma_latency, network_latency, network_throughput,
-memory_bandwidth.
+1. PM QoS classes for cpu_dma_latency
2. the per-device PM QoS framework provides the API to manage the per-device latency
constraints and PM QoS flags.
@@ -74,7 +73,7 @@ cleanup of a process, the interface requires the process to register its
parameter requests in the following way:
To register the default pm_qos target for the specific parameter, the process
-must open one of /dev/[cpu_dma_latency, network_latency, network_throughput]
+must open /dev/cpu_dma_latency
As long as the device node is held open that process has a registered
request on the parameter.
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 6ea1ae373d77c..2a3c237b1910d 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -13,9 +13,6 @@
enum {
PM_QOS_RESERVED = 0,
PM_QOS_CPU_DMA_LATENCY,
- PM_QOS_NETWORK_LATENCY,
- PM_QOS_NETWORK_THROUGHPUT,
- PM_QOS_MEMORY_BANDWIDTH,
/* insert new class ID */
PM_QOS_NUM_CLASSES,
@@ -33,9 +30,6 @@ enum pm_qos_flags_status {
#define PM_QOS_LATENCY_ANY_NS ((s64)PM_QOS_LATENCY_ANY * NSEC_PER_USEC)
#define PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC)
-#define PM_QOS_NETWORK_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC)
-#define PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE 0
-#define PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE 0
#define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE PM_QOS_LATENCY_ANY
#define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT PM_QOS_LATENCY_ANY
#define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS PM_QOS_LATENCY_ANY_NS
diff --git a/include/trace/events/power.h b/include/trace/events/power.h
index f7aece721aed7..7457e238e1b74 100644
--- a/include/trace/events/power.h
+++ b/include/trace/events/power.h
@@ -379,9 +379,7 @@ DECLARE_EVENT_CLASS(pm_qos_request,
TP_printk("pm_qos_class=%s value=%d",
__print_symbolic(__entry->pm_qos_class,
- { PM_QOS_CPU_DMA_LATENCY, "CPU_DMA_LATENCY" },
- { PM_QOS_NETWORK_LATENCY, "NETWORK_LATENCY" },
- { PM_QOS_NETWORK_THROUGHPUT, "NETWORK_THROUGHPUT" }),
+ { PM_QOS_CPU_DMA_LATENCY, "CPU_DMA_LATENCY" }),
__entry->value)
);
@@ -426,9 +424,7 @@ TRACE_EVENT(pm_qos_update_request_timeout,
TP_printk("pm_qos_class=%s value=%d, timeout_us=%ld",
__print_symbolic(__entry->pm_qos_class,
- { PM_QOS_CPU_DMA_LATENCY, "CPU_DMA_LATENCY" },
- { PM_QOS_NETWORK_LATENCY, "NETWORK_LATENCY" },
- { PM_QOS_NETWORK_THROUGHPUT, "NETWORK_THROUGHPUT" }),
+ { PM_QOS_CPU_DMA_LATENCY, "CPU_DMA_LATENCY" }),
__entry->value, __entry->timeout_us)
);
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 33e3febaba53f..9568a2fe7c116 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -78,57 +78,9 @@ static struct pm_qos_object cpu_dma_pm_qos = {
.name = "cpu_dma_latency",
};
-static BLOCKING_NOTIFIER_HEAD(network_lat_notifier);
-static struct pm_qos_constraints network_lat_constraints = {
- .list = PLIST_HEAD_INIT(network_lat_constraints.list),
- .target_value = PM_QOS_NETWORK_LAT_DEFAULT_VALUE,
- .default_value = PM_QOS_NETWORK_LAT_DEFAULT_VALUE,
- .no_constraint_value = PM_QOS_NETWORK_LAT_DEFAULT_VALUE,
- .type = PM_QOS_MIN,
- .notifiers = &network_lat_notifier,
-};
-static struct pm_qos_object network_lat_pm_qos = {
- .constraints = &network_lat_constraints,
- .name = "network_latency",
-};
-
-
-static BLOCKING_NOTIFIER_HEAD(network_throughput_notifier);
-static struct pm_qos_constraints network_tput_constraints = {
- .list = PLIST_HEAD_INIT(network_tput_constraints.list),
- .target_value = PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE,
- .default_value = PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE,
- .no_constraint_value = PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE,
- .type = PM_QOS_MAX,
- .notifiers = &network_throughput_notifier,
-};
-static struct pm_qos_object network_throughput_pm_qos = {
- .constraints = &network_tput_constraints,
- .name = "network_throughput",
-};
-
-
-static BLOCKING_NOTIFIER_HEAD(memory_bandwidth_notifier);
-static struct pm_qos_constraints memory_bw_constraints = {
- .list = PLIST_HEAD_INIT(memory_bw_constraints.list),
- .target_value = PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE,
- .default_value = PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE,
- .no_constraint_value = PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE,
- .type = PM_QOS_SUM,
- .notifiers = &memory_bandwidth_notifier,
-};
-static struct pm_qos_object memory_bandwidth_pm_qos = {
- .constraints = &memory_bw_constraints,
- .name = "memory_bandwidth",
-};
-
-
static struct pm_qos_object *pm_qos_array[] = {
&null_pm_qos,
&cpu_dma_pm_qos,
- &network_lat_pm_qos,
- &network_throughput_pm_qos,
- &memory_bandwidth_pm_qos,
};
static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] cpufreq: imx-cpufreq-dt: Add i.MX8MN support
From: Rafael J. Wysocki @ 2019-07-10 10:11 UTC (permalink / raw)
To: Viresh Kumar
Cc: Anson.Huang, shawnguo, s.hauer, kernel, festevam, linux-pm,
linux-arm-kernel, linux-kernel, Linux-imx
In-Reply-To: <20190708035057.h2lgadm56tgdqsor@vireshk-i7>
On Monday, July 8, 2019 5:50:57 AM CEST Viresh Kumar wrote:
> On 08-07-19, 11:03, Anson.Huang@nxp.com wrote:
> > From: Anson Huang <Anson.Huang@nxp.com>
> >
> > i.MX8MN is a new SoC of i.MX8M series, it also uses speed
> > grading and market segment fuses for OPP definitions, add
> > support for this SoC.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > drivers/cpufreq/imx-cpufreq-dt.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/cpufreq/imx-cpufreq-dt.c b/drivers/cpufreq/imx-cpufreq-dt.c
> > index b54fd26..4f85f31 100644
> > --- a/drivers/cpufreq/imx-cpufreq-dt.c
> > +++ b/drivers/cpufreq/imx-cpufreq-dt.c
> > @@ -44,10 +44,11 @@ static int imx_cpufreq_dt_probe(struct platform_device *pdev)
> > * According to datasheet minimum speed grading is not supported for
> > * consumer parts so clamp to 1 to avoid warning for "no OPPs"
> > *
> > - * Applies to 8mq and 8mm.
> > + * Applies to i.MX8M series SoCs.
> > */
> > if (mkt_segment == 0 && speed_grade == 0 && (
> > of_machine_is_compatible("fsl,imx8mm") ||
> > + of_machine_is_compatible("fsl,imx8mn") ||
> > of_machine_is_compatible("fsl,imx8mq")))
> > speed_grade = 1;
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>
> @Rafael: Can you pick this one directly, no point sending another pull request
> for just one patch. Thanks.
Done, thanks!
^ permalink raw reply
* Re: [PATCH v5] cpufreq/pasemi: fix an use-after-free in pas_cpufreq_cpu_init()
From: Rafael J. Wysocki @ 2019-07-10 10:07 UTC (permalink / raw)
To: Viresh Kumar, Wen Yang
Cc: linux-kernel, xue.zhihong, wang.yi59, cheng.shengyu,
Michael Ellerman, linuxppc-dev, linux-pm
In-Reply-To: <20190709081205.a5sjsqo2el6zt24b@vireshk-i7>
On Tuesday, July 9, 2019 10:12:05 AM CEST Viresh Kumar wrote:
> On 09-07-19, 16:04, Wen Yang wrote:
> > The cpu variable is still being used in the of_get_property() call
> > after the of_node_put() call, which may result in use-after-free.
> >
> > Fixes: a9acc26b75f ("cpufreq/pasemi: fix possible object reference leak")
> > Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > Cc: Viresh Kumar <viresh.kumar@linaro.org>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: linux-pm@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
Patch applied.
> > ---
> > v5: put together the code to get, use, and release cpu device_node.
> > v4: restore the blank line.
> > v3: fix a leaked reference.
> > v2: clean up the code according to the advice of viresh.
> >
> > drivers/cpufreq/pasemi-cpufreq.c | 21 +++++++++------------
> > 1 file changed, 9 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c
> > index 6b1e4ab..1f0beb7 100644
> > --- a/drivers/cpufreq/pasemi-cpufreq.c
> > +++ b/drivers/cpufreq/pasemi-cpufreq.c
> > @@ -131,10 +131,17 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
> > int err = -ENODEV;
> >
> > cpu = of_get_cpu_node(policy->cpu, NULL);
> > -
> > - of_node_put(cpu);
> > if (!cpu)
> > goto out;
>
> I would have loved a blank line here :)
And I added the blank line.
> > + max_freqp = of_get_property(cpu, "clock-frequency", NULL);
> > + of_node_put(cpu);
> > + if (!max_freqp) {
> > + err = -EINVAL;
> > + goto out;
> > + }
> > +
> > + /* we need the freq in kHz */
> > + max_freq = *max_freqp / 1000;
> >
> > dn = of_find_compatible_node(NULL, NULL, "1682m-sdc");
> > if (!dn)
> > @@ -171,16 +178,6 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy *policy)
> > }
> >
> > pr_debug("init cpufreq on CPU %d\n", policy->cpu);
> > -
> > - max_freqp = of_get_property(cpu, "clock-frequency", NULL);
> > - if (!max_freqp) {
> > - err = -EINVAL;
> > - goto out_unmap_sdcpwr;
> > - }
> > -
> > - /* we need the freq in kHz */
> > - max_freq = *max_freqp / 1000;
> > -
> > pr_debug("max clock-frequency is at %u kHz\n", max_freq);
> > pr_debug("initializing frequency table\n");
>
> Though, enough versions have happened now.
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>
>
Thanks!
^ permalink raw reply
* Re: [PATCH 0/3] add coupled regulators for Exynos5422/5800
From: Kamil Konieczny @ 2019-07-10 10:03 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Bartlomiej Zolnierkiewicz, Marek Szyprowski, Chanwoo Choi,
Kukjin Kim, Kyungmin Park, Mark Rutland, MyungJoo Ham,
Nishanth Menon, Rob Herring, Stephen Boyd, Viresh Kumar,
devicetree, linux-arm-kernel, linux-kernel, linux-pm,
linux-samsung-soc@vger.kernel.org
In-Reply-To: <CAJKOXPd+UZ2MdrTVfBv5UYzK5LgKNQHUFzRbRNeq271EaDSchg@mail.gmail.com>
On 10.07.2019 11:00, Krzysztof Kozlowski wrote:
> On Mon, 8 Jul 2019 at 16:12, <k.konieczny@partner.samsung.com> wrote:
>>
>> From: Kamil Konieczny <k.konieczny@partner.samsung.com>
>>
>> Hi,
>>
>> The main purpose of this patch series is to add coupled regulators for
>> Exynos5422/5800 to keep constrain on voltage difference between vdd_arm
>> and vdd_int to be at most 300mV. In exynos-bus instead of using
>> regulator_set_voltage_tol() with default voltage tolerance it should be
>> used regulator_set_voltage_triplet() with volatege range, and this is
>> already present in opp/core.c code, so it can be reused. While at this,
>> move setting regulators into opp/core.
>>
>> This patchset was tested on Odroid XU3.
>>
>> The last patch depends on two previous.
>
> So you break the ABI... I assume that patchset maintains
> bisectability. However there is no explanation why ABI break is needed
> so this does not look good...
Patchset is bisectable, first one is simple and do not depends on others,
second depends on first, last depends on first and second.
What do you mean by breaking ABI ?
> Best regards,
> Krzysztof
>
>>
>> Regards,
>> Kamil
>>
>> Kamil Konieczny (2):
>> opp: core: add regulators enable and disable
>> devfreq: exynos-bus: convert to use dev_pm_opp_set_rate()
>>
>> Marek Szyprowski (1):
>> ARM: dts: exynos: add initial data for coupled regulators for
>> Exynos5422/5800
>>
>> arch/arm/boot/dts/exynos5420.dtsi | 34 ++--
>> arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 4 +
>> arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 +
>> arch/arm/boot/dts/exynos5800.dtsi | 32 ++--
>> drivers/devfreq/exynos-bus.c | 172 +++++++-----------
>> drivers/opp/core.c | 13 ++
>> 6 files changed, 120 insertions(+), 139 deletions(-)
>>
>> --
>> 2.22.0
--
Best regards,
Kamil Konieczny
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [RESEND, PATCH v4 0/2] cpufreq: Add sunxi nvmem based CPU scaling driver
From: Viresh Kumar @ 2019-07-10 9:15 UTC (permalink / raw)
To: Yangtao Li
Cc: vireshk, nm, sboyd, robh+dt, mark.rutland, maxime.ripard, wens,
rjw, davem, mchehab+samsung, gregkh, linus.walleij, nicolas.ferre,
paulmck, linux-pm, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20190612162816.31713-1-tiny.windzz@gmail.com>
On 12-06-19, 12:28, Yangtao Li wrote:
> Add sunxi nvmem based CPU scaling driver, refers to qcom-cpufreq-kryo.
>
> Yangtao Li (2):
> cpufreq: Add sun50i nvmem based CPU scaling driver
> dt-bindings: cpufreq: Document allwinner,sun50i-h6-operating-points
>
> .../bindings/opp/sun50i-nvmem-cpufreq.txt | 167 +++++++++++++
> MAINTAINERS | 7 +
> drivers/cpufreq/Kconfig.arm | 12 +
> drivers/cpufreq/Makefile | 1 +
> drivers/cpufreq/cpufreq-dt-platdev.c | 2 +
> drivers/cpufreq/sun50i-cpufreq-nvmem.c | 226 ++++++++++++++++++
> 6 files changed, 415 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/opp/sun50i-nvmem-cpufreq.txt
> create mode 100644 drivers/cpufreq/sun50i-cpufreq-nvmem.c
>
> ---
> v4:
> -Remove sunxi_cpufreq_soc_data structure for now.
> -Convert to less generic name.
> -Update soc_bin xlate.
Applied. Thanks.
I will push it only after 5.3-rc1 is released.
--
viresh
^ permalink raw reply
* Re: [PATCH 3/3] ARM: dts: exynos: add initial data for coupled regulators for Exynos5422/5800
From: Krzysztof Kozlowski @ 2019-07-10 9:02 UTC (permalink / raw)
To: k.konieczny
Cc: Marek Szyprowski, Bartlomiej Zolnierkiewicz, Chanwoo Choi,
Kukjin Kim, Kyungmin Park, Mark Rutland, MyungJoo Ham,
Nishanth Menon, Rob Herring, Stephen Boyd, Viresh Kumar,
devicetree, linux-arm-kernel, linux-kernel, linux-pm,
linux-samsung-soc@vger.kernel.org
In-Reply-To: <20190708141140.24379-4-k.konieczny@partner.samsung.com>
On Mon, 8 Jul 2019 at 16:12, <k.konieczny@partner.samsung.com> wrote:
>
> From: Marek Szyprowski <m.szyprowski@samsung.com>
>
> Declare Exynos5422/5800 voltage ranges for opp points for big cpu core and
> bus wcore and couple their voltage supllies as vdd_arm and vdd_int should
> be in 300mV range.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Kamil Konieczny <k.konieczny@partner.samsung.com>
> ---
> arch/arm/boot/dts/exynos5420.dtsi | 34 +++++++++----------
> arch/arm/boot/dts/exynos5422-odroid-core.dtsi | 4 +++
> arch/arm/boot/dts/exynos5800-peach-pi.dts | 4 +++
> arch/arm/boot/dts/exynos5800.dtsi | 32 ++++++++---------
> 4 files changed, 41 insertions(+), 33 deletions(-)
Looks good, I assume bisectability is not affected, because of
dependency on the driver changes I will take it for next next release
(v5.5). Assuming that driver change goes into v5.4.
Best regards,
Krzysztof
^ 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