All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/4] DEVFREQ, DVFS Framework for Non-CPU Devices.
@ 2011-08-18 10:27 MyungJoo Ham
  2011-08-18 10:27 ` [PATCH v6 1/4] PM / OPP: Add OPP availability change notifier MyungJoo Ham
                   ` (3 more replies)
  0 siblings, 4 replies; 21+ messages in thread
From: MyungJoo Ham @ 2011-08-18 10:27 UTC (permalink / raw)
  To: linux-pm; +Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, Thomas Gleixner

For a usage example, please look at
http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq

In the above git tree, DVFS (dynamic voltage and frequency scaling) mechanism
is applied to the memory bus of Exynos4210 for Exynos4210-NURI boards.
In the example, the LPDDR2 DRAM frequency changes between 133, 266, and 400MHz
and other related clocks simply follow the determined DDR RAM clock.

The DEVFREQ driver for Exynos4210 memory bus is at
/drivers/devfreq/exynos4210_memorybus.c in the git tree.

In the dd (writing and reading 360MiB) test with NURI board, the memory
throughput was not changed (the performance is not deteriorated) while
the SoC power consumption has been reduced by 1%. When the memory access
is not that intense while the CPU is heavily used, the SoC power consumption
has been reduced by 6%. The power consumption has been compared with the
case using the conventional Exynos4210 CPUFREQ driver, which sets memory
bus frequency according to the CPU core frequency. Besides, when the CPU core
running slow and the memory access is intense, the performance (memory
throughput) has been increased by 11% (with higher SoC power consumption of
5%). The tested governor is "simple-ondemand".

Major changes since the last revision (v5)
- Moved from drivers/base/power to drivers/devfreq
- Removed devfreq_update() interface.
- Added OPP availability update notifier
- Removed devfreq_interval, using jiffy as the basis interval
- Seperated governors from devfreq core.
- Allow tunable values to be given at devfreq_add_device().
- And some minor revisions.

MyungJoo Ham (4):
  PM / OPP: Add OPP availability change notifier.
  PM: Introduce DEVFREQ: generic DVFS framework with device-specific
    OPPs
  PM / DEVFREQ: add basic governors
  PM / DEVFREQ: add sysfs interface

 Documentation/ABI/testing/sysfs-devices-power |   45 +++
 drivers/Kconfig                               |    2 +
 drivers/Makefile                              |    2 +
 drivers/base/power/opp.c                      |   28 ++
 drivers/devfreq/Kconfig                       |   75 ++++
 drivers/devfreq/Makefile                      |    5 +
 drivers/devfreq/devfreq.c                     |  454 +++++++++++++++++++++++++
 drivers/devfreq/governor_performance.c        |   24 ++
 drivers/devfreq/governor_powersave.c          |   24 ++
 drivers/devfreq/governor_simpleondemand.c     |   88 +++++
 drivers/devfreq/governor_userspace.c          |   27 ++
 include/linux/devfreq.h                       |  146 ++++++++
 include/linux/opp.h                           |   12 +
 13 files changed, 932 insertions(+), 0 deletions(-)
 create mode 100644 drivers/devfreq/Kconfig
 create mode 100644 drivers/devfreq/Makefile
 create mode 100644 drivers/devfreq/devfreq.c
 create mode 100644 drivers/devfreq/governor_performance.c
 create mode 100644 drivers/devfreq/governor_powersave.c
 create mode 100644 drivers/devfreq/governor_simpleondemand.c
 create mode 100644 drivers/devfreq/governor_userspace.c
 create mode 100644 include/linux/devfreq.h

-- 
1.7.4.1

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

* [PATCH v6 1/4] PM / OPP: Add OPP availability change notifier.
  2011-08-18 10:27 [PATCH v6 0/4] DEVFREQ, DVFS Framework for Non-CPU Devices MyungJoo Ham
@ 2011-08-18 10:27 ` MyungJoo Ham
  2011-08-18 22:06   ` Turquette, Mike
  2011-08-18 10:27 ` [PATCH v6 2/4] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs MyungJoo Ham
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 21+ messages in thread
From: MyungJoo Ham @ 2011-08-18 10:27 UTC (permalink / raw)
  To: linux-pm; +Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, Thomas Gleixner

The patch enables to register notifier_block for an OPP-device in order
to get notified for any changes in the availability of OPPs of the
device. For example, if a new OPP is inserted or enable/disable status
of an OPP is changed, the notifier is executed.

This enables the usage of opp_add, opp_enable, and opp_disable to
directly take effect with any connected entities such as CPUFREQ or
DEVFREQ.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

---
Added at devfreq patch set v6 replacing devfreq_update calls at OPP.
---
 drivers/base/power/opp.c |   28 ++++++++++++++++++++++++++++
 include/linux/opp.h      |   12 ++++++++++++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index b23de18..39e16c3 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -73,6 +73,7 @@ struct opp {
  *		RCU usage: nodes are not modified in the list of device_opp,
  *		however addition is possible and is secured by dev_opp_list_lock
  * @dev:	device pointer
+ * @head:	notifier head to notify the OPP availability changes.
  * @opp_list:	list of opps
  *
  * This is an internal data structure maintaining the link to opps attached to
@@ -83,6 +84,8 @@ struct device_opp {
 	struct list_head node;
 
 	struct device *dev;
+	struct srcu_notifier_head head;
+
 	struct list_head opp_list;
 };
 
@@ -404,6 +407,7 @@ int opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
 		}
 
 		dev_opp->dev = dev;
+		srcu_init_notifier_head(&dev_opp->head);
 		INIT_LIST_HEAD(&dev_opp->opp_list);
 
 		/* Secure the device list modification */
@@ -428,6 +432,11 @@ int opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
 	list_add_rcu(&new_opp->node, head);
 	mutex_unlock(&dev_opp_list_lock);
 
+	/*
+	 * Notify the changes in the availability of the operable
+	 * frequency/voltage list.
+	 */
+	srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_ADD, new_opp);
 	return 0;
 }
 
@@ -504,6 +513,11 @@ static int opp_set_availability(struct device *dev, unsigned long freq,
 	mutex_unlock(&dev_opp_list_lock);
 	synchronize_rcu();
 
+	/* Notify the change of the OPP availability */
+	srcu_notifier_call_chain(&dev_opp->head, availability_req ?
+				 OPP_EVENT_ENABLE : OPP_EVENT_DISABLE,
+				 new_opp);
+
 	/* clean up old opp */
 	new_opp = opp;
 	goto out;
@@ -512,6 +526,7 @@ unlock:
 	mutex_unlock(&dev_opp_list_lock);
 out:
 	kfree(new_opp);
+
 	return r;
 }
 
@@ -643,3 +658,16 @@ void opp_free_cpufreq_table(struct device *dev,
 	*table = NULL;
 }
 #endif		/* CONFIG_CPU_FREQ */
+
+/** opp_get_notifier() - find notifier_head of the device with opp
+ * @dev:	device pointer used to lookup device OPPs.
+ */
+struct srcu_notifier_head *opp_get_notifier(struct device *dev)
+{
+	struct device_opp *dev_opp = find_device_opp(dev);
+
+	if (IS_ERR(dev_opp))
+		return ERR_PTR(PTR_ERR(dev_opp)); /* matching type */
+
+	return &dev_opp->head;
+}
diff --git a/include/linux/opp.h b/include/linux/opp.h
index 7020e97..53d4538 100644
--- a/include/linux/opp.h
+++ b/include/linux/opp.h
@@ -16,9 +16,14 @@
 
 #include <linux/err.h>
 #include <linux/cpufreq.h>
+#include <linux/notifier.h>
 
 struct opp;
 
+enum opp_event {
+	OPP_EVENT_ADD, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE, OPP_EVENT_MAX
+};
+
 #if defined(CONFIG_PM_OPP)
 
 unsigned long opp_get_voltage(struct opp *opp);
@@ -40,6 +45,8 @@ int opp_enable(struct device *dev, unsigned long freq);
 
 int opp_disable(struct device *dev, unsigned long freq);
 
+struct srcu_notifier_head *opp_get_notifier(struct device *dev);
+
 #else
 static inline unsigned long opp_get_voltage(struct opp *opp)
 {
@@ -89,6 +96,11 @@ static inline int opp_disable(struct device *dev, unsigned long freq)
 {
 	return 0;
 }
+
+struct srcu_notifier_head *opp_get_notifier(struct device *dev)
+{
+	return ERR_PTR(-EINVAL);
+}
 #endif		/* CONFIG_PM */
 
 #if defined(CONFIG_CPU_FREQ) && defined(CONFIG_PM_OPP)
-- 
1.7.4.1

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

* [PATCH v6 2/4] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs
  2011-08-18 10:27 [PATCH v6 0/4] DEVFREQ, DVFS Framework for Non-CPU Devices MyungJoo Ham
  2011-08-18 10:27 ` [PATCH v6 1/4] PM / OPP: Add OPP availability change notifier MyungJoo Ham
@ 2011-08-18 10:27 ` MyungJoo Ham
  2011-08-19  2:43   ` Turquette, Mike
  2011-08-18 10:27 ` [PATCH v6 3/4] PM / DEVFREQ: add basic governors MyungJoo Ham
  2011-08-18 10:27 ` [PATCH v6 4/4] PM / DEVFREQ: add sysfs interface MyungJoo Ham
  3 siblings, 1 reply; 21+ messages in thread
From: MyungJoo Ham @ 2011-08-18 10:27 UTC (permalink / raw)
  To: linux-pm; +Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, Thomas Gleixner

With OPPs, a device may have multiple operable frequency and voltage
sets. However, there can be multiple possible operable sets and a system
will need to choose one from them. In order to reduce the power
consumption (by reducing frequency and voltage) without affecting the
performance too much, a Dynamic Voltage and Frequency Scaling (DVFS)
scheme may be used.

This patch introduces the DVFS capability to non-CPU devices with OPPs.
DVFS is a techique whereby the frequency and supplied voltage of a
device is adjusted on-the-fly. DVFS usually sets the frequency as low
as possible with given conditions (such as QoS assurance) and adjusts
voltage according to the chosen frequency in order to reduce power
consumption and heat dissipation.

The generic DVFS for devices, DEVFREQ, may appear quite similar with
/drivers/cpufreq.  However, CPUFREQ does not allow to have multiple
devices registered and is not suitable to have multiple heterogenous
devices with different (but simple) governors.

Normally, DVFS mechanism controls frequency based on the demand for
the device, and then, chooses voltage based on the chosen frequency.
DEVFREQ also controls the frequency based on the governor's frequency
recommendation and let OPP pick up the pair of frequency and voltage
based on the recommended frequency. Then, the chosen OPP is passed to
device driver's "target" callback.

When PM QoS is going to be used with the DEVFREQ device, the device
driver should enable OPPs that are appropriate with the current PM QoS
requests. In order to do so, the device driver may call opp_enable and
opp_disable at the notifier callback of PM QoS so that PM QoS's
update_target() call enables the appropriate OPPs. Note that at least
one of OPPs should be enabled at any time; be careful when there is a
transition.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

---
The test code with board support for Exynos4-NURI is at
http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq

---
Thank you for your valuable comments, Rafael, Greg, Pavel, Colin, Mike,
and Kevin.

Changes from v5
- Uses OPP availability change notifier
- Removed devfreq_interval. Uses one jiffy instead. DEVFREQ adjusts
  polling interval based on the interval requirement of DEVFREQ
  devices.
- Moved devfreq to /drivers/devfreq to accomodate devfreq-related files
  including governors and devfreq drivers.
- Coding style revised.
- Updated devfreq_add_device interface to get tunable values.

Changed from v4
- Removed tickle, which is a duplicated feature; PM QoS can do the same.
- Allow to extend polling interval if devices have longer polling intervals.
- Relocated private data of governors.
- Removed system-wide sysfs

Changed from v3
- In kerneldoc comments, DEVFREQ has ben replaced by devfreq
- Revised removing devfreq entries with error mechanism
- Added and revised comments
- Removed unnecessary codes
- Allow to give a name to a governor
- Bugfix: a tickle call may cancel an older tickle call that is still in
  effect.

Changed from v2
- Code style revised and cleaned up.
- Remove DEVFREQ entries that incur errors except for EAGAIN
- Bug fixed: tickle for devices without polling governors

Changes from v1(RFC)
- Rename: DVFS --> DEVFREQ
- Revised governor design
	. Governor receives the whole struct devfreq
	. Governor should gather usage information (thru get_dev_status) itself
- Periodic monitoring runs only when needed.
- DEVFREQ no more deals with voltage information directly
- Removed some printks.
- Some cosmetics update
- Use freezable_wq.
---
 drivers/Kconfig           |    2 +
 drivers/Makefile          |    2 +
 drivers/devfreq/Kconfig   |   39 ++++++
 drivers/devfreq/Makefile  |    1 +
 drivers/devfreq/devfreq.c |  302 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/devfreq.h   |  105 ++++++++++++++++
 6 files changed, 451 insertions(+), 0 deletions(-)
 create mode 100644 drivers/devfreq/Kconfig
 create mode 100644 drivers/devfreq/Makefile
 create mode 100644 drivers/devfreq/devfreq.c
 create mode 100644 include/linux/devfreq.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 95b9e7e..a1efd75 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -130,4 +130,6 @@ source "drivers/iommu/Kconfig"
 
 source "drivers/virt/Kconfig"
 
+source "drivers/devfreq/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 7fa433a..97c957b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -127,3 +127,5 @@ obj-$(CONFIG_IOMMU_SUPPORT)	+= iommu/
 
 # Virtualization drivers
 obj-$(CONFIG_VIRT_DRIVERS)	+= virt/
+
+obj-$(CONFIG_PM_DEVFREQ)	+= devfreq/
diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
new file mode 100644
index 0000000..1fb42de
--- /dev/null
+++ b/drivers/devfreq/Kconfig
@@ -0,0 +1,39 @@
+config ARCH_HAS_DEVFREQ
+	bool
+	depends on ARCH_HAS_OPP
+	help
+	  Denotes that the architecture supports DEVFREQ. If the architecture
+	  supports multiple OPP entries per device and the frequency of the
+	  devices with OPPs may be altered dynamically, the architecture
+	  supports DEVFREQ.
+
+menuconfig PM_DEVFREQ
+	bool "Generic Dynamic Voltage and Frequency Scaling (DVFS) support"
+	depends on PM_OPP && ARCH_HAS_DEVFREQ
+	help
+	  With OPP support, a device may have a list of frequencies and
+	  voltages available. DEVFREQ, a generic DVFS framework can be
+	  registered for a device with OPP support in order to let the
+	  governor provided to DEVFREQ choose an operating frequency
+	  based on the OPP's list and the policy given with DEVFREQ.
+
+	  Each device may have its own governor and policy. DEVFREQ can
+	  reevaluate the device state periodically and/or based on the
+	  OPP list changes (each frequency/voltage pair in OPP may be
+	  disabled or enabled).
+
+	  Like some CPUs with CPUFREQ, a device may have multiple clocks.
+	  However, because the clock frequencies of a single device are
+	  determined by the single device's state, an instance of DEVFREQ
+	  is attached to a single device and returns a "representative"
+	  clock frequency from the OPP of the device, which is also attached
+	  to a device by 1-to-1. The device registering DEVFREQ takes the
+	  responsiblity to "interpret" the frequency listed in OPP and
+	  to set its every clock accordingly with the "target" callback
+	  given to DEVFREQ.
+
+if PM_DEVFREQ
+
+comment "DEVFREQ Drivers"
+
+endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
new file mode 100644
index 0000000..168934a
--- /dev/null
+++ b/drivers/devfreq/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_PM_DEVFREQ)	+= devfreq.o
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
new file mode 100644
index 0000000..2036f2c
--- /dev/null
+++ b/drivers/devfreq/devfreq.c
@@ -0,0 +1,302 @@
+/*
+ * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
+ *	    for Non-CPU Devices Based on OPP.
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/opp.h>
+#include <linux/devfreq.h>
+#include <linux/workqueue.h>
+#include <linux/platform_device.h>
+#include <linux/list.h>
+#include <linux/printk.h>
+#include <linux/hrtimer.h>
+
+/*
+ * devfreq_work periodically monitors every registered device.
+ * The minimum polling interval is one jiffy. The polling interval is
+ * determined by the minimum polling period among all polling devfreq
+ * devices. The resolution of polling interval is one jiffy.
+ */
+static bool polling;
+static struct workqueue_struct *devfreq_wq;
+static struct delayed_work devfreq_work;
+
+/* The list of all device-devfreq */
+static LIST_HEAD(devfreq_list);
+static DEFINE_MUTEX(devfreq_list_lock);
+
+/**
+ * find_device_devfreq() - find devfreq struct using device pointer
+ * @dev:	device pointer used to lookup device devfreq.
+ *
+ * Search the list of device devfreqs and return the matched device's
+ * devfreq info. devfreq_list_lock should be held by the caller.
+ */
+static struct devfreq *find_device_devfreq(struct device *dev)
+{
+	struct devfreq *tmp_devfreq;
+
+	if (unlikely(IS_ERR_OR_NULL(dev))) {
+		pr_err("DEVFREQ: %s: Invalid parameters\n", __func__);
+		return ERR_PTR(-EINVAL);
+	}
+
+	list_for_each_entry(tmp_devfreq, &devfreq_list, node) {
+		if (tmp_devfreq->dev == dev)
+			return tmp_devfreq;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
+/**
+ * devfreq_do() - Check the usage profile of a given device and configure
+ *		frequency and voltage accordingly
+ * @devfreq:	devfreq info of the given device
+ */
+static int devfreq_do(struct devfreq *devfreq)
+{
+	struct opp *opp;
+	unsigned long freq;
+	int err;
+
+	err = devfreq->governor->get_target_freq(devfreq, &freq);
+	if (err)
+		return err;
+
+	opp = opp_find_freq_ceil(devfreq->dev, &freq);
+	if (opp == ERR_PTR(-ENODEV))
+		opp = opp_find_freq_floor(devfreq->dev, &freq);
+
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+
+	if (devfreq->previous_freq == freq)
+		return 0;
+
+	err = devfreq->profile->target(devfreq->dev, opp);
+	if (err)
+		return err;
+
+	devfreq->previous_freq = freq;
+	return 0;
+}
+
+/**
+ * devfreq_update() - Notify that the device OPP has been changed.
+ * @dev:	the device whose OPP has been changed.
+ */
+static int devfreq_update(struct notifier_block *nb, unsigned long type,
+			  void *devp)
+{
+	struct devfreq *devfreq;
+	int err = 0;
+
+	mutex_lock(&devfreq_list_lock);
+	devfreq = container_of(nb, struct devfreq, nb);
+	/* Reevaluate the proper frequency */
+	err = devfreq_do(devfreq);
+	mutex_unlock(&devfreq_list_lock);
+	return err;
+}
+
+/**
+ * devfreq_monitor() - Periodically run devfreq_do()
+ * @work: the work struct used to run devfreq_monitor periodically.
+ *
+ */
+static void devfreq_monitor(struct work_struct *work)
+{
+	static ktime_t last_polled_at;
+	struct devfreq *devfreq, *tmp;
+	int error;
+	unsigned int next_jiffies = UINT_MAX;
+	ktime_t now = ktime_get();
+	int jiffies_passed;
+
+	/* Initially last_polled_at = 0, polling every device at bootup */
+	jiffies_passed = msecs_to_jiffies(ktime_to_ms(
+				ktime_sub(now, last_polled_at)));
+	last_polled_at = now;
+
+	if (jiffies_passed == 0)
+		jiffies_passed = 1;
+	if (jiffies_passed < 0) /* "Infinite Timeout" */
+		jiffies_passed = INT_MAX;
+
+	mutex_lock(&devfreq_list_lock);
+
+	list_for_each_entry_safe(devfreq, tmp, &devfreq_list, node) {
+		if (devfreq->next_polling == 0)
+			continue;
+
+		/*
+		 * Reduce more next_polling if devfreq_wq took an extra
+		 * delay. (i.e., CPU has been idled.)
+		 */
+		if (devfreq->next_polling <= jiffies_passed) {
+			error = devfreq_do(devfreq);
+
+			/* Remove a devfreq with an error. */
+			if (error && error != -EAGAIN) {
+				dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
+					error, devfreq->governor->name);
+
+				list_del(&devfreq->node);
+				kfree(devfreq);
+
+				continue;
+			}
+			devfreq->next_polling = msecs_to_jiffies(
+						devfreq->profile->polling_ms);
+
+			/* No more polling required (polling_ms changed) */
+			if (devfreq->next_polling == 0)
+				continue;
+		} else {
+			devfreq->next_polling -= jiffies_passed;
+		}
+
+		next_jiffies = (next_jiffies > devfreq->next_polling) ?
+				devfreq->next_polling : next_jiffies;
+	}
+
+	if (next_jiffies > 0 && next_jiffies < UINT_MAX) {
+		polling = true;
+		queue_delayed_work(devfreq_wq, &devfreq_work, next_jiffies);
+	} else {
+		polling = false;
+	}
+
+	mutex_unlock(&devfreq_list_lock);
+}
+
+/**
+ * devfreq_add_device() - Add devfreq feature to the device
+ * @dev:	the device to add devfreq feature.
+ * @profile:	device-specific profile to run devfreq.
+ * @governor:	the policy to choose frequency.
+ * @data:	private data for the governor. The devfreq framework does not
+ *		touch this value.
+ */
+int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
+		       struct devfreq_governor *governor, void *data)
+{
+	struct devfreq *devfreq;
+	struct srcu_notifier_head *nh;
+	int err = 0;
+
+	if (!dev || !profile || !governor) {
+		dev_err(dev, "%s: Invalid parameters.\n", __func__);
+		return -EINVAL;
+	}
+
+	mutex_lock(&devfreq_list_lock);
+
+	devfreq = find_device_devfreq(dev);
+	if (!IS_ERR(devfreq)) {
+		dev_err(dev, "%s: Unable to create devfreq for the device. It already has one.\n", __func__);
+		err = -EINVAL;
+		goto out;
+	}
+
+	devfreq = kzalloc(sizeof(struct devfreq), GFP_KERNEL);
+	if (!devfreq) {
+		dev_err(dev, "%s: Unable to create devfreq for the device\n",
+			__func__);
+		err = -ENOMEM;
+		goto out;
+	}
+
+	devfreq->dev = dev;
+	devfreq->profile = profile;
+	devfreq->governor = governor;
+	devfreq->next_polling = msecs_to_jiffies(profile->polling_ms);
+	devfreq->previous_freq = profile->initial_freq;
+	devfreq->data = data;
+
+	devfreq->nb.notifier_call = devfreq_update;
+	nh = opp_get_notifier(dev);
+	if (IS_ERR(nh)) {
+		err = PTR_ERR(nh);
+		goto out;
+	}
+	err = srcu_notifier_chain_register(nh, &devfreq->nb);
+	if (err)
+		goto out;
+
+	list_add(&devfreq->node, &devfreq_list);
+
+	if (devfreq_wq && devfreq->next_polling && !polling) {
+		polling = true;
+		queue_delayed_work(devfreq_wq, &devfreq_work,
+				   devfreq->next_polling);
+	}
+out:
+	mutex_unlock(&devfreq_list_lock);
+
+	return err;
+}
+
+/**
+ * devfreq_remove_device() - Remove devfreq feature from a device.
+ * @device:	the device to remove devfreq feature.
+ */
+int devfreq_remove_device(struct device *dev)
+{
+	struct devfreq *devfreq;
+	struct srcu_notifier_head *nh;
+	int err = 0;
+
+	if (!dev)
+		return -EINVAL;
+
+	mutex_lock(&devfreq_list_lock);
+	devfreq = find_device_devfreq(dev);
+	if (IS_ERR(devfreq)) {
+		err = PTR_ERR(devfreq);
+		goto out;
+	}
+
+	nh = opp_get_notifier(dev);
+	if (IS_ERR(nh)) {
+		err = PTR_ERR(nh);
+		goto out;
+	}
+
+	list_del(&devfreq->node);
+	srcu_notifier_chain_unregister(nh, &devfreq->nb);
+	kfree(devfreq);
+out:
+	mutex_unlock(&devfreq_list_lock);
+	return 0;
+}
+
+/**
+ * devfreq_init() - Initialize data structure for devfreq framework and
+ *		  start polling registered devfreq devices.
+ */
+static int __init devfreq_init(void)
+{
+	mutex_lock(&devfreq_list_lock);
+	polling = false;
+	devfreq_wq = create_freezable_workqueue("devfreq_wq");
+	INIT_DELAYED_WORK_DEFERRABLE(&devfreq_work, devfreq_monitor);
+	mutex_unlock(&devfreq_list_lock);
+
+	devfreq_monitor(&devfreq_work.work);
+	return 0;
+}
+late_initcall(devfreq_init);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
new file mode 100644
index 0000000..13ddf49
--- /dev/null
+++ b/include/linux/devfreq.h
@@ -0,0 +1,105 @@
+/*
+ * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
+ *	    for Non-CPU Devices Based on OPP.
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * 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 __LINUX_DEVFREQ_H__
+#define __LINUX_DEVFREQ_H__
+
+#include <linux/notifier.h>
+
+#define DEVFREQ_NAME_LEN 16
+
+struct devfreq;
+struct devfreq_dev_status {
+	/* both since the last measure */
+	unsigned long total_time;
+	unsigned long busy_time;
+	unsigned long current_frequency;
+};
+
+struct devfreq_dev_profile {
+	unsigned long max_freq; /* may be larger than the actual value */
+	unsigned long initial_freq;
+	int polling_ms;	/* 0 for at opp change only */
+
+	int (*target)(struct device *dev, struct opp *opp);
+	int (*get_dev_status)(struct device *dev,
+			      struct devfreq_dev_status *stat);
+};
+
+/**
+ * struct devfreq_governor - Devfreq policy governor
+ * @name		Governor's name
+ * @get_target_freq	Returns desired operating frequency for the device.
+ *			Basically, get_target_freq will run
+ *			devfreq_dev_profile.get_dev_status() to get the
+ *			status of the device (load = busy_time / total_time).
+ */
+struct devfreq_governor {
+	char name[DEVFREQ_NAME_LEN];
+	int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
+};
+
+/**
+ * struct devfreq - Device devfreq structure
+ * @node	list node - contains the devices with devfreq that have been
+ *		registered.
+ * @dev		device pointer
+ * @profile	device-specific devfreq profile
+ * @governor	method how to choose frequency based on the usage.
+ * @nb		notifier block registered to the corresponding OPP to get
+ *		notified for frequency availability updates.
+ * @previous_freq	previously configured frequency value.
+ * @next_polling	the number of remaining jiffies to poll with
+ *			"devfreq_monitor" executions to reevaluate
+ *			frequency/voltage of the device. Set by
+ *			profile's polling_ms interval.
+ * @data	Private data of the governor. The devfreq framework does not
+ *		touch this.
+ *
+ * This structure stores the devfreq information for a give device.
+ */
+struct devfreq {
+	struct list_head node;
+
+	struct device *dev;
+	struct devfreq_dev_profile *profile;
+	struct devfreq_governor *governor;
+	struct notifier_block nb;
+
+	unsigned long previous_freq;
+	unsigned int next_polling;
+
+	void *data; /* private data for governors */
+};
+
+#if defined(CONFIG_PM_DEVFREQ)
+extern int devfreq_add_device(struct device *dev,
+			   struct devfreq_dev_profile *profile,
+			   struct devfreq_governor *governor,
+			   void *data);
+extern int devfreq_remove_device(struct device *dev);
+#else /* !CONFIG_PM_DEVFREQ */
+static int devfreq_add_device(struct device *dev,
+			   struct devfreq_dev_profile *profile,
+			   struct devfreq_governor *governor,
+			   void *data)
+{
+	return 0;
+}
+
+static int devfreq_remove_device(struct device *dev)
+{
+	return 0;
+}
+#endif /* CONFIG_PM_DEVFREQ */
+
+#endif /* __LINUX_DEVFREQ_H__ */
-- 
1.7.4.1

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

