RE: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay

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

 



> From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Reddy, Teerth
> Sent: Friday, December 11, 2009 6:06 AM
> To: linux-omap@xxxxxxxxxxxxxxx
> Subject: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock
> stabilization delay
> 
> Reposting the patch with proper format

Thanks for doing something automated instead of the old black magic
Loops based on SDRC_MPURATE_BASE_SHIFT tuning :) (hip hip hoorah - no
More days figuring out why display behaves weird when vdd2 opp switched
;) ).. anyways, my comments follow..

> 
> From: Teerth Reddy <teerth@xxxxxx>
> 
> This patch sets the dpll3 clock stabilization delay during
> DVFS. The stabilization delay is calculated dynamically
> using the ARM performance counter.
> Currently 6us of SDRC stabilization value is used to get
> the correct delay.

Is this hard 6uS delay valid for all SDRAMs and board variants? If your
Claim for dynamic detection is true, you don't need be using hard values
This is the type of #def mess we got out of MPURATE_BASE_SHIFT in the first
place..

Also Richard indicated that there might be a few tricky things with perf
Counters with specific devices - like EMU/HS/GP devices. It might need EMU
Domain for the values to pass through and there might be a yet-not-measured increase In power which could impact usage numbers and may need additional 
Code to switch off the domain correctly while hitting OFF/RET..

Might be good if community can try this on various OMAP3s and various SDRAMs
Mebbe who ever has access, could even try on EMU/HS/ etc..

