On 9/5/20 7:32 AM, Naveen Krishna Chatradhi wrote:
> 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@xxxxxxx>
> ---
> drivers/hwmon/amd_energy.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hwmon/amd_energy.c b/drivers/hwmon/amd_energy.c
> index 9580a16185b8..f0a13d6cc419 100644
> --- a/drivers/hwmon/amd_energy.c
> +++ b/drivers/hwmon/amd_energy.c
> @@ -48,6 +48,7 @@ struct amd_energy_data {
> struct sensor_accumulator *accums;
> /* Energy Status Units */
> u64 energy_units;
> + unsigned int timeout;
> 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;
>
> while (!kthread_should_stop()) {
> /*
> @@ -234,7 +236,7 @@ static int energy_accumulator(void *p)
> *
> * let us accumulate for every 100secs
> */
> - schedule_timeout(msecs_to_jiffies(100000));
> + schedule_timeout(msecs_to_jiffies(timeout));
Numbers below are in seconds, used as milli-seconds here.
> }
> return 0;
> }
> @@ -331,6 +333,14 @@ static int amd_energy_probe(struct platform_device *pdev)
> if (IS_ERR(hwmon_dev))
> return PTR_ERR(hwmon_dev);
>
> + /* Once in 3 minutes for a resolution of 1/2*16 */
* 16 or ^ 16 ?
> + if (data->energy_units == 0x10)
> + data->timeout = 3 * 60;
180 ms ? I assume this is a bug and meant to be 3 * 60 * 1000.
> +
> + /* Once in 3 days for a resolution of 1/2^6 */
> + if (data->energy_units == 0x6)
> + data->timeout = 3 * 24 * 60 * 60;
> +
... and else ?
This needs to cover all cases, including those not currently existing.
I would suggest to define a formula based on data->energy_units.
The energy units value can be anything from 0..31. Based on your numbers,
something like
timeout_ms = BIT(34 - data->energy_units);
should do. It translates to about 3.1 days for energy_units=6, and
4.3 minutes for energy_units=16. If that is too much, you can make it
timeout_ms = BIT(33 - data->energy_units);
To avoid overflow, it might make sense to max out at BIT(31).
timeout_ms = BIT((min(31, 33 - data->energy_units));
Guenter
> data->wrap_accumulate = kthread_run(energy_accumulator, data,
> "%s", dev_name(hwmon_dev));
> if (IS_ERR(data->wrap_accumulate))
>
