* [dpdk-dev] [PATCH] power: fix current frequency index
@ 2020-07-29 13:58 Reshma Pattan
2020-07-31 9:32 ` Liang, Ma
0 siblings, 1 reply; 3+ messages in thread
From: Reshma Pattan @ 2020-07-29 13:58 UTC (permalink / raw)
To: dev; +Cc: stable, liang.j.ma, Reshma Pattan
1)
During power initialization the pstate cpufreq api is
not setting the initial curr_idx of pstate_power_info
to corresponding current frequency index.
Without this the idx is always 0, which is causing the
below check to pass and returns without setting the initial
min/max frequency to system max frequency and this leads to
incorrect frequency settings when power_pstate_cpufreq_set_freq()
is called in the apps.
set_freq_internal(struct pstate_power_info *pi, uint32_t idx)
{
...
/* Check if it is the same as current */
if (idx == pi->curr_idx)
return 0;
...
}
scenario 1:
If system has starting scaling min/max: 1000/1000, and want to
set this to 2200/2200, the max frequency gets updated but not min.
scenario 2:
If system has starting scaling min/max: 2200/1000, and want to set
to 2200/2200, the max, min frequency was not updated. Since no change
in max that should be ok, but min was also ignored, which will be fixed
now with the new changes.
Fixes: e6c6dc0f ("power: add p-state driver compatibility")
Cc: stable@dpdk.org
CC: liang.j.ma@intel.com
Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
---
lib/librte_power/power_pstate_cpufreq.c | 59 +++++++++++++++++++++++++++++++++
1 file changed, 59 insertions(+)
diff --git a/lib/librte_power/power_pstate_cpufreq.c b/lib/librte_power/power_pstate_cpufreq.c
index 2526441..262f024 100644
--- a/lib/librte_power/power_pstate_cpufreq.c
+++ b/lib/librte_power/power_pstate_cpufreq.c
@@ -52,6 +52,9 @@
} \
} while (0)
+/* macros used for rounding frequency to nearest 100000 */
+#define FREQ_ROUNDING_DELTA 50000
+#define ROUND_FREQ_TO_N_100000 100000
#define POWER_CONVERT_TO_DECIMAL 10
#define BUS_FREQ 100000
@@ -532,6 +535,57 @@ struct pstate_power_info {
return ret;
}
+static int
+power_get_cur_idx(struct pstate_power_info *pi)
+{
+ FILE *f_cur;
+ int ret = -1;
+ char *p_cur;
+ char buf_cur[BUFSIZ];
+ char fullpath_cur[PATH_MAX];
+ char *s_cur;
+ uint32_t sys_cur_freq = 0;
+ unsigned int i;
+
+ snprintf(fullpath_cur, sizeof(fullpath_cur),
+ POWER_SYSFILE_CUR_FREQ,
+ pi->lcore_id);
+ f_cur = fopen(fullpath_cur, "r");
+ FOPEN_OR_ERR_RET(f_cur, ret);
+
+ /* initialize the cur_idx to matching current frequency freq index */
+ s_cur = fgets(buf_cur, sizeof(buf_cur), f_cur);
+ FOPS_OR_NULL_GOTO(s_cur, fail);
+
+ p_cur = strchr(buf_cur, '\n');
+ if (p_cur != NULL)
+ *p_cur = 0;
+ sys_cur_freq = strtoul(buf_cur, &p_cur, POWER_CONVERT_TO_DECIMAL);
+
+ /* convert the frequency to nearest 100000 value
+ * Ex: if sys_cur_freq=1396789 then freq_conv=1400000
+ * Ex: if sys_cur_freq=800030 then freq_conv=800000
+ * Ex: if sys_cur_freq=800030 then freq_conv=800000
+ */
+ unsigned int freq_conv = 0;
+ freq_conv = (sys_cur_freq + FREQ_ROUNDING_DELTA)
+ / ROUND_FREQ_TO_N_100000;
+ freq_conv = freq_conv * ROUND_FREQ_TO_N_100000;
+
+ for (i = 0; i < pi->nb_freqs; i++) {
+ if (freq_conv == pi->freqs[i]) {
+ pi->curr_idx = i;
+ break;
+ }
+ }
+
+ fclose(f_cur);
+ return 0;
+fail:
+ fclose(f_cur);
+ return ret;
+}
+
int
power_pstate_cpufreq_check_supported(void)
{
@@ -578,6 +632,11 @@ struct pstate_power_info {
goto fail;
}
+ if (power_get_cur_idx(pi) < 0) {
+ RTE_LOG(ERR, POWER, "Cannot get current frequency "
+ "index of lcore %u\n", lcore_id);
+ goto fail;
+ }
/* Set freq to max by default */
if (power_pstate_cpufreq_freq_max(lcore_id) < 0) {
--
1.8.3.1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [dpdk-dev] [PATCH] power: fix current frequency index
2020-07-29 13:58 [dpdk-dev] [PATCH] power: fix current frequency index Reshma Pattan
@ 2020-07-31 9:32 ` Liang, Ma
2020-10-07 12:55 ` Thomas Monjalon
0 siblings, 1 reply; 3+ messages in thread
From: Liang, Ma @ 2020-07-31 9:32 UTC (permalink / raw)
To: Reshma Pattan; +Cc: dev, stable
Hi Reshma,
I'm OK with the change. That's the missing part about init process.
Reviewed-by Liang Ma <liang.j.ma@intel.com>
Regards
Liang
On 29 Jul 14:58, Reshma Pattan wrote:
> 1)
> During power initialization the pstate cpufreq api is
> not setting the initial curr_idx of pstate_power_info
> to corresponding current frequency index.
>
> Without this the idx is always 0, which is causing the
> below check to pass and returns without setting the initial
> min/max frequency to system max frequency and this leads to
> incorrect frequency settings when power_pstate_cpufreq_set_freq()
> is called in the apps.
>
> set_freq_internal(struct pstate_power_info *pi, uint32_t idx)
> {
> ...
>
> /* Check if it is the same as current */
> if (idx == pi->curr_idx)
> return 0;
> ...
> }
>
> scenario 1:
> If system has starting scaling min/max: 1000/1000, and want to
> set this to 2200/2200, the max frequency gets updated but not min.
>
> scenario 2:
> If system has starting scaling min/max: 2200/1000, and want to set
> to 2200/2200, the max, min frequency was not updated. Since no change
> in max that should be ok, but min was also ignored, which will be fixed
> now with the new changes.
>
> Fixes: e6c6dc0f ("power: add p-state driver compatibility")
> Cc: stable@dpdk.org
> CC: liang.j.ma@intel.com
> Signed-off-by: Reshma Pattan <reshma.pattan@intel.com>
> ---
> lib/librte_power/power_pstate_cpufreq.c | 59 +++++++++++++++++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
> diff --git a/lib/librte_power/power_pstate_cpufreq.c b/lib/librte_power/power_pstate_cpufreq.c
> index 2526441..262f024 100644
> --- a/lib/librte_power/power_pstate_cpufreq.c
> +++ b/lib/librte_power/power_pstate_cpufreq.c
> @@ -52,6 +52,9 @@
> } \
> } while (0)
>
> +/* macros used for rounding frequency to nearest 100000 */
> +#define FREQ_ROUNDING_DELTA 50000
> +#define ROUND_FREQ_TO_N_100000 100000
>
> #define POWER_CONVERT_TO_DECIMAL 10
> #define BUS_FREQ 100000
> @@ -532,6 +535,57 @@ struct pstate_power_info {
> return ret;
> }
>
> +static int
> +power_get_cur_idx(struct pstate_power_info *pi)
> +{
> + FILE *f_cur;
> + int ret = -1;
> + char *p_cur;
> + char buf_cur[BUFSIZ];
> + char fullpath_cur[PATH_MAX];
> + char *s_cur;
> + uint32_t sys_cur_freq = 0;
> + unsigned int i;
> +
> + snprintf(fullpath_cur, sizeof(fullpath_cur),
> + POWER_SYSFILE_CUR_FREQ,
> + pi->lcore_id);
> + f_cur = fopen(fullpath_cur, "r");
> + FOPEN_OR_ERR_RET(f_cur, ret);
> +
> + /* initialize the cur_idx to matching current frequency freq index */
> + s_cur = fgets(buf_cur, sizeof(buf_cur), f_cur);
> + FOPS_OR_NULL_GOTO(s_cur, fail);
> +
> + p_cur = strchr(buf_cur, '\n');
> + if (p_cur != NULL)
> + *p_cur = 0;
> + sys_cur_freq = strtoul(buf_cur, &p_cur, POWER_CONVERT_TO_DECIMAL);
> +
> + /* convert the frequency to nearest 100000 value
> + * Ex: if sys_cur_freq=1396789 then freq_conv=1400000
> + * Ex: if sys_cur_freq=800030 then freq_conv=800000
> + * Ex: if sys_cur_freq=800030 then freq_conv=800000
> + */
> + unsigned int freq_conv = 0;
> + freq_conv = (sys_cur_freq + FREQ_ROUNDING_DELTA)
> + / ROUND_FREQ_TO_N_100000;
> + freq_conv = freq_conv * ROUND_FREQ_TO_N_100000;
> +
> + for (i = 0; i < pi->nb_freqs; i++) {
> + if (freq_conv == pi->freqs[i]) {
> + pi->curr_idx = i;
> + break;
> + }
> + }
> +
> + fclose(f_cur);
> + return 0;
> +fail:
> + fclose(f_cur);
> + return ret;
> +}
> +
> int
> power_pstate_cpufreq_check_supported(void)
> {
> @@ -578,6 +632,11 @@ struct pstate_power_info {
> goto fail;
> }
>
> + if (power_get_cur_idx(pi) < 0) {
> + RTE_LOG(ERR, POWER, "Cannot get current frequency "
> + "index of lcore %u\n", lcore_id);
> + goto fail;
> + }
>
> /* Set freq to max by default */
> if (power_pstate_cpufreq_freq_max(lcore_id) < 0) {
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [dpdk-dev] [PATCH] power: fix current frequency index
2020-07-31 9:32 ` Liang, Ma
@ 2020-10-07 12:55 ` Thomas Monjalon
0 siblings, 0 replies; 3+ messages in thread
From: Thomas Monjalon @ 2020-10-07 12:55 UTC (permalink / raw)
To: Reshma Pattan; +Cc: dev, stable, Liang, Ma, david.hunt
31/07/2020 11:32, Liang, Ma:
> Hi Reshma,
>
> I'm OK with the change. That's the missing part about init process.
>
> Reviewed-by Liang Ma <liang.j.ma@intel.com>
Because of a missing colon, this review was not counted in patchwork.
I guess Dave is OK with this patch but he was not Cc'ed.
Please use --cc-cmd devtools/get-maintainer.sh when sending patches.
Dave, you can poll the backlog with this request:
https://patches.dpdk.org/project/dpdk/list/?q=power
Patch applied, thanks
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-07 12:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-29 13:58 [dpdk-dev] [PATCH] power: fix current frequency index Reshma Pattan
2020-07-31 9:32 ` Liang, Ma
2020-10-07 12:55 ` Thomas Monjalon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).