Li, Aubrey wrote: > 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. >
Not sure I understand your point. We want to observe cstate data on systems for which we don't support C-states? Also, not sure how this makes the cstate cache function any more robust. We don't have to execute cpu_deep_cstates_supported() as part of the caching function? We're going to run it as part of the CPU PM initialization anyway. > 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. > Actually, it allows the user to disable deep cstate, not enable it. So, I don't see its pertinence in this discussion. Besides which, I don't see how it makes any difference in deciding whether or not to use cpu_deep_cstates_supported() to decide how to set the _PDC bits. > And, it helps to fix bugs to enable P-state, hehe, :D > Yes. The BIOS bugs that exist because the BIOS developers don't understand the _PDC bits. > Anyway, I didn't see a reason we can't do this in the current way. > So effectively we *are* enabling all the _PDC bits all the time (except in the case where we know Solaris won't support SpeedStep). Sigh. The _PDC was a bad idea. In any case ... Jason, SpeedStep should work for you starting with build 110. Mark > Thanks, > -Aubrey >
