Re: RFC: Simplification of Power Domain Control

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

 



Hi Jean,

On Thursday 05 July 2012 06:36 PM, Jean Pihet wrote:
Hi Rajendra,

On Thu, Jul 5, 2012 at 2:47 PM, Rajendra Nayak<rnayak@xxxxxx>  wrote:
Hi,

This RFC is aimed at simplifying the powerdomain control
(in software) for OMAP. Powerdomains on OMAP have been
fairly complex to program (as compared to other SoCs)
mainly due to the multiple memory/logic control exposed
to software. These controls are limited to achieving
whats know as 'OSWR: Open Switch Retention' state in OMAP.
Rest of the states supported (ON, INACTIVE, CSWR, OFF) are
fairly easy to program needing just a target power state
to be configured.

While in theory, the hardware does expose multiple ways/
combinations in which OSWR can be achieved, we in software
have always been  using just *one* combination all along.
OSWR has always been configured as *logic lost, all memory
retained* for all current OMAPs supported.

With future OMAPs planning to get rid of all the memory/logic
control altogether it makes sense to look at the apis exposed
through the current powerdomain framework to its users (like
cpuilde, suspend etc) and see if those could also be simplified.

The RFC is suspend tested on OMAP4430sdp and
OMAP3630Beagle-Xm.
This looks very similar to the functional power states patches that I
submitted a few times for review [1].

Sorry, I should have mentioned the context in which this RFC was
developed. Yes, its very similar to the functional power state
approach, infact some of these thoughts came up as part of the
discussion between me and Santosh while doing the review for
that very series.

regards,
Rajendra

The idea is to simplify the external API to program the power domains
states. The code has been used to implement the OMAP4 device OFF
support (from Tero) and the per-device PM QoS implementation for OMAP
[2].

[1] http://marc.info/?l=linux-omap&m=133968580905048&w=2
[2] http://marc.info/?l=linux-omap&m=133968657005566&w=2

Now that we have more than one implementation, I let it to the
maintainers for decision.

Paul, what do you think?

A few remarks here below.

Thanks for your RFC!


regards,
Rajendra

----
 From 5f5e4eb342110286bf719c7d9d7c1959f53e34f9 Mon Sep 17 00:00:00 2001
From: Rajendra Nayak<rnayak@xxxxxx>
Date: Thu, 5 Jul 2012 17:33:28 +0530
Subject: [RFC] ARM: OMAP: Powerdomain: control memory and logic bits internally

Powerdomain framework exposes various apis for memory and logic
control for powerdomains, for its users to program OSWR: Open SWitch
Retention state. While in theory, there are various combinations of
memory and logic states possible which can be configured as OSWR,
in practice all OMAPs use just one combination. Logic lost, memory retained.

This can very easily be handled within the powerdomain framework itself,
without exposing all complex memory/logic control apis to upper layer
drivers like cpuidle and suspend.

To do this, introduce 2 new power domain states PWRDM_POWER_CSWR and
PWRDM_POWER_OSWR usable by the users of powerdomain framework and
make all memory/logic control apis internal to powerdomain framework.
Change all users of powerdomain framework to get rid of all usage
of memory/logic control apis and use the newly defined states for
CSWR and OSWR with the already used powerstate control apis.

Some functions (which are now made internal) are forward declared
to avoid moving functions around in the file (which can be done in a
later patch) to help keep the patch reviewable.

Signed-off-by: Rajendra Nayak<rnayak@xxxxxx>
---
  arch/arm/mach-omap2/cpuidle44xx.c                |    9 +---
  arch/arm/mach-omap2/omap-mpuss-lowpower.c        |    7 +--
  arch/arm/mach-omap2/pm24xx.c                     |   17 +------
  arch/arm/mach-omap2/pm34xx.c                     |    8 ++--
  arch/arm/mach-omap2/pm44xx.c                     |   11 +---
  arch/arm/mach-omap2/powerdomain-private.h        |   17 +++++++
  arch/arm/mach-omap2/powerdomain.c                |   54 +++++++++++++++++++---
  arch/arm/mach-omap2/powerdomain.h                |   14 +-----
  arch/arm/mach-omap2/powerdomains2xxx_3xxx_data.c |    1 +
  arch/arm/mach-omap2/powerdomains2xxx_data.c      |    1 +
  arch/arm/mach-omap2/powerdomains3xxx_data.c      |    1 +
  arch/arm/mach-omap2/powerdomains44xx_data.c      |    1 +
  12 files changed, 85 insertions(+), 56 deletions(-)
  create mode 100644 arch/arm/mach-omap2/powerdomain-private.h

...

diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
index 13670aa..41d559b 100644
--- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
+++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
...
@@ -243,7 +243,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
         case PWRDM_POWER_OFF:
                 save_state = 1;
                 break;
-       case PWRDM_POWER_RET:
+       case PWRDM_POWER_CSWR:
         default:
                 /*
                  * CPUx CSWR is invalid hardware state. Also CPUx OSWR
@@ -262,8 +262,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
          * In MPUSS OSWR or device OFF, interrupt controller  contest is lost.
          */
         mpuss_clear_prev_logic_pwrst();
-       if ((pwrdm_read_next_pwrst(mpuss_pd) == PWRDM_POWER_RET)&&
-               (pwrdm_read_logic_retst(mpuss_pd) == PWRDM_POWER_OFF))
+       if (pwrdm_read_next_pwrst(mpuss_pd) == PWRDM_POWER_OSWR)
This is the kind of simplification that proves the benefit of the new API.

                 save_state = 2;

         cpu_clear_prev_logic_pwrst(cpu);
...
diff --git a/arch/arm/mach-omap2/powerdomain-private.h b/arch/arm/mach-omap2/powerdomain-private.h
new file mode 100644
index 0000000..0c2dd23
--- /dev/null
+++ b/arch/arm/mach-omap2/powerdomain-private.h
@@ -0,0 +1,17 @@
+/*
+ * Internal powerdomain header
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __ARCH_ARM_MACH_OMAP2_POWERDOMAIN_PRIVATE_H
+#define __ARCH_ARM_MACH_OMAP2_POWERDOMAIN_PRIVATE_H
+
+#define PWRDM_POWER_RET                0x1
+
+#endif /* __ARCH_ARM_MACH_OMAP2_POWERDOMAIN_PRIVATE_H */
I like this ;p
In fact much more of the powerdomain*.[ch] should be hidden in the
private API header file.
My patch series addresses this by separating the private from the
public APIs in powerdomain.h.

...

Regards,
Jean

--
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