* [PATCH v6 3/4] PM / DEVFREQ: add basic governors
  2011-08-18 10:27 [PATCH v6 0/4] DEVFREQ, DVFS Framework for Non-CPU Devices MyungJoo Ham
  2011-08-18 10:27 ` [PATCH v6 1/4] PM / OPP: Add OPP availability change notifier MyungJoo Ham
  2011-08-18 10:27 ` [PATCH v6 2/4] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs MyungJoo Ham
@ 2011-08-18 10:27 ` MyungJoo Ham
  2011-08-19  3:12   ` Turquette, Mike
  2011-08-18 10:27 ` [PATCH v6 4/4] PM / DEVFREQ: add sysfs interface MyungJoo Ham
  3 siblings, 1 reply; 21+ messages in thread
From: MyungJoo Ham @ 2011-08-18 10:27 UTC (permalink / raw)
  To: linux-pm; +Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, Thomas Gleixner

Four CPUFREQ-like governors are provided as examples.

powersave: use the lowest frequency possible. The user (device) should
set the polling_ms as 0 because polling is useless for this governor.

performance: use the highest freqeuncy possible. The user (device)
should set the polling_ms as 0 because polling is useless for this
governor.

userspace: use the user specified frequency stored at
devfreq.user_set_freq. With sysfs support in the following patch, a user
may set the value with the sysfs interface.

simple_ondemand: simplified version of CPUFREQ's ONDEMAND governor.

When a user updates OPP entries (enable/disable/add), OPP framework
automatically notifies DEVFREQ to update operating frequency
accordingly. Thus, DEVFREQ users (device drivers) do not need to update
DEVFREQ manually with OPP entry updates or set polling_ms for powersave
, performance, userspace, or any other "static" governors.

Note that these are given only as basic examples for governors and any
devices with DEVFREQ may implement their own governors with the drivers
and use them.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

---
Changed from v5
- Seperated governor files from devfreq.c
- Allow simple ondemand to be tuned for each device
---
 drivers/devfreq/Kconfig                   |   36 ++++++++++++
 drivers/devfreq/Makefile                  |    4 +
 drivers/devfreq/governor_performance.c    |   24 ++++++++
 drivers/devfreq/governor_powersave.c      |   24 ++++++++
 drivers/devfreq/governor_simpleondemand.c |   88 +++++++++++++++++++++++++++++
 drivers/devfreq/governor_userspace.c      |   27 +++++++++
 include/linux/devfreq.h                   |   41 +++++++++++++
 7 files changed, 244 insertions(+), 0 deletions(-)
 create mode 100644 drivers/devfreq/governor_performance.c
 create mode 100644 drivers/devfreq/governor_powersave.c
 create mode 100644 drivers/devfreq/governor_simpleondemand.c
 create mode 100644 drivers/devfreq/governor_userspace.c

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 1fb42de..643b055 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -34,6 +34,42 @@ menuconfig PM_DEVFREQ
 
 if PM_DEVFREQ
 
+comment "DEVFREQ Governors"
+
+config DEVFREQ_GOV_SIMPLE_ONDEMAND
+	bool "Simple Ondemand"
+	help
+	  Chooses frequency based on the recent load on the device. Works
+	  similar as ONDEMAND governor of CPUFREQ does. A device with
+	  Simple-Ondemand should be able to provide busy/total counter
+	  values that imply the usage rate. A device may provide tuned
+	  values to the governor with data field at devfreq_add_device().
+
+config DEVFREQ_GOV_PERFORMANCE
+	bool "Performance"
+	help
+	  Sets the frequency at the maximum available frequency.
+	  This governor always returns UINT_MAX as frequency so that
+	  the DEVFREQ framework returns the highest frequency available
+	  at any time.
+
+config DEVFREQ_GOV_POWERSAVE
+	bool "Powersave"
+	help
+	  Sets the frequency at the minimum available frequency.
+	  This governor always returns 0 as frequency so that
+	  the DEVFREQ framework returns the lowest frequency available
+	  at any time.
+
+config DEVFREQ_GOV_USERSPACE
+	bool "Userspace"
+	help
+	  Sets the frequency at the user specified one.
+	  This governor returns the user configured frequency if there
+	  has been an input to /sys/devices/.../power/devfreq_set_freq.
+	  Otherwise, the governor does not change the frequnecy
+	  given at the initialization.
+
 comment "DEVFREQ Drivers"
 
 endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
index 168934a..4564a89 100644
--- a/drivers/devfreq/Makefile
+++ b/drivers/devfreq/Makefile
@@ -1 +1,5 @@
 obj-$(CONFIG_PM_DEVFREQ)	+= devfreq.o
+obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)	+= governor_simpleondemand.o
+obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)	+= governor_performance.o
+obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)	+= governor_powersave.o
+obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)	+= governor_userspace.o
diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
new file mode 100644
index 0000000..c47eff8
--- /dev/null
+++ b/drivers/devfreq/governor_performance.c
@@ -0,0 +1,24 @@
+/*
+ *  linux/drivers/devfreq/governor_performance.c
+ *
+ *  Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * 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.
+ */
+
+#include <linux/devfreq.h>
+
+static int devfreq_performance_func(struct devfreq *df,
+				    unsigned long *freq)
+{
+	*freq = UINT_MAX; /* devfreq_do will run "floor" */
+	return 0;
+}
+
+struct devfreq_governor devfreq_performance = {
+	.name = "performance",
+	.get_target_freq = devfreq_performance_func,
+};
diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
new file mode 100644
index 0000000..4f128d8
--- /dev/null
+++ b/drivers/devfreq/governor_powersave.c
@@ -0,0 +1,24 @@
+/*
+ *  linux/drivers/devfreq/governor_powersave.c
+ *
+ *  Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * 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.
+ */
+
+#include <linux/devfreq.h>
+
+static int devfreq_powersave_func(struct devfreq *df,
+				  unsigned long *freq)
+{
+	*freq = 0; /* devfreq_do will run "ceiling" to 0 */
+	return 0;
+}
+
+struct devfreq_governor devfreq_powersave = {
+	.name = "powersave",
+	.get_target_freq = devfreq_powersave_func,
+};
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
new file mode 100644
index 0000000..18fe8be
--- /dev/null
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -0,0 +1,88 @@
+/*
+ *  linux/drivers/devfreq/governor_simpleondemand.c
+ *
+ *  Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * 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.
+ */
+
+#include <linux/errno.h>
+#include <linux/devfreq.h>
+#include <linux/math64.h>
+
+/* Default constants for DevFreq-Simple-Ondemand (DFSO) */
+#define DFSO_UPTHRESHOLD	(90)
+#define DFSO_DOWNDIFFERENCTIAL	(5)
+static int devfreq_simple_ondemand_func(struct devfreq *df,
+					unsigned long *freq)
+{
+	struct devfreq_dev_status stat;
+	int err = df->profile->get_dev_status(df->dev, &stat);
+	unsigned long long a, b;
+	unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
+	unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
+	struct devfreq_simple_ondemand_data *data = df->data;
+
+	if (err)
+		return err;
+
+	if (data) {
+		if (data->upthreshold)
+			dfso_upthreshold = data->upthreshold;
+		if (data->downdifferential)
+			dfso_downdifferential = data->downdifferential;
+	}
+	if (dfso_upthreshold > 100 ||
+	    dfso_upthreshold < dfso_downdifferential)
+		return -EINVAL;
+
+	/* Assume MAX if it is going to be divided by zero */
+	if (stat.total_time == 0) {
+		*freq = UINT_MAX;
+		return 0;
+	}
+
+	/* Prevent overflow */
+	if (stat.busy_time >= (1 << 24) || stat.total_time >= (1 << 24)) {
+		stat.busy_time >>= 7;
+		stat.total_time >>= 7;
+	}
+
+	/* Set MAX if it's busy enough */
+	if (stat.busy_time * 100 >
+	    stat.total_time * dfso_upthreshold) {
+		*freq = UINT_MAX;
+		return 0;
+	}
+
+	/* Set MAX if we do not know the initial frequency */
+	if (stat.current_frequency == 0) {
+		*freq = UINT_MAX;
+		return 0;
+	}
+
+	/* Keep the current frequency */
+	if (stat.busy_time * 100 >
+	    stat.total_time * (dfso_upthreshold - dfso_downdifferential)) {
+		*freq = stat.current_frequency;
+		return 0;
+	}
+
+	/* Set the desired frequency based on the load */
+	a = stat.busy_time;
+	a *= stat.current_frequency;
+	b = div_u64(a, stat.total_time);
+	b *= 100;
+	b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
+	*freq = (unsigned long) b;
+
+	return 0;
+}
+
+struct devfreq_governor devfreq_simple_ondemand = {
+	.name = "simple_ondemand",
+	.get_target_freq = devfreq_simple_ondemand_func,
+};
diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
new file mode 100644
index 0000000..6af8b6d
--- /dev/null
+++ b/drivers/devfreq/governor_userspace.c
@@ -0,0 +1,27 @@
+/*
+ *  linux/drivers/devfreq/governor_simpleondemand.c
+ *
+ *  Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * 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.
+ */
+
+#include <linux/devfreq.h>
+
+static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
+{
+	if (df->user_set_freq == 0)
+		*freq = df->previous_freq; /* No user freq specified yet */
+	else
+		*freq = df->user_set_freq;
+
+	return 0;
+}
+
+struct devfreq_governor devfreq_userspace = {
+	.name = "userspace",
+	.get_target_freq = devfreq_userspace_func,
+};
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 13ddf49..d088ad3 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -13,6 +13,7 @@
 #ifndef __LINUX_DEVFREQ_H__
 #define __LINUX_DEVFREQ_H__
 
+#include <linux/opp.h>
 #include <linux/notifier.h>
 
 #define DEVFREQ_NAME_LEN 16
@@ -62,6 +63,8 @@ struct devfreq_governor {
  *			"devfreq_monitor" executions to reevaluate
  *			frequency/voltage of the device. Set by
  *			profile's polling_ms interval.
+ * @user_set_freq	User specified adequete frequency value (thru sysfs
+ *		interface). Governors may and may not use this value.
  * @data	Private data of the governor. The devfreq framework does not
  *		touch this.
  *
@@ -78,6 +81,7 @@ struct devfreq {
 	unsigned long previous_freq;
 	unsigned int next_polling;
 
+	unsigned long user_set_freq; /* governors may ignore this. */
 	void *data; /* private data for governors */
 };
 
@@ -87,6 +91,37 @@ extern int devfreq_add_device(struct device *dev,
 			   struct devfreq_governor *governor,
 			   void *data);
 extern int devfreq_remove_device(struct device *dev);
+
+#ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
+extern struct devfreq_governor devfreq_powersave;
+#endif
+#ifdef CONFIG_DEVFREQ_GOV_PERFORMANCE
+extern struct devfreq_governor devfreq_performance;
+#endif
+#ifdef CONFIG_DEVFREQ_GOV_USERSPACE
+extern struct devfreq_governor devfreq_userspace;
+#endif
+#ifdef CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND
+extern struct devfreq_governor devfreq_simple_ondemand;
+/**
+ * struct devfreq_simple_ondemand_data - void *data fed to struct devfreq
+ *	and devfreq_add_device
+ * @ upthreshold	If the load is over this value, the frequency jumps.
+ *			Specify 0 to use the default. Valid value = 0 to 100.
+ * @ downdifferential	If the load is under upthreshold - downdifferential,
+ *			the governor may consider slowing the frequency down.
+ *			Specify 0 to use the default. Valid value = 0 to 100.
+ *			downdifferential < upthreshold must hold.
+ *
+ * If the fed devfreq_simple_ondemand_data pointer is NULL to the governor,
+ * the governor uses the default values.
+ */
+struct devfreq_simple_ondemand_data {
+	unsigned int upthreshold;
+	unsigned int downdifferential;
+};
+#endif
+
 #else /* !CONFIG_PM_DEVFREQ */
 static int devfreq_add_device(struct device *dev,
 			   struct devfreq_dev_profile *profile,
@@ -100,6 +135,12 @@ static int devfreq_remove_device(struct device *dev)
 {
 	return 0;
 }
+
+#define devfreq_powersave	NULL
+#define devfreq_performance	NULL
+#define devfreq_userspace	NULL
+#define devfreq_simple_ondemand	NULL
+
 #endif /* CONFIG_PM_DEVFREQ */
 
 #endif /* __LINUX_DEVFREQ_H__ */
-- 
1.7.4.1

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

* [PATCH v6 4/4] PM / DEVFREQ: add sysfs interface
  2011-08-18 10:27 [PATCH v6 0/4] DEVFREQ, DVFS Framework for Non-CPU Devices MyungJoo Ham
                   ` (2 preceding siblings ...)
  2011-08-18 10:27 ` [PATCH v6 3/4] PM / DEVFREQ: add basic governors MyungJoo Ham
@ 2011-08-18 10:27 ` MyungJoo Ham
  2011-08-19  3:15   ` Turquette, Mike
  3 siblings, 1 reply; 21+ messages in thread
From: MyungJoo Ham @ 2011-08-18 10:27 UTC (permalink / raw)
  To: linux-pm; +Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, Thomas Gleixner

Device specific sysfs interface /sys/devices/.../power/devfreq_*
- governor	R: name of governor
- cur_freq	R: current frequency
- max_freq	R: maximum operable frequency
- min_freq	R: minimum operable frequency
- set_freq	R: read user specified frequency (0 if not specified yet)
		W: set user specified frequency
- polling_interval	R: polling interval in ms given with devfreq profile

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

--
Changed from v5
- updated devferq_update usage.

Changed from v4
- removed system-wide sysfs interface
- removed tickling sysfs interface
- added set_freq for userspace governor (and any other governors that
  require user input)

Changed from v3
- corrected sysfs API usage
- corrected error messages
- moved sysfs entry location
- added sysfs entries

Changed from v2
- add ABI entries for devfreq sysfs interface
---
 Documentation/ABI/testing/sysfs-devices-power |   45 +++++++
 drivers/devfreq/devfreq.c                     |  152 +++++++++++++++++++++++++
 2 files changed, 197 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
index 8ffbc25..6e77604 100644
--- a/Documentation/ABI/testing/sysfs-devices-power
+++ b/Documentation/ABI/testing/sysfs-devices-power
@@ -165,3 +165,48 @@ Description:
 
 		Not all drivers support this attribute.  If it isn't supported,
 		attempts to read or write it will yield I/O errors.
+
+What:		/sys/devices/.../power/devfreq_governor
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_governor shows the name
+		of the governor used by the corresponding device.
+
+What:		/sys/devices/.../power/devfreq_cur_freq
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_cur_freq shows the current
+		frequency of the corresponding device.
+
+What:		/sys/devices/.../power/devfreq_max_freq
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_cur_freq shows the
+		maximum operable frequency of the corresponding device.
+
+What:		/sys/devices/.../power/devfreq_min_freq
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_cur_freq shows the
+		minimum operable frequency of the corresponding device.
+
+What:		/sys/devices/.../power/devfreq_set_freq
+Date:		August 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_set_freq sets and shows
+		the user specified desired frequency of the device. The
+		governor may and may not use the value. With the basic
+		governors given with devfreq.c, userspace governor is
+		using the value.
+
+What:		/sys/devices/.../power/devfreq_polling_interval
+Date:		July 2011
+Contact:	MyungJoo Ham <myungjoo.ham@samsung.com>
+Description:
+		The /sys/devices/.../power/devfreq_polling_interval shows the
+		requested polling interval of the corresponding device.
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 2036f2c..13b422f 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -37,6 +37,8 @@ static struct delayed_work devfreq_work;
 static LIST_HEAD(devfreq_list);
 static DEFINE_MUTEX(devfreq_list_lock);
 
+static struct attribute_group dev_attr_group;
+
 /**
  * find_device_devfreq() - find devfreq struct using device pointer
  * @dev:	device pointer used to lookup device devfreq.
@@ -154,6 +156,8 @@ static void devfreq_monitor(struct work_struct *work)
 				dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
 					error, devfreq->governor->name);
 
+				sysfs_unmerge_group(&devfreq->dev->kobj,
+						    &dev_attr_group);
 				list_del(&devfreq->node);
 				kfree(devfreq);
 
@@ -244,6 +248,8 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
 		queue_delayed_work(devfreq_wq, &devfreq_work,
 				   devfreq->next_polling);
 	}
+
+	sysfs_merge_group(&dev->kobj, &dev_attr_group);
 out:
 	mutex_unlock(&devfreq_list_lock);
 
@@ -276,6 +282,8 @@ int devfreq_remove_device(struct device *dev)
 		goto out;
 	}
 
+	sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
+
 	list_del(&devfreq->node);
 	srcu_notifier_chain_unregister(nh, &devfreq->nb);
 	kfree(devfreq);
@@ -284,6 +292,150 @@ out:
 	return 0;
 }
 
+static ssize_t show_governor(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = find_device_devfreq(dev);
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->governor)
+		return -EINVAL;
+
+	return sprintf(buf, "%s\n", df->governor->name);
+}
+
+static ssize_t show_freq(struct device *dev,
+			 struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = find_device_devfreq(dev);
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+
+	return sprintf(buf, "%lu\n", df->previous_freq);
+}
+
+static ssize_t show_max_freq(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = find_device_devfreq(dev);
+	unsigned long freq = ULONG_MAX;
+	struct opp *opp;
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->dev)
+		return -EINVAL;
+
+	opp = opp_find_freq_floor(df->dev, &freq);
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+
+	return sprintf(buf, "%lu\n", freq);
+}
+
+static ssize_t show_min_freq(struct device *dev,
+			     struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = find_device_devfreq(dev);
+	unsigned long freq = 0;
+	struct opp *opp;
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->dev)
+		return -EINVAL;
+
+	opp = opp_find_freq_ceil(df->dev, &freq);
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+
+	return sprintf(buf, "%lu\n", freq);
+}
+
+static ssize_t show_polling_interval(struct device *dev,
+				     struct device_attribute *attr, char *buf)
+{
+	struct devfreq *df = find_device_devfreq(dev);
+
+	if (IS_ERR(df))
+		return PTR_ERR(df);
+	if (!df->profile)
+		return -EINVAL;
+
+	return sprintf(buf, "%d\n", df->profile->polling_ms);
+}
+
+static ssize_t set_user_frequency(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	struct devfreq *devfreq;
+	unsigned long wanted;
+	int err = 0;
+
+	sscanf(buf, "%lu", &wanted);
+
+	mutex_lock(&devfreq_list_lock);
+	devfreq = find_device_devfreq(dev);
+
+	if (IS_ERR(devfreq)) {
+		err = PTR_ERR(devfreq);
+		goto out;
+	}
+
+	devfreq->user_set_freq = wanted;
+	err = count;
+out:
+	mutex_unlock(&devfreq_list_lock);
+	if (err >= 0)
+		devfreq_update(&devfreq->nb, 0, NULL);
+	return err;
+}
+
+static ssize_t show_user_frequency(struct device *dev,
+				   struct device_attribute *attr, char *buf)
+{
+	struct devfreq *devfreq;
+	int err = 0;
+
+	mutex_lock(&devfreq_list_lock);
+	devfreq = find_device_devfreq(dev);
+
+	if (IS_ERR(devfreq)) {
+		err = PTR_ERR(devfreq);
+		goto out;
+	}
+
+	err = sprintf(buf, "%lu\n", devfreq->user_set_freq);
+out:
+	mutex_unlock(&devfreq_list_lock);
+	return err;
+
+}
+
+static DEVICE_ATTR(devfreq_governor, 0444, show_governor, NULL);
+static DEVICE_ATTR(devfreq_cur_freq, 0444, show_freq, NULL);
+static DEVICE_ATTR(devfreq_max_freq, 0444, show_max_freq, NULL);
+static DEVICE_ATTR(devfreq_min_freq, 0444, show_min_freq, NULL);
+static DEVICE_ATTR(devfreq_polling_interval, 0444, show_polling_interval, NULL);
+static DEVICE_ATTR(devfreq_set_freq, 0644, show_user_frequency,
+		   set_user_frequency);
+static struct attribute *dev_entries[] = {
+	&dev_attr_devfreq_governor.attr,
+	&dev_attr_devfreq_cur_freq.attr,
+	&dev_attr_devfreq_max_freq.attr,
+	&dev_attr_devfreq_min_freq.attr,
+	&dev_attr_devfreq_polling_interval.attr,
+	&dev_attr_devfreq_set_freq.attr,
+	NULL,
+};
+static struct attribute_group dev_attr_group = {
+	.name	= power_group_name,
+	.attrs	= dev_entries,
+};
+
 /**
  * devfreq_init() - Initialize data structure for devfreq framework and
  *		  start polling registered devfreq devices.
-- 
1.7.4.1

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

* Re: [PATCH v6 1/4] PM / OPP: Add OPP availability change notifier.
  2011-08-18 10:27 ` [PATCH v6 1/4] PM / OPP: Add OPP availability change notifier MyungJoo Ham
@ 2011-08-18 22:06   ` Turquette, Mike
  2011-08-19  1:42     ` MyungJoo Ham
  2011-08-19  1:51     ` [PATCH v6-resubmit of " MyungJoo Ham
  0 siblings, 2 replies; 21+ messages in thread
From: Turquette, Mike @ 2011-08-18 22:06 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Thu, Aug 18, 2011 at 3:27 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> The patch enables to register notifier_block for an OPP-device in order
> to get notified for any changes in the availability of OPPs of the
> device. For example, if a new OPP is inserted or enable/disable status
> of an OPP is changed, the notifier is executed.
>
> This enables the usage of opp_add, opp_enable, and opp_disable to
> directly take effect with any connected entities such as CPUFREQ or
> DEVFREQ.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> ---
> Added at devfreq patch set v6 replacing devfreq_update calls at OPP.
> ---
>  drivers/base/power/opp.c |   28 ++++++++++++++++++++++++++++
>  include/linux/opp.h      |   12 ++++++++++++
>  2 files changed, 40 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index b23de18..39e16c3 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -73,6 +73,7 @@ struct opp {
>  *             RCU usage: nodes are not modified in the list of device_opp,
>  *             however addition is possible and is secured by dev_opp_list_lock
>  * @dev:       device pointer
> + * @head:      notifier head to notify the OPP availability changes.
>  * @opp_list:  list of opps
>  *
>  * This is an internal data structure maintaining the link to opps attached to
> @@ -83,6 +84,8 @@ struct device_opp {
>        struct list_head node;
>
>        struct device *dev;
> +       struct srcu_notifier_head head;
> +
>        struct list_head opp_list;
>  };
>
> @@ -404,6 +407,7 @@ int opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
>                }
>
>                dev_opp->dev = dev;
> +               srcu_init_notifier_head(&dev_opp->head);
>                INIT_LIST_HEAD(&dev_opp->opp_list);
>
>                /* Secure the device list modification */
> @@ -428,6 +432,11 @@ int opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
>        list_add_rcu(&new_opp->node, head);
>        mutex_unlock(&dev_opp_list_lock);
>
> +       /*
> +        * Notify the changes in the availability of the operable
> +        * frequency/voltage list.
> +        */
> +       srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_ADD, new_opp);
>        return 0;
>  }
>
> @@ -504,6 +513,11 @@ static int opp_set_availability(struct device *dev, unsigned long freq,
>        mutex_unlock(&dev_opp_list_lock);
>        synchronize_rcu();

I'm not an RCU expert.  Is the above synchronize_rcu call still needed
with the addition of the srcu_notifier_call_chain below?  The newly
added src_notifier_call_chain will result in the following call graph:

srcu_notifier_call_chain
 -> __srcu_notifier_call_chain
    -> srcu_read_lock
        -> __srcu_read_lock
            -> srcu_barrier

>
> +       /* Notify the change of the OPP availability */
> +       srcu_notifier_call_chain(&dev_opp->head, availability_req ?
> +                                OPP_EVENT_ENABLE : OPP_EVENT_DISABLE,

Nitpick: I'm not a fan of conditional statements as func parameters.

> +                                new_opp);
> +
>        /* clean up old opp */
>        new_opp = opp;
>        goto out;
> @@ -512,6 +526,7 @@ unlock:
>        mutex_unlock(&dev_opp_list_lock);
>  out:
>        kfree(new_opp);
> +

Nitpick: unnecessary whitespace addition.

>        return r;
>  }
>
> @@ -643,3 +658,16 @@ void opp_free_cpufreq_table(struct device *dev,
>        *table = NULL;
>  }
>  #endif         /* CONFIG_CPU_FREQ */
> +
> +/** opp_get_notifier() - find notifier_head of the device with opp
> + * @dev:       device pointer used to lookup device OPPs.
> + */
> +struct srcu_notifier_head *opp_get_notifier(struct device *dev)
> +{
> +       struct device_opp *dev_opp = find_device_opp(dev);
> +
> +       if (IS_ERR(dev_opp))
> +               return ERR_PTR(PTR_ERR(dev_opp)); /* matching type */
> +
> +       return &dev_opp->head;
> +}
> diff --git a/include/linux/opp.h b/include/linux/opp.h
> index 7020e97..53d4538 100644
> --- a/include/linux/opp.h
> +++ b/include/linux/opp.h
> @@ -16,9 +16,14 @@
>
>  #include <linux/err.h>
>  #include <linux/cpufreq.h>
> +#include <linux/notifier.h>
>
>  struct opp;
>
> +enum opp_event {
> +       OPP_EVENT_ADD, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE, OPP_EVENT_MAX

I don't see OPP_EVENT_MAX used anywhere else in the code.  What is it's purpose?

Regards,
Mike

> +};
> +
>  #if defined(CONFIG_PM_OPP)
>
>  unsigned long opp_get_voltage(struct opp *opp);
> @@ -40,6 +45,8 @@ int opp_enable(struct device *dev, unsigned long freq);
>
>  int opp_disable(struct device *dev, unsigned long freq);
>
> +struct srcu_notifier_head *opp_get_notifier(struct device *dev);
> +
>  #else
>  static inline unsigned long opp_get_voltage(struct opp *opp)
>  {
> @@ -89,6 +96,11 @@ static inline int opp_disable(struct device *dev, unsigned long freq)
>  {
>        return 0;
>  }
> +
> +struct srcu_notifier_head *opp_get_notifier(struct device *dev)
> +{
> +       return ERR_PTR(-EINVAL);
> +}
>  #endif         /* CONFIG_PM */
>
>  #if defined(CONFIG_CPU_FREQ) && defined(CONFIG_PM_OPP)
> --
> 1.7.4.1
>
>

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

* Re: [PATCH v6 1/4] PM / OPP: Add OPP availability change notifier.
  2011-08-18 22:06   ` Turquette, Mike
@ 2011-08-19  1:42     ` MyungJoo Ham
  2011-08-19  1:51     ` [PATCH v6-resubmit of " MyungJoo Ham
  1 sibling, 0 replies; 21+ messages in thread
From: MyungJoo Ham @ 2011-08-19  1:42 UTC (permalink / raw)
  To: Turquette, Mike
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Fri, Aug 19, 2011 at 7:06 AM, Turquette, Mike <mturquette@ti.com> wrote:
> On Thu, Aug 18, 2011 at 3:27 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>>
>> @@ -504,6 +513,11 @@ static int opp_set_availability(struct device *dev, unsigned long freq,
>>        mutex_unlock(&dev_opp_list_lock);
>>        synchronize_rcu();
>
> I'm not an RCU expert.  Is the above synchronize_rcu call still needed
> with the addition of the srcu_notifier_call_chain below?  The newly
> added src_notifier_call_chain will result in the following call graph:
>
> srcu_notifier_call_chain
>  -> __srcu_notifier_call_chain
>    -> srcu_read_lock
>        -> __srcu_read_lock
>            -> srcu_barrier

Though didn't read thorough kernel/rcutree_plugin.h, synchronize_rcu()
appears to do a lot more than sleepable RCU's srcu_barrier.
srcu_barrier() provides "barrier" for compiler at compile-time and
synchronize_rcu() provides "barrier" for processes at run-time. For
distributed systems (including multi-core systems) in general, I think
using srcu_barrier() instead of synchronize_rcu() is very dangerous.

>
[]
>
> Nitpick: I'm not a fan of conditional statements as func parameters.
>
[]
>
> Nitpick: unnecessary whitespace addition.
>

Ok, I'll revise them.

[]
>>
>> +enum opp_event {
>> +       OPP_EVENT_ADD, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE, OPP_EVENT_MAX
>
> I don't see OPP_EVENT_MAX used anywhere else in the code.  What is it's purpose?
>

It was placed to mark "the end of the list" for opp notifier users.
However, it seems useless. I'll remove it.


I'll resubmit this part of the patchset.


Thanks.

MyungJoo

-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

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

* [PATCH v6-resubmit of 1/4] PM / OPP: Add OPP availability change notifier.
  2011-08-18 22:06   ` Turquette, Mike
  2011-08-19  1:42     ` MyungJoo Ham
@ 2011-08-19  1:51     ` MyungJoo Ham
  2011-08-22 17:26       ` Turquette, Mike
  1 sibling, 1 reply; 21+ messages in thread
From: MyungJoo Ham @ 2011-08-19  1:51 UTC (permalink / raw)
  To: linux-pm, Mike Turquette
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, Thomas Gleixner

The patch enables to register notifier_block for an OPP-device in order
to get notified for any changes in the availability of OPPs of the
device. For example, if a new OPP is inserted or enable/disable status
of an OPP is changed, the notifier is executed.

This enables the usage of opp_add, opp_enable, and opp_disable to
directly take effect with any connected entities such as CPUFREQ or
DEVFREQ.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

---
Changes
- This patch (1/4 of the set) is resubmitted for the code style revise.
- Added at devfreq patch set v6 replacing devfreq_update calls at OPP.
---
 drivers/base/power/opp.c |   29 +++++++++++++++++++++++++++++
 include/linux/opp.h      |   12 ++++++++++++
 2 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
index b23de18..e6b4c89 100644
--- a/drivers/base/power/opp.c
+++ b/drivers/base/power/opp.c
@@ -73,6 +73,7 @@ struct opp {
  *		RCU usage: nodes are not modified in the list of device_opp,
  *		however addition is possible and is secured by dev_opp_list_lock
  * @dev:	device pointer
+ * @head:	notifier head to notify the OPP availability changes.
  * @opp_list:	list of opps
  *
  * This is an internal data structure maintaining the link to opps attached to
@@ -83,6 +84,7 @@ struct device_opp {
 	struct list_head node;
 
 	struct device *dev;
+	struct srcu_notifier_head head;
 	struct list_head opp_list;
 };
 
@@ -404,6 +406,7 @@ int opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
 		}
 
 		dev_opp->dev = dev;
+		srcu_init_notifier_head(&dev_opp->head);
 		INIT_LIST_HEAD(&dev_opp->opp_list);
 
 		/* Secure the device list modification */
@@ -428,6 +431,11 @@ int opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
 	list_add_rcu(&new_opp->node, head);
 	mutex_unlock(&dev_opp_list_lock);
 
+	/*
+	 * Notify the changes in the availability of the operable
+	 * frequency/voltage list.
+	 */
+	srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_ADD, new_opp);
 	return 0;
 }
 
@@ -504,6 +512,14 @@ static int opp_set_availability(struct device *dev, unsigned long freq,
 	mutex_unlock(&dev_opp_list_lock);
 	synchronize_rcu();
 
+	/* Notify the change of the OPP availability */
+	if (availability_req)
+		srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_ENABLE,
+					 new_opp);
+	else
+		srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_DISABLE,
+					 new_opp);
+
 	/* clean up old opp */
 	new_opp = opp;
 	goto out;
@@ -643,3 +659,16 @@ void opp_free_cpufreq_table(struct device *dev,
 	*table = NULL;
 }
 #endif		/* CONFIG_CPU_FREQ */
+
+/** opp_get_notifier() - find notifier_head of the device with opp
+ * @dev:	device pointer used to lookup device OPPs.
+ */
+struct srcu_notifier_head *opp_get_notifier(struct device *dev)
+{
+	struct device_opp *dev_opp = find_device_opp(dev);
+
+	if (IS_ERR(dev_opp))
+		return ERR_PTR(PTR_ERR(dev_opp)); /* matching type */
+
+	return &dev_opp->head;
+}
diff --git a/include/linux/opp.h b/include/linux/opp.h
index 7020e97..87a9208 100644
--- a/include/linux/opp.h
+++ b/include/linux/opp.h
@@ -16,9 +16,14 @@
 
 #include <linux/err.h>
 #include <linux/cpufreq.h>
+#include <linux/notifier.h>
 
 struct opp;
 
+enum opp_event {
+	OPP_EVENT_ADD, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE,
+};
+
 #if defined(CONFIG_PM_OPP)
 
 unsigned long opp_get_voltage(struct opp *opp);
@@ -40,6 +45,8 @@ int opp_enable(struct device *dev, unsigned long freq);
 
 int opp_disable(struct device *dev, unsigned long freq);
 
+struct srcu_notifier_head *opp_get_notifier(struct device *dev);
+
 #else
 static inline unsigned long opp_get_voltage(struct opp *opp)
 {
@@ -89,6 +96,11 @@ static inline int opp_disable(struct device *dev, unsigned long freq)
 {
 	return 0;
 }
+
+struct srcu_notifier_head *opp_get_notifier(struct device *dev)
+{
+	return ERR_PTR(-EINVAL);
+}
 #endif		/* CONFIG_PM */
 
 #if defined(CONFIG_CPU_FREQ) && defined(CONFIG_PM_OPP)
-- 
1.7.4.1

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

* Re: [PATCH v6 2/4] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs
  2011-08-18 10:27 ` [PATCH v6 2/4] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs MyungJoo Ham
@ 2011-08-19  2:43   ` Turquette, Mike
  2011-08-19  8:29     ` MyungJoo Ham
  2011-08-19  8:30     ` [PATCH v6-resubmit of " MyungJoo Ham
  0 siblings, 2 replies; 21+ messages in thread
From: Turquette, Mike @ 2011-08-19  2:43 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Thu, Aug 18, 2011 at 3:27 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> With OPPs, a device may have multiple operable frequency and voltage
> sets. However, there can be multiple possible operable sets and a system
> will need to choose one from them. In order to reduce the power
> consumption (by reducing frequency and voltage) without affecting the
> performance too much, a Dynamic Voltage and Frequency Scaling (DVFS)
> scheme may be used.
>
> This patch introduces the DVFS capability to non-CPU devices with OPPs.
> DVFS is a techique whereby the frequency and supplied voltage of a
> device is adjusted on-the-fly. DVFS usually sets the frequency as low
> as possible with given conditions (such as QoS assurance) and adjusts
> voltage according to the chosen frequency in order to reduce power
> consumption and heat dissipation.
>
> The generic DVFS for devices, DEVFREQ, may appear quite similar with
> /drivers/cpufreq.  However, CPUFREQ does not allow to have multiple
> devices registered and is not suitable to have multiple heterogenous
> devices with different (but simple) governors.
>
> Normally, DVFS mechanism controls frequency based on the demand for
> the device, and then, chooses voltage based on the chosen frequency.
> DEVFREQ also controls the frequency based on the governor's frequency
> recommendation and let OPP pick up the pair of frequency and voltage
> based on the recommended frequency. Then, the chosen OPP is passed to
> device driver's "target" callback.
>
> When PM QoS is going to be used with the DEVFREQ device, the device
> driver should enable OPPs that are appropriate with the current PM QoS
> requests. In order to do so, the device driver may call opp_enable and
> opp_disable at the notifier callback of PM QoS so that PM QoS's
> update_target() call enables the appropriate OPPs. Note that at least
> one of OPPs should be enabled at any time; be careful when there is a
> transition.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> ---
> The test code with board support for Exynos4-NURI is at
> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq
>
> ---
> Thank you for your valuable comments, Rafael, Greg, Pavel, Colin, Mike,
> and Kevin.
>
> Changes from v5
> - Uses OPP availability change notifier
> - Removed devfreq_interval. Uses one jiffy instead. DEVFREQ adjusts
>  polling interval based on the interval requirement of DEVFREQ
>  devices.
> - Moved devfreq to /drivers/devfreq to accomodate devfreq-related files
>  including governors and devfreq drivers.
> - Coding style revised.
> - Updated devfreq_add_device interface to get tunable values.
>
> Changed from v4
> - Removed tickle, which is a duplicated feature; PM QoS can do the same.
> - Allow to extend polling interval if devices have longer polling intervals.
> - Relocated private data of governors.
> - Removed system-wide sysfs
>
> Changed from v3
> - In kerneldoc comments, DEVFREQ has ben replaced by devfreq
> - Revised removing devfreq entries with error mechanism
> - Added and revised comments
> - Removed unnecessary codes
> - Allow to give a name to a governor
> - Bugfix: a tickle call may cancel an older tickle call that is still in
>  effect.
>
> Changed from v2
> - Code style revised and cleaned up.
> - Remove DEVFREQ entries that incur errors except for EAGAIN
> - Bug fixed: tickle for devices without polling governors
>
> Changes from v1(RFC)
> - Rename: DVFS --> DEVFREQ
> - Revised governor design
>        . Governor receives the whole struct devfreq
>        . Governor should gather usage information (thru get_dev_status) itself
> - Periodic monitoring runs only when needed.
> - DEVFREQ no more deals with voltage information directly
> - Removed some printks.
> - Some cosmetics update
> - Use freezable_wq.
> ---
>  drivers/Kconfig           |    2 +
>  drivers/Makefile          |    2 +
>  drivers/devfreq/Kconfig   |   39 ++++++
>  drivers/devfreq/Makefile  |    1 +
>  drivers/devfreq/devfreq.c |  302 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/devfreq.h   |  105 ++++++++++++++++
>  6 files changed, 451 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/devfreq/Kconfig
>  create mode 100644 drivers/devfreq/Makefile
>  create mode 100644 drivers/devfreq/devfreq.c
>  create mode 100644 include/linux/devfreq.h
>
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 95b9e7e..a1efd75 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -130,4 +130,6 @@ source "drivers/iommu/Kconfig"
>
>  source "drivers/virt/Kconfig"
>
> +source "drivers/devfreq/Kconfig"
> +
>  endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 7fa433a..97c957b 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -127,3 +127,5 @@ obj-$(CONFIG_IOMMU_SUPPORT) += iommu/
>
>  # Virtualization drivers
>  obj-$(CONFIG_VIRT_DRIVERS)     += virt/
> +
> +obj-$(CONFIG_PM_DEVFREQ)       += devfreq/
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> new file mode 100644
> index 0000000..1fb42de
> --- /dev/null
> +++ b/drivers/devfreq/Kconfig
> @@ -0,0 +1,39 @@
> +config ARCH_HAS_DEVFREQ
> +       bool
> +       depends on ARCH_HAS_OPP
> +       help
> +         Denotes that the architecture supports DEVFREQ. If the architecture
> +         supports multiple OPP entries per device and the frequency of the
> +         devices with OPPs may be altered dynamically, the architecture
> +         supports DEVFREQ.
> +
> +menuconfig PM_DEVFREQ
> +       bool "Generic Dynamic Voltage and Frequency Scaling (DVFS) support"
> +       depends on PM_OPP && ARCH_HAS_DEVFREQ
> +       help
> +         With OPP support, a device may have a list of frequencies and
> +         voltages available. DEVFREQ, a generic DVFS framework can be
> +         registered for a device with OPP support in order to let the
> +         governor provided to DEVFREQ choose an operating frequency
> +         based on the OPP's list and the policy given with DEVFREQ.
> +
> +         Each device may have its own governor and policy. DEVFREQ can
> +         reevaluate the device state periodically and/or based on the
> +         OPP list changes (each frequency/voltage pair in OPP may be
> +         disabled or enabled).
> +
> +         Like some CPUs with CPUFREQ, a device may have multiple clocks.
> +         However, because the clock frequencies of a single device are
> +         determined by the single device's state, an instance of DEVFREQ
> +         is attached to a single device and returns a "representative"
> +         clock frequency from the OPP of the device, which is also attached
> +         to a device by 1-to-1. The device registering DEVFREQ takes the
> +         responsiblity to "interpret" the frequency listed in OPP and
> +         to set its every clock accordingly with the "target" callback
> +         given to DEVFREQ.
> +
> +if PM_DEVFREQ
> +
> +comment "DEVFREQ Drivers"
> +
> +endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> new file mode 100644
> index 0000000..168934a
> --- /dev/null
> +++ b/drivers/devfreq/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_PM_DEVFREQ)       += devfreq.o
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> new file mode 100644
> index 0000000..2036f2c
> --- /dev/null
> +++ b/drivers/devfreq/devfreq.c
> @@ -0,0 +1,302 @@
> +/*
> + * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
> + *         for Non-CPU Devices Based on OPP.
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * 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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/opp.h>
> +#include <linux/devfreq.h>
> +#include <linux/workqueue.h>
> +#include <linux/platform_device.h>
> +#include <linux/list.h>
> +#include <linux/printk.h>
> +#include <linux/hrtimer.h>
> +
> +/*
> + * devfreq_work periodically monitors every registered device.
> + * The minimum polling interval is one jiffy. The polling interval is
> + * determined by the minimum polling period among all polling devfreq
> + * devices. The resolution of polling interval is one jiffy.
> + */
> +static bool polling;
> +static struct workqueue_struct *devfreq_wq;
> +static struct delayed_work devfreq_work;
> +
> +/* The list of all device-devfreq */
> +static LIST_HEAD(devfreq_list);
> +static DEFINE_MUTEX(devfreq_list_lock);
> +
> +/**
> + * find_device_devfreq() - find devfreq struct using device pointer
> + * @dev:       device pointer used to lookup device devfreq.
> + *
> + * Search the list of device devfreqs and return the matched device's
> + * devfreq info. devfreq_list_lock should be held by the caller.
> + */
> +static struct devfreq *find_device_devfreq(struct device *dev)
> +{
> +       struct devfreq *tmp_devfreq;
> +
> +       if (unlikely(IS_ERR_OR_NULL(dev))) {
> +               pr_err("DEVFREQ: %s: Invalid parameters\n", __func__);
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       list_for_each_entry(tmp_devfreq, &devfreq_list, node) {
> +               if (tmp_devfreq->dev == dev)
> +                       return tmp_devfreq;
> +       }
> +
> +       return ERR_PTR(-ENODEV);
> +}
> +
> +/**
> + * devfreq_do() - Check the usage profile of a given device and configure
> + *             frequency and voltage accordingly
> + * @devfreq:   devfreq info of the given device
> + */
> +static int devfreq_do(struct devfreq *devfreq)
> +{
> +       struct opp *opp;
> +       unsigned long freq;
> +       int err;
> +
> +       err = devfreq->governor->get_target_freq(devfreq, &freq);
> +       if (err)
> +               return err;
> +
> +       opp = opp_find_freq_ceil(devfreq->dev, &freq);
> +       if (opp == ERR_PTR(-ENODEV))
> +               opp = opp_find_freq_floor(devfreq->dev, &freq);
> +
> +       if (IS_ERR(opp))
> +               return PTR_ERR(opp);
> +
> +       if (devfreq->previous_freq == freq)
> +               return 0;
> +
> +       err = devfreq->profile->target(devfreq->dev, opp);
> +       if (err)
> +               return err;
> +
> +       devfreq->previous_freq = freq;
> +       return 0;
> +}
> +
> +/**
> + * devfreq_update() - Notify that the device OPP has been changed.
> + * @dev:       the device whose OPP has been changed.
> + */
> +static int devfreq_update(struct notifier_block *nb, unsigned long type,
> +                         void *devp)
> +{
> +       struct devfreq *devfreq;
> +       int err = 0;
> +
> +       mutex_lock(&devfreq_list_lock);
> +       devfreq = container_of(nb, struct devfreq, nb);
> +       /* Reevaluate the proper frequency */
> +       err = devfreq_do(devfreq);
> +       mutex_unlock(&devfreq_list_lock);
> +       return err;
> +}
> +
> +/**
> + * devfreq_monitor() - Periodically run devfreq_do()
> + * @work: the work struct used to run devfreq_monitor periodically.
> + *
> + */
> +static void devfreq_monitor(struct work_struct *work)
> +{
> +       static ktime_t last_polled_at;
> +       struct devfreq *devfreq, *tmp;
> +       int error;
> +       unsigned int next_jiffies = UINT_MAX;
> +       ktime_t now = ktime_get();
> +       int jiffies_passed;
> +
> +       /* Initially last_polled_at = 0, polling every device at bootup */
> +       jiffies_passed = msecs_to_jiffies(ktime_to_ms(
> +                               ktime_sub(now, last_polled_at)));

Better to deal natively in jiffies instead of doing conversions of
conversions.  Make last_polled_at an unsigned long and then
jiffies_passed just becomes jiffies - last_polled_at.

I still think that the timekeeping here is gross.  last_polled_at
should not be static.  Instead governors should track the timekeeping
info themselves (for now could live in struct devfreq_governor).  I
won't NACK the patch b/c of that, but I think that the code will have
to become more modularized in the future if devfreq sees wide
adoption.

> +       last_polled_at = now;
> +
> +       if (jiffies_passed == 0)
> +               jiffies_passed = 1;
> +       if (jiffies_passed < 0) /* "Infinite Timeout" */
> +               jiffies_passed = INT_MAX;
> +
> +       mutex_lock(&devfreq_list_lock);
> +
> +       list_for_each_entry_safe(devfreq, tmp, &devfreq_list, node) {
> +               if (devfreq->next_polling == 0)
> +                       continue;
> +
> +               /*
> +                * Reduce more next_polling if devfreq_wq took an extra
> +                * delay. (i.e., CPU has been idled.)
> +                */
> +               if (devfreq->next_polling <= jiffies_passed) {
> +                       error = devfreq_do(devfreq);
> +
> +                       /* Remove a devfreq with an error. */
> +                       if (error && error != -EAGAIN) {
> +                               dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
> +                                       error, devfreq->governor->name);
> +
> +                               list_del(&devfreq->node);
> +                               kfree(devfreq);
> +
> +                               continue;
> +                       }
> +                       devfreq->next_polling = msecs_to_jiffies(
> +                                               devfreq->profile->polling_ms);

Similar to the above, it's better to deal natively in jiffies instead
of doing a conversion every time.  When a polling interval is
specified at init-time, or via sysfs it should be in milliseconds, but
the storage should be jiffies to avoid conversions every single loop
through this code path.

Regards,
Mike

> +
> +                       /* No more polling required (polling_ms changed) */
> +                       if (devfreq->next_polling == 0)
> +                               continue;
> +               } else {
> +                       devfreq->next_polling -= jiffies_passed;
> +               }
> +
> +               next_jiffies = (next_jiffies > devfreq->next_polling) ?
> +                               devfreq->next_polling : next_jiffies;
> +       }
> +
> +       if (next_jiffies > 0 && next_jiffies < UINT_MAX) {
> +               polling = true;
> +               queue_delayed_work(devfreq_wq, &devfreq_work, next_jiffies);
> +       } else {
> +               polling = false;
> +       }
> +
> +       mutex_unlock(&devfreq_list_lock);
> +}
> +
> +/**
> + * devfreq_add_device() - Add devfreq feature to the device
> + * @dev:       the device to add devfreq feature.
> + * @profile:   device-specific profile to run devfreq.
> + * @governor:  the policy to choose frequency.
> + * @data:      private data for the governor. The devfreq framework does not
> + *             touch this value.
> + */
> +int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
> +                      struct devfreq_governor *governor, void *data)
> +{
> +       struct devfreq *devfreq;
> +       struct srcu_notifier_head *nh;
> +       int err = 0;
> +
> +       if (!dev || !profile || !governor) {
> +               dev_err(dev, "%s: Invalid parameters.\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       mutex_lock(&devfreq_list_lock);
> +
> +       devfreq = find_device_devfreq(dev);
> +       if (!IS_ERR(devfreq)) {
> +               dev_err(dev, "%s: Unable to create devfreq for the device. It already has one.\n", __func__);
> +               err = -EINVAL;
> +               goto out;
> +       }
> +
> +       devfreq = kzalloc(sizeof(struct devfreq), GFP_KERNEL);
> +       if (!devfreq) {
> +               dev_err(dev, "%s: Unable to create devfreq for the device\n",
> +                       __func__);
> +               err = -ENOMEM;
> +               goto out;
> +       }
> +
> +       devfreq->dev = dev;
> +       devfreq->profile = profile;
> +       devfreq->governor = governor;
> +       devfreq->next_polling = msecs_to_jiffies(profile->polling_ms);
> +       devfreq->previous_freq = profile->initial_freq;
> +       devfreq->data = data;
> +
> +       devfreq->nb.notifier_call = devfreq_update;
> +       nh = opp_get_notifier(dev);
> +       if (IS_ERR(nh)) {
> +               err = PTR_ERR(nh);
> +               goto out;
> +       }
> +       err = srcu_notifier_chain_register(nh, &devfreq->nb);
> +       if (err)
> +               goto out;
> +
> +       list_add(&devfreq->node, &devfreq_list);
> +
> +       if (devfreq_wq && devfreq->next_polling && !polling) {
> +               polling = true;
> +               queue_delayed_work(devfreq_wq, &devfreq_work,
> +                                  devfreq->next_polling);
> +       }
> +out:
> +       mutex_unlock(&devfreq_list_lock);
> +
> +       return err;
> +}
> +
> +/**
> + * devfreq_remove_device() - Remove devfreq feature from a device.
> + * @device:    the device to remove devfreq feature.
> + */
> +int devfreq_remove_device(struct device *dev)
> +{
> +       struct devfreq *devfreq;
> +       struct srcu_notifier_head *nh;
> +       int err = 0;
> +
> +       if (!dev)
> +               return -EINVAL;
> +
> +       mutex_lock(&devfreq_list_lock);
> +       devfreq = find_device_devfreq(dev);
> +       if (IS_ERR(devfreq)) {
> +               err = PTR_ERR(devfreq);
> +               goto out;
> +       }
> +
> +       nh = opp_get_notifier(dev);
> +       if (IS_ERR(nh)) {
> +               err = PTR_ERR(nh);
> +               goto out;
> +       }
> +
> +       list_del(&devfreq->node);
> +       srcu_notifier_chain_unregister(nh, &devfreq->nb);
> +       kfree(devfreq);
> +out:
> +       mutex_unlock(&devfreq_list_lock);
> +       return 0;
> +}
> +
> +/**
> + * devfreq_init() - Initialize data structure for devfreq framework and
> + *               start polling registered devfreq devices.
> + */
> +static int __init devfreq_init(void)
> +{
> +       mutex_lock(&devfreq_list_lock);
> +       polling = false;
> +       devfreq_wq = create_freezable_workqueue("devfreq_wq");
> +       INIT_DELAYED_WORK_DEFERRABLE(&devfreq_work, devfreq_monitor);
> +       mutex_unlock(&devfreq_list_lock);
> +
> +       devfreq_monitor(&devfreq_work.work);
> +       return 0;
> +}
> +late_initcall(devfreq_init);
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> new file mode 100644
> index 0000000..13ddf49
> --- /dev/null
> +++ b/include/linux/devfreq.h
> @@ -0,0 +1,105 @@
> +/*
> + * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
> + *         for Non-CPU Devices Based on OPP.
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * 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 __LINUX_DEVFREQ_H__
> +#define __LINUX_DEVFREQ_H__
> +
> +#include <linux/notifier.h>
> +
> +#define DEVFREQ_NAME_LEN 16
> +
> +struct devfreq;
> +struct devfreq_dev_status {
> +       /* both since the last measure */
> +       unsigned long total_time;
> +       unsigned long busy_time;
> +       unsigned long current_frequency;
> +};
> +
> +struct devfreq_dev_profile {
> +       unsigned long max_freq; /* may be larger than the actual value */
> +       unsigned long initial_freq;
> +       int polling_ms; /* 0 for at opp change only */
> +
> +       int (*target)(struct device *dev, struct opp *opp);
> +       int (*get_dev_status)(struct device *dev,
> +                             struct devfreq_dev_status *stat);
> +};
> +
> +/**
> + * struct devfreq_governor - Devfreq policy governor
> + * @name               Governor's name
> + * @get_target_freq    Returns desired operating frequency for the device.
> + *                     Basically, get_target_freq will run
> + *                     devfreq_dev_profile.get_dev_status() to get the
> + *                     status of the device (load = busy_time / total_time).
> + */
> +struct devfreq_governor {
> +       char name[DEVFREQ_NAME_LEN];
> +       int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
> +};
> +
> +/**
> + * struct devfreq - Device devfreq structure
> + * @node       list node - contains the devices with devfreq that have been
> + *             registered.
> + * @dev                device pointer
> + * @profile    device-specific devfreq profile
> + * @governor   method how to choose frequency based on the usage.
> + * @nb         notifier block registered to the corresponding OPP to get
> + *             notified for frequency availability updates.
> + * @previous_freq      previously configured frequency value.
> + * @next_polling       the number of remaining jiffies to poll with
> + *                     "devfreq_monitor" executions to reevaluate
> + *                     frequency/voltage of the device. Set by
> + *                     profile's polling_ms interval.
> + * @data       Private data of the governor. The devfreq framework does not
> + *             touch this.
> + *
> + * This structure stores the devfreq information for a give device.
> + */
> +struct devfreq {
> +       struct list_head node;
> +
> +       struct device *dev;
> +       struct devfreq_dev_profile *profile;
> +       struct devfreq_governor *governor;
> +       struct notifier_block nb;
> +
> +       unsigned long previous_freq;
> +       unsigned int next_polling;
> +
> +       void *data; /* private data for governors */
> +};
> +
> +#if defined(CONFIG_PM_DEVFREQ)
> +extern int devfreq_add_device(struct device *dev,
> +                          struct devfreq_dev_profile *profile,
> +                          struct devfreq_governor *governor,
> +                          void *data);
> +extern int devfreq_remove_device(struct device *dev);
> +#else /* !CONFIG_PM_DEVFREQ */
> +static int devfreq_add_device(struct device *dev,
> +                          struct devfreq_dev_profile *profile,
> +                          struct devfreq_governor *governor,
> +                          void *data)
> +{
> +       return 0;
> +}
> +
> +static int devfreq_remove_device(struct device *dev)
> +{
> +       return 0;
> +}
> +#endif /* CONFIG_PM_DEVFREQ */
> +
> +#endif /* __LINUX_DEVFREQ_H__ */
> --
> 1.7.4.1
>
>

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

* Re: [PATCH v6 3/4] PM / DEVFREQ: add basic governors
  2011-08-18 10:27 ` [PATCH v6 3/4] PM / DEVFREQ: add basic governors MyungJoo Ham
@ 2011-08-19  3:12   ` Turquette, Mike
  2011-08-19  5:58     ` MyungJoo Ham
  0 siblings, 1 reply; 21+ messages in thread
From: Turquette, Mike @ 2011-08-19  3:12 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Thu, Aug 18, 2011 at 3:27 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> Four CPUFREQ-like governors are provided as examples.
>
> powersave: use the lowest frequency possible. The user (device) should
> set the polling_ms as 0 because polling is useless for this governor.
>
> performance: use the highest freqeuncy possible. The user (device)
> should set the polling_ms as 0 because polling is useless for this
> governor.
>
> userspace: use the user specified frequency stored at
> devfreq.user_set_freq. With sysfs support in the following patch, a user
> may set the value with the sysfs interface.
>
> simple_ondemand: simplified version of CPUFREQ's ONDEMAND governor.

In patchset v5 I requested for sysfs entries to control
up-/down-thresholds for simple-ondemand.  Those still are not here;
but that got me thinking... what about multiple devices that want to
use simple-ondemand but have different up/down thresholds?  That's
when I realized that this code is monolithic and will never scale.  It
works today because there is one device on one platform using it.

Below code snippet is from the Nuri code that initializes
devfreq/simple-ondemand for memory bus:
(http://git.infradead.org/users/kmpark/linux-2.6-samsung/commitdiff/d9696269b79ca172474b2c1ae84150e8ac276542?hp=7dbd341e0780ab59b2aeb85e2691ddfb1ab2fd3a)

+static struct devfreq_dev_profile exynos4_devfreq_profile = {
+       .max_freq       = 400000, /* 400MHz */
+       .initial_freq   = 400000,
+       .polling_ms     = 50,
+       .target         = exynos4_bus_target,
+       .get_dev_status = exynos4_bus_get_dev_status,
+};
+static struct devfreq_governor *default_gov = &devfreq_simple_ondemand;

This is pretty bad.  Basically only one instance of simple-ondemand
exists and all devices must use it.  There should really be one
governor instance per device.  Later there might be some optimizations
around grouping multiple devices together that need to scale
simultaneously, but lets not concern ourselves with that now.

struct devfreq's governor member should point to a unique instance of
the governor.  That governor's structure should contain all of the
per-governor tunables:

timekeeping (last_polled_at, etc)
up_threshold
down_threshold
etc.

This in fact touches upon my previous complaints about the core code
managing too many of the governors responsibilities.

Regards,
Mike

> When a user updates OPP entries (enable/disable/add), OPP framework
> automatically notifies DEVFREQ to update operating frequency
> accordingly. Thus, DEVFREQ users (device drivers) do not need to update
> DEVFREQ manually with OPP entry updates or set polling_ms for powersave
> , performance, userspace, or any other "static" governors.
>
> Note that these are given only as basic examples for governors and any
> devices with DEVFREQ may implement their own governors with the drivers
> and use them.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> ---
> Changed from v5
> - Seperated governor files from devfreq.c
> - Allow simple ondemand to be tuned for each device
> ---
>  drivers/devfreq/Kconfig                   |   36 ++++++++++++
>  drivers/devfreq/Makefile                  |    4 +
>  drivers/devfreq/governor_performance.c    |   24 ++++++++
>  drivers/devfreq/governor_powersave.c      |   24 ++++++++
>  drivers/devfreq/governor_simpleondemand.c |   88 +++++++++++++++++++++++++++++
>  drivers/devfreq/governor_userspace.c      |   27 +++++++++
>  include/linux/devfreq.h                   |   41 +++++++++++++
>  7 files changed, 244 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/devfreq/governor_performance.c
>  create mode 100644 drivers/devfreq/governor_powersave.c
>  create mode 100644 drivers/devfreq/governor_simpleondemand.c
>  create mode 100644 drivers/devfreq/governor_userspace.c
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 1fb42de..643b055 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -34,6 +34,42 @@ menuconfig PM_DEVFREQ
>
>  if PM_DEVFREQ
>
> +comment "DEVFREQ Governors"
> +
> +config DEVFREQ_GOV_SIMPLE_ONDEMAND
> +       bool "Simple Ondemand"
> +       help
> +         Chooses frequency based on the recent load on the device. Works
> +         similar as ONDEMAND governor of CPUFREQ does. A device with
> +         Simple-Ondemand should be able to provide busy/total counter
> +         values that imply the usage rate. A device may provide tuned
> +         values to the governor with data field at devfreq_add_device().
> +
> +config DEVFREQ_GOV_PERFORMANCE
> +       bool "Performance"
> +       help
> +         Sets the frequency at the maximum available frequency.
> +         This governor always returns UINT_MAX as frequency so that
> +         the DEVFREQ framework returns the highest frequency available
> +         at any time.
> +
> +config DEVFREQ_GOV_POWERSAVE
> +       bool "Powersave"
> +       help
> +         Sets the frequency at the minimum available frequency.
> +         This governor always returns 0 as frequency so that
> +         the DEVFREQ framework returns the lowest frequency available
> +         at any time.
> +
> +config DEVFREQ_GOV_USERSPACE
> +       bool "Userspace"
> +       help
> +         Sets the frequency at the user specified one.
> +         This governor returns the user configured frequency if there
> +         has been an input to /sys/devices/.../power/devfreq_set_freq.
> +         Otherwise, the governor does not change the frequnecy
> +         given at the initialization.
> +
>  comment "DEVFREQ Drivers"
>
>  endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> index 168934a..4564a89 100644
> --- a/drivers/devfreq/Makefile
> +++ b/drivers/devfreq/Makefile
> @@ -1 +1,5 @@
>  obj-$(CONFIG_PM_DEVFREQ)       += devfreq.o
> +obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)      += governor_simpleondemand.o
> +obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)  += governor_performance.o
> +obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)    += governor_powersave.o
> +obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)    += governor_userspace.o
> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
> new file mode 100644
> index 0000000..c47eff8
> --- /dev/null
> +++ b/drivers/devfreq/governor_performance.c
> @@ -0,0 +1,24 @@
> +/*
> + *  linux/drivers/devfreq/governor_performance.c
> + *
> + *  Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * 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.
> + */
> +
> +#include <linux/devfreq.h>
> +
> +static int devfreq_performance_func(struct devfreq *df,
> +                                   unsigned long *freq)
> +{
> +       *freq = UINT_MAX; /* devfreq_do will run "floor" */
> +       return 0;
> +}
> +
> +struct devfreq_governor devfreq_performance = {
> +       .name = "performance",
> +       .get_target_freq = devfreq_performance_func,
> +};
> diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
> new file mode 100644
> index 0000000..4f128d8
> --- /dev/null
> +++ b/drivers/devfreq/governor_powersave.c
> @@ -0,0 +1,24 @@
> +/*
> + *  linux/drivers/devfreq/governor_powersave.c
> + *
> + *  Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * 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.
> + */
> +
> +#include <linux/devfreq.h>
> +
> +static int devfreq_powersave_func(struct devfreq *df,
> +                                 unsigned long *freq)
> +{
> +       *freq = 0; /* devfreq_do will run "ceiling" to 0 */
> +       return 0;
> +}
> +
> +struct devfreq_governor devfreq_powersave = {
> +       .name = "powersave",
> +       .get_target_freq = devfreq_powersave_func,
> +};
> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> new file mode 100644
> index 0000000..18fe8be
> --- /dev/null
> +++ b/drivers/devfreq/governor_simpleondemand.c
> @@ -0,0 +1,88 @@
> +/*
> + *  linux/drivers/devfreq/governor_simpleondemand.c
> + *
> + *  Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * 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.
> + */
> +
> +#include <linux/errno.h>
> +#include <linux/devfreq.h>
> +#include <linux/math64.h>
> +
> +/* Default constants for DevFreq-Simple-Ondemand (DFSO) */
> +#define DFSO_UPTHRESHOLD       (90)
> +#define DFSO_DOWNDIFFERENCTIAL (5)
> +static int devfreq_simple_ondemand_func(struct devfreq *df,
> +                                       unsigned long *freq)
> +{
> +       struct devfreq_dev_status stat;
> +       int err = df->profile->get_dev_status(df->dev, &stat);
> +       unsigned long long a, b;
> +       unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
> +       unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
> +       struct devfreq_simple_ondemand_data *data = df->data;
> +
> +       if (err)
> +               return err;
> +
> +       if (data) {
> +               if (data->upthreshold)
> +                       dfso_upthreshold = data->upthreshold;
> +               if (data->downdifferential)
> +                       dfso_downdifferential = data->downdifferential;
> +       }
> +       if (dfso_upthreshold > 100 ||
> +           dfso_upthreshold < dfso_downdifferential)
> +               return -EINVAL;
> +
> +       /* Assume MAX if it is going to be divided by zero */
> +       if (stat.total_time == 0) {
> +               *freq = UINT_MAX;
> +               return 0;
> +       }
> +
> +       /* Prevent overflow */
> +       if (stat.busy_time >= (1 << 24) || stat.total_time >= (1 << 24)) {
> +               stat.busy_time >>= 7;
> +               stat.total_time >>= 7;
> +       }
> +
> +       /* Set MAX if it's busy enough */
> +       if (stat.busy_time * 100 >
> +           stat.total_time * dfso_upthreshold) {
> +               *freq = UINT_MAX;
> +               return 0;
> +       }
> +
> +       /* Set MAX if we do not know the initial frequency */
> +       if (stat.current_frequency == 0) {
> +               *freq = UINT_MAX;
> +               return 0;
> +       }
> +
> +       /* Keep the current frequency */
> +       if (stat.busy_time * 100 >
> +           stat.total_time * (dfso_upthreshold - dfso_downdifferential)) {
> +               *freq = stat.current_frequency;
> +               return 0;
> +       }
> +
> +       /* Set the desired frequency based on the load */
> +       a = stat.busy_time;
> +       a *= stat.current_frequency;
> +       b = div_u64(a, stat.total_time);
> +       b *= 100;
> +       b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
> +       *freq = (unsigned long) b;
> +
> +       return 0;
> +}
> +
> +struct devfreq_governor devfreq_simple_ondemand = {
> +       .name = "simple_ondemand",
> +       .get_target_freq = devfreq_simple_ondemand_func,
> +};
> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
> new file mode 100644
> index 0000000..6af8b6d
> --- /dev/null
> +++ b/drivers/devfreq/governor_userspace.c
> @@ -0,0 +1,27 @@
> +/*
> + *  linux/drivers/devfreq/governor_simpleondemand.c
> + *
> + *  Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * 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.
> + */
> +
> +#include <linux/devfreq.h>
> +
> +static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
> +{
> +       if (df->user_set_freq == 0)
> +               *freq = df->previous_freq; /* No user freq specified yet */
> +       else
> +               *freq = df->user_set_freq;
> +
> +       return 0;
> +}
> +
> +struct devfreq_governor devfreq_userspace = {
> +       .name = "userspace",
> +       .get_target_freq = devfreq_userspace_func,
> +};
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 13ddf49..d088ad3 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -13,6 +13,7 @@
>  #ifndef __LINUX_DEVFREQ_H__
>  #define __LINUX_DEVFREQ_H__
>
> +#include <linux/opp.h>
>  #include <linux/notifier.h>
>
>  #define DEVFREQ_NAME_LEN 16
> @@ -62,6 +63,8 @@ struct devfreq_governor {
>  *                     "devfreq_monitor" executions to reevaluate
>  *                     frequency/voltage of the device. Set by
>  *                     profile's polling_ms interval.
> + * @user_set_freq      User specified adequete frequency value (thru sysfs
> + *             interface). Governors may and may not use this value.
>  * @data       Private data of the governor. The devfreq framework does not
>  *             touch this.
>  *
> @@ -78,6 +81,7 @@ struct devfreq {
>        unsigned long previous_freq;
>        unsigned int next_polling;
>
> +       unsigned long user_set_freq; /* governors may ignore this. */
>        void *data; /* private data for governors */
>  };
>
> @@ -87,6 +91,37 @@ extern int devfreq_add_device(struct device *dev,
>                           struct devfreq_governor *governor,
>                           void *data);
>  extern int devfreq_remove_device(struct device *dev);
> +
> +#ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
> +extern struct devfreq_governor devfreq_powersave;
> +#endif
> +#ifdef CONFIG_DEVFREQ_GOV_PERFORMANCE
> +extern struct devfreq_governor devfreq_performance;
> +#endif
> +#ifdef CONFIG_DEVFREQ_GOV_USERSPACE
> +extern struct devfreq_governor devfreq_userspace;
> +#endif
> +#ifdef CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND
> +extern struct devfreq_governor devfreq_simple_ondemand;
> +/**
> + * struct devfreq_simple_ondemand_data - void *data fed to struct devfreq
> + *     and devfreq_add_device
> + * @ upthreshold       If the load is over this value, the frequency jumps.
> + *                     Specify 0 to use the default. Valid value = 0 to 100.
> + * @ downdifferential  If the load is under upthreshold - downdifferential,
> + *                     the governor may consider slowing the frequency down.
> + *                     Specify 0 to use the default. Valid value = 0 to 100.
> + *                     downdifferential < upthreshold must hold.
> + *
> + * If the fed devfreq_simple_ondemand_data pointer is NULL to the governor,
> + * the governor uses the default values.
> + */
> +struct devfreq_simple_ondemand_data {
> +       unsigned int upthreshold;
> +       unsigned int downdifferential;
> +};
> +#endif
> +
>  #else /* !CONFIG_PM_DEVFREQ */
>  static int devfreq_add_device(struct device *dev,
>                           struct devfreq_dev_profile *profile,
> @@ -100,6 +135,12 @@ static int devfreq_remove_device(struct device *dev)
>  {
>        return 0;
>  }
> +
> +#define devfreq_powersave      NULL
> +#define devfreq_performance    NULL
> +#define devfreq_userspace      NULL
> +#define devfreq_simple_ondemand        NULL
> +
>  #endif /* CONFIG_PM_DEVFREQ */
>
>  #endif /* __LINUX_DEVFREQ_H__ */
> --
> 1.7.4.1
>
>

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

* Re: [PATCH v6 4/4] PM / DEVFREQ: add sysfs interface
  2011-08-18 10:27 ` [PATCH v6 4/4] PM / DEVFREQ: add sysfs interface MyungJoo Ham
@ 2011-08-19  3:15   ` Turquette, Mike
  2011-08-19  7:28     ` MyungJoo Ham
  0 siblings, 1 reply; 21+ messages in thread
From: Turquette, Mike @ 2011-08-19  3:15 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Thu, Aug 18, 2011 at 3:27 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> Device specific sysfs interface /sys/devices/.../power/devfreq_*
> - governor      R: name of governor
> - cur_freq      R: current frequency
> - max_freq      R: maximum operable frequency
> - min_freq      R: minimum operable frequency
> - set_freq      R: read user specified frequency (0 if not specified yet)
>                W: set user specified frequency
> - polling_interval      R: polling interval in ms given with devfreq profile

This code still does not export up_threshold/down_threshold entries
for simple-ondemand which I requested in V5.  However there is a much
bigger issue here brought out in patch 3.  Those values shouldn't be
global, since devices might want different thresholds.

The solution is not to put those tunables in struct devfreq.
up_threshold and down_threshold must be parameters of the governor,
and there must be a per-device instance of the governor.

Regards,
Mike

>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> --
> Changed from v5
> - updated devferq_update usage.
>
> Changed from v4
> - removed system-wide sysfs interface
> - removed tickling sysfs interface
> - added set_freq for userspace governor (and any other governors that
>  require user input)
>
> Changed from v3
> - corrected sysfs API usage
> - corrected error messages
> - moved sysfs entry location
> - added sysfs entries
>
> Changed from v2
> - add ABI entries for devfreq sysfs interface
> ---
>  Documentation/ABI/testing/sysfs-devices-power |   45 +++++++
>  drivers/devfreq/devfreq.c                     |  152 +++++++++++++++++++++++++
>  2 files changed, 197 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
> index 8ffbc25..6e77604 100644
> --- a/Documentation/ABI/testing/sysfs-devices-power
> +++ b/Documentation/ABI/testing/sysfs-devices-power
> @@ -165,3 +165,48 @@ Description:
>
>                Not all drivers support this attribute.  If it isn't supported,
>                attempts to read or write it will yield I/O errors.
> +
> +What:          /sys/devices/.../power/devfreq_governor
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_governor shows the name
> +               of the governor used by the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_cur_freq
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_cur_freq shows the current
> +               frequency of the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_max_freq
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_cur_freq shows the
> +               maximum operable frequency of the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_min_freq
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_cur_freq shows the
> +               minimum operable frequency of the corresponding device.
> +
> +What:          /sys/devices/.../power/devfreq_set_freq
> +Date:          August 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_set_freq sets and shows
> +               the user specified desired frequency of the device. The
> +               governor may and may not use the value. With the basic
> +               governors given with devfreq.c, userspace governor is
> +               using the value.
> +
> +What:          /sys/devices/.../power/devfreq_polling_interval
> +Date:          July 2011
> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
> +Description:
> +               The /sys/devices/.../power/devfreq_polling_interval shows the
> +               requested polling interval of the corresponding device.
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 2036f2c..13b422f 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -37,6 +37,8 @@ static struct delayed_work devfreq_work;
>  static LIST_HEAD(devfreq_list);
>  static DEFINE_MUTEX(devfreq_list_lock);
>
> +static struct attribute_group dev_attr_group;
> +
>  /**
>  * find_device_devfreq() - find devfreq struct using device pointer
>  * @dev:       device pointer used to lookup device devfreq.
> @@ -154,6 +156,8 @@ static void devfreq_monitor(struct work_struct *work)
>                                dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
>                                        error, devfreq->governor->name);
>
> +                               sysfs_unmerge_group(&devfreq->dev->kobj,
> +                                                   &dev_attr_group);
>                                list_del(&devfreq->node);
>                                kfree(devfreq);
>
> @@ -244,6 +248,8 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
>                queue_delayed_work(devfreq_wq, &devfreq_work,
>                                   devfreq->next_polling);
>        }
> +
> +       sysfs_merge_group(&dev->kobj, &dev_attr_group);
>  out:
>        mutex_unlock(&devfreq_list_lock);
>
> @@ -276,6 +282,8 @@ int devfreq_remove_device(struct device *dev)
>                goto out;
>        }
>
> +       sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
> +
>        list_del(&devfreq->node);
>        srcu_notifier_chain_unregister(nh, &devfreq->nb);
>        kfree(devfreq);
> @@ -284,6 +292,150 @@ out:
>        return 0;
>  }
>
> +static ssize_t show_governor(struct device *dev,
> +                            struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = find_device_devfreq(dev);
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->governor)
> +               return -EINVAL;
> +
> +       return sprintf(buf, "%s\n", df->governor->name);
> +}
> +
> +static ssize_t show_freq(struct device *dev,
> +                        struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = find_device_devfreq(dev);
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +
> +       return sprintf(buf, "%lu\n", df->previous_freq);
> +}
> +
> +static ssize_t show_max_freq(struct device *dev,
> +                            struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = find_device_devfreq(dev);
> +       unsigned long freq = ULONG_MAX;
> +       struct opp *opp;
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->dev)
> +               return -EINVAL;
> +
> +       opp = opp_find_freq_floor(df->dev, &freq);
> +       if (IS_ERR(opp))
> +               return PTR_ERR(opp);
> +
> +       return sprintf(buf, "%lu\n", freq);
> +}
> +
> +static ssize_t show_min_freq(struct device *dev,
> +                            struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = find_device_devfreq(dev);
> +       unsigned long freq = 0;
> +       struct opp *opp;
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->dev)
> +               return -EINVAL;
> +
> +       opp = opp_find_freq_ceil(df->dev, &freq);
> +       if (IS_ERR(opp))
> +               return PTR_ERR(opp);
> +
> +       return sprintf(buf, "%lu\n", freq);
> +}
> +
> +static ssize_t show_polling_interval(struct device *dev,
> +                                    struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *df = find_device_devfreq(dev);
> +
> +       if (IS_ERR(df))
> +               return PTR_ERR(df);
> +       if (!df->profile)
> +               return -EINVAL;
> +
> +       return sprintf(buf, "%d\n", df->profile->polling_ms);
> +}
> +
> +static ssize_t set_user_frequency(struct device *dev,
> +                                 struct device_attribute *attr,
> +                                 const char *buf, size_t count)
> +{
> +       struct devfreq *devfreq;
> +       unsigned long wanted;
> +       int err = 0;
> +
> +       sscanf(buf, "%lu", &wanted);
> +
> +       mutex_lock(&devfreq_list_lock);
> +       devfreq = find_device_devfreq(dev);
> +
> +       if (IS_ERR(devfreq)) {
> +               err = PTR_ERR(devfreq);
> +               goto out;
> +       }
> +
> +       devfreq->user_set_freq = wanted;
> +       err = count;
> +out:
> +       mutex_unlock(&devfreq_list_lock);
> +       if (err >= 0)
> +               devfreq_update(&devfreq->nb, 0, NULL);
> +       return err;
> +}
> +
> +static ssize_t show_user_frequency(struct device *dev,
> +                                  struct device_attribute *attr, char *buf)
> +{
> +       struct devfreq *devfreq;
> +       int err = 0;
> +
> +       mutex_lock(&devfreq_list_lock);
> +       devfreq = find_device_devfreq(dev);
> +
> +       if (IS_ERR(devfreq)) {
> +               err = PTR_ERR(devfreq);
> +               goto out;
> +       }
> +
> +       err = sprintf(buf, "%lu\n", devfreq->user_set_freq);
> +out:
> +       mutex_unlock(&devfreq_list_lock);
> +       return err;
> +
> +}
> +
> +static DEVICE_ATTR(devfreq_governor, 0444, show_governor, NULL);
> +static DEVICE_ATTR(devfreq_cur_freq, 0444, show_freq, NULL);
> +static DEVICE_ATTR(devfreq_max_freq, 0444, show_max_freq, NULL);
> +static DEVICE_ATTR(devfreq_min_freq, 0444, show_min_freq, NULL);
> +static DEVICE_ATTR(devfreq_polling_interval, 0444, show_polling_interval, NULL);
> +static DEVICE_ATTR(devfreq_set_freq, 0644, show_user_frequency,
> +                  set_user_frequency);
> +static struct attribute *dev_entries[] = {
> +       &dev_attr_devfreq_governor.attr,
> +       &dev_attr_devfreq_cur_freq.attr,
> +       &dev_attr_devfreq_max_freq.attr,
> +       &dev_attr_devfreq_min_freq.attr,
> +       &dev_attr_devfreq_polling_interval.attr,
> +       &dev_attr_devfreq_set_freq.attr,
> +       NULL,
> +};
> +static struct attribute_group dev_attr_group = {
> +       .name   = power_group_name,
> +       .attrs  = dev_entries,
> +};
> +
>  /**
>  * devfreq_init() - Initialize data structure for devfreq framework and
>  *               start polling registered devfreq devices.
> --
> 1.7.4.1
>
>

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

* Re: [PATCH v6 3/4] PM / DEVFREQ: add basic governors
  2011-08-19  3:12   ` Turquette, Mike
@ 2011-08-19  5:58     ` MyungJoo Ham
  2011-08-22 18:22       ` Turquette, Mike
  0 siblings, 1 reply; 21+ messages in thread
From: MyungJoo Ham @ 2011-08-19  5:58 UTC (permalink / raw)
  To: Turquette, Mike
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Fri, Aug 19, 2011 at 12:12 PM, Turquette, Mike <mturquette@ti.com> wrote:
> In patchset v5 I requested for sysfs entries to control
> up-/down-thresholds for simple-ondemand.  Those still are not here;
> but that got me thinking... what about multiple devices that want to
> use simple-ondemand but have different up/down thresholds?  That's
> when I realized that this code is monolithic and will never scale.  It
> works today because there is one device on one platform using it.
>
> Below code snippet is from the Nuri code that initializes
> devfreq/simple-ondemand for memory bus:
> (http://git.infradead.org/users/kmpark/linux-2.6-samsung/commitdiff/d9696269b79ca172474b2c1ae84150e8ac276542?hp=7dbd341e0780ab59b2aeb85e2691ddfb1ab2fd3a)
>
> +static struct devfreq_dev_profile exynos4_devfreq_profile = {
> +       .max_freq       = 400000, /* 400MHz */
> +       .initial_freq   = 400000,
> +       .polling_ms     = 50,
> +       .target         = exynos4_bus_target,
> +       .get_dev_status = exynos4_bus_get_dev_status,
> +};
> +static struct devfreq_governor *default_gov = &devfreq_simple_ondemand;
>
> This is pretty bad.  Basically only one instance of simple-ondemand
> exists and all devices must use it.  There should really be one
> governor instance per device.  Later there might be some optimizations
> around grouping multiple devices together that need to scale
> simultaneously, but lets not concern ourselves with that now.
>
> struct devfreq's governor member should point to a unique instance of
> the governor.  That governor's structure should contain all of the
> per-governor tunables:
>
> timekeeping (last_polled_at, etc)
> up_threshold
> down_threshold
> etc.
>
> This in fact touches upon my previous complaints about the core code
> managing too many of the governors responsibilities.
>
> Regards,
> Mike
>


No, the DEVFREQ governors are tuneable per-device basis. At least,
SIMPLE-ONDEMAND governor is capable of doing so. (just not did so in
the example code) I'll add an example in the last paragraph.


Although sysfs control for governors is not yet implemented--I guess
it may wait for later versions as it does not look critical--, in this
patchset v6, the governors are tuneable per device. Per-device data is
stored at struct devfreq and struct devfreq_dev_profile, not in struct
devfreq_governor. Governors can alter its behavior based on the
"devfreq.data" and save per-device information (if any) to
"devfreq.data". The "data" field of devfreq may be defined by each
governor.


[]
>> ---
>> Changed from v5
>> - Seperated governor files from devfreq.c
>> - Allow simple ondemand to be tuned for each device

The example code of Exynos4210-Memory-Bus does not tune the tuneable
value, but simply uses the default value.

However, in order to use custom values, we can use devfreq_add_device
with the custom values supplied.

In the example code (exynos4210_memorybus.c), it does:
     err = devfreq_add_device(dev, &exynos4_devfreq_profile, default_gov,
                                          NULL);
at line 502-503.

If we are going to use custom values, we can modify that as:
     struct devfreq_simple_ondemand_data tuneit = { .upthreshold = 80,
.downdifferential = 10, };
     err = devfreq_add_device(dev, &exynos4_devfreq_profile,
default_gov, &tuneit);

And, the device can update "tuneit" in run-time to alter the behavior.




Cheers!

MyungJoo


