RE: [PATCH]OMAP3 PM: Fix for DSP Crash at OPP 1 and 2 under DVFS+SR operation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Cousson, Benoit
> Sent: Friday, December 04, 2009 4:27 PM
> To: Sripathy, Vishwanath; linux-omap@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH]OMAP3 PM: Fix for DSP Crash at OPP 1 and 2 under DVFS+SR
> operation
> 
> 
> >From: Sripathy, Vishwanath
> >
> >Benoit
> >
> >>
> Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 Villeneuve Loubet. 036
> 420 040 R.C.S Antibes. Capital de EUR 753.920
> 
> -----Original Message-----
> 
> >> From: Cousson, Benoit
> >> Sent: Friday, December 04, 2009 3:46 PM
> >> To: Sripathy, Vishwanath; linux-omap@xxxxxxxxxxxxxxx
> >> Subject: RE: [PATCH]OMAP3 PM: Fix for DSP Crash at OPP 1 and 2 under
> >DVFS+SR
> >> operation
> >>
> >> Hi Vishwanath,
> >>
> >> >From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap-
> >> >owner@xxxxxxxxxxxxxxx] On Behalf Of Sripathy, Vishwanath
> >> >
> >> >From: Shweta Gulati <shweta.gulati@xxxxxx>
> >> >
> >> >DSP usage at VDD1 OPP1 and OPP2 with Smartreflex enabled and any MM UCs
> >> >running DSP codec was earlier restricted as DSP crashed.
> >> >The root cause is wrong DPLL1/DPLL2 Bypass clock at VDD1 OPP1 and OPP2.
> >> >The workaround is:
> >> >	For DPLL2 (DSP) select CORECLK/4 as DPLL2 bypass clock for 3430.
> >> >	Select CORECLK/2 as DPLL2 bypass clock for 3630.
> >> >During DPLL2 relock phase, DSP clock will be 83MHz/200MHz which is
> >always
> >> >OK
> >> >irrespective of Vdd1 voltage.
> >> >
> >> >For DPLL1 (MPU), prior to any DVFS transition to OPP1, select CORECLK/4
> >> >	(CORECLK/2 for 3630) as DPLL1 bypass clock.
> >> >	For other OPPs select CORECLK/2 (CORECLK/1 for 3630) as DPLL1 bypass
> >> >	clock.
> >>
> >> I think that you should not rely on the OMAP version to select the
> >correct
> >> bypass freq. Both 3430 and 3630 can support CORECLK/2 at OPP50. CORECLK/4
> >> should be used only if OPP25 is selected. The fact that OPP25 will not be
> >> used on 3630 should not be captured here.
> >> The divider selection should be only based on the OPP. It will work the
> >same
> >> way for all OMAP3 variants.
> >>
> >
> >The problem is in 3430, OPP1 is OPP25 where as in 3630 OPP1 is OPP50 (as
> >OPP25 is removed). So in the code, we still have to distinguish between
> >3430 and 3630.
> 
> Yes, but you do not have to care about the OMAP version at that level. In case of
> 3630, the OPP25 will be removed from the OPP table, so you'll just have to care about
> the OPP level. The fact that the OPP25 is there or not should not be handled here. For
> the moment, you can just check the target level.
> If VDD1_OPP1 is selected then you divide the CORE by 4 otherwise you divide the
> CORE by 2.

For 3630 I feel keeping Bypass divider at 4 @ OPP1 and 2 for others is suboptimal given that MPU can run at CORECLK/2@OPP1 and CORECLK for other OPPs.
When we can get better performance with a additional cost of couple of comparisons, I feel that's better (Note that MPU enters and comes out of standby very frequently in the system which means number of times MPU is fed with DPLL bypass clock is high compared to IVA. So it's better MPU is fed with higher bypass clock frequency.

> Ideally you should not even rely on the OPP, but on the frequency.
> 

