All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V10 0/3] PM / Domains: Performance state support
@ 2017-09-19 22:32 Viresh Kumar
  2017-09-19 22:32 ` [PATCH V10 1/7] PM / Domains: Add support to select performance-state of domains Viresh Kumar
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-09-19 22:32 UTC (permalink / raw)
  To: Rafael Wysocki, ulf.hansson, Kevin Hilman
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Stephen Boyd,
	Nishanth Menon, robh+dt, lina.iyer, rnayak, sudeep.holla,
	linux-kernel, Len Brown, Pavel Machek, Andy Gross, David Brown

Hi Ulf,

This version contains the changes we discussed during the LPC.

Some platforms have the capability to configure the performance state of
their power domains. The process of configuring the performance state is
pretty much platform dependent and we may need to work with a wide range
of configurables.  For some platforms, like Qcom, it can be a positive
integer value alone, while in other cases it can be voltage levels, etc.

The power-domain framework until now was only designed for the idle
state management of the device and this needs to change in order to
reuse the power-domain framework for active state management of the
devices.

The first patch updates the genpd framework to supply new APIs to
support active state management and the second patch uses them from the
OPP core. The third patch adds some more checks to the genpd core to
catch bugs.

Rest of the patches [4-7/7] are included just to show how user drivers
would end up using the new APIs and these patches aren't there to get
merged and are marked clearly like that.

The ideal way is still to get the relation between device and domain
states via the DT instead of platform code, but that can be done
incrementally later once we have some users for it upstream.

This is currently tested by:
- /me by hacking the kernel a bit with virtual power-domains for the ARM
  64 hikey platform. I have also tested the complex cases where the
  device's parent power domain doesn't have set_performance_state()
  callback set, but parents of that domains have it. Lockdep configs
  were enabled for these tests.
- Rajendra Nayak, on msm8996 platform (Qcom) with MMC controller.

Thanks Rajendra for helping me testing this out.

I also had a chat with Rajendra and we should be able to get a Qualcomm
specific power domain driver (which uses these changes) in coming weeks.
Though it wouldn't solve all the problems around corners they have, as
they need more updates later on (Like support for multiple masters for a
device, etc).

Pushed here as well:

https://git.linaro.org/people/viresh.kumar/linux.git/log/?h=opp/genpd-performance-state

Rebased over: 4.14-rc1

V9->V10:
- Performance state of masters is updated before the state of the genpd
  (Ulf).
- 2/7 and 3/7 are swapped.

V8->V9:
- Renamed genpd callbacks and internal routines.
- dev_pm_genpd_has_performance_state() simplified a lot and doesn't
  check master hierarchy now. Rather a new patch (2/7) is added to
  take care of that and WARN if no master has set
  genpd_set_performance_state() callback.
- Update is propagated to the masters even if the genpd's callback is
  already called.
- Exit _genpd_reeval_performance_state() early if no state change is
  required and it gets an additional argument (new state of the
  device/subdomain).
- Taken care of genpd on/off cases.
- s/parent/master everywhere in comments and logs.
- Better explanations in logs, comments etc.
- All the other patches (3-7/7) are same as V8. (Just minor update in
  5/7 to use the updated callback names).

V7->V8:
- Ulf helped a lot in reviewing V7 and pointed out couple of issues,
  specially in locking while dealing with a hierarchy of power domains.
- All those locking issues are sorted out now, even for the complex
  cases.
- genpd_lookup_dev() is used in pm_genpd_has_performance_state() to make
  sure we have a valid genpd available for the device.
- Validation of performance state callbacks isn't done anymore in
  pm_genpd_init() as it gets called very early and the binding of
  subdomains to their parent domains happens later. This is handled in
  pm_genpd_has_performance_state() now, which is called from user
  drivers.
- User driver changes (not to be merged) are included for the first time
  here, to demonstrate how changes would look finally.

V6->V7:
- Almost a rewrite, only two patches against 9 in earlier version.
- No bindings updated now and domain's performance state aren't passed
  via DT for now (until we know how users are going to use it).
- We also skipped the QoS framework completely and new APIs are provided
  directly by genpd.

V5->V6:
- Use freq/voltage in OPP table as it is for power domain and don't
  create "domain-performance-level" property
- Create new "power-domain-opp" property for the devices.
- Take care of domain providers that provide multiple domains and extend
  "operating-points-v2" property to contain a list of phandles
- Update code according to those bindings.

V4->V5:
- Only 3 patches were resent and 2 of them are Acked from Ulf.

V3->V4:
- Use OPP table for genpd devices as well.
- Add struct device to genpd, in order to reuse OPP infrastructure.
- Based over: https://marc.info/?l=linux-kernel&m=148972988002317&w=2
- Fixed examples in DT document to have voltage in target,min,max order.

V2->V3:
- Based over latest pm/linux-next
- Bindings and code are merged together
- Lots of updates in bindings
  - the performance-states node is present within the power-domain now,
    instead of its phandle.
  - performance-level property is replaced by "reg".
  - domain-performance-state property of the consumers contain an
    integer value now instead of phandle.
- Lots of updates to the code as well
  - Patch "PM / QOS: Add default case to the switch" is merged with
    other patches and the code is changed a bit as well.
  - Don't pass 'type' to dev_pm_qos_add_notifier(), rather handle all
    notifiers with a single list. A new patch is added for that.
  - The OPP framework patch can be applied now and has proper SoB from
    me.
  - Dropped "PM / domain: Save/restore performance state at runtime
    suspend/resume".
  - Drop all WARN().
  - Tested-by Rajendra nayak.

V1->V2:
- Based over latest pm/linux-next
- It is mostly a resend of what is sent earlier as this series hasn't
  got any reviews so far and Rafael suggested that its better I resend
  it.
- Only the 4/6 patch got an update, which was shared earlier as reply to
  V1 as well. It has got several fixes for taking care of power domain
  hierarchy, etc.

--
viresh

Rajendra Nayak (4):
  soc: qcom: rpmpd: Add a powerdomain driver to model cx/mx powerdomains
  soc: qcom: rpmpd: Add support for get/set performance state
  mmc: sdhci-msm: Adapt the driver to use OPPs to set clocks/performance
    state
  remoteproc: qcom: q6v5: Vote for proxy powerdomain performance state

Viresh Kumar (3):
  PM / Domains: Add support to select performance-state of domains
  PM / OPP: Support updating performance state of device's power domains
  PM / Domains: Catch missing genpd_set_performance_state() in masters

 .../devicetree/bindings/power/qcom,rpmpd.txt       |  10 +
 arch/arm64/boot/dts/qcom/msm8996.dtsi              |  39 ++
 drivers/base/power/domain.c                        | 293 ++++++++++++++-
 drivers/base/power/opp/core.c                      |  48 ++-
 drivers/base/power/opp/opp.h                       |   2 +
 drivers/clk/qcom/gcc-msm8996.c                     |   8 +-
 drivers/mmc/host/sdhci-msm.c                       |  39 +-
 drivers/remoteproc/qcom_q6v5_pil.c                 |  20 +-
 drivers/soc/qcom/Kconfig                           |   9 +
 drivers/soc/qcom/Makefile                          |   1 +
 drivers/soc/qcom/rpmpd.c                           | 412 +++++++++++++++++++++
 include/linux/pm_domain.h                          |  23 ++
 12 files changed, 880 insertions(+), 24 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/power/qcom,rpmpd.txt
 create mode 100644 drivers/soc/qcom/rpmpd.c

-- 
2.7.4

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH V10 1/7] PM / Domains: Add support to select performance-state of domains
  2017-09-19 22:32 [PATCH V10 0/3] PM / Domains: Performance state support Viresh Kumar
@ 2017-09-19 22:32 ` Viresh Kumar
  2017-09-19 22:32 ` [PATCH V10 2/7] PM / OPP: Support updating performance state of device's power domains Viresh Kumar
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-09-19 22:32 UTC (permalink / raw)
  To: Rafael Wysocki, ulf.hansson, Kevin Hilman
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Stephen Boyd,
	Nishanth Menon, robh+dt, lina.iyer, rnayak, sudeep.holla,
	linux-kernel, Len Brown, Pavel Machek, Andy Gross, David Brown

Some platforms have the capability to configure the performance state of
PM domains and this patch updates the genpd core to support them.

The performance levels (within the genpd core) are identified by
positive integer values, a lower value represents lower performance
state.

This patch adds two new genpd APIs:

- int dev_pm_genpd_has_performance_state(struct device *dev)

  This can be called (only once) by the device drivers to make sure all
  dependencies are met and the PM domain of the device supports
  performance states.

- int dev_pm_genpd_update_performance_state(struct device *dev,
  					    unsigned long rate);

  This can be called (any number of times) by the device drivers after
  they have called dev_pm_genpd_has_performance_state() once and it
  returned "true".

  This call will update the performance state of the PM domain of the
  device (for which this routine is called) and propagate it to the
  masters of the PM domain. This requires certain callbacks to be
  available for the genpd of the device, which are internally called by
  this routine in the order in which they are described below.

  - dev_get_performance_state()

    This shall return the performance state (integer value)
    corresponding to a target frequency for the device. This state is
    used by the genpd core as device's requested performance state and
    would be used while aggregating the requested states of all the
    devices and subdomains for a PM domain.

    Note that the same state value will be used by the device's PM
    domain and its masters hierarchy. We may want to implement master
    specific states later on once we have more complex cases available.

    Providing this callback is mandatory for any genpd which needs to
    manage performance states and is registered as master of one or more
    devices. Domains which only have sub domains and no devices, should
    not implement this callback.

  - genpd_set_performance_state()

    The aggregate of the performance states of the devices and
    subdomains of a PM genpd is then passed to this callback, which must
    change the performance state of the genpd. This callback of the
    masters of the genpd are also called to propagate the change.

  The power domains can avoid supplying these callbacks, if they don't
  support setting performance-states.

Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/domain.c | 277 +++++++++++++++++++++++++++++++++++++++++++-
 include/linux/pm_domain.h   |  23 ++++
 2 files changed, 298 insertions(+), 2 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index e8ca5e2cf1e5..6d05c91cf44f 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -237,6 +237,267 @@ static void genpd_update_accounting(struct generic_pm_domain *genpd)
 static inline void genpd_update_accounting(struct generic_pm_domain *genpd) {}
 #endif
 
+/**
+ * dev_pm_genpd_has_performance_state - Checks if power domain does performance
+ * state management.
+ *
+ * @dev: Device whose power domain is getting inquired.
+ *
+ * This can be called by the user drivers, before they start calling
+ * dev_pm_genpd_update_performance_state(), to guarantee that all dependencies
+ * are met and the device's genpd supports performance states.
+ *
+ * It is assumed that the user driver guarantees that the genpd wouldn't be
+ * detached while this routine is getting called.
+ *
+ * Returns "true" if device's genpd supports performance states, "false"
+ * otherwise.
+ */
+bool dev_pm_genpd_has_performance_state(struct device *dev)
+{
+	struct generic_pm_domain *genpd = genpd_lookup_dev(dev);
+
+	return !IS_ERR_OR_NULL(genpd) && genpd->dev_get_performance_state;
+}
+EXPORT_SYMBOL_GPL(dev_pm_genpd_has_performance_state);
+
+static int _genpd_reeval_performance_state(struct generic_pm_domain *genpd,
+					   int state, int depth);
+
+/* Returns -ve errors or 0 on success */
+static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
+					int state, int depth)
+{
+	struct generic_pm_domain *master;
+	struct gpd_link *link;
+	int prev = genpd->performance_state, ret;
+
+	/* Propagate to masters of genpd */
+	list_for_each_entry(link, &genpd->slave_links, slave_node) {
+		master = link->master;
+
+		genpd_lock_nested(master, depth + 1);
+
+		link->performance_state = state;
+		ret = _genpd_reeval_performance_state(master, state, depth + 1);
+		if (ret)
+			link->performance_state = prev;
+
+		genpd_unlock(master);
+
+		if (ret)
+			goto err;
+	}
+
+	if (genpd->genpd_set_performance_state) {
+		ret = genpd->genpd_set_performance_state(genpd, state);
+		if (ret)
+			goto err;
+	}
+
+	/*
+	 * The masters are updated now, lets get genpd performance_state in sync
+	 * with those.
+	 */
+	genpd->performance_state = state;
+	return 0;
+
+err:
+	/* Encountered an error, lets rollback */
+	list_for_each_entry_continue_reverse(link, &genpd->slave_links,
+					     slave_node) {
+		master = link->master;
+
+		genpd_lock_nested(master, depth + 1);
+		link->performance_state = prev;
+		if (_genpd_reeval_performance_state(master, prev, depth + 1)) {
+			pr_err("%s: Failed to roll back to %d performance state\n",
+			       master->name, prev);
+		}
+		genpd_unlock(master);
+	}
+
+	return ret;
+}
+
+/*
+ * Re-evaluate performance state of a genpd. Finds the highest requested
+ * performance state by the devices and subdomains within the genpd and then
+ * change genpd's performance state (if required). The request is then
+ * propagated to the masters of the genpd.
+ *
+ * @genpd: PM domain whose state needs to be reevaluated.
+ * @state: Newly requested performance state of the device or subdomain for
+ * which this routine is called.
+ * @depth: nesting count for lockdep.
+ *
+ * Locking rules followed are:
+ *
+ * - Device's state (pd_data->performance_state) should be accessed from within
+ *   its master's lock protected section.
+ *
+ * - Subdomains have a separate state field (link->performance_state) per master
+ *   domain and is accessed only from within master's lock protected section.
+ *
+ * - Subdomain's state (genpd->performance_state) should be accessed from within
+ *   its own lock protected section.
+ *
+ * - The locks are always taken in bottom->up order, i.e. subdomain first,
+ *   followed by its masters.
+ *
+ * Returns -ve errors or 0 on success.
+ */
+static int _genpd_reeval_performance_state(struct generic_pm_domain *genpd,
+					   int state, int depth)
+{
+	struct generic_pm_domain_data *pd_data;
+	struct pm_domain_data *pdd;
+	struct gpd_link *link;
+
+	/* New requested state is same as Max requested state */
+	if (state == genpd->performance_state)
+		return 0;
+
+	/* New requested state is higher than Max requested state */
+	if (state > genpd->performance_state)
+		goto update_state;
+
+	/* Traverse all devices within the domain */
+	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
+		pd_data = to_gpd_data(pdd);
+
+		if (pd_data->performance_state > state)
+			state = pd_data->performance_state;
+	}
+
+	/*
+	 * Traverse all subdomains within the domain. This can be done without
+	 * any additional locks as all link->performance_state fields are
+	 * protected by genpd->lock, which is already taken.
+	 */
+	list_for_each_entry(link, &genpd->master_links, master_node) {
+		if (link->performance_state > state)
+			state = link->performance_state;
+	}
+
+update_state:
+	return _genpd_set_performance_state(genpd, state, depth);
+}
+
+static void __genpd_dev_update_performance_state(struct device *dev, int state)
+{
+	struct generic_pm_domain_data *gpd_data;
+
+	spin_lock_irq(&dev->power.lock);
+
+	if (!dev->power.subsys_data || !dev->power.subsys_data->domain_data) {
+		WARN_ON(1);
+		goto unlock;
+	}
+
+	gpd_data = to_gpd_data(dev->power.subsys_data->domain_data);
+	gpd_data->performance_state = state;
+
+unlock:
+	spin_unlock_irq(&dev->power.lock);
+}
+
+/**
+ * dev_pm_genpd_update_performance_state - Update performance state of device's
+ * power domain for the target frequency for the device.
+ *
+ * @dev: Device for which the performance-state needs to be adjusted.
+ * @rate: Device's next frequency. This can be set as 0 when the device doesn't
+ * have any performance state constraints left (And so the device wouldn't
+ * participate anymore to find the target performance state of the genpd).
+ *
+ * The user drivers may want to call dev_pm_genpd_has_performance_state() (only
+ * once) before calling this routine (any number of times) to guarantee that all
+ * dependencies are met.
+ *
+ * It is assumed that the user driver guarantees that the genpd wouldn't be
+ * detached while this routine is getting called.
+ *
+ * Returns 0 on success and negative error values on failures.
+ */
+int dev_pm_genpd_update_performance_state(struct device *dev,
+					  unsigned long rate)
+{
+	struct generic_pm_domain *genpd;
+	int ret, state;
+
+	genpd = dev_to_genpd(dev);
+	if (IS_ERR(genpd))
+		return -ENODEV;
+
+	genpd_lock(genpd);
+
+	if (!genpd_status_on(genpd)) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	state = genpd->dev_get_performance_state(dev, rate);
+	if (state < 0) {
+		ret = state;
+		goto unlock;
+	}
+
+	ret = _genpd_reeval_performance_state(genpd, state, 0);
+	if (!ret) {
+		/*
+		 * Since we are passing "state" to
+		 * _genpd_reeval_performance_state() as well, we don't need to
+		 * call __genpd_dev_update_performance_state() before updating
+		 * genpd's state with the above call. Update it only after the
+		 * state of master domain is updated.
+		 */
+		__genpd_dev_update_performance_state(dev, state);
+	}
+
+unlock:
+	genpd_unlock(genpd);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_genpd_update_performance_state);
+
+static int _genpd_on_update_performance_state(struct generic_pm_domain *genpd,
+					      int depth)
+{
+	int ret, prev = genpd->prev_performance_state;
+
+	if (likely(!prev))
+		return 0;
+
+	ret = _genpd_set_performance_state(genpd, prev, depth);
+	if (ret) {
+		pr_err("%s: Failed to restore performance state to %d (%d)\n",
+		       genpd->name, prev, ret);
+	} else {
+		genpd->prev_performance_state = 0;
+	}
+
+	return ret;
+}
+
+static void _genpd_off_update_performance_state(struct generic_pm_domain *genpd,
+						int depth)
+{
+	int ret, state = genpd->performance_state;
+
+	if (likely(!state))
+		return;
+
+	ret = _genpd_set_performance_state(genpd, 0, depth);
+	if (ret) {
+		pr_err("%s: Failed to set performance state to 0 (%d)\n",
+		       genpd->name, ret);
+	} else {
+		genpd->prev_performance_state = state;
+	}
+}
+
 static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
 {
 	unsigned int state_idx = genpd->state_idx;
@@ -388,6 +649,8 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
 			return ret;
 	}
 
+	_genpd_off_update_performance_state(genpd, depth);
+
 	genpd->status = GPD_STATE_POWER_OFF;
 	genpd_update_accounting(genpd);
 
@@ -437,15 +700,21 @@ static int genpd_power_on(struct generic_pm_domain *genpd, unsigned int depth)
 		}
 	}
 
-	ret = _genpd_power_on(genpd, true);
+	ret = _genpd_on_update_performance_state(genpd, depth);
 	if (ret)
 		goto err;
 
+	ret = _genpd_power_on(genpd, true);
+	if (ret)
+		goto err_power_on;
+
 	genpd->status = GPD_STATE_ACTIVE;
 	genpd_update_accounting(genpd);
 
 	return 0;
 
+ err_power_on:
+	_genpd_off_update_performance_state(genpd, depth);
  err:
 	list_for_each_entry_continue_reverse(link,
 					&genpd->slave_links,
@@ -807,6 +1076,8 @@ static void genpd_sync_power_off(struct generic_pm_domain *genpd, bool use_lock,
 	if (_genpd_power_off(genpd, false))
 		return;
 
+	_genpd_off_update_performance_state(genpd, depth);
+
 	genpd->status = GPD_STATE_POWER_OFF;
 
 	list_for_each_entry(link, &genpd->slave_links, slave_node) {
@@ -852,7 +1123,9 @@ static void genpd_sync_power_on(struct generic_pm_domain *genpd, bool use_lock,
 			genpd_unlock(link->master);
 	}
 
-	_genpd_power_on(genpd, false);
+	if (!_genpd_on_update_performance_state(genpd, depth))
+		if (_genpd_power_on(genpd, false))
+			_genpd_off_update_performance_state(genpd, depth);
 
 	genpd->status = GPD_STATE_ACTIVE;
 }
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 84f423d5633e..715cca7ac399 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -64,8 +64,13 @@ struct generic_pm_domain {
 	unsigned int device_count;	/* Number of devices */
 	unsigned int suspended_count;	/* System suspend device counter */
 	unsigned int prepared_count;	/* Suspend counter of prepared devices */
+	unsigned int performance_state;	/* Max requested performance state */
+	unsigned int prev_performance_state;	/* Performance state before power off */
 	int (*power_off)(struct generic_pm_domain *domain);
 	int (*power_on)(struct generic_pm_domain *domain);
+	int (*dev_get_performance_state)(struct device *dev, unsigned long rate);
+	int (*genpd_set_performance_state)(struct generic_pm_domain *genpd,
+					   unsigned int state);
 	struct gpd_dev_ops dev_ops;
 	s64 max_off_time_ns;	/* Maximum allowed "suspended" time. */
 	bool max_off_time_changed;
@@ -102,6 +107,9 @@ struct gpd_link {
 	struct list_head master_node;
 	struct generic_pm_domain *slave;
 	struct list_head slave_node;
+
+	/* Sub-domain's per-master domain performance state */
+	unsigned int performance_state;
 };
 
 struct gpd_timing_data {
@@ -121,6 +129,7 @@ struct generic_pm_domain_data {
 	struct pm_domain_data base;
 	struct gpd_timing_data td;
 	struct notifier_block nb;
+	unsigned int performance_state;
 	void *data;
 };
 
@@ -148,6 +157,9 @@ extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 extern int pm_genpd_init(struct generic_pm_domain *genpd,
 			 struct dev_power_governor *gov, bool is_off);
 extern int pm_genpd_remove(struct generic_pm_domain *genpd);
+extern bool dev_pm_genpd_has_performance_state(struct device *dev);
+extern int dev_pm_genpd_update_performance_state(struct device *dev,
+						 unsigned long rate);
 
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;
@@ -188,6 +200,17 @@ static inline int pm_genpd_remove(struct generic_pm_domain *genpd)
 	return -ENOTSUPP;
 }
 
+static inline bool dev_pm_genpd_has_performance_state(struct device *dev)
+{
+	return false;
+}
+
+static inline int dev_pm_genpd_update_performance_state(struct device *dev,
+							unsigned long rate)
+{
+	return -ENOTSUPP;
+}
+
 #define simple_qos_governor		(*(struct dev_power_governor *)(NULL))
 #define pm_domain_always_on_gov		(*(struct dev_power_governor *)(NULL))
 #endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V10 2/7] PM / OPP: Support updating performance state of device's power domains
  2017-09-19 22:32 [PATCH V10 0/3] PM / Domains: Performance state support Viresh Kumar
  2017-09-19 22:32 ` [PATCH V10 1/7] PM / Domains: Add support to select performance-state of domains Viresh Kumar
@ 2017-09-19 22:32 ` Viresh Kumar
  2017-09-19 22:32 ` [PATCH V10 3/7] PM / Domains: Catch missing genpd_set_performance_state() in masters Viresh Kumar
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-09-19 22:32 UTC (permalink / raw)
  To: Rafael Wysocki, ulf.hansson, Kevin Hilman
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Stephen Boyd,
	Nishanth Menon, robh+dt, lina.iyer, rnayak, sudeep.holla,
	linux-kernel, Len Brown, Pavel Machek, Andy Gross, David Brown

The genpd framework now provides an API to request device's power
domain to update its performance state based on a particular target
frequency for the device.

Use that interface from the OPP core for devices whose power domains
support performance states.

Note that the current implementation is restricted to the case where the
device doesn't have separate regulators for itself. We shouldn't
over engineer the code before we have real use case for them. We can
always come back and add more code to support such cases later on.

Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/base/power/opp/core.c | 48 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/base/power/opp/opp.h  |  2 ++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c
index a8cc14fd8ae4..4360b4efcd4c 100644
--- a/drivers/base/power/opp/core.c
+++ b/drivers/base/power/opp/core.c
@@ -19,6 +19,7 @@
 #include <linux/slab.h>
 #include <linux/device.h>
 #include <linux/export.h>
