[PATCH 0/4] ARM: EXYNOS4: Support generic Power domain framework for EXYNOS4210

[Rafael J. Wysocki]

On Sunday, October 02, 2011, Sylwester Nawrocki wrote:
> On 10/02/2011 09:07 AM, Kukjin Kim wrote:
> > Rafael J. Wysocki wrote:
> >>
> >> On Saturday, August 13, 2011, Russell King - ARM Linux wrote:
> >>> On Sat, Aug 13, 2011 at 11:24:07PM +0200, Rafael J. Wysocki wrote:
> >>>> On Thursday, August 11, 2011, Chanwoo Choi wrote:
> >>>>> The following patch set use the generic Power domain Framework
> > instead of
> >>>>> power domain code depend of Samsung SoC.
> >>>>>
> >>>>> Chanwoo Choi (4):
> >>>>>    ARM: EXYNOS4: Support for generic I/O power domains on EXYNOS4210
> >>>>>    ARM: EXYNOS4: Support for generic Clock manipulation PM callbacks
> >>>>>    ARM: EXYNOS4: Delete the power-domain code depend on Samsung SoC
> >>>>>    ARM: EXYNOS4: Add power domain to use generic Power domain
> > Framework
> >>>>>
> >>>>>   arch/arm/mach-exynos4/Kconfig                      |   10 +-
> >>>>>   arch/arm/mach-exynos4/Makefile                     |    4 +-
> >>>>>   arch/arm/mach-exynos4/dev-pd.c                     |  139
> > --------------
> >>>>>   arch/arm/mach-exynos4/include/mach/pm-exynos4210.h |   52 ++++++
> >>>>>   arch/arm/mach-exynos4/include/mach/regs-clock.h    |    8 +
> >>>>>   arch/arm/mach-exynos4/mach-nuri.c                  |   21 ++-
> >>>>>   arch/arm/mach-exynos4/mach-smdkc210.c              |   26 ++-
> >>>>>   arch/arm/mach-exynos4/mach-smdkv310.c              |   23 ++-
> >>>>>   arch/arm/mach-exynos4/mach-universal_c210.c        |   21 ++-
> >>>>>   arch/arm/mach-exynos4/pm-exynos4210.c              |  189
> > ++++++++++++++++++++
> >>>>>   arch/arm/mach-exynos4/pm-runtime.c                 |   56 ++++++
> >>>>>   arch/arm/plat-samsung/Kconfig                      |    8 -
> >>>>>   arch/arm/plat-samsung/Makefile                     |    4 -
> >>>>>   arch/arm/plat-samsung/include/plat/pd.h            |   30 ---
> >>>>>   arch/arm/plat-samsung/pd.c                         |   95
> > ----------
> >>>>>   15 files changed, 377 insertions(+), 309 deletions(-)
> >>>>>   delete mode 100644 arch/arm/mach-exynos4/dev-pd.c
> >>>>>   create mode 100644
> > arch/arm/mach-exynos4/include/mach/pm-exynos4210.h
> >>>>>   create mode 100644 arch/arm/mach-exynos4/pm-exynos4210.c
> >>>>>   create mode 100644 arch/arm/mach-exynos4/pm-runtime.c
> >>>>>   delete mode 100644 arch/arm/plat-samsung/include/plat/pd.h
> >>>>>   delete mode 100644 arch/arm/plat-samsung/pd.c
> >>>>
> >>>> The patchset looks good to me, but please note that some code it
> >>>> is based on will most likely change in 3.2 due to this patchset:
> >>>>
> >>>> https://lkml.org/lkml/2011/8/8/420
> >>>
> >>> Err, isn't all that pm_clk stuff just duplicating what the clk API does?
> >>
> >> I'm not sure it's duplicating anything.  Maybe it does, but it came into
> >> being by moving some code that were duplicated in a few places throughout
> >> the ARM and sh trees into one place.
> >>
> >>> IOW, drivers _can_ (and should be) calling clk_disable() when they don't
> >>> need the clock running.
> >>
> >> Drivers may not know about what to do in a given situation.  For example,
> >> if the system has power domains, it may be better to switch a power domain
> >> off instead of or in addition to disabling the clock and the driver
> > usually
> >> doesn't know about that.
> >>
> > Hmm... Even though each driver cannot know the given situation, the driver
> > can know each own clock should be alive or not. I think, if clock gating
> > (enable, disable clock) is required, it should be handled in each driver. In
> > addition, the clock and power are not always one-on-one match.
> 
> AFAICS the genpd API doesn't for an architecture to move the clocks' control
> out from drivers, it can be decided on individual basis for each clock.

That's correct.
 
> With Chanwoo's patches at least exynos power domains and global clock gates 
> for devices (S5P_CLKGATE_BLOCK) are handled through the common API.
> 
> Also, it's not quite clear to me, can't the pm_clk_* calls be used by the drivers
> to control the clocks as they need ?

Well, the idea is to use them as a driver's or subsystem's .runtime_suspend()
and .runtime_resume() callbacks, or a PM domain's .stop_device() and
.start_device() callbacks, but I don't see fundamental arguments against
calling them directly.

> Last time I looked at the genpd API, an important drawback was that pm_clk_suspend/
> pm_clk_resume couldn't be used in interrupt context. Now it seems the mutexes in
> the core functions has been replaced with spinlocks and such limitation is gone.

Yes, it is gone now.

> I'm considering using pm_clk* calls to implement clock gating in a video codec
> driver. If, for instance, frame period is 30 ms and processing in the device takes
> only 10 ms, significant power savings could be achieved by turning the clocks off
> for the 20 ms idle period.  

That sounds like a good idea.

Thanks,
Rafael