Mark.Haywood at Sun.COM wrote:
> 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.
>
OK, I'll move this to be i86pc specific.
>>> - 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.
>
For now I can't imagine it's necessary. All the access should be done in
acpi_idle().
-Aubrey