> Keep in mind, that since the OPP layer is being completely rewritten by Nishanth and
> Kevin, it will have to change at that time to adapt that to the new API, and to a
> frequency based selection.
> 
> Regards,
> Benoit
> 
> 
> >> Regards,
> >> Benoit
> >>
> >> >These configurations are typically set in bootloader. However
> >bootloaders
> >> >may
> >> >mess up configuration and kernel with this chang ensures that system is
> >in
> >> >a
> >> >known state.
> >> >
> >> >Signed-off-by: Vishwanath BS <vishwanath.bs@xxxxxx>
> >> >---
> >> > arch/arm/mach-omap2/cm-regbits-34xx.h |    4 ++--
> >> > arch/arm/mach-omap2/pm34xx.c          |   19 +++++++++++++++++++
> >> > arch/arm/mach-omap2/resource34xx.c    |   20
> ++++++++++++++++++++
> >> > 3 files changed, 41 insertions(+), 2 deletions(-)
> >> > mode change 100644 => 100755 arch/arm/mach-omap2/cm-regbits-34xx.h
> >> > mode change 100644 => 100755 arch/arm/mach-omap2/pm34xx.c
> >> > mode change 100644 => 100755 arch/arm/mach-omap2/resource34xx.c
> >> >
> >> >diff --git a/arch/arm/mach-omap2/cm-regbits-34xx.h b/arch/arm/mach-
> >> >omap2/cm-regbits-34xx.h
> >> >index 6f2802b..0cf9a5d
> >> >--- a/arch/arm/mach-omap2/cm-regbits-34xx.h
> >> >+++ b/arch/arm/mach-omap2/cm-regbits-34xx.h
> >> >@@ -81,7 +81,7 @@
> >> >
> >> > /* CM_CLKSEL1_PLL_IVA2 */
> >> > #define OMAP3430_IVA2_CLK_SRC_SHIFT			19
> >> >-#define OMAP3430_IVA2_CLK_SRC_MASK			(0x3 << 19)
> >> >+#define OMAP3430_IVA2_CLK_SRC_MASK			(0x7 << 19)
> >> > #define OMAP3430_IVA2_DPLL_MULT_SHIFT			8
> >> > #define OMAP3430_IVA2_DPLL_MULT_MASK			(0x7ff << 8)
> >> > #define OMAP3430_IVA2_DPLL_DIV_SHIFT			0
> >> >@@ -126,7 +126,7 @@
> >> >
> >> > /* CM_CLKSEL1_PLL_MPU */
> >> > #define OMAP3430_MPU_CLK_SRC_SHIFT			19
> >> >-#define OMAP3430_MPU_CLK_SRC_MASK			(0x3 << 19)
> >> >+#define OMAP3430_MPU_CLK_SRC_MASK			(0x7 << 19)
> >> > #define OMAP3430_MPU_DPLL_MULT_SHIFT			8
> >> > #define OMAP3430_MPU_DPLL_MULT_MASK			(0x7ff << 8)
> >> > #define OMAP3430_MPU_DPLL_DIV_SHIFT			0
> >> >diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-
> omap2/pm34xx.c
> >> >index c328164..f260072
> >> >--- a/arch/arm/mach-omap2/pm34xx.c
> >> >+++ b/arch/arm/mach-omap2/pm34xx.c
> >> >@@ -802,6 +802,25 @@ static void __init omap3_d2d_idle(void)
> >> >
> >> > static void __init prcm_setup_regs(void)
> >> > {
> >> >+	u32 cm_clksel1_mpu, cm_clksel1_iva2;
> >> >+
> >> >+	/*set Bypass clock dividers for MPU and IVA */
> >> >+	cm_clksel1_mpu = cm_read_mod_reg(MPU_MOD, CM_CLKSEL1);
> >> >+	cm_clksel1_iva2 = cm_read_mod_reg(OMAP3430_IVA2_MOD,
> CM_CLKSEL1);
> >> >+	if (cpu_is_omap3630()) {
> >> >+		cm_clksel1_iva2 = (cm_clksel1_iva2 &
> >> >~(OMAP3430_IVA2_CLK_SRC_MASK)) |
> >> >+					(0x2 <<
> OMAP3430_IVA2_CLK_SRC_SHIFT);
> >> >+		cm_clksel1_mpu = (cm_clksel1_mpu &
> >> >~(OMAP3430_MPU_CLK_SRC_MASK)) |
> >> >+					(0x1 <<
> OMAP3430_MPU_CLK_SRC_SHIFT);
> >> >+	} else if (cpu_is_omap34xx()) {
> >> >+		cm_clksel1_iva2 = (cm_clksel1_iva2 &
> >> >~(OMAP3430_IVA2_CLK_SRC_MASK)) |
> >> >+					(0x4 <<
> OMAP3430_IVA2_CLK_SRC_SHIFT);
> >> >+		cm_clksel1_mpu = (cm_clksel1_mpu &
> >> >~(OMAP3430_MPU_CLK_SRC_MASK)) |
> >> >+					(0x2 <<
> OMAP3430_MPU_CLK_SRC_SHIFT);
> >> >+		}
> >> >+	cm_write_mod_reg(cm_clksel1_iva2, OMAP3430_IVA2_MOD,
> CM_CLKSEL1);
> >> >+	cm_write_mod_reg(cm_clksel1_mpu, MPU_MOD, CM_CLKSEL1);
> >> >+
> >> > 	/* XXX Reset all wkdeps. This should be done when initializing
> >> > 	 * powerdomains */
> >> > 	prm_write_mod_reg(0, OMAP3430_IVA2_MOD, PM_WKDEP);
> >> >diff --git a/arch/arm/mach-omap2/resource34xx.c b/arch/arm/mach-
> >> >omap2/resource34xx.c
> >> >index 04be4d2..8c3f2b3
> >> >--- a/arch/arm/mach-omap2/resource34xx.c
> >> >+++ b/arch/arm/mach-omap2/resource34xx.c
> >> >@@ -242,9 +242,29 @@ static int program_opp_freq(int res, int
> >target_level,
> >> >int current_level)
> >> > {
> >> > 	int ret = 0, l3_div;
> >> > 	int *curr_opp;
> >> >+	u32 cm_clksel1_mpu;
> >> >
> >> > 	lock_scratchpad_sem();
> >> > 	if (res == VDD1_OPP) {
> >> >+		if (target_level == VDD1_OPP1) {
> >> >+			cm_clksel1_mpu = cm_read_mod_reg(MPU_MOD,
> >> CM_CLKSEL1);
> >> >+			if (cpu_is_omap3630())
> >> >+				cm_clksel1_mpu = (cm_clksel1_mpu &
> >> >~(OMAP3430_MPU_CLK_SRC_MASK)) |
> >> >+							(0x2 <<
> >> >OMAP3430_MPU_CLK_SRC_SHIFT);
> >> >+			else if (cpu_is_omap34xx())
> >> >+				cm_clksel1_mpu = (cm_clksel1_mpu &
> >> >~(OMAP3430_MPU_CLK_SRC_MASK)) |
> >> >+							(0x4 <<
> >> >OMAP3430_MPU_CLK_SRC_SHIFT);
> >> >+			cm_write_mod_reg(cm_clksel1_mpu, MPU_MOD,
> CM_CLKSEL1);
> >> >+		} else if ((current_level == VDD1_OPP1) && (target_level !=
> >> >VDD1_OPP1)) {
> >> >+			cm_clksel1_mpu = cm_read_mod_reg(MPU_MOD,
> >> CM_CLKSEL1);
> >> >+			if (cpu_is_omap3630())
> >> >+				cm_clksel1_mpu = (cm_clksel1_mpu &
> >> >~(OMAP3430_MPU_CLK_SRC_MASK)) |
> >> >+							(0x1 <<
> >> >OMAP3430_MPU_CLK_SRC_SHIFT);
> >> >+			else if (cpu_is_omap34xx())
> >> >+				cm_clksel1_mpu = (cm_clksel1_mpu &
> >> >~(OMAP3430_MPU_CLK_SRC_MASK)) |
> >> >+							(0x2 <<
> >> >OMAP3430_MPU_CLK_SRC_SHIFT);
> >> >+			cm_write_mod_reg(cm_clksel1_mpu, MPU_MOD,
> CM_CLKSEL1);
> >> >+		}
> >> > 		curr_opp = &curr_vdd1_opp;
> >> > 		clk_set_rate(dpll1_clk, mpu_opps[target_level].rate);
> >> > 		clk_set_rate(dpll2_clk, dsp_opps[target_level].rate);
> >> >--
> >> >1.5.6.3
> >> >
> >> >--
> >> >To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> >> >the body of a message to majordomo@xxxxxxxxxxxxxxx
> >> >More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> >> Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 Villeneuve
> >Loubet. 036
> >> 420 040 R.C.S Antibes. Capital de EUR 753.920
> >>

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux