Re: [PATCH v2] PM / Domains: Keep the pd status during system PM phases

[Ulf Hansson]

On 5 February 2017 at 10:34, Elaine Zhang <zhangqing@xxxxxxxxxxxxxx> wrote:
>
>
> On 01/26/2017 05:30 AM, Ulf Hansson wrote:
>>
>> On 22 January 2017 at 04:34, Elaine Zhang <zhangqing@xxxxxxxxxxxxxx>
>> wrote:
>>>
>>>
>>>
>>> On 01/20/2017 09:16 PM, Ulf Hansson wrote:
>>>>
>>>>
>>>> On 20 January 2017 at 03:21, Elaine Zhang <zhangqing@xxxxxxxxxxxxxx>
>>>> wrote:
>>>>>
>>>>>
>>>>> If a PM domain is powered off before system suspend,
>>>>> we hope do nothing in system runtime suspend noirq phase
>>>>> and system runtime resume noirq phase.
>>>>
>>>>
>>>>
>>>> One can hope, but that isn't good enough. :-)
>>>>
>>>>
>>>>>
>>>>> This modify is to slove system resume issue for RK3399.
>>>>> RK3399 SOC pd_gpu have voltage domain vdd_gpu,
>>>>> so we must follow open vdd_gpu and power on pd_gpu,
>>>>> power off pd_gpu and disable vdd_gpu.
>>>>> Fix up in runtime resume noirq phase power on all PDs.
>>>>
>>>>
>>>>
>>>> This doesn't make any sense to me. Can please try to explain this is
>>>> in great more detail, then I can try to help.
>>>>
>>> For example:
>>> -->device suspend
>>>     (mali gpu driver set pd_gpu off by pm_runtime_put_sync(),
>>
>>
>> This is the wrong approach, as runtime suspend is prevented by the PM
>> core in this phase. More precisely, it does a
>> pm_runtime_get_noresume() in the device prepare phase.
>>
>> I think it seems like you would benefit from using the so called the
>> runtime PM centric approach, which gives you system PM support for
>> "free". Please have a look at the pm_runtime_force_suspend|resume()
>> helpers.
>>
>>>      and then disabled the vdd_gpu by regulator_disable().)
>>>          --> system suspend
>>>                  -->prepare
>>>                          -->suspend_noirq():
>>>                          (power off all pds)
>>>          -->system resume
>>>                  -->resume_noirq():
>>>                  (power up all pds)
>>>                  (in this case the vdd_gpu is still disabled,
>>>                   if power on the pd_gpu maybe make the system crash)
>>>          -->complete : power off the not used pd
>>> -->device resuem
>>>     (mali gpu driver enable vdd_gpu by regulator_enable(),
>>>      and then power up the pd_gpu by pm_runtime_get_sync())
>>
>>
>> Seems like there is also a missing configuration of the relationship
>> between the PM domains. In the genpd terminology, you probably want to
>> set pd_gpu as a subdomain of the vdd_gpu.
>>
>> In that way, the vdd_gpu is always powered on before pd_gpu is powered
>> on. And vice verse when powering off.
>>
> I thought about this project, but can't slove the problem of sleep and wakp
> up issue.
> Because
>         -->system resume
>                  -->resume_noirq():
>                   (power up all pds)
> In this case, we set pd_gpu as a subdomain of the vdd_gpu.
> And then to enable the vdd_gpu,but the system resume is not complete yet,
> the regulator and i2c can't work.So the vdd_gpu enable will failed.

Okay, I see.

>
> So I think the more appropriate solution is keep the pd status during system
> PM phases.
> If a PM domain is powered off before system suspend,
> we hope do nothing in system runtime suspend noirq phase
> and system runtime resume noirq phase.

Unfortunate, I am still not fully understanding the scenarios. As you
indicate, the problem seems related to wakeup settings.

Could you please try to answer the below questions, hopefully it helps
me to better understand.

1)
While starting the system suspend sequence, under what circumstances
are you expecting the vdd_gpu and pd_gpu to be powered on?

2)
While starting the system suspend sequence, under what circumstances
are you expecting the vdd_gpu and the pd_gpu to be powered off?

3)
What devices are attached to vdd_gpu?

4)
What devices are attached to pd_gpu?


>
>>>
>>> So for RK3399 soc, if to set pd_gpu the vdd_gpu must be enabled, or else
>>> will can't get the ack back.
>>> I hope the pd_gpu power up/off by the driver itself.
>>>

[...]

Kind regards
Uffe