| Message ID | 1402090985-8061-1-git-send-email-dianders@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 6 Jun 2014, Doug Anderson wrote: > On exynos mcpm systems the firmware is hardcoded to jump to an address > in SRAM (0x02073000) when secondary CPUs come up. By default the > firmware puts a bunch of code at that location. That code expects the > kernel to fill in a few slots with addresses that it uses to jump back > to the kernel's entry point for secondary CPUs. > > Originally (on prerelease hardware) this firmware code contained a > bunch of workarounds to deal with boot ROM bugs. However on all > shipped hardware we simply use this code to redirect to a kernel > function for bringing up the CPUs. > > Let's stop relying on the code provided by the bootloader and just > plumb in our own (simple) code jump to the kernel. This has the nice > benefit of fixing problems due to the fact that older bootloaders > (like the one shipped on the Samsung Chromebook 2) might have put > slightly different code into this location. > > Once suspend/resume is implemented for systems using exynos-mcpm we'll > need to make sure we reinstall our fixed up code after resume. ...but > that's not anything new since IRAM (and thus the address of the > mcpm_entry_point) is lost across suspend/resume anyway. > > Signed-off-by: Doug Anderson <dianders@chromium.org> > --- > arch/arm/mach-exynos/mcpm-exynos.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c > index 0498d0b..3a7fad0 100644 > --- a/arch/arm/mach-exynos/mcpm-exynos.c > +++ b/arch/arm/mach-exynos/mcpm-exynos.c > @@ -343,11 +343,13 @@ static int __init exynos_mcpm_init(void) > pr_info("Exynos MCPM support installed\n"); > > /* > - * Future entries into the kernel can now go > - * through the cluster entry vectors. > + * U-Boot SPL is hardcoded to jump to the start of ns_sram_base_addr > + * as part of secondary_cpu_start(). Let's redirect it to the > + * mcpm_entry_point(). > */ > - __raw_writel(virt_to_phys(mcpm_entry_point), > - ns_sram_base_addr + MCPM_BOOT_ADDR_OFFSET); > + __raw_writel(0xe59f0000, ns_sram_base_addr); /* ldr r0, [pc, #0] */ > + __raw_writel(0xe12fff10, ns_sram_base_addr + 4); /* bx r0 */ > + __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 8); Is this valid for all exynos systems, or is this particular to Chromebooks? If this is all that is needed to solve the problem being discussed in the other thread I have absolutely no issue with such a workaround going into mainline. Nicolas
Nicolas, On Fri, Jun 6, 2014 at 3:35 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: > On Fri, 6 Jun 2014, Doug Anderson wrote: > >> On exynos mcpm systems the firmware is hardcoded to jump to an address >> in SRAM (0x02073000) when secondary CPUs come up. By default the >> firmware puts a bunch of code at that location. That code expects the >> kernel to fill in a few slots with addresses that it uses to jump back >> to the kernel's entry point for secondary CPUs. >> >> Originally (on prerelease hardware) this firmware code contained a >> bunch of workarounds to deal with boot ROM bugs. However on all >> shipped hardware we simply use this code to redirect to a kernel >> function for bringing up the CPUs. >> >> Let's stop relying on the code provided by the bootloader and just >> plumb in our own (simple) code jump to the kernel. This has the nice >> benefit of fixing problems due to the fact that older bootloaders >> (like the one shipped on the Samsung Chromebook 2) might have put >> slightly different code into this location. >> >> Once suspend/resume is implemented for systems using exynos-mcpm we'll >> need to make sure we reinstall our fixed up code after resume. ...but >> that's not anything new since IRAM (and thus the address of the >> mcpm_entry_point) is lost across suspend/resume anyway. >> >> Signed-off-by: Doug Anderson <dianders@chromium.org> >> --- >> arch/arm/mach-exynos/mcpm-exynos.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c >> index 0498d0b..3a7fad0 100644 >> --- a/arch/arm/mach-exynos/mcpm-exynos.c >> +++ b/arch/arm/mach-exynos/mcpm-exynos.c >> @@ -343,11 +343,13 @@ static int __init exynos_mcpm_init(void) >> pr_info("Exynos MCPM support installed\n"); >> >> /* >> - * Future entries into the kernel can now go >> - * through the cluster entry vectors. >> + * U-Boot SPL is hardcoded to jump to the start of ns_sram_base_addr >> + * as part of secondary_cpu_start(). Let's redirect it to the >> + * mcpm_entry_point(). >> */ >> - __raw_writel(virt_to_phys(mcpm_entry_point), >> - ns_sram_base_addr + MCPM_BOOT_ADDR_OFFSET); >> + __raw_writel(0xe59f0000, ns_sram_base_addr); /* ldr r0, [pc, #0] */ >> + __raw_writel(0xe12fff10, ns_sram_base_addr + 4); /* bx r0 */ >> + __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 8); > > Is this valid for all exynos systems, or is this particular to > Chromebooks? I will have to let others comment on that since I can't be certain of anything other than the firmware branch I've seen. ...but given that the code before my patch was putting the resume address at the magic 0x0207301C and that matches the code I've seen, I'm going to say that they all behave the same. > If this is all that is needed to solve the problem being discussed in > the other thread I have absolutely no issue with such a workaround going > into mainline. This plus the CCI fix that Andrew is planning to post.
>> If this is all that is needed to solve the problem being discussed in >> the other thread I have absolutely no issue with such a workaround going >> into mainline. > > This plus the CCI fix that Andrew is planning to post. Right - we'll need a patch to enable the CCI port for the cluster we booted on at boot and resume time since the first CPU up won't enter through exynos_pm_power_up_setup(). I'll try to post something later today or Monday.
On Fri, Jun 06, 2014 at 10:43:05PM +0100, Doug Anderson wrote: > On exynos mcpm systems the firmware is hardcoded to jump to an address > in SRAM (0x02073000) when secondary CPUs come up. By default the > firmware puts a bunch of code at that location. That code expects the > kernel to fill in a few slots with addresses that it uses to jump back > to the kernel's entry point for secondary CPUs. > > Originally (on prerelease hardware) this firmware code contained a > bunch of workarounds to deal with boot ROM bugs. However on all > shipped hardware we simply use this code to redirect to a kernel > function for bringing up the CPUs. > > Let's stop relying on the code provided by the bootloader and just > plumb in our own (simple) code jump to the kernel. This has the nice > benefit of fixing problems due to the fact that older bootloaders > (like the one shipped on the Samsung Chromebook 2) might have put > slightly different code into this location. > > Once suspend/resume is implemented for systems using exynos-mcpm we'll > need to make sure we reinstall our fixed up code after resume. ...but > that's not anything new since IRAM (and thus the address of the > mcpm_entry_point) is lost across suspend/resume anyway. Can I ask you please what the firmware does for the boot (primary) cpu on cold-boot and warm-boot (resume from suspend) ? Does it jump to a specific (hardcoded) location ? Is the primary CPU (MPIDR) hardcoded in firmware so that on both cold and warm-boot firmware sees a specific MPIDR as "special" ? I am asking to check if on this platform CPUidle (where the notion of primary CPU disappears) has a chance to run properly. Probably CPUidle won't attain idle states where IRAM content is lost, but I am still worried about the primary vs secondaries firmware boot behaviour. What happens on reboot from suspend to RAM (or to put it differently, what does secure firmware do on reboot from suspend to RAM - in particular how is the "jump" address to bootloader/kernel set ?) Thank you very much. Lorenzo > > Signed-off-by: Doug Anderson <dianders@chromium.org> > --- > arch/arm/mach-exynos/mcpm-exynos.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c > index 0498d0b..3a7fad0 100644 > --- a/arch/arm/mach-exynos/mcpm-exynos.c > +++ b/arch/arm/mach-exynos/mcpm-exynos.c > @@ -343,11 +343,13 @@ static int __init exynos_mcpm_init(void) > pr_info("Exynos MCPM support installed\n"); > > /* > - * Future entries into the kernel can now go > - * through the cluster entry vectors. > + * U-Boot SPL is hardcoded to jump to the start of ns_sram_base_addr > + * as part of secondary_cpu_start(). Let's redirect it to the > + * mcpm_entry_point(). > */ > - __raw_writel(virt_to_phys(mcpm_entry_point), > - ns_sram_base_addr + MCPM_BOOT_ADDR_OFFSET); > + __raw_writel(0xe59f0000, ns_sram_base_addr); /* ldr r0, [pc, #0] */ > + __raw_writel(0xe12fff10, ns_sram_base_addr + 4); /* bx r0 */ > + __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 8); > > iounmap(ns_sram_base_addr); > > -- > 2.0.0.526.g5318336 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
Lorenzo, On Sat, Jun 7, 2014 at 11:12 AM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Fri, Jun 06, 2014 at 10:43:05PM +0100, Doug Anderson wrote: >> On exynos mcpm systems the firmware is hardcoded to jump to an address >> in SRAM (0x02073000) when secondary CPUs come up. By default the >> firmware puts a bunch of code at that location. That code expects the >> kernel to fill in a few slots with addresses that it uses to jump back >> to the kernel's entry point for secondary CPUs. >> >> Originally (on prerelease hardware) this firmware code contained a >> bunch of workarounds to deal with boot ROM bugs. However on all >> shipped hardware we simply use this code to redirect to a kernel >> function for bringing up the CPUs. >> >> Let's stop relying on the code provided by the bootloader and just >> plumb in our own (simple) code jump to the kernel. This has the nice >> benefit of fixing problems due to the fact that older bootloaders >> (like the one shipped on the Samsung Chromebook 2) might have put >> slightly different code into this location. >> >> Once suspend/resume is implemented for systems using exynos-mcpm we'll >> need to make sure we reinstall our fixed up code after resume. ...but >> that's not anything new since IRAM (and thus the address of the >> mcpm_entry_point) is lost across suspend/resume anyway. > > Can I ask you please what the firmware does for the boot (primary) cpu > on cold-boot and warm-boot (resume from suspend) ? On cold boot: 1. iROM loads BL1 from SPI flash 2. BL1 loads BL2 (SPL) from SPI flash 3. BL2 loads loads RO U-Boot from SPI flash 4. RO U-Boot (might) load RW U-Boot from SPI flash 5. U-Boot loads kernel from eMMC (or SD card or USB) 6. U-Boot boots kernel using bootm. On resume from suspend: 1. iROM loads BL1 from SPI flash 2. BL1 loads BL2 (SPL) from SPI flash 3. BL2 does some basic init code (clocks, SDRAM out of self refresh, ...) and looks at some SoC registers that are preserved across suspend/resume. These registers contain flags indicating that this is a resume from suspend and a pointer to the address in the kernel to jump to at resume time. The address of these registers and which bits mean which flags is hardcoded and is part of the contract between the bootloader and the kernel. 4. BL2 jumps to kernel. --- I will note that BL1, BL2, and "RO U-Boot" are read only in production systems. Note that at resume time only Read-Only code is loaded from persistent storage. That means no extra verification is needed. A system exploit could survive suspend/resume but a reboot would clear it and a reboot + suspend/resume wouldn't bring it back. > Does it jump to a specific (hardcoded) location ? I guess I could say it this way: A) In resume from suspend, by agreement between the kernel and the bootloader we'll jump to the address stored in 0x10040800 (INFORM0) B) In bringing up a new CPU, by agreement between the kernel and the bootloader we'll jump directly to 0x02073000 (and address in the SoC's internal SRAM). > Is the primary CPU (MPIDR) hardcoded in firmware so that on both > cold and warm-boot firmware sees a specific MPIDR as "special" ? Cold boot and resume from suspend are detected via various special flags in various special locations. Resume from suspend looks at INFORM1 (0x10048004) for flags. This register is 0 during a cold boot and has special values set by the kernel at resume time. It also looks as if some code looks at 0x10040900 (PMU_SPARE0) to help tell initial cold boot and secondary CPU bringup. > I am asking to check if on this platform CPUidle (where the notion of > primary CPU disappears) has a chance to run properly. I believe it should be possible, but we don't have CPUidle implemented in our current system. Abhilash may be able to comment more. > Probably CPUidle won't attain idle states where IRAM content is lost, but I > am still worried about the primary vs secondaries firmware boot behaviour. I don't think iRAM can be turned off for CPUidle. > What happens on reboot from suspend to RAM (or to put it differently, > what does secure firmware do on reboot from suspend to RAM - in > particular how is the "jump" address to bootloader/kernel set ?) Should be described above now. -Doug
On Mon, Jun 09, 2014 at 06:03:31PM +0100, Doug Anderson wrote: [...] > Cold boot and resume from suspend are detected via various special > flags in various special locations. Resume from suspend looks at > INFORM1 (0x10048004) for flags. This register is 0 during a cold boot > and has special values set by the kernel at resume time. > > It also looks as if some code looks at 0x10040900 (PMU_SPARE0) to help > tell initial cold boot and secondary CPU bringup. Ok, thanks a lot. It looks like firmware paths should be ready to detect cold vs warm boot, and hopefully do not rely on a specific MPIDR to come up first out of power states. > > I am asking to check if on this platform CPUidle (where the notion of > > primary CPU disappears) has a chance to run properly. > > I believe it should be possible, but we don't have CPUidle implemented > in our current system. Abhilash may be able to comment more. I am interested in more insights, that's very helpful thanks. > > Probably CPUidle won't attain idle states where IRAM content is lost, but I > > am still worried about the primary vs secondaries firmware boot behaviour. > > I don't think iRAM can be turned off for CPUidle. It might be added a system state but I doubt that too and if you are relying on registers for jump addresses that's not even a problem in the first place. > > What happens on reboot from suspend to RAM (or to put it differently, > > what does secure firmware do on reboot from suspend to RAM - in > > particular how is the "jump" address to bootloader/kernel set ?) > > Should be described above now. Thank you very much. Lorenzo
On 10 June 2014 04:08, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Mon, Jun 09, 2014 at 06:03:31PM +0100, Doug Anderson wrote: > > [...] > >> Cold boot and resume from suspend are detected via various special >> flags in various special locations. Resume from suspend looks at >> INFORM1 (0x10048004) for flags. This register is 0 during a cold boot >> and has special values set by the kernel at resume time. >> >> It also looks as if some code looks at 0x10040900 (PMU_SPARE0) to help >> tell initial cold boot and secondary CPU bringup. > > Ok, thanks a lot. It looks like firmware paths should be ready to > detect cold vs warm boot, and hopefully do not rely on a specific > MPIDR to come up first out of power states. > >> > I am asking to check if on this platform CPUidle (where the notion of >> > primary CPU disappears) has a chance to run properly. >> >> I believe it should be possible, but we don't have CPUidle implemented >> in our current system. Abhilash may be able to comment more. > Cpuidle is implemented for exynos5420, and is tested on chromebook. > I am interested in more insights, that's very helpful thanks. > >> > Probably CPUidle won't attain idle states where IRAM content is lost, but I >> > am still worried about the primary vs secondaries firmware boot behaviour. >> >> I don't think iRAM can be turned off for CPUidle. Yes thats true. > > It might be added a system state but I doubt that too and if you are > relying on registers for jump addresses that's not even a problem in > the first place. > >> > What happens on reboot from suspend to RAM (or to put it differently, >> > what does secure firmware do on reboot from suspend to RAM - in >> > particular how is the "jump" address to bootloader/kernel set ?) >> >> Should be described above now. > > Thank you very much. > > Lorenzo > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Chander, On Tue, Jun 10, 2014 at 1:12 AM, Chander Kashyap <chander.kashyap@linaro.org> wrote: > On 10 June 2014 04:08, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: >> On Mon, Jun 09, 2014 at 06:03:31PM +0100, Doug Anderson wrote: >> >> [...] >> >>> Cold boot and resume from suspend are detected via various special >>> flags in various special locations. Resume from suspend looks at >>> INFORM1 (0x10048004) for flags. This register is 0 during a cold boot >>> and has special values set by the kernel at resume time. >>> >>> It also looks as if some code looks at 0x10040900 (PMU_SPARE0) to help >>> tell initial cold boot and secondary CPU bringup. >> >> Ok, thanks a lot. It looks like firmware paths should be ready to >> detect cold vs warm boot, and hopefully do not rely on a specific >> MPIDR to come up first out of power states. >> >>> > I am asking to check if on this platform CPUidle (where the notion of >>> > primary CPU disappears) has a chance to run properly. >>> >>> I believe it should be possible, but we don't have CPUidle implemented >>> in our current system. Abhilash may be able to comment more. >> > > Cpuidle is implemented for exynos5420, and is tested on chromebook. My S-state knowledge is not strong, but I believe that Lorenzo's questions matter if we're using S2 for CPUidle (where we actually turn off power and hot unplug CPUs) but not when we're using S1 for CPUidle (where we just enter WFI/WFE). I believe that in ChromeOS we use S1 CPUidle and that it works fine. We've never implemented S2 that I'm aware of. -Doug
On Tue, 10 Jun 2014, Doug Anderson wrote: > My S-state knowledge is not strong, but I believe that Lorenzo's > questions matter if we're using S2 for CPUidle (where we actually turn > off power and hot unplug CPUs) but not when we're using S1 for CPUidle > (where we just enter WFI/WFE). > > I believe that in ChromeOS we use S1 CPUidle and that it works fine. > We've never implemented S2 that I'm aware of. You'll have to rely on MCPM for that. That's probably why it hasn't been implemented before. Nicolas
Hi Doug, On Tue, Jun 10, 2014 at 9:19 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: > On Tue, 10 Jun 2014, Doug Anderson wrote: > >> My S-state knowledge is not strong, but I believe that Lorenzo's >> questions matter if we're using S2 for CPUidle (where we actually turn >> off power and hot unplug CPUs) but not when we're using S1 for CPUidle >> (where we just enter WFI/WFE). >> No Its not plain WFI. All cores in Exynos5420 can be powered off independently. This functionality has been tested. Below is the link for the posted patches. https://lkml.org/lkml/2014/6/10/194 And as Nicolas wrote, these patches need MCPM for that. >> I believe that in ChromeOS we use S1 CPUidle and that it works fine. >> We've never implemented S2 that I'm aware of. > > You'll have to rely on MCPM for that. That's probably why it hasn't > been implemented before. > > > Nicolas > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Jun 11, 2014 at 05:52:10AM +0100, Chander Kashyap wrote: > Hi Doug, > > On Tue, Jun 10, 2014 at 9:19 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: > > On Tue, 10 Jun 2014, Doug Anderson wrote: > > > >> My S-state knowledge is not strong, but I believe that Lorenzo's > >> questions matter if we're using S2 for CPUidle (where we actually turn > >> off power and hot unplug CPUs) but not when we're using S1 for CPUidle > >> (where we just enter WFI/WFE). > >> > > No Its not plain WFI. > > All cores in Exynos5420 can be powered off independently. > This functionality has been tested. > > Below is the link for the posted patches. > > https://lkml.org/lkml/2014/6/10/194 > > And as Nicolas wrote, these patches need MCPM for that. Chander, I cast a look into the code and I have a question (you also told me on CPUidle review that only core off is supported in CPUidle). When you say all cores can be powered off independently, do you also mean that clusters can be powered off (in CPUidle) ? I am pointing this out since in the MCPM backend I noticed: "/* TODO: Turn off the cluster here to save power. */" I do not see any cluster power down request in the down path. If I am wrong, ignore my message, I am just writing to help. If you can only power down cores, but not the clusters on idle, please keep in mind that you are currently cleaning and invalidating the L2 when last man is running and this must not be taken lightly since, if L2 stays on, that's a massive energy waste for nothing. So, if clusters stay up, you _have_ to tweak the MCPM backend slightly to avoid cleaning L2, that's pivotal. Lorenzo > > >> I believe that in ChromeOS we use S1 CPUidle and that it works fine. > >> We've never implemented S2 that I'm aware of. > > > > You'll have to rely on MCPM for that. That's probably why it hasn't > > been implemented before. > > > > > > Nicolas > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
On Wed, Jun 11, 2014 at 3:43 PM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Wed, Jun 11, 2014 at 05:52:10AM +0100, Chander Kashyap wrote: >> Hi Doug, >> >> On Tue, Jun 10, 2014 at 9:19 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: >> > On Tue, 10 Jun 2014, Doug Anderson wrote: >> > >> >> My S-state knowledge is not strong, but I believe that Lorenzo's >> >> questions matter if we're using S2 for CPUidle (where we actually turn >> >> off power and hot unplug CPUs) but not when we're using S1 for CPUidle >> >> (where we just enter WFI/WFE). >> >> >> >> No Its not plain WFI. >> >> All cores in Exynos5420 can be powered off independently. >> This functionality has been tested. >> >> Below is the link for the posted patches. >> >> https://lkml.org/lkml/2014/6/10/194 >> >> And as Nicolas wrote, these patches need MCPM for that. > > Chander, I cast a look into the code and I have a question > (you also told me on CPUidle review that only core off > is supported in CPUidle). > > When you say all cores can be powered off independently, do > you also mean that clusters can be powered off (in CPUidle) ? > > I am pointing this out since in the MCPM backend I noticed: > > "/* TODO: Turn off the cluster here to save power. */" > > I do not see any cluster power down request in the down path. > > If I am wrong, ignore my message, I am just writing to help. > > If you can only power down cores, but not the clusters on idle, > please keep in mind that you are currently cleaning and invalidating > the L2 when last man is running and this must not be taken > lightly since, if L2 stays on, that's a massive energy waste > for nothing. > > So, if clusters stay up, you _have_ to tweak the MCPM backend slightly > to avoid cleaning L2, that's pivotal. Hi Lorenzo, Cluster shutdown is in progress. Abhilash will add support for that. https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg31104.html > > Lorenzo > >> >> >> I believe that in ChromeOS we use S1 CPUidle and that it works fine. >> >> We've never implemented S2 that I'm aware of. >> > >> > You'll have to rely on MCPM for that. That's probably why it hasn't >> > been implemented before. >> > >> > >> > Nicolas >> > >> > _______________________________________________ >> > linux-arm-kernel mailing list >> > linux-arm-kernel@lists.infradead.org >> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 11, 2014 at 01:14:21PM +0100, Chander Kashyap wrote: > On Wed, Jun 11, 2014 at 3:43 PM, Lorenzo Pieralisi > <lorenzo.pieralisi@arm.com> wrote: > > On Wed, Jun 11, 2014 at 05:52:10AM +0100, Chander Kashyap wrote: > >> Hi Doug, > >> > >> On Tue, Jun 10, 2014 at 9:19 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: > >> > On Tue, 10 Jun 2014, Doug Anderson wrote: > >> > > >> >> My S-state knowledge is not strong, but I believe that Lorenzo's > >> >> questions matter if we're using S2 for CPUidle (where we actually turn > >> >> off power and hot unplug CPUs) but not when we're using S1 for CPUidle > >> >> (where we just enter WFI/WFE). > >> >> > >> > >> No Its not plain WFI. > >> > >> All cores in Exynos5420 can be powered off independently. > >> This functionality has been tested. > >> > >> Below is the link for the posted patches. > >> > >> https://lkml.org/lkml/2014/6/10/194 > >> > >> And as Nicolas wrote, these patches need MCPM for that. > > > > Chander, I cast a look into the code and I have a question > > (you also told me on CPUidle review that only core off > > is supported in CPUidle). > > > > When you say all cores can be powered off independently, do > > you also mean that clusters can be powered off (in CPUidle) ? > > > > I am pointing this out since in the MCPM backend I noticed: > > > > "/* TODO: Turn off the cluster here to save power. */" > > > > I do not see any cluster power down request in the down path. > > > > If I am wrong, ignore my message, I am just writing to help. > > > > If you can only power down cores, but not the clusters on idle, > > please keep in mind that you are currently cleaning and invalidating > > the L2 when last man is running and this must not be taken > > lightly since, if L2 stays on, that's a massive energy waste > > for nothing. > > > > So, if clusters stay up, you _have_ to tweak the MCPM backend slightly > > to avoid cleaning L2, that's pivotal. > > Hi Lorenzo, > Cluster shutdown is in progress. Abhilash will add support for that. > > https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg31104.html Hi Chander, thanks. So, as a heads-up: 1) if you merge CPUidle support now, as it is it is at least suboptimal, may even burn more energy than it saves. Latencies in the bL idle driver are likely to be wrong, since they are for cluster shutdown and for TC2, not core power gating that should require shorter target_residencies. On top of that, L2 is cleaned and invalidated for nothing. 2) when cluster support is merged, you might want to extend the CPUidle driver to add an additional state (ie C1 core gating, C2 cluster gating) and to do that you should extend the driver and the MCPM back-end accordingly, I discussed that with Nico already some time ago and actually it should be fairly easy to do but we have got to talk about that. Thank you, Lorenzo > > > > > > Lorenzo > > > >> > >> >> I believe that in ChromeOS we use S1 CPUidle and that it works fine. > >> >> We've never implemented S2 that I'm aware of. > >> > > >> > You'll have to rely on MCPM for that. That's probably why it hasn't > >> > been implemented before. > >> > > >> > > >> > Nicolas > >> > > >> > _______________________________________________ > >> > linux-arm-kernel mailing list > >> > linux-arm-kernel@lists.infradead.org > >> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > >> the body of a message to majordomo@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > >> Please read the FAQ at http://www.tux.org/lkml/ > >> > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On 11 June 2014 18:45, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Wed, Jun 11, 2014 at 01:14:21PM +0100, Chander Kashyap wrote: >> On Wed, Jun 11, 2014 at 3:43 PM, Lorenzo Pieralisi >> <lorenzo.pieralisi@arm.com> wrote: >> > On Wed, Jun 11, 2014 at 05:52:10AM +0100, Chander Kashyap wrote: >> >> Hi Doug, >> >> >> >> On Tue, Jun 10, 2014 at 9:19 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: >> >> > On Tue, 10 Jun 2014, Doug Anderson wrote: >> >> > >> >> >> My S-state knowledge is not strong, but I believe that Lorenzo's >> >> >> questions matter if we're using S2 for CPUidle (where we actually turn >> >> >> off power and hot unplug CPUs) but not when we're using S1 for CPUidle >> >> >> (where we just enter WFI/WFE). >> >> >> >> >> >> >> No Its not plain WFI. >> >> >> >> All cores in Exynos5420 can be powered off independently. >> >> This functionality has been tested. >> >> >> >> Below is the link for the posted patches. >> >> >> >> https://lkml.org/lkml/2014/6/10/194 >> >> >> >> And as Nicolas wrote, these patches need MCPM for that. >> > >> > Chander, I cast a look into the code and I have a question >> > (you also told me on CPUidle review that only core off >> > is supported in CPUidle). >> > >> > When you say all cores can be powered off independently, do >> > you also mean that clusters can be powered off (in CPUidle) ? >> > >> > I am pointing this out since in the MCPM backend I noticed: >> > >> > "/* TODO: Turn off the cluster here to save power. */" >> > >> > I do not see any cluster power down request in the down path. >> > >> > If I am wrong, ignore my message, I am just writing to help. >> > >> > If you can only power down cores, but not the clusters on idle, >> > please keep in mind that you are currently cleaning and invalidating >> > the L2 when last man is running and this must not be taken >> > lightly since, if L2 stays on, that's a massive energy waste >> > for nothing. >> > >> > So, if clusters stay up, you _have_ to tweak the MCPM backend slightly >> > to avoid cleaning L2, that's pivotal. >> >> Hi Lorenzo, >> Cluster shutdown is in progress. Abhilash will add support for that. >> >> https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg31104.html > Hi Lorenzo, > Hi Chander, > > thanks. So, as a heads-up: > > 1) if you merge CPUidle support now, as it is it is at least suboptimal, may > even burn more energy than it saves. Latencies in the bL idle driver > are likely to be wrong, since they are for cluster shutdown and for > TC2, not core power gating that should require shorter target_residencies. > On top of that, L2 is cleaned and invalidated for nothing. Yes true. > 2) when cluster support is merged, you might want to extend the CPUidle > driver to add an additional state (ie C1 core gating, C2 cluster > gating) and to do that you should extend the driver and the MCPM > back-end accordingly, I discussed that with Nico already some time ago > and actually it should be fairly easy to do but we have got to talk > about that. > Yes thats the final goal to add two states (C1 core gating, C2 cluster gating). But that will be done in combination with yours patches(to pass cpuidle data thru DT). So finally the mcpm backend will take care for c1 and c2 cpuidle state implementation. > Thank you, > Lorenzo > >> >> >> > >> > Lorenzo >> > >> >> >> >> >> I believe that in ChromeOS we use S1 CPUidle and that it works fine. >> >> >> We've never implemented S2 that I'm aware of. >> >> > >> >> > You'll have to rely on MCPM for that. That's probably why it hasn't >> >> > been implemented before. >> >> > >> >> > >> >> > Nicolas >> >> > >> >> > _______________________________________________ >> >> > linux-arm-kernel mailing list >> >> > linux-arm-kernel@lists.infradead.org >> >> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> >> -- >> >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> >> the body of a message to majordomo@vger.kernel.org >> >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> Please read the FAQ at http://www.tux.org/lkml/ >> >> >> > >> > -- >> > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in >> > the body of a message to majordomo@vger.kernel.org >> > More majordomo info at http://vger.kernel.org/majordomo-info.html >> >
Hi Lorenzo and Chander, On Wed, Jun 11, 2014 at 6:45 PM, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Wed, Jun 11, 2014 at 01:14:21PM +0100, Chander Kashyap wrote: >> On Wed, Jun 11, 2014 at 3:43 PM, Lorenzo Pieralisi >> <lorenzo.pieralisi@arm.com> wrote: >> > On Wed, Jun 11, 2014 at 05:52:10AM +0100, Chander Kashyap wrote: >> >> Hi Doug, >> >> >> >> On Tue, Jun 10, 2014 at 9:19 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: >> >> > On Tue, 10 Jun 2014, Doug Anderson wrote: >> >> > >> >> >> My S-state knowledge is not strong, but I believe that Lorenzo's >> >> >> questions matter if we're using S2 for CPUidle (where we actually turn >> >> >> off power and hot unplug CPUs) but not when we're using S1 for CPUidle >> >> >> (where we just enter WFI/WFE). >> >> >> >> >> >> >> No Its not plain WFI. >> >> >> >> All cores in Exynos5420 can be powered off independently. >> >> This functionality has been tested. >> >> >> >> Below is the link for the posted patches. >> >> >> >> https://lkml.org/lkml/2014/6/10/194 >> >> >> >> And as Nicolas wrote, these patches need MCPM for that. >> > >> > Chander, I cast a look into the code and I have a question >> > (you also told me on CPUidle review that only core off >> > is supported in CPUidle). >> > >> > When you say all cores can be powered off independently, do >> > you also mean that clusters can be powered off (in CPUidle) ? >> > >> > I am pointing this out since in the MCPM backend I noticed: >> > >> > "/* TODO: Turn off the cluster here to save power. */" This is something that pends on me. >> > >> > I do not see any cluster power down request in the down path. >> > >> > If I am wrong, ignore my message, I am just writing to help. >> > >> > If you can only power down cores, but not the clusters on idle, >> > please keep in mind that you are currently cleaning and invalidating >> > the L2 when last man is running and this must not be taken >> > lightly since, if L2 stays on, that's a massive energy waste >> > for nothing. >> > >> > So, if clusters stay up, you _have_ to tweak the MCPM backend slightly >> > to avoid cleaning L2, that's pivotal. >> >> Hi Lorenzo, >> Cluster shutdown is in progress. Abhilash will add support for that. >> >> https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg31104.html > > Hi Chander, > > thanks. So, as a heads-up: > > 1) if you merge CPUidle support now, as it is it is at least suboptimal, may > even burn more energy than it saves. Latencies in the bL idle driver > are likely to be wrong, since they are for cluster shutdown and for > TC2, not core power gating that should require shorter target_residencies. > On top of that, L2 is cleaned and invalidated for nothing. > 2) when cluster support is merged, you might want to extend the CPUidle > driver to add an additional state (ie C1 core gating, C2 cluster > gating) and to do that you should extend the driver and the MCPM > back-end accordingly, I discussed that with Nico already some time ago > and actually it should be fairly easy to do but we have got to talk > about that. I have already implemented the missing cluster power down code and tested it on an exynos5420 based chromebook. I plan to post the patch as soon as I am back in office, which should be early next week. Regards, Abhilash > > Thank you, > Lorenzo > >> >> >> > >> > Lorenzo >> > >> >> >> >> >> I believe that in ChromeOS we use S1 CPUidle and that it works fine. >> >> >> We've never implemented S2 that I'm aware of. >> >> > >> >> > You'll have to rely on MCPM for that. That's probably why it hasn't >> >> > been implemented before. >> >> > >> >> > >> >> > Nicolas >> >> > >> >> > _______________________________________________ >> >> > linux-arm-kernel mailing list >> >> > linux-arm-kernel@lists.infradead.org >> >> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >> >> -- >> >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> >> the body of a message to majordomo@vger.kernel.org >> >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> Please read the FAQ at http://www.tux.org/lkml/ >> >> >> > >> > -- >> > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in >> > the body of a message to majordomo@vger.kernel.org >> > More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
Chander, On Tue, Jun 10, 2014 at 9:52 PM, Chander Kashyap <k.chander@samsung.com> wrote: > Hi Doug, > > On Tue, Jun 10, 2014 at 9:19 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote: >> On Tue, 10 Jun 2014, Doug Anderson wrote: >> >>> My S-state knowledge is not strong, but I believe that Lorenzo's >>> questions matter if we're using S2 for CPUidle (where we actually turn >>> off power and hot unplug CPUs) but not when we're using S1 for CPUidle >>> (where we just enter WFI/WFE). >>> > > No Its not plain WFI. > > All cores in Exynos5420 can be powered off independently. > This functionality has been tested. > > Below is the link for the posted patches. > > https://lkml.org/lkml/2014/6/10/194 > > And as Nicolas wrote, these patches need MCPM for that. Most excellent! I should have been more clear that I only knew about how CPUidle worked in our local production kernel. There I'm pretty sure CPUidle is just WFI/WFE. If you've got patches to do better then that's great! ...can you confirm that my patch doesn't interfere with your improved CPUidle? It's been Acked by Nicolas (thanks!) so I'd imagine it will land shortly. Kukjin: I assume you'll be taking this? -Doug
On 06/12/14 00:19, Doug Anderson wrote: > Chander, > > On Tue, Jun 10, 2014 at 9:52 PM, Chander Kashyap<k.chander@samsung.com> wrote: >> Hi Doug, >> >> On Tue, Jun 10, 2014 at 9:19 PM, Nicolas Pitre<nicolas.pitre@linaro.org> wrote: >>> On Tue, 10 Jun 2014, Doug Anderson wrote: >>> >>>> My S-state knowledge is not strong, but I believe that Lorenzo's >>>> questions matter if we're using S2 for CPUidle (where we actually turn >>>> off power and hot unplug CPUs) but not when we're using S1 for CPUidle >>>> (where we just enter WFI/WFE). >>>> >> >> No Its not plain WFI. >> >> All cores in Exynos5420 can be powered off independently. >> This functionality has been tested. >> >> Below is the link for the posted patches. >> >> https://lkml.org/lkml/2014/6/10/194 >> >> And as Nicolas wrote, these patches need MCPM for that. > > Most excellent! I should have been more clear that I only knew about > how CPUidle worked in our local production kernel. There I'm pretty > sure CPUidle is just WFI/WFE. If you've got patches to do better then > that's great! > > ...can you confirm that my patch doesn't interfere with your improved > CPUidle? It's been Acked by Nicolas (thanks!) so I'd imagine it will > land shortly. Kukjin: I assume you'll be taking this? > Sure, I will ;-) Thanks, Kukjin
On Wed, Jun 11, 2014 at 8:58 PM, Kukjin Kim <kgene.kim@samsung.com> wrote: > On 06/12/14 00:19, Doug Anderson wrote: >> >> Chander, >> >> On Tue, Jun 10, 2014 at 9:52 PM, Chander Kashyap<k.chander@samsung.com> >> wrote: >>> >>> Hi Doug, >>> >>> On Tue, Jun 10, 2014 at 9:19 PM, Nicolas Pitre<nicolas.pitre@linaro.org> >>> wrote: >>>> >>>> On Tue, 10 Jun 2014, Doug Anderson wrote: >>>> >>>>> My S-state knowledge is not strong, but I believe that Lorenzo's >>>>> questions matter if we're using S2 for CPUidle (where we actually turn >>>>> off power and hot unplug CPUs) but not when we're using S1 for CPUidle >>>>> (where we just enter WFI/WFE). >>>>> >>> >>> No Its not plain WFI. >>> >>> All cores in Exynos5420 can be powered off independently. >>> This functionality has been tested. >>> >>> Below is the link for the posted patches. >>> >>> https://lkml.org/lkml/2014/6/10/194 >>> >>> And as Nicolas wrote, these patches need MCPM for that. >> >> >> Most excellent! I should have been more clear that I only knew about >> how CPUidle worked in our local production kernel. There I'm pretty >> sure CPUidle is just WFI/WFE. If you've got patches to do better then >> that's great! >> >> ...can you confirm that my patch doesn't interfere with your improved >> CPUidle? It's been Acked by Nicolas (thanks!) so I'd imagine it will >> land shortly. Kukjin: I assume you'll be taking this? >> This patch is effectively changing the mcpm_entry_point address from nsbase + 0x1c to nsbase + 0x8 Hence while integrating with mainline u-boot we need to take care for new mcpm_entry_point address. With Chromebook it works straightforward. > Sure, I will ;-) > > Thanks, > Kukjin > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" > in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 13 Jun 2014, Chander Kashyap wrote: > This patch is effectively changing the mcpm_entry_point address from > nsbase + 0x1c to nsbase + 0x8 > > Hence while integrating with mainline u-boot we need to take care for > new mcpm_entry_point address. Why not inserting a NOP as the first instruction then? That way the offset will remain the same. Nicolas
Chander, On Fri, Jun 13, 2014 at 4:54 AM, Chander Kashyap <k.chander@samsung.com> wrote: > This patch is effectively changing the mcpm_entry_point address from > nsbase + 0x1c to nsbase + 0x8 > > Hence while integrating with mainline u-boot we need to take care for > new mcpm_entry_point address. > > With Chromebook it works straightforward. Can you explain more and point to the code that is using the nsbase + 0x1c? Specifically the only code I see that uses the nsbase + 0x1c is the code that is located at nsbase, which is the code we're overwriting here. I'd imagine you're using U-Boot code that looks something like the bits that start at code_base here: https://chromium.googlesource.com/chromiumos/third_party/u-boot/+/ce358daf5069f1dc145b0f9d403cfbb028271807/arch/arm/cpu/armv7/exynos/lowlevel.S With my kernel change you can completely eliminate U-Boot's installation of this code (or keep it, it makes no difference). -Doug
Hi Doug, On 13 June 2014 20:40, Doug Anderson <dianders@chromium.org> wrote: > Chander, > > On Fri, Jun 13, 2014 at 4:54 AM, Chander Kashyap <k.chander@samsung.com> wrote: >> This patch is effectively changing the mcpm_entry_point address from >> nsbase + 0x1c to nsbase + 0x8 >> >> Hence while integrating with mainline u-boot we need to take care for >> new mcpm_entry_point address. >> >> With Chromebook it works straightforward. > > Can you explain more and point to the code that is using the nsbase + > 0x1c? Specifically the only code I see that uses the nsbase + 0x1c is > the code that is located at nsbase, which is the code we're > overwriting here. I'd imagine you're using U-Boot code that looks > something like the bits that start at code_base here: > > https://chromium.googlesource.com/chromiumos/third_party/u-boot/+/ce358daf5069f1dc145b0f9d403cfbb028271807/arch/arm/cpu/armv7/exynos/lowlevel.S > > With my kernel change you can completely eliminate U-Boot's > installation of this code (or keep it, it makes no difference). Yes i agree with your point. What i am saying is when there is full support for Exynos5420 in mainline u-boot we need to take care for the mcpm_entry_point address. > > -Doug
Kukjin, On Wed, Jun 11, 2014 at 8:28 AM, Kukjin Kim <kgene.kim@samsung.com> wrote: > On 06/12/14 00:19, Doug Anderson wrote: >> >> Chander, >> >> On Tue, Jun 10, 2014 at 9:52 PM, Chander Kashyap<k.chander@samsung.com> >> wrote: >>> >>> Hi Doug, >>> >>> On Tue, Jun 10, 2014 at 9:19 PM, Nicolas Pitre<nicolas.pitre@linaro.org> >>> wrote: >>>> >>>> On Tue, 10 Jun 2014, Doug Anderson wrote: >>>> >>>>> My S-state knowledge is not strong, but I believe that Lorenzo's >>>>> questions matter if we're using S2 for CPUidle (where we actually turn >>>>> off power and hot unplug CPUs) but not when we're using S1 for CPUidle >>>>> (where we just enter WFI/WFE). >>>>> >>> >>> No Its not plain WFI. >>> >>> All cores in Exynos5420 can be powered off independently. >>> This functionality has been tested. >>> >>> Below is the link for the posted patches. >>> >>> https://lkml.org/lkml/2014/6/10/194 >>> >>> And as Nicolas wrote, these patches need MCPM for that. >> >> >> Most excellent! I should have been more clear that I only knew about >> how CPUidle worked in our local production kernel. There I'm pretty >> sure CPUidle is just WFI/WFE. If you've got patches to do better then >> that's great! >> >> ...can you confirm that my patch doesn't interfere with your improved >> CPUidle? It's been Acked by Nicolas (thanks!) so I'd imagine it will >> land shortly. Kukjin: I assume you'll be taking this? >> > Sure, I will ;-) I see that you put some branches up about 3 hours ago and I don't see this patch. Are you planning on applying it? It would be nice if it was in 3.16 (since it causes problems booting), but if there's some reason it needs to be for-next that's OK too. -Doug
diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c index 0498d0b..3a7fad0 100644 --- a/arch/arm/mach-exynos/mcpm-exynos.c +++ b/arch/arm/mach-exynos/mcpm-exynos.c @@ -343,11 +343,13 @@ static int __init exynos_mcpm_init(void) pr_info("Exynos MCPM support installed\n"); /* - * Future entries into the kernel can now go - * through the cluster entry vectors. + * U-Boot SPL is hardcoded to jump to the start of ns_sram_base_addr + * as part of secondary_cpu_start(). Let's redirect it to the + * mcpm_entry_point(). */ - __raw_writel(virt_to_phys(mcpm_entry_point), - ns_sram_base_addr + MCPM_BOOT_ADDR_OFFSET); + __raw_writel(0xe59f0000, ns_sram_base_addr); /* ldr r0, [pc, #0] */ + __raw_writel(0xe12fff10, ns_sram_base_addr + 4); /* bx r0 */ + __raw_writel(virt_to_phys(mcpm_entry_point), ns_sram_base_addr + 8); iounmap(ns_sram_base_addr);
On exynos mcpm systems the firmware is hardcoded to jump to an address in SRAM (0x02073000) when secondary CPUs come up. By default the firmware puts a bunch of code at that location. That code expects the kernel to fill in a few slots with addresses that it uses to jump back to the kernel's entry point for secondary CPUs. Originally (on prerelease hardware) this firmware code contained a bunch of workarounds to deal with boot ROM bugs. However on all shipped hardware we simply use this code to redirect to a kernel function for bringing up the CPUs. Let's stop relying on the code provided by the bootloader and just plumb in our own (simple) code jump to the kernel. This has the nice benefit of fixing problems due to the fact that older bootloaders (like the one shipped on the Samsung Chromebook 2) might have put slightly different code into this location. Once suspend/resume is implemented for systems using exynos-mcpm we'll need to make sure we reinstall our fixed up code after resume. ...but that's not anything new since IRAM (and thus the address of the mcpm_entry_point) is lost across suspend/resume anyway. Signed-off-by: Doug Anderson <dianders@chromium.org> --- arch/arm/mach-exynos/mcpm-exynos.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)