>> ---
>>  drivers/devfreq/Kconfig                   |   36 ++++++++++++
>>  drivers/devfreq/Makefile                  |    4 +
>>  drivers/devfreq/governor_performance.c    |   24 ++++++++
>>  drivers/devfreq/governor_powersave.c      |   24 ++++++++
>>  drivers/devfreq/governor_simpleondemand.c |   88 +++++++++++++++++++++++++++++
>>  drivers/devfreq/governor_userspace.c      |   27 +++++++++
>>  include/linux/devfreq.h                   |   41 +++++++++++++
>>  7 files changed, 244 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/devfreq/governor_performance.c
>>  create mode 100644 drivers/devfreq/governor_powersave.c
>>  create mode 100644 drivers/devfreq/governor_simpleondemand.c
>>  create mode 100644 drivers/devfreq/governor_userspace.c
>>
>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>> index 1fb42de..643b055 100644
>> --- a/drivers/devfreq/Kconfig
>> +++ b/drivers/devfreq/Kconfig
>> @@ -34,6 +34,42 @@ menuconfig PM_DEVFREQ
>>
>>  if PM_DEVFREQ
>>
>> +comment "DEVFREQ Governors"
>> +
>> +config DEVFREQ_GOV_SIMPLE_ONDEMAND
>> +       bool "Simple Ondemand"
>> +       help
>> +         Chooses frequency based on the recent load on the device. Works
>> +         similar as ONDEMAND governor of CPUFREQ does. A device with
>> +         Simple-Ondemand should be able to provide busy/total counter
>> +         values that imply the usage rate. A device may provide tuned
>> +         values to the governor with data field at devfreq_add_device().
>> +
>> +config DEVFREQ_GOV_PERFORMANCE
>> +       bool "Performance"
>> +       help
>> +         Sets the frequency at the maximum available frequency.
>> +         This governor always returns UINT_MAX as frequency so that
>> +         the DEVFREQ framework returns the highest frequency available
>> +         at any time.
>> +
>> +config DEVFREQ_GOV_POWERSAVE
>> +       bool "Powersave"
>> +       help
>> +         Sets the frequency at the minimum available frequency.
>> +         This governor always returns 0 as frequency so that
>> +         the DEVFREQ framework returns the lowest frequency available
>> +         at any time.
>> +
>> +config DEVFREQ_GOV_USERSPACE
>> +       bool "Userspace"
>> +       help
>> +         Sets the frequency at the user specified one.
>> +         This governor returns the user configured frequency if there
>> +         has been an input to /sys/devices/.../power/devfreq_set_freq.
>> +         Otherwise, the governor does not change the frequnecy
>> +         given at the initialization.
>> +
>>  comment "DEVFREQ Drivers"
>>
>>  endif # PM_DEVFREQ
>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>> index 168934a..4564a89 100644
>> --- a/drivers/devfreq/Makefile
>> +++ b/drivers/devfreq/Makefile
>> @@ -1 +1,5 @@
>>  obj-$(CONFIG_PM_DEVFREQ)       += devfreq.o
>> +obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)      += governor_simpleondemand.o
>> +obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)  += governor_performance.o
>> +obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)    += governor_powersave.o
>> +obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)    += governor_userspace.o
>> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
>> new file mode 100644
>> index 0000000..c47eff8
>> --- /dev/null
>> +++ b/drivers/devfreq/governor_performance.c
>> @@ -0,0 +1,24 @@
>> +/*
>> + *  linux/drivers/devfreq/governor_performance.c
>> + *
>> + *  Copyright (C) 2011 Samsung Electronics
>> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/devfreq.h>
>> +
>> +static int devfreq_performance_func(struct devfreq *df,
>> +                                   unsigned long *freq)
>> +{
>> +       *freq = UINT_MAX; /* devfreq_do will run "floor" */
>> +       return 0;
>> +}
>> +
>> +struct devfreq_governor devfreq_performance = {
>> +       .name = "performance",
>> +       .get_target_freq = devfreq_performance_func,
>> +};
>> diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
>> new file mode 100644
>> index 0000000..4f128d8
>> --- /dev/null
>> +++ b/drivers/devfreq/governor_powersave.c
>> @@ -0,0 +1,24 @@
>> +/*
>> + *  linux/drivers/devfreq/governor_powersave.c
>> + *
>> + *  Copyright (C) 2011 Samsung Electronics
>> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/devfreq.h>
>> +
>> +static int devfreq_powersave_func(struct devfreq *df,
>> +                                 unsigned long *freq)
>> +{
>> +       *freq = 0; /* devfreq_do will run "ceiling" to 0 */
>> +       return 0;
>> +}
>> +
>> +struct devfreq_governor devfreq_powersave = {
>> +       .name = "powersave",
>> +       .get_target_freq = devfreq_powersave_func,
>> +};
>> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
>> new file mode 100644
>> index 0000000..18fe8be
>> --- /dev/null
>> +++ b/drivers/devfreq/governor_simpleondemand.c
>> @@ -0,0 +1,88 @@
>> +/*
>> + *  linux/drivers/devfreq/governor_simpleondemand.c
>> + *
>> + *  Copyright (C) 2011 Samsung Electronics
>> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/errno.h>
>> +#include <linux/devfreq.h>
>> +#include <linux/math64.h>
>> +
>> +/* Default constants for DevFreq-Simple-Ondemand (DFSO) */
>> +#define DFSO_UPTHRESHOLD       (90)
>> +#define DFSO_DOWNDIFFERENCTIAL (5)
>> +static int devfreq_simple_ondemand_func(struct devfreq *df,
>> +                                       unsigned long *freq)
>> +{
>> +       struct devfreq_dev_status stat;
>> +       int err = df->profile->get_dev_status(df->dev, &stat);
>> +       unsigned long long a, b;
>> +       unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
>> +       unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
>> +       struct devfreq_simple_ondemand_data *data = df->data;
>> +
>> +       if (err)
>> +               return err;
>> +
>> +       if (data) {
>> +               if (data->upthreshold)
>> +                       dfso_upthreshold = data->upthreshold;
>> +               if (data->downdifferential)
>> +                       dfso_downdifferential = data->downdifferential;
>> +       }
>> +       if (dfso_upthreshold > 100 ||
>> +           dfso_upthreshold < dfso_downdifferential)
>> +               return -EINVAL;
>> +
>> +       /* Assume MAX if it is going to be divided by zero */
>> +       if (stat.total_time == 0) {
>> +               *freq = UINT_MAX;
>> +               return 0;
>> +       }
>> +
>> +       /* Prevent overflow */
>> +       if (stat.busy_time >= (1 << 24) || stat.total_time >= (1 << 24)) {
>> +               stat.busy_time >>= 7;
>> +               stat.total_time >>= 7;
>> +       }
>> +
>> +       /* Set MAX if it's busy enough */
>> +       if (stat.busy_time * 100 >
>> +           stat.total_time * dfso_upthreshold) {
>> +               *freq = UINT_MAX;
>> +               return 0;
>> +       }
>> +
>> +       /* Set MAX if we do not know the initial frequency */
>> +       if (stat.current_frequency == 0) {
>> +               *freq = UINT_MAX;
>> +               return 0;
>> +       }
>> +
>> +       /* Keep the current frequency */
>> +       if (stat.busy_time * 100 >
>> +           stat.total_time * (dfso_upthreshold - dfso_downdifferential)) {
>> +               *freq = stat.current_frequency;
>> +               return 0;
>> +       }
>> +
>> +       /* Set the desired frequency based on the load */
>> +       a = stat.busy_time;
>> +       a *= stat.current_frequency;
>> +       b = div_u64(a, stat.total_time);
>> +       b *= 100;
>> +       b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
>> +       *freq = (unsigned long) b;
>> +
>> +       return 0;
>> +}
>> +
>> +struct devfreq_governor devfreq_simple_ondemand = {
>> +       .name = "simple_ondemand",
>> +       .get_target_freq = devfreq_simple_ondemand_func,
>> +};
>> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
>> new file mode 100644
>> index 0000000..6af8b6d
>> --- /dev/null
>> +++ b/drivers/devfreq/governor_userspace.c
>> @@ -0,0 +1,27 @@
>> +/*
>> + *  linux/drivers/devfreq/governor_simpleondemand.c
>> + *
>> + *  Copyright (C) 2011 Samsung Electronics
>> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/devfreq.h>
>> +
>> +static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
>> +{
>> +       if (df->user_set_freq == 0)
>> +               *freq = df->previous_freq; /* No user freq specified yet */
>> +       else
>> +               *freq = df->user_set_freq;
>> +
>> +       return 0;
>> +}
>> +
>> +struct devfreq_governor devfreq_userspace = {
>> +       .name = "userspace",
>> +       .get_target_freq = devfreq_userspace_func,
>> +};
>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>> index 13ddf49..d088ad3 100644
>> --- a/include/linux/devfreq.h
>> +++ b/include/linux/devfreq.h
>> @@ -13,6 +13,7 @@
>>  #ifndef __LINUX_DEVFREQ_H__
>>  #define __LINUX_DEVFREQ_H__
>>
>> +#include <linux/opp.h>
>>  #include <linux/notifier.h>
>>
>>  #define DEVFREQ_NAME_LEN 16
>> @@ -62,6 +63,8 @@ struct devfreq_governor {
>>  *                     "devfreq_monitor" executions to reevaluate
>>  *                     frequency/voltage of the device. Set by
>>  *                     profile's polling_ms interval.
>> + * @user_set_freq      User specified adequete frequency value (thru sysfs
>> + *             interface). Governors may and may not use this value.
>>  * @data       Private data of the governor. The devfreq framework does not
>>  *             touch this.
>>  *
>> @@ -78,6 +81,7 @@ struct devfreq {
>>        unsigned long previous_freq;
>>        unsigned int next_polling;
>>
>> +       unsigned long user_set_freq; /* governors may ignore this. */
>>        void *data; /* private data for governors */
>>  };
>>
>> @@ -87,6 +91,37 @@ extern int devfreq_add_device(struct device *dev,
>>                           struct devfreq_governor *governor,
>>                           void *data);
>>  extern int devfreq_remove_device(struct device *dev);
>> +
>> +#ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
>> +extern struct devfreq_governor devfreq_powersave;
>> +#endif
>> +#ifdef CONFIG_DEVFREQ_GOV_PERFORMANCE
>> +extern struct devfreq_governor devfreq_performance;
>> +#endif
>> +#ifdef CONFIG_DEVFREQ_GOV_USERSPACE
>> +extern struct devfreq_governor devfreq_userspace;
>> +#endif
>> +#ifdef CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND
>> +extern struct devfreq_governor devfreq_simple_ondemand;
>> +/**
>> + * struct devfreq_simple_ondemand_data - void *data fed to struct devfreq
>> + *     and devfreq_add_device
>> + * @ upthreshold       If the load is over this value, the frequency jumps.
>> + *                     Specify 0 to use the default. Valid value = 0 to 100.
>> + * @ downdifferential  If the load is under upthreshold - downdifferential,
>> + *                     the governor may consider slowing the frequency down.
>> + *                     Specify 0 to use the default. Valid value = 0 to 100.
>> + *                     downdifferential < upthreshold must hold.
>> + *
>> + * If the fed devfreq_simple_ondemand_data pointer is NULL to the governor,
>> + * the governor uses the default values.
>> + */
>> +struct devfreq_simple_ondemand_data {
>> +       unsigned int upthreshold;
>> +       unsigned int downdifferential;
>> +};
>> +#endif
>> +
>>  #else /* !CONFIG_PM_DEVFREQ */
>>  static int devfreq_add_device(struct device *dev,
>>                           struct devfreq_dev_profile *profile,
>> @@ -100,6 +135,12 @@ static int devfreq_remove_device(struct device *dev)
>>  {
>>        return 0;
>>  }
>> +
>> +#define devfreq_powersave      NULL
>> +#define devfreq_performance    NULL
>> +#define devfreq_userspace      NULL
>> +#define devfreq_simple_ondemand        NULL
>> +
>>  #endif /* CONFIG_PM_DEVFREQ */
>>
>>  #endif /* __LINUX_DEVFREQ_H__ */
>> --
>> 1.7.4.1
>>
>>
>



-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

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

* Re: [PATCH v6 4/4] PM / DEVFREQ: add sysfs interface
  2011-08-19  3:15   ` Turquette, Mike
@ 2011-08-19  7:28     ` MyungJoo Ham
  2011-08-22 18:28       ` Turquette, Mike
  0 siblings, 1 reply; 21+ messages in thread
From: MyungJoo Ham @ 2011-08-19  7:28 UTC (permalink / raw)
  To: Turquette, Mike
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Fri, Aug 19, 2011 at 12:15 PM, Turquette, Mike <mturquette@ti.com> wrote:
> On Thu, Aug 18, 2011 at 3:27 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>> Device specific sysfs interface /sys/devices/.../power/devfreq_*
>> - governor      R: name of governor
>> - cur_freq      R: current frequency
>> - max_freq      R: maximum operable frequency
>> - min_freq      R: minimum operable frequency
>> - set_freq      R: read user specified frequency (0 if not specified yet)
>>                W: set user specified frequency
>> - polling_interval      R: polling interval in ms given with devfreq profile
>
> This code still does not export up_threshold/down_threshold entries
> for simple-ondemand which I requested in V5.  However there is a much
> bigger issue here brought out in patch 3.  Those values shouldn't be
> global, since devices might want different thresholds.
>
> The solution is not to put those tunables in struct devfreq.
> up_threshold and down_threshold must be parameters of the governor,
> and there must be a per-device instance of the governor.
>
> Regards,
> Mike

As in the example in the reply to the thread of patch 3/4 of v6
patchset, we do not need to create another per-device object in
DEVFREQ in order to allow governors to be tuned per device. The global
values you've mentioned are only used as default and each instance of
devfreq (per device) can specify its own tuneable values and they can
alter the values in run-time.

For the sysfs interfaces of governor-specific tunable values, I'd like
to see that as a "future work" and that would be implemented
per-governor basis; i.e., probably under
/sysfs/devices/.../power/NAME_OF_GOVERNOR/variable_name. Or should I
regard the sysfs interface to tune governors at userland is somewhat
essential and critical for the initial release?



Cheers!

MyungJoo

>
>>
>> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>
>> --
>> Changed from v5
>> - updated devferq_update usage.
>>
>> Changed from v4
>> - removed system-wide sysfs interface
>> - removed tickling sysfs interface
>> - added set_freq for userspace governor (and any other governors that
>>  require user input)
>>
>> Changed from v3
>> - corrected sysfs API usage
>> - corrected error messages
>> - moved sysfs entry location
>> - added sysfs entries
>>
>> Changed from v2
>> - add ABI entries for devfreq sysfs interface
>> ---
>>  Documentation/ABI/testing/sysfs-devices-power |   45 +++++++
>>  drivers/devfreq/devfreq.c                     |  152 +++++++++++++++++++++++++
>>  2 files changed, 197 insertions(+), 0 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
>> index 8ffbc25..6e77604 100644
>> --- a/Documentation/ABI/testing/sysfs-devices-power
>> +++ b/Documentation/ABI/testing/sysfs-devices-power
>> @@ -165,3 +165,48 @@ Description:
>>
>>                Not all drivers support this attribute.  If it isn't supported,
>>                attempts to read or write it will yield I/O errors.
>> +
>> +What:          /sys/devices/.../power/devfreq_governor
>> +Date:          July 2011
>> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
>> +Description:
>> +               The /sys/devices/.../power/devfreq_governor shows the name
>> +               of the governor used by the corresponding device.
>> +
>> +What:          /sys/devices/.../power/devfreq_cur_freq
>> +Date:          July 2011
>> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
>> +Description:
>> +               The /sys/devices/.../power/devfreq_cur_freq shows the current
>> +               frequency of the corresponding device.
>> +
>> +What:          /sys/devices/.../power/devfreq_max_freq
>> +Date:          July 2011
>> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
>> +Description:
>> +               The /sys/devices/.../power/devfreq_cur_freq shows the
>> +               maximum operable frequency of the corresponding device.
>> +
>> +What:          /sys/devices/.../power/devfreq_min_freq
>> +Date:          July 2011
>> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
>> +Description:
>> +               The /sys/devices/.../power/devfreq_cur_freq shows the
>> +               minimum operable frequency of the corresponding device.
>> +
>> +What:          /sys/devices/.../power/devfreq_set_freq
>> +Date:          August 2011
>> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
>> +Description:
>> +               The /sys/devices/.../power/devfreq_set_freq sets and shows
>> +               the user specified desired frequency of the device. The
>> +               governor may and may not use the value. With the basic
>> +               governors given with devfreq.c, userspace governor is
>> +               using the value.
>> +
>> +What:          /sys/devices/.../power/devfreq_polling_interval
>> +Date:          July 2011
>> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
>> +Description:
>> +               The /sys/devices/.../power/devfreq_polling_interval shows the
>> +               requested polling interval of the corresponding device.
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index 2036f2c..13b422f 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -37,6 +37,8 @@ static struct delayed_work devfreq_work;
>>  static LIST_HEAD(devfreq_list);
>>  static DEFINE_MUTEX(devfreq_list_lock);
>>
>> +static struct attribute_group dev_attr_group;
>> +
>>  /**
>>  * find_device_devfreq() - find devfreq struct using device pointer
>>  * @dev:       device pointer used to lookup device devfreq.
>> @@ -154,6 +156,8 @@ static void devfreq_monitor(struct work_struct *work)
>>                                dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
>>                                        error, devfreq->governor->name);
>>
>> +                               sysfs_unmerge_group(&devfreq->dev->kobj,
>> +                                                   &dev_attr_group);
>>                                list_del(&devfreq->node);
>>                                kfree(devfreq);
>>
>> @@ -244,6 +248,8 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
>>                queue_delayed_work(devfreq_wq, &devfreq_work,
>>                                   devfreq->next_polling);
>>        }
>> +
>> +       sysfs_merge_group(&dev->kobj, &dev_attr_group);
>>  out:
>>        mutex_unlock(&devfreq_list_lock);
>>
>> @@ -276,6 +282,8 @@ int devfreq_remove_device(struct device *dev)
>>                goto out;
>>        }
>>
>> +       sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
>> +
>>        list_del(&devfreq->node);
>>        srcu_notifier_chain_unregister(nh, &devfreq->nb);
>>        kfree(devfreq);
>> @@ -284,6 +292,150 @@ out:
>>        return 0;
>>  }
>>
>> +static ssize_t show_governor(struct device *dev,
>> +                            struct device_attribute *attr, char *buf)
>> +{
>> +       struct devfreq *df = find_device_devfreq(dev);
>> +
>> +       if (IS_ERR(df))
>> +               return PTR_ERR(df);
>> +       if (!df->governor)
>> +               return -EINVAL;
>> +
>> +       return sprintf(buf, "%s\n", df->governor->name);
>> +}
>> +
>> +static ssize_t show_freq(struct device *dev,
>> +                        struct device_attribute *attr, char *buf)
>> +{
>> +       struct devfreq *df = find_device_devfreq(dev);
>> +
>> +       if (IS_ERR(df))
>> +               return PTR_ERR(df);
>> +
>> +       return sprintf(buf, "%lu\n", df->previous_freq);
>> +}
>> +
>> +static ssize_t show_max_freq(struct device *dev,
>> +                            struct device_attribute *attr, char *buf)
>> +{
>> +       struct devfreq *df = find_device_devfreq(dev);
>> +       unsigned long freq = ULONG_MAX;
>> +       struct opp *opp;
>> +
>> +       if (IS_ERR(df))
>> +               return PTR_ERR(df);
>> +       if (!df->dev)
>> +               return -EINVAL;
>> +
>> +       opp = opp_find_freq_floor(df->dev, &freq);
>> +       if (IS_ERR(opp))
>> +               return PTR_ERR(opp);
>> +
>> +       return sprintf(buf, "%lu\n", freq);
>> +}
>> +
>> +static ssize_t show_min_freq(struct device *dev,
>> +                            struct device_attribute *attr, char *buf)
>> +{
>> +       struct devfreq *df = find_device_devfreq(dev);
>> +       unsigned long freq = 0;
>> +       struct opp *opp;
>> +
>> +       if (IS_ERR(df))
>> +               return PTR_ERR(df);
>> +       if (!df->dev)
>> +               return -EINVAL;
>> +
>> +       opp = opp_find_freq_ceil(df->dev, &freq);
>> +       if (IS_ERR(opp))
>> +               return PTR_ERR(opp);
>> +
>> +       return sprintf(buf, "%lu\n", freq);
>> +}
>> +
>> +static ssize_t show_polling_interval(struct device *dev,
>> +                                    struct device_attribute *attr, char *buf)
>> +{
>> +       struct devfreq *df = find_device_devfreq(dev);
>> +
>> +       if (IS_ERR(df))
>> +               return PTR_ERR(df);
>> +       if (!df->profile)
>> +               return -EINVAL;
>> +
>> +       return sprintf(buf, "%d\n", df->profile->polling_ms);
>> +}
>> +
>> +static ssize_t set_user_frequency(struct device *dev,
>> +                                 struct device_attribute *attr,
>> +                                 const char *buf, size_t count)
>> +{
>> +       struct devfreq *devfreq;
>> +       unsigned long wanted;
>> +       int err = 0;
>> +
>> +       sscanf(buf, "%lu", &wanted);
>> +
>> +       mutex_lock(&devfreq_list_lock);
>> +       devfreq = find_device_devfreq(dev);
>> +
>> +       if (IS_ERR(devfreq)) {
>> +               err = PTR_ERR(devfreq);
>> +               goto out;
>> +       }
>> +
>> +       devfreq->user_set_freq = wanted;
>> +       err = count;
>> +out:
>> +       mutex_unlock(&devfreq_list_lock);
>> +       if (err >= 0)
>> +               devfreq_update(&devfreq->nb, 0, NULL);
>> +       return err;
>> +}
>> +
>> +static ssize_t show_user_frequency(struct device *dev,
>> +                                  struct device_attribute *attr, char *buf)
>> +{
>> +       struct devfreq *devfreq;
>> +       int err = 0;
>> +
>> +       mutex_lock(&devfreq_list_lock);
>> +       devfreq = find_device_devfreq(dev);
>> +
>> +       if (IS_ERR(devfreq)) {
>> +               err = PTR_ERR(devfreq);
>> +               goto out;
>> +       }
>> +
>> +       err = sprintf(buf, "%lu\n", devfreq->user_set_freq);
>> +out:
>> +       mutex_unlock(&devfreq_list_lock);
>> +       return err;
>> +
>> +}
>> +
>> +static DEVICE_ATTR(devfreq_governor, 0444, show_governor, NULL);
>> +static DEVICE_ATTR(devfreq_cur_freq, 0444, show_freq, NULL);
>> +static DEVICE_ATTR(devfreq_max_freq, 0444, show_max_freq, NULL);
>> +static DEVICE_ATTR(devfreq_min_freq, 0444, show_min_freq, NULL);
>> +static DEVICE_ATTR(devfreq_polling_interval, 0444, show_polling_interval, NULL);
>> +static DEVICE_ATTR(devfreq_set_freq, 0644, show_user_frequency,
>> +                  set_user_frequency);
>> +static struct attribute *dev_entries[] = {
>> +       &dev_attr_devfreq_governor.attr,
>> +       &dev_attr_devfreq_cur_freq.attr,
>> +       &dev_attr_devfreq_max_freq.attr,
>> +       &dev_attr_devfreq_min_freq.attr,
>> +       &dev_attr_devfreq_polling_interval.attr,
>> +       &dev_attr_devfreq_set_freq.attr,
>> +       NULL,
>> +};
>> +static struct attribute_group dev_attr_group = {
>> +       .name   = power_group_name,
>> +       .attrs  = dev_entries,
>> +};
>> +
>>  /**
>>  * devfreq_init() - Initialize data structure for devfreq framework and
>>  *               start polling registered devfreq devices.
>> --
>> 1.7.4.1
>>
>>
>



-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

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

* Re: [PATCH v6 2/4] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs
  2011-08-19  2:43   ` Turquette, Mike
@ 2011-08-19  8:29     ` MyungJoo Ham
  2011-08-19  8:30     ` [PATCH v6-resubmit of " MyungJoo Ham
  1 sibling, 0 replies; 21+ messages in thread
From: MyungJoo Ham @ 2011-08-19  8:29 UTC (permalink / raw)
  To: Turquette, Mike
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Fri, Aug 19, 2011 at 11:43 AM, Turquette, Mike <mturquette@ti.com> wrote:
> On Thu, Aug 18, 2011 at 3:27 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>
> Better to deal natively in jiffies instead of doing conversions of
> conversions.  Make last_polled_at an unsigned long and then
> jiffies_passed just becomes jiffies - last_polled_at.

Oh,, good point. We don't need that ktime here. I'll submit a partial
resubmit for this as well in minutes.

>
> I still think that the timekeeping here is gross.  last_polled_at
> should not be static.  Instead governors should track the timekeeping
> info themselves (for now could live in struct devfreq_governor).  I
> won't NACK the patch b/c of that, but I think that the code will have
> to become more modularized in the future if devfreq sees wide
> adoption.
>

Fine. For the case the possibility of a DEVFREQ device that must not
share the polling loop (devices inside a core of multi-core CPUs other
than the core itself?), I'd think about allowing governors to have its
own loop optionally for later versions in the future when the needs
appear to be approaching. Before that point, it just looks like an
additional overhead.

>
> Similar to the above, it's better to deal natively in jiffies instead
> of doing a conversion every time.  When a polling interval is
> specified at init-time, or via sysfs it should be in milliseconds, but
> the storage should be jiffies to avoid conversions every single loop
> through this code path.

The soon-to-be-following partial resubmitting patch will address this as well.

>
> Regards,
> Mike
>

Thank you!


Cheers,
MyungJoo

-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

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

* [PATCH v6-resubmit of 2/4] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs
  2011-08-19  2:43   ` Turquette, Mike
  2011-08-19  8:29     ` MyungJoo Ham
@ 2011-08-19  8:30     ` MyungJoo Ham
  2011-08-22 18:19       ` Turquette, Mike
  1 sibling, 1 reply; 21+ messages in thread
From: MyungJoo Ham @ 2011-08-19  8:30 UTC (permalink / raw)
  To: linux-pm, Mike Turquette
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, Thomas Gleixner

With OPPs, a device may have multiple operable frequency and voltage
sets. However, there can be multiple possible operable sets and a system
will need to choose one from them. In order to reduce the power
consumption (by reducing frequency and voltage) without affecting the
performance too much, a Dynamic Voltage and Frequency Scaling (DVFS)
scheme may be used.

This patch introduces the DVFS capability to non-CPU devices with OPPs.
DVFS is a techique whereby the frequency and supplied voltage of a
device is adjusted on-the-fly. DVFS usually sets the frequency as low
as possible with given conditions (such as QoS assurance) and adjusts
voltage according to the chosen frequency in order to reduce power
consumption and heat dissipation.

The generic DVFS for devices, DEVFREQ, may appear quite similar with
/drivers/cpufreq.  However, CPUFREQ does not allow to have multiple
devices registered and is not suitable to have multiple heterogenous
devices with different (but simple) governors.

Normally, DVFS mechanism controls frequency based on the demand for
the device, and then, chooses voltage based on the chosen frequency.
DEVFREQ also controls the frequency based on the governor's frequency
recommendation and let OPP pick up the pair of frequency and voltage
based on the recommended frequency. Then, the chosen OPP is passed to
device driver's "target" callback.

When PM QoS is going to be used with the DEVFREQ device, the device
driver should enable OPPs that are appropriate with the current PM QoS
requests. In order to do so, the device driver may call opp_enable and
opp_disable at the notifier callback of PM QoS so that PM QoS's
update_target() call enables the appropriate OPPs. Note that at least
one of OPPs should be enabled at any time; be careful when there is a
transition.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

---
The test code with board support for Exynos4-NURI is at
http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq

---
Thank you for your valuable comments, Rafael, Greg, Pavel, Colin, Mike,
and Kevin.

Changes at v6-resubmit from v6
- Use jiffy directly instead of ktime
- Be prepared for profile->polling_ms changes (not supported fully at
  this stage)

Changes from v5
- Uses OPP availability change notifier
- Removed devfreq_interval. Uses one jiffy instead. DEVFREQ adjusts
  polling interval based on the interval requirement of DEVFREQ
  devices.
- Moved devfreq to /drivers/devfreq to accomodate devfreq-related files
  including governors and devfreq drivers.
- Coding style revised.
- Updated devfreq_add_device interface to get tunable values.

Changed from v4
- Removed tickle, which is a duplicated feature; PM QoS can do the same.
- Allow to extend polling interval if devices have longer polling intervals.
- Relocated private data of governors.
- Removed system-wide sysfs

Changed from v3
- In kerneldoc comments, DEVFREQ has ben replaced by devfreq
- Revised removing devfreq entries with error mechanism
- Added and revised comments
- Removed unnecessary codes
- Allow to give a name to a governor
- Bugfix: a tickle call may cancel an older tickle call that is still in
  effect.

Changed from v2
- Code style revised and cleaned up.
- Remove DEVFREQ entries that incur errors except for EAGAIN
- Bug fixed: tickle for devices without polling governors

Changes from v1(RFC)
- Rename: DVFS --> DEVFREQ
- Revised governor design
	. Governor receives the whole struct devfreq
	. Governor should gather usage information (thru get_dev_status) itself
- Periodic monitoring runs only when needed.
- DEVFREQ no more deals with voltage information directly
- Removed some printks.
- Some cosmetics update
- Use freezable_wq.
---
 drivers/Kconfig           |    2 +
 drivers/Makefile          |    2 +
 drivers/devfreq/Kconfig   |   39 ++++++
 drivers/devfreq/Makefile  |    1 +
 drivers/devfreq/devfreq.c |  312 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/devfreq.h   |  111 ++++++++++++++++
 6 files changed, 467 insertions(+), 0 deletions(-)
 create mode 100644 drivers/devfreq/Kconfig
 create mode 100644 drivers/devfreq/Makefile
 create mode 100644 drivers/devfreq/devfreq.c
 create mode 100644 include/linux/devfreq.h

diff --git a/drivers/Kconfig b/drivers/Kconfig
index 95b9e7e..a1efd75 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -130,4 +130,6 @@ source "drivers/iommu/Kconfig"
 
 source "drivers/virt/Kconfig"
 
+source "drivers/devfreq/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 7fa433a..97c957b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -127,3 +127,5 @@ obj-$(CONFIG_IOMMU_SUPPORT)	+= iommu/
 
 # Virtualization drivers
 obj-$(CONFIG_VIRT_DRIVERS)	+= virt/
+
+obj-$(CONFIG_PM_DEVFREQ)	+= devfreq/
diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
new file mode 100644
index 0000000..1fb42de
--- /dev/null
+++ b/drivers/devfreq/Kconfig
@@ -0,0 +1,39 @@
+config ARCH_HAS_DEVFREQ
+	bool
+	depends on ARCH_HAS_OPP
+	help
+	  Denotes that the architecture supports DEVFREQ. If the architecture
+	  supports multiple OPP entries per device and the frequency of the
+	  devices with OPPs may be altered dynamically, the architecture
+	  supports DEVFREQ.
+
+menuconfig PM_DEVFREQ
+	bool "Generic Dynamic Voltage and Frequency Scaling (DVFS) support"
+	depends on PM_OPP && ARCH_HAS_DEVFREQ
+	help
+	  With OPP support, a device may have a list of frequencies and
+	  voltages available. DEVFREQ, a generic DVFS framework can be
+	  registered for a device with OPP support in order to let the
+	  governor provided to DEVFREQ choose an operating frequency
+	  based on the OPP's list and the policy given with DEVFREQ.
+
+	  Each device may have its own governor and policy. DEVFREQ can
+	  reevaluate the device state periodically and/or based on the
+	  OPP list changes (each frequency/voltage pair in OPP may be
+	  disabled or enabled).
+
+	  Like some CPUs with CPUFREQ, a device may have multiple clocks.
+	  However, because the clock frequencies of a single device are
+	  determined by the single device's state, an instance of DEVFREQ
+	  is attached to a single device and returns a "representative"
+	  clock frequency from the OPP of the device, which is also attached
+	  to a device by 1-to-1. The device registering DEVFREQ takes the
+	  responsiblity to "interpret" the frequency listed in OPP and
+	  to set its every clock accordingly with the "target" callback
+	  given to DEVFREQ.
+
+if PM_DEVFREQ
+
+comment "DEVFREQ Drivers"
+
+endif # PM_DEVFREQ
diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
new file mode 100644
index 0000000..168934a
--- /dev/null
+++ b/drivers/devfreq/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_PM_DEVFREQ)	+= devfreq.o
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
new file mode 100644
index 0000000..af848ffa
--- /dev/null
+++ b/drivers/devfreq/devfreq.c
@@ -0,0 +1,312 @@
+/*
+ * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
+ *	    for Non-CPU Devices Based on OPP.
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/opp.h>
+#include <linux/devfreq.h>
+#include <linux/workqueue.h>
+#include <linux/platform_device.h>
+#include <linux/list.h>
+#include <linux/printk.h>
+#include <linux/hrtimer.h>
+
+/*
+ * devfreq_work periodically monitors every registered device.
+ * The minimum polling interval is one jiffy. The polling interval is
+ * determined by the minimum polling period among all polling devfreq
+ * devices. The resolution of polling interval is one jiffy.
+ */
+static bool polling;
+static struct workqueue_struct *devfreq_wq;
+static struct delayed_work devfreq_work;
+
+/* The list of all device-devfreq */
+static LIST_HEAD(devfreq_list);
+static DEFINE_MUTEX(devfreq_list_lock);
+
+/**
+ * find_device_devfreq() - find devfreq struct using device pointer
+ * @dev:	device pointer used to lookup device devfreq.
+ *
+ * Search the list of device devfreqs and return the matched device's
+ * devfreq info. devfreq_list_lock should be held by the caller.
+ */
+static struct devfreq *find_device_devfreq(struct device *dev)
+{
+	struct devfreq *tmp_devfreq;
+
+	if (unlikely(IS_ERR_OR_NULL(dev))) {
+		pr_err("DEVFREQ: %s: Invalid parameters\n", __func__);
+		return ERR_PTR(-EINVAL);
+	}
+
+	list_for_each_entry(tmp_devfreq, &devfreq_list, node) {
+		if (tmp_devfreq->dev == dev)
+			return tmp_devfreq;
+	}
+
+	return ERR_PTR(-ENODEV);
+}
+
+/**
+ * devfreq_do() - Check the usage profile of a given device and configure
+ *		frequency and voltage accordingly
+ * @devfreq:	devfreq info of the given device
+ */
+static int devfreq_do(struct devfreq *devfreq)
+{
+	struct opp *opp;
+	unsigned long freq;
+	int err;
+
+	err = devfreq->governor->get_target_freq(devfreq, &freq);
+	if (err)
+		return err;
+
+	opp = opp_find_freq_ceil(devfreq->dev, &freq);
+	if (opp == ERR_PTR(-ENODEV))
+		opp = opp_find_freq_floor(devfreq->dev, &freq);
+
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+
+	if (devfreq->previous_freq == freq)
+		return 0;
+
+	err = devfreq->profile->target(devfreq->dev, opp);
+	if (err)
+		return err;
+
+	devfreq->previous_freq = freq;
+	return 0;
+}
+
+/**
+ * devfreq_update() - Notify that the device OPP has been changed.
+ * @dev:	the device whose OPP has been changed.
+ */
+static int devfreq_update(struct notifier_block *nb, unsigned long type,
+			  void *devp)
+{
+	struct devfreq *devfreq;
+	int err = 0;
+
+	mutex_lock(&devfreq_list_lock);
+	devfreq = container_of(nb, struct devfreq, nb);
+	/* Reevaluate the proper frequency */
+	err = devfreq_do(devfreq);
+	mutex_unlock(&devfreq_list_lock);
+	return err;
+}
+
+/**
+ * devfreq_monitor() - Periodically run devfreq_do()
+ * @work: the work struct used to run devfreq_monitor periodically.
+ *
+ */
+static void devfreq_monitor(struct work_struct *work)
+{
+	static unsigned long last_polled_at;
+	struct devfreq *devfreq, *tmp;
+	int error;
+	int jiffies_passed;
+	unsigned long next_jiffies = ULONG_MAX, now = jiffies;
+
+	/* Initially last_polled_at = 0, polling every device at bootup */
+	jiffies_passed = now - last_polled_at;
+	last_polled_at = now;
+
+	if (jiffies_passed == 0)
+		jiffies_passed = 1;
+	if (jiffies_passed < 0) /* "Infinite Timeout" */
+		jiffies_passed = INT_MAX;
+
+	mutex_lock(&devfreq_list_lock);
+
+	list_for_each_entry_safe(devfreq, tmp, &devfreq_list, node) {
+		/* Reflect the changes in profile->polling_ms */
+		if (devfreq->polling_ms != devfreq->profile->polling_ms) {
+			devfreq->polling_ms = devfreq->profile->polling_ms;
+			devfreq->polling_jiffies = msecs_to_jiffies(
+					devfreq->polling_ms);
+			if (devfreq->next_polling == 0 &&
+			    devfreq->polling_jiffies)
+				devfreq->next_polling = 1; /* Poll Now */
+			else if (devfreq->polling_jiffies == 0)
+				devfreq->next_polling = 0; /* Stop Polling */
+		}
+		if (devfreq->next_polling == 0)
+			continue;
+
+		/*
+		 * Reduce more next_polling if devfreq_wq took an extra
+		 * delay. (i.e., CPU has been idled.)
+		 */
+		if (devfreq->next_polling <= jiffies_passed) {
+			error = devfreq_do(devfreq);
+
+			/* Remove a devfreq with an error. */
+			if (error && error != -EAGAIN) {
+				dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
+					error, devfreq->governor->name);
+
+				list_del(&devfreq->node);
+				kfree(devfreq);
+
+				continue;
+			}
+			devfreq->next_polling = devfreq->polling_jiffies;
+
+			/* No more polling required (polling_ms changed) */
+			if (devfreq->next_polling == 0)
+				continue;
+		} else {
+			devfreq->next_polling -= jiffies_passed;
+		}
+
+		next_jiffies = (next_jiffies > devfreq->next_polling) ?
+				devfreq->next_polling : next_jiffies;
+	}
+
+	if (next_jiffies > 0 && next_jiffies < ULONG_MAX) {
+		polling = true;
+		queue_delayed_work(devfreq_wq, &devfreq_work, next_jiffies);
+	} else {
+		polling = false;
+	}
+
+	mutex_unlock(&devfreq_list_lock);
+}
+
+/**
+ * devfreq_add_device() - Add devfreq feature to the device
+ * @dev:	the device to add devfreq feature.
+ * @profile:	device-specific profile to run devfreq.
+ * @governor:	the policy to choose frequency.
+ * @data:	private data for the governor. The devfreq framework does not
+ *		touch this value.
+ */
+int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
+		       struct devfreq_governor *governor, void *data)
+{
+	struct devfreq *devfreq;
+	struct srcu_notifier_head *nh;
+	int err = 0;
+
+	if (!dev || !profile || !governor) {
+		dev_err(dev, "%s: Invalid parameters.\n", __func__);
+		return -EINVAL;
+	}
+
+	mutex_lock(&devfreq_list_lock);
+
+	devfreq = find_device_devfreq(dev);
+	if (!IS_ERR(devfreq)) {
+		dev_err(dev, "%s: Unable to create devfreq for the device. It already has one.\n", __func__);
+		err = -EINVAL;
+		goto out;
+	}
+
+	devfreq = kzalloc(sizeof(struct devfreq), GFP_KERNEL);
+	if (!devfreq) {
+		dev_err(dev, "%s: Unable to create devfreq for the device\n",
+			__func__);
+		err = -ENOMEM;
+		goto out;
+	}
+
+	devfreq->dev = dev;
+	devfreq->profile = profile;
+	devfreq->governor = governor;
+	devfreq->polling_ms = profile->polling_ms;
+	devfreq->next_polling = devfreq->polling_jiffies
+			      = msecs_to_jiffies(devfreq->polling_ms);
+	devfreq->previous_freq = profile->initial_freq;
+	devfreq->data = data;
+
+	devfreq->nb.notifier_call = devfreq_update;
+	nh = opp_get_notifier(dev);
+	if (IS_ERR(nh)) {
+		err = PTR_ERR(nh);
+		goto out;
+	}
+	err = srcu_notifier_chain_register(nh, &devfreq->nb);
+	if (err)
+		goto out;
+
+	list_add(&devfreq->node, &devfreq_list);
+
+	if (devfreq_wq && devfreq->next_polling && !polling) {
+		polling = true;
+		queue_delayed_work(devfreq_wq, &devfreq_work,
+				   devfreq->next_polling);
+	}
+out:
+	mutex_unlock(&devfreq_list_lock);
+
+	return err;
+}
+
+/**
+ * devfreq_remove_device() - Remove devfreq feature from a device.
+ * @device:	the device to remove devfreq feature.
+ */
+int devfreq_remove_device(struct device *dev)
+{
+	struct devfreq *devfreq;
+	struct srcu_notifier_head *nh;
+	int err = 0;
+
+	if (!dev)
+		return -EINVAL;
+
+	mutex_lock(&devfreq_list_lock);
+	devfreq = find_device_devfreq(dev);
+	if (IS_ERR(devfreq)) {
+		err = PTR_ERR(devfreq);
+		goto out;
+	}
+
+	nh = opp_get_notifier(dev);
+	if (IS_ERR(nh)) {
+		err = PTR_ERR(nh);
+		goto out;
+	}
+
+	list_del(&devfreq->node);
+	srcu_notifier_chain_unregister(nh, &devfreq->nb);
+	kfree(devfreq);
+out:
+	mutex_unlock(&devfreq_list_lock);
+	return 0;
+}
+
+/**
+ * devfreq_init() - Initialize data structure for devfreq framework and
+ *		  start polling registered devfreq devices.
+ */
+static int __init devfreq_init(void)
+{
+	mutex_lock(&devfreq_list_lock);
+	polling = false;
+	devfreq_wq = create_freezable_workqueue("devfreq_wq");
+	INIT_DELAYED_WORK_DEFERRABLE(&devfreq_work, devfreq_monitor);
+	mutex_unlock(&devfreq_list_lock);
+
+	devfreq_monitor(&devfreq_work.work);
+	return 0;
+}
+late_initcall(devfreq_init);
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
new file mode 100644
index 0000000..18e94cb
--- /dev/null
+++ b/include/linux/devfreq.h
@@ -0,0 +1,111 @@
+/*
+ * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
+ *	    for Non-CPU Devices Based on OPP.
+ *
+ * Copyright (C) 2011 Samsung Electronics
+ *	MyungJoo Ham <myungjoo.ham@samsung.com>
+ *
+ * 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 __LINUX_DEVFREQ_H__
+#define __LINUX_DEVFREQ_H__
+
+#include <linux/notifier.h>
+
+#define DEVFREQ_NAME_LEN 16
+
+struct devfreq;
+struct devfreq_dev_status {
+	/* both since the last measure */
+	unsigned long total_time;
+	unsigned long busy_time;
+	unsigned long current_frequency;
+};
+
+struct devfreq_dev_profile {
+	unsigned long max_freq; /* may be larger than the actual value */
+	unsigned long initial_freq;
+	int polling_ms;	/* 0 for at opp change only */
+
+	int (*target)(struct device *dev, struct opp *opp);
+	int (*get_dev_status)(struct device *dev,
+			      struct devfreq_dev_status *stat);
+};
+
+/**
+ * struct devfreq_governor - Devfreq policy governor
+ * @name		Governor's name
+ * @get_target_freq	Returns desired operating frequency for the device.
+ *			Basically, get_target_freq will run
+ *			devfreq_dev_profile.get_dev_status() to get the
+ *			status of the device (load = busy_time / total_time).
+ */
+struct devfreq_governor {
+	char name[DEVFREQ_NAME_LEN];
+	int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
+};
+
+/**
+ * struct devfreq - Device devfreq structure
+ * @node	list node - contains the devices with devfreq that have been
+ *		registered.
+ * @dev		device pointer
+ * @profile	device-specific devfreq profile
+ * @governor	method how to choose frequency based on the usage.
+ * @nb		notifier block registered to the corresponding OPP to get
+ *		notified for frequency availability updates.
+ * @polling_jiffies	interval in jiffies.
+ * @polling_ms		interval in ms. to compare with profile's polling_ms
+ *			and update polling_jiffies if the device has changed
+ *			the polling interval.
+ * @previous_freq	previously configured frequency value.
+ * @next_polling	the number of remaining jiffies to poll with
+ *			"devfreq_monitor" executions to reevaluate
+ *			frequency/voltage of the device. Set by
+ *			profile's polling_ms interval.
+ * @data	Private data of the governor. The devfreq framework does not
+ *		touch this.
+ *
+ * This structure stores the devfreq information for a give device.
+ */
+struct devfreq {
+	struct list_head node;
+
+	struct device *dev;
+	struct devfreq_dev_profile *profile;
+	struct devfreq_governor *governor;
+	struct notifier_block nb;
+
+	unsigned long polling_jiffies;
+	int polling_ms;
+	unsigned long previous_freq;
+	unsigned int next_polling;
+
+	void *data; /* private data for governors */
+};
+
+#if defined(CONFIG_PM_DEVFREQ)
+extern int devfreq_add_device(struct device *dev,
+			   struct devfreq_dev_profile *profile,
+			   struct devfreq_governor *governor,
+			   void *data);
+extern int devfreq_remove_device(struct device *dev);
+#else /* !CONFIG_PM_DEVFREQ */
+static int devfreq_add_device(struct device *dev,
+			   struct devfreq_dev_profile *profile,
+			   struct devfreq_governor *governor,
+			   void *data)
+{
+	return 0;
+}
+
+static int devfreq_remove_device(struct device *dev)
+{
+	return 0;
+}
+#endif /* CONFIG_PM_DEVFREQ */
+
+#endif /* __LINUX_DEVFREQ_H__ */
-- 
1.7.4.1

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

