From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id E87D2A052B; Fri, 31 Jul 2020 11:32:30 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C4E3D1C00D; Fri, 31 Jul 2020 11:32:30 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id E03A71C002; Fri, 31 Jul 2020 11:32:28 +0200 (CEST) IronPort-SDR: mgsgbRAAewCiYgaOaatdnV3tVpkH5kTlSVvowXhoy0ewhsPYR7BGUBSyBJlqUUOEEEyWGM/IsS r8fSWkAFSjlA== X-IronPort-AV: E=McAfee;i="6000,8403,9698"; a="149219235" X-IronPort-AV: E=Sophos;i="5.75,417,1589266800"; d="scan'208";a="149219235" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 31 Jul 2020 02:32:28 -0700 IronPort-SDR: GL/3Jbste6elJj1vjC4p3TGGFKpASCmHDBFn6L0h9EPaNSqLvHqchV4y/qXdzblrFPGtC/IjY+ d8q/plduYj/Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,417,1589266800"; d="scan'208";a="395261055" Received: from irvmail001.ir.intel.com ([163.33.26.43]) by fmsmga001.fm.intel.com with ESMTP; 31 Jul 2020 02:32:27 -0700 Received: from sivswdev09.ir.intel.com (sivswdev09.ir.intel.com [10.237.217.48]) by irvmail001.ir.intel.com (8.14.3/8.13.6/MailSET/Hub) with ESMTP id 06V9WQ1q011365; Fri, 31 Jul 2020 10:32:26 +0100 Received: from sivswdev09.ir.intel.com (localhost [127.0.0.1]) by sivswdev09.ir.intel.com with ESMTP id 06V9WQ4I013464; Fri, 31 Jul 2020 10:32:26 +0100 Received: (from lma25@localhost) by sivswdev09.ir.intel.com with LOCAL id 06V9WQSP013460; Fri, 31 Jul 2020 10:32:26 +0100 Date: Fri, 31 Jul 2020 10:32:26 +0100 From: "Liang, Ma" To: Reshma Pattan Cc: dev@dpdk.org, stable@dpdk.org Message-ID: <20200731093226.GA7166@sivswdev09.ir.intel.com> References: <1596031133-110878-1-git-send-email-reshma.pattan@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1596031133-110878-1-git-send-email-reshma.pattan@intel.com> Subject: Re: [dpdk-dev] [PATCH] power: fix current frequency index X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Reshma, I'm OK with the change. That's the missing part about init process. Reviewed-by Liang Ma 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 > --- > 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 >