[PATCH v4 0/8] Runtime PM for Thunderbolt on Macs

[PATCH v4 0/8] Runtime PM for Thunderbolt on Macs

[Lukas Wunner]

Power down Thunderbolt controllers on Macs when nothing is plugged in
to save around 2W per controller.

For details see the cover letter of v3:
https://lkml.org/lkml/2016/12/17/56


Patches [1/7] to [3/7] need an ack from Bjorn (and possibly Rafael or
Mika).  Patches [4/7] to [6/7] need an ack from Rafael.


Changes since v3:

- Additional patch [6/8] by Chen Yu to define a DPM_DIRECT_COMPLETE macro.
  I had expected this to land in v4.10 but it didn't.

- Rework patch [7/8] ("thunderbolt: Power down controller when idle")
  according to Andy Shevchenko's feedback:  Drop unnecessary #ifdef pr_fmt,
  add explanatory comments, rename goto labels.

- Fix error path if the upstream bridge cannot be found.

The patches are also browseable on GitHub:
https://github.com/l1k/linux/commits/thunderbolt_runpm_v4

Thanks,

Lukas


Chen Yu (1):
  PM / sleep: Define constant for direct_complete

Lukas Wunner (7):
  PCI: Recognize Thunderbolt devices
  PCI: Allow runtime PM on Thunderbolt ports
  PCI: Don't block runtime PM for Thunderbolt host hotplug ports
  Revert "PM / Runtime: Remove the exported function
    pm_children_suspended()"
  PM: Make requirements of dev_pm_domain_set() more precise
  thunderbolt: Power down controller when idle
  thunderbolt: Runtime suspend NHI when idle

 drivers/base/power/common.c  |  15 +-
 drivers/base/power/runtime.c |   3 +-
 drivers/pci/pci.c            |  20 ++-
 drivers/pci/pci.h            |   2 +
 drivers/pci/probe.c          |  34 +++++
 drivers/thunderbolt/Kconfig  |   3 +-
 drivers/thunderbolt/Makefile |   4 +-
 drivers/thunderbolt/nhi.c    |   5 +
 drivers/thunderbolt/power.c  | 355 +++++++++++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/power.h  |  37 +++++
 drivers/thunderbolt/switch.c |   9 ++
 drivers/thunderbolt/tb.c     |  13 ++
 drivers/thunderbolt/tb.h     |   2 +
 include/linux/pci.h          |   1 +
 include/linux/pm.h           |   7 +
 include/linux/pm_runtime.h   |   7 +
 16 files changed, 506 insertions(+), 11 deletions(-)
 create mode 100644 drivers/thunderbolt/power.c
 create mode 100644 drivers/thunderbolt/power.h

-- 
2.11.0

[PATCH v4 2/8] PCI: Allow runtime PM on Thunderbolt ports

[Lukas Wunner]

Currently PCIe ports are only allowed to go to D3 if the BIOS is dated
2015 or newer to avoid potential issues with old chipsets.  However for
Thunderbolt we know that even the oldest controller, Light Ridge (2010),
is able to suspend its ports to D3 just fine.

We're about to add runtime PM for Thunderbolt on the Mac.  Apple has
released two EFI security updates in 2015 which encompass all machines
with Thunderbolt, but the achieved power saving should be made available
to users even if they haven't updated their BIOS.  To this end,
special-case Thunderbolt in pci_bridge_d3_possible().

This allows the Thunderbolt controller to power down but the root port
to which the Thunderbolt controller is attached remains in D0 unless
the EFI update is installed.  Users can pass pcie_port_pm=force on the
kernel command line if they cannot install the EFI update but still want
to benefit from the additional power saving of putting the root port
into D3.  In practice, root ports can be suspended to D3 without issues
at least on 2012 Ivy Bridge machines.

If the BIOS cut-off date is ever lowered to 2010, the Thunderbolt
special case can be removed.

Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Andreas Noever <andreas.noever@gmail.com>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Cc: Amir Levy <amir.jer.levy@intel.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/pci.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index a881c0d..8ed098d 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2224,7 +2224,7 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev)
  * @bridge: Bridge to check
  *
  * This function checks if it is possible to move the bridge to D3.
- * Currently we only allow D3 for recent enough PCIe ports.
+ * Currently we only allow D3 for recent enough PCIe ports and Thunderbolt.
  */
 bool pci_bridge_d3_possible(struct pci_dev *bridge)
 {
@@ -2258,6 +2258,11 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
 		    year >= 2015) {
 			return true;
 		}
+
+		/* Even the oldest 2010 Thunderbolt controller supports D3. */
+		if (bridge->is_thunderbolt)
+			return true;
+
 		break;
 	}
 
-- 
2.11.0

Re: [PATCH v4 2/8] PCI: Allow runtime PM on Thunderbolt ports

[Mika Westerberg]

On Sun, Jan 08, 2017 at 09:41:45AM +0100, Lukas Wunner wrote:
> Currently PCIe ports are only allowed to go to D3 if the BIOS is dated
> 2015 or newer to avoid potential issues with old chipsets.  However for
> Thunderbolt we know that even the oldest controller, Light Ridge (2010),
> is able to suspend its ports to D3 just fine.
> 
> We're about to add runtime PM for Thunderbolt on the Mac.  Apple has
> released two EFI security updates in 2015 which encompass all machines
> with Thunderbolt, but the achieved power saving should be made available
> to users even if they haven't updated their BIOS.  To this end,
> special-case Thunderbolt in pci_bridge_d3_possible().
> 
> This allows the Thunderbolt controller to power down but the root port
> to which the Thunderbolt controller is attached remains in D0 unless
> the EFI update is installed.  Users can pass pcie_port_pm=force on the
> kernel command line if they cannot install the EFI update but still want
> to benefit from the additional power saving of putting the root port
> into D3.  In practice, root ports can be suspended to D3 without issues
> at least on 2012 Ivy Bridge machines.
> 
> If the BIOS cut-off date is ever lowered to 2010, the Thunderbolt
> special case can be removed.
> 
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>

Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>

[PATCH v4 3/8] PCI: Don't block runtime PM for Thunderbolt host hotplug ports

[Lukas Wunner]

Hotplug ports generally block their parents from suspending to D3hot as
otherwise their interrupts couldn't be delivered.

An exception are Thunderbolt host controllers:  They have a separate
GPIO pin to side-band signal plug events even if the controller is
powered down or its parent ports are suspended to D3.  They can be told
apart from Thunderbolt controllers in attached devices by checking if
they're situated below a non-Thunderbolt device (typically a root port,
or the downstream port of a PCIe switch in the case of the MacPro6,1).

To enable runtime PM for Thunderbolt on the Mac, the downstream bridges
of a host controller must not block runtime PM on the upstream bridge as
power to the chip is only cut once the upstream bridge has suspended.
Amend the condition in pci_dev_check_d3cold() accordingly.

Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Andreas Noever <andreas.noever@gmail.com>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Cc: Amir Levy <amir.jer.levy@intel.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/pci.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 8ed098d..0b03fe7 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2271,6 +2271,7 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
 
 static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
 {
+	struct pci_dev *parent, *grandparent;
 	bool *d3cold_ok = data;
 
 	if (/* The device needs to be allowed to go D3cold ... */
@@ -2284,7 +2285,17 @@ static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
 	    !pci_power_manageable(dev) ||
 
 	    /* Hotplug interrupts cannot be delivered if the link is down. */
-	    dev->is_hotplug_bridge)
+	    (dev->is_hotplug_bridge &&
+
+		/*
+		 * Exception:  Thunderbolt host controllers have a pin to
+		 * side-band signal plug events.  Their hotplug ports are
+		 * recognizable by having a non-Thunderbolt device as
+		 * grandparent.
+		 */
+		!(dev->is_thunderbolt && (parent = pci_upstream_bridge(dev)) &&
+				 (grandparent = pci_upstream_bridge(parent)) &&
+						!grandparent->is_thunderbolt)))
 
 		*d3cold_ok = false;
 
-- 
2.11.0

Re: [PATCH v4 3/8] PCI: Don't block runtime PM for Thunderbolt host hotplug ports

[Mika Westerberg]

On Sun, Jan 08, 2017 at 09:41:45AM +0100, Lukas Wunner wrote:
> Hotplug ports generally block their parents from suspending to D3hot as
> otherwise their interrupts couldn't be delivered.
> 
> An exception are Thunderbolt host controllers:  They have a separate
> GPIO pin to side-band signal plug events even if the controller is
> powered down or its parent ports are suspended to D3.  They can be told
> apart from Thunderbolt controllers in attached devices by checking if
> they're situated below a non-Thunderbolt device (typically a root port,
> or the downstream port of a PCIe switch in the case of the MacPro6,1).
> 
> To enable runtime PM for Thunderbolt on the Mac, the downstream bridges
> of a host controller must not block runtime PM on the upstream bridge as
> power to the chip is only cut once the upstream bridge has suspended.
> Amend the condition in pci_dev_check_d3cold() accordingly.
> 
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Andreas Noever <andreas.noever@gmail.com>
> Cc: Tomas Winkler <tomas.winkler@intel.com>
> Cc: Amir Levy <amir.jer.levy@intel.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  drivers/pci/pci.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8ed098d..0b03fe7 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2271,6 +2271,7 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
>  
>  static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
>  {
> +	struct pci_dev *parent, *grandparent;
>  	bool *d3cold_ok = data;
>  
>  	if (/* The device needs to be allowed to go D3cold ... */
> @@ -2284,7 +2285,17 @@ static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
>  	    !pci_power_manageable(dev) ||
>  
>  	    /* Hotplug interrupts cannot be delivered if the link is down. */
> -	    dev->is_hotplug_bridge)
> +	    (dev->is_hotplug_bridge &&
> +
> +		/*
> +		 * Exception:  Thunderbolt host controllers have a pin to
> +		 * side-band signal plug events.  Their hotplug ports are
> +		 * recognizable by having a non-Thunderbolt device as
> +		 * grandparent.
> +		 */
> +		!(dev->is_thunderbolt && (parent = pci_upstream_bridge(dev)) &&
> +				 (grandparent = pci_upstream_bridge(parent)) &&
> +						!grandparent->is_thunderbolt)))

Can you move this to its own helper function?

>  
>  		*d3cold_ok = false;
>  
> -- 
> 2.11.0

Re: [PATCH v4 3/8] PCI: Don't block runtime PM for Thunderbolt host hotplug ports

[Lukas Wunner]

