On Wed, 2023-10-04 at 21:34 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Restore enough of the original behaviour to stop spamming
> dmesg with warnings about BIOS locked limits when trying
> to restore them during resume.
>
> This still doesn't 100% match the original behaviour
> as we no longer attempt to blindly restore the BIOS locked
> limits. No idea if that makes any difference in practice.
>
I lost the context here. Why can't we simply change pr_warn to pr_debug
here?
Thanks,
Srinivas
> Cc: Zhang Rui <rui.zhang@xxxxxxxxx>
> Cc: Wang Wendy <wendy.wang@xxxxxxxxx>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> Cc: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxx>
> Fixes: 9050a9cd5e4c ("powercap: intel_rapl: Cleanup Power Limits
> support")
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/powercap/intel_rapl_common.c | 28 ++++++++++++++++++++------
> --
> 1 file changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/powercap/intel_rapl_common.c
> b/drivers/powercap/intel_rapl_common.c
> index 40a2cc649c79..9a6a40c83f82 100644
> --- a/drivers/powercap/intel_rapl_common.c
> +++ b/drivers/powercap/intel_rapl_common.c
> @@ -882,22 +882,34 @@ static int rapl_read_pl_data(struct rapl_domain
> *rd, int pl,
> return rapl_read_data_raw(rd, prim, xlate, data);
> }
>
> -static int rapl_write_pl_data(struct rapl_domain *rd, int pl,
> - enum pl_prims pl_prim,
> - unsigned long long value)
> +static int rapl_write_pl_data_nowarn(struct rapl_domain *rd, int pl,
> + enum pl_prims pl_prim,
> + unsigned long long value)
> {
> enum rapl_primitives prim = get_pl_prim(rd, pl, pl_prim);
>
> if (!is_pl_valid(rd, pl))
> return -EINVAL;
>
> - if (rd->rpl[pl].locked) {
> - pr_warn("%s:%s:%s locked by BIOS\n", rd->rp->name,
> rd->name, pl_names[pl]);
> + if (rd->rpl[pl].locked)
> return -EACCES;
> - }
>
> return rapl_write_data_raw(rd, prim, value);
> }
> +
> +static int rapl_write_pl_data(struct rapl_domain *rd, int pl,
> + enum pl_prims pl_prim,
> + unsigned long long value)
> +{
> + int ret;
> +
> + ret = rapl_write_pl_data_nowarn(rd, pl, pl_prim, value);
> + if (ret == -EACCES)
> + pr_warn("%s:%s:%s locked by BIOS\n", rd->rp->name,
> rd->name, pl_names[pl]);
> +
> + return ret;
> +}
> +
> /*
> * Raw RAPL data stored in MSRs are in certain scales. We need to
> * convert them into standard units based on the units reported in
> @@ -1634,8 +1646,8 @@ static void power_limit_state_restore(void)
> rd = power_zone_to_rapl_domain(rp->power_zone);
> for (i = POWER_LIMIT1; i < NR_POWER_LIMITS; i++)
> if (rd->rpl[i].last_power_limit)
> - rapl_write_pl_data(rd, i, PL_LIMIT,
> - rd-
> >rpl[i].last_power_limit);
> + rapl_write_pl_data_nowarn(rd, i,
> PL_LIMIT,
> + rd-
> >rpl[i].last_power_limit);
> }
> cpus_read_unlock();
> }
