Re: [PATCH 2/2] PM / devfreq: Use exclusively PM QoS to determine frequency limits

From: Chanwoo Choi
Date: Mon Jan 13 2020 - 02:24:23 EST


Hi,


Any device driver except for devfreq_cooling.c might
use dev_pm_opp_enable/disable interface.
So, don't need to remove the devfreq->scaling_max_freq
and devfreq->scaling_min_freq for supporting OPP interface.

Regards,
Chanwoo Choi

On 1/11/20 2:49 AM, Matthias Kaehlcke wrote:
> Traditionally devfreq cooling devices dynamically disabled OPPs
> that shouldn't be used because of thermal pressure. Devfreq cooling
> devices now use PM QoS to set frequency limits, hence the devfreq
> code dealing that deals with disabled OPPs can be removed.
>
> Signed-off-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>
> ---
>
> drivers/devfreq/devfreq.c | 75 +++++----------------------------------
> include/linux/devfreq.h | 4 ---
> 2 files changed, 8 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 57f6944d65a6..ec66e2c27cc4 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -73,34 +73,6 @@ static struct devfreq *find_device_devfreq(struct device *dev)
> return ERR_PTR(-ENODEV);
> }
>
> -static unsigned long find_available_min_freq(struct devfreq *devfreq)
> -{
> - struct dev_pm_opp *opp;
> - unsigned long min_freq = 0;
> -
> - opp = dev_pm_opp_find_freq_ceil(devfreq->dev.parent, &min_freq);
> - if (IS_ERR(opp))
> - min_freq = 0;
> - else
> - dev_pm_opp_put(opp);
> -
> - return min_freq;
> -}
> -
> -static unsigned long find_available_max_freq(struct devfreq *devfreq)
> -{
> - struct dev_pm_opp *opp;
> - unsigned long max_freq = ULONG_MAX;
> -
> - opp = dev_pm_opp_find_freq_floor(devfreq->dev.parent, &max_freq);
> - if (IS_ERR(opp))
> - max_freq = 0;
> - else
> - dev_pm_opp_put(opp);
> -
> - return max_freq;
> -}
> -
> /**
> * get_freq_range() - Get the current freq range
> * @devfreq: the devfreq instance
> @@ -141,10 +113,6 @@ static void get_freq_range(struct devfreq *devfreq,
> *max_freq = min(*max_freq,
> (unsigned long)HZ_PER_KHZ * qos_max_freq);
>
> - /* Apply constraints from OPP interface */
> - *min_freq = max(*min_freq, devfreq->scaling_min_freq);
> - *max_freq = min(*max_freq, devfreq->scaling_max_freq);
> -
> if (*min_freq > *max_freq)
> *min_freq = *max_freq;
> }
> @@ -610,23 +578,10 @@ static int devfreq_notifier_call(struct notifier_block *nb, unsigned long type,
> void *devp)
> {
> struct devfreq *devfreq = container_of(nb, struct devfreq, nb);
> - int err = -EINVAL;
> + int err;
>
> mutex_lock(&devfreq->lock);
> -
> - devfreq->scaling_min_freq = find_available_min_freq(devfreq);
> - if (!devfreq->scaling_min_freq)
> - goto out;
> -
> - devfreq->scaling_max_freq = find_available_max_freq(devfreq);
> - if (!devfreq->scaling_max_freq) {
> - devfreq->scaling_max_freq = ULONG_MAX;
> - goto out;
> - }
> -
> err = update_devfreq(devfreq);
> -
> -out:
> mutex_unlock(&devfreq->lock);
> if (err)
> dev_err(devfreq->dev.parent,
> @@ -781,19 +736,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
> mutex_lock(&devfreq->lock);
> }
>
> - devfreq->scaling_min_freq = find_available_min_freq(devfreq);
> - if (!devfreq->scaling_min_freq) {
> - mutex_unlock(&devfreq->lock);
> - err = -EINVAL;
> + err = dev_pm_qos_add_request(dev, &devfreq->user_min_freq_req,
> + DEV_PM_QOS_MIN_FREQUENCY, 0);
> + if (err < 0)
> goto err_dev;
> - }
> -
> - devfreq->scaling_max_freq = find_available_max_freq(devfreq);
> - if (!devfreq->scaling_max_freq) {
> - mutex_unlock(&devfreq->lock);
> - err = -EINVAL;
> + err = dev_pm_qos_add_request(dev, &devfreq->user_max_freq_req,
> + DEV_PM_QOS_MAX_FREQUENCY,
> + PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE);
> + if (err < 0)
> goto err_dev;
> - }
>
> devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev);
> atomic_set(&devfreq->suspend_count, 0);
> @@ -834,16 +785,6 @@ struct devfreq *devfreq_add_device(struct device *dev,
>
> mutex_unlock(&devfreq->lock);
>
> - err = dev_pm_qos_add_request(dev, &devfreq->user_min_freq_req,
> - DEV_PM_QOS_MIN_FREQUENCY, 0);
> - if (err < 0)
> - goto err_devfreq;
> - err = dev_pm_qos_add_request(dev, &devfreq->user_max_freq_req,
> - DEV_PM_QOS_MAX_FREQUENCY,
> - PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE);
> - if (err < 0)
> - goto err_devfreq;
> -
> devfreq->nb_min.notifier_call = qos_min_notifier_call;
> err = dev_pm_qos_add_notifier(devfreq->dev.parent, &devfreq->nb_min,
> DEV_PM_QOS_MIN_FREQUENCY);
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index fb376b5b7281..cb75f23ad2f4 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -126,8 +126,6 @@ struct devfreq_dev_profile {
> * touch this.
> * @user_min_freq_req: PM QoS minimum frequency request from user (via sysfs)
> * @user_max_freq_req: PM QoS maximum frequency request from user (via sysfs)
> - * @scaling_min_freq: Limit minimum frequency requested by OPP interface
> - * @scaling_max_freq: Limit maximum frequency requested by OPP interface
> * @stop_polling: devfreq polling status of a device.
> * @suspend_freq: frequency of a device set during suspend phase.
> * @resume_freq: frequency of a device set in resume phase.
> @@ -166,8 +164,6 @@ struct devfreq {
>
> struct dev_pm_qos_request user_min_freq_req;
> struct dev_pm_qos_request user_max_freq_req;
> - unsigned long scaling_min_freq;
> - unsigned long scaling_max_freq;
> bool stop_polling;
>
> unsigned long suspend_freq;
>