Re: [PATCH] powercap: intel_rapl: Don't warn about BIOS locked limits during resume

[Pandruvada, Srinivas]

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();
>  }