On Wed, Jan 11, 2017 at 12:02:10PM +0200, Mika Westerberg wrote:
> On Sun, Jan 08, 2017 at 09:41:45AM +0100, Lukas Wunner wrote:
> > Hotplug ports generally block their parents from suspending to D3hot as
> > otherwise their interrupts couldn't be delivered.
> > 
> > An exception are Thunderbolt host controllers:  They have a separate
> > GPIO pin to side-band signal plug events even if the controller is
> > powered down or its parent ports are suspended to D3.  They can be told
> > apart from Thunderbolt controllers in attached devices by checking if
> > they're situated below a non-Thunderbolt device (typically a root port,
> > or the downstream port of a PCIe switch in the case of the MacPro6,1).
> > 
> > To enable runtime PM for Thunderbolt on the Mac, the downstream bridges
> > of a host controller must not block runtime PM on the upstream bridge as
> > power to the chip is only cut once the upstream bridge has suspended.
> > Amend the condition in pci_dev_check_d3cold() accordingly.
> > 
> > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Andreas Noever <andreas.noever@gmail.com>
> > Cc: Tomas Winkler <tomas.winkler@intel.com>
> > Cc: Amir Levy <amir.jer.levy@intel.com>
> > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > ---
> >  drivers/pci/pci.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 8ed098d..0b03fe7 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -2271,6 +2271,7 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
> >  
> >  static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
> >  {
> > +	struct pci_dev *parent, *grandparent;
> >  	bool *d3cold_ok = data;
> >  
> >  	if (/* The device needs to be allowed to go D3cold ... */
> > @@ -2284,7 +2285,17 @@ static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
> >  	    !pci_power_manageable(dev) ||
> >  
> >  	    /* Hotplug interrupts cannot be delivered if the link is down. */
> > -	    dev->is_hotplug_bridge)
> > +	    (dev->is_hotplug_bridge &&
> > +
> > +		/*
> > +		 * Exception:  Thunderbolt host controllers have a pin to
> > +		 * side-band signal plug events.  Their hotplug ports are
> > +		 * recognizable by having a non-Thunderbolt device as
> > +		 * grandparent.
> > +		 */
> > +		!(dev->is_thunderbolt && (parent = pci_upstream_bridge(dev)) &&
> > +				 (grandparent = pci_upstream_bridge(parent)) &&
> > +						!grandparent->is_thunderbolt)))
> 
> Can you move this to its own helper function?

I can certainly do that.

Could one of you guys confirm that the code above is safe on non-Macs?

Specifically, the very first Thunderbolt chips (Light Ridge, Eagle Ridge)
had no POC, i.e. they were unable to power themselves off.  Apple put an
ARM Cortex (NXP LPC1112) on the logic board which snoops on the connector
lines for hotplug detection while the Thunderbolt controller is powered
down.  The power rails to the controller are brought up and down with
separate load switches.  This functionality became integrated into the
controller starting with Cactus Ridge in 2012.

So I know the above code is safe on Macs.  However on non-Macs these
extra chips for power management may not exist, i.e. the controller
stays on all the time and then I shouldn't suspend the upstream bridge
to D3.  I assume that such machines do not exist as Apple was pretty
much the only vendor with Thunderbolt gear in the 2010-2012 time frame.
The only other one I know of was the Sony Vaio Z21 which used the
optical version of Thunderbolt to attach a docking station, but these
are rare.

If you know of non-Macs which might be broken by the above code snippet,
I could dmi-quirk this to Macs plus constrain to CONFIG_THUNDERBOLT being
enabled.

Thanks,

Lukas

> 
> >  
> >  		*d3cold_ok = false;
> >  
> > -- 
> > 2.11.0

Re: [PATCH v4 3/8] PCI: Don't block runtime PM for Thunderbolt host hotplug ports

[Mika Westerberg]

On Thu, Jan 12, 2017 at 02:47:07AM +0100, Lukas Wunner wrote:
> On Wed, Jan 11, 2017 at 12:02:10PM +0200, Mika Westerberg wrote:
> > On Sun, Jan 08, 2017 at 09:41:45AM +0100, Lukas Wunner wrote:
> > > Hotplug ports generally block their parents from suspending to D3hot as
> > > otherwise their interrupts couldn't be delivered.
> > > 
> > > An exception are Thunderbolt host controllers:  They have a separate
> > > GPIO pin to side-band signal plug events even if the controller is
> > > powered down or its parent ports are suspended to D3.  They can be told
> > > apart from Thunderbolt controllers in attached devices by checking if
> > > they're situated below a non-Thunderbolt device (typically a root port,
> > > or the downstream port of a PCIe switch in the case of the MacPro6,1).
> > > 
> > > To enable runtime PM for Thunderbolt on the Mac, the downstream bridges
> > > of a host controller must not block runtime PM on the upstream bridge as
> > > power to the chip is only cut once the upstream bridge has suspended.
> > > Amend the condition in pci_dev_check_d3cold() accordingly.
> > > 
> > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > Cc: Andreas Noever <andreas.noever@gmail.com>
> > > Cc: Tomas Winkler <tomas.winkler@intel.com>
> > > Cc: Amir Levy <amir.jer.levy@intel.com>
> > > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > > ---
> > >  drivers/pci/pci.c | 13 ++++++++++++-
> > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > index 8ed098d..0b03fe7 100644
> > > --- a/drivers/pci/pci.c
> > > +++ b/drivers/pci/pci.c
> > > @@ -2271,6 +2271,7 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
> > >  
> > >  static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
> > >  {
> > > +	struct pci_dev *parent, *grandparent;
> > >  	bool *d3cold_ok = data;
> > >  
> > >  	if (/* The device needs to be allowed to go D3cold ... */
> > > @@ -2284,7 +2285,17 @@ static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
> > >  	    !pci_power_manageable(dev) ||
> > >  
> > >  	    /* Hotplug interrupts cannot be delivered if the link is down. */
> > > -	    dev->is_hotplug_bridge)
> > > +	    (dev->is_hotplug_bridge &&
> > > +
> > > +		/*
> > > +		 * Exception:  Thunderbolt host controllers have a pin to
> > > +		 * side-band signal plug events.  Their hotplug ports are
> > > +		 * recognizable by having a non-Thunderbolt device as
> > > +		 * grandparent.
> > > +		 */
> > > +		!(dev->is_thunderbolt && (parent = pci_upstream_bridge(dev)) &&
> > > +				 (grandparent = pci_upstream_bridge(parent)) &&
> > > +						!grandparent->is_thunderbolt)))
> > 
> > Can you move this to its own helper function?
> 
> I can certainly do that.
> 
> Could one of you guys confirm that the code above is safe on non-Macs?
> 
> Specifically, the very first Thunderbolt chips (Light Ridge, Eagle Ridge)
> had no POC, i.e. they were unable to power themselves off.  Apple put an
> ARM Cortex (NXP LPC1112) on the logic board which snoops on the connector
> lines for hotplug detection while the Thunderbolt controller is powered
> down.  The power rails to the controller are brought up and down with
> separate load switches.  This functionality became integrated into the
> controller starting with Cactus Ridge in 2012.
> 
> So I know the above code is safe on Macs.  However on non-Macs these
> extra chips for power management may not exist, i.e. the controller
> stays on all the time and then I shouldn't suspend the upstream bridge
> to D3.  I assume that such machines do not exist as Apple was pretty
> much the only vendor with Thunderbolt gear in the 2010-2012 time frame.
> The only other one I know of was the Sony Vaio Z21 which used the
> optical version of Thunderbolt to attach a docking station, but these
> are rare.
> 
> If you know of non-Macs which might be broken by the above code snippet,
> I could dmi-quirk this to Macs plus constrain to CONFIG_THUNDERBOLT being
> enabled.

The thunderbolt chips I have seen all include the side-band hotplug
detection GPIO. In addition the whole PCIe hierarchy is powered down
when there is nothing connected. So in that sense, I don't see how this
could break them.

Constraining this to CONFIG_THUNDERBOLT does not limit anything because
distros will have it enabled anyway ;-) Having DMI quirk might be good
idea, just in case.

Re: [PATCH v4 3/8] PCI: Don't block runtime PM for Thunderbolt host hotplug ports

[Lukas Wunner]

On Thu, Jan 12, 2017 at 11:02:59AM +0200, Mika Westerberg wrote:
> On Thu, Jan 12, 2017 at 02:47:07AM +0100, Lukas Wunner wrote:
> > On Wed, Jan 11, 2017 at 12:02:10PM +0200, Mika Westerberg wrote:
> > > On Sun, Jan 08, 2017 at 09:41:45AM +0100, Lukas Wunner wrote:
> > > > Hotplug ports generally block their parents from suspending to D3hot as
> > > > otherwise their interrupts couldn't be delivered.
> > > > 
> > > > An exception are Thunderbolt host controllers:  They have a separate
> > > > GPIO pin to side-band signal plug events even if the controller is
> > > > powered down or its parent ports are suspended to D3.  They can be told
> > > > apart from Thunderbolt controllers in attached devices by checking if
> > > > they're situated below a non-Thunderbolt device (typically a root port,
> > > > or the downstream port of a PCIe switch in the case of the MacPro6,1).
> > > > 
> > > > To enable runtime PM for Thunderbolt on the Mac, the downstream bridges
> > > > of a host controller must not block runtime PM on the upstream bridge as
> > > > power to the chip is only cut once the upstream bridge has suspended.
> > > > Amend the condition in pci_dev_check_d3cold() accordingly.
> > > > 
> > > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > > > Cc: Andreas Noever <andreas.noever@gmail.com>
> > > > Cc: Tomas Winkler <tomas.winkler@intel.com>
> > > > Cc: Amir Levy <amir.jer.levy@intel.com>
> > > > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > > > ---
> > > >  drivers/pci/pci.c | 13 ++++++++++++-
> > > >  1 file changed, 12 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > > > index 8ed098d..0b03fe7 100644
> > > > --- a/drivers/pci/pci.c
> > > > +++ b/drivers/pci/pci.c
> > > > @@ -2271,6 +2271,7 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
> > > >  
> > > >  static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
> > > >  {
> > > > +	struct pci_dev *parent, *grandparent;
> > > >  	bool *d3cold_ok = data;
> > > >  
> > > >  	if (/* The device needs to be allowed to go D3cold ... */
> > > > @@ -2284,7 +2285,17 @@ static int pci_dev_check_d3cold(struct pci_dev *dev, void *data)
> > > >  	    !pci_power_manageable(dev) ||
> > > >  
> > > >  	    /* Hotplug interrupts cannot be delivered if the link is down. */
> > > > -	    dev->is_hotplug_bridge)
> > > > +	    (dev->is_hotplug_bridge &&
> > > > +
> > > > +		/*
> > > > +		 * Exception:  Thunderbolt host controllers have a pin to
> > > > +		 * side-band signal plug events.  Their hotplug ports are
> > > > +		 * recognizable by having a non-Thunderbolt device as
> > > > +		 * grandparent.
> > > > +		 */
> > > > +		!(dev->is_thunderbolt && (parent = pci_upstream_bridge(dev)) &&
> > > > +				 (grandparent = pci_upstream_bridge(parent)) &&
> > > > +						!grandparent->is_thunderbolt)))
> > > 
> > > Can you move this to its own helper function?
> > 
> > I can certainly do that.
> > 
> > Could one of you guys confirm that the code above is safe on non-Macs?
> > 
> > Specifically, the very first Thunderbolt chips (Light Ridge, Eagle Ridge)
> > had no POC, i.e. they were unable to power themselves off.  Apple put an
> > ARM Cortex (NXP LPC1112) on the logic board which snoops on the connector
> > lines for hotplug detection while the Thunderbolt controller is powered
> > down.  The power rails to the controller are brought up and down with
> > separate load switches.  This functionality became integrated into the
> > controller starting with Cactus Ridge in 2012.
> > 
> > So I know the above code is safe on Macs.  However on non-Macs these
> > extra chips for power management may not exist, i.e. the controller
> > stays on all the time and then I shouldn't suspend the upstream bridge
> > to D3.  I assume that such machines do not exist as Apple was pretty
> > much the only vendor with Thunderbolt gear in the 2010-2012 time frame.
> > The only other one I know of was the Sony Vaio Z21 which used the
> > optical version of Thunderbolt to attach a docking station, but these
> > are rare.
> > 
> > If you know of non-Macs which might be broken by the above code snippet,
> > I could dmi-quirk this to Macs plus constrain to CONFIG_THUNDERBOLT being
> > enabled.
> 
> The thunderbolt chips I have seen all include the side-band hotplug
> detection GPIO. In addition the whole PCIe hierarchy is powered down
> when there is nothing connected. So in that sense, I don't see how this
> could break them.

The pin to side-band signal a plug event is not present on Light Ridge
(and presumably Light Peak), only on Eagle Ridge and newer (2011+).

The pins for power control (Force Power, Go2Sx, Ok2Go2Sx) are not present
on Light Ridge and Eagle Ridge, only on Cactus Ridge and newer (2012+).

In other words, Thunderbolt controllers had no power control built in
before Cactus Ridge.  If you've only seen chips which have the plug event
pin then you've only seen newer chips. ;-)


> Constraining this to CONFIG_THUNDERBOLT does not limit anything because
> distros will have it enabled anyway ;-) Having DMI quirk might be good
> idea, just in case.

Actually, never mind.  It occurred to me that we only allow hotplug ports
to suspend in pci_bridge_d3_possible() if they're handled natively by the
OS, which is only the case on Macs.  On non-Macs the hotplug ports block
their parent ports from suspending by virtue of staying in D0 all the time.
Thus it's meaningless whether or not they block their parent ports from
suspending in pci_dev_check_d3cold().

So the patch is safe, I just forgot all this context because I wrote it
months ago.  I've now amended the commit message with this background
information.

Thanks,

Lukas

[PATCH v4 5/8] PM: Make requirements of dev_pm_domain_set() more precise

[Lukas Wunner]

Since commit 989561de9b51 ("PM / Domains: add setter for dev.pm_domain")
a PM domain may only be assigned to unbound devices.

The motivation was not made explicit in the changelog other than "in the
general case that can cause problems and also [...] we can simplify code
quite a bit if we can always assume that".  Rafael J. Wysocki elaborated
in a mailing list conversation that "setting a PM domain generally
changes the set of PM callbacks for the device and it may not be safe to
call it after the driver has been bound".

The concern seems to be that if a device is put to sleep and its PM
callbacks are changed, the device may end up in an undefined state or
not resume at all.  The real underlying requirement is thus to ensure
that the device is awake and execution of its PM callbacks is prevented
while the PM domain is assigned.  Unbound devices happen to fulfill this
requirement, but bound devices can be made to satisfy it as well:
The caller can prevent execution of PM ops with lock_system_sleep() and
by holding a runtime PM reference to the device.

Accordingly, adjust dev_pm_domain_set() to WARN only if the device is in
the midst of a system sleep transition, or runtime PM is enabled and the
device is either not active or may become inactive imminently (because
it has no active children or its refcount is zero).

The change is required to support runtime PM for Thunderbolt on the Mac,
which poses the unique issue that a child device (the NHI) needs to
assign a PM domain to its grandparent (the upstream bridge).  Because
the grandparent's driver is built-in and the child's driver is a module,
the grandparent is usually already bound when the child probes,
resulting in a WARN splat when calling dev_pm_domain_set().  However the
PM core guarantees both that the grandparent is active and that system
sleep is not commenced until the child has finished probing.  So in this
case it is safe to call dev_pm_domain_set() from the child's ->probe
hook and the WARN splat is entirely gratuitous.

Note that commit e79aee49bcf9 ("PM: Avoid false-positive warnings in
dev_pm_domain_set()") modified the WARN to not apply if a PM domain is
removed.  This is unsafe as it allows removal of the PM domain while
the device is asleep.  The present commit rectifies this.

Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/base/power/common.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
index f6a9ad5..d02c1e0 100644
--- a/drivers/base/power/common.c
+++ b/drivers/base/power/common.c
@@ -13,6 +13,7 @@
 #include <linux/pm_clock.h>
 #include <linux/acpi.h>
 #include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
 
 #include "power.h"
 
@@ -136,8 +137,10 @@ void dev_pm_domain_detach(struct device *dev, bool power_off)
  * @dev: Device whose PM domain is to be set.
  * @pd: PM domain to be set, or NULL.
  *
- * Sets the PM domain the device belongs to. The PM domain of a device needs
- * to be set before its probe finishes (it's bound to a driver).
+ * Sets the PM domain the device belongs to.  The PM domain of a device needs
+ * to be set while the device is awake.  This is guaranteed during ->probe.
+ * Otherwise the caller is responsible for ensuring wakefulness, e.g. by
+ * holding a runtime PM reference as well as invoking lock_system_sleep().
  *
  * This function must be called with the device lock held.
  */
@@ -146,8 +149,12 @@ void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd)
 	if (dev->pm_domain == pd)
 		return;
 
-	WARN(pd && device_is_bound(dev),
-	     "PM domains can only be changed for unbound devices\n");
+	WARN(dev->power.is_prepared || dev->power.is_suspended ||
+	     (pm_runtime_enabled(dev) &&
+	      (dev->power.runtime_status != RPM_ACTIVE ||
+	       (pm_children_suspended(dev) &&
+		!atomic_read(&dev->power.usage_count)))),
+	     "PM domains can only be changed for awake devices\n");
 	dev->pm_domain = pd;
 	device_pm_check_callbacks(dev);
 }
-- 
2.11.0

[PATCH v4 1/8] PCI: Recognize Thunderbolt devices

[Lukas Wunner]

We're about to allow runtime PM on Thunderbolt ports in
pci_bridge_d3_possible() and unblock runtime PM for Thunderbolt host
hotplug ports in pci_dev_check_d3cold().  In both cases we need to
uniquely identify if a PCI device belongs to a Thunderbolt controller.

We also have the need to detect presence of a Thunderbolt controller in
drivers/platform/x86/apple-gmux.c because dual GPU MacBook Pros cannot
switch external DP/HDMI ports between GPUs if they have Thunderbolt.

Furthermore, in multiple places in the DRM subsystem we need to detect
whether a GPU is on-board or attached with Thunderbolt.  As an example,
Thunderbolt-attached GPUs shall not be registered with vga_switcheroo.

Intel uses a Vendor-Specific Extended Capability (VSEC) with ID 0x1234
on devices belonging to a Thunderbolt controller which allows us to
recognize them.

Detect presence of this VSEC on device probe and cache it in a newly
added is_thunderbolt bit in struct pci_dev which can then be queried by
pci_bridge_d3_possible(), pci_dev_check_d3cold(), apple-gmux and others.

Cc: Andreas Noever <andreas.noever@gmail.com>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Cc: Amir Levy <amir.jer.levy@intel.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/pci/pci.h   |  2 ++
 drivers/pci/probe.c | 34 ++++++++++++++++++++++++++++++++++
 include/linux/pci.h |  1 +
 3 files changed, 37 insertions(+)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index cb17db2..45c2b81 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -3,6 +3,8 @@
 
 #define PCI_FIND_CAP_TTL	48
 
+#define PCI_VSEC_ID_INTEL_TBT	0x1234	/* Thunderbolt */
+
 extern const unsigned char pcie_link_speed[];
 
 bool pcie_cap_has_lnkctl(const struct pci_dev *dev);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e164b5c..891a8fa 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1206,6 +1206,37 @@ void set_pcie_hotplug_bridge(struct pci_dev *pdev)
 		pdev->is_hotplug_bridge = 1;
 }
 
+static void set_pcie_vendor_specific(struct pci_dev *dev)
+{
+	int vsec = 0;
+	u32 header;
+
+	while ((vsec = pci_find_next_ext_capability(dev, vsec,
+						    PCI_EXT_CAP_ID_VNDR))) {
+		pci_read_config_dword(dev, vsec + PCI_VNDR_HEADER, &header);
+
+		/* Is the device part of a Thunderbolt controller? */
+		if (dev->vendor == PCI_VENDOR_ID_INTEL &&
+		    PCI_VNDR_HEADER_ID(header) == PCI_VSEC_ID_INTEL_TBT)
+			dev->is_thunderbolt = 1;
+	}
+
+	/*
+	 * Is the device attached with Thunderbolt?  Walk upwards and check for
+	 * each encountered bridge if it's part of a Thunderbolt controller.
+	 * Reaching the host bridge means dev is soldered to the mainboard.
+	 */
+	if (!dev->is_thunderbolt) {
+		struct pci_dev *parent = dev;
+
+		while ((parent = pci_upstream_bridge(parent)))
+			if (parent->is_thunderbolt) {
+				dev->is_thunderbolt = 1;
+				break;
+			}
+	}
+}
+
 /**
  * pci_ext_cfg_is_aliased - is ext config space just an alias of std config?
  * @dev: PCI device
@@ -1358,6 +1389,9 @@ int pci_setup_device(struct pci_dev *dev)
 	/* need to have dev->class ready */
 	dev->cfg_size = pci_cfg_space_size(dev);
 
+	/* need to have dev->cfg_size ready */
+	set_pcie_vendor_specific(dev);
+
 	/* "Unknown power state" */
 	dev->current_state = PCI_UNKNOWN;
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e2d1a12..3c775e8 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -358,6 +358,7 @@ struct pci_dev {
 	unsigned int	is_virtfn:1;
 	unsigned int	reset_fn:1;
 	unsigned int    is_hotplug_bridge:1;
+	unsigned int	is_thunderbolt:1; /* part of Thunderbolt daisy chain */
 	unsigned int    __aer_firmware_first_valid:1;
 	unsigned int	__aer_firmware_first:1;
 	unsigned int	broken_intx_masking:1;
-- 
2.11.0

RE: [PATCH v4 1/8] PCI: Recognize Thunderbolt devices

[Winkler, Tomas]

> 
> We're about to allow runtime PM on Thunderbolt ports in
> pci_bridge_d3_possible() and unblock runtime PM for Thunderbolt host
> hotplug ports in pci_dev_check_d3cold().  In both cases we need to uniquely
> identify if a PCI device belongs to a Thunderbolt controller.
> 
> We also have the need to detect presence of a Thunderbolt controller in
> drivers/platform/x86/apple-gmux.c because dual GPU MacBook Pros cannot
> switch external DP/HDMI ports between GPUs if they have Thunderbolt.
> 
> Furthermore, in multiple places in the DRM subsystem we need to detect
> whether a GPU is on-board or attached with Thunderbolt.  As an example,
> Thunderbolt-attached GPUs shall not be registered with vga_switcheroo.
> 
> Intel uses a Vendor-Specific Extended Capability (VSEC) with ID 0x1234 on
> devices belonging to a Thunderbolt controller which allows us to recognize
> them.
> 
> Detect presence of this VSEC on device probe and cache it in a newly added
> is_thunderbolt bit in struct pci_dev which can then be queried by
> pci_bridge_d3_possible(), pci_dev_check_d3cold(), apple-gmux and others.
> 
> Cc: Andreas Noever <andreas.noever@gmail.com>
> Cc: Tomas Winkler <tomas.winkler@intel.com>
> Cc: Amir Levy <amir.jer.levy@intel.com>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  drivers/pci/pci.h   |  2 ++
>  drivers/pci/probe.c | 34 ++++++++++++++++++++++++++++++++++
>  include/linux/pci.h |  1 +
>  3 files changed, 37 insertions(+)
> 
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index cb17db2..45c2b81
> 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -3,6 +3,8 @@
> 
>  #define PCI_FIND_CAP_TTL	48
> 
> +#define PCI_VSEC_ID_INTEL_TBT	0x1234	/* Thunderbolt */


Shouldn't bet this defined in pci_ids.h ? 

>  extern const unsigned char pcie_link_speed[];
> 
>  bool pcie_cap_has_lnkctl(const struct pci_dev *dev); diff --git
> a/drivers/pci/probe.c b/drivers/pci/probe.c index e164b5c..891a8fa 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1206,6 +1206,37 @@ void set_pcie_hotplug_bridge(struct pci_dev
> *pdev)
>  		pdev->is_hotplug_bridge = 1;
>  }
> 
> +static void set_pcie_vendor_specific(struct pci_dev *dev) {


Why don't u implement this  in quirk.c ?

> +	int vsec = 0;
> +	u32 header;
> +
> +	while ((vsec = pci_find_next_ext_capability(dev, vsec,
> +						    PCI_EXT_CAP_ID_VNDR))) {
> +		pci_read_config_dword(dev, vsec + PCI_VNDR_HEADER,
> &header);
> +
> +		/* Is the device part of a Thunderbolt controller? */
> +		if (dev->vendor == PCI_VENDOR_ID_INTEL &&
> +		    PCI_VNDR_HEADER_ID(header) == PCI_VSEC_ID_INTEL_TBT)
> +			dev->is_thunderbolt = 1;
> +	}
> +
> +	/*
> +	 * Is the device attached with Thunderbolt?  Walk upwards and check
> for
> +	 * each encountered bridge if it's part of a Thunderbolt controller.
> +	 * Reaching the host bridge means dev is soldered to the mainboard.
> +	 */
> +	if (!dev->is_thunderbolt) {
> +		struct pci_dev *parent = dev;
> +
> +		while ((parent = pci_upstream_bridge(parent)))
> +			if (parent->is_thunderbolt) {
> +				dev->is_thunderbolt = 1;
> +				break;
> +			}
> +	}
> +}
> +
>  /**
>   * pci_ext_cfg_is_aliased - is ext config space just an alias of std config?
>   * @dev: PCI device
> @@ -1358,6 +1389,9 @@ int pci_setup_device(struct pci_dev *dev)
>  	/* need to have dev->class ready */
>  	dev->cfg_size = pci_cfg_space_size(dev);
> 
> +	/* need to have dev->cfg_size ready */
> +	set_pcie_vendor_specific(dev);
> +
>  	/* "Unknown power state" */
>  	dev->current_state = PCI_UNKNOWN;
> 
> diff --git a/include/linux/pci.h b/include/linux/pci.h index e2d1a12..3c775e8
> 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -358,6 +358,7 @@ struct pci_dev {
>  	unsigned int	is_virtfn:1;
>  	unsigned int	reset_fn:1;
>  	unsigned int    is_hotplug_bridge:1;
> +	unsigned int	is_thunderbolt:1; /* part of Thunderbolt daisy chain
> */
>  	unsigned int    __aer_firmware_first_valid:1;
>  	unsigned int	__aer_firmware_first:1;
>  	unsigned int	broken_intx_masking:1;
> --
> 2.11.0

Re: [PATCH v4 1/8] PCI: Recognize Thunderbolt devices

[Lukas Wunner]

On Sun, Jan 08, 2017 at 10:23:03AM +0000, Winkler, Tomas wrote:
> > We're about to allow runtime PM on Thunderbolt ports in
> > pci_bridge_d3_possible() and unblock runtime PM for Thunderbolt host
> > hotplug ports in pci_dev_check_d3cold().  In both cases we need to uniquely
> > identify if a PCI device belongs to a Thunderbolt controller.
> > 
> > We also have the need to detect presence of a Thunderbolt controller in
> > drivers/platform/x86/apple-gmux.c because dual GPU MacBook Pros cannot
> > switch external DP/HDMI ports between GPUs if they have Thunderbolt.
> > 
> > Furthermore, in multiple places in the DRM subsystem we need to detect
> > whether a GPU is on-board or attached with Thunderbolt.  As an example,
> > Thunderbolt-attached GPUs shall not be registered with vga_switcheroo.
> > 
> > Intel uses a Vendor-Specific Extended Capability (VSEC) with ID 0x1234 on
> > devices belonging to a Thunderbolt controller which allows us to recognize
> > them.
> > 
> > Detect presence of this VSEC on device probe and cache it in a newly added
> > is_thunderbolt bit in struct pci_dev which can then be queried by
> > pci_bridge_d3_possible(), pci_dev_check_d3cold(), apple-gmux and others.
> > 
> > Cc: Andreas Noever <andreas.noever@gmail.com>
> > Cc: Tomas Winkler <tomas.winkler@intel.com>
> > Cc: Amir Levy <amir.jer.levy@intel.com>
> > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > ---
> >  drivers/pci/pci.h   |  2 ++
> >  drivers/pci/probe.c | 34 ++++++++++++++++++++++++++++++++++
> >  include/linux/pci.h |  1 +
> >  3 files changed, 37 insertions(+)
> > 
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index cb17db2..45c2b81
> > 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -3,6 +3,8 @@
> > 
> >  #define PCI_FIND_CAP_TTL	48
> > 
> > +#define PCI_VSEC_ID_INTEL_TBT	0x1234	/* Thunderbolt */
> 
> Shouldn't bet this defined in pci_ids.h ?

That file is solely intended for IDs used in multiple places, as
explained in the comment at its top.  This particular ID is only used
in the PCI core, hence it's in the PCI core's private drivers/pci/pci.h.

However the line is formatted such that it can just be moved to the
global include/linux/pci_ids.h should it be needed someplace else in
the future.


> >  extern const unsigned char pcie_link_speed[];
> > 
> >  bool pcie_cap_has_lnkctl(const struct pci_dev *dev); diff --git
> > a/drivers/pci/probe.c b/drivers/pci/probe.c index e164b5c..891a8fa 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1206,6 +1206,37 @@ void set_pcie_hotplug_bridge(struct pci_dev
> > *pdev)
> >  		pdev->is_hotplug_bridge = 1;
> >  }
> > 
> > +static void set_pcie_vendor_specific(struct pci_dev *dev) {
> 
> 
> Why don't u implement this  in quirk.c ?

The is_thunderbolt bit signifies whether a given PCI device is part
of a Thunderbolt daisy chain.  (As explained in the comment in
include/linux/pci.h, see below.)  So it is set on all the PCI devices
that comprise a Thunderbolt controller, but also on all endpoint devices
that are attached via Thunderbolt.

Consequently this function needs to be executed for all PCI devices,
not just for a subset that could be identified by a vendor, device or
class ID.

In the DRM drivers nouveau, radeon and amdgpu I need to prevent calls
to vga_switcheroo_register_client() and vga_switcheroo_init_domain_pm_ops()
if the device in question is attached via Thunderbolt.  That's because
external GPUs can't drive the panel of a laptop or be powered down like an
on-board Optimus/PowerXpress GPU.  We've got a bug there right now that
manifests itself once someone attaches an eGPU to a dual GPU laptop.
With this patch I'll be able to just skip registration with vga_switcheroo
if is_thunderbolt is set on a GPU's pci_dev, thereby fixing the bug.


BTW do you have information on the contents/meaning of the VSEC that you
would be able/allowed to share?  What I've seen so far is that its length
is 0x1c bytes on older controllers (e.g. Light Ridge, Port Ridge) and its
contents are always the same, regardless if the controller is used in
host mode or endpoint mode:

500: 0b 00 01 00 34 12 c1 01|50 09 00 00 39 00 00 00
510: 00 00 00 00 00 00 00 00 00 00 00 00

However on Alpine Ridge the size of the VSEC has increased significantly
to 0xd8 bytes, even though the version stayed at 1 as before:

500: 0b 00 01 60 34 12 81 0d 50 09 b0 0c b9 06 18 08
510: 00 38 00 01 00 00 00 00 00 00 00 00 32 02 10 00
520: ef d3 00 40 00 00 00 00 f0 f0 30 c1 00 02 30 00
530: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
540: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
550: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
560: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
570: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
590: 00 00 00 08 00 00 00 00 00 00 00 00 00 00 00 00
5a0: 00 00 00 00 00 00 00 00 00 00 00 00 38 24 01 00
5b0: 08 40 00 1c 00 00 01 18 04 03 00 f0 06 00 00 00
5c0: 00 00 08 40 06 00 00 01 90 00 08 1b 00 08 80 08
5d0: 80 7f 08 00 00 00 00 00

Thanks,

Lukas

> 
> > +	int vsec = 0;
> > +	u32 header;
> > +
> > +	while ((vsec = pci_find_next_ext_capability(dev, vsec,
> > +						    PCI_EXT_CAP_ID_VNDR))) {
> > +		pci_read_config_dword(dev, vsec + PCI_VNDR_HEADER,
> > &header);
> > +
> > +		/* Is the device part of a Thunderbolt controller? */
> > +		if (dev->vendor == PCI_VENDOR_ID_INTEL &&
> > +		    PCI_VNDR_HEADER_ID(header) == PCI_VSEC_ID_INTEL_TBT)
> > +			dev->is_thunderbolt = 1;
> > +	}
> > +
> > +	/*
> > +	 * Is the device attached with Thunderbolt?  Walk upwards and check
> > for
> > +	 * each encountered bridge if it's part of a Thunderbolt controller.
> > +	 * Reaching the host bridge means dev is soldered to the mainboard.
> > +	 */
> > +	if (!dev->is_thunderbolt) {
> > +		struct pci_dev *parent = dev;
> > +
> > +		while ((parent = pci_upstream_bridge(parent)))
> > +			if (parent->is_thunderbolt) {
> > +				dev->is_thunderbolt = 1;
> > +				break;
> > +			}
> > +	}
> > +}
> > +
> >  /**
> >   * pci_ext_cfg_is_aliased - is ext config space just an alias of std config?
> >   * @dev: PCI device
> > @@ -1358,6 +1389,9 @@ int pci_setup_device(struct pci_dev *dev)
> >  	/* need to have dev->class ready */
> >  	dev->cfg_size = pci_cfg_space_size(dev);
> > 
> > +	/* need to have dev->cfg_size ready */
> > +	set_pcie_vendor_specific(dev);
> > +
> >  	/* "Unknown power state" */
> >  	dev->current_state = PCI_UNKNOWN;
> > 
> > diff --git a/include/linux/pci.h b/include/linux/pci.h index e2d1a12..3c775e8
> > 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -358,6 +358,7 @@ struct pci_dev {
> >  	unsigned int	is_virtfn:1;
> >  	unsigned int	reset_fn:1;
> >  	unsigned int    is_hotplug_bridge:1;
> > +	unsigned int	is_thunderbolt:1; /* part of Thunderbolt daisy chain */
> >  	unsigned int    __aer_firmware_first_valid:1;
> >  	unsigned int	__aer_firmware_first:1;
> >  	unsigned int	broken_intx_masking:1;
> > --
> > 2.11.0

RE: [PATCH v4 1/8] PCI: Recognize Thunderbolt devices

[Winkler, Tomas]

> 
> On Sun, Jan 08, 2017 at 10:23:03AM +0000, Winkler, Tomas wrote:
> > > We're about to allow runtime PM on Thunderbolt ports in
> > > pci_bridge_d3_possible() and unblock runtime PM for Thunderbolt host
> > > hotplug ports in pci_dev_check_d3cold().  In both cases we need to
> > > uniquely identify if a PCI device belongs to a Thunderbolt controller.
> > >
> > > We also have the need to detect presence of a Thunderbolt controller
> > > in drivers/platform/x86/apple-gmux.c because dual GPU MacBook Pros
> > > cannot switch external DP/HDMI ports between GPUs if they have
> Thunderbolt.
> > >
> > > Furthermore, in multiple places in the DRM subsystem we need to
> > > detect whether a GPU is on-board or attached with Thunderbolt.  As
> > > an example, Thunderbolt-attached GPUs shall not be registered with
> vga_switcheroo.
> > >
> > > Intel uses a Vendor-Specific Extended Capability (VSEC) with ID
> > > 0x1234 on devices belonging to a Thunderbolt controller which allows
> > > us to recognize them.
> > >
> > > Detect presence of this VSEC on device probe and cache it in a newly
> > > added is_thunderbolt bit in struct pci_dev which can then be queried
> > > by pci_bridge_d3_possible(), pci_dev_check_d3cold(), apple-gmux and
> others.
> > >
> > > Cc: Andreas Noever <andreas.noever@gmail.com>
> > > Cc: Tomas Winkler <tomas.winkler@intel.com>
> > > Cc: Amir Levy <amir.jer.levy@intel.com>
> > > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > > ---
> > >  drivers/pci/pci.h   |  2 ++
> > >  drivers/pci/probe.c | 34 ++++++++++++++++++++++++++++++++++
> > >  include/linux/pci.h |  1 +
> > >  3 files changed, 37 insertions(+)
> > >
> > > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index
> > > cb17db2..45c2b81
> > > 100644
> > > --- a/drivers/pci/pci.h
> > > +++ b/drivers/pci/pci.h
> > > @@ -3,6 +3,8 @@
> > >
> > >  #define PCI_FIND_CAP_TTL	48
> > >
> > > +#define PCI_VSEC_ID_INTEL_TBT	0x1234	/* Thunderbolt */
> >
> > Shouldn't bet this defined in pci_ids.h ?
> 
> That file is solely intended for IDs used in multiple places, as explained in the
> comment at its top.  This particular ID is only used in the PCI core, hence it's in
> the PCI core's private drivers/pci/pci.h.
> 
> However the line is formatted such that it can just be moved to the global
> include/linux/pci_ids.h should it be needed someplace else in the future.
> 
> 
> > >  extern const unsigned char pcie_link_speed[];
> > >
> > >  bool pcie_cap_has_lnkctl(const struct pci_dev *dev); diff --git
> > > a/drivers/pci/probe.c b/drivers/pci/probe.c index e164b5c..891a8fa
> > > 100644
> > > --- a/drivers/pci/probe.c
> > > +++ b/drivers/pci/probe.c
> > > @@ -1206,6 +1206,37 @@ void set_pcie_hotplug_bridge(struct pci_dev
> > > *pdev)
> > >  		pdev->is_hotplug_bridge = 1;
> > >  }
> > >
> > > +static void set_pcie_vendor_specific(struct pci_dev *dev) {
> >
> >
> > Why don't u implement this  in quirk.c ?
> 
> The is_thunderbolt bit signifies whether a given PCI device is part of a
> Thunderbolt daisy chain.  (As explained in the comment in include/linux/pci.h,
> see below.)  So it is set on all the PCI devices that comprise a Thunderbolt
> controller, but also on all endpoint devices that are attached via Thunderbolt.
> 
> Consequently this function needs to be executed for all PCI devices, not just
> for a subset that could be identified by a vendor, device or class ID.

Ok, god it. 

> 
> In the DRM drivers nouveau, radeon and amdgpu I need to prevent calls to
> vga_switcheroo_register_client() and vga_switcheroo_init_domain_pm_ops()
> if the device in question is attached via Thunderbolt.  That's because external
> GPUs can't drive the panel of a laptop or be powered down like an on-board
> Optimus/PowerXpress GPU.  We've got a bug there right now that manifests
> itself once someone attaches an eGPU to a dual GPU laptop.
> With this patch I'll be able to just skip registration with vga_switcheroo if
> is_thunderbolt is set on a GPU's pci_dev, thereby fixing the bug.
> 
> 
> BTW do you have information on the contents/meaning of the VSEC that you
> would be able/allowed to share?  What I've seen so far is that its length is 0x1c
> bytes on older controllers (e.g. Light Ridge, Port Ridge) and its contents are
> always the same, regardless if the controller is used in host mode or endpoint
> mode:

Sorry, currently nothing I can share.
 
> 500: 0b 00 01 00 34 12 c1 01|50 09 00 00 39 00 00 00
> 510: 00 00 00 00 00 00 00 00 00 00 00 00
> 
> However on Alpine Ridge the size of the VSEC has increased significantly to
> 0xd8 bytes, even though the version stayed at 1 as before:
> 
> 500: 0b 00 01 60 34 12 81 0d 50 09 b0 0c b9 06 18 08
> 510: 00 38 00 01 00 00 00 00 00 00 00 00 32 02 10 00
> 520: ef d3 00 40 00 00 00 00 f0 f0 30 c1 00 02 30 00
> 530: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 540: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 550: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 560: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 570: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 580: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 590: 00 00 00 08 00 00 00 00 00 00 00 00 00 00 00 00
> 5a0: 00 00 00 00 00 00 00 00 00 00 00 00 38 24 01 00
> 5b0: 08 40 00 1c 00 00 01 18 04 03 00 f0 06 00 00 00
> 5c0: 00 00 08 40 06 00 00 01 90 00 08 1b 00 08 80 08
> 5d0: 80 7f 08 00 00 00 00 00
> 
> Thanks,
> 
> Lukas
> 
> >
> > > +	int vsec = 0;
> > > +	u32 header;
> > > +
> > > +	while ((vsec = pci_find_next_ext_capability(dev, vsec,
> > > +						    PCI_EXT_CAP_ID_VNDR))) {
> > > +		pci_read_config_dword(dev, vsec + PCI_VNDR_HEADER,
> > > &header);
> > > +
> > > +		/* Is the device part of a Thunderbolt controller? */
> > > +		if (dev->vendor == PCI_VENDOR_ID_INTEL &&
> > > +		    PCI_VNDR_HEADER_ID(header) == PCI_VSEC_ID_INTEL_TBT)
> > > +			dev->is_thunderbolt = 1;
> > > +	}
> > > +
> > > +	/*
> > > +	 * Is the device attached with Thunderbolt?  Walk upwards and
> > > +check
> > > for
> > > +	 * each encountered bridge if it's part of a Thunderbolt controller.
> > > +	 * Reaching the host bridge means dev is soldered to the mainboard.
> > > +	 */
> > > +	if (!dev->is_thunderbolt) {
> > > +		struct pci_dev *parent = dev;
> > > +
> > > +		while ((parent = pci_upstream_bridge(parent)))
> > > +			if (parent->is_thunderbolt) {
> > > +				dev->is_thunderbolt = 1;
> > > +				break;
> > > +			}
> > > +	}
> > > +}
> > > +
> > >  /**
> > >   * pci_ext_cfg_is_aliased - is ext config space just an alias of std config?
> > >   * @dev: PCI device
> > > @@ -1358,6 +1389,9 @@ int pci_setup_device(struct pci_dev *dev)
> > >  	/* need to have dev->class ready */
> > >  	dev->cfg_size = pci_cfg_space_size(dev);
> > >
> > > +	/* need to have dev->cfg_size ready */
> > > +	set_pcie_vendor_specific(dev);
> > > +
> > >  	/* "Unknown power state" */
> > >  	dev->current_state = PCI_UNKNOWN;
> > >
> > > diff --git a/include/linux/pci.h b/include/linux/pci.h index
> > > e2d1a12..3c775e8
> > > 100644
> > > --- a/include/linux/pci.h
> > > +++ b/include/linux/pci.h
> > > @@ -358,6 +358,7 @@ struct pci_dev {
> > >  	unsigned int	is_virtfn:1;
> > >  	unsigned int	reset_fn:1;
> > >  	unsigned int    is_hotplug_bridge:1;
> > > +	unsigned int	is_thunderbolt:1; /* part of Thunderbolt daisy chain
> */
> > >  	unsigned int    __aer_firmware_first_valid:1;
> > >  	unsigned int	__aer_firmware_first:1;
> > >  	unsigned int	broken_intx_masking:1;
> > > --
> > > 2.11.0

[PATCH v4 6/8] PM / sleep: Define constant for direct_complete

[Lukas Wunner]

From: Chen Yu <yu.c.chen@intel.com>

The PM core introduced the ability to keep devices runtime suspended
during the entire system sleep process with commit aae4518b3124
("PM / sleep: Mechanism to avoid resuming runtime-suspended devices
unnecessarily").

Drivers opt in to this so-called "direct_complete" mechanism by
returning a positive value from their ->prepare hook.  Usually this is
achieved with a "return 1;" statement, which looks somewhat cryptic to
readers not intimately familiar with the PM core.

Improve clarity by defining a DPM_DIRECT_COMPLETE constant which drivers
may use instead.

Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
[LW: separate this hunk out of mfd patch, add commit message]
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 include/linux/pm.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/linux/pm.h b/include/linux/pm.h
index f926af4..2d651a3 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -788,4 +788,11 @@ enum dpm_order {
 	DPM_ORDER_DEV_LAST,
 };
 
+/*
+ * Return this from system suspend/hibernation ->prepare() callback to
+ * request the core to leave the device runtime-suspended during system
+ * suspend if possible.
+ */
+#define DPM_DIRECT_COMPLETE 1
+
 #endif /* _LINUX_PM_H */
-- 
2.11.0

[PATCH v4 7/8] thunderbolt: Power down controller when idle

[Lukas Wunner]

Document and implement Apple's ACPI-based (but nonstandard) pm mechanism
for Thunderbolt.  Briefly, an ACPI method provided by Apple is used to
cut power to the controller.  A GPE is enabled while the controller is
powered down which sideband-signals a plug event, whereupon we reinstate
power using the ACPI method.

This saves 1.7 W on machines with a Light Ridge controller and is
reported to save 4 W on Cactus Ridge 4C and Falcon Ridge 4C.  (I believe
4 W includes the bus power drawn by Apple's Gigabit Ethernet adapter.)
It fixes (at least partially) a power regression introduced in 3.17 by
commit 7bc5a2bad0b8 ("ACPI: Support _OSI("Darwin") correctly").

A Thunderbolt controller appears to the OS as a set of virtual devices:
One upstream bridge, multiple downstream bridges and one NHI (Native
Host Interface).  The upstream and downstream bridges represent a PCIe
switch (see definition of a switch in the PCIe spec).  The NHI device is
used to manage the switch fabric.  Hotplugged devices appear behind the
downstream bridges:

  (Root Port) ---- Upstream Bridge --+-- Downstream Bridge 0 ---- NHI
                                     +-- Downstream Bridge 1 --
                                     +-- Downstream Bridge 2 --
                                     ...

Power is cut to the entire set of devices.  The Linux pm model is
hierarchical and assumes that a child cannot resume before its parent.
To conform to this model, power control must be governed by the
Thunderbolt controller's topmost device, which is the upstream bridge.
The NHI and downstream bridges go to D3hot independently and the
upstream bridge goes to D3cold once all its children have suspended.
This commit only adds runtime pm for the upstream bridge.  Runtime pm
for the NHI is added in a separate commit to signify its independence.
Runtime pm for the downstream bridges is handled by the pcieport driver.

Because Apple's ACPI methods are nonstandard, a struct dev_pm_domain is
used to override the PCI bus pm_ops.  The thunderbolt driver binds to
the NHI, thus the dev_pm_domain is assigned to the upstream bridge when
its grandchild ->probes and evicted when it ->removes.

There are no Thunderbolt specs publicly available from Intel or Apple,
so I've included documentation to the extent that I was able to reverse-
engineer things.  Documentation on the Go2Sx and Ok2Go2Sx pins is
tentative as those are missing on my Light Ridge.  Apple only uses them
on Cactus Ridge 4C.  Someone with such a controller needs to find out
through experimentation if the documentation is accurate and amend it if
necessary.

To maximize power saving, the controller utilizes the PM core's direct-
complete procedure, i.e. it stays suspended during the system sleep
process.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=92111
Cc: Andreas Noever <andreas.noever@gmail.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/thunderbolt/Kconfig  |   3 +-
 drivers/thunderbolt/Makefile |   4 +-
 drivers/thunderbolt/nhi.c    |   3 +
 drivers/thunderbolt/power.c  | 346 +++++++++++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/power.h  |  37 +++++
 drivers/thunderbolt/tb.h     |   2 +
 6 files changed, 392 insertions(+), 3 deletions(-)
 create mode 100644 drivers/thunderbolt/power.c
 create mode 100644 drivers/thunderbolt/power.h

diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
index d35db16..41625cf 100644
--- a/drivers/thunderbolt/Kconfig
+++ b/drivers/thunderbolt/Kconfig
@@ -1,9 +1,10 @@
 menuconfig THUNDERBOLT
 	tristate "Thunderbolt support for Apple devices"
-	depends on PCI
+	depends on PCI && ACPI
 	depends on X86 || COMPILE_TEST
 	select APPLE_PROPERTIES if EFI_STUB && X86
 	select CRC32
+	select PM
 	help
 	  Cactus Ridge Thunderbolt Controller driver
 	  This driver is required if you want to hotplug Thunderbolt devices on
diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
index 5d1053c..b220825 100644
--- a/drivers/thunderbolt/Makefile
+++ b/drivers/thunderbolt/Makefile
@@ -1,3 +1,3 @@
 obj-${CONFIG_THUNDERBOLT} := thunderbolt.o
-thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel_pci.o eeprom.o
-
+thunderbolt-objs := nhi.o ctl.o tb.o switch.o cap.o path.o tunnel_pci.o \
+		    eeprom.o power.o
diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index a8c2041..88fb2fb 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -605,6 +605,8 @@ static int nhi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	}
 	pci_set_drvdata(pdev, tb);
 
+	thunderbolt_power_init(tb);
+
 	return 0;
 }
 
@@ -612,6 +614,7 @@ static void nhi_remove(struct pci_dev *pdev)
 {
 	struct tb *tb = pci_get_drvdata(pdev);
 	struct tb_nhi *nhi = tb->nhi;
+	thunderbolt_power_fini(tb);
 	thunderbolt_shutdown_and_free(tb);
 	nhi_shutdown(nhi);
 }
diff --git a/drivers/thunderbolt/power.c b/drivers/thunderbolt/power.c
new file mode 100644
index 0000000..6e7ef07
--- /dev/null
+++ b/drivers/thunderbolt/power.c
@@ -0,0 +1,346 @@
+/*
+ * power.c - power thunderbolt controller down when idle
+ * Copyright (C) 2016 Lukas Wunner <lukas@wunner.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License (version 2) as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * Apple provides the following means for power control in ACPI:
+ *
+ * * On Macs with Thunderbolt 1 Gen 1 controllers (Light Ridge, Eagle Ridge):
+ *   * XRPE method ("Power Enable"), takes argument 1 or 0, toggles a GPIO pin
+ *     to switch the controller on or off.
+ *   * XRIN named object (alternatively _GPE), contains number of a GPE which
+ *     fires as long as something is plugged in (regardless of power state).
+ *   * XRIL method ("Interrupt Low"), returns 0 as long as something is
+ *     plugged in, 1 otherwise.
+ *   * XRIP and XRIO methods, unused by macOS driver.
+ *
+ * * On Macs with Thunderbolt 1 Gen 2 controllers (Cactus Ridge 4C):
+ *   * XRIN not only fires as long as something is plugged in, but also as long
+ *     as the controller's CIO switch is powered up.
+ *   * XRIL method changed its meaning, it returns 0 as long as the CIO switch
+ *     is powered up, 1 otherwise.
+ *   * Additional SXFP method ("Force Power"), accepts only argument 0,
+ *     switches the controller off. This carries out just the raw power change,
+ *     unlike XRPE which disables the link on the PCIe Root Port in an orderly
+ *     fashion before switching off the controller.
+ *   * Additional SXLV, SXIO, SXIL methods to utilize the Go2Sx and Ok2Go2Sx
+ *     pins (see background below). Apparently SXLV toggles the value given to
+ *     the POC via Go2Sx (0 or 1), SXIO changes the direction (0 or 1) and SXIL
+ *     returns the value received from the POC via Ok2Go2Sx.
+ *   * On some Macs, additional XRST method, takes argument 1 or 0, asserts or
+ *     deasserts a GPIO pin to reset the controller.
+ *   * On Macs introduced 2013, XRPE was renamed TRPE.
+ *
+ * * On Macs with Thunderbolt 2 controllers (Falcon Ridge 4C and 2C):
+ *   * SXLV, SXIO, SXIL methods to utilize Go2Sx and Ok2Go2Sx are gone.
+ *   * On the MacPro6,1 which has multiple Thunderbolt controllers, each NHI
+ *     device has a separate XRIN GPE and separate TRPE, SXFP and XRIL methods.
+ *
+ * Background:
+ *
+ * * Gen 1 controllers (Light Ridge, Eagle Ridge) had no power management
+ *   and no ability to distinguish whether a DP or Thunderbolt device is
+ *   plugged in. Apple put an ARM Cortex MCU (NXP LPC1112A) on the logic board
+ *   which snoops on the connector lines and, depending on the type of device,
+ *   sends an HPD signal to the GPU or rings the Thunderbolt XRIN doorbell
+ *   interrupt. The switches for the 3.3V and 1.05V power rails to the
+ *   Thunderbolt controller are toggled by a GPIO pin on the southbridge.
+ *
+ * * On gen 2 controllers (Cactus Ridge 4C), Intel integrated the MCU into the
+ *   controller and called it POC. This caused a change of semantics for XRIN
+ *   and XRIL. The POC is powered by a separate 3.3V rail which is active even
+ *   in sleep state S4. It only draws a very small current. The regular 3.3V
+ *   and 1.05V power rails are no longer controlled by the southbridge but by
+ *   the POC. In other words the controller powers *itself* up and down! It is
+ *   instructed to do so with the Go2Sx pin. Another pin, Ok2Go2Sx, allows the
+ *   controller to indicate if it is ready to power itself down. Apple wires
+ *   Go2Sx and Ok2Go2Sx to the same GPIO pin on the southbridge, hence the pin
+ *   is used bidirectionally. A third pin, Force Power, is intended by Intel
+ *   for debug only but Apple abuses it for XRPE/TRPE and SXFP. Perhaps it
+ *   leads to larger power saving gains. They utilize Go2Sx and Ok2Go2Sx only
+ *   on Cactus Ridge, presumably because the controller somehow requires that.
+ *   On Falcon Ridge they forego these pins and rely solely on Force Power.
+ *
+ * Implementation Notes:
+ *
+ * * To conform to Linux' hierarchical power management model, power control
+ *   is governed by the topmost PCI device of the controller, which is the
+ *   upstream bridge. The controller is powered down once all child devices
+ *   of the upstream bridge have suspended and its autosuspend delay has
+ *   elapsed.
+ *
+ * * The autosuspend delay is user configurable via sysfs and should be lower
+ *   or equal to that of the NHI since hotplug events are not acted upon if
+ *   the NHI has suspended but the controller has not yet powered down.
+ *   However the delay should not be zero to avoid frequent power changes
+ *   (e.g. multiple times just for lspci -vv) since powering up takes 2 sec.
+ *   (Powering down is almost instantaneous.)
+ */
+
+#include <linux/delay.h>
+#include <linux/pci.h>
+#include <linux/pm_runtime.h>
+
+#include "power.h"
+
+#undef pr_fmt
+#define pr_fmt(fmt) KBUILD_MODNAME " %s: " fmt, dev_name(dev)
+
+#define to_power(dev) container_of(dev->pm_domain, struct tb_power, pm_domain)
+
+static int upstream_prepare(struct device *dev)
+{
+	struct tb_power *power = to_power(dev);
+
+	if (pm_runtime_active(dev))
+		return 0;
+
+	/* prevent interrupts during system sleep transition */
+	if (ACPI_FAILURE(acpi_disable_gpe(NULL, power->wake_gpe))) {
+		pr_err("cannot disable wake GPE, resuming\n");
+		pm_request_resume(dev);
+		return -EAGAIN;
+	}
+
+	return DPM_DIRECT_COMPLETE;
+}
+
+static void upstream_complete(struct device *dev)
+{
+	struct tb_power *power = to_power(dev);
+
+	if (pm_runtime_active(dev))
+		return;
+
+	/*
+	 * If the controller was powered down before system sleep, calling XRPE
+	 * to power it up will fail on the next runtime resume. An additional
+	 * call to XRPE is necessary to reset the power switch first.
+	 */
+	pr_info("resetting power switch\n");
+	if (ACPI_FAILURE(acpi_execute_simple_method(power->set, NULL, 0))) {
+		pr_err("cannot call power->set method\n");
+		dev->power.runtime_error = -EIO;
+	}
+
+	if (ACPI_FAILURE(acpi_enable_gpe(NULL, power->wake_gpe))) {
+		pr_err("cannot enable wake GPE, resuming\n");
+		pm_request_resume(dev);
+	}
+}
+
+static int set_d3cold(struct pci_dev *pdev, void *ptr)
+{
+	pdev->current_state = PCI_D3cold;
+	return 0;
+}
+
+static int request_resume(struct pci_dev *pdev, void *ptr)
+{
+	WARN_ON(pm_request_resume(&pdev->dev) < 0);
+	return 0;
+}
+
+static int upstream_runtime_suspend(struct device *dev)
+{
+	struct tb_power *power = to_power(dev);
+	struct pci_dev *pdev = to_pci_dev(dev);
+	unsigned long long powered_down;
+	int ret, i;
+
+	/* children are effectively in D3cold once upstream goes to D3hot */
+	pci_walk_bus(pdev->subordinate, set_d3cold, NULL);
+
+	ret = dev->bus->pm->runtime_suspend(dev);
+	if (ret) {
+		pci_walk_bus(pdev->subordinate, request_resume, NULL);
+		return ret;
+	}
+
+	pr_info("powering down\n");
+	pdev->current_state = PCI_D3cold;
+	if (ACPI_FAILURE(acpi_execute_simple_method(power->set, NULL, 0))) {
+		pr_err("cannot call power->set method, resuming\n");
+		goto err_resume;
+	}
+
+	/*
+	 * On gen 2 controllers, the wake GPE fires as long as the controller
+	 * is powered up. Poll until it's powered down before enabling the GPE.
+	 * macOS polls up to 300 times with a 1 ms delay, just mimic that.
+	 */
+	for (i = 0; i < 300; i++) {
+		if (ACPI_FAILURE(acpi_evaluate_integer(power->get,
+					      NULL, NULL, &powered_down))) {
+			pr_err("cannot call power->get method, resuming\n");
+			goto err_resume;
+		}
+		if (powered_down)
+			break;
+		usleep_range(800, 1200);
+	}
+	if (!powered_down) {
+		pr_err("refused to power down, resuming\n");
+		goto err_resume;
+	}
+
+	if (ACPI_FAILURE(acpi_enable_gpe(NULL, power->wake_gpe))) {
+		pr_err("cannot enable wake GPE, resuming\n");
+		goto err_resume;
+	}
+
+	return 0;
+
+err_resume:
+	acpi_execute_simple_method(power->set, NULL, 1);
+	dev->bus->pm->runtime_resume(dev);
+	pci_walk_bus(pdev->subordinate, request_resume, NULL);
+	return -EAGAIN;
+}
+
+static int upstream_runtime_resume(struct device *dev)
+{
+	struct tb_power *power = to_power(dev);
+	struct pci_dev *pdev = to_pci_dev(dev);
+	int ret;
+
+	if (!dev->power.is_prepared &&
+	    ACPI_FAILURE(acpi_disable_gpe(NULL, power->wake_gpe))) {
+		pr_err("cannot disable wake GPE, disabling runtime pm\n");
+		pm_runtime_get_noresume(&power->tb->nhi->pdev->dev);
+	}
+
+	pr_info("powering up\n");
+	if (ACPI_FAILURE(acpi_execute_simple_method(power->set, NULL, 1))) {
+		pr_err("cannot call power->set method\n");
+		return -ENODEV;
+	}
+
+	ret = dev->bus->pm->runtime_resume(dev);
+
+	/* wake children to force pci_restore_state() after D3cold */
+	pci_walk_bus(pdev->subordinate, request_resume, NULL);
+
+	return ret;
+}
+
+static u32 nhi_wake(acpi_handle gpe_device, u32 gpe_number, void *ctx)
+{
+	struct device *nhi_dev = ctx;
+	WARN_ON(pm_request_resume(nhi_dev) < 0);
+	return ACPI_INTERRUPT_HANDLED;
+}
+
+static int disable_pme_poll(struct pci_dev *pdev, void *ptr)
+{
+	struct pci_bus *downstream_bus = (struct pci_bus *)ptr;
+
+	/* PME# pin is not connected, the wake GPE is used instead */
+	if (pdev->bus == downstream_bus	||		/* downstream bridge */
+	    pdev->subordinate == downstream_bus ||	  /* upstream bridge */
+	    (pdev->bus->parent == downstream_bus &&
+	     pdev->class == PCI_CLASS_SYSTEM_OTHER << 8))	      /* NHI */
+		pdev->pme_poll = false;
+
+	return 0;
+}
+
+void thunderbolt_power_init(struct tb *tb)
+{
+	struct device *upstream_dev, *nhi_dev = &tb->nhi->pdev->dev;
+	struct tb_power *power = NULL;
+	struct acpi_handle *nhi_handle;
+
+	power = kzalloc(sizeof(*power), GFP_KERNEL);
+	if (!power) {
+		dev_err(nhi_dev, "cannot allocate power data\n");
+		goto err_free;
+	}
+
+	nhi_handle = ACPI_HANDLE(nhi_dev);
+	if (!nhi_handle) {
+		dev_err(nhi_dev, "cannot find ACPI handle\n");
+		goto err_free;
+	}
+
+	if (!nhi_dev->parent || !nhi_dev->parent->parent) {
+		dev_err(nhi_dev, "cannot find upstream bridge\n");
+		goto err_free;
+	}
+	upstream_dev = nhi_dev->parent->parent;
+
+	/* Macs introduced 2011/2012 have XRPE, 2013+ have TRPE */
+	if (ACPI_FAILURE(acpi_get_handle(nhi_handle, "XRPE", &power->set)) &&
+	    ACPI_FAILURE(acpi_get_handle(nhi_handle, "TRPE", &power->set))) {
+		dev_err(nhi_dev, "cannot find power->set method\n");
+		goto err_free;
+	}
+
+	if (ACPI_FAILURE(acpi_get_handle(nhi_handle, "XRIL", &power->get))) {
+		dev_err(nhi_dev, "cannot find power->get method\n");
+		goto err_free;
+	}
+
+	if (ACPI_FAILURE(acpi_evaluate_integer(nhi_handle, "XRIN", NULL,
+							&power->wake_gpe))) {
+		dev_err(nhi_dev, "cannot find wake GPE\n");
+		goto err_free;
+	}
+
+	if (ACPI_FAILURE(acpi_install_gpe_handler(NULL, power->wake_gpe,
+			     ACPI_GPE_LEVEL_TRIGGERED, nhi_wake, nhi_dev))) {
+		dev_err(nhi_dev, "cannot install GPE handler\n");
+		goto err_free;
+	}
+
+	pci_walk_bus(to_pci_dev(upstream_dev)->bus, disable_pme_poll,
+		     to_pci_dev(upstream_dev)->subordinate);
+
+	power->pm_domain.ops		     = *upstream_dev->bus->pm;
+	power->pm_domain.ops.prepare	     =  upstream_prepare;
+	power->pm_domain.ops.complete	     =  upstream_complete;
+	power->pm_domain.ops.runtime_suspend =  upstream_runtime_suspend;
+	power->pm_domain.ops.runtime_resume  =  upstream_runtime_resume;
+	power->tb			     =  tb;
+	dev_pm_domain_set(upstream_dev, &power->pm_domain);
+
+	tb->power = power;
+
+	return;
+
+err_free:
+	dev_err(nhi_dev, "controller will stay powered up permanently\n");
+	kfree(power);
+}
+
+void thunderbolt_power_fini(struct tb *tb)
+{
+	struct device *nhi_dev = &tb->nhi->pdev->dev;
+	struct device *upstream_dev = nhi_dev->parent->parent;
+	struct tb_power *power = tb->power;
+
+	if (!power)
+		return; /* thunderbolt_power_init() failed */
+
+	tb->power = NULL;
+	dev_pm_domain_set(upstream_dev, NULL);
+
+	if (ACPI_FAILURE(acpi_remove_gpe_handler(NULL, power->wake_gpe,
+						 nhi_wake)))
+		dev_err(nhi_dev, "cannot remove GPE handler\n");
+
+	kfree(power);
+}
diff --git a/drivers/thunderbolt/power.h b/drivers/thunderbolt/power.h
new file mode 100644
index 0000000..4ab9b29
--- /dev/null
+++ b/drivers/thunderbolt/power.h
@@ -0,0 +1,37 @@
+/*
+ * power.h - power thunderbolt controller down when idle
+ * Copyright (C) 2016 Lukas Wunner <lukas@wunner.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License (version 2) as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef THUNDERBOLT_POWER_H
+#define THUNDERBOLT_POWER_H
+
+#include <linux/acpi.h>
+#include <linux/pm_domain.h>
+
+#include "tb.h"
+
+struct tb_power {
+	struct tb *tb;
+	struct dev_pm_domain pm_domain; /* assigned to upstream bridge */
+	unsigned long long wake_gpe; /* hotplug interrupt during powerdown */
+	acpi_handle set; /* method to power controller up/down */
+	acpi_handle get; /* method to query power state */
+};
+
+void thunderbolt_power_init(struct tb *tb);
+void thunderbolt_power_fini(struct tb *tb);
+
+#endif
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 61d57ba..43aed58 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -11,6 +11,7 @@
 
 #include "tb_regs.h"
 #include "ctl.h"
+#include "power.h"
 
 /**
  * struct tb_switch - a thunderbolt switch
@@ -103,6 +104,7 @@ struct tb {
 				 */
 	struct tb_nhi *nhi;
 	struct tb_ctl *ctl;
+	struct tb_power *power;
 	struct workqueue_struct *wq; /* ordered workqueue for plug events */
 	struct tb_switch *root_switch;
 	struct list_head tunnel_list; /* list of active PCIe tunnels */
-- 
2.11.0

[PATCH v4 8/8] thunderbolt: Runtime suspend NHI when idle

[Lukas Wunner]

Runtime suspend the NHI when no Thunderbolt devices have been plugged in
for 10 sec (user-configurable via autosuspend_delay_ms in sysfs).

The NHI is not able to detect plug events while suspended, it relies on
the GPE handler to resume it on hotplug.

After the NHI resumes, it takes about 700 ms until a hotplug event
appears on the RX ring.  In case autosuspend_delay_ms has been reduced
to 0 by the user, we need to wait in tb_resume() to avoid going back to
sleep before we had a chance to detect a hotplugged device.  A runtime
pm ref is held for the duration of tb_handle_hotplug() to keep the NHI
awake while the hotplug event is processed.

Apart from that we acquire a runtime pm ref for each newly allocated
switch (except for the root switch) and drop one when a switch is freed,
thereby ensuring the NHI stays active as long as devices are plugged in.
This behaviour is identical to the macOS driver.

Cc: Andreas Noever <andreas.noever@gmail.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/thunderbolt/nhi.c    |  2 ++
 drivers/thunderbolt/power.c  |  9 +++++++++
 drivers/thunderbolt/switch.c |  9 +++++++++
 drivers/thunderbolt/tb.c     | 13 +++++++++++++
 4 files changed, 33 insertions(+)

diff --git a/drivers/thunderbolt/nhi.c b/drivers/thunderbolt/nhi.c
index 88fb2fb..319ed81 100644
--- a/drivers/thunderbolt/nhi.c
+++ b/drivers/thunderbolt/nhi.c
@@ -632,6 +632,8 @@ static void nhi_remove(struct pci_dev *pdev)
 					    * pci-tunnels stay alive.
 					    */
 	.restore_noirq = nhi_resume_noirq,
+	.runtime_suspend = nhi_suspend_noirq,
+	.runtime_resume = nhi_resume_noirq,
 };
 
 static struct pci_device_id nhi_ids[] = {
diff --git a/drivers/thunderbolt/power.c b/drivers/thunderbolt/power.c
index 6e7ef07..06f535c 100644
--- a/drivers/thunderbolt/power.c
+++ b/drivers/thunderbolt/power.c
@@ -319,6 +319,12 @@ void thunderbolt_power_init(struct tb *tb)
 
 	tb->power = power;
 
+	pm_runtime_allow(nhi_dev);
+	pm_runtime_set_autosuspend_delay(nhi_dev, 10000);
+	pm_runtime_use_autosuspend(nhi_dev);
+	pm_runtime_mark_last_busy(nhi_dev);
+	pm_runtime_put_autosuspend(nhi_dev);
+
 	return;
 
 err_free:
@@ -335,6 +341,9 @@ void thunderbolt_power_fini(struct tb *tb)
 	if (!power)
 		return; /* thunderbolt_power_init() failed */
 
+	pm_runtime_get(nhi_dev);
+	pm_runtime_forbid(nhi_dev);
+
 	tb->power = NULL;
 	dev_pm_domain_set(upstream_dev, NULL);
 
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index c6f30b1..422fe6e 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/delay.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 
 #include "tb.h"
@@ -326,6 +327,11 @@ void tb_switch_free(struct tb_switch *sw)
 	if (!sw->is_unplugged)
 		tb_plug_events_active(sw, false);
 
+	if (sw != sw->tb->root_switch) {
+		pm_runtime_mark_last_busy(&sw->tb->nhi->pdev->dev);
+		pm_runtime_put_autosuspend(&sw->tb->nhi->pdev->dev);
+	}
+
 	kfree(sw->ports);
 	kfree(sw->drom);
 	kfree(sw);
@@ -420,6 +426,9 @@ struct tb_switch *tb_switch_alloc(struct tb *tb, u64 route)
 	if (tb_plug_events_active(sw, true))
 		goto err;
 
+	if (tb->root_switch)
+		pm_runtime_get(&tb->nhi->pdev->dev);
+
 	return sw;
 err:
 	kfree(sw->ports);
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 24b6d30..a3fedf9 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -7,6 +7,7 @@
 #include <linux/slab.h>
 #include <linux/errno.h>
 #include <linux/delay.h>
+#include <linux/pm_runtime.h>
 
 #include "tb.h"
 #include "tb_regs.h"
@@ -217,8 +218,11 @@ static void tb_handle_hotplug(struct work_struct *work)
 {
 	struct tb_hotplug_event *ev = container_of(work, typeof(*ev), work);
 	struct tb *tb = ev->tb;
+	struct device *dev = &tb->nhi->pdev->dev;
 	struct tb_switch *sw;
 	struct tb_port *port;
+
+	pm_runtime_get(dev);
 	mutex_lock(&tb->lock);
 	if (!tb->hotplug_active)
 		goto out; /* during init, suspend or shutdown */
@@ -274,6 +278,8 @@ static void tb_handle_hotplug(struct work_struct *work)
 out:
 	mutex_unlock(&tb->lock);
 	kfree(ev);
+	pm_runtime_mark_last_busy(dev);
+	pm_runtime_put_autosuspend(dev);
 }
 
 /**
@@ -433,4 +439,11 @@ void thunderbolt_resume(struct tb *tb)
 	tb->hotplug_active = true;
 	mutex_unlock(&tb->lock);
 	tb_info(tb, "resume finished\n");
+
+	/*
+	 * If runtime resuming due to a hotplug event (rather than resuming
+	 * from system sleep), wait for it to arrive. May take about 700 ms.
+	 */
+	if (tb->nhi->pdev->dev.power.runtime_status == RPM_RESUMING)
+		msleep(1000);
 }
-- 
2.11.0

[PATCH v4 4/8] Revert "PM / Runtime: Remove the exported function pm_children_suspended()"

[Lukas Wunner]

This reverts commit 62006c1702b3b1be0c0726949e0ee0ea2326be9c which
removed pm_children_suspended() because it had only a single caller.
We're about to add a second caller, so establish the status quo ante.

Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drivers/base/power/runtime.c | 3 +--
 include/linux/pm_runtime.h   | 7 +++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 872eac4..03293c3 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -243,8 +243,7 @@ static int rpm_check_suspend_allowed(struct device *dev)
 		retval = -EACCES;
 	else if (atomic_read(&dev->power.usage_count) > 0)
 		retval = -EAGAIN;
-	else if (!dev->power.ignore_children &&
-			atomic_read(&dev->power.child_count))
+	else if (!pm_children_suspended(dev))
 		retval = -EBUSY;
 
 	/* Pending resume requests take precedence over suspends. */
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index ca4823e..7de2aa5c 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -66,6 +66,12 @@ static inline void pm_suspend_ignore_children(struct device *dev, bool enable)
 	dev->power.ignore_children = enable;
 }
 
+static inline bool pm_children_suspended(struct device *dev)
+{
+	return dev->power.ignore_children
+		|| !atomic_read(&dev->power.child_count);
+}
+
 static inline void pm_runtime_get_noresume(struct device *dev)
 {
 	atomic_inc(&dev->power.usage_count);
@@ -161,6 +167,7 @@ static inline void pm_runtime_allow(struct device *dev) {}
 static inline void pm_runtime_forbid(struct device *dev) {}
 
 static inline void pm_suspend_ignore_children(struct device *dev, bool enable) {}
+static inline bool pm_children_suspended(struct device *dev) { return false; }
 static inline void pm_runtime_get_noresume(struct device *dev) {}
 static inline void pm_runtime_put_noidle(struct device *dev) {}
 static inline bool device_run_wake(struct device *dev) { return false; }
-- 
2.11.0