On Thu, Aug 11, 2011 at 10:35 AM, Turquette, Mike <mturquette@xxxxxx> wrote: [] > > Polling is the only practical use for devfreq, assuming a QoS API > exists for DVFS. As such powersave and performance governors should > be removed. Although powersave/performance governors may seem useless, they are used as basis on measuring the usefulness of DVFS mechanism of specific devices. If a device is going to use DVFS, we can test the device with them to find out the potential power save (compare powersave to performance) and the performance deterioration (compared to performance). Often, in testing phase, QA teams use performance to find out any issues with DVFS features (in CPUFREQ). Users may simply want to use performance governor in some cases (power is not an issue sometimes). Using QoS APIs simply to set "minimum" or "maximum" is possible. However, they are not that straightforward; e.g., how should we set "DMA latency" to be fixed at the minimum frequency regardless of others, or how should we set "Network latency" to be fixed at the maximum frequency? especially without knowing the specifications of each DVFS-capable device (such as available frequencies, valid latency values, ...). > >> 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. > > I won't repeate everything from patch 1 of this series, but the > governors should implement the queue/loop logic in the same way that > CPUfreq does, and the individual devices should have their own > delayed_work. First, in case where we want to let each DVFS-capable device have exact polling frequency (up to jiffy resolution), we only need to set polling_interval = jiffies_to_msecs(1);. In case of CPUFREQ, there would be only one polling loop at most for each core. However, in case of DEVFREQ, there could be multiple polling loops at a core if CPUFREQ-like looping logic is introduced. Why don't we reduce that overhead while their function is same, it is easily doable, and it reduces redundancy? > >> 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 > > It would be nice if OPP library "notified" devfreq but it does not > today. OPP library needs notifiers and devfreq can provide handlers > for them. That's why devfreq_update() is added in the patch. While DEVFREQ is the only one requiring notifications from OPP, do you think we may incur the overhead of notifier at OPP by replacing devfreq_update with notifier? If we somehow add another module that requires notifications from OPP for frequency availability changes, we will need to implement notifier at OPP side, but not just yet, I guess: (discussed before at https://lists.linux-foundation.org/pipermail/linux-pm/2011-July/032053.html ) > [] >> --- >> drivers/base/power/devfreq.c | 100 ++++++++++++++++++++++++++++++++++++++++++ >> include/linux/devfreq.h | 8 +++ >> 2 files changed, 108 insertions(+), 0 deletions(-) > > Governors should be split out into their own file, especially since > they need to grow to include polling/queueing logic. We will need to decide where to settle devfreq core, drivers, and governors first. Would /drivers/devfreq/ be appropriate? [] >> + >> + /* Set MAX if it's busy enough */ >> + if (stat.busy_time * 100 > >> + stat.total_time * DFSO_UPTHRESHOLD) { > > Thresholds should not be constants, but should be tuneable parameters, > per-device. This is yet another reason for revising the existing > relationship between devfreq core code, governors and devices. > I agree. I think I should add governor specific "setup" value at devfreq_add_device(); modifying the interface from extern int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile, struct devfreq_governor *governor); ==> extern int devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile, struct devfreq_governor *governor, void *gov_data); where gov_data is fed to struct devfreq's data field. >> + *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_DOWNDIFFERENCTIAL)) { > > Same as above. Yes. > >> + *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_DOWNDIFFERENCTIAL / 2)); > > Same as above. Yes. > > Regards, > Mike > 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@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
