On Mon, Feb 29, 2016 at 02:56:04PM +0200, Mika Westerberg wrote:
> Add back runtime PM support for PCIe ports that was removed in
> commit fe9a743a2601 ("PCI/PM: Drop unused runtime PM support code for PCIe
> ports").
>
> First of all we cannot enable it automatically for all root ports since
> there have been problems previously,
Pointers to these problems would be useful.
> so we enable it only based on per port
> configuration (runtime_suspend_allowed).
I see that the next patch basically adds a quirk to set
runtime_suspend_allowed for Sunrisepoint. How do we prevent this
from being a maintenance nightmare? I don't want to have to add
a quirk for every new device that supports runtime PM.
> Furthermore we need to check that
> the device, if wake capable, can do so from D3cold (which is the state it
> goes after the root port is powered down).
I don't see this check. I was expecting something like
device_may_wakeup(&pdev->dev) && !pci_pme_capable(pdev, PCI_D3cold)
like you did in the "Move PCIe ports to D3hot during suspend" patch.
> This follows what we did for
> PCIe port system suspend already.
>
> Runtime PM is still blocked and needs to be unblocked from userspace like
> with other PCI devices.
Can you expand on this a bit? I assume you mean that runtime PM is
disabled by default, and userspace has to enable it with a sysfs switch or
something? Can you include information about how to do that?
> Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> ---
> drivers/pci/pcie/portdrv_pci.c | 49 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
> index fe3349685141..eb0f425f51cc 100644
> --- a/drivers/pci/pcie/portdrv_pci.c
> +++ b/drivers/pci/pcie/portdrv_pci.c
> @@ -85,6 +85,7 @@ enum pcie_port_type {
>
> struct pcie_port_config {
> bool suspend_allowed;
> + bool runtime_suspend_allowed;
> };
>
> static const struct pcie_port_config pcie_port_configs[] = {
> @@ -154,6 +155,11 @@ static bool pcie_port_suspend_allowed(struct pci_dev *pdev)
> return pcie_port_can_suspend(pdev);
> }
>
> +static bool pcie_port_runtime_suspend_allowed(struct pci_dev *pdev)
> +{
> + return pcie_port_get_config(pdev)->runtime_suspend_allowed;
> +}
> +
> static int pcie_port_suspend_noirq(struct device *dev)
> {
> struct pci_dev *pdev = to_pci_dev(dev);
> @@ -188,6 +194,34 @@ static int pcie_port_resume_noirq(struct device *dev)
> return 0;
> }
>
> +static int pcie_port_runtime_suspend(struct device *dev)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> +
> + /*
> + * All devices behind the port are assumed to be in D3cold so
> + * update their state now.
> + */
> + __pci_bus_set_current_state(pdev->subordinate, PCI_D3cold);
> + return 0;
> +}
> +
> +static int pcie_port_runtime_resume(struct device *dev)
> +{
> + return 0;
> +}
> +
> +static int pcie_port_runtime_idle(struct device *dev)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> +
> + if (pcie_port_can_suspend(pdev)) {
> + pm_schedule_suspend(dev, 10);
> + return 0;
> + }
> + return -EBUSY;
> +}
> +
> static const struct dev_pm_ops pcie_portdrv_pm_ops = {
> .suspend = pcie_port_device_suspend,
> .resume = pcie_port_device_resume,
> @@ -197,12 +231,20 @@ static const struct dev_pm_ops pcie_portdrv_pm_ops = {
> .restore = pcie_port_device_resume,
> .suspend_noirq = pcie_port_suspend_noirq,
> .resume_noirq = pcie_port_resume_noirq,
> + .runtime_suspend = pcie_port_runtime_suspend,
> + .runtime_resume = pcie_port_runtime_resume,
> + .runtime_idle = pcie_port_runtime_idle,
> };
>
> #define PCIE_PORTDRV_PM_OPS (&pcie_portdrv_pm_ops)
>
> #else /* !PM */
>
> +static inline bool pcie_port_runtime_suspend_allowed(struct pci_dev *pdev)
> +{
> + return false;
> +}
> +
> #define PCIE_PORTDRV_PM_OPS NULL
> #endif /* !PM */
>
> @@ -230,11 +272,18 @@ static int pcie_portdrv_probe(struct pci_dev *dev,
> return status;
>
> pci_save_state(dev);
> +
> + if (pcie_port_runtime_suspend_allowed(dev))
> + pm_runtime_put_noidle(&dev->dev);
> +
> return 0;
> }
>
> static void pcie_portdrv_remove(struct pci_dev *dev)
> {
> + if (pcie_port_runtime_suspend_allowed(dev))
> + pm_runtime_get_noresume(&dev->dev);
> +
> pcie_port_device_remove(dev);
> }
>
> --
> 2.7.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
