Re: [PATCH 8/8] ARM / shmobile: Support for I/O PM domains for SH7372 (v5)

On Wednesday, June 15, 2011, Magnus Damm wrote:

On Wed, Jun 15, 2011 at 6:16 AM, Rafael J. Wysocki [off-list ref] wrote:

quoted

On Tuesday, June 14, 2011, Magnus Damm wrote:

quoted

On Sun, Jun 12, 2011 at 5:40 AM, Rafael J. Wysocki [off-list ref] wrote:

quoted

From: Rafael J. Wysocki <redacted>

Use the generic power domains support introduced by the previous
patch to implement support for power domains on SH7372.

Signed-off-by: Rafael J. Wysocki <redacted>
---

Thanks for your work on this. I just tried this on my Mackerel board,
but I can't seem to get the pd_power_up() and pd_power_down()
callbacks to be executed. It is probably a misconfiguration from my
side.

They trigger for me e.g. after doing

# echo 3 > /sys/devices/platform/sh_mobile_lcdc_fb.0/graphics/fb0/blank

Attached is the .config I've been using.

Thanks, I can trigger using sysfs and your kernel configuration.

Good.

However, I assumed it also would work when the sceen saver kicked in.
I recall it being fbcon that controls the screen save, perhaps
something else. So just wait a bit and see if you also can reproduce
it. The console gets black but the power is still on...

I noticed that, but I think it simply means pm_runtime_put() isn't
called in that code path.

Also forcing to go back to powered-on state (see below) doesn't work that well:
# echo 0 > /sys/devices/platform/sh_mobile_lcdc_fb.0/graphics/fb0/blank

It looks like we loose the panning information somehow. Most likely a
LCDC driver bug. Unless the driver callbacks are not being invoked as
expected.

That should be easy to verify, I'll do that.

Also, there is garbage in on the screen if FB_SH_MOBILE_MERAM is
enabled. The MERAM hardware is a 1.5 MiB memory block that can be used
as a LCD cache. It sits in the same hardware power domain as the
LCDCs. I don't think the MERAM software supports power down
unfortunately. Disabling MERAM support removes the garbage on the
screen.

Well, I can't really comment here.

quoted

quoted

Here's some feedback on the sh7372-specific code:

quoted

--- linux-2.6.orig/arch/arm/mach-shmobile/Kconfig
+++ linux-2.6/arch/arm/mach-shmobile/Kconfig

@@ -19,6 +19,7 @@ config ARCH_SH7372
       select CPU_V7
       select SH_CLK_CPG
       select ARCH_WANT_OPTIONAL_GPIOLIB
+       select PM_GENERIC_DOMAINS

We want to support a single ARM binary for multiple boards,

Surely CONFIG_ARCH_SH7372 will be set in that binary?

quoted

so this should be enabled for all SoCs in mach-shmobile as a whole.

OK, where exactly do you want me to move it?

Ideally to ARCH_SHMOBILE in arch/arm/Kconfig.

OK

quoted

quoted

quoted

--- linux-2.6.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c

@@ -15,16 +15,97 @@
 #include <linux/list.h>
 #include <linux/err.h>
 #include <linux/slab.h>
+#include <linux/pm_runtime.h>
+#include <linux/platform_device.h>
 #include <asm/system.h>
 #include <asm/io.h>
 #include <asm/tlbflush.h>
 #include <mach/common.h>
+#include <mach/sh7372.h>

 #define SMFRAM 0xe6a70000
 #define SYSTBCR 0xe6150024
 #define SBAR 0xe6180020
 #define APARMBAREA 0xe6f10020

+#define SPDCR 0xe6180008
+#define SWUCR 0xe6180014
+#define PSTR 0xe6180080
+
+struct sh7372_domain_data {
+       unsigned int bit_shift;
+};

Is it possible to make struct sh7372_domain_data include struct
generic_pm_domain?

It should be possible to do that.

Do I understand it correctly that you want one structure definition per
power domain instead of the two?

Yes, at least that's what I would do to keep the data together. I
don't care that much though, so feel free to implement it however
you'd like.

OK, I think I can merge them.

Thanks,
Rafael