On Thu, Aug 18, 2011 at 10:58 PM, MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx> wrote: > 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. 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@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