> 
> Signed-off-by: Romit Dasgupta <romit@xxxxxx>
> Signed-off-by: Teerth Reddy <teerth@xxxxxx>
> ---
>  mach-omap2/clock34xx.c        |   32 +++++++++++++---------------
>  mach-omap2/sram34xx.S         |    7 ++++++
>  plat-omap/Makefile            |    1
>  plat-omap/include/plat/pmc.h  |    9 ++++++++
>  plat-omap/include/plat/sram.h |    6 +++++
>  plat-omap/pmc.c               |   47
> ++++++++++++++++++++++++++++++++++++++++++
>  plat-omap/sram.c              |   23 ++++++++++++++++++++
>  7 files changed, 108 insertions(+), 17 deletions(-)
> 
> 
> diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-
> omap2/clock34xx.c
> index da5bc1f..dd49938 100644
> --- a/arch/arm/mach-omap2/clock34xx.c
> +++ b/arch/arm/mach-omap2/clock34xx.c
> @@ -37,6 +37,8 @@
>  #include <asm/div64.h>
>  #include <asm/clkdev.h>
> 
> +#include <plat/pmc.h>
> +
>  #include <plat/sdrc.h>
>  #include "clock.h"
>  #include "prm.h"
> @@ -46,6 +48,8 @@
> 
>  static const struct clkops clkops_noncore_dpll_ops;
> 
> +unsigned int delay_sram;
> +
>  static void omap3430es2_clk_ssi_find_idlest(struct clk *clk,
>  					    void __iomem **idlest_reg,
>  					    u8 *idlest_bit);
> @@ -333,14 +337,9 @@ static struct omap_clk omap34xx_clks[] = {
>  /* Scale factor for fixed-point arith in omap3_core_dpll_m2_set_rate() */
>  #define SDRC_MPURATE_SCALE		8
> 
> -/* 2^SDRC_MPURATE_BASE_SHIFT: MPU MHz that SDRC_MPURATE_LOOPS is defined
> for */
> -#define SDRC_MPURATE_BASE_SHIFT		9
> -
> -/*
> - * SDRC_MPURATE_LOOPS: Number of MPU loops to execute at
> - * 2^MPURATE_BASE_SHIFT MHz for SDRC to stabilize
> - */
> -#define SDRC_MPURATE_LOOPS		96
> +/* SDRC_TIME_STABILIZE: Time for SDRC to stabilize in us */
> +/* hardcoded currently */
> +#define	SDRC_TIME_STABILIZE		6

Need a reason why we are hard coding -> it will be good to know if this
Delay varies across boards, and if so, you may need this info to come from
 board files OR derived out of sdrc parameters already being provided by 
board files (this would be my preferred way though).


> 
>  /*
>   * DPLL5_FREQ_FOR_USBHOST: USBHOST and USBTLL are the only clocks
> @@ -870,16 +869,10 @@ static int omap3_core_dpll_m2_set_rate(struct clk
> *clk, unsigned long rate)
>  		unlock_dll = 1;
>  	}
> 
> -	/*
> -	 * XXX This only needs to be done when the CPU frequency changes
> -	 */
> +	/* Calculate the number of MPU cycles to wait for SDRC to stabilize
> */
> +
>  	mpurate = arm_fck.rate / CYCLES_PER_MHZ;
> -	c = (mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT;
> -	c += 1;  /* for safety */
> -	c *= SDRC_MPURATE_LOOPS;
> -	c >>= SDRC_MPURATE_SCALE;
> -	if (c == 0)
> -		c = 1;
> +	c = ((SDRC_TIME_STABILIZE * mpurate) / (delay_sram * 2));
> 
>  	pr_debug("clock: changing CORE DPLL rate from %lu to %lu\n", clk-
> >rate,
>  		 validrate);
> @@ -1228,6 +1221,11 @@ int __init omap2_clk_init(void)
>  	vclk = clk_get(NULL, "virt_prcm_set");
>  	sclk = clk_get(NULL, "sys_ck");
>  #endif
> +
> +	/* Measure sram delay */
> +	delay_sram = measure_sram_delay(10000)/(10000*2);
                                             ^^^^^^^^^ <- you have not run
 checkpatch.pl on this patch in the first place.. sorry.. that should be 
easy to do I guess..

> +	pr_debug("SRAM delay: %d\n", delay_sram);


Arrgh more spam..

> +
>  	return 0;
>  }
> 
> diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-
> omap2/sram34xx.S
> index 82aa4a3..dc0ac72 100644
> --- a/arch/arm/mach-omap2/sram34xx.S
> +++ b/arch/arm/mach-omap2/sram34xx.S
> @@ -298,3 +298,10 @@ core_m2_mask_val:
>  ENTRY(omap3_sram_configure_core_dpll_sz)
>  	.word	. - omap3_sram_configure_core_dpll
> 
> +ENTRY(__sram_wait_delay)
> +	subs 	r0, r0, #1
> +	bne	__sram_wait_delay
> +	bx	lr
> +
> +ENTRY(__sram_wait_delay_sz)
> +	.word	. - __sram_wait_delay
> diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
> index 95f8413..aac43da 100644
> --- a/arch/arm/plat-omap/Makefile
> +++ b/arch/arm/plat-omap/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
>  # omap_device support (OMAP2+ only at the moment)
>  obj-$(CONFIG_ARCH_OMAP2) += omap_device.o
>  obj-$(CONFIG_ARCH_OMAP3) += omap_device.o
> +obj-$(CONFIG_ARCH_OMAP3) += pmc.o
> 
>  obj-$(CONFIG_OMAP_MCBSP) += mcbsp.o
>  obj-$(CONFIG_OMAP_IOMMU) += iommu.o iovmm.o
> diff --git a/arch/arm/plat-omap/include/plat/pmc.h b/arch/arm/plat-
> omap/include/plat/pmc.h
> new file mode 100644
> index 0000000..6d73782
> --- /dev/null
> +++ b/arch/arm/plat-omap/include/plat/pmc.h
> @@ -0,0 +1,9 @@
> +#ifndef __PMC_H__
> +#define __PMC_H__
> +extern void start_perf_counter(void);
> +extern void start_cycle_counter(void);
> +extern void stop_cycle_counter(void);
> +extern void stop_perf_counter(void);
> +extern unsigned int cycle_count(void);
> +extern unsigned int delay_sram;
> +#endif
> diff --git a/arch/arm/plat-omap/include/plat/sram.h b/arch/arm/plat-
> omap/include/plat/sram.h
> index 16a1b45..6019ed8 100644
> --- a/arch/arm/plat-omap/include/plat/sram.h
> +++ b/arch/arm/plat-omap/include/plat/sram.h
> @@ -69,6 +69,12 @@ extern u32 omap3_sram_configure_core_dpll(
>  			u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1);
>  extern unsigned long omap3_sram_configure_core_dpll_sz;
> 
> +extern unsigned int measure_sram_delay(unsigned int);
> +
> +extern u32 __sram_wait_delay(
> +		unsigned int);

fix formatting please..

> +extern unsigned long __sram_wait_delay_sz;
> +
>  #ifdef CONFIG_PM
>  extern void omap_push_sram_idle(void);
>  #else
> diff --git a/arch/arm/plat-omap/pmc.c b/arch/arm/plat-omap/pmc.c
> new file mode 100644
> index 0000000..11acf42
> --- /dev/null
> +++ b/arch/arm/plat-omap/pmc.c
> @@ -0,0 +1,47 @@
> +#include <linux/module.h>
> +
> +void start_perf_counter(void)
> +{
> +	unsigned int r1 = 0;
> +	asm("mrc p15, 0, %0, c9, c12, 0" : "=r" (r1));
> +	r1 |= 0x1; /* enable counters */
> +	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
> +	return;
> +}
> +EXPORT_SYMBOL(start_perf_counter);
> +
> +void start_cycle_counter(void)
> +{
> +	unsigned int r2 = 0;
> +	r2 = 0x80000000; /* enable cycle counter only */
> +	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r2));
> +	return;
> +}
> +EXPORT_SYMBOL(start_cycle_counter);
> +
> +unsigned int cycle_count(void)
> +{
> +	unsigned int rd = 0;
> +	asm("mrc p15, 0, %0, c9, c13, 0" : "=r" (rd));
> +	return rd;
> +}
> +EXPORT_SYMBOL(cycle_count);
> +
> +
> +void stop_cycle_counter(void)
> +{
> +	unsigned int r3 = 0;
> +	r3 = 0x0; /* disable cycle counter */
> +	asm("mcr p15, 0, %0, c9, c12, 1" : : "r" (r3));
> +	return;
> +}
> +EXPORT_SYMBOL(stop_cycle_counter);
> +
> +void stop_perf_counter(void)
> +{
> +	unsigned int r1 = 0;
> +	r1 = 0x0; /* disable counters */
> +	asm("mcr p15, 0, %0, c9, c12, 0" : : "r" (r1));
> +	return;
> +}
> +EXPORT_SYMBOL(stop_perf_counter);
> diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
> index 3e92366..4ed6a48 100644
> --- a/arch/arm/plat-omap/sram.c
> +++ b/arch/arm/plat-omap/sram.c
> @@ -29,6 +29,10 @@
>  #include <plat/board.h>
>  #include <plat/cpu.h>
> 
> +#ifdef CONFIG_ARCH_OMAP3
> +#include <plat/pmc.h>
> +#endif
> +
>  #include <plat/control.h>
> 
>  #if defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
> @@ -423,6 +427,25 @@ static inline int omap34xx_sram_init(void)
>  }
>  #endif
> 
> +
> +#ifdef CONFIG_ARCH_OMAP3
> +unsigned int measure_sram_delay(unsigned int loop)
> +{
> +	unsigned int start = 0, end = 0;
> +	void (*_omap3_sram_delay)(unsigned long);
> +	_omap3_sram_delay = omap_sram_push(__sram_wait_delay,
> +						__sram_wait_delay_sz);
> +	start_perf_counter();
> +	start_cycle_counter();
> +	start = cycle_count();
> +	_omap3_sram_delay(loop);
> +	end = cycle_count();
> +	stop_cycle_counter();
> +	stop_perf_counter();
> +	return end - start;
> +}
> +#endif
> +

>  int __init omap_sram_init(void)
>  {
>  	omap_detect_sram();
> --
> 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
--
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