Skip to contentSkip to conversation

Prevent PM suspend from powering off the domain for non-wakeup in-use devices

DISCUSSION Ajay Agarwal 4 participants 9 messages Jul 6, 2023 – Aug 11, 2023

lore
Share & tools
lore
#0 lore raw

Hello Linux PM experts
I have a question on PM domain. As per the current PM domain driver design, the genpd_finish_suspend turns OFF a power domain if it is not already turned OFF by runtime suspend.

I have a usecase of a device having to stay ON during system suspend. This device will be used by a co-processor which is running its own OS. Thereby, it requires the power domain to stay turned ON, so genpd_finish_suspend should not be powering down the domain.

I studied the code and found the flag GENPD_FLAG_ACTIVE_WAKEUP which can be set for the power domain. And device_set_wakeup_path can be invoked in the suspend() callback of the device. Together, these will prevent the genpd_finish_suspend from turning OFF the domain. See: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/power/domain.c#n1260

But this flag is really intended to be used for devices which are capable of waking up the system from PM suspend. But my usecase does not involve a scenario of the device potentially waking up the system, it just needs to stay powered for the co-processor to be able to use it.

Can you suggest if I should go ahead with using the existing framework of GENPD_FLAG_ACTIVE_WAKEUP? Or add a new flag, say GENPD_FLAG_RPM_ONLY for this scenario?

Thanks
Ajay

#1 lore raw

+ Manivannan

Quoted 7 lines · Ajay Agarwal

Hello Linux PM experts
I have a question on PM domain. As per the current PM domain driver design, the genpd_finish_suspend turns OFF a power domain if it is not already turned OFF by runtime suspend.

I have a usecase of a device having to stay ON during system suspend. This device will be used by a co-processor which is running its own OS. Thereby, it requires the power domain to stay turned ON, so genpd_finish_suspend should not be powering down the domain.

I studied the code and found the flag GENPD_FLAG_ACTIVE_WAKEUP which can be set for the power domain. And device_set_wakeup_path can be invoked in the suspend() callback of the device. Together, these will prevent the genpd_finish_suspend from turning OFF the domain. See: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/power/domain.c#n1260

To solve the problem, this is exactly what I would have started to explore too.

Quoted 2 lines

But this flag is really intended to be used for devices which are capable of waking up the system from PM suspend. But my usecase does not involve a scenario of the device potentially waking up the system, it just needs to stay powered for the co-processor to be able to use it.

Sure, I understand your point. Maybe it's as simple as renaming the device_set_wakeup_path() and corresponding flag to something more generic.

Solving the problem that you are looking for, would probably be done along the lines of device_set_wakeup_path() and GENPD_FLAG_ACTIVE_WAKEUP anyway. And we really don't want to two solutions for one similar problem, if you get my point.

Quoted 2 lines

Can you suggest if I should go ahead with using the existing framework of GENPD_FLAG_ACTIVE_WAKEUP? Or add a new flag, say GENPD_FLAG_RPM_ONLY for this scenario?

Let's see what Rafael thinks, but renaming is an option (or add a wrapper function that calls device_set_wakeup_path()) that would work for me.

Quoted 3 lines

Thanks
Ajay

Note that, I have looped in Manivannan, who is working on a very similar problem. The problem can be summarized like this:

Depending if there is an nvme storage device added to the pcie interface, the nvmw storage must remain powered on during system suspend. For that reason, we also need the part from the PM core where propagation of the flag is done from child to parents.

It's been a couple of weeks ago since I chatted with Manivannan about this - and I suggested the approach you have been exploring too. Let's see if Manivannan has some more updates to share from his side around this.

Kind regards
Uffe

#2 lore raw
Quoted 44 lines · Ulf Hansson

+ Manivannan

Quoted 7 lines · Ajay Agarwal

Hello Linux PM experts
I have a question on PM domain. As per the current PM domain driver design, the genpd_finish_suspend turns OFF a power domain if it is not already turned OFF by runtime suspend.

I have a usecase of a device having to stay ON during system suspend. This device will be used by a co-processor which is running its own OS. Thereby, it requires the power domain to stay turned ON, so genpd_finish_suspend should not be powering down the domain.

I studied the code and found the flag GENPD_FLAG_ACTIVE_WAKEUP which can be set for the power domain. And device_set_wakeup_path can be invoked in the suspend() callback of the device. Together, these will prevent the genpd_finish_suspend from turning OFF the domain. See: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/power/domain.c#n1260

To solve the problem, this is exactly what I would have started to explore too.

Quoted 2 lines

But this flag is really intended to be used for devices which are capable of waking up the system from PM suspend. But my usecase does not involve a scenario of the device potentially waking up the system, it just needs to stay powered for the co-processor to be able to use it.

Sure, I understand your point. Maybe it's as simple as renaming the device_set_wakeup_path() and corresponding flag to something more generic.

Solving the problem that you are looking for, would probably be done along the lines of device_set_wakeup_path() and GENPD_FLAG_ACTIVE_WAKEUP anyway. And we really don't want to two solutions for one similar problem, if you get my point.

Quoted 2 lines

Can you suggest if I should go ahead with using the existing framework of GENPD_FLAG_ACTIVE_WAKEUP? Or add a new flag, say GENPD_FLAG_RPM_ONLY for this scenario?

Let's see what Rafael thinks, but renaming is an option (or add a wrapper function that calls device_set_wakeup_path()) that would work for me.

Quoted 3 lines

Thanks
Ajay

Note that, I have looped in Manivannan, who is working on a very similar problem. The problem can be summarized like this:

Depending if there is an nvme storage device added to the pcie interface, the nvmw storage must remain powered on during system suspend. For that reason, we also need the part from the PM core where propagation of the flag is done from child to parents.

Quoted 5 lines

It's been a couple of weeks ago since I chatted with Manivannan about this - and I suggested the approach you have been exploring too. Let's see if Manivannan has some more updates to share from his side around this.

Thanks Ulf for looping me in. I worked around the issue at the hardware level i.e., On the Qcom SoCs we can set the power domain in retention mode. So linux genpd driver will not turn off those domains marked as retention and the power management hardware (RPMh) will use a backup domain to preserve the state of the domain even if it's parent domain goes down during system suspend. This keeps the NVMe device ON during suspend and at the same time allows the SoC to enter low power state.

I couldn't update you on the progress of this issue since I was out for a couple of weeks.

But the solution I described above is pretty much Qcom specific, so it is indeed useful to have a generic solution in genpd framework similar to device_set_wakeup_path().

My suggestion would be to introduce a new helper instead of renaming the existing device_set_wakeup_path() API, eventhough both are doing the same but their purposes differ.

- Mani

Quoted 2 lines

Kind regards
Uffe

Signature · 1 line
மணிவண்ணன் சதாசிவம்
#3 lore raw

Thanks Ulf and Mani for looking into this. I could not quite understand your suggestions here. I believe you are talking about adding a new helper API. What flag will this API update? Will it be dev->power.wakeup_path itself? Or do we introduce a new boolean flag in dev_pm_info structure for this?

Also, how about the genpd flag? Shall we re-use the GENPD_FLAG_ACTIVE_WAKEUP flag? Or create a new one?

I feel that we should come up with a generic name for both the genpd flag and the power flag to satisfy both the wakeup usecase and the stay-on usecase. Let me know what you think.

Rafael, can you chime in as well?

#4 lore raw
Quoted 6 lines · Ajay Agarwal

Thanks Ulf and Mani for looking into this. I could not quite understand your suggestions here. I believe you are talking about adding a new helper API. What flag will this API update? Will it be dev->power.wakeup_path itself? Or do we introduce a new boolean flag in dev_pm_info structure for this?

Honestly I am not exactly sure on what I think. Someone (possibly myself) needs to draft a patch to see what it looks like. Although, adding a new flag that would behave exactly like 'dev->power.wakeup_path' seems a bit silly to me.

Quoted 7 lines

Also, how about the genpd flag? Shall we re-use the GENPD_FLAG_ACTIVE_WAKEUP flag? Or create a new one?

I feel that we should come up with a generic name for both the genpd flag and the power flag to satisfy both the wakeup usecase and the stay-on usecase. Let me know what you think.

I agree that the naming isn't particularly good for this use case. I don't have a good suggestion for how to deal with it. If these things aren't being used too much out in the kernel, maybe a tree wide commit (at some good point) to rename things could work?

Quoted 2 lines

Rafael, can you chime in as well?

Kind regards
Uffe

#5 lore raw
Quoted 3 lines · Ajay Agarwal

Hello Linux PM experts
I have a question on PM domain. As per the current PM domain driver design, the genpd_finish_suspend turns OFF a power domain if it is not already turned OFF by runtime suspend.

I don't really understand why genPD does this in the first place. Not only does it power off domains that might have RPM_ACTIVE devices during suspend, it also powers on domains that might not have any active devices in them during resume (except for wakeup devices, similar to power-off. See genpd_finish_resume).

Why isn't genPD powering-off/on domains based on device's RPM status sufficient? Assuming those are correctly set by device drivers during system suspend/resume.

Such that this:

Quoted 1 line

But this flag is really intended to be used for devices which are capable of waking up the system from PM suspend. But my usecase does not involve a scenario of the device potentially waking up the system, it just needs to stay powered for the co-processor to be able to use it.

becomes just a matter of the device keeping its RPM status as active during system suspend.

#6 lore raw
Quoted 12 lines · Michael Shavit
Quoted 3 lines · Ajay Agarwal

Hello Linux PM experts
I have a question on PM domain. As per the current PM domain driver design, the genpd_finish_suspend turns OFF a power domain if it is not already turned OFF by runtime suspend.

I don't really understand why genPD does this in the first place. Not only does it power off domains that might have RPM_ACTIVE devices during suspend, it also powers on domains that might not have any active devices in them during resume (except for wakeup devices, similar to power-off. See genpd_finish_resume).

Quoted 3 lines

Why isn't genPD powering-off/on domains based on device's RPM status sufficient? Assuming those are correctly set by device drivers during system suspend/resume.

I think you kind of already answered this by yourself. We can't rely on all drivers to update the runtime PM status of their devices during system wide suspend - simply because there are no requirements (legacy wise) that they need to.

Moreover, as runtime PM gets disabled/enabled for all devices by the PM core in the suspend_late/resume_early phase (for good reasons) - we have a window when runtime PM can't be used to power-on/off devices. In other words, if a driver needs to power on its device (and the corresponding PM domain) during this window, how will that be done?

Quoted 6 lines

Such that this:

Quoted 1 line

But this flag is really intended to be used for devices which are capable of waking up the system from PM suspend. But my usecase does not involve a scenario of the device potentially waking up the system, it just needs to stay powered for the co-processor to be able to use it.

becomes just a matter of the device keeping its RPM status as active during system suspend.

Theoretically this could work, but unfortunately not in practice with all the legacy code also taken into account.

Kind regards
Uffe

#7 lore raw
Quoted 4 lines · Ulf Hansson

I think you kind of already answered this by yourself. We can't rely on all drivers to update the runtime PM status of their devices during system wide suspend - simply because there are no requirements (legacy wise) that they need to.

[...]

Quoted 5 lines

Moreover, as runtime PM gets disabled/enabled for all devices by the PM core in the suspend_late/resume_early phase (for good reasons) - we have a window when runtime PM can't be used to power-on/off devices. In other words, if a driver needs to power on its device (and the corresponding PM domain) during this window, how will that be done?

Ok makes sense. If we're to rename dev->power.wakeup_path, could we then think of it as a way for device drivers to declare that their RPM state is in fact managed during system suspend/resume?

Another question that I haven't been able to figure out by tracing through the code history, is why the genpd domain *also* needs to declare the GENPD_FLAG_ACTIVE_WAKEUP flag. Shouldn't device drivers setting device_set_wakeup_path() be sufficient for the core to decide not to force power it off during suspend? In what scenarios would a genpd provider want to leave that flag unset?

#8 lore raw
Quoted 13 lines · Michael Shavit
Quoted 4 lines · Ulf Hansson

I think you kind of already answered this by yourself. We can't rely on all drivers to update the runtime PM status of their devices during system wide suspend - simply because there are no requirements (legacy wise) that they need to.

[...]

Quoted 5 lines

Moreover, as runtime PM gets disabled/enabled for all devices by the PM core in the suspend_late/resume_early phase (for good reasons) - we have a window when runtime PM can't be used to power-on/off devices. In other words, if a driver needs to power on its device (and the corresponding PM domain) during this window, how will that be done?

Quoted 3 lines

Ok makes sense. If we're to rename dev->power.wakeup_path, could we then think of it as a way for device drivers to declare that their RPM state is in fact managed during system suspend/resume?

I just posted a patch and decided to start simple, just by adding a couple of new helper functions that act as wrappers of the existing ones.

What you are proposing seems to be something entirely new. We can't reuse the existing flag for this kind of thing, I think.

Quoted 7 lines

Another question that I haven't been able to figure out by tracing through the code history, is why the genpd domain *also* needs to declare the GENPD_FLAG_ACTIVE_WAKEUP flag. Shouldn't device drivers setting device_set_wakeup_path() be sufficient for the core to decide not to force power it off during suspend? In what scenarios would a genpd provider want to leave that flag unset?

That's a good question. In principle we decided to keep GENPD_FLAG_ACTIVE_WAKEUP a while ago [1], as a way to let the genpd provider opt-in to support so called in-band wakeup irqs.

For out-band wakeup irqs there may be some additional pieces missing, to allow the driver/subsystem to inform the PM domain that it has configured a system wakeup, that doesn't require the in-band wakeup to be armed.

Kind regards
Uffe

[1]
https://lore.kernel.org/all/[email protected]/