* Re: [PATCH v6-resubmit of 1/4] PM / OPP: Add OPP availability change notifier.
  2011-08-19  1:51     ` [PATCH v6-resubmit of " MyungJoo Ham
@ 2011-08-22 17:26       ` Turquette, Mike
  0 siblings, 0 replies; 21+ messages in thread
From: Turquette, Mike @ 2011-08-22 17:26 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Thu, Aug 18, 2011 at 6:51 PM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> The patch enables to register notifier_block for an OPP-device in order
> to get notified for any changes in the availability of OPPs of the
> device. For example, if a new OPP is inserted or enable/disable status
> of an OPP is changed, the notifier is executed.
>
> This enables the usage of opp_add, opp_enable, and opp_disable to
> directly take effect with any connected entities such as CPUFREQ or
> DEVFREQ.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

Reviewed-by: Mike Turquette <mturquette@ti.com>

Looks good to me.

Mike

> ---
> Changes
> - This patch (1/4 of the set) is resubmitted for the code style revise.
> - Added at devfreq patch set v6 replacing devfreq_update calls at OPP.
> ---
>  drivers/base/power/opp.c |   29 +++++++++++++++++++++++++++++
>  include/linux/opp.h      |   12 ++++++++++++
>  2 files changed, 41 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/base/power/opp.c b/drivers/base/power/opp.c
> index b23de18..e6b4c89 100644
> --- a/drivers/base/power/opp.c
> +++ b/drivers/base/power/opp.c
> @@ -73,6 +73,7 @@ struct opp {
>  *             RCU usage: nodes are not modified in the list of device_opp,
>  *             however addition is possible and is secured by dev_opp_list_lock
>  * @dev:       device pointer
> + * @head:      notifier head to notify the OPP availability changes.
>  * @opp_list:  list of opps
>  *
>  * This is an internal data structure maintaining the link to opps attached to
> @@ -83,6 +84,7 @@ struct device_opp {
>        struct list_head node;
>
>        struct device *dev;
> +       struct srcu_notifier_head head;
>        struct list_head opp_list;
>  };
>
> @@ -404,6 +406,7 @@ int opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
>                }
>
>                dev_opp->dev = dev;
> +               srcu_init_notifier_head(&dev_opp->head);
>                INIT_LIST_HEAD(&dev_opp->opp_list);
>
>                /* Secure the device list modification */
> @@ -428,6 +431,11 @@ int opp_add(struct device *dev, unsigned long freq, unsigned long u_volt)
>        list_add_rcu(&new_opp->node, head);
>        mutex_unlock(&dev_opp_list_lock);
>
> +       /*
> +        * Notify the changes in the availability of the operable
> +        * frequency/voltage list.
> +        */
> +       srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_ADD, new_opp);
>        return 0;
>  }
>
> @@ -504,6 +512,14 @@ static int opp_set_availability(struct device *dev, unsigned long freq,
>        mutex_unlock(&dev_opp_list_lock);
>        synchronize_rcu();
>
> +       /* Notify the change of the OPP availability */
> +       if (availability_req)
> +               srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_ENABLE,
> +                                        new_opp);
> +       else
> +               srcu_notifier_call_chain(&dev_opp->head, OPP_EVENT_DISABLE,
> +                                        new_opp);
> +
>        /* clean up old opp */
>        new_opp = opp;
>        goto out;
> @@ -643,3 +659,16 @@ void opp_free_cpufreq_table(struct device *dev,
>        *table = NULL;
>  }
>  #endif         /* CONFIG_CPU_FREQ */
> +
> +/** opp_get_notifier() - find notifier_head of the device with opp
> + * @dev:       device pointer used to lookup device OPPs.
> + */
> +struct srcu_notifier_head *opp_get_notifier(struct device *dev)
> +{
> +       struct device_opp *dev_opp = find_device_opp(dev);
> +
> +       if (IS_ERR(dev_opp))
> +               return ERR_PTR(PTR_ERR(dev_opp)); /* matching type */
> +
> +       return &dev_opp->head;
> +}
> diff --git a/include/linux/opp.h b/include/linux/opp.h
> index 7020e97..87a9208 100644
> --- a/include/linux/opp.h
> +++ b/include/linux/opp.h
> @@ -16,9 +16,14 @@
>
>  #include <linux/err.h>
>  #include <linux/cpufreq.h>
> +#include <linux/notifier.h>
>
>  struct opp;
>
> +enum opp_event {
> +       OPP_EVENT_ADD, OPP_EVENT_ENABLE, OPP_EVENT_DISABLE,
> +};
> +
>  #if defined(CONFIG_PM_OPP)
>
>  unsigned long opp_get_voltage(struct opp *opp);
> @@ -40,6 +45,8 @@ int opp_enable(struct device *dev, unsigned long freq);
>
>  int opp_disable(struct device *dev, unsigned long freq);
>
> +struct srcu_notifier_head *opp_get_notifier(struct device *dev);
> +
>  #else
>  static inline unsigned long opp_get_voltage(struct opp *opp)
>  {
> @@ -89,6 +96,11 @@ static inline int opp_disable(struct device *dev, unsigned long freq)
>  {
>        return 0;
>  }
> +
> +struct srcu_notifier_head *opp_get_notifier(struct device *dev)
> +{
> +       return ERR_PTR(-EINVAL);
> +}
>  #endif         /* CONFIG_PM */
>
>  #if defined(CONFIG_CPU_FREQ) && defined(CONFIG_PM_OPP)
> --
> 1.7.4.1
>
>

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

* Re: [PATCH v6-resubmit of 2/4] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs
  2011-08-19  8:30     ` [PATCH v6-resubmit of " MyungJoo Ham
@ 2011-08-22 18:19       ` Turquette, Mike
  2011-08-23  4:08         ` MyungJoo Ham
  0 siblings, 1 reply; 21+ messages in thread
From: Turquette, Mike @ 2011-08-22 18:19 UTC (permalink / raw)
  To: MyungJoo Ham
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Fri, Aug 19, 2011 at 1:30 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> With OPPs, a device may have multiple operable frequency and voltage
> sets. However, there can be multiple possible operable sets and a system
> will need to choose one from them. In order to reduce the power
> consumption (by reducing frequency and voltage) without affecting the
> performance too much, a Dynamic Voltage and Frequency Scaling (DVFS)
> scheme may be used.
>
> This patch introduces the DVFS capability to non-CPU devices with OPPs.
> DVFS is a techique whereby the frequency and supplied voltage of a
> device is adjusted on-the-fly. DVFS usually sets the frequency as low
> as possible with given conditions (such as QoS assurance) and adjusts
> voltage according to the chosen frequency in order to reduce power
> consumption and heat dissipation.
>
> The generic DVFS for devices, DEVFREQ, may appear quite similar with
> /drivers/cpufreq.  However, CPUFREQ does not allow to have multiple
> devices registered and is not suitable to have multiple heterogenous
> devices with different (but simple) governors.
>
> Normally, DVFS mechanism controls frequency based on the demand for
> the device, and then, chooses voltage based on the chosen frequency.
> DEVFREQ also controls the frequency based on the governor's frequency
> recommendation and let OPP pick up the pair of frequency and voltage
> based on the recommended frequency. Then, the chosen OPP is passed to
> device driver's "target" callback.
>
> When PM QoS is going to be used with the DEVFREQ device, the device
> driver should enable OPPs that are appropriate with the current PM QoS
> requests. In order to do so, the device driver may call opp_enable and
> opp_disable at the notifier callback of PM QoS so that PM QoS's
> update_target() call enables the appropriate OPPs. Note that at least
> one of OPPs should be enabled at any time; be careful when there is a
> transition.
>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>
> ---
> The test code with board support for Exynos4-NURI is at
> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/devfreq
>
> ---
> Thank you for your valuable comments, Rafael, Greg, Pavel, Colin, Mike,
> and Kevin.
>
> Changes at v6-resubmit from v6
> - Use jiffy directly instead of ktime
> - Be prepared for profile->polling_ms changes (not supported fully at
>  this stage)
>
> Changes from v5
> - Uses OPP availability change notifier
> - Removed devfreq_interval. Uses one jiffy instead. DEVFREQ adjusts
>  polling interval based on the interval requirement of DEVFREQ
>  devices.
> - Moved devfreq to /drivers/devfreq to accomodate devfreq-related files
>  including governors and devfreq drivers.
> - Coding style revised.
> - Updated devfreq_add_device interface to get tunable values.
>
> Changed from v4
> - Removed tickle, which is a duplicated feature; PM QoS can do the same.
> - Allow to extend polling interval if devices have longer polling intervals.
> - Relocated private data of governors.
> - Removed system-wide sysfs
>
> Changed from v3
> - In kerneldoc comments, DEVFREQ has ben replaced by devfreq
> - Revised removing devfreq entries with error mechanism
> - Added and revised comments
> - Removed unnecessary codes
> - Allow to give a name to a governor
> - Bugfix: a tickle call may cancel an older tickle call that is still in
>  effect.
>
> Changed from v2
> - Code style revised and cleaned up.
> - Remove DEVFREQ entries that incur errors except for EAGAIN
> - Bug fixed: tickle for devices without polling governors
>
> Changes from v1(RFC)
> - Rename: DVFS --> DEVFREQ
> - Revised governor design
>        . Governor receives the whole struct devfreq
>        . Governor should gather usage information (thru get_dev_status) itself
> - Periodic monitoring runs only when needed.
> - DEVFREQ no more deals with voltage information directly
> - Removed some printks.
> - Some cosmetics update
> - Use freezable_wq.
> ---
>  drivers/Kconfig           |    2 +
>  drivers/Makefile          |    2 +
>  drivers/devfreq/Kconfig   |   39 ++++++
>  drivers/devfreq/Makefile  |    1 +
>  drivers/devfreq/devfreq.c |  312 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/devfreq.h   |  111 ++++++++++++++++
>  6 files changed, 467 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/devfreq/Kconfig
>  create mode 100644 drivers/devfreq/Makefile
>  create mode 100644 drivers/devfreq/devfreq.c
>  create mode 100644 include/linux/devfreq.h
>
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 95b9e7e..a1efd75 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -130,4 +130,6 @@ source "drivers/iommu/Kconfig"
>
>  source "drivers/virt/Kconfig"
>
> +source "drivers/devfreq/Kconfig"
> +
>  endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 7fa433a..97c957b 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -127,3 +127,5 @@ obj-$(CONFIG_IOMMU_SUPPORT) += iommu/
>
>  # Virtualization drivers
>  obj-$(CONFIG_VIRT_DRIVERS)     += virt/
> +
> +obj-$(CONFIG_PM_DEVFREQ)       += devfreq/
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> new file mode 100644
> index 0000000..1fb42de
> --- /dev/null
> +++ b/drivers/devfreq/Kconfig
> @@ -0,0 +1,39 @@
> +config ARCH_HAS_DEVFREQ
> +       bool
> +       depends on ARCH_HAS_OPP
> +       help
> +         Denotes that the architecture supports DEVFREQ. If the architecture
> +         supports multiple OPP entries per device and the frequency of the
> +         devices with OPPs may be altered dynamically, the architecture
> +         supports DEVFREQ.
> +
> +menuconfig PM_DEVFREQ
> +       bool "Generic Dynamic Voltage and Frequency Scaling (DVFS) support"
> +       depends on PM_OPP && ARCH_HAS_DEVFREQ
> +       help
> +         With OPP support, a device may have a list of frequencies and
> +         voltages available. DEVFREQ, a generic DVFS framework can be
> +         registered for a device with OPP support in order to let the
> +         governor provided to DEVFREQ choose an operating frequency
> +         based on the OPP's list and the policy given with DEVFREQ.
> +
> +         Each device may have its own governor and policy. DEVFREQ can
> +         reevaluate the device state periodically and/or based on the
> +         OPP list changes (each frequency/voltage pair in OPP may be
> +         disabled or enabled).
> +
> +         Like some CPUs with CPUFREQ, a device may have multiple clocks.
> +         However, because the clock frequencies of a single device are
> +         determined by the single device's state, an instance of DEVFREQ
> +         is attached to a single device and returns a "representative"
> +         clock frequency from the OPP of the device, which is also attached
> +         to a device by 1-to-1. The device registering DEVFREQ takes the
> +         responsiblity to "interpret" the frequency listed in OPP and
> +         to set its every clock accordingly with the "target" callback
> +         given to DEVFREQ.
> +
> +if PM_DEVFREQ
> +
> +comment "DEVFREQ Drivers"
> +
> +endif # PM_DEVFREQ
> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
> new file mode 100644
> index 0000000..168934a
> --- /dev/null
> +++ b/drivers/devfreq/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_PM_DEVFREQ)       += devfreq.o
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> new file mode 100644
> index 0000000..af848ffa
> --- /dev/null
> +++ b/drivers/devfreq/devfreq.c
> @@ -0,0 +1,312 @@
> +/*
> + * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
> + *         for Non-CPU Devices Based on OPP.
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * 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.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/errno.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/opp.h>
> +#include <linux/devfreq.h>
> +#include <linux/workqueue.h>
> +#include <linux/platform_device.h>
> +#include <linux/list.h>
> +#include <linux/printk.h>
> +#include <linux/hrtimer.h>
> +
> +/*
> + * devfreq_work periodically monitors every registered device.
> + * The minimum polling interval is one jiffy. The polling interval is
> + * determined by the minimum polling period among all polling devfreq
> + * devices. The resolution of polling interval is one jiffy.
> + */
> +static bool polling;
> +static struct workqueue_struct *devfreq_wq;
> +static struct delayed_work devfreq_work;
> +
> +/* The list of all device-devfreq */
> +static LIST_HEAD(devfreq_list);
> +static DEFINE_MUTEX(devfreq_list_lock);
> +
> +/**
> + * find_device_devfreq() - find devfreq struct using device pointer
> + * @dev:       device pointer used to lookup device devfreq.
> + *
> + * Search the list of device devfreqs and return the matched device's
> + * devfreq info. devfreq_list_lock should be held by the caller.
> + */
> +static struct devfreq *find_device_devfreq(struct device *dev)
> +{
> +       struct devfreq *tmp_devfreq;
> +
> +       if (unlikely(IS_ERR_OR_NULL(dev))) {
> +               pr_err("DEVFREQ: %s: Invalid parameters\n", __func__);
> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       list_for_each_entry(tmp_devfreq, &devfreq_list, node) {
> +               if (tmp_devfreq->dev == dev)
> +                       return tmp_devfreq;
> +       }
> +
> +       return ERR_PTR(-ENODEV);
> +}
> +
> +/**
> + * devfreq_do() - Check the usage profile of a given device and configure
> + *             frequency and voltage accordingly
> + * @devfreq:   devfreq info of the given device
> + */
> +static int devfreq_do(struct devfreq *devfreq)
> +{
> +       struct opp *opp;
> +       unsigned long freq;
> +       int err;
> +
> +       err = devfreq->governor->get_target_freq(devfreq, &freq);
> +       if (err)
> +               return err;
> +
> +       opp = opp_find_freq_ceil(devfreq->dev, &freq);
> +       if (opp == ERR_PTR(-ENODEV))
> +               opp = opp_find_freq_floor(devfreq->dev, &freq);
> +
> +       if (IS_ERR(opp))
> +               return PTR_ERR(opp);
> +
> +       if (devfreq->previous_freq == freq)
> +               return 0;
> +
> +       err = devfreq->profile->target(devfreq->dev, opp);
> +       if (err)
> +               return err;
> +
> +       devfreq->previous_freq = freq;
> +       return 0;
> +}
> +
> +/**
> + * devfreq_update() - Notify that the device OPP has been changed.
> + * @dev:       the device whose OPP has been changed.
> + */
> +static int devfreq_update(struct notifier_block *nb, unsigned long type,
> +                         void *devp)
> +{
> +       struct devfreq *devfreq;
> +       int err = 0;
> +
> +       mutex_lock(&devfreq_list_lock);
> +       devfreq = container_of(nb, struct devfreq, nb);
> +       /* Reevaluate the proper frequency */
> +       err = devfreq_do(devfreq);
> +       mutex_unlock(&devfreq_list_lock);
> +       return err;
> +}
> +
> +/**
> + * devfreq_monitor() - Periodically run devfreq_do()
> + * @work: the work struct used to run devfreq_monitor periodically.
> + *
> + */
> +static void devfreq_monitor(struct work_struct *work)
> +{
> +       static unsigned long last_polled_at;
> +       struct devfreq *devfreq, *tmp;
> +       int error;
> +       int jiffies_passed;
> +       unsigned long next_jiffies = ULONG_MAX, now = jiffies;
> +
> +       /* Initially last_polled_at = 0, polling every device at bootup */
> +       jiffies_passed = now - last_polled_at;
> +       last_polled_at = now;
> +
> +       if (jiffies_passed == 0)
> +               jiffies_passed = 1;
> +       if (jiffies_passed < 0) /* "Infinite Timeout" */
> +               jiffies_passed = INT_MAX;

This doesn't account for jiffies rollover (~49 days on architectures
with HZ == 1000).  At rollover-time some devices might incorrectly get
marked as having an infinite timeout.

> +
> +       mutex_lock(&devfreq_list_lock);
> +
> +       list_for_each_entry_safe(devfreq, tmp, &devfreq_list, node) {
> +               /* Reflect the changes in profile->polling_ms */
> +               if (devfreq->polling_ms != devfreq->profile->polling_ms) {
> +                       devfreq->polling_ms = devfreq->profile->polling_ms;
> +                       devfreq->polling_jiffies = msecs_to_jiffies(
> +                                       devfreq->polling_ms);

Does struct devfreq need ->polling_ms?  It seems like useless storage
since we're really interested in ->polling_jiffies.

How about removing devfreq->polling_ms and then just doing the
conversion whenever the sysfs file is written to?

Regards,
Mike

> +                       if (devfreq->next_polling == 0 &&
> +                           devfreq->polling_jiffies)
> +                               devfreq->next_polling = 1; /* Poll Now */
> +                       else if (devfreq->polling_jiffies == 0)
> +                               devfreq->next_polling = 0; /* Stop Polling */
> +               }
> +               if (devfreq->next_polling == 0)
> +                       continue;
> +
> +               /*
> +                * Reduce more next_polling if devfreq_wq took an extra
> +                * delay. (i.e., CPU has been idled.)
> +                */
> +               if (devfreq->next_polling <= jiffies_passed) {
> +                       error = devfreq_do(devfreq);
> +
> +                       /* Remove a devfreq with an error. */
> +                       if (error && error != -EAGAIN) {
> +                               dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
> +                                       error, devfreq->governor->name);
> +
> +                               list_del(&devfreq->node);
> +                               kfree(devfreq);
> +
> +                               continue;
> +                       }
> +                       devfreq->next_polling = devfreq->polling_jiffies;
> +
> +                       /* No more polling required (polling_ms changed) */
> +                       if (devfreq->next_polling == 0)
> +                               continue;
> +               } else {
> +                       devfreq->next_polling -= jiffies_passed;
> +               }
> +
> +               next_jiffies = (next_jiffies > devfreq->next_polling) ?
> +                               devfreq->next_polling : next_jiffies;
> +       }
> +
> +       if (next_jiffies > 0 && next_jiffies < ULONG_MAX) {
> +               polling = true;
> +               queue_delayed_work(devfreq_wq, &devfreq_work, next_jiffies);
> +       } else {
> +               polling = false;
> +       }
> +
> +       mutex_unlock(&devfreq_list_lock);
> +}
> +
> +/**
> + * devfreq_add_device() - Add devfreq feature to the device
> + * @dev:       the device to add devfreq feature.
> + * @profile:   device-specific profile to run devfreq.
> + * @governor:  the policy to choose frequency.
> + * @data:      private data for the governor. The devfreq framework does not
> + *             touch this value.
> + */
> +int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
> +                      struct devfreq_governor *governor, void *data)
> +{
> +       struct devfreq *devfreq;
> +       struct srcu_notifier_head *nh;
> +       int err = 0;
> +
> +       if (!dev || !profile || !governor) {
> +               dev_err(dev, "%s: Invalid parameters.\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       mutex_lock(&devfreq_list_lock);
> +
> +       devfreq = find_device_devfreq(dev);
> +       if (!IS_ERR(devfreq)) {
> +               dev_err(dev, "%s: Unable to create devfreq for the device. It already has one.\n", __func__);
> +               err = -EINVAL;
> +               goto out;
> +       }
> +
> +       devfreq = kzalloc(sizeof(struct devfreq), GFP_KERNEL);
> +       if (!devfreq) {
> +               dev_err(dev, "%s: Unable to create devfreq for the device\n",
> +                       __func__);
> +               err = -ENOMEM;
> +               goto out;
> +       }
> +
> +       devfreq->dev = dev;
> +       devfreq->profile = profile;
> +       devfreq->governor = governor;
> +       devfreq->polling_ms = profile->polling_ms;
> +       devfreq->next_polling = devfreq->polling_jiffies
> +                             = msecs_to_jiffies(devfreq->polling_ms);
> +       devfreq->previous_freq = profile->initial_freq;
> +       devfreq->data = data;
> +
> +       devfreq->nb.notifier_call = devfreq_update;
> +       nh = opp_get_notifier(dev);
> +       if (IS_ERR(nh)) {
> +               err = PTR_ERR(nh);
> +               goto out;
> +       }
> +       err = srcu_notifier_chain_register(nh, &devfreq->nb);
> +       if (err)
> +               goto out;
> +
> +       list_add(&devfreq->node, &devfreq_list);
> +
> +       if (devfreq_wq && devfreq->next_polling && !polling) {
> +               polling = true;
> +               queue_delayed_work(devfreq_wq, &devfreq_work,
> +                                  devfreq->next_polling);
> +       }
> +out:
> +       mutex_unlock(&devfreq_list_lock);
> +
> +       return err;
> +}
> +
> +/**
> + * devfreq_remove_device() - Remove devfreq feature from a device.
> + * @device:    the device to remove devfreq feature.
> + */
> +int devfreq_remove_device(struct device *dev)
> +{
> +       struct devfreq *devfreq;
> +       struct srcu_notifier_head *nh;
> +       int err = 0;
> +
> +       if (!dev)
> +               return -EINVAL;
> +
> +       mutex_lock(&devfreq_list_lock);
> +       devfreq = find_device_devfreq(dev);
> +       if (IS_ERR(devfreq)) {
> +               err = PTR_ERR(devfreq);
> +               goto out;
> +       }
> +
> +       nh = opp_get_notifier(dev);
> +       if (IS_ERR(nh)) {
> +               err = PTR_ERR(nh);
> +               goto out;
> +       }
> +
> +       list_del(&devfreq->node);
> +       srcu_notifier_chain_unregister(nh, &devfreq->nb);
> +       kfree(devfreq);
> +out:
> +       mutex_unlock(&devfreq_list_lock);
> +       return 0;
> +}
> +
> +/**
> + * devfreq_init() - Initialize data structure for devfreq framework and
> + *               start polling registered devfreq devices.
> + */
> +static int __init devfreq_init(void)
> +{
> +       mutex_lock(&devfreq_list_lock);
> +       polling = false;
> +       devfreq_wq = create_freezable_workqueue("devfreq_wq");
> +       INIT_DELAYED_WORK_DEFERRABLE(&devfreq_work, devfreq_monitor);
> +       mutex_unlock(&devfreq_list_lock);
> +
> +       devfreq_monitor(&devfreq_work.work);
> +       return 0;
> +}
> +late_initcall(devfreq_init);
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> new file mode 100644
> index 0000000..18e94cb
> --- /dev/null
> +++ b/include/linux/devfreq.h
> @@ -0,0 +1,111 @@
> +/*
> + * devfreq: Generic Dynamic Voltage and Frequency Scaling (DVFS) Framework
> + *         for Non-CPU Devices Based on OPP.
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
> + *
> + * 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 __LINUX_DEVFREQ_H__
> +#define __LINUX_DEVFREQ_H__
> +
> +#include <linux/notifier.h>
> +
> +#define DEVFREQ_NAME_LEN 16
> +
> +struct devfreq;
> +struct devfreq_dev_status {
> +       /* both since the last measure */
> +       unsigned long total_time;
> +       unsigned long busy_time;
> +       unsigned long current_frequency;
> +};
> +
> +struct devfreq_dev_profile {
> +       unsigned long max_freq; /* may be larger than the actual value */
> +       unsigned long initial_freq;
> +       int polling_ms; /* 0 for at opp change only */
> +
> +       int (*target)(struct device *dev, struct opp *opp);
> +       int (*get_dev_status)(struct device *dev,
> +                             struct devfreq_dev_status *stat);
> +};
> +
> +/**
> + * struct devfreq_governor - Devfreq policy governor
> + * @name               Governor's name
> + * @get_target_freq    Returns desired operating frequency for the device.
> + *                     Basically, get_target_freq will run
> + *                     devfreq_dev_profile.get_dev_status() to get the
> + *                     status of the device (load = busy_time / total_time).
> + */
> +struct devfreq_governor {
> +       char name[DEVFREQ_NAME_LEN];
> +       int (*get_target_freq)(struct devfreq *this, unsigned long *freq);
> +};
> +
> +/**
> + * struct devfreq - Device devfreq structure
> + * @node       list node - contains the devices with devfreq that have been
> + *             registered.
> + * @dev                device pointer
> + * @profile    device-specific devfreq profile
> + * @governor   method how to choose frequency based on the usage.
> + * @nb         notifier block registered to the corresponding OPP to get
> + *             notified for frequency availability updates.
> + * @polling_jiffies    interval in jiffies.
> + * @polling_ms         interval in ms. to compare with profile's polling_ms
> + *                     and update polling_jiffies if the device has changed
> + *                     the polling interval.
> + * @previous_freq      previously configured frequency value.
> + * @next_polling       the number of remaining jiffies to poll with
> + *                     "devfreq_monitor" executions to reevaluate
> + *                     frequency/voltage of the device. Set by
> + *                     profile's polling_ms interval.
> + * @data       Private data of the governor. The devfreq framework does not
> + *             touch this.
> + *
> + * This structure stores the devfreq information for a give device.
> + */
> +struct devfreq {
> +       struct list_head node;
> +
> +       struct device *dev;
> +       struct devfreq_dev_profile *profile;
> +       struct devfreq_governor *governor;
> +       struct notifier_block nb;
> +
> +       unsigned long polling_jiffies;
> +       int polling_ms;
> +       unsigned long previous_freq;
> +       unsigned int next_polling;
> +
> +       void *data; /* private data for governors */
> +};
> +
> +#if defined(CONFIG_PM_DEVFREQ)
> +extern int devfreq_add_device(struct device *dev,
> +                          struct devfreq_dev_profile *profile,
> +                          struct devfreq_governor *governor,
> +                          void *data);
> +extern int devfreq_remove_device(struct device *dev);
> +#else /* !CONFIG_PM_DEVFREQ */
> +static int devfreq_add_device(struct device *dev,
> +                          struct devfreq_dev_profile *profile,
> +                          struct devfreq_governor *governor,
> +                          void *data)
> +{
> +       return 0;
> +}
> +
> +static int devfreq_remove_device(struct device *dev)
> +{
> +       return 0;
> +}
> +#endif /* CONFIG_PM_DEVFREQ */
> +
> +#endif /* __LINUX_DEVFREQ_H__ */
> --
> 1.7.4.1
>
>

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

* Re: [PATCH v6 3/4] PM / DEVFREQ: add basic governors
  2011-08-19  5:58     ` MyungJoo Ham
@ 2011-08-22 18:22       ` Turquette, Mike
  0 siblings, 0 replies; 21+ messages in thread
From: Turquette, Mike @ 2011-08-22 18:22 UTC (permalink / raw)
  To: myungjoo.ham
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Thu, Aug 18, 2011 at 10:58 PM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> On Fri, Aug 19, 2011 at 12:12 PM, Turquette, Mike <mturquette@ti.com> wrote:
>> In patchset v5 I requested for sysfs entries to control
>> up-/down-thresholds for simple-ondemand.  Those still are not here;
>> but that got me thinking... what about multiple devices that want to
>> use simple-ondemand but have different up/down thresholds?  That's
>> when I realized that this code is monolithic and will never scale.  It
>> works today because there is one device on one platform using it.
>>
>> Below code snippet is from the Nuri code that initializes
>> devfreq/simple-ondemand for memory bus:
>> (http://git.infradead.org/users/kmpark/linux-2.6-samsung/commitdiff/d9696269b79ca172474b2c1ae84150e8ac276542?hp=7dbd341e0780ab59b2aeb85e2691ddfb1ab2fd3a)
>>
>> +static struct devfreq_dev_profile exynos4_devfreq_profile = {
>> +       .max_freq       = 400000, /* 400MHz */
>> +       .initial_freq   = 400000,
>> +       .polling_ms     = 50,
>> +       .target         = exynos4_bus_target,
>> +       .get_dev_status = exynos4_bus_get_dev_status,
>> +};
>> +static struct devfreq_governor *default_gov = &devfreq_simple_ondemand;
>>
>> This is pretty bad.  Basically only one instance of simple-ondemand
>> exists and all devices must use it.  There should really be one
>> governor instance per device.  Later there might be some optimizations
>> around grouping multiple devices together that need to scale
>> simultaneously, but lets not concern ourselves with that now.
>>
>> struct devfreq's governor member should point to a unique instance of
>> the governor.  That governor's structure should contain all of the
>> per-governor tunables:
>>
>> timekeeping (last_polled_at, etc)
>> up_threshold
>> down_threshold
>> etc.
>>
>> This in fact touches upon my previous complaints about the core code
>> managing too many of the governors responsibilities.
>>
>> Regards,
>> Mike
>>
>
>
> No, the DEVFREQ governors are tuneable per-device basis. At least,
> SIMPLE-ONDEMAND governor is capable of doing so. (just not did so in
> the example code) I'll add an example in the last paragraph.
>
>
> Although sysfs control for governors is not yet implemented--I guess
> it may wait for later versions as it does not look critical--, in this
> patchset v6, the governors are tuneable per device. Per-device data is
> stored at struct devfreq and struct devfreq_dev_profile, not in struct
> devfreq_governor. Governors can alter its behavior based on the
> "devfreq.data" and save per-device information (if any) to
> "devfreq.data". The "data" field of devfreq may be defined by each
> governor.

Ah, you're right.  I missed the use of the private data since it isn't
really used in Exynos4 example.

Regards,
Mike

>
>
> []
>>> ---
>>> Changed from v5
>>> - Seperated governor files from devfreq.c
>>> - Allow simple ondemand to be tuned for each device
>
> The example code of Exynos4210-Memory-Bus does not tune the tuneable
> value, but simply uses the default value.
>
> However, in order to use custom values, we can use devfreq_add_device
> with the custom values supplied.
>
> In the example code (exynos4210_memorybus.c), it does:
>     err = devfreq_add_device(dev, &exynos4_devfreq_profile, default_gov,
>                                          NULL);
> at line 502-503.
>
> If we are going to use custom values, we can modify that as:
>     struct devfreq_simple_ondemand_data tuneit = { .upthreshold = 80,
> .downdifferential = 10, };
>     err = devfreq_add_device(dev, &exynos4_devfreq_profile,
> default_gov, &tuneit);
>
> And, the device can update "tuneit" in run-time to alter the behavior.
>
>
>
>
> Cheers!
>
> MyungJoo
>
>
>>> ---
>>>  drivers/devfreq/Kconfig                   |   36 ++++++++++++
>>>  drivers/devfreq/Makefile                  |    4 +
>>>  drivers/devfreq/governor_performance.c    |   24 ++++++++
>>>  drivers/devfreq/governor_powersave.c      |   24 ++++++++
>>>  drivers/devfreq/governor_simpleondemand.c |   88 +++++++++++++++++++++++++++++
>>>  drivers/devfreq/governor_userspace.c      |   27 +++++++++
>>>  include/linux/devfreq.h                   |   41 +++++++++++++
>>>  7 files changed, 244 insertions(+), 0 deletions(-)
>>>  create mode 100644 drivers/devfreq/governor_performance.c
>>>  create mode 100644 drivers/devfreq/governor_powersave.c
>>>  create mode 100644 drivers/devfreq/governor_simpleondemand.c
>>>  create mode 100644 drivers/devfreq/governor_userspace.c
>>>
>>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>>> index 1fb42de..643b055 100644
>>> --- a/drivers/devfreq/Kconfig
>>> +++ b/drivers/devfreq/Kconfig
>>> @@ -34,6 +34,42 @@ menuconfig PM_DEVFREQ
>>>
>>>  if PM_DEVFREQ
>>>
>>> +comment "DEVFREQ Governors"
>>> +
>>> +config DEVFREQ_GOV_SIMPLE_ONDEMAND
>>> +       bool "Simple Ondemand"
>>> +       help
>>> +         Chooses frequency based on the recent load on the device. Works
>>> +         similar as ONDEMAND governor of CPUFREQ does. A device with
>>> +         Simple-Ondemand should be able to provide busy/total counter
>>> +         values that imply the usage rate. A device may provide tuned
>>> +         values to the governor with data field at devfreq_add_device().
>>> +
>>> +config DEVFREQ_GOV_PERFORMANCE
>>> +       bool "Performance"
>>> +       help
>>> +         Sets the frequency at the maximum available frequency.
>>> +         This governor always returns UINT_MAX as frequency so that
>>> +         the DEVFREQ framework returns the highest frequency available
>>> +         at any time.
>>> +
>>> +config DEVFREQ_GOV_POWERSAVE
>>> +       bool "Powersave"
>>> +       help
>>> +         Sets the frequency at the minimum available frequency.
>>> +         This governor always returns 0 as frequency so that
>>> +         the DEVFREQ framework returns the lowest frequency available
>>> +         at any time.
>>> +
>>> +config DEVFREQ_GOV_USERSPACE
>>> +       bool "Userspace"
>>> +       help
>>> +         Sets the frequency at the user specified one.
>>> +         This governor returns the user configured frequency if there
>>> +         has been an input to /sys/devices/.../power/devfreq_set_freq.
>>> +         Otherwise, the governor does not change the frequnecy
>>> +         given at the initialization.
>>> +
>>>  comment "DEVFREQ Drivers"
>>>
>>>  endif # PM_DEVFREQ
>>> diff --git a/drivers/devfreq/Makefile b/drivers/devfreq/Makefile
>>> index 168934a..4564a89 100644
>>> --- a/drivers/devfreq/Makefile
>>> +++ b/drivers/devfreq/Makefile
>>> @@ -1 +1,5 @@
>>>  obj-$(CONFIG_PM_DEVFREQ)       += devfreq.o
>>> +obj-$(CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND)      += governor_simpleondemand.o
>>> +obj-$(CONFIG_DEVFREQ_GOV_PERFORMANCE)  += governor_performance.o
>>> +obj-$(CONFIG_DEVFREQ_GOV_POWERSAVE)    += governor_powersave.o
>>> +obj-$(CONFIG_DEVFREQ_GOV_USERSPACE)    += governor_userspace.o
>>> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
>>> new file mode 100644
>>> index 0000000..c47eff8
>>> --- /dev/null
>>> +++ b/drivers/devfreq/governor_performance.c
>>> @@ -0,0 +1,24 @@
>>> +/*
>>> + *  linux/drivers/devfreq/governor_performance.c
>>> + *
>>> + *  Copyright (C) 2011 Samsung Electronics
>>> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#include <linux/devfreq.h>
>>> +
>>> +static int devfreq_performance_func(struct devfreq *df,
>>> +                                   unsigned long *freq)
>>> +{
>>> +       *freq = UINT_MAX; /* devfreq_do will run "floor" */
>>> +       return 0;
>>> +}
>>> +
>>> +struct devfreq_governor devfreq_performance = {
>>> +       .name = "performance",
>>> +       .get_target_freq = devfreq_performance_func,
>>> +};
>>> diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
>>> new file mode 100644
>>> index 0000000..4f128d8
>>> --- /dev/null
>>> +++ b/drivers/devfreq/governor_powersave.c
>>> @@ -0,0 +1,24 @@
>>> +/*
>>> + *  linux/drivers/devfreq/governor_powersave.c
>>> + *
>>> + *  Copyright (C) 2011 Samsung Electronics
>>> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#include <linux/devfreq.h>
>>> +
>>> +static int devfreq_powersave_func(struct devfreq *df,
>>> +                                 unsigned long *freq)
>>> +{
>>> +       *freq = 0; /* devfreq_do will run "ceiling" to 0 */
>>> +       return 0;
>>> +}
>>> +
>>> +struct devfreq_governor devfreq_powersave = {
>>> +       .name = "powersave",
>>> +       .get_target_freq = devfreq_powersave_func,
>>> +};
>>> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
>>> new file mode 100644
>>> index 0000000..18fe8be
>>> --- /dev/null
>>> +++ b/drivers/devfreq/governor_simpleondemand.c
>>> @@ -0,0 +1,88 @@
>>> +/*
>>> + *  linux/drivers/devfreq/governor_simpleondemand.c
>>> + *
>>> + *  Copyright (C) 2011 Samsung Electronics
>>> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#include <linux/errno.h>
>>> +#include <linux/devfreq.h>
>>> +#include <linux/math64.h>
>>> +
>>> +/* Default constants for DevFreq-Simple-Ondemand (DFSO) */
>>> +#define DFSO_UPTHRESHOLD       (90)
>>> +#define DFSO_DOWNDIFFERENCTIAL (5)
>>> +static int devfreq_simple_ondemand_func(struct devfreq *df,
>>> +                                       unsigned long *freq)
>>> +{
>>> +       struct devfreq_dev_status stat;
>>> +       int err = df->profile->get_dev_status(df->dev, &stat);
>>> +       unsigned long long a, b;
>>> +       unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
>>> +       unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
>>> +       struct devfreq_simple_ondemand_data *data = df->data;
>>> +
>>> +       if (err)
>>> +               return err;
>>> +
>>> +       if (data) {
>>> +               if (data->upthreshold)
>>> +                       dfso_upthreshold = data->upthreshold;
>>> +               if (data->downdifferential)
>>> +                       dfso_downdifferential = data->downdifferential;
>>> +       }
>>> +       if (dfso_upthreshold > 100 ||
>>> +           dfso_upthreshold < dfso_downdifferential)
>>> +               return -EINVAL;
>>> +
>>> +       /* Assume MAX if it is going to be divided by zero */
>>> +       if (stat.total_time == 0) {
>>> +               *freq = UINT_MAX;
>>> +               return 0;
>>> +       }
>>> +
>>> +       /* Prevent overflow */
>>> +       if (stat.busy_time >= (1 << 24) || stat.total_time >= (1 << 24)) {
>>> +               stat.busy_time >>= 7;
>>> +               stat.total_time >>= 7;
>>> +       }
>>> +
>>> +       /* Set MAX if it's busy enough */
>>> +       if (stat.busy_time * 100 >
>>> +           stat.total_time * dfso_upthreshold) {
>>> +               *freq = UINT_MAX;
>>> +               return 0;
>>> +       }
>>> +
>>> +       /* Set MAX if we do not know the initial frequency */
>>> +       if (stat.current_frequency == 0) {
>>> +               *freq = UINT_MAX;
>>> +               return 0;
>>> +       }
>>> +
>>> +       /* Keep the current frequency */
>>> +       if (stat.busy_time * 100 >
>>> +           stat.total_time * (dfso_upthreshold - dfso_downdifferential)) {
>>> +               *freq = stat.current_frequency;
>>> +               return 0;
>>> +       }
>>> +
>>> +       /* Set the desired frequency based on the load */
>>> +       a = stat.busy_time;
>>> +       a *= stat.current_frequency;
>>> +       b = div_u64(a, stat.total_time);
>>> +       b *= 100;
>>> +       b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
>>> +       *freq = (unsigned long) b;
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +struct devfreq_governor devfreq_simple_ondemand = {
>>> +       .name = "simple_ondemand",
>>> +       .get_target_freq = devfreq_simple_ondemand_func,
>>> +};
>>> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
>>> new file mode 100644
>>> index 0000000..6af8b6d
>>> --- /dev/null
>>> +++ b/drivers/devfreq/governor_userspace.c
>>> @@ -0,0 +1,27 @@
>>> +/*
>>> + *  linux/drivers/devfreq/governor_simpleondemand.c
>>> + *
>>> + *  Copyright (C) 2011 Samsung Electronics
>>> + *     MyungJoo Ham <myungjoo.ham@samsung.com>
>>> + *
>>> + * 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.
>>> + */
>>> +
>>> +#include <linux/devfreq.h>
>>> +
>>> +static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
>>> +{
>>> +       if (df->user_set_freq == 0)
>>> +               *freq = df->previous_freq; /* No user freq specified yet */
>>> +       else
>>> +               *freq = df->user_set_freq;
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +struct devfreq_governor devfreq_userspace = {
>>> +       .name = "userspace",
>>> +       .get_target_freq = devfreq_userspace_func,
>>> +};
>>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>>> index 13ddf49..d088ad3 100644
>>> --- a/include/linux/devfreq.h
>>> +++ b/include/linux/devfreq.h
>>> @@ -13,6 +13,7 @@
>>>  #ifndef __LINUX_DEVFREQ_H__
>>>  #define __LINUX_DEVFREQ_H__
>>>
>>> +#include <linux/opp.h>
>>>  #include <linux/notifier.h>
>>>
>>>  #define DEVFREQ_NAME_LEN 16
>>> @@ -62,6 +63,8 @@ struct devfreq_governor {
>>>  *                     "devfreq_monitor" executions to reevaluate
>>>  *                     frequency/voltage of the device. Set by
>>>  *                     profile's polling_ms interval.
>>> + * @user_set_freq      User specified adequete frequency value (thru sysfs
>>> + *             interface). Governors may and may not use this value.
>>>  * @data       Private data of the governor. The devfreq framework does not
>>>  *             touch this.
>>>  *
>>> @@ -78,6 +81,7 @@ struct devfreq {
>>>        unsigned long previous_freq;
>>>        unsigned int next_polling;
>>>
>>> +       unsigned long user_set_freq; /* governors may ignore this. */
>>>        void *data; /* private data for governors */
>>>  };
>>>
>>> @@ -87,6 +91,37 @@ extern int devfreq_add_device(struct device *dev,
>>>                           struct devfreq_governor *governor,
>>>                           void *data);
>>>  extern int devfreq_remove_device(struct device *dev);
>>> +
>>> +#ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
>>> +extern struct devfreq_governor devfreq_powersave;
>>> +#endif
>>> +#ifdef CONFIG_DEVFREQ_GOV_PERFORMANCE
>>> +extern struct devfreq_governor devfreq_performance;
>>> +#endif
>>> +#ifdef CONFIG_DEVFREQ_GOV_USERSPACE
>>> +extern struct devfreq_governor devfreq_userspace;
>>> +#endif
>>> +#ifdef CONFIG_DEVFREQ_GOV_SIMPLE_ONDEMAND
>>> +extern struct devfreq_governor devfreq_simple_ondemand;
>>> +/**
>>> + * struct devfreq_simple_ondemand_data - void *data fed to struct devfreq
>>> + *     and devfreq_add_device
>>> + * @ upthreshold       If the load is over this value, the frequency jumps.
>>> + *                     Specify 0 to use the default. Valid value = 0 to 100.
>>> + * @ downdifferential  If the load is under upthreshold - downdifferential,
>>> + *                     the governor may consider slowing the frequency down.
>>> + *                     Specify 0 to use the default. Valid value = 0 to 100.
>>> + *                     downdifferential < upthreshold must hold.
>>> + *
>>> + * If the fed devfreq_simple_ondemand_data pointer is NULL to the governor,
>>> + * the governor uses the default values.
>>> + */
>>> +struct devfreq_simple_ondemand_data {
>>> +       unsigned int upthreshold;
>>> +       unsigned int downdifferential;
>>> +};
>>> +#endif
>>> +
>>>  #else /* !CONFIG_PM_DEVFREQ */
>>>  static int devfreq_add_device(struct device *dev,
>>>                           struct devfreq_dev_profile *profile,
>>> @@ -100,6 +135,12 @@ static int devfreq_remove_device(struct device *dev)
>>>  {
>>>        return 0;
>>>  }
>>> +
>>> +#define devfreq_powersave      NULL
>>> +#define devfreq_performance    NULL
>>> +#define devfreq_userspace      NULL
>>> +#define devfreq_simple_ondemand        NULL
>>> +
>>>  #endif /* CONFIG_PM_DEVFREQ */
>>>
>>>  #endif /* __LINUX_DEVFREQ_H__ */
>>> --
>>> 1.7.4.1
>>>
>>>
>>
>
>
>
> --
> MyungJoo Ham (함명주), Ph.D.
> Mobile Software Platform Lab,
> Digital Media and Communications (DMC) Business
> Samsung Electronics
> cell: 82-10-6714-2858
>
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

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

* Re: [PATCH v6 4/4] PM / DEVFREQ: add sysfs interface
  2011-08-19  7:28     ` MyungJoo Ham
@ 2011-08-22 18:28       ` Turquette, Mike
  0 siblings, 0 replies; 21+ messages in thread
From: Turquette, Mike @ 2011-08-22 18:28 UTC (permalink / raw)
  To: myungjoo.ham
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Fri, Aug 19, 2011 at 12:28 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> On Fri, Aug 19, 2011 at 12:15 PM, Turquette, Mike <mturquette@ti.com> wrote:
>> On Thu, Aug 18, 2011 at 3:27 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>>> Device specific sysfs interface /sys/devices/.../power/devfreq_*
>>> - governor      R: name of governor
>>> - cur_freq      R: current frequency
>>> - max_freq      R: maximum operable frequency
>>> - min_freq      R: minimum operable frequency
>>> - set_freq      R: read user specified frequency (0 if not specified yet)
>>>                W: set user specified frequency
>>> - polling_interval      R: polling interval in ms given with devfreq profile
>>
>> This code still does not export up_threshold/down_threshold entries
>> for simple-ondemand which I requested in V5.  However there is a much
>> bigger issue here brought out in patch 3.  Those values shouldn't be
>> global, since devices might want different thresholds.
>>
>> The solution is not to put those tunables in struct devfreq.
>> up_threshold and down_threshold must be parameters of the governor,
>> and there must be a per-device instance of the governor.
>>
>> Regards,
>> Mike
>
> As in the example in the reply to the thread of patch 3/4 of v6
> patchset, we do not need to create another per-device object in
> DEVFREQ in order to allow governors to be tuned per device. The global
> values you've mentioned are only used as default and each instance of
> devfreq (per device) can specify its own tuneable values and they can
> alter the values in run-time.
>
> For the sysfs interfaces of governor-specific tunable values, I'd like
> to see that as a "future work" and that would be implemented
> per-governor basis; i.e., probably under
> /sysfs/devices/.../power/NAME_OF_GOVERNOR/variable_name. Or should I
> regard the sysfs interface to tune governors at userland is somewhat
> essential and critical for the initial release?

I guess it is not critical but I feel that implementing all of the
sysfs bits will reveal some designs issues.

E.g. by pushing threshold data into the device's private_data you're
crossing data paths with the governors: the private data tunable must
be associated per-device in sysfs, but the read/write functions must
come from the governor.  If the core code assumes too much about what
private data looks like (up_threshold, down_threshold, etc) then it
might be limiting to future governor authors.

The patches are shaping up however and I won't consider sysfs support
a blocker.  Rafael's point about revisiting design at a later date is
good guidance here.

Regards,
Mike

> Cheers!
>
> MyungJoo
>
>>
>>>
>>> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
>>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>>
>>> --
>>> Changed from v5
>>> - updated devferq_update usage.
>>>
>>> Changed from v4
>>> - removed system-wide sysfs interface
>>> - removed tickling sysfs interface
>>> - added set_freq for userspace governor (and any other governors that
>>>  require user input)
>>>
>>> Changed from v3
>>> - corrected sysfs API usage
>>> - corrected error messages
>>> - moved sysfs entry location
>>> - added sysfs entries
>>>
>>> Changed from v2
>>> - add ABI entries for devfreq sysfs interface
>>> ---
>>>  Documentation/ABI/testing/sysfs-devices-power |   45 +++++++
>>>  drivers/devfreq/devfreq.c                     |  152 +++++++++++++++++++++++++
>>>  2 files changed, 197 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-devices-power b/Documentation/ABI/testing/sysfs-devices-power
>>> index 8ffbc25..6e77604 100644
>>> --- a/Documentation/ABI/testing/sysfs-devices-power
>>> +++ b/Documentation/ABI/testing/sysfs-devices-power
>>> @@ -165,3 +165,48 @@ Description:
>>>
>>>                Not all drivers support this attribute.  If it isn't supported,
>>>                attempts to read or write it will yield I/O errors.
>>> +
>>> +What:          /sys/devices/.../power/devfreq_governor
>>> +Date:          July 2011
>>> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
>>> +Description:
>>> +               The /sys/devices/.../power/devfreq_governor shows the name
>>> +               of the governor used by the corresponding device.
>>> +
>>> +What:          /sys/devices/.../power/devfreq_cur_freq
>>> +Date:          July 2011
>>> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
>>> +Description:
>>> +               The /sys/devices/.../power/devfreq_cur_freq shows the current
>>> +               frequency of the corresponding device.
>>> +
>>> +What:          /sys/devices/.../power/devfreq_max_freq
>>> +Date:          July 2011
>>> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
>>> +Description:
>>> +               The /sys/devices/.../power/devfreq_cur_freq shows the
>>> +               maximum operable frequency of the corresponding device.
>>> +
>>> +What:          /sys/devices/.../power/devfreq_min_freq
>>> +Date:          July 2011
>>> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
>>> +Description:
>>> +               The /sys/devices/.../power/devfreq_cur_freq shows the
>>> +               minimum operable frequency of the corresponding device.
>>> +
>>> +What:          /sys/devices/.../power/devfreq_set_freq
>>> +Date:          August 2011
>>> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
>>> +Description:
>>> +               The /sys/devices/.../power/devfreq_set_freq sets and shows
>>> +               the user specified desired frequency of the device. The
>>> +               governor may and may not use the value. With the basic
>>> +               governors given with devfreq.c, userspace governor is
>>> +               using the value.
>>> +
>>> +What:          /sys/devices/.../power/devfreq_polling_interval
>>> +Date:          July 2011
>>> +Contact:       MyungJoo Ham <myungjoo.ham@samsung.com>
>>> +Description:
>>> +               The /sys/devices/.../power/devfreq_polling_interval shows the
>>> +               requested polling interval of the corresponding device.
>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>> index 2036f2c..13b422f 100644
>>> --- a/drivers/devfreq/devfreq.c
>>> +++ b/drivers/devfreq/devfreq.c
>>> @@ -37,6 +37,8 @@ static struct delayed_work devfreq_work;
>>>  static LIST_HEAD(devfreq_list);
>>>  static DEFINE_MUTEX(devfreq_list_lock);
>>>
>>> +static struct attribute_group dev_attr_group;
>>> +
>>>  /**
>>>  * find_device_devfreq() - find devfreq struct using device pointer
>>>  * @dev:       device pointer used to lookup device devfreq.
>>> @@ -154,6 +156,8 @@ static void devfreq_monitor(struct work_struct *work)
>>>                                dev_err(devfreq->dev, "Due to devfreq_do error(%d), devfreq(%s) is removed from the device\n",
>>>                                        error, devfreq->governor->name);
>>>
>>> +                               sysfs_unmerge_group(&devfreq->dev->kobj,
>>> +                                                   &dev_attr_group);
>>>                                list_del(&devfreq->node);
>>>                                kfree(devfreq);
>>>
>>> @@ -244,6 +248,8 @@ int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile,
>>>                queue_delayed_work(devfreq_wq, &devfreq_work,
>>>                                   devfreq->next_polling);
>>>        }
>>> +
>>> +       sysfs_merge_group(&dev->kobj, &dev_attr_group);
>>>  out:
>>>        mutex_unlock(&devfreq_list_lock);
>>>
>>> @@ -276,6 +282,8 @@ int devfreq_remove_device(struct device *dev)
>>>                goto out;
>>>        }
>>>
>>> +       sysfs_unmerge_group(&dev->kobj, &dev_attr_group);
>>> +
>>>        list_del(&devfreq->node);
>>>        srcu_notifier_chain_unregister(nh, &devfreq->nb);
>>>        kfree(devfreq);
>>> @@ -284,6 +292,150 @@ out:
>>>        return 0;
>>>  }
>>>
>>> +static ssize_t show_governor(struct device *dev,
>>> +                            struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct devfreq *df = find_device_devfreq(dev);
>>> +
>>> +       if (IS_ERR(df))
>>> +               return PTR_ERR(df);
>>> +       if (!df->governor)
>>> +               return -EINVAL;
>>> +
>>> +       return sprintf(buf, "%s\n", df->governor->name);
>>> +}
>>> +
>>> +static ssize_t show_freq(struct device *dev,
>>> +                        struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct devfreq *df = find_device_devfreq(dev);
>>> +
>>> +       if (IS_ERR(df))
>>> +               return PTR_ERR(df);
>>> +
>>> +       return sprintf(buf, "%lu\n", df->previous_freq);
>>> +}
>>> +
>>> +static ssize_t show_max_freq(struct device *dev,
>>> +                            struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct devfreq *df = find_device_devfreq(dev);
>>> +       unsigned long freq = ULONG_MAX;
>>> +       struct opp *opp;
>>> +
>>> +       if (IS_ERR(df))
>>> +               return PTR_ERR(df);
>>> +       if (!df->dev)
>>> +               return -EINVAL;
>>> +
>>> +       opp = opp_find_freq_floor(df->dev, &freq);
>>> +       if (IS_ERR(opp))
>>> +               return PTR_ERR(opp);
>>> +
>>> +       return sprintf(buf, "%lu\n", freq);
>>> +}
>>> +
>>> +static ssize_t show_min_freq(struct device *dev,
>>> +                            struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct devfreq *df = find_device_devfreq(dev);
>>> +       unsigned long freq = 0;
>>> +       struct opp *opp;
>>> +
>>> +       if (IS_ERR(df))
>>> +               return PTR_ERR(df);
>>> +       if (!df->dev)
>>> +               return -EINVAL;
>>> +
>>> +       opp = opp_find_freq_ceil(df->dev, &freq);
>>> +       if (IS_ERR(opp))
>>> +               return PTR_ERR(opp);
>>> +
>>> +       return sprintf(buf, "%lu\n", freq);
>>> +}
>>> +
>>> +static ssize_t show_polling_interval(struct device *dev,
>>> +                                    struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct devfreq *df = find_device_devfreq(dev);
>>> +
>>> +       if (IS_ERR(df))
>>> +               return PTR_ERR(df);
>>> +       if (!df->profile)
>>> +               return -EINVAL;
>>> +
>>> +       return sprintf(buf, "%d\n", df->profile->polling_ms);
>>> +}
>>> +
>>> +static ssize_t set_user_frequency(struct device *dev,
>>> +                                 struct device_attribute *attr,
>>> +                                 const char *buf, size_t count)
>>> +{
>>> +       struct devfreq *devfreq;
>>> +       unsigned long wanted;
>>> +       int err = 0;
>>> +
>>> +       sscanf(buf, "%lu", &wanted);
>>> +
>>> +       mutex_lock(&devfreq_list_lock);
>>> +       devfreq = find_device_devfreq(dev);
>>> +
>>> +       if (IS_ERR(devfreq)) {
>>> +               err = PTR_ERR(devfreq);
>>> +               goto out;
>>> +       }
>>> +
>>> +       devfreq->user_set_freq = wanted;
>>> +       err = count;
>>> +out:
>>> +       mutex_unlock(&devfreq_list_lock);
>>> +       if (err >= 0)
>>> +               devfreq_update(&devfreq->nb, 0, NULL);
>>> +       return err;
>>> +}
>>> +
>>> +static ssize_t show_user_frequency(struct device *dev,
>>> +                                  struct device_attribute *attr, char *buf)
>>> +{
>>> +       struct devfreq *devfreq;
>>> +       int err = 0;
>>> +
>>> +       mutex_lock(&devfreq_list_lock);
>>> +       devfreq = find_device_devfreq(dev);
>>> +
>>> +       if (IS_ERR(devfreq)) {
>>> +               err = PTR_ERR(devfreq);
>>> +               goto out;
>>> +       }
>>> +
>>> +       err = sprintf(buf, "%lu\n", devfreq->user_set_freq);
>>> +out:
>>> +       mutex_unlock(&devfreq_list_lock);
>>> +       return err;
>>> +
>>> +}
>>> +
>>> +static DEVICE_ATTR(devfreq_governor, 0444, show_governor, NULL);
>>> +static DEVICE_ATTR(devfreq_cur_freq, 0444, show_freq, NULL);
>>> +static DEVICE_ATTR(devfreq_max_freq, 0444, show_max_freq, NULL);
>>> +static DEVICE_ATTR(devfreq_min_freq, 0444, show_min_freq, NULL);
>>> +static DEVICE_ATTR(devfreq_polling_interval, 0444, show_polling_interval, NULL);
>>> +static DEVICE_ATTR(devfreq_set_freq, 0644, show_user_frequency,
>>> +                  set_user_frequency);
>>> +static struct attribute *dev_entries[] = {
>>> +       &dev_attr_devfreq_governor.attr,
>>> +       &dev_attr_devfreq_cur_freq.attr,
>>> +       &dev_attr_devfreq_max_freq.attr,
>>> +       &dev_attr_devfreq_min_freq.attr,
>>> +       &dev_attr_devfreq_polling_interval.attr,
>>> +       &dev_attr_devfreq_set_freq.attr,
>>> +       NULL,
>>> +};
>>> +static struct attribute_group dev_attr_group = {
>>> +       .name   = power_group_name,
>>> +       .attrs  = dev_entries,
>>> +};
>>> +
>>>  /**
>>>  * devfreq_init() - Initialize data structure for devfreq framework and
>>>  *               start polling registered devfreq devices.
>>> --
>>> 1.7.4.1
>>>
>>>
>>
>
>
>
> --
> MyungJoo Ham (함명주), Ph.D.
> Mobile Software Platform Lab,
> Digital Media and Communications (DMC) Business
> Samsung Electronics
> cell: 82-10-6714-2858
>
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

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

* Re: [PATCH v6-resubmit of 2/4] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs
  2011-08-22 18:19       ` Turquette, Mike
@ 2011-08-23  4:08         ` MyungJoo Ham
  2011-08-23  4:48           ` MyungJoo Ham
  0 siblings, 1 reply; 21+ messages in thread
From: MyungJoo Ham @ 2011-08-23  4:08 UTC (permalink / raw)
  To: Turquette, Mike
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Tue, Aug 23, 2011 at 3:19 AM, Turquette, Mike <mturquette@ti.com> wrote:
> On Fri, Aug 19, 2011 at 1:30 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>> +
>> +       if (jiffies_passed == 0)
>> +               jiffies_passed = 1;
>> +       if (jiffies_passed < 0) /* "Infinite Timeout" */
>> +               jiffies_passed = INT_MAX;
>
> This doesn't account for jiffies rollover (~49 days on architectures
> with HZ == 1000).  At rollover-time some devices might incorrectly get
> marked as having an infinite timeout.

