On Sat, 2013-08-03 at 04:40 +0800, Stephen Warren wrote: > On 08/02/2013 03:27 AM, Joseph Lo wrote: > > On Tue, 2013-07-30 at 07:13 +0800, Stephen Warren wrote: > >> On 07/26/2013 03:15 AM, Joseph Lo wrote: > >>> The LP1 suspending mode on Tegra means CPU rail off, devices and PLLs are > >>> clock gated and SDRAM in self-refresh mode. That means the low level LP1 > >>> suspending and resuming code couldn't be run on DRAM and the CPU must > >>> switch to the always on clock domain (a.k.a. CLK_M 12MHz oscillator). And > >>> the system clock (SCLK) would be switched to CLK_S, a 32KHz oscillator. > >>> The LP1 low level handling code need to be moved to IRAM area first. And > >>> marking the LP1 mask for indicating the Tegra device is in LP1. The CPU > >>> power timer needs to be re-calculated based on 32KHz that was originally > >>> based on PCLK. > >> > >>> diff --git a/arch/arm/mach-tegra/pm.c b/arch/arm/mach-tegra/pm.c > >> > >>> #ifdef CONFIG_PM_SLEEP > >>> static DEFINE_SPINLOCK(tegra_lp2_lock); > >>> +static void __iomem *iram_code = IO_ADDRESS(TEGRA_IRAM_CODE_AREA); > >>> +static u32 iram_save_size; > >>> +static void *iram_save_addr; > >>> +struct tegra_lp1_iram *tegra_lp1_iram; > >>> void (*tegra_tear_down_cpu)(void); > >>> +void (*tegra_sleep_core_finish)(unsigned long v2p); > >> > >> I'm not sure all of those are required to be global variables. For > >> example, iram_code is just a constant, so you could easily just use it > >> directly in code. > > > > All of them does not mean the same thing. The LP1 resume code was built > > in kernel image and store in RAM. > > The tegra_lp1_iram hooks the LP1 resume code for different chips. Before > > LP1 suspend, the original stuffs that in the area of IRAM would be store > > in the iram_save_addr (RAM). Then copy the LP1 resume code to iram_code > > area (IRAM). > > Sure, some of those variable may differ based on the SoC at runtime etc. > > But at least the value of iram_code never changes, right? Yes. > >>> +static void tegra_suspend_enter_lp1(void) > >>> +{ > >>> + tegra_pmc_suspend(); > >>> + > >>> + /* copy the reset vector & SDRAM shutdown code into IRAM */ > >>> + memcpy(iram_save_addr, iram_code, iram_save_size); > >>> + memcpy(iram_code, tegra_lp1_iram->start_addr, iram_save_size); > >>> + > >>> + *((u32 *)tegra_cpu_lp1_mask) = 1; > >>> +} > >>> + > >>> +static void tegra_suspend_exit_lp1(void) > >>> +{ > >>> + tegra_pmc_resume(); > >>> + > >>> + /* restore IRAM */ > >>> + memcpy(iram_code, iram_save_addr, iram_save_size); > >>> + > >>> + *(u32 *)tegra_cpu_lp1_mask = 0; > >>> +} > >> > >> I'm not really sure I like that, but I suppose it's OK. It sure seems > >> like a performance limiter, but I suppose LP1 is so slow it doesn't > >> matter, due to the need to ramp power rails, PLLs, SDRAM controller, etc. > > > > That's why we only back up the code size that exactly same with the LP1 > > resume code of the SoC. > > > >> It'd be nice to simply reserve more IRAM for the kernel's use. Right > >> now, only 1K is reserved, and presumably the code running on the AVP > >> can't use the rest of that page anyway, or can it? > > > > The LP1 resume code still running on the CPU (The LP0 would resume from > > AVP). > > Sure. However, if the AVP never touched the IRAM region where the LP1 > resume code is placed, we would only need to copy the LP1 resume code to > IRAM once at kernel boot time, rather than every time we enter/leave LP1. > > I guess the idea is that once we have an AVP driver, we will force the > AVP to suspend first, save the IRAM that it might have been using, do > the system suspend/resume, then restore the IRAM. And that changing that > sequence so that the AVP never ever touched the LP1 IRAM area would > require AVP firmware changes that we can't make? Yes, exactly. -- To unsubscribe from this list: send the line "unsubscribe linux-tegra" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html