Re: [PATCH 09/11] PM / devfreq: governor: optimize simpleondemand get_target_freq

[Chanwoo Choi]

On 21. 3. 9. 오후 9:58, Dong Aisheng wrote:
> devfreq_simple_ondemand_data only needs to be initialized once when
> calling devm_devfreq_add_device. It's unnecessary to put the data
> check logic in the hot path (.get_target_freq()) where it will be
> called all the time during polling. Instead, we only check and initialize
> it one time during DEVFREQ_GOV_START.
>
> This also helps check data validability in advance during DEVFREQ_GOV_START
> rather than checking it later when running .get_target_freq().
>
> Signed-off-by: Dong Aisheng <aisheng.dong@xxxxxxx>
> ---
>   drivers/devfreq/governor_simpleondemand.c | 50 +++++++++++++++--------
>   1 file changed, 34 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/devfreq/governor_simpleondemand.c b/drivers/devfreq/governor_simpleondemand.c
> index ea287b57cbf3..341eb7e9dc04 100644
> --- a/drivers/devfreq/governor_simpleondemand.c
> +++ b/drivers/devfreq/governor_simpleondemand.c
> @@ -15,15 +15,19 @@
>   /* Default constants for DevFreq-Simple-Ondemand (DFSO) */
>   #define DFSO_UPTHRESHOLD	(90)
>   #define DFSO_DOWNDIFFERENCTIAL	(5)
> +
> +static struct devfreq_simple_ondemand_data od_default = {
> +	.upthreshold = DFSO_UPTHRESHOLD,
> +	.downdifferential = DFSO_DOWNDIFFERENCTIAL,
> +};
> +
>   static int devfreq_simple_ondemand_func(struct devfreq *df,
>   					unsigned long *freq)
>   {
>   	int err;
>   	struct devfreq_dev_status *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;
> +	struct devfreq_simple_ondemand_data *od = df->data;
>   
>   	err = devfreq_update_stats(df);
>   	if (err)
> @@ -31,16 +35,6 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>   
>   	stat = &df->last_status;
>   
> -	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 = DEVFREQ_MAX_FREQ;
> @@ -55,7 +49,7 @@ 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) {
> +	    stat->total_time * od->upthreshold) {
>   		*freq = DEVFREQ_MAX_FREQ;
>   		return 0;
>   	}
> @@ -68,7 +62,7 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>   
>   	/* Keep the current frequency */
>   	if (stat->busy_time * 100 >
> -	    stat->total_time * (dfso_upthreshold - dfso_downdifferential)) {
> +	    stat->total_time * (od->upthreshold - od->downdifferential)) {
>   		*freq = stat->current_frequency;
>   		return 0;
>   	}
> @@ -78,17 +72,41 @@ static int devfreq_simple_ondemand_func(struct devfreq *df,
>   	a *= stat->current_frequency;
>   	b = div_u64(a, stat->total_time);
>   	b *= 100;
> -	b = div_u64(b, (dfso_upthreshold - dfso_downdifferential / 2));
> +	b = div_u64(b, (od->upthreshold - od->downdifferential / 2));
>   	*freq = (unsigned long) b;
>   
>   	return 0;
>   }
>   
> +static int devfreq_simple_ondemand_check_od(struct devfreq *devfreq)
> +{
> +	struct devfreq_simple_ondemand_data *od = devfreq->data;
> +
> +	if (od) {
> +		if (!od->upthreshold)
> +			od->upthreshold = DFSO_UPTHRESHOLD;
> +
> +		if (!od->downdifferential)
> +			od->downdifferential = DFSO_DOWNDIFFERENCTIAL;
> +
> +		if (od->upthreshold > 100 ||
> +		    od->upthreshold < od->downdifferential)
> +			return -EINVAL;
> +	} else {
> +		od = &od_default;
> +	}
> +
> +	return 0;
> +}
> +
>   static int devfreq_simple_ondemand_handler(struct devfreq *devfreq,
>   				unsigned int event, void *data)
>   {
>   	switch (event) {
>   	case DEVFREQ_GOV_START:
> +		if (devfreq_simple_ondemand_check_od(devfreq))
> +			return -EINVAL;
> +
>   		return devfreq_monitor_start(devfreq);
>   
>   	case DEVFREQ_GOV_STOP:

I'm editing the upthreshold and downdifferential for exposing them
via sysfs. So that after my work to expose them via sysfs,
send the patches if you think that need to do more about them.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing&id=dc9e557845c17cee173a6adcc3ae14940da03f44

--
Best Regards,
Samsung Electronics
Chanwoo Choi