Yes, that is to be removed and jiffies_passed appears to be better
being unsigned long, not int.

>
>> +
>> +       mutex_lock(&devfreq_list_lock);
>> +
>> +       list_for_each_entry_safe(devfreq, tmp, &devfreq_list, node) {
>> +               /* Reflect the changes in profile->polling_ms */
>> +               if (devfreq->polling_ms != devfreq->profile->polling_ms) {
>> +                       devfreq->polling_ms = devfreq->profile->polling_ms;
>> +                       devfreq->polling_jiffies = msecs_to_jiffies(
>> +                                       devfreq->polling_ms);
>
> Does struct devfreq need ->polling_ms?  It seems like useless storage
> since we're really interested in ->polling_jiffies.
>
> How about removing devfreq->polling_ms and then just doing the
> conversion whenever the sysfs file is written to?

It we assume that profile->polling_ms is never changed by either the
driver or the governor, we can remove it in devfreq.
It was added to struct devfreq in order to see if the value has been
changed and get polling_jiffies updated accordingly.
Although there is no "store" function for polling_ms in sysfs, the
drivers and governors are able to update the value at any time.

>
> Regards,
> Mike
>

I'll send "v7" patch with some minor updates soon. It appears that
devfreq.polling_ms would be better unsigned along with jiffies_passed.

Thanks

MyungJoo

-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

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

* Re: [PATCH v6-resubmit of 2/4] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs
  2011-08-23  4:08         ` MyungJoo Ham
@ 2011-08-23  4:48           ` MyungJoo Ham
  0 siblings, 0 replies; 21+ messages in thread
From: MyungJoo Ham @ 2011-08-23  4:48 UTC (permalink / raw)
  To: Turquette, Mike
  Cc: Len Brown, Greg Kroah-Hartman, Kyungmin Park, linux-pm, Thomas Gleixner

On Tue, Aug 23, 2011 at 1:08 PM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> On Tue, Aug 23, 2011 at 3:19 AM, Turquette, Mike <mturquette@ti.com> wrote:
>> On Fri, Aug 19, 2011 at 1:30 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>>> +
>>> +       if (jiffies_passed == 0)
>>> +               jiffies_passed = 1;
>>> +       if (jiffies_passed < 0) /* "Infinite Timeout" */
>>> +               jiffies_passed = INT_MAX;
>>
>> This doesn't account for jiffies rollover (~49 days on architectures
>> with HZ == 1000).  At rollover-time some devices might incorrectly get
>> marked as having an infinite timeout.
>
> Yes, that is to be removed and jiffies_passed appears to be better
> being unsigned long, not int.
>
>>
>>> +
>>> +       mutex_lock(&devfreq_list_lock);
>>> +
>>> +       list_for_each_entry_safe(devfreq, tmp, &devfreq_list, node) {
>>> +               /* Reflect the changes in profile->polling_ms */
>>> +               if (devfreq->polling_ms != devfreq->profile->polling_ms) {
>>> +                       devfreq->polling_ms = devfreq->profile->polling_ms;
>>> +                       devfreq->polling_jiffies = msecs_to_jiffies(
>>> +                                       devfreq->polling_ms);
>>
>> Does struct devfreq need ->polling_ms?  It seems like useless storage
>> since we're really interested in ->polling_jiffies.
>>
>> How about removing devfreq->polling_ms and then just doing the
>> conversion whenever the sysfs file is written to?
>
> It we assume that profile->polling_ms is never changed by either the
> driver or the governor, we can remove it in devfreq.
> It was added to struct devfreq in order to see if the value has been
> changed and get polling_jiffies updated accordingly.
> Although there is no "store" function for polling_ms in sysfs, the
> drivers and governors are able to update the value at any time.

Plase never mind this paragraph. I'll remove that devfre->polling_ms
and make an sysfs entry to update polling_ms properly with soon-to-be
release v7 patchset. It looks more beautiful that way to me either.

>
>>
>> Regards,
>> Mike
>>
>
> I'll send "v7" patch with some minor updates soon. It appears that
> devfreq.polling_ms would be better unsigned along with jiffies_passed.
>
> Thanks
>
> MyungJoo
>
> --
> MyungJoo Ham (함명주), Ph.D.
> Mobile Software Platform Lab,
> Digital Media and Communications (DMC) Business
> Samsung Electronics
> cell: 82-10-6714-2858
>



-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

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

end of thread, other threads:[~2011-08-23  4:48 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-18 10:27 [PATCH v6 0/4] DEVFREQ, DVFS Framework for Non-CPU Devices MyungJoo Ham
2011-08-18 10:27 ` [PATCH v6 1/4] PM / OPP: Add OPP availability change notifier MyungJoo Ham
2011-08-18 22:06   ` Turquette, Mike
2011-08-19  1:42     ` MyungJoo Ham
2011-08-19  1:51     ` [PATCH v6-resubmit of " MyungJoo Ham
2011-08-22 17:26       ` Turquette, Mike
2011-08-18 10:27 ` [PATCH v6 2/4] PM: Introduce DEVFREQ: generic DVFS framework with device-specific OPPs MyungJoo Ham
2011-08-19  2:43   ` Turquette, Mike
2011-08-19  8:29     ` MyungJoo Ham
2011-08-19  8:30     ` [PATCH v6-resubmit of " MyungJoo Ham
2011-08-22 18:19       ` Turquette, Mike
2011-08-23  4:08         ` MyungJoo Ham
2011-08-23  4:48           ` MyungJoo Ham
2011-08-18 10:27 ` [PATCH v6 3/4] PM / DEVFREQ: add basic governors MyungJoo Ham
2011-08-19  3:12   ` Turquette, Mike
2011-08-19  5:58     ` MyungJoo Ham
2011-08-22 18:22       ` Turquette, Mike
2011-08-18 10:27 ` [PATCH v6 4/4] PM / DEVFREQ: add sysfs interface MyungJoo Ham
2011-08-19  3:15   ` Turquette, Mike
2011-08-19  7:28     ` MyungJoo Ham
2011-08-22 18:28       ` Turquette, Mike

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.