Li, Aubrey wrote:
> Hi Mark,
>
> Mark.Haywood wrote:
>
>> Hi Aubrey,
>>
>> Sorry it's taken me so long to take a look at this. I think your
>> changes look very good. A couple of very minor comments:
>
> Thanks for the comments.
>
>> - In cpuvar.h, I'm not sure if cpu_max_cstates should be added
>> directly to the cpu structure. Is this field going to be used
>> by SPARC? If so, then it's fine. Otherwise, I think it might
>> need to move to the i86pc version of machcpu.
>>
> Yeah, that's a good point and is what I want to ask.
> Will the different idle level be supported by SPARC?
>
> Another issue is, what of C-state need be exported in kstat info?
> Cpu max cstate, latency, power, anything else?
> Any thoughts?
Those are questions better answered by Eric and Bill. But I believe
SPARC just support C0 and C1.
>> - cpu_idle.c:
>>
>> Really just a nit. Comments refer to SpeedStep?
>
> Sure, ;)
>
>> - Have you given any thought on how the kernel is going to
>> access the ACPI C-state data?
>
> Here is what I am thinking now. Only an acpi_idle() function will be
> offered to the kernel.
> If deeper c-state is supported, idle_cpu will be acpi_idle().
>
> So here is acpi_idle() prototype.
>
> acpi_idle()
> {
> switch(next_idle_type)
> case C1:
> idle_cpu = cpu_idle or cpu_idle_mwait
> case C2:
> idle_cpu = acpi_idle_C2
> case C3:
> idle_cpu = acpi_idle_C3
>
> idle_policy();
> }
> Any suggestions?
Sure that looks good. My question was more directed at how the kernel is
going to access C-state data/functions in the driver. The P-state
support works because Sun's DDI includes a dev_ops devo_power entry.
>
> As for the C/P/T state ops, currently I don't see any necessary to keep
> them different.
> C/T state driver can set the function point to be NULL if it's not used.
Agreed.
> Thanks,
> -Aubrey
