Li, Aubrey wrote: > Mark.Haywood wrote: > > >>> 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? >>> >>> >> I just think it would have been better to have had this stuff disabled >> by default and then provide a boolean to enable it if the user wanted >> it. Otherwise, I think we are providing data that is basically useless >> to most users. *And* if the BIOS developers ever got smart and >> made some >> real decisions based on whether or not the OSPM supported certain >> features, we'd be lying to them. >> >> >> >>>> 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 >>> >>> >> Again, I'd prefer we had the option disabled by default if we don't >> really support C-states for the system. >> > > I still prefer we keep the current way. At least Jason will support my > opinion. > Because if we disable this by default, his box will not support P-state > forever, ;) > Yes, I think you are right. For the sake for folks like Jason I think we'll have to leave it as it is. We might as well just enable all the _PDC bits so that our users don't get screwed over by this mechanism any more. I do wonder if there are any Linux/Windows users out there with versions that don't support C-states. If so they are probably screwed too until they upgrade their OS.
> > >>> 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? >>> >>> >> I don't think the _PDC was intended to be used that way, "Yeah, we >> have the ability, but hehe we're not really supporting it for this >> processor. Got you!!!" But then again, I think the mechanism is >> totally >> broken. It >> hardly matters whether the OSPM supports the stuff it says it >> supports. Just tell the BIOS you can do it all. >> >> >> >> Let's just replace the _PDC with a boolean that tells the BIOS whether >> or not is should load the CPU PM related tables or not. Unfortunately, >> we can't really do that since some BIOS developers return different >> tables depending on whether the OSPM can support MP. This was probably >> the only useful functionality provided by the _PDC. >> >> I've made a bigger deal out of this than it deserves. But the _PDC has >> caused more trouble than anything else. I sympathize for anyone who is >> running an older OS that does not support deep C-states, >> because they no >> longer have SpeedStep support on a system like the one >> mentioned in this >> thread. >> >> > > I agree the _PDC mechanism is totally broken. On Jason's box, SSDT table > needs both P-state and C-state _PDC setting to be loaded. There is no better > way to deal with this case except enabling all the bits or give the P-state > support up. > I guess his box is at least tested by Windows to make sure P-state works > properly. > BIOS guys only care about Windows, they don't care any other OSes. Maybe Jason > can have a try some linux liveCD like Ubuntu to see if P-state works. I just > want to > make solaris pm competitive to the other OSes. I will not persist my opinion > if there > is a better reason to disable C-state PDC bits by default. > I'm not asking for it to be changed. I was just surprised that you'd coded it that way to begin with. Seems wrong to me. But it works out to benefit broken BIOS implementations. At least for the time being it does. Mark > Thanks, > -Aubrey >
