[PATCH 1/2] powerpc/85xx: add hardware automatically enter altivec idle state

[Scott Wood]

On Sun, 2013-08-18 at 21:53 -0500, Wang Dongsheng-B40534 wrote:
> Thanks for your feedback.
> 
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Saturday, August 17, 2013 12:51 AM
> > To: Kumar Gala
> > Cc: Wang Dongsheng-B40534; linuxppc-dev at lists.ozlabs.org
> > Subject: Re: [PATCH 1/2] powerpc/85xx: add hardware automatically enter
> > altivec idle state
> > 
> > On Fri, 2013-08-16 at 06:02 -0500, Kumar Gala wrote:
> > > On Aug 16, 2013, at 2:23 AM, Dongsheng Wang wrote:
> > >
> > > > From: Wang Dongsheng <dongsheng.wang at freescale.com>
> > > >
> > > > Each core's AltiVec unit may be placed into a power savings mode
> > > > by turning off power to the unit. Core hardware will automatically
> > > > power down the AltiVec unit after no AltiVec instructions have
> > > > executed in N cycles. The AltiVec power-control is triggered by
> > hardware.
> > > >
> > > > Signed-off-by: Wang Dongsheng <dongsheng.wang at freescale.com>
> > >
> > > Why treat this as a idle HW governor vs just some one time setup at
> > boot of the time delay?
> > 
> > It is being done as one-time setup, despite the function name.
> > 
> > Maybe it should be moved into __setup/restore_cpu_e6500 (BTW, we really
> > should refactor those to reduce duplication) with the timebase bit
> > number hardcoded rather than a time in us.
> > 
> The frequency of the different platforms timebase is not the same.

It's close enough.  Remember, this number is a vague initial guess, not
something that's been carefully calibrated.  Though, it would make it
harder to substitute the number with one that's been more carefully
calibrated...  but wouldn't different chips possibly have different
optimal delays anyway?

> If we use it, we need to set different timebase bit on each platform.
> That is why I did not put the code in __setup/restore_cpu_e6500, I need
> use tb_ticks_per_usec to get timebase bit. So we only need to set a time
> for each platform, and not set different timebase bit.

It just seems wrong to have an ad-hoc mechanism for running
core-specific code when we have cputable...  If we really need this,
maybe we should add a "cpu_setup_late" function pointer.

With your patch, when does the power management register get set when
hot plugging a cpu?

> > As for the PVR check, the upstream kernel doesn't need to care about
> > rev1, so knowing it's an e6500 is good enough.
> > 
> But AltiVec idle & PW20 cannot work on rev1 platform.
> We didn't have to deal with it?

Upstream does not run on rev1.

-Scott