On Wed, 21 Aug 2024, Tero Kristo wrote:
> Add the TPMI efficiency latency control fields to the sysfs interface.
> The sysfs files are mapped to the TPMI uncore driver via the registered
> uncore_read and uncore_write driver callbacks. These fields are not
> populated on older non TPMI hardware.
>
> Signed-off-by: Tero Kristo <tero.kristo@xxxxxxxxxxxxxxx>
> ---
> .../uncore-frequency-common.c | 42 ++++++++++++++++---
> .../uncore-frequency-common.h | 13 +++++-
> 2 files changed, 49 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
> index 4e880585cbe4..e22b683a7a43 100644
> --- a/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
> +++ b/drivers/platform/x86/intel/uncore-frequency/uncore-frequency-common.c
> @@ -60,11 +60,16 @@ static ssize_t show_attr(struct uncore_data *data, char *buf, enum uncore_index
> static ssize_t store_attr(struct uncore_data *data, const char *buf, ssize_t count,
> enum uncore_index index)
> {
> - unsigned int input;
> + unsigned int input = 0;
> int ret;
>
> - if (kstrtouint(buf, 10, &input))
> - return -EINVAL;
> + if (index == UNCORE_INDEX_EFF_LAT_CTRL_HIGH_THRESHOLD_ENABLE) {
> + if (kstrtobool(buf, (bool *)&input))
> + return -EINVAL;
> + } else {
> + if (kstrtouint(buf, 10, &input))
> + return -EINVAL;
> + }
>
> mutex_lock(&uncore_lock);
> ret = uncore_write(data, input, index);
> @@ -103,6 +108,18 @@ show_uncore_attr(max_freq_khz, UNCORE_INDEX_MAX_FREQ);
>
> show_uncore_attr(current_freq_khz, UNCORE_INDEX_CURRENT_FREQ);
>
> +store_uncore_attr(elc_low_threshold_percent, UNCORE_INDEX_EFF_LAT_CTRL_LOW_THRESHOLD);
> +store_uncore_attr(elc_high_threshold_percent, UNCORE_INDEX_EFF_LAT_CTRL_HIGH_THRESHOLD);
> +store_uncore_attr(elc_high_threshold_enable,
> + UNCORE_INDEX_EFF_LAT_CTRL_HIGH_THRESHOLD_ENABLE);
> +store_uncore_attr(elc_floor_freq_khz, UNCORE_INDEX_EFF_LAT_CTRL_FREQ);
> +
> +show_uncore_attr(elc_low_threshold_percent, UNCORE_INDEX_EFF_LAT_CTRL_LOW_THRESHOLD);
> +show_uncore_attr(elc_high_threshold_percent, UNCORE_INDEX_EFF_LAT_CTRL_HIGH_THRESHOLD);
> +show_uncore_attr(elc_high_threshold_enable,
> + UNCORE_INDEX_EFF_LAT_CTRL_HIGH_THRESHOLD_ENABLE);
> +show_uncore_attr(elc_floor_freq_khz, UNCORE_INDEX_EFF_LAT_CTRL_FREQ);
> +
> #define show_uncore_data(member_name) \
> static ssize_t show_##member_name(struct kobject *kobj, \
> struct kobj_attribute *attr, char *buf)\
> @@ -146,7 +163,8 @@ show_uncore_data(initial_max_freq_khz);
>
> static int create_attr_group(struct uncore_data *data, char *name)
> {
> - int ret, freq, index = 0;
> + int ret, index = 0;
> + unsigned int val;
>
> init_attribute_rw(max_freq_khz);
> init_attribute_rw(min_freq_khz);
> @@ -168,10 +186,24 @@ static int create_attr_group(struct uncore_data *data, char *name)
> data->uncore_attrs[index++] = &data->initial_min_freq_khz_kobj_attr.attr;
> data->uncore_attrs[index++] = &data->initial_max_freq_khz_kobj_attr.attr;
>
> - ret = uncore_read(data, &freq, UNCORE_INDEX_CURRENT_FREQ);
> + ret = uncore_read(data, &val, UNCORE_INDEX_CURRENT_FREQ);
> if (!ret)
> data->uncore_attrs[index++] = &data->current_freq_khz_kobj_attr.attr;
>
> + ret = uncore_read(data, &val, UNCORE_INDEX_EFF_LAT_CTRL_LOW_THRESHOLD);
> + if (!ret) {
> + init_attribute_rw(elc_low_threshold_percent);
> + init_attribute_rw(elc_high_threshold_percent);
> + init_attribute_rw(elc_high_threshold_enable);
> + init_attribute_rw(elc_floor_freq_khz);
> +
> + data->uncore_attrs[index++] = &data->elc_low_threshold_percent_kobj_attr.attr;
> + data->uncore_attrs[index++] = &data->elc_high_threshold_percent_kobj_attr.attr;
> + data->uncore_attrs[index++] =
> + &data->elc_high_threshold_enable_kobj_attr.attr;
> + data->uncore_attrs[index++] = &data->elc_floor_freq_khz_kobj_attr.attr;
> + }
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
But I have to say I'm not big fan of this function treating any error as
an implicit indication of ELC not supported.
Is that even going to be true after this:
https://patchwork.kernel.org/project/platform-driver-x86/patch/20240820204558.1296319-1-srinivas.pandruvada@xxxxxxxxxxxxxxx/
...as root_domain is eliminated for other reasons than ELC
supported/not-supported (-ENODATA return path)?
--
i.
