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

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Fri, Jul 07, 2023 at 04:49:59PM +0200, Ulf Hansson wrote:
> + Manivannan
> 
> On Thu, 6 Jul 2023 at 05:06, Ajay Agarwal <ajayagarwal@google.com> wrote:
> >
> > 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.
> 
> >
> > 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.
> 
> >
> > 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.
> 
> >
> > 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.
> 

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

> Kind regards
> Uffe

-- 
மணிவண்ணன் சதாசிவம்