On 10 September 2012 03:16, Rafael J. Wysocki rjw@sisk.pl wrote:
On Monday, September 03, 2012, Rajagopal Venkat wrote:
Prepare devfreq core framework to support devices which can idle. When device idleness is detected perhaps through runtime-pm, need some mechanism to suspend devfreq load monitoring and resume back when device is online. Present code continues monitoring unless device is removed from devfreq core.
This patch introduces following design changes,
- use per device work instead of global work to monitor device load. This enables suspend/resume of device devfreq and reduces monitoring code complexity.
- decouple delayed work based load monitoring logic from core by introducing helpers functions to be used by governors. This provides flexibility for governors either to use delayed work based monitoring functions or to implement their own mechanism.
- devfreq core interacts with governors via events to perform specific actions. These events include start/stop devfreq. This sets ground for adding suspend/resume events.
The devfreq apis are not modified and are kept intact.
Signed-off-by: Rajagopal Venkat rajagopal.venkat@linaro.org
This one looks like a nice simplification. I wonder if everyone in the CC list is fine with it?
One remark below.
drivers/devfreq/devfreq.c | 376 ++++++++++-------------------- drivers/devfreq/governor.h | 9 + drivers/devfreq/governor_performance.c | 16 +- drivers/devfreq/governor_powersave.c | 16 +- drivers/devfreq/governor_simpleondemand.c | 33 +++ drivers/devfreq/governor_userspace.c | 23 +- include/linux/devfreq.h | 31 +-- 7 files changed, 220 insertions(+), 284 deletions(-)
diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index b146d76..be524c7 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -30,17 +30,11 @@ struct class *devfreq_class;
/*
- 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.
- devfreq core provides delayed work based load monitoring helper
- functions. Governors can use these or can implement their own
*/
- monitoring mechanism.
-static bool polling; static struct workqueue_struct *devfreq_wq; -static struct delayed_work devfreq_work;
-/* wait removing if this is to be removed */ -static struct devfreq *wait_remove_device;
/* The list of all device-devfreq */ static LIST_HEAD(devfreq_list); @@ -72,6 +66,8 @@ static struct devfreq *find_device_devfreq(struct device *dev) return ERR_PTR(-ENODEV); }
+/* Load monitoring helper functions for governors use */
/**
- update_devfreq() - Reevaluate the device and configure frequency.
- @devfreq: the devfreq instance.
@@ -121,6 +117,90 @@ int update_devfreq(struct devfreq *devfreq) }
/**
- devfreq_monitor() - Periodically poll devfreq objects.
- @work: the work struct used to run devfreq_monitor periodically.
- */
+static void devfreq_monitor(struct work_struct *work) +{
int err;struct devfreq *devfreq = container_of(work,struct devfreq, work.work);mutex_lock(&devfreq->lock);err = update_devfreq(devfreq);mutex_unlock(&devfreq->lock);if (err)dev_err(&devfreq->dev, "dvfs failed with (%d) error\n", err);queue_delayed_work(devfreq_wq, &devfreq->work,msecs_to_jiffies(devfreq->profile->polling_ms));+}
+/**
- devfreq_monitor_start() - Start load monitoring of devfreq instance
using default delayed work
- @devfreq: the devfreq instance.
- Returns 0 if monitoring started, non-zero otherwise.
- Note: This function is exported for governors.
- */
+int devfreq_monitor_start(struct devfreq *devfreq) +{
INIT_DEFERRABLE_WORK(&devfreq->work, devfreq_monitor);return !queue_delayed_work(devfreq_wq, &devfreq->work,msecs_to_jiffies(devfreq->profile->polling_ms));+}
+/**
- devfreq_monitor_stop() - Stop load monitoring of a devfreq instance
- @devfreq: the devfreq instance.
- Note: This function is exported for governors.
- */
+void devfreq_monitor_stop(struct devfreq *devfreq) +{
cancel_delayed_work_sync(&devfreq->work);+}
+/**
- devfreq_monitor_suspend() - Suspend load monitoring of a devfreq instance
- @devfreq: the devfreq instance.
- Note: This function is exported for governors.
- */
It would be good to say in the kerneldoc comment what the idea is, because it is not particularly clear from the code. In particular, why can't devfreq_monitor_suspend() be the same as devfreq_monitor_stop()?
Ok. Kerneldoc comment will be added here.
The idea is, devfreq_monitor_suspend() - called to suspend device devfreq during device idleness.
devfreq_monitor_stop() - called when device is removed from the devfreq framework.
Though these two functions can be same, intentionally I kept them separate to provide hooks for collecting transition statistics.
+int devfreq_monitor_suspend(struct devfreq *devfreq) +{
int ret = -EPERM;if (delayed_work_pending(&devfreq->work)) {cancel_delayed_work_sync(&devfreq->work);ret = 0;}return ret;+}
+/**
- devfreq_monitor_resume() - Resume load monitoring of a devfreq instance
- @devfreq: the devfreq instance.
- Returns 0 if monitoring re-started, non-zero otherwise.
- Note: This function is exported for governors.
It would be good to explain the idea here too.
Ok. Done.
- */
+int devfreq_monitor_resume(struct devfreq *devfreq) +{
int ret = -EPERM;if (!delayed_work_pending(&devfreq->work)) {ret = !queue_delayed_work(devfreq_wq, &devfreq->work,msecs_to_jiffies(devfreq->profile->polling_ms));}return ret;+}
Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html