Mark.Haywood wrote:

>>>> sigh, buggy BIOS. "0x1B" checking will load "SSDT" table. Maybe
>>>> there is something else in the table need to be loaded to support
>>>> deep c-state, although I saw _CST(C1ST, C2ST, C3ST, C4ST) are
>>>> already there. 
>>>> 
>>>> This issue should be fixed in 110 and later build due to the deep
>>>> c-states support, by accident, :D. So no more work need to be done.
>>>> 
>>>> 
>>> Not necessarily. Only if cpu_deep_cstates_supported() returns
>>> B_TRUE. 
>>> 
>>> 
>> 
>> No, PDC setting doesn't need to do that check.
>> 
> 
> Probably doesn't matter much, but besides the comment being wrong:
> 
> 
> /*
>         * If we support deep cstates on this processor, then set the
>         * correct cstate_ops for the processor and enable appropriate
>         * _PDC
> bits.
>         */
>        mach_state->ms_cstate.cma_ops = &cpu_idle_ops;
>        cpupm_intel_pdccap |= CPUPM_INTEL_PDC_C1_HALT |
>            CPUPM_INTEL_PDC_C2C3_MP | CPUPM_INTEL_PDC_C1_FFH;
> 
> I do wonder why we are setting the bits if we don't support it? And
> unless I'm misreading the code, we even try to read the C-state data
> even in the case where we don't support the processor?
> 

For development/observation purpose. We can see the cstate data in kstat.
This helps to diagnose system. And it also helps us to make the cstate cache 
function robust.

We also have "idle_cpu_no_deep_c" mechanism. If the user set it in kmdb during
boot, he/she still can get deep cstate support without reboot the system.

And, it helps to fix bugs to enable P-state, hehe, :D

Anyway, I didn't see a reason we can't do this in the current way.

Thanks,
-Aubrey

Reply via email to