Hi Bill,

Thanks for the quick response, :-)

Bill.Holler <> wrote:

> Hi Aubrey,
> 
> We probably should have used something like this generic
> cpmpm_callback interface when we originally implemented
> p/t/c-state callbacks via pmconfig.  :-)

Do we have a generic one?
c-state callback is cpu_deep_idle_callb, event-mode pstate doesn't
use callback mechanism, neither does poll-mode pstate. Currently,
we don't have an option in /etc/power.conf to tune t-state.

Did I miss something?

> 
> 
> Should the per-CPU data structures be in the mcpu_pm_mach_state
> structures instead of the struct machcpu?
> All cpupm (cpu power) related data structures are currently
> consolidated in the mcpu_pm_mach_state.

I think the name mcpu_pm_mach_"state" means pm state, including
c/p/t-state. So I put the policy at the same level of it. I'm okay to move
this into mcpu_pm_mach_state. 

> 
> This interface does not allow for generic power policy
> settings that are system wide.

Do you need an interface to manage other periopheral devices as well?
Currently I only implement the cpu level. The system wide profile could
call cpu level policy. 

>  Do we want this infrastructure
> to also handle other policy cases that are not per-CPU?
> Perhaps that could be added later.

If you are meaning CPU, what policy cases that are not per-CPU in your
mind?

> 
> cpu_pm_policy_callb_id will not accommodate multiple
> callbacks.  Some other data structure will be needed to
> add/delete multiple callbacks.  One option would be to add
> a callback id field in cpupm_policy_callback_t.  That would
> allow the callback to be deleted at the same time the
> cpupm_policy_callback_t is deleted.

cpu_pm_policy_callb_id here is to avoid that every cpu register
a callback, like cpu_deep_idle_callback, just one is enough.
All the pm feature callback will be on the same list and be called
by this callback. For what case multiple callbacks are needed here?

> 
> Adding setting to /etc/power.conf will require a PSARC case.
>

Yeah, Are you satisfied with the current proposal?
----------/etc/power.conf----------------
new keyword: "cpu-power-policy", 
Four options
1) perf-bias
2) balanced
3) power-bias
4) default

> 
> 
> * The default cpupm_iepb_policy policy will be EPB_MAX_PERF.
> I could not get approval to put back a default change which
> regressed performance.

okay, minimal impact to the code impl. I can change it anytime.

Thanks,
-Aubrey

> 
> Regards,
> Bill
> 
> 
> On 10/27/09 02:14, Li, Aubrey wrote:
>> Hi,
>> 
>> When we enable intel energy performance bias feature, we found the
>> power 
>> profile implementation is necessary. Here I did a draft for cpu
>> level power policy.
>> http://cr.opensolaris.org/~aubrey/cpu_power_policy_v1/ 
>> 
>> The proposal added a new keyword to /etc/power.conf
>> "cpu-power-policy", 
>> 
>> And we have 4 options for this new keyword:
>> 1) perf-bias
>> 2) balanced
>> 3) power-bias
>> 4) default, the same as balanced
>> 
>> /etc/power.conf accepts the user input and passes the prefered policy
>> to the kernel thru ioctl. Then pm_ioctl calls the callback to walk a
>> cpu 
>> power policy list. Every cpu pm feature which wants to be adjusted by
>> this option and verified to be supported will register its callback
>> function 
>> to the list, so that it can be called and adjusted by pmconfig.
>> --------------------------------------------------------
>>      /etc/power.conf |
>>     pm_ioctl(cpu_power_policy, policy)
>>      |
>> cpu_power_policy_callb (policy)
>>      |
>>      ----> registered pm feature callback 1 (ENERGY_PERF_BIAS)       |
>>      ----> registered pm feature callback 2
>>      ...
>> ---------------------------------------------------------
>> Currently, only energy_perf_bias feature is registered, because my
>> intention is 
>> to support dynamically adjusting energy_perf_bias MSR(without
>> reboot). I guess 
>> we probably can add p/t/c-state support later. When we add
>> p/t/c-state support, 
>> my quick thought is, this option will override "cpupm" and
>> "cpu-deep-idle" setting. 
>> 
>> Welcome your any comments and suggestions.
>> 
>> Thanks,
>> -Aubrey

Reply via email to