[PATCH v1 1/2] PM/devfreq: add suspend frequency support

[hl]

On 2016年11月22日 20:48, MyungJoo Ham wrote:
> On Thu, Nov 3, 2016 at 4:03 PM, hl <hl at rock-chips.com> wrote:
>> Hi MyungJoo Ham,
>>
>>
>> On 2016年11月02日 14:35, MyungJoo Ham wrote:
>>>> Add suspend frequency support and if needed set it to
>>>> the frequency obtained from the suspend opp (can be defined
>>>> using opp-v2 bindings and is optional).
>>>>
>>>> Change-Id: Iaa0d3848d63d9ce03f65ea76f263e4685a4c295e
>>>> Signed-off-by: Lin Huang <hl at rock-chips.com>
>>>> ---
>>>>    drivers/devfreq/devfreq.c | 17 ++++++++++++++++-
>>>>    include/linux/devfreq.h   |  9 +++++++++
>>>>    2 files changed, 25 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>>> index bf3ea76..d1152eb 100644
>>>> --- a/drivers/devfreq/devfreq.c
>>>> +++ b/drivers/devfreq/devfreq.c
>>>> @@ -364,7 +364,10 @@ void devfreq_monitor_suspend(struct devfreq
>>>> *devfreq)
>>>>                  return;
>>>>          }
>>>>    -     devfreq_update_status(devfreq, devfreq->previous_freq);
>>>> +       if (devfreq->suspend_freq)
>>>> +               devfreq_update_status(devfreq, devfreq->suspend_freq);
>>>> +       else
>>>> +               devfreq_update_status(devfreq, devfreq->previous_freq);
>>> This is incorrect.
>>>
>>> devfreq_update_status is to record the statistics.
>>> This code intends to record the current status before entering suspend;
>>> thus you should not record "suspend_freq" here.
>>>
>>> If you really need to record the frequency during suspended state
>>> for the statistics, you need to do that at resume; however, I object to
>>> that idea either.
>>>
>>> If you really want to set a predefined suspend-to-RAM mode frequency,
>>> (probably because of HW instability during resume process)
>>> you need to update, not udpate_status (statistics).
>>>
>> Thanks for pointing it, but if i use update_devfreq(), it can not specify
>> the
>> frequency, it call the get_target_freq() to get the frequency and setting,
>> use now devfreq framework, it seems hard to set the specific freqeuency
>> when suspend. Do you have any idea, thanks.
>
> It appears that we need to set the frequency directly with target() function
> at suspend/resume after it is assured that related routines are
> already suspended.
>
> Plus, you might need to consider using devfreq_suspend/resume_device instead
> of monitor. monitor functions are for the governor-specific behaviors.
> You'll need such capabilities regardless of governors. (even userspace-governor
> needs this)
We still need to sync the all status even i call target() in 
devfreq_suspend/resume_device
directly, so still need update_devfreq() other setp except
devfreq->governor->get_target_freq(devfreq, &freq);
>
> Cheers,
> MyungJoo
>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>

-- 
Lin Huang