| Message ID | 1484878893-25270-1-git-send-email-zhangqing@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 20, 2017 at 10:21:33AM +0800, Elaine Zhang 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. > > 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. > > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > --- > drivers/base/power/domain.c | 10 +++++++--- > include/linux/pm_domain.h | 1 + > 2 files changed, 8 insertions(+), 3 deletions(-) What changed from v1? You always have to list that below the --- line. thanks, greg k-h
On 20 January 2017 at 03:21, Elaine Zhang <zhangqing@rock-chips.com> 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. > > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > --- > drivers/base/power/domain.c | 10 +++++++--- > include/linux/pm_domain.h | 1 + > 2 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 5711708532db..81351eec570f 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -842,8 +842,10 @@ static int pm_genpd_prepare(struct device *dev) > > genpd_lock(genpd); > > - if (genpd->prepared_count++ == 0) > + if (genpd->prepared_count++ == 0) { > genpd->suspended_count = 0; > + genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF; > + } > > genpd_unlock(genpd); > > @@ -877,7 +879,8 @@ static int pm_genpd_suspend_noirq(struct device *dev) > if (IS_ERR(genpd)) > return -EINVAL; > > - if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)) > + if (genpd->suspend_power_off || > + (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))) > return 0; This isn't going to work, because the PM domain might have been powered on. There is simply no guarantee to that the PM domain remains powered off during the system PM suspend phases. For example, any device in the PM domain that get runtime resumed after the ->prepare() callback is invoked this, will of course also trigger the PM domain to be powered on. > > if (genpd->dev_ops.stop && genpd->dev_ops.start) { > @@ -914,7 +917,8 @@ static int pm_genpd_resume_noirq(struct device *dev) > if (IS_ERR(genpd)) > return -EINVAL; > > - if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)) > + if (genpd->suspend_power_off || > + (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))) > return 0; Ditto, for the same reasons as above. > > /* > diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h > index 81ece61075df..9c0dc364f089 100644 > --- a/include/linux/pm_domain.h > +++ b/include/linux/pm_domain.h > @@ -62,6 +62,7 @@ struct generic_pm_domain { > unsigned int device_count; /* Number of devices */ > unsigned int suspended_count; /* System suspend device counter */ > unsigned int prepared_count; /* Suspend counter of prepared devices */ > + bool suspend_power_off; /* Power status before system suspend */ > int (*power_off)(struct generic_pm_domain *domain); > int (*power_on)(struct generic_pm_domain *domain); > struct gpd_dev_ops dev_ops; > -- > 1.9.1 > > Kind regards Uffe
On 01/20/2017 09:16 PM, Ulf Hansson wrote: > On 20 January 2017 at 03:21, Elaine Zhang <zhangqing@rock-chips.com> 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(), 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()) 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. May be I solution is not the optimal solution,Do you have better suggestion? >> >> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> >> --- >> drivers/base/power/domain.c | 10 +++++++--- >> include/linux/pm_domain.h | 1 + >> 2 files changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c >> index 5711708532db..81351eec570f 100644 >> --- a/drivers/base/power/domain.c >> +++ b/drivers/base/power/domain.c >> @@ -842,8 +842,10 @@ static int pm_genpd_prepare(struct device *dev) >> >> genpd_lock(genpd); >> >> - if (genpd->prepared_count++ == 0) >> + if (genpd->prepared_count++ == 0) { >> genpd->suspended_count = 0; >> + genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF; >> + } >> >> genpd_unlock(genpd); >> >> @@ -877,7 +879,8 @@ static int pm_genpd_suspend_noirq(struct device *dev) >> if (IS_ERR(genpd)) >> return -EINVAL; >> >> - if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)) >> + if (genpd->suspend_power_off || >> + (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))) >> return 0; > > This isn't going to work, because the PM domain might have been powered on. > > There is simply no guarantee to that the PM domain remains powered off > during the system PM suspend phases. For example, any device in the PM > domain that get runtime resumed after the ->prepare() callback is > invoked this, will of course also trigger the PM domain to be powered > on. > >> >> if (genpd->dev_ops.stop && genpd->dev_ops.start) { >> @@ -914,7 +917,8 @@ static int pm_genpd_resume_noirq(struct device *dev) >> if (IS_ERR(genpd)) >> return -EINVAL; >> >> - if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)) >> + if (genpd->suspend_power_off || >> + (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))) >> return 0; > > Ditto, for the same reasons as above. > >> >> /* >> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h >> index 81ece61075df..9c0dc364f089 100644 >> --- a/include/linux/pm_domain.h >> +++ b/include/linux/pm_domain.h >> @@ -62,6 +62,7 @@ struct generic_pm_domain { >> unsigned int device_count; /* Number of devices */ >> unsigned int suspended_count; /* System suspend device counter */ >> unsigned int prepared_count; /* Suspend counter of prepared devices */ >> + bool suspend_power_off; /* Power status before system suspend */ >> int (*power_off)(struct generic_pm_domain *domain); >> int (*power_on)(struct generic_pm_domain *domain); >> struct gpd_dev_ops dev_ops; >> -- >> 1.9.1 >> >> > > Kind regards > Uffe > > >
On 22 January 2017 at 04:34, Elaine Zhang <zhangqing@rock-chips.com> wrote: > > > On 01/20/2017 09:16 PM, Ulf Hansson wrote: >> >> On 20 January 2017 at 03:21, Elaine Zhang <zhangqing@rock-chips.com> >> 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. > > 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. > > May be I solution is not the optimal solution,Do you have better suggestion? > Please see my comment above. [...] Kind regards Uffe
On 01/26/2017 05:30 AM, Ulf Hansson wrote: > On 22 January 2017 at 04:34, Elaine Zhang <zhangqing@rock-chips.com> wrote: >> >> >> On 01/20/2017 09:16 PM, Ulf Hansson wrote: >>> >>> On 20 January 2017 at 03:21, Elaine Zhang <zhangqing@rock-chips.com> >>> 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. 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. >> >> 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. >> >> May be I solution is not the optimal solution,Do you have better suggestion? >> > > Please see my comment above. > > [...] > > Kind regards > Uffe > > >
On 5 February 2017 at 10:34, Elaine Zhang <zhangqing@rock-chips.com> wrote: > > > On 01/26/2017 05:30 AM, Ulf Hansson wrote: >> >> On 22 January 2017 at 04:34, Elaine Zhang <zhangqing@rock-chips.com> >> wrote: >>> >>> >>> >>> On 01/20/2017 09:16 PM, Ulf Hansson wrote: >>>> >>>> >>>> On 20 January 2017 at 03:21, Elaine Zhang <zhangqing@rock-chips.com> >>>> 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
On 02/06/2017 08:46 PM, Ulf Hansson wrote: > On 5 February 2017 at 10:34, Elaine Zhang <zhangqing@rock-chips.com> wrote: >> >> >> On 01/26/2017 05:30 AM, Ulf Hansson wrote: >>> >>> On 22 January 2017 at 04:34, Elaine Zhang <zhangqing@rock-chips.com> >>> wrote: >>>> >>>> >>>> >>>> On 01/20/2017 09:16 PM, Ulf Hansson wrote: >>>>> >>>>> >>>>> On 20 January 2017 at 03:21, Elaine Zhang <zhangqing@rock-chips.com> >>>>> 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? > I don't want to power on the vdd_gpu and pd_gpu during the system suspend sequence. I hope the pd_gpu and vdd_gpu power on/off just by gpu device. > 2) > While starting the system suspend sequence, under what circumstances > are you expecting the vdd_gpu and the pd_gpu to be powered off? > I don't want to power off the vdd_gpu and pd_gpu during the system suspend sequence. Because before the system suspend,the vdd_gpu and pd_gpu has been power off by gpu device(pm_ruantime_put). > 3) > What devices are attached to vdd_gpu? > just pd_gpu > 4) > What devices are attached to pd_gpu? > just gpu device(mali driver) vdd_gpu ------- pd_gpu ----------------devices/platform/ff9a0000.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 > > >
[...] >> >> >> 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? >> > > I don't want to power on the vdd_gpu and pd_gpu during the system suspend > sequence. > I hope the pd_gpu and vdd_gpu power on/off just by gpu device. Let me rephrase my question. Can the vdd_gpu/pd_gpu ever remain in a powered on state while the system is suspended? If yes, when is that the case? > >> 2) >> While starting the system suspend sequence, under what circumstances >> are you expecting the vdd_gpu and the pd_gpu to be powered off? >> > > I don't want to power off the vdd_gpu and pd_gpu during the system suspend > sequence. > Because before the system suspend,the vdd_gpu and pd_gpu has been power off > by gpu device(pm_ruantime_put). *Exactly*, how do you guarantee that a pm_runtime_put() for the gpu device triggers a runtime suspend - before a system suspend sequence starts? For example, userspace may via sysfs prevent runtime suspend for any device with runtime PM enabled. > >> 3) >> What devices are attached to vdd_gpu? >> > just pd_gpu >> >> 4) >> What devices are attached to pd_gpu? >> > just gpu device(mali driver) > > vdd_gpu > ------- pd_gpu > ----------------devices/platform/ff9a0000.gpu >> >> Thanks for these details, very useful! [...] Kind regards Uffe
On 02/07/2017 04:19 PM, Ulf Hansson wrote: > [...] > >>> >>> >>> 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? >>> >> >> I don't want to power on the vdd_gpu and pd_gpu during the system suspend >> sequence. >> I hope the pd_gpu and vdd_gpu power on/off just by gpu device. > > Let me rephrase my question. > > Can the vdd_gpu/pd_gpu ever remain in a powered on state while the > system is suspended? > NO, the vdd_gpu/pd_gpu ever remain in a powered off state while the system is suspended. > If yes, when is that the case? > >> >>> 2) >>> While starting the system suspend sequence, under what circumstances >>> are you expecting the vdd_gpu and the pd_gpu to be powered off? >>> >> >> I don't want to power off the vdd_gpu and pd_gpu during the system suspend >> sequence. >> Because before the system suspend,the vdd_gpu and pd_gpu has been power off >> by gpu device(pm_ruantime_put). > > *Exactly*, how do you guarantee that a pm_runtime_put() for the gpu > device triggers a runtime suspend - before a system suspend sequence > starts? > > For example, userspace may via sysfs prevent runtime suspend for any > device with runtime PM enabled. > Yes, you are right.I did't consider this. Whether on or off pd_gpu, the vdd_gpu is must be enabled.How to deal with the enable/disable the vdd_gpu? our process in devices/platform/ff9a0000.gpu: device init pm_runtime_enable() gpu_power_on regulator_enable(vdd_gpu) pm_runtime_get_sync(pd_gpu) gpu_power_off pm_runtime_put_sync(pd_gpu) regulator_disable(vdd_gpu) device remove pm_runtime_disable() >> >>> 3) >>> What devices are attached to vdd_gpu? >>> >> just pd_gpu >>> >>> 4) >>> What devices are attached to pd_gpu? >>> >> just gpu device(mali driver) >> >> vdd_gpu >> ------- pd_gpu >> ----------------devices/platform/ff9a0000.gpu >>> >>> > > Thanks for these details, very useful! > > [...] > > Kind regards > Uffe > > >
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 5711708532db..81351eec570f 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -842,8 +842,10 @@ static int pm_genpd_prepare(struct device *dev) genpd_lock(genpd); - if (genpd->prepared_count++ == 0) + if (genpd->prepared_count++ == 0) { genpd->suspended_count = 0; + genpd->suspend_power_off = genpd->status == GPD_STATE_POWER_OFF; + } genpd_unlock(genpd); @@ -877,7 +879,8 @@ static int pm_genpd_suspend_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)) + if (genpd->suspend_power_off || + (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))) return 0; if (genpd->dev_ops.stop && genpd->dev_ops.start) { @@ -914,7 +917,8 @@ static int pm_genpd_resume_noirq(struct device *dev) if (IS_ERR(genpd)) return -EINVAL; - if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev)) + if (genpd->suspend_power_off || + (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))) return 0; /* diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 81ece61075df..9c0dc364f089 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -62,6 +62,7 @@ struct generic_pm_domain { unsigned int device_count; /* Number of devices */ unsigned int suspended_count; /* System suspend device counter */ unsigned int prepared_count; /* Suspend counter of prepared devices */ + bool suspend_power_off; /* Power status before system suspend */ int (*power_off)(struct generic_pm_domain *domain); int (*power_on)(struct generic_pm_domain *domain); struct gpd_dev_ops dev_ops;
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. 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. Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> --- drivers/base/power/domain.c | 10 +++++++--- include/linux/pm_domain.h | 1 + 2 files changed, 8 insertions(+), 3 deletions(-)