* [PATCH v2 1/4] hwmon: amd_energy: Move label out of accumulation structure
2020-09-29 10:53 [PATCH v2 0/4] hwmon: improvements to amd_energy driver Naveen Krishna Chatradhi
@ 2020-09-29 10:53 ` Naveen Krishna Chatradhi
2020-09-30 5:03 ` Guenter Roeck
2020-09-29 10:53 ` [PATCH v2 2/4] hwmon: amd_energy: optimize accumulation interval Naveen Krishna Chatradhi
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Naveen Krishna Chatradhi @ 2020-09-29 10:53 UTC (permalink / raw)
To: linux-hwmon; +Cc: naveenkrishna.ch, Akshay Gupta
From: Akshay Gupta <Akshay.Gupta@amd.com>
At present, core & socket labels are defined in struct sensor_accumulator
This patch moves it to the amd_energy_data structure, which will
help in calling memset on struct sensor_accumulator to optimize the code.
Signed-off-by: Akshay Gupta <Akshay.Gupta@amd.com>
---
Changes since v1:
None
drivers/hwmon/amd_energy.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/amd_energy.c b/drivers/hwmon/amd_energy.c
index 29603742c858..9580a16185b8 100644
--- a/drivers/hwmon/amd_energy.c
+++ b/drivers/hwmon/amd_energy.c
@@ -35,7 +35,6 @@
struct sensor_accumulator {
u64 energy_ctr;
u64 prev_value;
- char label[10];
};
struct amd_energy_data {
@@ -52,6 +51,7 @@ struct amd_energy_data {
int nr_cpus;
int nr_socks;
int core_id;
+ char (*label)[10];
};
static int amd_energy_read_labels(struct device *dev,
@@ -61,7 +61,7 @@ static int amd_energy_read_labels(struct device *dev,
{
struct amd_energy_data *data = dev_get_drvdata(dev);
- *str = data->accums[channel].label;
+ *str = data->label[channel];
return 0;
}
@@ -253,6 +253,7 @@ static int amd_create_sensor(struct device *dev,
struct sensor_accumulator *accums;
int i, num_siblings, cpus, sockets;
u32 *s_config;
+ char (*label_l)[10];
/* Identify the number of siblings per core */
num_siblings = ((cpuid_ebx(0x8000001e) >> 8) & 0xff) + 1;
@@ -276,21 +277,25 @@ static int amd_create_sensor(struct device *dev,
if (!accums)
return -ENOMEM;
+ label_l = devm_kcalloc(dev, cpus + sockets,
+ sizeof(*label_l), GFP_KERNEL);
+ if (!label_l)
+ return -ENOMEM;
+
info->type = type;
info->config = s_config;
data->nr_cpus = cpus;
data->nr_socks = sockets;
data->accums = accums;
+ data->label = label_l;
for (i = 0; i < cpus + sockets; i++) {
s_config[i] = config;
if (i < cpus)
- scnprintf(accums[i].label, 10,
- "Ecore%03u", i);
+ scnprintf(label_l[i], 10, "Ecore%03u", i);
else
- scnprintf(accums[i].label, 10,
- "Esocket%u", (i - cpus));
+ scnprintf(label_l[i], 10, "Esocket%u", (i - cpus));
}
return 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/4] hwmon: amd_energy: optimize accumulation interval
2020-09-29 10:53 [PATCH v2 0/4] hwmon: improvements to amd_energy driver Naveen Krishna Chatradhi
2020-09-29 10:53 ` [PATCH v2 1/4] hwmon: amd_energy: Move label out of accumulation structure Naveen Krishna Chatradhi
@ 2020-09-29 10:53 ` Naveen Krishna Chatradhi
2020-09-29 10:53 ` [PATCH v2 3/4] hwmon: amd_energy: Improve the accumulation logic Naveen Krishna Chatradhi
2020-09-29 10:53 ` [PATCH v2 4/4] hwmon: (amd_energy) Update driver documentation Naveen Krishna Chatradhi
3 siblings, 0 replies; 6+ messages in thread
From: Naveen Krishna Chatradhi @ 2020-09-29 10:53 UTC (permalink / raw)
To: linux-hwmon; +Cc: naveenkrishna.ch, Naveen Krishna Chatradhi
On a system with course grain resolution of energy unit (milli J) the
accumulation thread can be executed less frequently than on the system
with fine grain resolution(micro J).
This patch sets the accumulation thread interval to an optimum value
calculated based on the (energy unit) resolution supported by the
hardware (assuming a peak wattage of 240W).
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
Changes since v1:
1. Generalized formula as suggested by Guenter.
2. Use milli seconds for interval
drivers/hwmon/amd_energy.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/hwmon/amd_energy.c b/drivers/hwmon/amd_energy.c
index 9580a16185b8..c413adfc6a73 100644
--- a/drivers/hwmon/amd_energy.c
+++ b/drivers/hwmon/amd_energy.c
@@ -46,8 +46,9 @@ struct amd_energy_data {
struct mutex lock;
/* An accumulator for each core and socket */
struct sensor_accumulator *accums;
+ unsigned int timeout_ms;
/* Energy Status Units */
- u64 energy_units;
+ int energy_units;
int nr_cpus;
int nr_socks;
int core_id;
@@ -215,6 +216,7 @@ static umode_t amd_energy_is_visible(const void *_data,
static int energy_accumulator(void *p)
{
struct amd_energy_data *data = (struct amd_energy_data *)p;
+ unsigned int timeout = data->timeout_ms;
while (!kthread_should_stop()) {
/*
@@ -227,14 +229,7 @@ static int energy_accumulator(void *p)
if (kthread_should_stop())
break;
- /*
- * On a 240W system, with default resolution the
- * Socket Energy status register may wrap around in
- * 2^32*15.3 e-6/240 = 273.8041 secs (~4.5 mins)
- *
- * let us accumulate for every 100secs
- */
- schedule_timeout(msecs_to_jiffies(100000));
+ schedule_timeout(msecs_to_jiffies(timeout));
}
return 0;
}
@@ -331,6 +326,13 @@ static int amd_energy_probe(struct platform_device *pdev)
if (IS_ERR(hwmon_dev))
return PTR_ERR(hwmon_dev);
+ /*
+ * On a system with peak wattage of 250W
+ * timeout = 2 ^ 32 / 2 ^ energy_units / 250 secs
+ */
+ data->timeout_ms = 1000 *
+ BIT(min(28, 31 - data->energy_units)) / 250;
+
data->wrap_accumulate = kthread_run(energy_accumulator, data,
"%s", dev_name(hwmon_dev));
if (IS_ERR(data->wrap_accumulate))
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/4] hwmon: amd_energy: Improve the accumulation logic
2020-09-29 10:53 [PATCH v2 0/4] hwmon: improvements to amd_energy driver Naveen Krishna Chatradhi
2020-09-29 10:53 ` [PATCH v2 1/4] hwmon: amd_energy: Move label out of accumulation structure Naveen Krishna Chatradhi
2020-09-29 10:53 ` [PATCH v2 2/4] hwmon: amd_energy: optimize accumulation interval Naveen Krishna Chatradhi
@ 2020-09-29 10:53 ` Naveen Krishna Chatradhi
2020-09-29 10:53 ` [PATCH v2 4/4] hwmon: (amd_energy) Update driver documentation Naveen Krishna Chatradhi
3 siblings, 0 replies; 6+ messages in thread
From: Naveen Krishna Chatradhi @ 2020-09-29 10:53 UTC (permalink / raw)
To: linux-hwmon; +Cc: naveenkrishna.ch, Naveen Krishna Chatradhi
Factor out the common code in the accumulation functions for core and
socket accumulation.
While at it, handle the return value of the amd_create_sensor() function.
Signed-off-by: Naveen Krishna Chatradhi <nchatrad@amd.com>
---
Changes since v1:
1. Return values are handled in the probe.
drivers/hwmon/amd_energy.c | 126 +++++++++++++------------------------
1 file changed, 45 insertions(+), 81 deletions(-)
diff --git a/drivers/hwmon/amd_energy.c b/drivers/hwmon/amd_energy.c
index c413adfc6a73..cbc6a6e466c5 100644
--- a/drivers/hwmon/amd_energy.c
+++ b/drivers/hwmon/amd_energy.c
@@ -74,108 +74,67 @@ static void get_energy_units(struct amd_energy_data *data)
data->energy_units = (rapl_units & AMD_ENERGY_UNIT_MASK) >> 8;
}
-static void accumulate_socket_delta(struct amd_energy_data *data,
- int sock, int cpu)
+static void accumulate_delta(struct amd_energy_data *data,
+ int channel, int cpu, u32 reg)
{
- struct sensor_accumulator *s_accum;
+ struct sensor_accumulator *accum;
u64 input;
mutex_lock(&data->lock);
- rdmsrl_safe_on_cpu(cpu, ENERGY_PKG_MSR, &input);
+ rdmsrl_safe_on_cpu(cpu, reg, &input);
input &= AMD_ENERGY_MASK;
- s_accum = &data->accums[data->nr_cpus + sock];
- if (input >= s_accum->prev_value)
- s_accum->energy_ctr +=
- input - s_accum->prev_value;
+ accum = &data->accums[channel];
+ if (input >= accum->prev_value)
+ accum->energy_ctr +=
+ input - accum->prev_value;
else
- s_accum->energy_ctr += UINT_MAX -
- s_accum->prev_value + input;
+ accum->energy_ctr += UINT_MAX -
+ accum->prev_value + input;
- s_accum->prev_value = input;
+ accum->prev_value = input;
mutex_unlock(&data->lock);
}
-static void accumulate_core_delta(struct amd_energy_data *data)
+static void read_accumulate(struct amd_energy_data *data)
{
- struct sensor_accumulator *c_accum;
- u64 input;
- int cpu;
+ int sock, scpu, cpu;
+
+ for (sock = 0; sock < data->nr_socks; sock++) {
+ scpu = cpumask_first_and(cpu_online_mask,
+ cpumask_of_node(sock));
+
+ accumulate_delta(data, data->nr_cpus + sock,
+ scpu, ENERGY_PKG_MSR);
+ }
- mutex_lock(&data->lock);
if (data->core_id >= data->nr_cpus)
data->core_id = 0;
cpu = data->core_id;
+ if (cpu_online(cpu))
+ accumulate_delta(data, cpu, cpu, ENERGY_CORE_MSR);
- if (!cpu_online(cpu))
- goto out;
-
- rdmsrl_safe_on_cpu(cpu, ENERGY_CORE_MSR, &input);
- input &= AMD_ENERGY_MASK;
-
- c_accum = &data->accums[cpu];
-
- if (input >= c_accum->prev_value)
- c_accum->energy_ctr +=
- input - c_accum->prev_value;
- else
- c_accum->energy_ctr += UINT_MAX -
- c_accum->prev_value + input;
-
- c_accum->prev_value = input;
-
-out:
data->core_id++;
- mutex_unlock(&data->lock);
-}
-
-static void read_accumulate(struct amd_energy_data *data)
-{
- int sock;
-
- for (sock = 0; sock < data->nr_socks; sock++) {
- int cpu;
-
- cpu = cpumask_first_and(cpu_online_mask,
- cpumask_of_node(sock));
-
- accumulate_socket_delta(data, sock, cpu);
- }
-
- accumulate_core_delta(data);
}
static void amd_add_delta(struct amd_energy_data *data, int ch,
- int cpu, long *val, bool is_core)
+ int cpu, long *val, u32 reg)
{
- struct sensor_accumulator *s_accum, *c_accum;
+ struct sensor_accumulator *accum;
u64 input;
mutex_lock(&data->lock);
- if (!is_core) {
- rdmsrl_safe_on_cpu(cpu, ENERGY_PKG_MSR, &input);
- input &= AMD_ENERGY_MASK;
-
- s_accum = &data->accums[ch];
- if (input >= s_accum->prev_value)
- input += s_accum->energy_ctr -
- s_accum->prev_value;
- else
- input += UINT_MAX - s_accum->prev_value +
- s_accum->energy_ctr;
- } else {
- rdmsrl_safe_on_cpu(cpu, ENERGY_CORE_MSR, &input);
- input &= AMD_ENERGY_MASK;
+ rdmsrl_safe_on_cpu(cpu, reg, &input);
+ input &= AMD_ENERGY_MASK;
- c_accum = &data->accums[ch];
- if (input >= c_accum->prev_value)
- input += c_accum->energy_ctr -
- c_accum->prev_value;
- else
- input += UINT_MAX - c_accum->prev_value +
- c_accum->energy_ctr;
- }
+ accum = &data->accums[ch];
+ if (input >= accum->prev_value)
+ input += accum->energy_ctr -
+ accum->prev_value;
+ else
+ input += UINT_MAX - accum->prev_value +
+ accum->energy_ctr;
/* Energy consumed = (1/(2^ESU) * RAW * 1000000UL) μJoules */
*val = div64_ul(input * 1000000UL, BIT(data->energy_units));
@@ -188,20 +147,22 @@ static int amd_energy_read(struct device *dev,
u32 attr, int channel, long *val)
{
struct amd_energy_data *data = dev_get_drvdata(dev);
+ u32 reg;
int cpu;
if (channel >= data->nr_cpus) {
cpu = cpumask_first_and(cpu_online_mask,
cpumask_of_node
(channel - data->nr_cpus));
- amd_add_delta(data, channel, cpu, val, false);
+ reg = ENERGY_PKG_MSR;
} else {
cpu = channel;
if (!cpu_online(cpu))
return -ENODEV;
- amd_add_delta(data, channel, cpu, val, true);
+ reg = ENERGY_CORE_MSR;
}
+ amd_add_delta(data, channel, cpu, val, reg);
return 0;
}
@@ -242,7 +203,7 @@ static const struct hwmon_ops amd_energy_ops = {
static int amd_create_sensor(struct device *dev,
struct amd_energy_data *data,
- u8 type, u32 config)
+ enum hwmon_sensor_types type, u32 config)
{
struct hwmon_channel_info *info = &data->energy_info;
struct sensor_accumulator *accums;
@@ -301,6 +262,7 @@ static int amd_energy_probe(struct platform_device *pdev)
struct device *hwmon_dev;
struct amd_energy_data *data;
struct device *dev = &pdev->dev;
+ int ret;
data = devm_kzalloc(dev,
sizeof(struct amd_energy_data), GFP_KERNEL);
@@ -313,8 +275,10 @@ static int amd_energy_probe(struct platform_device *pdev)
dev_set_drvdata(dev, data);
/* Populate per-core energy reporting */
data->info[0] = &data->energy_info;
- amd_create_sensor(dev, data, hwmon_energy,
- HWMON_E_INPUT | HWMON_E_LABEL);
+ ret = amd_create_sensor(dev, data, hwmon_energy,
+ HWMON_E_INPUT | HWMON_E_LABEL);
+ if (ret)
+ return ret;
mutex_init(&data->lock);
get_energy_units(data);
@@ -338,7 +302,7 @@ static int amd_energy_probe(struct platform_device *pdev)
if (IS_ERR(data->wrap_accumulate))
return PTR_ERR(data->wrap_accumulate);
- return PTR_ERR_OR_ZERO(data->wrap_accumulate);
+ return 0;
}
static int amd_energy_remove(struct platform_device *pdev)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread