Re: [PATCH v5] PCI: Enable runtime pm of the host bridge

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Oct 09, 2024 at 08:10:32PM +0200, Marek Szyprowski wrote:
> On 03.10.2024 08:02, Krishna chaitanya chundru wrote:
> > The Controller driver is the parent device of the PCIe host bridge,
> > PCI-PCI bridge and PCIe endpoint as shown below.
> >
> >          PCIe controller(Top level parent & parent of host bridge)
> >                          |
> >                          v
> >          PCIe Host bridge(Parent of PCI-PCI bridge)
> >                          |
> >                          v
> >          PCI-PCI bridge(Parent of endpoint driver)
> >                          |
> >                          v
> >                  PCIe endpoint driver
> >
> > Now, when the controller device goes to runtime suspend, PM framework
> > will check the runtime PM state of the child device (host bridge) and
> > will find it to be disabled. So it will allow the parent (controller
> > device) to go to runtime suspend. Only if the child device's state was
> > 'active' it will prevent the parent to get suspended.
> >
> > It is a property of the runtime PM framework that it can only
> > follow continuous dependency chains.  That is, if there is a device
> > with runtime PM disabled in a dependency chain, runtime PM cannot be
> > enabled for devices below it and above it in that chain both at the
> > same time.
> >
> > Since runtime PM is disabled for host bridge, the state of the child
> > devices under the host bridge is not taken into account by PM framework
> > for the top level parent, PCIe controller. So PM framework, allows
> > the controller driver to enter runtime PM irrespective of the state
> > of the devices under the host bridge. And this causes the topology
> > breakage and also possible PM issues like controller driver goes to
> > runtime suspend while endpoint driver is doing some transfers.
> >
> > Because of the above, in order to enable runtime PM for a PCIe
> > controller device, one needs to ensure that runtime PM is enabled for
> > all devices in every dependency chain between it and any PCIe endpoint
> > (as runtime PM is enabled for PCIe endpoints).
> >
> > This means that runtime PM needs to be enabled for the host bridge
> > device, which is present in all of these dependency chains.
> >
> > After this change, the host bridge device will be runtime-suspended
> > by the runtime PM framework automatically after suspending its last
> > child and it will be runtime-resumed automatically before resuming its
> > first child which will allow the runtime PM framework to track
> > dependencies between the host bridge device and all of its
> > descendants.
> >
> > Signed-off-by: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> 
> This patch landed in today's linux-next as commit 02787a3b4d10 ("PCI/PM: 
> Enable runtime power management for host bridges"). In my tests I found 
> that it triggers a warning on StarFive VisionFive2 RISC-V board. It 
> looks that some more changes are needed in the dwc-pci driver or so. 
> There is a message from runtime pm subsystem about inactive device with 
> active children and suspicious locking pattern. Here is the log I 
> observed on that board:
> ...

Thanks very much for the testing and report, Marek!

I dropped this patch from the PCI -next for now.  We can add it back
with the fix squashed into it after the complete patch is posted and
tested.

Bjorn