Re: [PATCH 4/4] PM / devfreq: Handle monitor start/stop in the devfreq core

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Matthias,

On 19. 2. 15. 오전 4:28, Matthias Kaehlcke wrote:
> Hi Chanwoo,
> 
> On Thu, Feb 14, 2019 at 11:17:36PM +0900, Chanwoo Choi wrote:
>> Hi Matthias,
>>
>> As I commented on the first patch, it is not possible to call some codes
>> according to the intention of each governor between 'devfreq_moniotr_*()'
>> and some codes which are executed before or after 'devfreq_moniotr_*()'
>>
>> For example, if some governor requires the following sequence,
>> after this patch, it is not possible.
>>
>> case DEVFREQ_GOV_xxx:
>>         /* execute some code before devfreq_monitor_xxx() */
>>         devfreq_monitor_xxx()
>>         /* execute some code after devfreq_monitor_xxx() */
> 
> As for the suspend/resume case I agree that the patch introduces this
> limitation, but I'm not convinced that this is an actual problem.
> 
> For governor_start(): why can't the governor execute the code
> before polling started, does it make any difference to the governor
> that a work is scheduled?

The some governors might want to do something before starting
the work and do something after work started. I think that
the existing style is more flexible.

And,
It has one more problem when changing the governor on the fly
from simple_ondemand to other governors like performance,
powersave and so on.

Even if other governors don't need to monitor the utilization,
the work timer will be executed continually because the devfreq
device has the polling_ms value. It is not necessary
of the other governors such as performance, powersave.

It means that only specific governor like simple_ondemand
have to call the devfreq_monitor_start() by itself
instead of calling it on devfreq core.

> 
> For governor_stop(): why would the governor require polling to be
> active during stop? If it needs update_devfreq() to run (called by
> devfreq_monitor()) it can call it directly, instead of waiting for the
> monitor to run at some later time.

As I knew, if the current governors are performance/powersave/
userspace, the monitoring is already stopped and not used.
Because they don't need to handle the codes related to the work
like queue_delayed_work(), cancel_delayed_work_sync().

And,
In case of the existing style for calling devfreq_monitor_*(),
other governors like performance/powersave/userspace/passice
don't need to call the devfreq_monitor_stop() because they
didn't use the work timer.

> 
> Cheers
> 
> Matthias
> 
> 
> 
> 
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux