[1/4] intel_idle: stop using driver_data for static flags

Message ID	9a38405338d7464c852c4524465f84f8a2ac22fb.1359691799.git.len.brown@intel.com (mailing list archive)
State	Accepted, archived
Delegated to:	Len Brown
Headers	show Return-Path: <linux-pm-owner@vger.kernel.org> From: Len Brown <lenb@kernel.org> To: linux-pm@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Len Brown <len.brown@intel.com> Subject: [PATCH 1/4] intel_idle: stop using driver_data for static flags Date: Thu, 31 Jan 2013 23:11:33 -0500 Message-Id: <9a38405338d7464c852c4524465f84f8a2ac22fb.1359691799.git.len.brown@intel.com> In-Reply-To: <1359691896-23567-1-git-send-email-lenb@kernel.org> References: <1359691896-23567-1-git-send-email-lenb@kernel.org> Reply-To: Len Brown <lenb@kernel.org> Organization: Intel Open Source Technology Center Sender: linux-pm-owner@vger.kernel.org Precedence: bulk

Len Brown Feb. 1, 2013, 4:11 a.m. UTC

From: Len Brown <len.brown@intel.com>

The commit, 4202735e8ab6ecfb0381631a0d0b58fefe0bd4e2
(cpuidle: Split cpuidle_state structure and move per-cpu statistics fields)
observed that the MWAIT flags for Cn on every processor to date were the
same, and created get_driver_data() to supply them.

Unfortunately, that assumption is false, going forward.
So here we restore the MWAIT flags to the cpuidle_state table.
However, instead restoring the old "driver_data" field,
we put the flags into the existing "flags" field,
where they probalby should have lived all along.

This patch does not change any operation.

This patch removes 1 of the 3 users of cpuidle_state_usage.driver_data.
Perhaps some day we'll get rid of the other 2.

Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/idle/intel_idle.c | 75 ++++++++++++++++-------------------------------
 1 file changed, 26 insertions(+), 49 deletions(-)

Daniel Lezcano Feb. 1, 2013, 8:44 a.m. UTC | #1

On 02/01/2013 05:11 AM, Len Brown wrote:
> From: Len Brown <len.brown@intel.com>
> 
> The commit, 4202735e8ab6ecfb0381631a0d0b58fefe0bd4e2
> (cpuidle: Split cpuidle_state structure and move per-cpu statistics fields)
> observed that the MWAIT flags for Cn on every processor to date were the
> same, and created get_driver_data() to supply them.
> 
> Unfortunately, that assumption is false, going forward.
> So here we restore the MWAIT flags to the cpuidle_state table.
> However, instead restoring the old "driver_data" field,
> we put the flags into the existing "flags" field,
> where they probalby should have lived all along.

Removing the driver_data is a good thing but I am not sure it is the
case by moving the MWAIT flags to the cpuidle_state's flags field. We
are mixing arch specific flags with a generic code.

This is prone to errors because new flags could appear for the cpuidle
core code and could collide with the arch specific flags.

Wouldn't make sense to add a private field in the struct cpuidle_state
structure to let the driver/arch specific to store whatever is needed ?

struct cpuidle_state {

	...
	unsigned long private;
	...

}


> This patch does not change any operation.
> 
> This patch removes 1 of the 3 users of cpuidle_state_usage.driver_data.
> Perhaps some day we'll get rid of the other 2.

+1 :)

> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
>  drivers/idle/intel_idle.c | 75 ++++++++++++++++-------------------------------
>  1 file changed, 26 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index 2df9414..8331753 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -109,6 +109,16 @@ static struct cpuidle_state *cpuidle_state_table;
>  #define CPUIDLE_FLAG_TLB_FLUSHED	0x10000
>  
>  /*
> + * MWAIT takes an 8-bit "hint" in EAX "suggesting"
> + * the C-state (top nibble) and sub-state (bottom nibble)
> + * 0x00 means "MWAIT(C1)", 0x10 means "MWAIT(C2)" etc.
> + * 
> + * We store the hint at the top of our "flags" for each state.
> + */
> +#define flags_2_MWAIT_EAX(flags) (((flags) >> 24) & 0xFF)
> +#define MWAIT_EAX_2_flags(eax) ((eax & 0xFF) << 24)
> +
> +/*
>   * States are indexed by the cstate number,
>   * which is also the index into the MWAIT hint array.
>   * Thus C0 is a dummy.
> @@ -118,21 +128,21 @@ static struct cpuidle_state nehalem_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C1 */
>  		.name = "C1-NHM",
>  		.desc = "MWAIT 0x00",
> -		.flags = CPUIDLE_FLAG_TIME_VALID,
> +		.flags = MWAIT_EAX_2_flags(0x00) | CPUIDLE_FLAG_TIME_VALID,
>  		.exit_latency = 3,
>  		.target_residency = 6,
>  		.enter = &intel_idle },
>  	{ /* MWAIT C2 */
>  		.name = "C3-NHM",
>  		.desc = "MWAIT 0x10",
> -		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
> +		.flags = MWAIT_EAX_2_flags(0x10) | CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 20,
>  		.target_residency = 80,
>  		.enter = &intel_idle },
>  	{ /* MWAIT C3 */
>  		.name = "C6-NHM",
>  		.desc = "MWAIT 0x20",
> -		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
> +		.flags = MWAIT_EAX_2_flags(0x20) |CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 200,
>  		.target_residency = 800,
>  		.enter = &intel_idle },
> @@ -143,28 +153,28 @@ static struct cpuidle_state snb_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C1 */
>  		.name = "C1-SNB",
>  		.desc = "MWAIT 0x00",
> -		.flags = CPUIDLE_FLAG_TIME_VALID,
> +		.flags = MWAIT_EAX_2_flags(0x00) | CPUIDLE_FLAG_TIME_VALID,
>  		.exit_latency = 1,
>  		.target_residency = 1,
>  		.enter = &intel_idle },
>  	{ /* MWAIT C2 */
>  		.name = "C3-SNB",
>  		.desc = "MWAIT 0x10",
> -		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
> +		.flags = MWAIT_EAX_2_flags(0x10) | CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 80,
>  		.target_residency = 211,
>  		.enter = &intel_idle },
>  	{ /* MWAIT C3 */
>  		.name = "C6-SNB",
>  		.desc = "MWAIT 0x20",
> -		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
> +		.flags = MWAIT_EAX_2_flags(0x20) | CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 104,
>  		.target_residency = 345,
>  		.enter = &intel_idle },
>  	{ /* MWAIT C4 */
>  		.name = "C7-SNB",
>  		.desc = "MWAIT 0x30",
> -		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
> +		.flags = MWAIT_EAX_2_flags(0x30) | CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 109,
>  		.target_residency = 345,
>  		.enter = &intel_idle },
> @@ -175,28 +185,28 @@ static struct cpuidle_state ivb_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C1 */
>  		.name = "C1-IVB",
>  		.desc = "MWAIT 0x00",
> -		.flags = CPUIDLE_FLAG_TIME_VALID,
> +		.flags = MWAIT_EAX_2_flags(0x00) | CPUIDLE_FLAG_TIME_VALID,
>  		.exit_latency = 1,
>  		.target_residency = 1,
>  		.enter = &intel_idle },
>  	{ /* MWAIT C2 */
>  		.name = "C3-IVB",
>  		.desc = "MWAIT 0x10",
> -		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
> +		.flags = MWAIT_EAX_2_flags(0x10) | CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 59,
>  		.target_residency = 156,
>  		.enter = &intel_idle },
>  	{ /* MWAIT C3 */
>  		.name = "C6-IVB",
>  		.desc = "MWAIT 0x20",
> -		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
> +		.flags = MWAIT_EAX_2_flags(0x20) | CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 80,
>  		.target_residency = 300,
>  		.enter = &intel_idle },
>  	{ /* MWAIT C4 */
>  		.name = "C7-IVB",
>  		.desc = "MWAIT 0x30",
> -		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
> +		.flags = MWAIT_EAX_2_flags(0x30) | CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 87,
>  		.target_residency = 300,
>  		.enter = &intel_idle },
> @@ -207,14 +217,14 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C1 */
>  		.name = "C1-ATM",
>  		.desc = "MWAIT 0x00",
> -		.flags = CPUIDLE_FLAG_TIME_VALID,
> +		.flags = MWAIT_EAX_2_flags(0x00) | CPUIDLE_FLAG_TIME_VALID,
>  		.exit_latency = 1,
>  		.target_residency = 4,
>  		.enter = &intel_idle },
>  	{ /* MWAIT C2 */
>  		.name = "C2-ATM",
>  		.desc = "MWAIT 0x10",
> -		.flags = CPUIDLE_FLAG_TIME_VALID,
> +		.flags = MWAIT_EAX_2_flags(0x10) | CPUIDLE_FLAG_TIME_VALID,
>  		.exit_latency = 20,
>  		.target_residency = 80,
>  		.enter = &intel_idle },
> @@ -222,7 +232,7 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C4 */
>  		.name = "C4-ATM",
>  		.desc = "MWAIT 0x30",
> -		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
> +		.flags = MWAIT_EAX_2_flags(0x30) | CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 100,
>  		.target_residency = 400,
>  		.enter = &intel_idle },
> @@ -230,41 +240,12 @@ static struct cpuidle_state atom_cstates[MWAIT_MAX_NUM_CSTATES] = {
>  	{ /* MWAIT C6 */
>  		.name = "C6-ATM",
>  		.desc = "MWAIT 0x52",
> -		.flags = CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
> +		.flags = MWAIT_EAX_2_flags(0x52) | CPUIDLE_FLAG_TIME_VALID | CPUIDLE_FLAG_TLB_FLUSHED,
>  		.exit_latency = 140,
>  		.target_residency = 560,
>  		.enter = &intel_idle },
>  };
>  
> -static long get_driver_data(int cstate)
> -{
> -	int driver_data;
> -	switch (cstate) {
> -
> -	case 1:	/* MWAIT C1 */
> -		driver_data = 0x00;
> -		break;
> -	case 2:	/* MWAIT C2 */
> -		driver_data = 0x10;
> -		break;
> -	case 3:	/* MWAIT C3 */
> -		driver_data = 0x20;
> -		break;
> -	case 4:	/* MWAIT C4 */
> -		driver_data = 0x30;
> -		break;
> -	case 5:	/* MWAIT C5 */
> -		driver_data = 0x40;
> -		break;
> -	case 6:	/* MWAIT C6 */
> -		driver_data = 0x52;
> -		break;
> -	default:
> -		driver_data = 0x00;
> -	}
> -	return driver_data;
> -}
> -
>  /**
>   * intel_idle
>   * @dev: cpuidle_device
> @@ -278,8 +259,7 @@ static int intel_idle(struct cpuidle_device *dev,
>  {
>  	unsigned long ecx = 1; /* break on interrupt flag */
>  	struct cpuidle_state *state = &drv->states[index];
> -	struct cpuidle_state_usage *state_usage = &dev->states_usage[index];
> -	unsigned long eax = (unsigned long)cpuidle_get_statedata(state_usage);
> +	unsigned long eax = flags_2_MWAIT_EAX(state->flags);
>  	unsigned int cstate;
>  	int cpu = smp_processor_id();
>  
> @@ -558,9 +538,6 @@ static int intel_idle_cpu_init(int cpu)
>  		if (cpuidle_state_table[cstate].enter == NULL)
>  			continue;
>  
> -		dev->states_usage[dev->state_count].driver_data =
> -			(void *)get_driver_data(cstate);
> -
>  		dev->state_count += 1;
>  	}
>  
>

Len Brown Feb. 1, 2013, 6:40 p.m. UTC | #2

On 02/01/2013 03:44 AM, Daniel Lezcano wrote:
> On 02/01/2013 05:11 AM, Len Brown wrote:
>> From: Len Brown <len.brown@intel.com>
>>
>> The commit, 4202735e8ab6ecfb0381631a0d0b58fefe0bd4e2
>> (cpuidle: Split cpuidle_state structure and move per-cpu statistics fields)
>> observed that the MWAIT flags for Cn on every processor to date were the
>> same, and created get_driver_data() to supply them.
>>
>> Unfortunately, that assumption is false, going forward.
>> So here we restore the MWAIT flags to the cpuidle_state table.
>> However, instead restoring the old "driver_data" field,
>> we put the flags into the existing "flags" field,
>> where they probalby should have lived all along.
> 
> Removing the driver_data is a good thing but I am not sure it is the
> case by moving the MWAIT flags to the cpuidle_state's flags field. We
> are mixing arch specific flags with a generic code.
> 
> This is prone to errors because new flags could appear for the cpuidle
> core code and could collide with the arch specific flags.
> 
> Wouldn't make sense to add a private field in the struct cpuidle_state
> structure to let the driver/arch specific to store whatever is needed ?
> 
> struct cpuidle_state {
> 
> 	...
> 	unsigned long private;
> 	...
> 
> }

The top half of the flags are reserved for the driver,
as noted by the definition of CPUIDLE_DRIVER_FLAGS_MASK
with the generic flag definitions:

#define CPUIDLE_FLAG_TIME_VALID (0x01) /* is residency time measurable? */
#define CPUIDLE_FLAG_COUPLED    (0x02) /* state applies to multiple cpus */

#define CPUIDLE_DRIVER_FLAGS_MASK (0xFFFF0000)

intel_idle already uses a driver-specific flag:

#define CPUIDLE_FLAG_TLB_FLUSHED        0x10000

This patch just uses 4 more bits along with that one.

Sure, if we run out of space, we can add an additional field.
But I don't see an immediate need for it.

thanks,
Len Brown
Intel Open Source Technology Center

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Daniel Lezcano Feb. 1, 2013, 10:16 p.m. UTC | #3

On 02/01/2013 07:40 PM, Len Brown wrote:
> On 02/01/2013 03:44 AM, Daniel Lezcano wrote:
>> On 02/01/2013 05:11 AM, Len Brown wrote:
>>> From: Len Brown <len.brown@intel.com>
>>>
>>> The commit, 4202735e8ab6ecfb0381631a0d0b58fefe0bd4e2
>>> (cpuidle: Split cpuidle_state structure and move per-cpu statistics fields)
>>> observed that the MWAIT flags for Cn on every processor to date were the
>>> same, and created get_driver_data() to supply them.
>>>
>>> Unfortunately, that assumption is false, going forward.
>>> So here we restore the MWAIT flags to the cpuidle_state table.
>>> However, instead restoring the old "driver_data" field,
>>> we put the flags into the existing "flags" field,
>>> where they probalby should have lived all along.
>>
>> Removing the driver_data is a good thing but I am not sure it is the
>> case by moving the MWAIT flags to the cpuidle_state's flags field. We
>> are mixing arch specific flags with a generic code.
>>
>> This is prone to errors because new flags could appear for the cpuidle
>> core code and could collide with the arch specific flags.
>>
>> Wouldn't make sense to add a private field in the struct cpuidle_state
>> structure to let the driver/arch specific to store whatever is needed ?
>>
>> struct cpuidle_state {
>>
>> 	...
>> 	unsigned long private;
>> 	...
>>
>> }
> 
> The top half of the flags are reserved for the driver,
> as noted by the definition of CPUIDLE_DRIVER_FLAGS_MASK
> with the generic flag definitions:
> 
> #define CPUIDLE_FLAG_TIME_VALID (0x01) /* is residency time measurable? */
> #define CPUIDLE_FLAG_COUPLED    (0x02) /* state applies to multiple cpus */
> 
> #define CPUIDLE_DRIVER_FLAGS_MASK (0xFFFF0000)
> 
> intel_idle already uses a driver-specific flag:
> 
> #define CPUIDLE_FLAG_TLB_FLUSHED        0x10000
> 
> This patch just uses 4 more bits along with that one.

Ok. In this case it would make sense to move this flag out of the
generic core code to the intel_idle.c file ? and move the [dec/en]coding
macro flags_2_MWAIT_EAX and MWAIT_EAX_2_flags (with a more appropriate
name for a generic code) to the cpuidle.h file ?

  -- Daniel



> Sure, if we run out of space, we can add an additional field.
> But I don't see an immediate need for it.
> 
> thanks,
> Len Brown
> Intel Open Source Technology Center
>

Len Brown Feb. 2, 2013, 2:16 a.m. UTC | #4

>> intel_idle already uses a driver-specific flag:
>>
>> #define CPUIDLE_FLAG_TLB_FLUSHED        0x10000
>>
>> This patch just uses 4 more bits along with that one.
> 
> Ok. In this case it would make sense to move this flag out of the
> generic core code to the intel_idle.c file ?

This flag is already local to intel_idle.c.
If another architecture finds it useful,
then yes, it would make sense to move it to the shared header.

> and move the [dec/en]coding
> macro flags_2_MWAIT_EAX and MWAIT_EAX_2_flags (with a more appropriate
> name for a generic code) to the cpuidle.h file ?

I think that a driver's private flag definitions
should remain local to the driver.  It makes no sense
to pollute the name space of other drivers for stuff
that doesn't mean anything to them.  MWAIT is pretty
specific to x86 -- and re-naming it to something 'generic'
isn't going to make the code easier to read.

thanks,
Len Brown, Intel Open Source Technology Center


--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Daniel Lezcano Feb. 2, 2013, 9:44 a.m. UTC | #5

On 02/02/2013 03:16 AM, Len Brown wrote:
> 
>>> intel_idle already uses a driver-specific flag:
>>>
>>> #define CPUIDLE_FLAG_TLB_FLUSHED        0x10000
>>>
>>> This patch just uses 4 more bits along with that one.
>>
>> Ok. In this case it would make sense to move this flag out of the
>> generic core code to the intel_idle.c file ?
> 
> This flag is already local to intel_idle.c.
> If another architecture finds it useful,
> then yes, it would make sense to move it to the shared header.

