Simon Burge wrote:

> Quentin Garnier wrote:
> 
> > On Thu, Aug 31, 2006 at 10:17:18PM +0000, Michael van Elst wrote:
> > > juan@xtrarom.org (Juan RP) writes:
> > >
> > > >Anybody could please review it before committing? at least this version
> > > >of est.c will detect the highest and lowest frequency in CPUs that we
> > > >don't know the table and will work in more CPUs than before.
> > >
> > > On a T2300 it works somewhat. I have to run sysctl several times
> > > to get the speed changed. I guess the speed needs to be set on
> > > both cores indvidually.
> > 
> > This is actually the behaviour I expected!  It actually depends on the
> > state of the other core.  Thanks for confirming I wasn't reading the
> > datasheet wrong :)
> 
> The spec said that on a Core Duo that the greater of the two target
> speeds would be detected.  On my T2400, the second core always had the
> lowest frequency selected, and any changes made with sysctl only seemed
> to affect the first core.  Note that I haven't verified which core the
> sysctl was running on yet - I'll add some instrumentation to check that.
> 
> I'm not sure if the behaviour I've seen so far blind luck or not...

And blind luck wins!

I've got a little script that cycles through the frequencies every 2
seconds, and prints some info gathered from the kernel.  I then started
a "make -j4" to keep the system busy.  Note here that the MSR_PERF_CTL
value of 0x613 is the value for the lowest CPU speed (1000MHz).

	machdep.est.frequency.target: 1333 -> 1833
	machdep.est.frequency.current = 1833
	cpu0: set MSR_PERF_CTL to 0xb2c
	cpu0 perfctl = 0xb2c
	cpu1 perfctl = 0x613

	machdep.est.frequency.target: 1833 -> 1667
	machdep.est.frequency.current = 1667
	cpu0: set MSR_PERF_CTL to 0xa28
	cpu0 perfctl = 0xa28
	cpu1 perfctl = 0x613

	...

	machdep.est.frequency.target: 1000 -> 1167
	cpu1: set MSR_PERF_CTL to 0x719
	cpu0 perfctl = 0x81e
	cpu1 perfctl = 0x719

	...

	machdep.est.frequency.target: 1333 -> 1167
	machdep.est.frequency.current = 1167
	cpu0: set MSR_PERF_CTL to 0x719
	cpu0 perfctl = 0x719
	cpu1 perfctl = 0x719

	machdep.est.frequency.target: 1167 -> 1000
	machdep.est.frequency.current = 1167
	cpu0: set MSR_PERF_CTL to 0x614
	cpu0 perfctl = 0x614
	cpu1 perfctl = 0x719

In the third entry, we can see that cpu1 handles the sysctl call, and
now MSR_PERF_CTL isn't set to it's lowest possible value.

In the last entry, we can see that cpu0 handles the sysctl call, and
tries to set the slowest frequency but this fails because cpu1's
MSR_PERF_CTL is still set to a higher frequency.  This is the behaviour
that Michael saw, and Quentin was expecting but we hadn't seen until
now.

For Core Duo systems, I guess the easy answer is "do EST only on the
first core".  I think a better fix is to run the EST setting code on all
available CPUs - this will work on Core Duo CPUs as well as normal SMP
machines.  What's the best way to do this?  Just add a flag somewhere
that each CPU checks regularly and runs the EST code, and if so where is
the best place to do this?  I don't think it's high enough priority to
need an IPI for.

Simon.