Re: [Xen-devel] [PATCH v2 6/9] x86/intel_pstate: the main boby of the intel_pstate driver

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

On 08/06/2015 14:50, Jan Beulich wrote:
> >>> On 08.06.15 at 03:47, <wei.w.wang@xxxxxxxxx> wrote:
> > On 26/05/2015 21:58, Jan Beulich wrote
> >> >>> On 13.05.16 at 09:50, <wei.w.wang@xxxxxxxxx> wrote:
> >> > +static int byt_get_min_pstate(void) {
> >> > +    u64 value;
> >> > +
> >> > +    rdmsrl(BYT_RATIOS, value);
> >> > +    return (value >> 8) & 0x7F;
> >> > +}
> >> > +
> >> > +static int byt_get_max_pstate(void) {
> >> > +    u64 value;
> >> > +
> >> > +    rdmsrl(BYT_RATIOS, value);
> >> > +    return (value >> 16) & 0x7F;
> >> > +}
> >> > +
> >> > +static int byt_get_turbo_pstate(void) {
> >> > +    u64 value;
> >> > +
> >> > +    rdmsrl(BYT_TURBO_RATIOS, value);
> >> > +    return value & 0x7F;
> >> > +}
> >> > +
> >> > +static void byt_set_pstate(struct cpudata *cpudata, int pstate) {
> >> > +    u64 val;
> >> > +    int32_t vid_fp;
> >> > +    u32 vid;
> >> > +
> >> > +    val = pstate << 8;
> >> > +    if (limits.no_turbo && !limits.turbo_disabled)
> >> > +        val |= (u64)1 << 32;
> >>
> >> All of the literal numbers above (and there are more further down)
> >> would better become self-documenting manifest constants.
> >
> > I just realized that it would be better to remove these bay trail
> > platform related code. What do you think?
> 
> First of all - why? Unless these are 32-bit only systems, I don't see why we
> wouldn't want to support Atoms at least on a best effort basis. And if you
> consider removal of code present in Linux, then this also needs to be done
> with the earlier mentioned "as little changes as possible compared to the
> Linux original" in mind.

I don't have an objection to keeping it. Probably, some people are running Xen 
on tablets.

Regarding the self-documenting related comment here, which of the following do 
you think is better?
1) 
#define BYT_MIN_PSTATE_SHIFT            8
#define BYT_MAX_PSTATE_SHIFT           16
#define BYT_PSTATE_MASK                       0x7f
#define BYT_TURBO_CONTROL_BIT        32

2)
#define BYT_MIN_PSTATE(value)     (((value) >> 8) & 0x7F)
#define BYT_MAX_PSTATE(value)    (((value) >> 16) & 0x7F)
#define BYT_SET_TURBO_CONTROL_BIT(value)    ((val) |= (u64)1 << 32)

Best,
Wei

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