Oh, right. Sorry I puzzled myself with the name. I was convinced it was
in the shared header.

>> and move the [dec/en]coding
>> macro flags_2_MWAIT_EAX and MWAIT_EAX_2_flags (with a more appropriate
>> name for a generic code) to the cpuidle.h file ?
> 
> I think that a driver's private flag definitions
> should remain local to the driver.  It makes no sense
> to pollute the name space of other drivers for stuff
> that doesn't mean anything to them.  MWAIT is pretty
> specific to x86 -- and re-naming it to something 'generic'
> isn't going to make the code easier to read.

Ok, let me rephrase it because I think how it was presented was not clear.

As we want to use the half of the state flags for private purpose, I
suggested to add the encoding/decoding function in the shared header file.

The mwait eax flags are not encoded and the CPUIDLE_FLAG_TLB_FLUSHED is
encoded.

I suggested to unify both and to use an encoding function from the
shared header file.

#define CPUIDLE_PRIVATE_FLAGS(_flags_) \
	((_flags_) << 16) & CPUIDLE_DRIVER_FLAGS_MASK

For example:

#define FLAG_TLB_FLUSHED CPUIDLE_PRIVATE_FLAGS(0x1)
#define FLAG_MWAIT_C1    CPUIDLE_PRIVATE_FLAGS(0x0)
#define FLAG_MWAIT_C2    CPUIDLE_PRIVATE_FLAGS(0x10)
#define FLAG_MWAIT_C3    CPUIDLE_PRIVATE_FLAGS(0x20)
#define FLAG_MWAIT_C4    CPUIDLE_PRIVATE_FLAGS(0x30)
#define FLAG_MWAIT_C5    CPUIDLE_PRIVATE_FLAGS(0x40)
#define FLAG_MWAIT_C6    CPUIDLE_PRIVATE_FLAGS(0x52)

And then:

...

.flags = FLAG_TLB_FLUSHED | FLAG_MWAIT_C2 | CPUIDLE_FLAG_TIME_VALID

...

But in the idle function, you need to retrieve the 'value' of the EAX
not a flag, so there is the need for an extra macro conversion and mask
the TLB flag.

Well, this is a detail, so feel free to ignore this suggestion :)

Thanks
  -- Daniel

Len Brown Feb. 2, 2013, 8:18 p.m. UTC | #6

>> I think that a driver's private flag definitions
>> should remain local to the driver.  It makes no sense
>> to pollute the name space of other drivers for stuff
>> that doesn't mean anything to them.  MWAIT is pretty
>> specific to x86 -- and re-naming it to something 'generic'
>> isn't going to make the code easier to read.
> 
> Ok, let me rephrase it because I think how it was presented was not clear.
> 
> As we want to use the half of the state flags for private purpose, I
> suggested to add the encoding/decoding function in the shared header file.
> 
> The mwait eax flags are not encoded and the CPUIDLE_FLAG_TLB_FLUSHED is
> encoded.
> 
> I suggested to unify both and to use an encoding function from the
> shared header file.
> 
> #define CPUIDLE_PRIVATE_FLAGS(_flags_) \
> 	((_flags_) << 16) & CPUIDLE_DRIVER_FLAGS_MASK
> 
> For example:
> 
> #define FLAG_TLB_FLUSHED CPUIDLE_PRIVATE_FLAGS(0x1)
> #define FLAG_MWAIT_C1    CPUIDLE_PRIVATE_FLAGS(0x0)
> #define FLAG_MWAIT_C2    CPUIDLE_PRIVATE_FLAGS(0x10)
> #define FLAG_MWAIT_C3    CPUIDLE_PRIVATE_FLAGS(0x20)
> #define FLAG_MWAIT_C4    CPUIDLE_PRIVATE_FLAGS(0x30)
> #define FLAG_MWAIT_C5    CPUIDLE_PRIVATE_FLAGS(0x40)
> #define FLAG_MWAIT_C6    CPUIDLE_PRIVATE_FLAGS(0x52)
> 
> And then:
> 
> ...
> 
> .flags = FLAG_TLB_FLUSHED | FLAG_MWAIT_C2 | CPUIDLE_FLAG_TIME_VALID

I like your syntax better than what I used -- thanks for suggesting it.

I can't use it as-is, however, because I really do need to be able to
support unique parameters on each entry (eg. different flags for C3
on different processors).  When some of the logical and functional
changes in flux settle out I'll come back to re-visit the syntax.

thanks,
-Len Brown, Intel Open Source Technology Center

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[1/4] intel_idle: stop using driver_data for static flags

Commit Message

Comments

Patch