On Fri, Aug 19, 2011 at 12:12 PM, Turquette, Mike <mturquette@xxxxxx> 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@xxxxxxxxxxx> >> + * >> + * 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@xxxxxxxxxxx> >> + * >> + * 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@xxxxxxxxxxx> >> + * >> + * 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@xxxxxxxxxxx> >> + * >> + * 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@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