+#include <linux/pm_domain.h>
 #include <linux/regulator/consumer.h>
 
 #include "opp.h"
@@ -535,6 +536,42 @@ _generic_set_opp_clk_only(struct device *dev, struct clk *clk,
 	return ret;
 }
 
+static inline int
+_generic_set_opp_domain(struct device *dev, struct clk *clk,
+			unsigned long old_freq, unsigned long freq)
+{
+	int ret;
+
+	/* Scaling up? Scale domain performance state before frequency */
+	if (freq > old_freq) {
+		ret = dev_pm_genpd_update_performance_state(dev, freq);
+		if (ret)
+			return ret;
+	}
+
+	ret = _generic_set_opp_clk_only(dev, clk, old_freq, freq);
+	if (ret)
+		goto restore_domain_state;
+
+	/* Scaling down? Scale domain performance state after frequency */
+	if (freq < old_freq) {
+		ret = dev_pm_genpd_update_performance_state(dev, freq);
+		if (ret)
+			goto restore_freq;
+	}
+
+	return 0;
+
+restore_freq:
+	if (_generic_set_opp_clk_only(dev, clk, freq, old_freq))
+		dev_err(dev, "%s: failed to restore old-freq (%lu Hz)\n",
+			__func__, old_freq);
+restore_domain_state:
+	dev_pm_genpd_update_performance_state(dev, old_freq);
+
+	return ret;
+}
+
 static int _generic_set_opp_regulator(const struct opp_table *opp_table,
 				      struct device *dev,
 				      unsigned long old_freq,
@@ -653,7 +690,14 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 
 	/* Only frequency scaling */
 	if (!opp_table->regulators) {
-		ret = _generic_set_opp_clk_only(dev, clk, old_freq, freq);
+		/*
+		 * We don't support devices with both regulator and
+		 * domain performance-state for now.
+		 */
+		if (opp_table->genpd_performance_state)
+			ret = _generic_set_opp_domain(dev, clk, old_freq, freq);
+		else
+			ret = _generic_set_opp_clk_only(dev, clk, old_freq, freq);
 	} else if (!opp_table->set_opp) {
 		ret = _generic_set_opp_regulator(opp_table, dev, old_freq, freq,
 						 IS_ERR(old_opp) ? NULL : old_opp->supplies,
@@ -755,6 +799,8 @@ static struct opp_table *_allocate_opp_table(struct device *dev)
 				ret);
 	}
 
+	opp_table->genpd_performance_state = dev_pm_genpd_has_performance_state(dev);
+
 	BLOCKING_INIT_NOTIFIER_HEAD(&opp_table->head);
 	INIT_LIST_HEAD(&opp_table->opp_list);
 	mutex_init(&opp_table->lock);
diff --git a/drivers/base/power/opp/opp.h b/drivers/base/power/opp/opp.h
index 166eef990599..1efa253e1934 100644
--- a/drivers/base/power/opp/opp.h
+++ b/drivers/base/power/opp/opp.h
@@ -135,6 +135,7 @@ enum opp_table_access {
  * @clk: Device's clock handle
  * @regulators: Supply regulators
  * @regulator_count: Number of power supply regulators
+ * @genpd_performance_state: Device's power domain support performance state.
  * @set_opp: Platform specific set_opp callback
  * @set_opp_data: Data to be passed to set_opp callback
  * @dentry:	debugfs dentry pointer of the real device directory (not links).
@@ -170,6 +171,7 @@ struct opp_table {
 	struct clk *clk;
 	struct regulator **regulators;
 	unsigned int regulator_count;
+	bool genpd_performance_state;
 
 	int (*set_opp)(struct dev_pm_set_opp_data *data);
 	struct dev_pm_set_opp_data *set_opp_data;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V10 3/7] PM / Domains: Catch missing genpd_set_performance_state() in masters
  2017-09-19 22:32 [PATCH V10 0/3] PM / Domains: Performance state support Viresh Kumar
  2017-09-19 22:32 ` [PATCH V10 1/7] PM / Domains: Add support to select performance-state of domains Viresh Kumar
  2017-09-19 22:32 ` [PATCH V10 2/7] PM / OPP: Support updating performance state of device's power domains Viresh Kumar
@ 2017-09-19 22:32 ` Viresh Kumar
  2017-09-19 22:32 ` [PATCH V10 4/7] soc: qcom: rpmpd: Add a powerdomain driver to model cx/mx powerdomains Viresh Kumar
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-09-19 22:32 UTC (permalink / raw)
  To: Rafael Wysocki, ulf.hansson, Kevin Hilman
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Stephen Boyd,
	Nishanth Menon, robh+dt, lina.iyer, rnayak, sudeep.holla,
	linux-kernel, Len Brown, Pavel Machek, Andy Gross, David Brown

This patch catches the cases where dev_get_performance_state() callback
is implemented for a genpd, but none of its masters or their masters
(and so on) have implemented genpd_set_performance_state() callback.

The internal performance state routines don't return 0 anymore for
success, rather they return count of the domains whose performance state
is updated and the top level routine checks for that.

A zero value there would indicate that the genpd_set_performance_state()
callbacks are missing in the master hierarchy of the device.

This adds very little burden on the API and can be pretty useful.

Tested-by: Rajendra Nayak <rnayak@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

---
Note that similar checks aren't present in other APIs in genpd, like
genpd_power_on/off(). Maybe we should add some there as well?
---
 drivers/base/power/domain.c | 42 +++++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 6d05c91cf44f..7e00b817abc7 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -264,13 +264,13 @@ EXPORT_SYMBOL_GPL(dev_pm_genpd_has_performance_state);
 static int _genpd_reeval_performance_state(struct generic_pm_domain *genpd,
 					   int state, int depth);
 
-/* Returns -ve errors or 0 on success */
+/* Returns -ve errors or number of domains whose performance is set */
 static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
 					int state, int depth)
 {
 	struct generic_pm_domain *master;
 	struct gpd_link *link;
-	int prev = genpd->performance_state, ret;
+	int prev = genpd->performance_state, ret, count = 0;
 
 	/* Propagate to masters of genpd */
 	list_for_each_entry(link, &genpd->slave_links, slave_node) {
@@ -280,19 +280,23 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
 
 		link->performance_state = state;
 		ret = _genpd_reeval_performance_state(master, state, depth + 1);
-		if (ret)
+		if (ret < 0)
 			link->performance_state = prev;
 
 		genpd_unlock(master);
 
-		if (ret)
+		if (ret < 0)
 			goto err;
+
+		count += ret;
 	}
 
 	if (genpd->genpd_set_performance_state) {
 		ret = genpd->genpd_set_performance_state(genpd, state);
 		if (ret)
 			goto err;
+
+		count++;
 	}
 
 	/*
@@ -300,7 +304,7 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
 	 * with those.
 	 */
 	genpd->performance_state = state;
-	return 0;
+	return count;
 
 err:
 	/* Encountered an error, lets rollback */
@@ -310,7 +314,7 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
 
 		genpd_lock_nested(master, depth + 1);
 		link->performance_state = prev;
-		if (_genpd_reeval_performance_state(master, prev, depth + 1)) {
+		if (_genpd_reeval_performance_state(master, prev, depth + 1) < 0) {
 			pr_err("%s: Failed to roll back to %d performance state\n",
 			       master->name, prev);
 		}
@@ -345,7 +349,7 @@ static int _genpd_set_performance_state(struct generic_pm_domain *genpd,
  * - The locks are always taken in bottom->up order, i.e. subdomain first,
  *   followed by its masters.
  *
- * Returns -ve errors or 0 on success.
+ * Returns -ve errors or number of domains whose performance is set.
  */
 static int _genpd_reeval_performance_state(struct generic_pm_domain *genpd,
 					   int state, int depth)
@@ -354,9 +358,14 @@ static int _genpd_reeval_performance_state(struct generic_pm_domain *genpd,
 	struct pm_domain_data *pdd;
 	struct gpd_link *link;
 
-	/* New requested state is same as Max requested state */
-	if (state == genpd->performance_state)
-		return 0;
+	if (state == genpd->performance_state) {
+		/*
+		 * New requested state is same as Max requested state, return 1
+		 * to distinguish from the case where none of the masters have
+		 * set their genpd_set_performance_state() callback.
+		 */
+		return 1;
+	}
 
 	/* New requested state is higher than Max requested state */
 	if (state > genpd->performance_state)
@@ -444,7 +453,7 @@ int dev_pm_genpd_update_performance_state(struct device *dev,
 	}
 
 	ret = _genpd_reeval_performance_state(genpd, state, 0);
-	if (!ret) {
+	if (ret > 0) {
 		/*
 		 * Since we are passing "state" to
 		 * _genpd_reeval_performance_state() as well, we don't need to
@@ -453,6 +462,13 @@ int dev_pm_genpd_update_performance_state(struct device *dev,
 		 * state of master domain is updated.
 		 */
 		__genpd_dev_update_performance_state(dev, state);
+		ret = 0;
+	} else if (ret < 0) {
+		dev_err(dev, "Couldn't update performance state (%d)\n", ret);
+	} else {
+		WARN(1, "%s: None of %s and its masters have provided genpd_set_performance_state()\n",
+		     __func__, genpd->name);
+		ret = -ENODEV;
 	}
 
 unlock:
@@ -471,7 +487,7 @@ static int _genpd_on_update_performance_state(struct generic_pm_domain *genpd,
 		return 0;
 
 	ret = _genpd_set_performance_state(genpd, prev, depth);
-	if (ret) {
+	if (ret < 0) {
 		pr_err("%s: Failed to restore performance state to %d (%d)\n",
 		       genpd->name, prev, ret);
 	} else {
@@ -490,7 +506,7 @@ static void _genpd_off_update_performance_state(struct generic_pm_domain *genpd,
 		return;
 
 	ret = _genpd_set_performance_state(genpd, 0, depth);
-	if (ret) {
+	if (ret < 0) {
 		pr_err("%s: Failed to set performance state to 0 (%d)\n",
 		       genpd->name, ret);
 	} else {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V10 4/7] soc: qcom: rpmpd: Add a powerdomain driver to model cx/mx powerdomains
  2017-09-19 22:32 [PATCH V10 0/3] PM / Domains: Performance state support Viresh Kumar
                   ` (2 preceding siblings ...)
  2017-09-19 22:32 ` [PATCH V10 3/7] PM / Domains: Catch missing genpd_set_performance_state() in masters Viresh Kumar
@ 2017-09-19 22:32 ` Viresh Kumar
  2017-09-19 22:32 ` [PATCH V10 5/7] soc: qcom: rpmpd: Add support for get/set performance state Viresh Kumar
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-09-19 22:32 UTC (permalink / raw)
  To: Rafael Wysocki, ulf.hansson, Kevin Hilman
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Stephen Boyd,
	Nishanth Menon, robh+dt, lina.iyer, rnayak, sudeep.holla,
	linux-kernel, Len Brown, Pavel Machek, Andy Gross, David Brown

From: Rajendra Nayak <rnayak@codeaurora.org>

THIS IS TEST CODE, SHOULDN'T BE MERGED.

The cx/mx powerdomains just pass the performance state set by the
consumers to the RPM (Remote Power manager) which then takes care
of setting the appropriate voltage on the corresponding rails to
meet the performance needs.

Add data for all powerdomains on msm8996.

Not-signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Not-signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 .../devicetree/bindings/power/qcom,rpmpd.txt       |  10 +
 arch/arm64/boot/dts/qcom/msm8996.dtsi              |   5 +
 drivers/soc/qcom/Kconfig                           |   9 +
 drivers/soc/qcom/Makefile                          |   1 +
 drivers/soc/qcom/rpmpd.c                           | 307 +++++++++++++++++++++
 5 files changed, 332 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/power/qcom,rpmpd.txt
 create mode 100644 drivers/soc/qcom/rpmpd.c

diff --git a/Documentation/devicetree/bindings/power/qcom,rpmpd.txt b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
new file mode 100644
index 000000000000..8b48ce57a563
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/qcom,rpmpd.txt
@@ -0,0 +1,10 @@
+Qualcomm RPM Powerdomains
+
+* For RPM powerdomains, we communicate a performance state to RPM
+which then translates it into a corresponding voltage on a rail
+
+Required Properties:
+ - compatible: Should be one of the following
+	* qcom,rpmpd-msm8996: RPM Powerdomain for the msm8996 family of SoC
+ - power-domain-cells: number of cells in power domain specifier
+	must be 1.
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 887b61c872dd..0be1db559d61 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -496,6 +496,11 @@
 			status = "disabled";
 		};
 
+		rpmpd: qcom,rpmpd {
+			compatible = "qcom,rpmpd-msm8996", "qcom,rpmpd";
+			#power-domain-cells = <1>;
+		};
+
 		sdhc2: sdhci@74a4900 {
 			 status = "disabled";
 			 compatible = "qcom,sdhci-msm-v4";
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index b00bccddcd3b..38ff424bdebe 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -35,6 +35,15 @@ config QCOM_PM
 	  modes. It interface with various system drivers to put the cores in
 	  low power modes.
 
+config QCOM_RPMPD
+	tristate "Qualcomm RPM Powerdomain driver"
+	depends on MFD_QCOM_RPM && QCOM_SMD_RPM
+	help
+	  QCOM RPM powerdomain driver to support powerdomain with
+	  performance states. The driver communicates a performance state
+	  value to RPM which then translates it into corresponding voltage
+	  for the voltage rail.
+
 config QCOM_SMEM
 	tristate "Qualcomm Shared Memory Manager (SMEM)"
 	depends on ARCH_QCOM
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index f151de41eb93..3fa9af1e2c93 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
 obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
 obj-$(CONFIG_QCOM_SMSM)	+= smsm.o
 obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
+obj-$(CONFIG_QCOM_RPMPD) += rpmpd.o
diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
new file mode 100644
index 000000000000..d34d9c363815
--- /dev/null
+++ b/drivers/soc/qcom/rpmpd.c
@@ -0,0 +1,307 @@
+/*
+ * Copyright (c) 2017, The Linux Foundation. All rights reserved.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pm_domain.h>
+#include <linux/mfd/qcom_rpm.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/soc/qcom/smd-rpm.h>
+
+#include <dt-bindings/mfd/qcom-rpm.h>
+
+#define domain_to_rpmpd(domain) container_of(domain, struct rpmpd, pd)
+
+/* Resource types */
+#define RPMPD_SMPA 0x61706d73
+#define RPMPD_LDOA 0x616f646c
+
+/* Operation Keys */
+#define KEY_CORNER		0x6e726f63 /* corn */
+#define KEY_ENABLE		0x6e657773 /* swen */
+#define KEY_FLOOR_CORNER	0x636676   /* vfc */
+
+#define DEFINE_RPMPD_CORN_SMPA(_platform, _name, _active, r_id)		\
+	static struct rpmpd _platform##_##_active;			\
+	static struct rpmpd _platform##_##_name = {			\
+		.pd = {	.name = #_name,	},				\
+		.peer = &_platform##_##_active,				\
+		.res_type = RPMPD_SMPA,					\
+		.res_id = r_id,						\
+		.key = KEY_CORNER,					\
+	};								\
+	static struct rpmpd _platform##_##_active = {			\
+		.pd = { .name = #_active, },				\
+		.peer = &_platform##_##_name,				\
+		.active_only = true,					\
+		.res_type = RPMPD_SMPA,					\
+		.res_id = r_id,						\
+		.key = KEY_CORNER,					\
+	}
+
+#define DEFINE_RPMPD_CORN_LDOA(_platform, _name, r_id)			\
+	static struct rpmpd _platform##_##_name = {			\
+		.pd = { .name = #_name, },				\
+		.res_type = RPMPD_LDOA,					\
+		.res_id = r_id,						\
+		.key = KEY_CORNER,					\
+	}
+
+#define DEFINE_RPMPD_VFC(_platform, _name, r_id, r_type)		\
+	static struct rpmpd _platform##_##_name = {			\
+		.pd = { .name = #_name, },				\
+		.res_type = r_type,					\
+		.res_id = r_id,						\
+		.key = KEY_FLOOR_CORNER,				\
+	}
+
+#define DEFINE_RPMPD_VFC_SMPA(_platform, _name, r_id)			\
+	DEFINE_RPMPD_VFC(_platform, _name, r_id, RPMPD_SMPA)
+
+#define DEFINE_RPMPD_VFC_LDOA(_platform, _name, r_id)			\
+	DEFINE_RPMPD_VFC(_platform, _name, r_id, RPMPD_LDOA)
+
+struct rpmpd_req {
+	__le32 key;
+	__le32 nbytes;
+	__le32 value;
+};
+
+struct rpmpd {
+	struct generic_pm_domain pd;
+	struct rpmpd *peer;
+	const bool active_only;
+	unsigned long corner;
+	bool enabled;
+	const char *res_name;
+	const int res_type;
+	const int res_id;
+	struct qcom_smd_rpm *rpm;
+	__le32 key;
+};
+
+struct rpmpd_desc {
+	struct rpmpd **rpmpds;
+	size_t num_pds;
+};
+
+static DEFINE_MUTEX(rpmpd_lock);
+
+/* msm8996 RPM powerdomains */
+DEFINE_RPMPD_CORN_SMPA(msm8996, vddcx, vddcx_ao, 1);
+DEFINE_RPMPD_CORN_SMPA(msm8996, vddmx, vddmx_ao, 2);
+DEFINE_RPMPD_CORN_LDOA(msm8996, vddsscx, 26);
+
+DEFINE_RPMPD_VFC_SMPA(msm8996, vddcx_vfc, 1);
+DEFINE_RPMPD_VFC_LDOA(msm8996, vddsscx_vfc, 26);
+
+static struct rpmpd *msm8996_rpmpds[] = {
+	[0] = &msm8996_vddcx,
+	[1] = &msm8996_vddcx_ao,
+	[2] = &msm8996_vddcx_vfc,
+	[3] = &msm8996_vddmx,
+	[4] = &msm8996_vddmx_ao,
+	[5] = &msm8996_vddsscx,
+	[6] = &msm8996_vddsscx_vfc,
+};
+
+static const struct rpmpd_desc msm8996_desc = {
+	.rpmpds = msm8996_rpmpds,
+	.num_pds = ARRAY_SIZE(msm8996_rpmpds),
+};
+
+static const struct of_device_id rpmpd_match_table[] = {
+	{ .compatible = "qcom,rpmpd-msm8996", .data = &msm8996_desc },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, rpmpd_match_table);
+
+static int rpmpd_send_enable(struct rpmpd *pd, bool enable)
+{
+	struct rpmpd_req req = {
+		.key = KEY_ENABLE,
+		.nbytes = cpu_to_le32(sizeof(u32)),
+		.value = cpu_to_le32(enable),
+	};
+
+	return qcom_rpm_smd_write(pd->rpm, QCOM_RPM_ACTIVE_STATE, pd->res_type,
+				  pd->res_id, &req, sizeof(req));
+}
+
+static int rpmpd_send_corner(struct rpmpd *pd, int state, unsigned int corner)
+{
+	struct rpmpd_req req = {
+		.key = pd->key,
+		.nbytes = cpu_to_le32(sizeof(u32)),
+		.value = cpu_to_le32(corner),
+	};
+
+	return qcom_rpm_smd_write(pd->rpm, state, pd->res_type, pd->res_id,
+				  &req, sizeof(req));
+};
+
+static void to_active_sleep(struct rpmpd *pd, unsigned long corner,
+			    unsigned long *active, unsigned long *sleep)
+{
+	*active = corner;
+
+	if (pd->active_only)
+		*sleep = 0;
+	else
+		*sleep = *active;
+}
+
+static int rpmpd_aggregate_corner(struct rpmpd *pd)
+{
+	int ret;
+	struct rpmpd *peer = pd->peer;
+	unsigned long active_corner, sleep_corner;
+	unsigned long this_corner = 0, this_sleep_corner = 0;
+	unsigned long peer_corner = 0, peer_sleep_corner = 0;
+
+	to_active_sleep(pd, pd->corner, &this_corner, &this_sleep_corner);
+
+	if (peer && peer->enabled)
+		to_active_sleep(peer, peer->corner, &peer_corner,
+				&peer_sleep_corner);
+
+	active_corner = max(this_corner, peer_corner);
+
+	ret = rpmpd_send_corner(pd, QCOM_RPM_ACTIVE_STATE, active_corner);
+	if (ret)
+		return ret;
+
+	sleep_corner = max(this_sleep_corner, peer_sleep_corner);
+
+	return rpmpd_send_corner(pd, QCOM_RPM_SLEEP_STATE, sleep_corner);
+}
+
+static int rpmpd_power_on(struct generic_pm_domain *domain)
+{
+	int ret;
+	struct rpmpd *pd = domain_to_rpmpd(domain);
+
+	mutex_lock(&rpmpd_lock);
+
+	ret = rpmpd_send_enable(pd, true);
+	if (ret)
+		goto out;
+
+	pd->enabled = true;
+
+	if (pd->corner)
+		ret = rpmpd_aggregate_corner(pd);
+
+out:
+	mutex_unlock(&rpmpd_lock);
+
+	return ret;
+}
+
+static int rpmpd_power_off(struct generic_pm_domain *domain)
+{
+	int ret;
+	struct rpmpd *pd = domain_to_rpmpd(domain);
+
+	mutex_lock(&rpmpd_lock);
+
+	ret = rpmpd_send_enable(pd, false);
+	if (!ret)
+		pd->enabled = false;
+
+	mutex_unlock(&rpmpd_lock);
+
+	return ret;
+}
+
+static int rpmpd_probe(struct platform_device *pdev)
+{
+	int i;
+	size_t num;
+	struct genpd_onecell_data *data;
+	struct qcom_smd_rpm *rpm;
+	struct rpmpd **rpmpds;
+	const struct rpmpd_desc *desc;
+
+	rpm = dev_get_drvdata(pdev->dev.parent);
+	if (!rpm) {
+		dev_err(&pdev->dev, "Unable to retrieve handle to RPM\n");
+		return -ENODEV;
+	}
+
+	desc = of_device_get_match_data(&pdev->dev);
+	if (!desc)
+		return -EINVAL;
+
+	rpmpds = desc->rpmpds;
+	num = desc->num_pds;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->domains = devm_kcalloc(&pdev->dev, num, sizeof(*data->domains),
+				     GFP_KERNEL);
+	data->num_domains = num;
+
+	for (i = 0; i < num; i++) {
+		if (!rpmpds[i])
+			continue;
+
+		rpmpds[i]->rpm = rpm;
+		rpmpds[i]->pd.power_off = rpmpd_power_off;
+		rpmpds[i]->pd.power_on = rpmpd_power_on;
+		pm_genpd_init(&rpmpds[i]->pd, NULL, true);
+
+		data->domains[i] = &rpmpds[i]->pd;
+	}
+
+	return of_genpd_add_provider_onecell(pdev->dev.of_node, data);
+}
+
+static int rpmpd_remove(struct platform_device *pdev)
+{
+	of_genpd_del_provider(pdev->dev.of_node);
+	return 0;
+}
+
+static struct platform_driver rpmpd_driver = {
+	.driver = {
+		.name = "qcom-rpmpd",
+		.of_match_table = rpmpd_match_table,
+	},
+	.probe = rpmpd_probe,
+	.remove = rpmpd_remove,
+};
+
+static int __init rpmpd_init(void)
+{
+	return platform_driver_register(&rpmpd_driver);
+}
+core_initcall(rpmpd_init);
+
+static void __exit rpmpd_exit(void)
+{
+	platform_driver_unregister(&rpmpd_driver);
+}
+module_exit(rpmpd_exit);
+
+MODULE_DESCRIPTION("Qualcomm RPM Power Domain Driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:qcom-rpmpd");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V10 5/7] soc: qcom: rpmpd: Add support for get/set performance state
  2017-09-19 22:32 [PATCH V10 0/3] PM / Domains: Performance state support Viresh Kumar
                   ` (3 preceding siblings ...)
  2017-09-19 22:32 ` [PATCH V10 4/7] soc: qcom: rpmpd: Add a powerdomain driver to model cx/mx powerdomains Viresh Kumar
@ 2017-09-19 22:32 ` Viresh Kumar
  2017-09-19 22:32 ` [PATCH V10 6/7] mmc: sdhci-msm: Adapt the driver to use OPPs to set clocks/performance state Viresh Kumar
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-09-19 22:32 UTC (permalink / raw)
  To: Rafael Wysocki, ulf.hansson, Kevin Hilman
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Stephen Boyd,
	Nishanth Menon, robh+dt, lina.iyer, rnayak, sudeep.holla,
	linux-kernel, Len Brown, Pavel Machek, Andy Gross, David Brown

From: Rajendra Nayak <rnayak@codeaurora.org>

THIS IS TEST CODE, SHOULDN'T BE MERGED.

With genpd now expecting powerdomain drivers supporting performance
state to support get/set performance state callbacks, add support for it
in the rpmpd driver.

Not-signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Not-signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/soc/qcom/rpmpd.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 100 insertions(+)

diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
index d34d9c363815..4e4f5cda9ce2 100644
--- a/drivers/soc/qcom/rpmpd.c
+++ b/drivers/soc/qcom/rpmpd.c
@@ -101,6 +101,20 @@ struct rpmpd_desc {
 	size_t num_pds;
 };
 
+enum rpmpd_levels {
+	NONE,
+	LOWER,          /* SVS2 */
+	LOW,            /* SVS */
+	NOMINAL,        /* NOMINAL */
+	HIGH,           /* Turbo */
+	MAX_LEVEL,
+};
+
+struct rpmpd_freq_map {
+	struct rpmpd *pd;
+	unsigned long freq[MAX_LEVEL];
+};
+
 static DEFINE_MUTEX(rpmpd_lock);
 
 /* msm8996 RPM powerdomains */
@@ -126,6 +140,47 @@ static const struct rpmpd_desc msm8996_desc = {
 	.num_pds = ARRAY_SIZE(msm8996_rpmpds),
 };
 
+enum msm8996_devices {
+	SDHCI,
+	UFS,
+	PCIE,
+	USB3,
+};
+
+static struct rpmpd_freq_map msm8996_rpmpd_freq_map[] = {
+	[SDHCI] = {
+		.pd = &msm8996_vddcx,
+		.freq[LOWER] = 19200000,
+		.freq[LOW] = 200000000,
+		.freq[NOMINAL] = 400000000,
+	},
+	[UFS] = {
+		.pd = &msm8996_vddcx,
+		.freq[LOWER] = 19200000,
+		.freq[LOW] = 100000000,
+		.freq[NOMINAL] = 200000000,
+		.freq[HIGH] = 240000000,
+	},
+	[PCIE] = {
+		.pd = &msm8996_vddcx,
+		.freq[LOWER] = 1011000,
+	},
+	[USB3] = {
+		.pd = &msm8996_vddcx,
+		.freq[LOWER] = 60000000,
+		.freq[LOW] = 120000000,
+		.freq[NOMINAL] = 150000000,
+	},
+};
+
+static const struct of_device_id rpmpd_performance_table[] = {
+	{ .compatible = "qcom,sdhci-msm-v4", .data = (void *)SDHCI },
+	{ .compatible = "qcom,ufshc", .data = (void *)UFS },
+	{ .compatible = "qcom,pcie-msm8996", .data = (void *)PCIE },
+	{ .compatible = "qcom,dwc3", .data = (void *)USB3 },
+	{ }
+};
+
 static const struct of_device_id rpmpd_match_table[] = {
 	{ .compatible = "qcom,rpmpd-msm8996", .data = &msm8996_desc },
 	{ }
@@ -230,6 +285,49 @@ static int rpmpd_power_off(struct generic_pm_domain *domain)
 	return ret;
 }
 
+static int rpmpd_set_performance(struct generic_pm_domain *domain,
+			     unsigned int state)
+{
+	int ret = 0;
+	struct rpmpd *pd = domain_to_rpmpd(domain);
+
+	mutex_lock(&rpmpd_lock);
+
+	pd->corner = state;
+
+	if (!pd->enabled && (pd->key != KEY_FLOOR_CORNER))
+		goto out;
+
+	ret = rpmpd_aggregate_corner(pd);
+
+out:
+	mutex_unlock(&rpmpd_lock);
+
+	return ret;
+}
+
+
+static int rpmpd_get_performance(struct device *dev, unsigned long rate)
+{
+	int i;
+	unsigned long index;
+	const struct of_device_id *id;
+
+	if (!rate)
+		return 0;
+
+	id = of_match_device(rpmpd_performance_table, dev);
+	if (!id)
+		return -EINVAL;
+
+	index = (unsigned long)id->data;
+	for (i = 0; i < MAX_LEVEL; i++)
+		if (msm8996_rpmpd_freq_map[index].freq[i] >= rate)
+			return i;
+
+	return MAX_LEVEL;
+}
+
 static int rpmpd_probe(struct platform_device *pdev)
 {
 	int i;
@@ -267,6 +365,8 @@ static int rpmpd_probe(struct platform_device *pdev)
 		rpmpds[i]->rpm = rpm;
 		rpmpds[i]->pd.power_off = rpmpd_power_off;
 		rpmpds[i]->pd.power_on = rpmpd_power_on;
+		rpmpds[i]->pd.genpd_set_performance_state = rpmpd_set_performance;
+		rpmpds[i]->pd.dev_get_performance_state = rpmpd_get_performance;
 		pm_genpd_init(&rpmpds[i]->pd, NULL, true);
 
 		data->domains[i] = &rpmpds[i]->pd;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V10 6/7] mmc: sdhci-msm: Adapt the driver to use OPPs to set clocks/performance state
  2017-09-19 22:32 [PATCH V10 0/3] PM / Domains: Performance state support Viresh Kumar
                   ` (4 preceding siblings ...)
  2017-09-19 22:32 ` [PATCH V10 5/7] soc: qcom: rpmpd: Add support for get/set performance state Viresh Kumar
@ 2017-09-19 22:32 ` Viresh Kumar
  2017-09-19 22:32 ` [PATCH V10 7/7] remoteproc: qcom: q6v5: Vote for proxy powerdomain performance state Viresh Kumar
  2017-10-03  7:52 ` [PATCH V10 0/3] PM / Domains: Performance state support Ulf Hansson
  7 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-09-19 22:32 UTC (permalink / raw)
  To: Rafael Wysocki, ulf.hansson, Kevin Hilman
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Stephen Boyd,
	Nishanth Menon, robh+dt, lina.iyer, rnayak, sudeep.holla,
	linux-kernel, Len Brown, Pavel Machek, Andy Gross, David Brown

From: Rajendra Nayak <rnayak@codeaurora.org>

THIS IS TEST CODE, SHOULDN'T BE MERGED.

SDHCI driver needs to set a performance state along with scaling its
clocks. Modify the driver to use the newly introducded powerdomain
performance state based OPPs to scale clocks as well as set an
appropriate powerdomain performance state.

The patch also adds OPPs for sdhci device on msm8996.

The changes have to be validated by populating similar OPP tables
on all other devices which use the sdhci driver. This is for now
validated only on msm8996 and with missing OPP tables for other
devices is known to break those platforms.

Not-signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Not-signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 34 ++++++++++++++++++++++++++++++
 drivers/clk/qcom/gcc-msm8996.c        |  8 +++----
 drivers/mmc/host/sdhci-msm.c          | 39 ++++++++++++++++++++++++++---------
 3 files changed, 67 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 0be1db559d61..71183c009c58 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -515,8 +515,42 @@
 			 <&gcc GCC_SDCC2_APPS_CLK>,
 			 <&xo_board>;
 			 bus-width = <4>;
+			power-domains = <&rpmpd 0>;
+			operating-points-v2 = <&sdhc_opp_table>;
 		 };
 
+		sdhc_opp_table: opp_table {
+			compatible = "operating-points-v2";
+
+			opp@400000 {
+				opp-hz = /bits/ 64 <400000>;
+			};
+
+			opp@20000000 {
+				opp-hz = /bits/ 64 <20000000>;
+			};
+
+			opp@25000000 {
+				opp-hz = /bits/ 64 <25000000>;
+			};
+
+			opp@50000000 {
+				opp-hz = /bits/ 64 <50000000>;
+			};
+
+			opp@96000000 {
+				opp-hz = /bits/ 64 <96000000>;
+			};
+
+			opp@192000000 {
+				opp-hz = /bits/ 64 <192000000>;
+			};
+
+			opp@384000000 {
+				opp-hz = /bits/ 64 <384000000>;
+			};
+		};
+
 		msmgpio: pinctrl@1010000 {
 			compatible = "qcom,msm8996-pinctrl";
 			reg = <0x01010000 0x300000>;
diff --git a/drivers/clk/qcom/gcc-msm8996.c b/drivers/clk/qcom/gcc-msm8996.c
index 7ddec886fcd3..38034692f5aa 100644
--- a/drivers/clk/qcom/gcc-msm8996.c
+++ b/drivers/clk/qcom/gcc-msm8996.c
@@ -460,7 +460,7 @@ static struct clk_rcg2 sdcc1_apps_clk_src = {
 		.name = "sdcc1_apps_clk_src",
 		.parent_names = gcc_xo_gpll0_gpll4_gpll0_early_div,
 		.num_parents = 4,
-		.ops = &clk_rcg2_floor_ops,
+		.ops = &clk_rcg2_ops,
 	},
 };
 
@@ -505,7 +505,7 @@ static struct clk_rcg2 sdcc2_apps_clk_src = {
 		.name = "sdcc2_apps_clk_src",
 		.parent_names = gcc_xo_gpll0_gpll4,
 		.num_parents = 3,
-		.ops = &clk_rcg2_floor_ops,
+		.ops = &clk_rcg2_ops,
 	},
 };
 
@@ -519,7 +519,7 @@ static struct clk_rcg2 sdcc3_apps_clk_src = {
 		.name = "sdcc3_apps_clk_src",
 		.parent_names = gcc_xo_gpll0_gpll4,
 		.num_parents = 3,
-		.ops = &clk_rcg2_floor_ops,
+		.ops = &clk_rcg2_ops,
 	},
 };
 
@@ -543,7 +543,7 @@ static struct clk_rcg2 sdcc4_apps_clk_src = {
 		.name = "sdcc4_apps_clk_src",
 		.parent_names = gcc_xo_gpll0,
 		.num_parents = 2,
-		.ops = &clk_rcg2_floor_ops,
+		.ops = &clk_rcg2_ops,
 	},
 };
 
diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index fc73e56eb1e2..cbc5a12af772 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -21,6 +21,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
 #include <linux/iopoll.h>
+#include <linux/pm_opp.h>
 
 #include "sdhci-pltfm.h"
 
@@ -131,6 +132,7 @@ struct sdhci_msm_host {
 	struct clk *pclk;	/* SDHC peripheral bus clock */
 	struct clk *bus_clk;	/* SDHC bus voter clock */
 	struct clk *xo_clk;	/* TCXO clk needed for FLL feature of cm_dll*/
+	struct opp_table *opp_table;
 	unsigned long clk_rate;
 	struct mmc_host *mmc;
 	bool use_14lpp_dll_reset;
@@ -140,7 +142,7 @@ struct sdhci_msm_host {
 	bool use_cdclp533;
 };
 
-static unsigned int msm_get_clock_rate_for_bus_mode(struct sdhci_host *host,
+static long unsigned int msm_get_clock_rate_for_bus_mode(struct sdhci_host *host,
 						    unsigned int clock)
 {
 	struct mmc_ios ios = host->mmc->ios;
@@ -165,16 +167,22 @@ static void msm_set_clock_rate_for_bus_mode(struct sdhci_host *host,
 	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
 	struct mmc_ios curr_ios = host->mmc->ios;
 	int rc;
+	struct device *dev = &msm_host->pdev->dev;
+	struct dev_pm_opp *opp;
+	long unsigned int freq;
+
+	freq = msm_get_clock_rate_for_bus_mode(host, clock);
+	opp = dev_pm_opp_find_freq_floor(dev, &freq);
+	if (IS_ERR(opp))
+		pr_err("%s: failed to find OPP for %u at timing %d\n",
+				mmc_hostname(host->mmc), clock, curr_ios.timing);
+
+	rc = dev_pm_opp_set_rate(dev, freq);
+	if (rc)
+		pr_err("%s: error in setting opp\n", __func__);
+
+	msm_host->clk_rate = freq;
 
-	clock = msm_get_clock_rate_for_bus_mode(host, clock);
-	rc = clk_set_rate(msm_host->clk, clock);
-	if (rc) {
-		pr_err("%s: Failed to set clock at rate %u at timing %d\n",
-		       mmc_hostname(host->mmc), clock,
-		       curr_ios.timing);
-		return;
-	}
-	msm_host->clk_rate = clock;
 	pr_debug("%s: Setting clock at rate %lu at timing %d\n",
 		 mmc_hostname(host->mmc), clk_get_rate(msm_host->clk),
 		 curr_ios.timing);
@@ -1268,6 +1276,13 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		goto clk_disable;
 	}
 
+	/* Set up the OPP table */
+	msm_host->opp_table = dev_pm_opp_set_clkname(&pdev->dev, "core");
+
+	ret = dev_pm_opp_of_add_table(&pdev->dev);
+	if (ret)
+		dev_warn(&pdev->dev, "%s: No OPP table specified\n", __func__);
+
 	pm_runtime_get_noresume(&pdev->dev);
 	pm_runtime_set_active(&pdev->dev);
 	pm_runtime_enable(&pdev->dev);
@@ -1289,6 +1304,8 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_set_suspended(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
+	dev_pm_opp_of_remove_table(&pdev->dev);
+	dev_pm_opp_put_clkname(msm_host->opp_table);
 clk_disable:
 	clk_disable_unprepare(msm_host->clk);
 pclk_disable:
@@ -1314,6 +1331,8 @@ static int sdhci_msm_remove(struct platform_device *pdev)
 	pm_runtime_get_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 	pm_runtime_put_noidle(&pdev->dev);
+	dev_pm_opp_of_remove_table(&pdev->dev);
+	dev_pm_opp_put_clkname(msm_host->opp_table);
 
 	clk_disable_unprepare(msm_host->clk);
 	clk_disable_unprepare(msm_host->pclk);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH V10 7/7] remoteproc: qcom: q6v5: Vote for proxy powerdomain performance state
  2017-09-19 22:32 [PATCH V10 0/3] PM / Domains: Performance state support Viresh Kumar
                   ` (5 preceding siblings ...)
  2017-09-19 22:32 ` [PATCH V10 6/7] mmc: sdhci-msm: Adapt the driver to use OPPs to set clocks/performance state Viresh Kumar
@ 2017-09-19 22:32 ` Viresh Kumar
  2017-10-03  7:52 ` [PATCH V10 0/3] PM / Domains: Performance state support Ulf Hansson
  7 siblings, 0 replies; 11+ messages in thread
From: Viresh Kumar @ 2017-09-19 22:32 UTC (permalink / raw)
  To: Rafael Wysocki, ulf.hansson, Kevin Hilman
  Cc: Viresh Kumar, linux-pm, Vincent Guittot, Stephen Boyd,
	Nishanth Menon, robh+dt, lina.iyer, rnayak, sudeep.holla,
	linux-kernel, Len Brown, Pavel Machek, Andy Gross, David Brown

From: Rajendra Nayak <rnayak@codeaurora.org>

THIS IS TEST CODE, SHOULDN'T BE MERGED.

This patch just demonstrates the usage of pm_genpd_update_performance_state()
api in cases where users need to set performance state of a powerdomain without
having to do it via the OPP framework.

q6v5 remoteproc driver needs to proxy vote for performance states of multiple
powerdomains (but we currently only demonstate how it can be done for
one powerdomain, as there is no way to associate multiple powerdomains
to a device at this time) while it loads the firmware, and then releases
the vote, once the firmware is up and can vote for itself.

This is not a functional patch since rpmpd driver only supports msm8996
and there is no msm8996 support in the q6v5 remoteproc driver at this
point in mainline.

This patch is not tested as well.

Not-signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
Not-signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/remoteproc/qcom_q6v5_pil.c | 20 +++++++++++++-------
 drivers/soc/qcom/rpmpd.c           |  5 +++++
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pil.c b/drivers/remoteproc/qcom_q6v5_pil.c
index 2d3d5ac92c06..dcb865b13c5c 100644
--- a/drivers/remoteproc/qcom_q6v5_pil.c
+++ b/drivers/remoteproc/qcom_q6v5_pil.c
@@ -18,6 +18,7 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
+#include <linux/pm_domain.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
@@ -152,6 +153,8 @@ struct q6v5 {
 	void *mpss_region;
 	size_t mpss_size;
 
+	bool has_perf_state;
+
 	struct qcom_rproc_subdev smd_subdev;
 	struct qcom_rproc_ssr ssr_subdev;
 };
@@ -604,11 +607,12 @@ static int q6v5_start(struct rproc *rproc)
 	struct q6v5 *qproc = (struct q6v5 *)rproc->priv;
 	int ret;
 
-	ret = q6v5_regulator_enable(qproc, qproc->proxy_regs,
-				    qproc->proxy_reg_count);
-	if (ret) {
-		dev_err(qproc->dev, "failed to enable proxy supplies\n");
-		return ret;
+	if (qproc->has_perf_state) {
+		ret = dev_pm_genpd_update_performance_state(qproc->dev, INT_MAX);
+		if (ret) {
+			dev_err(qproc->dev, "Failed to set performance state.\n");
+			return ret;
+		}
 	}
 
 	ret = q6v5_clk_enable(qproc->dev, qproc->proxy_clks,
@@ -672,8 +676,8 @@ static int q6v5_start(struct rproc *rproc)
 
 	q6v5_clk_disable(qproc->dev, qproc->proxy_clks,
 			 qproc->proxy_clk_count);
-	q6v5_regulator_disable(qproc, qproc->proxy_regs,
-			       qproc->proxy_reg_count);
+	if (qproc->has_perf_state)
+		dev_pm_genpd_update_performance_state(qproc->dev, 0);
 
 	return 0;
 
@@ -1046,6 +1050,8 @@ static int q6v5_probe(struct platform_device *pdev)
 	if (ret)
 		goto free_rproc;
 
+	qproc->has_perf_state = pm_genpd_has_performance_state(&qproc->dev);
+
 	return 0;
 
 free_rproc:
diff --git a/drivers/soc/qcom/rpmpd.c b/drivers/soc/qcom/rpmpd.c
index 4e4f5cda9ce2..c1733da8a7c8 100644
--- a/drivers/soc/qcom/rpmpd.c
+++ b/drivers/soc/qcom/rpmpd.c
@@ -145,6 +145,7 @@ enum msm8996_devices {
 	UFS,
 	PCIE,
 	USB3,
+	Q6V5_PIL,
 };
 
 static struct rpmpd_freq_map msm8996_rpmpd_freq_map[] = {
@@ -171,6 +172,10 @@ static struct rpmpd_freq_map msm8996_rpmpd_freq_map[] = {
 		.freq[LOW] = 120000000,
 		.freq[NOMINAL] = 150000000,
 	},
+	[Q6V5_PIL] = {
+		.pd = &msm8996_vddcx,
+		.freq[HIGH] = INT_MAX,
+	},
 };
 
 static const struct of_device_id rpmpd_performance_table[] = {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH V10 0/3] PM / Domains: Performance state support
  2017-09-19 22:32 [PATCH V10 0/3] PM / Domains: Performance state support Viresh Kumar
                   ` (6 preceding siblings ...)
  2017-09-19 22:32 ` [PATCH V10 7/7] remoteproc: qcom: q6v5: Vote for proxy powerdomain performance state Viresh Kumar
@ 2017-10-03  7:52 ` Ulf Hansson
  2017-10-04  6:45   ` Viresh Kumar
  7 siblings, 1 reply; 11+ messages in thread
From: Ulf Hansson @ 2017-10-03  7:52 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael Wysocki, Kevin Hilman, linux-pm, Vincent Guittot,
	Stephen Boyd, Nishanth Menon, Rob Herring, Lina Iyer,
	Rajendra Nayak, Sudeep Holla, linux-kernel, Len Brown,
	Pavel Machek, Andy Gross, David Brown

On 20 September 2017 at 00:32, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> Hi Ulf,
>
> This version contains the changes we discussed during the LPC.
>
> Some platforms have the capability to configure the performance state of
> their power domains. The process of configuring the performance state is
> pretty much platform dependent and we may need to work with a wide range
> of configurables.  For some platforms, like Qcom, it can be a positive
> integer value alone, while in other cases it can be voltage levels, etc.
>
> The power-domain framework until now was only designed for the idle
> state management of the device and this needs to change in order to
> reuse the power-domain framework for active state management of the
> devices.
>
> The first patch updates the genpd framework to supply new APIs to
> support active state management and the second patch uses them from the
> OPP core. The third patch adds some more checks to the genpd core to
> catch bugs.
>
> Rest of the patches [4-7/7] are included just to show how user drivers
> would end up using the new APIs and these patches aren't there to get
> merged and are marked clearly like that.
>
> The ideal way is still to get the relation between device and domain
> states via the DT instead of platform code, but that can be done
> incrementally later once we have some users for it upstream.
>
> This is currently tested by:
> - /me by hacking the kernel a bit with virtual power-domains for the ARM
>   64 hikey platform. I have also tested the complex cases where the
>   device's parent power domain doesn't have set_performance_state()
>   callback set, but parents of that domains have it. Lockdep configs
>   were enabled for these tests.
> - Rajendra Nayak, on msm8996 platform (Qcom) with MMC controller.
>
> Thanks Rajendra for helping me testing this out.
>
> I also had a chat with Rajendra and we should be able to get a Qualcomm
> specific power domain driver (which uses these changes) in coming weeks.
> Though it wouldn't solve all the problems around corners they have, as
> they need more updates later on (Like support for multiple masters for a
> device, etc).

We sorted out things at LPC!

However, the last weeks discussions at Linaro connect, raised a couple
of more concerns with the current approach. Let me summarize them
here.

1)
The ->dev_get_performance_state(), which currently translates
frequency for a device to a performance index of its PM domain, is too
closely integrated with genpd. Instead this kind of translation rather
belongs as a part of the OPP core, because of not limiting this only
to translate frequencies, but perhaps *later* also voltages.

2)
Propagating an aggregated increased requested performance state index
for a genpd, upwards in the hierarchy of its master domains, is
currently not needed by any existing SoCs.

3) If some day the need for 2) becomes required, we must not assume a
1 to 1 mapping of the supported performance state index for a
master/subdomain. For example a domain may support 1-5, while its
master may support 1-8.

Taking this into consideration, this series need yet another round of
re-spin. The ->dev_get_performance_state() part should be move to the
OPP layer and the code dealing with the aggregation of the performance
state index can be greatly simplified.

[...]

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V10 0/3] PM / Domains: Performance state support
  2017-10-03  7:52 ` [PATCH V10 0/3] PM / Domains: Performance state support Ulf Hansson
@ 2017-10-04  6:45   ` Viresh Kumar
  2017-10-04  7:54     ` Ulf Hansson
  0 siblings, 1 reply; 11+ messages in thread
From: Viresh Kumar @ 2017-10-04  6:45 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rafael Wysocki, Kevin Hilman, linux-pm, Vincent Guittot,
	Stephen Boyd, Nishanth Menon, Rob Herring, Lina Iyer,
	Rajendra Nayak, Sudeep Holla, linux-kernel, Len Brown,
	Pavel Machek, Andy Gross, David Brown

On 03-10-17, 09:52, Ulf Hansson wrote:
> We sorted out things at LPC!
> 
> However, the last weeks discussions at Linaro connect, raised a couple
> of more concerns with the current approach. Let me summarize them
> here.
> 
> 1)
> The ->dev_get_performance_state(), which currently translates
> frequency for a device to a performance index of its PM domain, is too
> closely integrated with genpd. Instead this kind of translation rather
> belongs as a part of the OPP core, because of not limiting this only
> to translate frequencies, but perhaps *later* also voltages.
> 
> 2)
> Propagating an aggregated increased requested performance state index
> for a genpd, upwards in the hierarchy of its master domains, is
> currently not needed by any existing SoCs.
> 
> 3) If some day the need for 2) becomes required, we must not assume a
> 1 to 1 mapping of the supported performance state index for a
> master/subdomain. For example a domain may support 1-5, while its
> master may support 1-8.
> 
> Taking this into consideration, this series need yet another round of
> re-spin. The ->dev_get_performance_state() part should be move to the
> OPP layer and the code dealing with the aggregation of the performance
> state index can be greatly simplified.

Thanks for the summary.

>From the above, it looks like I can send this series right away instead of
waiting for the multiple genpd per device thing? Is that the case ?

-- 
viresh

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH V10 0/3] PM / Domains: Performance state support
  2017-10-04  6:45   ` Viresh Kumar
@ 2017-10-04  7:54     ` Ulf Hansson
  0 siblings, 0 replies; 11+ messages in thread
From: Ulf Hansson @ 2017-10-04  7:54 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Rafael Wysocki, Kevin Hilman, linux-pm, Vincent Guittot,
	Stephen Boyd, Nishanth Menon, Rob Herring, Lina Iyer,
	Rajendra Nayak, Sudeep Holla, linux-kernel, Len Brown,
	Pavel Machek, Andy Gross, David Brown

On 4 October 2017 at 08:45, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 03-10-17, 09:52, Ulf Hansson wrote:
>> We sorted out things at LPC!
>>
>> However, the last weeks discussions at Linaro connect, raised a couple
>> of more concerns with the current approach. Let me summarize them
>> here.
>>
>> 1)
>> The ->dev_get_performance_state(), which currently translates
>> frequency for a device to a performance index of its PM domain, is too
>> closely integrated with genpd. Instead this kind of translation rather
>> belongs as a part of the OPP core, because of not limiting this only
>> to translate frequencies, but perhaps *later* also voltages.
>>
>> 2)
>> Propagating an aggregated increased requested performance state index
>> for a genpd, upwards in the hierarchy of its master domains, is
>> currently not needed by any existing SoCs.
>>
>> 3) If some day the need for 2) becomes required, we must not assume a
>> 1 to 1 mapping of the supported performance state index for a
>> master/subdomain. For example a domain may support 1-5, while its
>> master may support 1-8.
>>
>> Taking this into consideration, this series need yet another round of
>> re-spin. The ->dev_get_performance_state() part should be move to the
>> OPP layer and the code dealing with the aggregation of the performance
>> state index can be greatly simplified.
>
> Thanks for the summary.
>
> From the above, it looks like I can send this series right away instead of
> waiting for the multiple genpd per device thing? Is that the case ?

Yes, I think so!

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-10-04  7:54 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-19 22:32 [PATCH V10 0/3] PM / Domains: Performance state support Viresh Kumar
2017-09-19 22:32 ` [PATCH V10 1/7] PM / Domains: Add support to select performance-state of domains Viresh Kumar
2017-09-19 22:32 ` [PATCH V10 2/7] PM / OPP: Support updating performance state of device's power domains Viresh Kumar
2017-09-19 22:32 ` [PATCH V10 3/7] PM / Domains: Catch missing genpd_set_performance_state() in masters Viresh Kumar
2017-09-19 22:32 ` [PATCH V10 4/7] soc: qcom: rpmpd: Add a powerdomain driver to model cx/mx powerdomains Viresh Kumar
2017-09-19 22:32 ` [PATCH V10 5/7] soc: qcom: rpmpd: Add support for get/set performance state Viresh Kumar
2017-09-19 22:32 ` [PATCH V10 6/7] mmc: sdhci-msm: Adapt the driver to use OPPs to set clocks/performance state Viresh Kumar
2017-09-19 22:32 ` [PATCH V10 7/7] remoteproc: qcom: q6v5: Vote for proxy powerdomain performance state Viresh Kumar
2017-10-03  7:52 ` [PATCH V10 0/3] PM / Domains: Performance state support Ulf Hansson
2017-10-04  6:45   ` Viresh Kumar
2017-10-04  7:54     ` Ulf Hansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.