Re: [PATCH v1 1/2] PCI: Add PCIe port runtime suspend details

Re: [PATCH v1 1/2] PCI: Add PCIe port runtime suspend details

[Mika Westerberg]

On Tue, Feb 20, 2018 at 10:31:51AM +0100, Rafael J. Wysocki wrote:
> On Tue, Feb 20, 2018 at 12:14 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> >
> > Add details about how we decide whether we can put a PCI bridge in D3.
> > 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") added this
> > support to reduce power consumption on Intel Sunrise Point and Broxton
> > platforms.
> >
> > In some cases we don't use D3 for bridges even when it should work, simply
> > because it's impractical to test the configuration, or we tripped over some
> > possible hardware issue on older platforms.  Links to discussion of the
> > PCIe port runtime power management patches, which includes mention of these
> > issues, are below.
> >
> > No functional change.
> >
> > Link: v1: https://lkml.kernel.org/r/1456750566-116248-1-git-send-email-mika.westerberg@linux.intel.com
> > Link: v2: https://lkml.kernel.org/r/1460111790-92836-1-git-send-email-mika.westerberg@linux.intel.com
> > Link: v3: https://lkml.kernel.org/r/1460628268-16204-1-git-send-email-mika.westerberg@linux.intel.com
> > Link: v4: https://lkml.kernel.org/r/1461578004-129094-1-git-send-email-mika.westerberg@linux.intel.com
> > Link: v5: https://lkml.kernel.org/r/1461919919-120102-1-git-send-email-mika.westerberg@linux.intel.com
> > Link: v6: https://lkml.kernel.org/r/1464855435-32960-1-git-send-email-mika.westerberg@linux.intel.com
> > Link: https://lkml.kernel.org/r/2858019.9TUCWsDpTB@aspire.rjw.lan
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Also

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

Re: [PATCH v1 2/2] PCI: Allow user to request power management of conventional and hotplug bridges

[Mika Westerberg]

On Thu, Feb 22, 2018 at 02:18:34PM +0100, Lukas Wunner wrote:
> We do need runtime PM on hotplug ports to power off Thunderbolt
> controllers when nothing is plugged in.  That saves 1.5 W, so a
> noticeable amount of power.  I was going to respin the series one
> of these days, I think the best I can do is continue to forbid
> runtime PM on hotplug ports by default, but whitelist it for
> Thunderbolt and allow manually enabling it on other platforms via
> the command line.  That way, vendors are put in a position to
> validate their platforms for runtime PM of hotplug ports, and
> perhaps someday we can enable it for all platforms by default,
> but with a BIOS cut-off date.

AFAIK Windows started to enable runtime PM (RTD3) for native PCIe
hotplug ports with the latest release (I guess it's the RS3 release) but
only when there is a special ACPI _DSD property ("HotPlugSupportInD3")
associated with the root port. I think we can take advantage of that in
Linux as well and I already have a patch series to enable runtime PM for
such ports but I haven't been able to test it properly yet.

Re: [PATCH v1 2/2] PCI: Allow user to request power management of conventional and hotplug bridges

[Lukas Wunner]

On Mon, Feb 26, 2018 at 02:05:34PM +0200, Mika Westerberg wrote:
> On Thu, Feb 22, 2018 at 02:18:34PM +0100, Lukas Wunner wrote:
> > We do need runtime PM on hotplug ports to power off Thunderbolt
> > controllers when nothing is plugged in.  That saves 1.5 W, so a
> > noticeable amount of power.  I was going to respin the series one
> > of these days, I think the best I can do is continue to forbid
> > runtime PM on hotplug ports by default, but whitelist it for
> > Thunderbolt and allow manually enabling it on other platforms via
> > the command line.  That way, vendors are put in a position to
> > validate their platforms for runtime PM of hotplug ports, and
> > perhaps someday we can enable it for all platforms by default,
> > but with a BIOS cut-off date.
> 
> AFAIK Windows started to enable runtime PM (RTD3) for native PCIe
> hotplug ports with the latest release (I guess it's the RS3 release) but
> only when there is a special ACPI _DSD property ("HotPlugSupportInD3")
> associated with the root port. I think we can take advantage of that in
> Linux as well and I already have a patch series to enable runtime PM for
> such ports but I haven't been able to test it properly yet.

Okay.  Well it would be trivial to whitelist those ports with
device_property_present("HotPlugSupportInD3").  In how far are
your patches identical with the patches I submitted last May?
I've started reworking them for v2 but that would be a waste of
time if you're working on this issue in parallel.

Thanks,

Lukas

Re: [PATCH v1 2/2] PCI: Allow user to request power management of conventional and hotplug bridges

[Mika Westerberg]

On Mon, Feb 26, 2018 at 01:22:43PM +0100, Lukas Wunner wrote:
> On Mon, Feb 26, 2018 at 02:05:34PM +0200, Mika Westerberg wrote:
> > On Thu, Feb 22, 2018 at 02:18:34PM +0100, Lukas Wunner wrote:
> > > We do need runtime PM on hotplug ports to power off Thunderbolt
> > > controllers when nothing is plugged in.  That saves 1.5 W, so a
> > > noticeable amount of power.  I was going to respin the series one
> > > of these days, I think the best I can do is continue to forbid
> > > runtime PM on hotplug ports by default, but whitelist it for
> > > Thunderbolt and allow manually enabling it on other platforms via
> > > the command line.  That way, vendors are put in a position to
> > > validate their platforms for runtime PM of hotplug ports, and
> > > perhaps someday we can enable it for all platforms by default,
> > > but with a BIOS cut-off date.
> > 
> > AFAIK Windows started to enable runtime PM (RTD3) for native PCIe
> > hotplug ports with the latest release (I guess it's the RS3 release) but
> > only when there is a special ACPI _DSD property ("HotPlugSupportInD3")
> > associated with the root port. I think we can take advantage of that in
> > Linux as well and I already have a patch series to enable runtime PM for
> > such ports but I haven't been able to test it properly yet.
> 
> Okay.  Well it would be trivial to whitelist those ports with
> device_property_present("HotPlugSupportInD3").  In how far are
> your patches identical with the patches I submitted last May?

My patches pretty much only touch the whitelist part not the other fixes
you made for pciehp in your series. We can add those on top of your
series or I can send them out separately.

> I've started reworking them for v2 but that would be a waste of
> time if you're working on this issue in parallel.

Please continue with your v2 :) I can provide testing assistance if you
need any.