Li, Aubrey wrote: > Mark.Haywood wrote: > > >> 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? >> > > Cstate data including Cx power consumption, latency, which are interesting > to the developer. And some systems have two C3, not duplicate, the two > are valid, these info are useful. > > C-state is more complicated than P-state. It depends on HPET and only support > on NHM now, and may not need HPET in future, or support pre-NHM platform by > another time source, I'd like to separate the cstate data and c-state support. > > Is there a reason why we can't do that? > > >> Also, not sure how this makes the cstate cache function any more robust. >> > > Currently, if caching cstate data only works when cpu_deep_cstates_supported() > returns true, we'll lost the chance to test it on the different kinds of > machine. For > example, we can not find the panic issue reported several days ago on Toshiba > M9/M10. Does this make sense? :p > > >> 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 just export the data for observation, like we export the turbo info, Is > there > a reason why we can't do that? > > >>> 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. >> > > Actually I often use this way to enable deep cstate when I did the testing, > to save the reboot time, :) > > > >> 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. >> > > No difference to set _PDC, but be different to install idle function. > This is enough, I think. > > >>> 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. >> > > As long as the OSPM has the ability/function to support all of what > _PDC bits indicate, why we can't just enable all the bits? > > This might be the case post snv_110, but pre snv_110 since Deep C States were not supported there was no reason to enable them right?
Anup >> In any case ... Jason, SpeedStep should work for you starting >> with build >> 110. >> >> > > hehe, :D > > -Aubrey > _______________________________________________ > pm-discuss mailing list > pm-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/pm-discuss >
