[PATCH 1/2] PM / devfreq: add min/max_freq limit requested by users.

[PATCH 1/2] PM / devfreq: add min/max_freq limit requested by users.

[MyungJoo Ham]

The frequency requested to devfreq device driver from devfreq governors
is restricted by min_freq and max_freq input.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/devfreq/devfreq.c                 |   70 +++++++++++++++++++++++++++++
 drivers/devfreq/governor_performance.c    |    5 ++-
 drivers/devfreq/governor_powersave.c      |    2 +-
 drivers/devfreq/governor_simpleondemand.c |   12 ++++-
 drivers/devfreq/governor_userspace.c      |   15 +++++-
 include/linux/devfreq.h                   |    5 ++
 6 files changed, 101 insertions(+), 8 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 59d24e9..358d129 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -502,12 +502,82 @@ static ssize_t show_central_polling(struct device *dev,
 		       !to_devfreq(dev)->governor->no_central_polling);
 }
 
+static ssize_t store_min_freq(struct device *dev, struct device_attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct devfreq *df = to_devfreq(dev);
+	unsigned long value;
+	int ret;
+	unsigned long max;
+
+	ret = sscanf(buf, "%lu", &value);
+	if (ret != 1)
+		goto out;
+
+	mutex_lock(&df->lock);
+	max = df->max_freq;
+	if (value && max && value > max) {
+		ret = -EINVAL;
+		goto unlock;
+	}
+
+	df->min_freq = value;
+	update_devfreq(df);
+	ret = count;
+unlock:
+	mutex_unlock(&df->lock);
+out:
+	return ret;
+}
+
+static ssize_t show_min_freq(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	return sprintf(buf, "%lu\n", to_devfreq(dev)->min_freq);
+}
+
+static ssize_t store_max_freq(struct device *dev, struct device_attribute *attr,
+			      const char *buf, size_t count)
+{
+	struct devfreq *df = to_devfreq(dev);
+	unsigned long value;
+	int ret;
+	unsigned long min;
+
+	ret = sscanf(buf, "%lu", &value);
+	if (ret != 1)
+		goto out;
+
+	mutex_lock(&df->lock);
+	min = df->min_freq;
+	if (value && min && value < min) {
+		ret = -EINVAL;
+		goto unlock;
+	}
+
+	df->max_freq = value;
+	update_devfreq(df);
+	ret = count;
+unlock:
+	mutex_unlock(&df->lock);
+out:
+	return ret;
+}
+
+static ssize_t show_max_freq(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	return sprintf(buf, "%lu\n", to_devfreq(dev)->max_freq);
+}
+
 static struct device_attribute devfreq_attrs[] = {
 	__ATTR(governor, S_IRUGO, show_governor, NULL),
 	__ATTR(cur_freq, S_IRUGO, show_freq, NULL),
 	__ATTR(central_polling, S_IRUGO, show_central_polling, NULL),
 	__ATTR(polling_interval, S_IRUGO | S_IWUSR, show_polling_interval,
 	       store_polling_interval),
+	__ATTR(min_freq, S_IRUGO | S_IWUSR, show_min_freq, store_min_freq),
+	__ATTR(max_freq, S_IRUGO | S_IWUSR, show_max_freq, store_max_freq),
 	{ },
 };
 
diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
index c0596b2..574a06b 100644
--- a/drivers/devfreq/governor_performance.c
+++ b/drivers/devfreq/governor_performance.c
@@ -18,7 +18,10 @@ static int devfreq_performance_func(struct devfreq *df,
 	 * target callback should be able to get floor value as
 	 * said in devfreq.h
 	 */
-	*freq = UINT_MAX;
+	if (!df->max_freq)
+		*freq = UINT_MAX;
+	else
+		*freq = df->max_freq;
 	return 0;
 }
 
diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
index 2483a85..d742d4a 100644
--- a/drivers/devfreq/governor_powersave.c
+++ b/drivers/devfreq/governor_powersave.c
@@ -18,7 +18,7 @@ static int devfreq_powersave_func(struct devfreq *df,
 	 * target callback should be able to get ceiling value as
 	 * said in devfreq.h
 	 */
-	*freq = 0;
+	*freq = df->min_freq;
 	return 0;
 }
 
diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
index efad8dc..a2e3eae 100644
--- a/drivers/devfreq/governor_simpleondemand.c
+++ b/drivers/devfreq/governor_simpleondemand.c
@@ -25,6 +25,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
 	unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
 	unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
 	struct devfreq_simple_ondemand_data *data = df->data;
+	unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
 
 	if (err)
 		return err;
@@ -41,7 +42,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
 
 	/* Assume MAX if it is going to be divided by zero */
 	if (stat.total_time == 0) {
-		*freq = UINT_MAX;
+		*freq = max;
 		return 0;
 	}
 
@@ -54,13 +55,13 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
 	/* Set MAX if it's busy enough */
 	if (stat.busy_time * 100 >
 	    stat.total_time * dfso_upthreshold) {
-		*freq = UINT_MAX;
+		*freq = max;
 		return 0;
 	}
 
 	/* Set MAX if we do not know the initial frequency */
 	if (stat.current_frequency == 0) {
-		*freq = UINT_MAX;
+		*freq = max;
 		return 0;
 	}
 
@@ -79,6 +80,11 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
 	b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
 	*freq = (unsigned long) b;
 
+	if (df->min_freq && *freq < df->min_freq)
+		*freq = df->min_freq;
+	if (df->max_freq && *freq > df->max_freq)
+		*freq = df->max_freq;
+
 	return 0;
 }
 
diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
index 4f8b563..0681246 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -25,10 +25,19 @@ static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
 {
 	struct userspace_data *data = df->data;
 
-	if (!data->valid)
+	if (data->valid) {
+		unsigned long adjusted_freq = data->user_frequency;
+
+		if (df->max_freq && adjusted_freq > df->max_freq)
+			adjusted_freq = df->max_freq;
+
+		if (df->min_freq && adjusted_freq < df->min_freq)
+			adjusted_freq = df->min_freq;
+
+		*freq = adjusted_freq;
+	} else {
 		*freq = df->previous_freq; /* No user freq specified yet */
-	else
-		*freq = data->user_frequency;
+	}
 	return 0;
 }
 
diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 98ce812..e9385bf 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -124,6 +124,8 @@ struct devfreq_governor {
  *		touch this.
  * @being_removed	a flag to mark that this object is being removed in
  *			order to prevent trying to remove the object multiple times.
+ * @min_freq	Limit minimum frequency requested by user (0: none)
+ * @max_freq	Limit maximum frequency requested by user (0: none)
  *
  * This structure stores the devfreq information for a give device.
  *
@@ -149,6 +151,9 @@ struct devfreq {
 	void *data; /* private data for governors */
 
 	bool being_removed;
+
+	unsigned long min_freq;
+	unsigned long max_freq;
 };
 
 #if defined(CONFIG_PM_DEVFREQ)
-- 
1.7.4.1

[PATCH 2/2] PM / devfreq: fixed syntax errors.

[MyungJoo Ham]

If devfreq.h was included without CONFIG_PM_DEVFREQ, there has been a
compiler error with an additional semicolon added. This patch removes
that errorneous semicolon.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 include/linux/devfreq.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index e9385bf..5862475 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -205,12 +205,12 @@ struct devfreq_simple_ondemand_data {
 static struct devfreq *devfreq_add_device(struct device *dev,
 					  struct devfreq_dev_profile *profile,
 					  struct devfreq_governor *governor,
-					  void *data);
+					  void *data)
 {
 	return NULL;
 }
 
-static int devfreq_remove_device(struct devfreq *devfreq);
+static int devfreq_remove_device(struct devfreq *devfreq)
 {
 	return 0;
 }
-- 
1.7.4.1

Re: [PATCH 1/2] PM / devfreq: add min/max_freq limit requested by users.

[Rafael J. Wysocki]

On Wednesday, January 11, 2012, MyungJoo Ham wrote:
> The frequency requested to devfreq device driver from devfreq governors
> is restricted by min_freq and max_freq input.

Can you please give us a bit more information about why this change is
necessary or desirable?

Rafael


> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/devfreq/devfreq.c                 |   70 +++++++++++++++++++++++++++++
>  drivers/devfreq/governor_performance.c    |    5 ++-
>  drivers/devfreq/governor_powersave.c      |    2 +-
>  drivers/devfreq/governor_simpleondemand.c |   12 ++++-
>  drivers/devfreq/governor_userspace.c      |   15 +++++-
>  include/linux/devfreq.h                   |    5 ++
>  6 files changed, 101 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 59d24e9..358d129 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -502,12 +502,82 @@ static ssize_t show_central_polling(struct device *dev,
>  		       !to_devfreq(dev)->governor->no_central_polling);
>  }
>  
> +static ssize_t store_min_freq(struct device *dev, struct device_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	struct devfreq *df = to_devfreq(dev);
> +	unsigned long value;
> +	int ret;
> +	unsigned long max;
> +
> +	ret = sscanf(buf, "%lu", &value);
> +	if (ret != 1)
> +		goto out;
> +
> +	mutex_lock(&df->lock);
> +	max = df->max_freq;
> +	if (value && max && value > max) {
> +		ret = -EINVAL;
> +		goto unlock;
> +	}
> +
> +	df->min_freq = value;
> +	update_devfreq(df);
> +	ret = count;
> +unlock:
> +	mutex_unlock(&df->lock);
> +out:
> +	return ret;
> +}
> +
> +static ssize_t show_min_freq(struct device *dev, struct device_attribute *attr,
> +			     char *buf)
> +{
> +	return sprintf(buf, "%lu\n", to_devfreq(dev)->min_freq);
> +}
> +
> +static ssize_t store_max_freq(struct device *dev, struct device_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	struct devfreq *df = to_devfreq(dev);
> +	unsigned long value;
> +	int ret;
> +	unsigned long min;
> +
> +	ret = sscanf(buf, "%lu", &value);
> +	if (ret != 1)
> +		goto out;
> +
> +	mutex_lock(&df->lock);
> +	min = df->min_freq;
> +	if (value && min && value < min) {
> +		ret = -EINVAL;
> +		goto unlock;
> +	}
> +
> +	df->max_freq = value;
> +	update_devfreq(df);
> +	ret = count;
> +unlock:
> +	mutex_unlock(&df->lock);
> +out:
> +	return ret;
> +}
> +
> +static ssize_t show_max_freq(struct device *dev, struct device_attribute *attr,
> +			     char *buf)
> +{
> +	return sprintf(buf, "%lu\n", to_devfreq(dev)->max_freq);
> +}
> +
>  static struct device_attribute devfreq_attrs[] = {
>  	__ATTR(governor, S_IRUGO, show_governor, NULL),
>  	__ATTR(cur_freq, S_IRUGO, show_freq, NULL),
>  	__ATTR(central_polling, S_IRUGO, show_central_polling, NULL),
>  	__ATTR(polling_interval, S_IRUGO | S_IWUSR, show_polling_interval,
>  	       store_polling_interval),
> +	__ATTR(min_freq, S_IRUGO | S_IWUSR, show_min_freq, store_min_freq),
> +	__ATTR(max_freq, S_IRUGO | S_IWUSR, show_max_freq, store_max_freq),
>  	{ },
>  };
>  
> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
> index c0596b2..574a06b 100644
> --- a/drivers/devfreq/governor_performance.c
> +++ b/drivers/devfreq/governor_performance.c
> @@ -18,7 +18,10 @@ static int devfreq_performance_func(struct devfreq *df,
>  	 * target callback should be able to get floor value as
>  	 * said in devfreq.h
>  	 */
> -	*freq = UINT_MAX;
> +	if (!df->max_freq)
> +		*freq = UINT_MAX;
> +	else
> +		*freq = df->max_freq;
>  	return 0;
>  }
>  
> diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
> index 2483a85..d742d4a 100644
> --- a/drivers/devfreq/governor_powersave.c
> +++ b/drivers/devfreq/governor_powersave.c
> @@ -18,7 +18,7 @@ static int devfreq_powersave_func(struct devfreq *df,
>  	 * target callback should be able to get ceiling value as
>  	 * said in devfreq.h
>  	 */
> -	*freq = 0;
> +	*freq = df->min_freq;
>  	return 0;
>  }
>  
> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> index efad8dc..a2e3eae 100644
> --- a/drivers/devfreq/governor_simpleondemand.c
> +++ b/drivers/devfreq/governor_simpleondemand.c
> @@ -25,6 +25,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>  	unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
>  	unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
>  	struct devfreq_simple_ondemand_data *data = df->data;
> +	unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
>  
>  	if (err)
>  		return err;
> @@ -41,7 +42,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>  
>  	/* Assume MAX if it is going to be divided by zero */
>  	if (stat.total_time == 0) {
> -		*freq = UINT_MAX;
> +		*freq = max;
>  		return 0;
>  	}
>  
> @@ -54,13 +55,13 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>  	/* Set MAX if it's busy enough */
>  	if (stat.busy_time * 100 >
>  	    stat.total_time * dfso_upthreshold) {
> -		*freq = UINT_MAX;
> +		*freq = max;
>  		return 0;
>  	}
>  
>  	/* Set MAX if we do not know the initial frequency */
>  	if (stat.current_frequency == 0) {
> -		*freq = UINT_MAX;
> +		*freq = max;
>  		return 0;
>  	}
>  
> @@ -79,6 +80,11 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>  	b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
>  	*freq = (unsigned long) b;
>  
> +	if (df->min_freq && *freq < df->min_freq)
> +		*freq = df->min_freq;
> +	if (df->max_freq && *freq > df->max_freq)
> +		*freq = df->max_freq;
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
> index 4f8b563..0681246 100644
> --- a/drivers/devfreq/governor_userspace.c
> +++ b/drivers/devfreq/governor_userspace.c
> @@ -25,10 +25,19 @@ static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
>  {
>  	struct userspace_data *data = df->data;
>  
> -	if (!data->valid)
> +	if (data->valid) {
> +		unsigned long adjusted_freq = data->user_frequency;
> +
> +		if (df->max_freq && adjusted_freq > df->max_freq)
> +			adjusted_freq = df->max_freq;
> +
> +		if (df->min_freq && adjusted_freq < df->min_freq)
> +			adjusted_freq = df->min_freq;
> +
> +		*freq = adjusted_freq;
> +	} else {
>  		*freq = df->previous_freq; /* No user freq specified yet */
> -	else
> -		*freq = data->user_frequency;
> +	}
>  	return 0;
>  }
>  
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 98ce812..e9385bf 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -124,6 +124,8 @@ struct devfreq_governor {
>   *		touch this.
>   * @being_removed	a flag to mark that this object is being removed in
>   *			order to prevent trying to remove the object multiple times.
> + * @min_freq	Limit minimum frequency requested by user (0: none)
> + * @max_freq	Limit maximum frequency requested by user (0: none)
>   *
>   * This structure stores the devfreq information for a give device.
>   *
> @@ -149,6 +151,9 @@ struct devfreq {
>  	void *data; /* private data for governors */
>  
>  	bool being_removed;
> +
> +	unsigned long min_freq;
> +	unsigned long max_freq;
>  };
>  
>  #if defined(CONFIG_PM_DEVFREQ)
> 

Re: [PATCH 1/2] PM / devfreq: add min/max_freq limit requested by users.

[Turquette, Mike]

On Wed, Jan 11, 2012 at 2:02 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> The frequency requested to devfreq device driver from devfreq governors
> is restricted by min_freq and max_freq input.

Hello MyungJoo,

This change appears to allow userspace to set min/max limits on
devfreq devices via sysfs.  Not everyone likes to expose this stuff
(similar to the discussions around controlling clocks from debugfs).

Should it be wrapped in some new config option?  I think a sane
default is that if the sysfs config option for devfreq is selected
then it should include all of the read-only stuff.  A second config
option (which depends on the option in my previous sentence) should
allow the read-write stuff to be enabled separately.  Thoughts?

Also, how are you using this feature in practice?  Is this just for
test or are you planning on more fine-grained control of device
frequencies from userspace?

Mike

>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/devfreq/devfreq.c                 |   70 +++++++++++++++++++++++++++++
>  drivers/devfreq/governor_performance.c    |    5 ++-
>  drivers/devfreq/governor_powersave.c      |    2 +-
>  drivers/devfreq/governor_simpleondemand.c |   12 ++++-
>  drivers/devfreq/governor_userspace.c      |   15 +++++-
>  include/linux/devfreq.h                   |    5 ++
>  6 files changed, 101 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 59d24e9..358d129 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -502,12 +502,82 @@ static ssize_t show_central_polling(struct device *dev,
>                       !to_devfreq(dev)->governor->no_central_polling);
>  }
>
> +static ssize_t store_min_freq(struct device *dev, struct device_attribute *attr,
> +                             const char *buf, size_t count)
> +{
> +       struct devfreq *df = to_devfreq(dev);
> +       unsigned long value;
> +       int ret;
> +       unsigned long max;
> +
> +       ret = sscanf(buf, "%lu", &value);
> +       if (ret != 1)
> +               goto out;
> +
> +       mutex_lock(&df->lock);
> +       max = df->max_freq;
> +       if (value && max && value > max) {
> +               ret = -EINVAL;
> +               goto unlock;
> +       }
> +
> +       df->min_freq = value;
> +       update_devfreq(df);
> +       ret = count;
> +unlock:
> +       mutex_unlock(&df->lock);
> +out:
> +       return ret;
> +}
> +
> +static ssize_t show_min_freq(struct device *dev, struct device_attribute *attr,
> +                            char *buf)
> +{
> +       return sprintf(buf, "%lu\n", to_devfreq(dev)->min_freq);
> +}
> +
> +static ssize_t store_max_freq(struct device *dev, struct device_attribute *attr,
> +                             const char *buf, size_t count)
> +{
> +       struct devfreq *df = to_devfreq(dev);
> +       unsigned long value;
> +       int ret;
> +       unsigned long min;
> +
> +       ret = sscanf(buf, "%lu", &value);
> +       if (ret != 1)
> +               goto out;
> +
> +       mutex_lock(&df->lock);
> +       min = df->min_freq;
> +       if (value && min && value < min) {
> +               ret = -EINVAL;
> +               goto unlock;
> +       }
> +
> +       df->max_freq = value;
> +       update_devfreq(df);
> +       ret = count;
> +unlock:
> +       mutex_unlock(&df->lock);
> +out:
> +       return ret;
> +}
> +
> +static ssize_t show_max_freq(struct device *dev, struct device_attribute *attr,
> +                            char *buf)
> +{
> +       return sprintf(buf, "%lu\n", to_devfreq(dev)->max_freq);
> +}
> +
>  static struct device_attribute devfreq_attrs[] = {
>        __ATTR(governor, S_IRUGO, show_governor, NULL),
>        __ATTR(cur_freq, S_IRUGO, show_freq, NULL),
>        __ATTR(central_polling, S_IRUGO, show_central_polling, NULL),
>        __ATTR(polling_interval, S_IRUGO | S_IWUSR, show_polling_interval,
>               store_polling_interval),
> +       __ATTR(min_freq, S_IRUGO | S_IWUSR, show_min_freq, store_min_freq),
> +       __ATTR(max_freq, S_IRUGO | S_IWUSR, show_max_freq, store_max_freq),
>        { },
>  };
>
> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
> index c0596b2..574a06b 100644
> --- a/drivers/devfreq/governor_performance.c
> +++ b/drivers/devfreq/governor_performance.c
> @@ -18,7 +18,10 @@ static int devfreq_performance_func(struct devfreq *df,
>         * target callback should be able to get floor value as
>         * said in devfreq.h
>         */
> -       *freq = UINT_MAX;
> +       if (!df->max_freq)
> +               *freq = UINT_MAX;
> +       else
> +               *freq = df->max_freq;
>        return 0;
>  }
>
> diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
> index 2483a85..d742d4a 100644
> --- a/drivers/devfreq/governor_powersave.c
> +++ b/drivers/devfreq/governor_powersave.c
> @@ -18,7 +18,7 @@ static int devfreq_powersave_func(struct devfreq *df,
>         * target callback should be able to get ceiling value as
>         * said in devfreq.h
>         */
> -       *freq = 0;
> +       *freq = df->min_freq;
>        return 0;
>  }
>
> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> index efad8dc..a2e3eae 100644
> --- a/drivers/devfreq/governor_simpleondemand.c
> +++ b/drivers/devfreq/governor_simpleondemand.c
> @@ -25,6 +25,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>        unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
>        unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
>        struct devfreq_simple_ondemand_data *data = df->data;
> +       unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
>
>        if (err)
>                return err;
> @@ -41,7 +42,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>
>        /* Assume MAX if it is going to be divided by zero */
>        if (stat.total_time == 0) {
> -               *freq = UINT_MAX;
> +               *freq = max;
>                return 0;
>        }
>
> @@ -54,13 +55,13 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>        /* Set MAX if it's busy enough */
>        if (stat.busy_time * 100 >
>            stat.total_time * dfso_upthreshold) {
> -               *freq = UINT_MAX;
> +               *freq = max;
>                return 0;
>        }
>
>        /* Set MAX if we do not know the initial frequency */
>        if (stat.current_frequency == 0) {
> -               *freq = UINT_MAX;
> +               *freq = max;
>                return 0;
>        }
>
> @@ -79,6 +80,11 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>        b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
>        *freq = (unsigned long) b;
>
> +       if (df->min_freq && *freq < df->min_freq)
> +               *freq = df->min_freq;
> +       if (df->max_freq && *freq > df->max_freq)
> +               *freq = df->max_freq;
> +
>        return 0;
>  }
>
> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
> index 4f8b563..0681246 100644
> --- a/drivers/devfreq/governor_userspace.c
> +++ b/drivers/devfreq/governor_userspace.c
> @@ -25,10 +25,19 @@ static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
>  {
>        struct userspace_data *data = df->data;
>
> -       if (!data->valid)
> +       if (data->valid) {
> +               unsigned long adjusted_freq = data->user_frequency;
> +
> +               if (df->max_freq && adjusted_freq > df->max_freq)
> +                       adjusted_freq = df->max_freq;
> +
> +               if (df->min_freq && adjusted_freq < df->min_freq)
> +                       adjusted_freq = df->min_freq;
> +
> +               *freq = adjusted_freq;
> +       } else {
>                *freq = df->previous_freq; /* No user freq specified yet */
> -       else
> -               *freq = data->user_frequency;
> +       }
>        return 0;
>  }
>
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 98ce812..e9385bf 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -124,6 +124,8 @@ struct devfreq_governor {
>  *             touch this.
>  * @being_removed      a flag to mark that this object is being removed in
>  *                     order to prevent trying to remove the object multiple times.
> + * @min_freq   Limit minimum frequency requested by user (0: none)
> + * @max_freq   Limit maximum frequency requested by user (0: none)
>  *
>  * This structure stores the devfreq information for a give device.
>  *
> @@ -149,6 +151,9 @@ struct devfreq {
>        void *data; /* private data for governors */
>
>        bool being_removed;
> +
> +       unsigned long min_freq;
> +       unsigned long max_freq;
>  };
>
>  #if defined(CONFIG_PM_DEVFREQ)
> --
> 1.7.4.1
>

Re: [PATCH 2/2] PM / devfreq: fixed syntax errors.

[Turquette, Mike]

On Wed, Jan 11, 2012 at 2:02 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> If devfreq.h was included without CONFIG_PM_DEVFREQ, there has been a
> compiler error with an additional semicolon added. This patch removes
> that errorneous semicolon.

Looks good.

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

>
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  include/linux/devfreq.h |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index e9385bf..5862475 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -205,12 +205,12 @@ struct devfreq_simple_ondemand_data {
>  static struct devfreq *devfreq_add_device(struct device *dev,
>                                          struct devfreq_dev_profile *profile,
>                                          struct devfreq_governor *governor,
> -                                         void *data);
> +                                         void *data)
>  {
>        return NULL;
>  }
>
> -static int devfreq_remove_device(struct devfreq *devfreq);
> +static int devfreq_remove_device(struct devfreq *devfreq)
>  {
>        return 0;
>  }
> --
> 1.7.4.1
>

Re: [PATCH 1/2] PM / devfreq: add min/max_freq limit requested by users.

[MyungJoo Ham]

2012/1/12 Rafael J. Wysocki <rjw@sisk.pl>:
> On Wednesday, January 11, 2012, MyungJoo Ham wrote:
>> The frequency requested to devfreq device driver from devfreq governors
>> is restricted by min_freq and max_freq input.
>
> Can you please give us a bit more information about why this change is
> necessary or desirable?
>
> Rafael


During developing a device with DVFS capabilities on Memory-Interface,
Bus, and GPU, there have been needs for configuring min/max freuqency
at userspace (by either "platform software" or "command line inputs of
developers"):

1. (debugging/testing) Test whether a symptom is occurred by DVFS
mechanism setting the frequency too low.

2. Set minimum frequency of a specific device for a specific behavior
(e.g., set GPU >= 200MHz if it is playing 1080p video because if we
let DVFS handle without minimum, sometimes it goes lower than 200MHz
and up, which incurs flickering.). Because userspace often directly
access such devices (accessing device with memory allocated by
DRM+GEM), kernel device driver may not be able to get what's going on
easily. This will probably be handled by platform software.

3. Userspace (the platform such as Tizen, Android, or others) wants to
limit (max frequency) the usage when 1. it's too hot, or 2. the
battery level is low, especially for GPUs.

I guess these are what I've been experiencing with DVFS recently. For
kernel developers, 1. was the most significant anyway.


Cheers!
MyungJoo

>
>
>> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  drivers/devfreq/devfreq.c                 |   70 +++++++++++++++++++++++++++++
>>  drivers/devfreq/governor_performance.c    |    5 ++-
>>  drivers/devfreq/governor_powersave.c      |    2 +-
>>  drivers/devfreq/governor_simpleondemand.c |   12 ++++-
>>  drivers/devfreq/governor_userspace.c      |   15 +++++-
>>  include/linux/devfreq.h                   |    5 ++
>>  6 files changed, 101 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index 59d24e9..358d129 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -502,12 +502,82 @@ static ssize_t show_central_polling(struct device *dev,
>>                      !to_devfreq(dev)->governor->no_central_polling);
>>  }
>>
>> +static ssize_t store_min_freq(struct device *dev, struct device_attribute *attr,
>> +                           const char *buf, size_t count)
>> +{
>> +     struct devfreq *df = to_devfreq(dev);
>> +     unsigned long value;
>> +     int ret;
>> +     unsigned long max;
>> +
>> +     ret = sscanf(buf, "%lu", &value);
>> +     if (ret != 1)
>> +             goto out;
>> +
>> +     mutex_lock(&df->lock);
>> +     max = df->max_freq;
>> +     if (value && max && value > max) {
>> +             ret = -EINVAL;
>> +             goto unlock;
>> +     }
>> +
>> +     df->min_freq = value;
>> +     update_devfreq(df);
>> +     ret = count;
>> +unlock:
>> +     mutex_unlock(&df->lock);
>> +out:
>> +     return ret;
>> +}
>> +
>> +static ssize_t show_min_freq(struct device *dev, struct device_attribute *attr,
>> +                          char *buf)
>> +{
>> +     return sprintf(buf, "%lu\n", to_devfreq(dev)->min_freq);
>> +}
>> +
>> +static ssize_t store_max_freq(struct device *dev, struct device_attribute *attr,
>> +                           const char *buf, size_t count)
>> +{
>> +     struct devfreq *df = to_devfreq(dev);
>> +     unsigned long value;
>> +     int ret;
>> +     unsigned long min;
>> +
>> +     ret = sscanf(buf, "%lu", &value);
>> +     if (ret != 1)
>> +             goto out;
>> +
>> +     mutex_lock(&df->lock);
>> +     min = df->min_freq;
>> +     if (value && min && value < min) {
>> +             ret = -EINVAL;
>> +             goto unlock;
>> +     }
>> +
>> +     df->max_freq = value;
>> +     update_devfreq(df);
>> +     ret = count;
>> +unlock:
>> +     mutex_unlock(&df->lock);
>> +out:
>> +     return ret;
>> +}
>> +
>> +static ssize_t show_max_freq(struct device *dev, struct device_attribute *attr,
>> +                          char *buf)
>> +{
>> +     return sprintf(buf, "%lu\n", to_devfreq(dev)->max_freq);
>> +}
>> +
>>  static struct device_attribute devfreq_attrs[] = {
>>       __ATTR(governor, S_IRUGO, show_governor, NULL),
>>       __ATTR(cur_freq, S_IRUGO, show_freq, NULL),
>>       __ATTR(central_polling, S_IRUGO, show_central_polling, NULL),
>>       __ATTR(polling_interval, S_IRUGO | S_IWUSR, show_polling_interval,
>>              store_polling_interval),
>> +     __ATTR(min_freq, S_IRUGO | S_IWUSR, show_min_freq, store_min_freq),
>> +     __ATTR(max_freq, S_IRUGO | S_IWUSR, show_max_freq, store_max_freq),
>>       { },
>>  };
>>
>> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
>> index c0596b2..574a06b 100644
>> --- a/drivers/devfreq/governor_performance.c
>> +++ b/drivers/devfreq/governor_performance.c
>> @@ -18,7 +18,10 @@ static int devfreq_performance_func(struct devfreq *df,
>>        * target callback should be able to get floor value as
>>        * said in devfreq.h
>>        */
>> -     *freq = UINT_MAX;
>> +     if (!df->max_freq)
>> +             *freq = UINT_MAX;
>> +     else
>> +             *freq = df->max_freq;
>>       return 0;
>>  }
>>
>> diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
>> index 2483a85..d742d4a 100644
>> --- a/drivers/devfreq/governor_powersave.c
>> +++ b/drivers/devfreq/governor_powersave.c
>> @@ -18,7 +18,7 @@ static int devfreq_powersave_func(struct devfreq *df,
>>        * target callback should be able to get ceiling value as
>>        * said in devfreq.h
>>        */
>> -     *freq = 0;
>> +     *freq = df->min_freq;
>>       return 0;
>>  }
>>
>> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
>> index efad8dc..a2e3eae 100644
>> --- a/drivers/devfreq/governor_simpleondemand.c
>> +++ b/drivers/devfreq/governor_simpleondemand.c
>> @@ -25,6 +25,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>>       unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
>>       unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
>>       struct devfreq_simple_ondemand_data *data = df->data;
>> +     unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
>>
>>       if (err)
>>               return err;
>> @@ -41,7 +42,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>>
>>       /* Assume MAX if it is going to be divided by zero */
>>       if (stat.total_time == 0) {
>> -             *freq = UINT_MAX;
>> +             *freq = max;
>>               return 0;
>>       }
>>
>> @@ -54,13 +55,13 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>>       /* Set MAX if it's busy enough */
>>       if (stat.busy_time * 100 >
>>           stat.total_time * dfso_upthreshold) {
>> -             *freq = UINT_MAX;
>> +             *freq = max;
>>               return 0;
>>       }
>>
>>       /* Set MAX if we do not know the initial frequency */
>>       if (stat.current_frequency == 0) {
>> -             *freq = UINT_MAX;
>> +             *freq = max;
>>               return 0;
>>       }
>>
>> @@ -79,6 +80,11 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>>       b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
>>       *freq = (unsigned long) b;
>>
>> +     if (df->min_freq && *freq < df->min_freq)
>> +             *freq = df->min_freq;
>> +     if (df->max_freq && *freq > df->max_freq)
>> +             *freq = df->max_freq;
>> +
>>       return 0;
>>  }
>>
>> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
>> index 4f8b563..0681246 100644
>> --- a/drivers/devfreq/governor_userspace.c
>> +++ b/drivers/devfreq/governor_userspace.c
>> @@ -25,10 +25,19 @@ static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
>>  {
>>       struct userspace_data *data = df->data;
>>
>> -     if (!data->valid)
>> +     if (data->valid) {
>> +             unsigned long adjusted_freq = data->user_frequency;
>> +
>> +             if (df->max_freq && adjusted_freq > df->max_freq)
>> +                     adjusted_freq = df->max_freq;
>> +
>> +             if (df->min_freq && adjusted_freq < df->min_freq)
>> +                     adjusted_freq = df->min_freq;
>> +
>> +             *freq = adjusted_freq;
>> +     } else {
>>               *freq = df->previous_freq; /* No user freq specified yet */
>> -     else
>> -             *freq = data->user_frequency;
>> +     }
>>       return 0;
>>  }
>>
>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>> index 98ce812..e9385bf 100644
>> --- a/include/linux/devfreq.h
>> +++ b/include/linux/devfreq.h
>> @@ -124,6 +124,8 @@ struct devfreq_governor {
>>   *           touch this.
>>   * @being_removed    a flag to mark that this object is being removed in
>>   *                   order to prevent trying to remove the object multiple times.
>> + * @min_freq Limit minimum frequency requested by user (0: none)
>> + * @max_freq Limit maximum frequency requested by user (0: none)
>>   *
>>   * This structure stores the devfreq information for a give device.
>>   *
>> @@ -149,6 +151,9 @@ struct devfreq {
>>       void *data; /* private data for governors */
>>
>>       bool being_removed;
>> +
>> +     unsigned long min_freq;
>> +     unsigned long max_freq;
>>  };
>>
>>  #if defined(CONFIG_PM_DEVFREQ)
>>
>



-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics

Re: [PATCH 1/2] PM / devfreq: add min/max_freq limit requested by users.

[MyungJoo Ham]

On Thu, Jan 12, 2012 at 9:20 AM, Turquette, Mike <mturquette@ti.com> wrote:
> On Wed, Jan 11, 2012 at 2:02 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>> The frequency requested to devfreq device driver from devfreq governors
>> is restricted by min_freq and max_freq input.
>
> Hello MyungJoo,
>
> This change appears to allow userspace to set min/max limits on
> devfreq devices via sysfs.  Not everyone likes to expose this stuff
> (similar to the discussions around controlling clocks from debugfs).
>
> Should it be wrapped in some new config option?  I think a sane
> default is that if the sysfs config option for devfreq is selected
> then it should include all of the read-only stuff.  A second config
> option (which depends on the option in my previous sentence) should
> allow the read-write stuff to be enabled separately.  Thoughts?
>
> Also, how are you using this feature in practice?  Is this just for
> test or are you planning on more fine-grained control of device
> frequencies from userspace?
>
> Mike

Hello Mike,


Although turning off clocks inconsiderately usually crashes the
system, setting min/max frequencies generally affects (if not always)
only the performance and power consumption.

For the optional min/max freq, I think each device should be able to
choose to use it or not. Thus, rather than adding a Kconfig option,
I'll let "profile" (struct devfreq_dev_profile) include
"expose_user_min_max_freq" option.

In practice, we have been using min/max to test DVFS behaviors and its
side effects. And we are going to use them to 1. restrict power
consumption forcibly by the platform software if it is too hot or the
battery is low, and to 2. guarantee the minimum performance for
specific tasks controlled by the platform software.

Anyway, the reason 2 could be tackled by pm-qos if we allow more
options in pm-qos with 1. pm qos type to enforce DVFS response time.
2. pm qos type to enforce graphics performance. And adding a duration
option to pm-qos requests will be helpful (sort of a helper function):
i.e., pm_qos_timed_request(struct pm_qos_request *req, int
pm_qos_class, s32 value, unsigned long duration_ms);



Cheers!
MyungJoo.


>
>>
>> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>>  drivers/devfreq/devfreq.c                 |   70 +++++++++++++++++++++++++++++
>>  drivers/devfreq/governor_performance.c    |    5 ++-
>>  drivers/devfreq/governor_powersave.c      |    2 +-
>>  drivers/devfreq/governor_simpleondemand.c |   12 ++++-
>>  drivers/devfreq/governor_userspace.c      |   15 +++++-
>>  include/linux/devfreq.h                   |    5 ++
>>  6 files changed, 101 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> index 59d24e9..358d129 100644
>> --- a/drivers/devfreq/devfreq.c
>> +++ b/drivers/devfreq/devfreq.c
>> @@ -502,12 +502,82 @@ static ssize_t show_central_polling(struct device *dev,
>>                       !to_devfreq(dev)->governor->no_central_polling);
>>  }
>>
>> +static ssize_t store_min_freq(struct device *dev, struct device_attribute *attr,
>> +                             const char *buf, size_t count)
>> +{
>> +       struct devfreq *df = to_devfreq(dev);
>> +       unsigned long value;
>> +       int ret;
>> +       unsigned long max;
>> +
>> +       ret = sscanf(buf, "%lu", &value);
>> +       if (ret != 1)
>> +               goto out;
>> +
>> +       mutex_lock(&df->lock);
>> +       max = df->max_freq;
>> +       if (value && max && value > max) {
>> +               ret = -EINVAL;
>> +               goto unlock;
>> +       }
>> +
>> +       df->min_freq = value;
>> +       update_devfreq(df);
>> +       ret = count;
>> +unlock:
>> +       mutex_unlock(&df->lock);
>> +out:
>> +       return ret;
>> +}
>> +
>> +static ssize_t show_min_freq(struct device *dev, struct device_attribute *attr,
>> +                            char *buf)
>> +{
>> +       return sprintf(buf, "%lu\n", to_devfreq(dev)->min_freq);
>> +}
>> +
>> +static ssize_t store_max_freq(struct device *dev, struct device_attribute *attr,
>> +                             const char *buf, size_t count)
>> +{
>> +       struct devfreq *df = to_devfreq(dev);
>> +       unsigned long value;
>> +       int ret;
>> +       unsigned long min;
>> +
>> +       ret = sscanf(buf, "%lu", &value);
>> +       if (ret != 1)
>> +               goto out;
>> +
>> +       mutex_lock(&df->lock);
>> +       min = df->min_freq;
>> +       if (value && min && value < min) {
>> +               ret = -EINVAL;
>> +               goto unlock;
>> +       }
>> +
>> +       df->max_freq = value;
>> +       update_devfreq(df);
>> +       ret = count;
>> +unlock:
>> +       mutex_unlock(&df->lock);
>> +out:
>> +       return ret;
>> +}
>> +
>> +static ssize_t show_max_freq(struct device *dev, struct device_attribute *attr,
>> +                            char *buf)
>> +{
>> +       return sprintf(buf, "%lu\n", to_devfreq(dev)->max_freq);
>> +}
>> +
>>  static struct device_attribute devfreq_attrs[] = {
>>        __ATTR(governor, S_IRUGO, show_governor, NULL),
>>        __ATTR(cur_freq, S_IRUGO, show_freq, NULL),
>>        __ATTR(central_polling, S_IRUGO, show_central_polling, NULL),
>>        __ATTR(polling_interval, S_IRUGO | S_IWUSR, show_polling_interval,
>>               store_polling_interval),
>> +       __ATTR(min_freq, S_IRUGO | S_IWUSR, show_min_freq, store_min_freq),
>> +       __ATTR(max_freq, S_IRUGO | S_IWUSR, show_max_freq, store_max_freq),
>>        { },
>>  };
>>
>> diff --git a/drivers/devfreq/governor_performance.c b/drivers/devfreq/governor_performance.c
>> index c0596b2..574a06b 100644
>> --- a/drivers/devfreq/governor_performance.c
>> +++ b/drivers/devfreq/governor_performance.c
>> @@ -18,7 +18,10 @@ static int devfreq_performance_func(struct devfreq *df,
>>         * target callback should be able to get floor value as
>>         * said in devfreq.h
>>         */
>> -       *freq = UINT_MAX;
>> +       if (!df->max_freq)
>> +               *freq = UINT_MAX;
>> +       else
>> +               *freq = df->max_freq;
>>        return 0;
>>  }
>>
>> diff --git a/drivers/devfreq/governor_powersave.c b/drivers/devfreq/governor_powersave.c
>> index 2483a85..d742d4a 100644
>> --- a/drivers/devfreq/governor_powersave.c
>> +++ b/drivers/devfreq/governor_powersave.c
>> @@ -18,7 +18,7 @@ static int devfreq_powersave_func(struct devfreq *df,
>>         * target callback should be able to get ceiling value as
>>         * said in devfreq.h
>>         */
>> -       *freq = 0;
>> +       *freq = df->min_freq;
>>        return 0;
>>  }
>>
>> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
>> index efad8dc..a2e3eae 100644
>> --- a/drivers/devfreq/governor_simpleondemand.c
>> +++ b/drivers/devfreq/governor_simpleondemand.c
>> @@ -25,6 +25,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>>        unsigned int dfso_upthreshold = DFSO_UPTHRESHOLD;
>>        unsigned int dfso_downdifferential = DFSO_DOWNDIFFERENCTIAL;
>>        struct devfreq_simple_ondemand_data *data = df->data;
>> +       unsigned long max = (df->max_freq) ? df->max_freq : UINT_MAX;
>>
>>        if (err)
>>                return err;
>> @@ -41,7 +42,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>>
>>        /* Assume MAX if it is going to be divided by zero */
>>        if (stat.total_time == 0) {
>> -               *freq = UINT_MAX;
>> +               *freq = max;
>>                return 0;
>>        }
>>
>> @@ -54,13 +55,13 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>>        /* Set MAX if it's busy enough */
>>        if (stat.busy_time * 100 >
>>            stat.total_time * dfso_upthreshold) {
>> -               *freq = UINT_MAX;
>> +               *freq = max;
>>                return 0;
>>        }
>>
>>        /* Set MAX if we do not know the initial frequency */
>>        if (stat.current_frequency == 0) {
>> -               *freq = UINT_MAX;
>> +               *freq = max;
>>                return 0;
>>        }
>>
>> @@ -79,6 +80,11 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>>        b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
>>        *freq = (unsigned long) b;
>>
>> +       if (df->min_freq && *freq < df->min_freq)
>> +               *freq = df->min_freq;
>> +       if (df->max_freq && *freq > df->max_freq)
>> +               *freq = df->max_freq;
>> +
>>        return 0;
>>  }
>>
>> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
>> index 4f8b563..0681246 100644
>> --- a/drivers/devfreq/governor_userspace.c
>> +++ b/drivers/devfreq/governor_userspace.c
>> @@ -25,10 +25,19 @@ static int devfreq_userspace_func(struct devfreq *df, unsigned long *freq)
>>  {
>>        struct userspace_data *data = df->data;
>>
>> -       if (!data->valid)
>> +       if (data->valid) {
>> +               unsigned long adjusted_freq = data->user_frequency;
>> +
>> +               if (df->max_freq && adjusted_freq > df->max_freq)
>> +                       adjusted_freq = df->max_freq;
>> +
>> +               if (df->min_freq && adjusted_freq < df->min_freq)
>> +                       adjusted_freq = df->min_freq;
>> +
>> +               *freq = adjusted_freq;
>> +       } else {
>>                *freq = df->previous_freq; /* No user freq specified yet */
>> -       else
>> -               *freq = data->user_frequency;
>> +       }
>>        return 0;
>>  }
>>
>> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
>> index 98ce812..e9385bf 100644
>> --- a/include/linux/devfreq.h
>> +++ b/include/linux/devfreq.h
>> @@ -124,6 +124,8 @@ struct devfreq_governor {
>>  *             touch this.
>>  * @being_removed      a flag to mark that this object is being removed in
>>  *                     order to prevent trying to remove the object multiple times.
>> + * @min_freq   Limit minimum frequency requested by user (0: none)
>> + * @max_freq   Limit maximum frequency requested by user (0: none)
>>  *
>>  * This structure stores the devfreq information for a give device.
>>  *
>> @@ -149,6 +151,9 @@ struct devfreq {
>>        void *data; /* private data for governors */
>>
>>        bool being_removed;
>> +
>> +       unsigned long min_freq;
>> +       unsigned long max_freq;
>>  };
>>
>>  #if defined(CONFIG_PM_DEVFREQ)
>> --
>> 1.7.4.1
>>



-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics

Re: [PATCH 1/2] PM / devfreq: add min/max_freq limit requested by users.

[mark gross]

On Thu, Jan 12, 2012 at 11:08:44AM +0900, MyungJoo Ham wrote:
> On Thu, Jan 12, 2012 at 9:20 AM, Turquette, Mike <mturquette@ti.com> wrote:
> > On Wed, Jan 11, 2012 at 2:02 AM, MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
> >> The frequency requested to devfreq device driver from devfreq governors
> >> is restricted by min_freq and max_freq input.
> >
> > Hello MyungJoo,
> >
> > This change appears to allow userspace to set min/max limits on
> > devfreq devices via sysfs.  Not everyone likes to expose this stuff
> > (similar to the discussions around controlling clocks from debugfs).
> >
> > Should it be wrapped in some new config option?  I think a sane
> > default is that if the sysfs config option for devfreq is selected
> > then it should include all of the read-only stuff.  A second config
> > option (which depends on the option in my previous sentence) should
> > allow the read-write stuff to be enabled separately.  Thoughts?
> >
> > Also, how are you using this feature in practice?  Is this just for
> > test or are you planning on more fine-grained control of device
> > frequencies from userspace?
> >
> > Mike
> 
> Hello Mike,
> 
> 
> Although turning off clocks inconsiderately usually crashes the
> system, setting min/max frequencies generally affects (if not always)
> only the performance and power consumption.
> 
> For the optional min/max freq, I think each device should be able to
> choose to use it or not. Thus, rather than adding a Kconfig option,
> I'll let "profile" (struct devfreq_dev_profile) include
> "expose_user_min_max_freq" option.
> 
> In practice, we have been using min/max to test DVFS behaviors and its
> side effects. And we are going to use them to 1. restrict power
> consumption forcibly by the platform software if it is too hot or the
> battery is low, and to 2. guarantee the minimum performance for
> specific tasks controlled by the platform software.
> 
> Anyway, the reason 2 could be tackled by pm-qos if we allow more
> options in pm-qos with 1. pm qos type to enforce DVFS response time.
what would pm_qos do with DVFS response time?  What power management
knob would it enable a constraint for?

pm_qos doesn't do anything but enable power throttling code to consider
a constraint on how far to throttle "something".  pm_qos has no
enforcement power.

> 2. pm qos type to enforce graphics performance. And adding a duration
> option to pm-qos requests will be helpful (sort of a helper function):
> i.e., pm_qos_timed_request(struct pm_qos_request *req, int
> pm_qos_class, s32 value, unsigned long duration_ms);

What would be good units for graphics throughput?
Where in the graphics driver would you insert the equivalent of cpufreq?
to control the GPU core frequency?

--mark

Re: [PATCH 1/2] PM / devfreq: add min/max_freq limit requested by users.

[MyungJoo Ham]

On Fri, Jan 13, 2012 at 1:47 PM, mark gross <markgross@thegnar.org> wrote:
> On Thu, Jan 12, 2012 at 11:08:44AM +0900, MyungJoo Ham wrote:
>>
>> In practice, we have been using min/max to test DVFS behaviors and its
>> side effects. And we are going to use them to 1. restrict power
>> consumption forcibly by the platform software if it is too hot or the
>> battery is low, and to 2. guarantee the minimum performance for
>> specific tasks controlled by the platform software.
>>
>> Anyway, the reason 2 could be tackled by pm-qos if we allow more
>> options in pm-qos with 1. pm qos type to enforce DVFS response time.
> what would pm_qos do with DVFS response time?  What power management
> knob would it enable a constraint for?
>
> pm_qos doesn't do anything but enable power throttling code to consider
> a constraint on how far to throttle "something".  pm_qos has no
> enforcement power.


- The control knob: polling interval of ondemand-like DVFS mechanisms
- It's ok to have no enforcement power. The DVFS mechanism only needs
an interface (PM QoS seems fine for this) from user space / device
drivers to get the response-time requirement.

With some events, we need to adjust DVFS polling interval.

For now, we do this in our devices for user input events (key input,
touchscreen input, ...). And some peripheral device drivers want to
get "guaranteed response time" depending on their operational modes
from memory and bus at the start of their operations.

With user input events, user may (doing something heavy) or may not
(doing something light) want fast reaction from CPU/MEM/GPU/... in
many occasions, and we cannot determine it until the DVFS polling has
been done.

In average, with near 100% threshold, ondemand-like governors will
take 1.5 x polling interval to response. In a system with 100ms
polling interval, DVFS mechanism will take usuallly 150ms (and up to
200ms) to react and this is significantly noticable to human users.
With 60Hz display system, this is loss of almost 10 frames.

In order to address this, a touch event handler (or any
thread/callback or anything deals with it) may request QoS with an
incoming event to reduce polling interval temporarily.

Although PM-QoS does not have the QoS Type for this kind of metric;
however, DVFS response time seems to be another QoS metric candidate.



>
>> 2. pm qos type to enforce graphics performance. And adding a duration
>> option to pm-qos requests will be helpful (sort of a helper function):
>> i.e., pm_qos_timed_request(struct pm_qos_request *req, int
>> pm_qos_class, s32 value, unsigned long duration_ms);
>
> What would be good units for graphics throughput?
> Where in the graphics driver would you insert the equivalent of cpufreq?
> to control the GPU core frequency?

I've not thought about this much yet. I've just seen the need for QoS
requirements from GPU people because DVFS mechanism loses a frame or
frames often during GPU usage without QoS information.

I'm not so familiar with GPUs, so I can't be sure about the metric for
graphics throughput. However, could it be "FLOPS", "triangles per
second", or GPU clock speed?

We have GPU DVFS drivers in linux/drivers/media/video/..., which
controls GPU core frequency and measures GPU usage. However, they can
be implemented with devfreq framework and move into drivers/devfreq/
later.



Cheers!
MyungJoo.


-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics

Re: [PATCH 1/2] PM / devfreq: add min/max_freq limit requested by users.

[mark gross]

Either my mail reader is acting up or this email is a bit off topic.  It
seems to be talking about DVFS/ondemand sampling rates when the
discussion has been about anding CPU operating frequency constraint
requests to pm_qos

On Tue, Jan 17, 2012 at 07:32:05PM +0900, MyungJoo Ham wrote:
> On Fri, Jan 13, 2012 at 1:47 PM, mark gross <markgross@thegnar.org> wrote:
> > On Thu, Jan 12, 2012 at 11:08:44AM +0900, MyungJoo Ham wrote:
> >>
> >> In practice, we have been using min/max to test DVFS behaviors and its
> >> side effects. And we are going to use them to 1. restrict power
> >> consumption forcibly by the platform software if it is too hot or the
> >> battery is low, and to 2. guarantee the minimum performance for
> >> specific tasks controlled by the platform software.
> >>
> >> Anyway, the reason 2 could be tackled by pm-qos if we allow more
> >> options in pm-qos with 1. pm qos type to enforce DVFS response time.
> > what would pm_qos do with DVFS response time?  What power management
> > knob would it enable a constraint for?
> >
> > pm_qos doesn't do anything but enable power throttling code to consider
> > a constraint on how far to throttle "something".  pm_qos has no
> > enforcement power.
> 
> 
> - The control knob: polling interval of ondemand-like DVFS mechanisms
> - It's ok to have no enforcement power. The DVFS mechanism only needs
> an interface (PM QoS seems fine for this) from user space / device
> drivers to get the response-time requirement.
> 
> With some events, we need to adjust DVFS polling interval.

This would be a new type of request.  I don't see it as a pm_qos type of
data item but, I can see it may be useful.

My first thought is that this should be exported by cpufreq.

> For now, we do this in our devices for user input events (key input,
> touchscreen input, ...). And some peripheral device drivers want to
> get "guaranteed response time" depending on their operational modes
> from memory and bus at the start of their operations.
> 
> With user input events, user may (doing something heavy) or may not
> (doing something light) want fast reaction from CPU/MEM/GPU/... in
> many occasions, and we cannot determine it until the DVFS polling has
> been done.
> 
> In average, with near 100% threshold, ondemand-like governors will
> take 1.5 x polling interval to response. In a system with 100ms
> polling interval, DVFS mechanism will take usuallly 150ms (and up to
> 200ms) to react and this is significantly noticable to human users.
> With 60Hz display system, this is loss of almost 10 frames.

Do you really want to change the sampling rate of the DVFS governors or
just use a pm_qos request to limit to minimum frequency the governor
goes for and use a timer to modtime to remove the request on every UI
event from the touch screen driver interrupt handler?

> In order to address this, a touch event handler (or any
> thread/callback or anything deals with it) may request QoS with an
> incoming event to reduce polling interval temporarily.
> 
> Although PM-QoS does not have the QoS Type for this kind of metric;
> however, DVFS response time seems to be another QoS metric candidate.

sure its a candidate but I think perhaps an export from cpufreq may be a
good candidate too.  But, I like my timer scheme best.

However; this still needs a pm_qos class for cpu_min_feq to work.

--mark

> 
> 
> >
> >> 2. pm qos type to enforce graphics performance. And adding a duration
> >> option to pm-qos requests will be helpful (sort of a helper function):
> >> i.e., pm_qos_timed_request(struct pm_qos_request *req, int
> >> pm_qos_class, s32 value, unsigned long duration_ms);
> >
> > What would be good units for graphics throughput?
> > Where in the graphics driver would you insert the equivalent of cpufreq?
> > to control the GPU core frequency?
> 
> I've not thought about this much yet. I've just seen the need for QoS
> requirements from GPU people because DVFS mechanism loses a frame or
> frames often during GPU usage without QoS information.
> 
> I'm not so familiar with GPUs, so I can't be sure about the metric for
> graphics throughput. However, could it be "FLOPS", "triangles per
> second", or GPU clock speed?
> 
> We have GPU DVFS drivers in linux/drivers/media/video/..., which
> controls GPU core frequency and measures GPU usage. However, they can
> be implemented with devfreq framework and move into drivers/devfreq/
> later.
> 
> 
> 
> Cheers!
> MyungJoo.
> 
> 
> -- 
> MyungJoo Ham, Ph.D.
> Mobile Software Platform Lab, DMC Business, Samsung Electronics
> --
> 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

Re: [PATCH 1/2] PM / devfreq: add min/max_freq limit requested by users.

[MyungJoo Ham]

On Wed, Jan 18, 2012 at 12:27 PM, mark gross <markgross@thegnar.org> wrote:
> Either my mail reader is acting up or this email is a bit off topic.  It
> seems to be talking about DVFS/ondemand sampling rates when the
> discussion has been about anding CPU operating frequency constraint
> requests to pm_qos

Yes, indeed. This thread has gone off topic and adjusting sampling
rates has nothing to do with configuring min/max frequencies.

I'll start another thread for adjusting sampling rates for CPUfreq +
Devfreq with PM QoS when a patchset is ready for this.


Thanks.



Cheers!

MyungJoo.

>
> On Tue, Jan 17, 2012 at 07:32:05PM +0900, MyungJoo Ham wrote:
>> On Fri, Jan 13, 2012 at 1:47 PM, mark gross <markgross@thegnar.org> wrote:
>> > On Thu, Jan 12, 2012 at 11:08:44AM +0900, MyungJoo Ham wrote:
>> >>
>> >> In practice, we have been using min/max to test DVFS behaviors and its
>> >> side effects. And we are going to use them to 1. restrict power
>> >> consumption forcibly by the platform software if it is too hot or the
>> >> battery is low, and to 2. guarantee the minimum performance for
>> >> specific tasks controlled by the platform software.
>> >>
>> >> Anyway, the reason 2 could be tackled by pm-qos if we allow more
>> >> options in pm-qos with 1. pm qos type to enforce DVFS response time.
>> > what would pm_qos do with DVFS response time?  What power management
>> > knob would it enable a constraint for?
>> >
>> > pm_qos doesn't do anything but enable power throttling code to consider
>> > a constraint on how far to throttle "something".  pm_qos has no
>> > enforcement power.
>>
>>
>> - The control knob: polling interval of ondemand-like DVFS mechanisms
>> - It's ok to have no enforcement power. The DVFS mechanism only needs
>> an interface (PM QoS seems fine for this) from user space / device
>> drivers to get the response-time requirement.
>>
>> With some events, we need to adjust DVFS polling interval.
>
> This would be a new type of request.  I don't see it as a pm_qos type of
> data item but, I can see it may be useful.
>
> My first thought is that this should be exported by cpufreq.
>
>> For now, we do this in our devices for user input events (key input,
>> touchscreen input, ...). And some peripheral device drivers want to
>> get "guaranteed response time" depending on their operational modes
>> from memory and bus at the start of their operations.
>>
>> With user input events, user may (doing something heavy) or may not
>> (doing something light) want fast reaction from CPU/MEM/GPU/... in
>> many occasions, and we cannot determine it until the DVFS polling has
>> been done.
>>
>> In average, with near 100% threshold, ondemand-like governors will
>> take 1.5 x polling interval to response. In a system with 100ms
>> polling interval, DVFS mechanism will take usuallly 150ms (and up to
>> 200ms) to react and this is significantly noticable to human users.
>> With 60Hz display system, this is loss of almost 10 frames.
>
> Do you really want to change the sampling rate of the DVFS governors or
> just use a pm_qos request to limit to minimum frequency the governor
> goes for and use a timer to modtime to remove the request on every UI
> event from the touch screen driver interrupt handler?
>
>> In order to address this, a touch event handler (or any
>> thread/callback or anything deals with it) may request QoS with an
>> incoming event to reduce polling interval temporarily.
>>
>> Although PM-QoS does not have the QoS Type for this kind of metric;
>> however, DVFS response time seems to be another QoS metric candidate.
>
> sure its a candidate but I think perhaps an export from cpufreq may be a
> good candidate too.  But, I like my timer scheme best.
>
> However; this still needs a pm_qos class for cpu_min_feq to work.
>
> --mark
>
>>
>>
>> >
>> >> 2. pm qos type to enforce graphics performance. And adding a duration
>> >> option to pm-qos requests will be helpful (sort of a helper function):
>> >> i.e., pm_qos_timed_request(struct pm_qos_request *req, int
>> >> pm_qos_class, s32 value, unsigned long duration_ms);
>> >
>> > What would be good units for graphics throughput?
>> > Where in the graphics driver would you insert the equivalent of cpufreq?
>> > to control the GPU core frequency?
>>
>> I've not thought about this much yet. I've just seen the need for QoS
>> requirements from GPU people because DVFS mechanism loses a frame or
>> frames often during GPU usage without QoS information.
>>
>> I'm not so familiar with GPUs, so I can't be sure about the metric for
>> graphics throughput. However, could it be "FLOPS", "triangles per
>> second", or GPU clock speed?
>>
>> We have GPU DVFS drivers in linux/drivers/media/video/..., which
>> controls GPU core frequency and measures GPU usage. However, they can
>> be implemented with devfreq framework and move into drivers/devfreq/
>> later.
>>
>>
>>
>> Cheers!
>> MyungJoo.
>>
>>
>> --
>> MyungJoo Ham, Ph.D.
>> Mobile Software Platform Lab, DMC Business, Samsung Electronics
>> --
>> 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



-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics