Re: [Xen-devel] [PATCH v5 8/9] x86/intel_pstate: support the use of intel_pstate in pmstat.c

["Wang, Wei W" <wei.w.wang@xxxxxxxxx>]

On 26/10/2015 17:54,  Jan Beulich wrote:
> >>> On 26.10.15 at 10:48, <wei.w.wang@xxxxxxxxx> wrote:
> > On 26/10/2015 17:42,  Jan Beulich wrote:
> >> >>> On 26.10.15 at 08:59, <wei.w.wang@xxxxxxxxx> wrote:
> >> > On 26/10/2015 15:03,  Jan Beulich wrote:
> >> >> >>> "Wang, Wei W" <wei.w.wang@xxxxxxxxx> 10/26/15 7:27 AM >>>
> >> >> >On 08/10/2015 12:11,  Jan Beulich wrote:
> >> >> >> >>> On 14.09.15 at 04:32, <wei.w.wang@xxxxxxxxx> wrote:
> >> >> >> > @@ -309,23 +326,13 @@ struct xen_get_cpufreq_para {
> >> >> >> >      XEN_GUEST_HANDLE_64(uint32) scaling_available_frequencies;
> >> >> >> >      XEN_GUEST_HANDLE_64(char)   scaling_available_governors;
> >> >> >> >      char scaling_driver[CPUFREQ_NAME_LEN];
> >> >> >> > -
> >> >> >> > -    uint32_t cpuinfo_cur_freq;
> >> >> >> > -    uint32_t cpuinfo_max_freq;
> >> >> >> > -    uint32_t cpuinfo_min_freq;
> >> >> >> > -    uint32_t scaling_cur_freq;
> >> >> >> > -
> >> >> >> >      char scaling_governor[CPUFREQ_NAME_LEN];
> >> >> >> > -    uint32_t scaling_max_freq;
> >> >> >> > -    uint32_t scaling_min_freq;
> >> >> >> >
> >> >> >> >      /* for specific governor */
> >> >> >> >      union {
> >> >> >> >          struct  xen_userspace userspace;
> >> >> >> >          struct  xen_ondemand ondemand;
> >> >> >> >      } u;
> >> >> >> > -
> >> >> >> > -    int32_t turbo_enabled;
> >> >> >> >  };
> >> >> >>
> >> >> >> Is all of this re-arrangement really needed? Also can't
> >> >> >> turbo_enabled and scaling_turbo_pct be combined into a single field?
> >> >> >
> >> >> >Personally, we should not combine the two.
> >> >> > turbo_enabled is used by both the old pstate driver and
> >> >> >intel_pstate, but  scaling_turbo_pct is only used in intel_pstate.
> >> >> >If we use
> >> >> "scaling_turbo_pct=0"
> >> >> > to represent "turbo_enabled=0", and "scaling_turbo_pct>0" to
> >> >> >represent " turbo_enabled=1", then we will need to modify the old
> >> >> >driver to use scaling_turbo_pct, i.e. changing the old driver to
> >> >> >be aware
> > of
> >> the "percentage"
> >> >> > concept, which is proposed in intel_pstate. On the other side, I
> >> >> >think keeping  turbo_enabled and scaling_turbo_pct separated
> >> >> >makes the code
> >> >> easier to read.
> >> >>
> >> >> Note that "combine" doesn't necessarily mean "eliminate the old one"
> >> >> - they could as well become field of a union. The basic question
> >> >> you should ask yourself in such cases is: "Do both fields have a
> >> >> meaning at the same time?" If the answer is yes, then of course
> >> >> they should remain separate. If the answer is no _and_ their
> >> >> purpose is reasonably similar, then combining them should at least be
> considered.
> >> >
> >> > Ok. I will keep the two separated, since they do have their own
> >> > meaning at the same time.
> >>
> >> Being which?
> >
> > Keeping both of the two there. Just as what they are now - two
> > independent fields.
> 
> That wasn't the question; I rather inquired what "meaning at the same time"
> both fields have.

turbo_enable: indicates if turbo is enabled or not.
turbo_pct: shows the capability of turbo in percentage. For example if the CPU 
has the following operating frequency range:
From [Min] to [Max]: 1.2GH, 1.3GHZ,1.4GHZ,...,2.3GHZ, so 1.2--->2.3, length = 12
Turbo frequency: 3.7GHZ, so 1.2---->3.7, length = 26
Then turbo_pct = (26 - 12) / 26 = 54%, that is turbo_pct = 54.

Best,
Wei


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel