Patchwork [v3,2/4] powerpc/85xx: add hardware automatically enter altivec idle state

login
register
mail settings
Submitter Dongsheng Wang
Date Sept. 11, 2013, 5:56 a.m.
Message ID <1378879004-2446-2-git-send-email-dongsheng.wang@freescale.com>
Download mbox | patch
Permalink /patch/274118/
State Superseded
Headers show

Comments

Dongsheng Wang - Sept. 11, 2013, 5:56 a.m.
From: Wang Dongsheng <dongsheng.wang@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@freescale.com>
---
*v3:
Assembly code instead of C code.

*v2:
Remove:
delete setup_idle_hw_governor function.
delete "Fix erratum" for rev1.

Move:
move setup_* into __setup/restore_cpu_e6500.

 arch/powerpc/kernel/cpu_setup_fsl_booke.S | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

--
1.8.0
Scott Wood - Sept. 11, 2013, 10:42 p.m.
On Wed, 2013-09-11 at 13:56 +0800, Dongsheng Wang wrote:
> From: Wang Dongsheng <dongsheng.wang@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@freescale.com>
> ---
> *v3:
> Assembly code instead of C code.
> 
> *v2:
> Remove:
> delete setup_idle_hw_governor function.
> delete "Fix erratum" for rev1.
> 
> Move:
> move setup_* into __setup/restore_cpu_e6500.
> 
>  arch/powerpc/kernel/cpu_setup_fsl_booke.S | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> index bfb18c7..3c03c109 100644
> --- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> +++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> @@ -53,11 +53,30 @@ _GLOBAL(__e500_dcache_setup)
>  	isync
>  	blr
> 
> +/*
> + * FIXME - We don't know the AltiVec application scenarios.
> + */
> +#define AV_WAIT_IDLE_BIT		50 /* 1ms, TB frequency is 41.66MHZ */
> +_GLOBAL(setup_altivec_idle)
> +	mfspr	r3, SPRN_PWRMGTCR0
> +
> +	/* Enable Altivec Idle */
> +	oris	r3, r3, PWRMGTCR0_AV_IDLE_PD_EN@h
> +	li	r4, AV_WAIT_IDLE_BIT
> +
> +	/* Set Automatic AltiVec Idle Count */
> +	rlwimi	r3, r4, PWRMGTCR0_AV_IDLE_CNT_SHIFT, PWRMGTCR0_AV_IDLE_CNT
> +
> +	mtspr	SPRN_PWRMGTCR0, r3
> +
> +	blr

The FIXME comment is not clear.  If you mean that we haven't yet done
testing to determine a reasonable default value for AV_WAIT_IDLE_BIT,
then just say that.  Likewise with the FIXME comment in the pw20 patch
-- the uncertainty is due to a lack of investigation, not "because we
don't know the current state of the cpu load".

While some workloads may want a different value than whatever default we
set, that's what the sysfs interface is for.  The only thing missing
here is benchmarking to determine a good overall default.

-Scott
Wang Dongsheng-B40534 - Sept. 12, 2013, 2:27 a.m.
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Thursday, September 12, 2013 6:43 AM
> To: Wang Dongsheng-B40534
> Cc: galak@kernel.crashing.org; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH v3 2/4] powerpc/85xx: add hardware automatically
> enter altivec idle state
> 
> On Wed, 2013-09-11 at 13:56 +0800, Dongsheng Wang wrote:
> > From: Wang Dongsheng <dongsheng.wang@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@freescale.com>
> > ---
> > *v3:
> > Assembly code instead of C code.
> >
> > *v2:
> > Remove:
> > delete setup_idle_hw_governor function.
> > delete "Fix erratum" for rev1.
> >
> > Move:
> > move setup_* into __setup/restore_cpu_e6500.
> >
> >  arch/powerpc/kernel/cpu_setup_fsl_booke.S | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> > index bfb18c7..3c03c109 100644
> > --- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> > +++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
> > @@ -53,11 +53,30 @@ _GLOBAL(__e500_dcache_setup)
> >  	isync
> >  	blr
> >
> > +/*
> > + * FIXME - We don't know the AltiVec application scenarios.
> > + */
> > +#define AV_WAIT_IDLE_BIT		50 /* 1ms, TB frequency is 41.66MHZ
> */
> > +_GLOBAL(setup_altivec_idle)
> > +	mfspr	r3, SPRN_PWRMGTCR0
> > +
> > +	/* Enable Altivec Idle */
> > +	oris	r3, r3, PWRMGTCR0_AV_IDLE_PD_EN@h
> > +	li	r4, AV_WAIT_IDLE_BIT
> > +
> > +	/* Set Automatic AltiVec Idle Count */
> > +	rlwimi	r3, r4, PWRMGTCR0_AV_IDLE_CNT_SHIFT,
> PWRMGTCR0_AV_IDLE_CNT
> > +
> > +	mtspr	SPRN_PWRMGTCR0, r3
> > +
> > +	blr
> 
> The FIXME comment is not clear.  If you mean that we haven't yet done
> testing to determine a reasonable default value for AV_WAIT_IDLE_BIT,
> then just say that.  Likewise with the FIXME comment in the pw20 patch
> -- the uncertainty is due to a lack of investigation, not "because we
> don't know the current state of the cpu load".
> 
> While some workloads may want a different value than whatever default we
> set, that's what the sysfs interface is for.  The only thing missing
> here is benchmarking to determine a good overall default.
> 
Thanks.

Patch

diff --git a/arch/powerpc/kernel/cpu_setup_fsl_booke.S b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
index bfb18c7..3c03c109 100644
--- a/arch/powerpc/kernel/cpu_setup_fsl_booke.S
+++ b/arch/powerpc/kernel/cpu_setup_fsl_booke.S
@@ -53,11 +53,30 @@  _GLOBAL(__e500_dcache_setup)
 	isync
 	blr

+/*
+ * FIXME - We don't know the AltiVec application scenarios.
+ */
+#define AV_WAIT_IDLE_BIT		50 /* 1ms, TB frequency is 41.66MHZ */
+_GLOBAL(setup_altivec_idle)
+	mfspr	r3, SPRN_PWRMGTCR0
+
+	/* Enable Altivec Idle */
+	oris	r3, r3, PWRMGTCR0_AV_IDLE_PD_EN@h
+	li	r4, AV_WAIT_IDLE_BIT
+
+	/* Set Automatic AltiVec Idle Count */
+	rlwimi	r3, r4, PWRMGTCR0_AV_IDLE_CNT_SHIFT, PWRMGTCR0_AV_IDLE_CNT
+
+	mtspr	SPRN_PWRMGTCR0, r3
+
+	blr
+
 _GLOBAL(__setup_cpu_e6500)
 	mflr	r6
 #ifdef CONFIG_PPC64
 	bl	.setup_altivec_ivors
 #endif
+	bl	.setup_altivec_idle
 	bl	__setup_cpu_e5500
 	mtlr	r6
 	blr
@@ -119,6 +138,7 @@  _GLOBAL(__setup_cpu_e5500)
 _GLOBAL(__restore_cpu_e6500)
 	mflr	r5
 	bl	.setup_altivec_ivors
+	bl	.setup_altivec_idle
 	bl	__restore_cpu_e5500
 	mtlr	r5
 	blr