[1/2] vga_switcheroo: add power support for windows 10 machines.

Commit Message

Dave Airlie March 14, 2016, 2:19 a.m. UTC

On 11 March 2016 at 23:45, Rafael J. Wysocki <[e-mail]> wrote:
> On Friday, March 11, 2016 12:58:15 PM Mika Westerberg wrote:
>> On Thu, Mar 10, 2016 at 09:57:09PM +0100, Rafael J. Wysocki wrote:
>> > > It doesn't seem to do any runtime PM,
>> > > I do wonder if pcieport should be doing it's own runtime PM handling,
>> > > but that is a
>> > > larger task than I'm thinking to tackle here.
>> >
>> > PCIe ports don't do PM - yet.  Mika has posted a series of patches to implement
>> > that, however, that are waiting for comments now:
>> >
>> > https://patchwork.kernel.org/patch/8453311/
>> > https://patchwork.kernel.org/patch/8453381/
>> > https://patchwork.kernel.org/patch/8453391/
>> > https://patchwork.kernel.org/patch/8453411/
>> > https://patchwork.kernel.org/patch/8453371/
>> > https://patchwork.kernel.org/patch/8453351/
>> >
>> > > Maybe I should be doing
>> > >
>> > > pci_set_power_state(pdev->bus->self, PCI_D3cold) ? I'm not really sure.
>> >
>> > Using pci_set_power_state() would be more appropriate IMO, but you can get
>> > to the bridge via dev->parent too, can't you?
>> >
>> > In any case, it looks like you and Mika need to talk. :-)
>>
>> When the vga_switcheroo device gets runtime suspended (with the above
>> runtime PM patchs for PCIe root ports) the root port should also be
>> runtime suspended by the PM core.
>
> Right, after your patches have been applied, the additional handling
> won't be needed.
>
> So Dave, maybe you can check if the Mika's patches help?

Hi Mika,

I tested your patches with a couple of changes on the Lenovo W541.

The attached patch contains the two things I needed to get the same
functionality
as my patches.

I'm really not in love with the per-chipset enablement for this,
really any chipsets
after a certain year should probably be better, as we'll constantly be
adding PCI Ids
for every chipset ever made, and I expect we'll forget some.

Dave.

Comments

Mika Westerberg March 14, 2016, 9:43 a.m. UTC | #1

On Mon, Mar 14, 2016 at 12:19:14PM +1000, Dave Airlie wrote:
> On 11 March 2016 at 23:45, Rafael J. Wysocki <[e-mail]> wrote:
> > On Friday, March 11, 2016 12:58:15 PM Mika Westerberg wrote:
> >> On Thu, Mar 10, 2016 at 09:57:09PM +0100, Rafael J. Wysocki wrote:
> >> > > It doesn't seem to do any runtime PM,
> >> > > I do wonder if pcieport should be doing it's own runtime PM handling,
> >> > > but that is a
> >> > > larger task than I'm thinking to tackle here.
> >> >
> >> > PCIe ports don't do PM - yet.  Mika has posted a series of patches to implement
> >> > that, however, that are waiting for comments now:
> >> >
> >> > https://patchwork.kernel.org/patch/8453311/
> >> > https://patchwork.kernel.org/patch/8453381/
> >> > https://patchwork.kernel.org/patch/8453391/
> >> > https://patchwork.kernel.org/patch/8453411/
> >> > https://patchwork.kernel.org/patch/8453371/
> >> > https://patchwork.kernel.org/patch/8453351/
> >> >
> >> > > Maybe I should be doing
> >> > >
> >> > > pci_set_power_state(pdev->bus->self, PCI_D3cold) ? I'm not really sure.
> >> >
> >> > Using pci_set_power_state() would be more appropriate IMO, but you can get
> >> > to the bridge via dev->parent too, can't you?
> >> >
> >> > In any case, it looks like you and Mika need to talk. :-)
> >>
> >> When the vga_switcheroo device gets runtime suspended (with the above
> >> runtime PM patchs for PCIe root ports) the root port should also be
> >> runtime suspended by the PM core.
> >
> > Right, after your patches have been applied, the additional handling
> > won't be needed.
> >
> > So Dave, maybe you can check if the Mika's patches help?
> 
> Hi Mika,
> 
> I tested your patches with a couple of changes on the Lenovo W541.

Thanks for testing.

> The attached patch contains the two things I needed to get the same
> functionality
> as my patches.
> 
> I'm really not in love with the per-chipset enablement for this,
> really any chipsets
> after a certain year should probably be better, as we'll constantly be
> adding PCI Ids
> for every chipset ever made, and I expect we'll forget some.

Bjorn also had the same comment. I think I'll change it to use cut-off
date instead of list of PCI IDs of nobody objects.

> Dave.

> From eea412076c9d24262ebd4811f766d5379b728045 Mon Sep 17 00:00:00 2001
> From: Dave Airlie <[e-mail]>
> Date: Mon, 14 Mar 2016 23:37:43 +1000
> Subject: [PATCH] [RFC] PCI: enable runtime PM on pcieports to let secondary
>  GPUs powerdown.
> 
> With secondary GPUs in Windows 10 laptops we need to use runtime PM
> to power down the PCIe ports after the devices goes to D3Cold.
> 
> This patch adds the PCI ID for the Haswell 16x PCIE slot found
> in the W541 laptops. I should probably try and gather the PCI IDs,
> for broadwell/skylake variants as well if we can't find them.
> 
> This fixes a regression on W541 laptops on top of Mika's PCIE patches
> since we started advertising Windows 2013 ACPI.
> 
> [probably should be two patches - hence RFC]
> 
> Signed-off-by: Dave Airlie <[e-mail]>
> ---
>  drivers/pci/pcie/portdrv_pci.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
> index 43dd23e..1405de8 100644
> --- a/drivers/pci/pcie/portdrv_pci.c
> +++ b/drivers/pci/pcie/portdrv_pci.c
> @@ -278,8 +278,10 @@ static int pcie_portdrv_probe(struct pci_dev *dev,
>  
>  	pci_save_state(dev);
>  
> -	if (pcie_port_runtime_suspend_allowed(dev))
> +	if (pcie_port_runtime_suspend_allowed(dev)) {
> +		pm_runtime_allow(&dev->dev);

PCI drivers typically have left this decision up to the userspace. I'm
wondering whether it is good idea to deviate from that here? Of course
this allows immediate power savings but could potentially cause problems
as well.

I think we need to add corresponding call to pm_runtime_forbid() in
pcie_portdrv_remove().

>  		pm_runtime_put_noidle(&dev->dev);
> +	}
>  
>  	return 0;
>  }
> @@ -436,6 +438,8 @@ static void pcie_portdrv_err_resume(struct pci_dev *dev)
>   * LINUX Device Driver Model
>   */
>  static const struct pci_device_id port_pci_ids[] = {
> +	/* Intel Skylake PCIE graphics port */
> +	{ PCI_VDEVICE(INTEL, 0x0c01), .driver_data = PCIE_PORT_SPT },
>  	/* Intel Broxton */
>  	{ PCI_VDEVICE(INTEL, 0x1ad6), .driver_data = PCIE_PORT_SPT },
>  	{ PCI_VDEVICE(INTEL, 0x1ad7), .driver_data = PCIE_PORT_SPT },
> -- 
> 2.5.0
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Dave Airlie March 14, 2016, 9:47 a.m. UTC | #2

>
>> -     if (pcie_port_runtime_suspend_allowed(dev))
>> +     if (pcie_port_runtime_suspend_allowed(dev)) {
>> +             pm_runtime_allow(&dev->dev);
>
> PCI drivers typically have left this decision up to the userspace. I'm
> wondering whether it is good idea to deviate from that here? Of course
> this allows immediate power savings but could potentially cause problems
> as well.
>

No distro has ever shipped userspace to do this, I really think this
is a bad design.
We have wasted countless watts of power on this stupid idea that people will
run powertop, only a few people in the world run powertop, lots of
people use Linux.

The kernel should power stuff down not wait for the user to run powertop,
At least for the GPU it's in the area of 8W of power, and I've got the
GPU drivers doing this themselves,

I could have the GPU driver call runtime allow for it's host bridge I suppose,
if we insist on the userspace cares, but I'd prefer not doing so.

> I think we need to add corresponding call to pm_runtime_forbid() in
> pcie_portdrv_remove().

Yes most likely.

Dave.
>

>>               pm_runtime_put_noidle(&dev->dev);
>> +     }
>>
>>       return 0;
>>  }
>> @@ -436,6 +438,8 @@ static void pcie_portdrv_err_resume(struct pci_dev *dev)
>>   * LINUX Device Driver Model
>>   */
>>  static const struct pci_device_id port_pci_ids[] = {
>> +     /* Intel Skylake PCIE graphics port */
>> +     { PCI_VDEVICE(INTEL, 0x0c01), .driver_data = PCIE_PORT_SPT },
>>       /* Intel Broxton */
>>       { PCI_VDEVICE(INTEL, 0x1ad6), .driver_data = PCIE_PORT_SPT },
>>       { PCI_VDEVICE(INTEL, 0x1ad7), .driver_data = PCIE_PORT_SPT },
>> --
>> 2.5.0
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Daniel Vetter March 14, 2016, 10:02 a.m. UTC | #3

On Mon, Mar 14, 2016 at 07:47:39PM +1000, Dave Airlie wrote:
> >
> >> -     if (pcie_port_runtime_suspend_allowed(dev))
> >> +     if (pcie_port_runtime_suspend_allowed(dev)) {
> >> +             pm_runtime_allow(&dev->dev);
> >
> > PCI drivers typically have left this decision up to the userspace. I'm
> > wondering whether it is good idea to deviate from that here? Of course
> > this allows immediate power savings but could potentially cause problems
> > as well.
> >
> 
> No distro has ever shipped userspace to do this, I really think this
> is a bad design.
> We have wasted countless watts of power on this stupid idea that people will
> run powertop, only a few people in the world run powertop, lots of
> people use Linux.
> 
> The kernel should power stuff down not wait for the user to run powertop,
> At least for the GPU it's in the area of 8W of power, and I've got the
> GPU drivers doing this themselves,
> 
> I could have the GPU driver call runtime allow for it's host bridge I suppose,
> if we insist on the userspace cares, but I'd prefer not doing so.

Yes, fully agreed. Runtime PM that's not enabled by default is useless,
since no one runs powertop, which also means it's buggy because no one
tests it. Not enabling power saving features by default is imo just a lame
excuse. Shit better just work, and tuneables should only exposed if
there's a real reason why autotuning is infeasible. E.g. we auto-ramp the
gpu clock down/up in i915 in software to compensate for some of the
silliness in how the hw does it alone. And the sysfs tunables are mostly
just to make benchmarking (where slower, but without any thermal
throttling) is sometimes preferred.

And yes i915 hasn't enabled rpm yet by default because there's bugs left
:(
-Daniel

Mika Westerberg March 14, 2016, 10:23 a.m. UTC | #4

On Mon, Mar 14, 2016 at 07:47:39PM +1000, Dave Airlie wrote:
> >
> >> -     if (pcie_port_runtime_suspend_allowed(dev))
> >> +     if (pcie_port_runtime_suspend_allowed(dev)) {
> >> +             pm_runtime_allow(&dev->dev);
> >
> > PCI drivers typically have left this decision up to the userspace. I'm
> > wondering whether it is good idea to deviate from that here? Of course
> > this allows immediate power savings but could potentially cause problems
> > as well.
> >
> 
> No distro has ever shipped userspace to do this, I really think this
> is a bad design.
> We have wasted countless watts of power on this stupid idea that people will
> run powertop, only a few people in the world run powertop, lots of
> people use Linux.

That is a fair point.

I do not have anything against calling pm_runtime_allow() here. In fact
we already do the same in Intel LPSS drivers. I just wanted to bring
that up.

Rafael, what do you think?

If we anyway are going to add cut-off date to enable runtime PM we
should expect that the hardware is also capable of doing so (and if not
we can always blacklist the exceptions).

> The kernel should power stuff down not wait for the user to run powertop,
> At least for the GPU it's in the area of 8W of power, and I've got the
> GPU drivers doing this themselves,
> 
> I could have the GPU driver call runtime allow for it's host bridge I suppose,
> if we insist on the userspace cares, but I'd prefer not doing so.
> 
> > I think we need to add corresponding call to pm_runtime_forbid() in
> > pcie_portdrv_remove().
> 
> Yes most likely.

BTW, I can add both calls to the next version of PCIe runtime PM patches
if you are OK with that, and all agree this is a good idea.
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Rafael J. Wysocki March 14, 2016, 12:50 p.m. UTC | #5

On Mon, Mar 14, 2016 at 11:23 AM, Mika Westerberg
<[e-mail]> wrote:
> On Mon, Mar 14, 2016 at 07:47:39PM +1000, Dave Airlie wrote:
>> >
>> >> -     if (pcie_port_runtime_suspend_allowed(dev))
>> >> +     if (pcie_port_runtime_suspend_allowed(dev)) {
>> >> +             pm_runtime_allow(&dev->dev);
>> >
>> > PCI drivers typically have left this decision up to the userspace. I'm
>> > wondering whether it is good idea to deviate from that here? Of course
>> > this allows immediate power savings but could potentially cause problems
>> > as well.
>> >
>>
>> No distro has ever shipped userspace to do this, I really think this
>> is a bad design.
>> We have wasted countless watts of power on this stupid idea that people will
>> run powertop, only a few people in the world run powertop, lots of
>> people use Linux.
>
> That is a fair point.
>
> I do not have anything against calling pm_runtime_allow() here. In fact
> we already do the same in Intel LPSS drivers. I just wanted to bring
> that up.
>
> Rafael, what do you think?

We can do that to start with.  If there are no problems in the field
with it, I don't see any problems in principle.

> If we anyway are going to add cut-off date to enable runtime PM we
> should expect that the hardware is also capable of doing so (and if not
> we can always blacklist the exceptions).

Sounds reasonable.

>> The kernel should power stuff down not wait for the user to run powertop,
>> At least for the GPU it's in the area of 8W of power, and I've got the
>> GPU drivers doing this themselves,
>>
>> I could have the GPU driver call runtime allow for it's host bridge I suppose,
>> if we insist on the userspace cares, but I'd prefer not doing so.
>>
>> > I think we need to add corresponding call to pm_runtime_forbid() in
>> > pcie_portdrv_remove().
>>
>> Yes most likely.
>
> BTW, I can add both calls to the next version of PCIe runtime PM patches
> if you are OK with that, and all agree this is a good idea.

That would be fine by me.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Alex Deucher March 14, 2016, 2:30 p.m. UTC | #6

On Sun, Mar 13, 2016 at 10:19 PM, Dave Airlie <[e-mail]> wrote:
> On 11 March 2016 at 23:45, Rafael J. Wysocki <[e-mail]> wrote:
>> On Friday, March 11, 2016 12:58:15 PM Mika Westerberg wrote:
>>> On Thu, Mar 10, 2016 at 09:57:09PM +0100, Rafael J. Wysocki wrote:
>>> > > It doesn't seem to do any runtime PM,
>>> > > I do wonder if pcieport should be doing it's own runtime PM handling,
>>> > > but that is a
>>> > > larger task than I'm thinking to tackle here.
>>> >
>>> > PCIe ports don't do PM - yet.  Mika has posted a series of patches to implement
>>> > that, however, that are waiting for comments now:
>>> >
>>> > https://patchwork.kernel.org/patch/8453311/
>>> > https://patchwork.kernel.org/patch/8453381/
>>> > https://patchwork.kernel.org/patch/8453391/
>>> > https://patchwork.kernel.org/patch/8453411/
>>> > https://patchwork.kernel.org/patch/8453371/
>>> > https://patchwork.kernel.org/patch/8453351/
>>> >
>>> > > Maybe I should be doing
>>> > >
>>> > > pci_set_power_state(pdev->bus->self, PCI_D3cold) ? I'm not really sure.
>>> >
>>> > Using pci_set_power_state() would be more appropriate IMO, but you can get
>>> > to the bridge via dev->parent too, can't you?
>>> >
>>> > In any case, it looks like you and Mika need to talk. :-)
>>>
>>> When the vga_switcheroo device gets runtime suspended (with the above
>>> runtime PM patchs for PCIe root ports) the root port should also be
>>> runtime suspended by the PM core.
>>
>> Right, after your patches have been applied, the additional handling
>> won't be needed.
>>
>> So Dave, maybe you can check if the Mika's patches help?
>
> Hi Mika,
>
> I tested your patches with a couple of changes on the Lenovo W541.
>
> The attached patch contains the two things I needed to get the same
> functionality
> as my patches.
>
> I'm really not in love with the per-chipset enablement for this,
> really any chipsets
> after a certain year should probably be better, as we'll constantly be
> adding PCI Ids
> for every chipset ever made, and I expect we'll forget some.


Yeah, I don't care of that either.  There are always going to be
chipsets falling through the cracks.

Alex

>
> Dave.
>
> _______________________________________________
> dri-devel mailing list
> [e-mail]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mika Westerberg March 14, 2016, 2:30 p.m. UTC | #7

On Mon, Mar 14, 2016 at 01:50:41PM +0100, Rafael J. Wysocki wrote:
> On Mon, Mar 14, 2016 at 11:23 AM, Mika Westerberg
> <[e-mail]> wrote:
> > On Mon, Mar 14, 2016 at 07:47:39PM +1000, Dave Airlie wrote:
> >> >
> >> >> -     if (pcie_port_runtime_suspend_allowed(dev))
> >> >> +     if (pcie_port_runtime_suspend_allowed(dev)) {
> >> >> +             pm_runtime_allow(&dev->dev);
> >> >
> >> > PCI drivers typically have left this decision up to the userspace. I'm
> >> > wondering whether it is good idea to deviate from that here? Of course
> >> > this allows immediate power savings but could potentially cause problems
> >> > as well.
> >> >
> >>
> >> No distro has ever shipped userspace to do this, I really think this
> >> is a bad design.
> >> We have wasted countless watts of power on this stupid idea that people will
> >> run powertop, only a few people in the world run powertop, lots of
> >> people use Linux.
> >
> > That is a fair point.
> >
> > I do not have anything against calling pm_runtime_allow() here. In fact
> > we already do the same in Intel LPSS drivers. I just wanted to bring
> > that up.
> >
> > Rafael, what do you think?
> 
> We can do that to start with.  If there are no problems in the field
> with it, I don't see any problems in principle.
> 
> > If we anyway are going to add cut-off date to enable runtime PM we
> > should expect that the hardware is also capable of doing so (and if not
> > we can always blacklist the exceptions).
> 
> Sounds reasonable.
> 
> >> The kernel should power stuff down not wait for the user to run powertop,
> >> At least for the GPU it's in the area of 8W of power, and I've got the
> >> GPU drivers doing this themselves,
> >>
> >> I could have the GPU driver call runtime allow for it's host bridge I suppose,
> >> if we insist on the userspace cares, but I'd prefer not doing so.
> >>
> >> > I think we need to add corresponding call to pm_runtime_forbid() in
> >> > pcie_portdrv_remove().
> >>
> >> Yes most likely.
> >
> > BTW, I can add both calls to the next version of PCIe runtime PM patches
> > if you are OK with that, and all agree this is a good idea.
> 
> That would be fine by me.

OK thanks.

I'll do these changes to the next version of the patch series then.
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Lukas Wunner March 15, 2016, 1:39 p.m. UTC | #8

Hi Mika,

On Mon, Mar 14, 2016 at 11:43:35AM +0200, Mika Westerberg wrote:
> On Mon, Mar 14, 2016 at 12:19:14PM +1000, Dave Airlie wrote:
> > On 11 March 2016 at 23:45, Rafael J. Wysocki <[e-mail]> wrote:
> > > On Friday, March 11, 2016 12:58:15 PM Mika Westerberg wrote:
> > >> On Thu, Mar 10, 2016 at 09:57:09PM +0100, Rafael J. Wysocki wrote:
> > >> > > It doesn't seem to do any runtime PM,
> > >> > > I do wonder if pcieport should be doing it's own runtime PM handling,
> > >> > > but that is a
> > >> > > larger task than I'm thinking to tackle here.
> > >> >
> > >> > PCIe ports don't do PM - yet.  Mika has posted a series of patches to implement
> > >> > that, however, that are waiting for comments now:
> > >> >
> > >> > https://patchwork.kernel.org/patch/8453311/
> > >> > https://patchwork.kernel.org/patch/8453381/
> > >> > https://patchwork.kernel.org/patch/8453391/
> > >> > https://patchwork.kernel.org/patch/8453411/
> > >> > https://patchwork.kernel.org/patch/8453371/
> > >> > https://patchwork.kernel.org/patch/8453351/

If a pciehp port is runtime suspended and pciehp_poll_mode is enabled,
the poll timer needs to be disabled and later reenabled on runtime resume.

Don't we need to add runtime pm callbacks to struct pcie_port_service_driver
for that, and call them from pcie_port_runtime_*()?

Best regards,

Lukas
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mika Westerberg March 15, 2016, 1:57 p.m. UTC | #9

On Tue, Mar 15, 2016 at 02:39:58PM +0100, Lukas Wunner wrote:
> Hi Mika,
> 
> On Mon, Mar 14, 2016 at 11:43:35AM +0200, Mika Westerberg wrote:
> > On Mon, Mar 14, 2016 at 12:19:14PM +1000, Dave Airlie wrote:
> > > On 11 March 2016 at 23:45, Rafael J. Wysocki <[e-mail]> wrote:
> > > > On Friday, March 11, 2016 12:58:15 PM Mika Westerberg wrote:
> > > >> On Thu, Mar 10, 2016 at 09:57:09PM +0100, Rafael J. Wysocki wrote:
> > > >> > > It doesn't seem to do any runtime PM,
> > > >> > > I do wonder if pcieport should be doing it's own runtime PM handling,
> > > >> > > but that is a
> > > >> > > larger task than I'm thinking to tackle here.
> > > >> >
> > > >> > PCIe ports don't do PM - yet.  Mika has posted a series of patches to implement
> > > >> > that, however, that are waiting for comments now:
> > > >> >
> > > >> > https://patchwork.kernel.org/patch/8453311/
> > > >> > https://patchwork.kernel.org/patch/8453381/
> > > >> > https://patchwork.kernel.org/patch/8453391/
> > > >> > https://patchwork.kernel.org/patch/8453411/
> > > >> > https://patchwork.kernel.org/patch/8453371/
> > > >> > https://patchwork.kernel.org/patch/8453351/
> 
> If a pciehp port is runtime suspended and pciehp_poll_mode is enabled,
> the poll timer needs to be disabled and later reenabled on runtime resume.

If we disable the timer then we can't detect when a new device is
connected to the port.

I think in this case it might be better not to enable runtime PM for the
port at all.
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

From eea412076c9d24262ebd4811f766d5379b728045 Mon Sep 17 00:00:00 2001
From: Dave Airlie <[e-mail]>
Date: Mon, 14 Mar 2016 23:37:43 +1000
Subject: [PATCH] [RFC] PCI: enable runtime PM on pcieports to let secondary
 GPUs powerdown.

With secondary GPUs in Windows 10 laptops we need to use runtime PM
to power down the PCIe ports after the devices goes to D3Cold.

This patch adds the PCI ID for the Haswell 16x PCIE slot found
in the W541 laptops. I should probably try and gather the PCI IDs,
for broadwell/skylake variants as well if we can't find them.

This fixes a regression on W541 laptops on top of Mika's PCIE patches
since we started advertising Windows 2013 ACPI.

[probably should be two patches - hence RFC]

Signed-off-by: Dave Airlie <[e-mail]>
---
 drivers/pci/pcie/portdrv_pci.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index 43dd23e..1405de8 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -278,8 +278,10 @@  static int pcie_portdrv_probe(struct pci_dev *dev,
 
 	pci_save_state(dev);
 
-	if (pcie_port_runtime_suspend_allowed(dev))
+	if (pcie_port_runtime_suspend_allowed(dev)) {
+		pm_runtime_allow(&dev->dev);
 		pm_runtime_put_noidle(&dev->dev);
+	}
 
 	return 0;
 }
@@ -436,6 +438,8 @@  static void pcie_portdrv_err_resume(struct pci_dev *dev)
  * LINUX Device Driver Model
  */
 static const struct pci_device_id port_pci_ids[] = {
+	/* Intel Skylake PCIE graphics port */
+	{ PCI_VDEVICE(INTEL, 0x0c01), .driver_data = PCIE_PORT_SPT },
 	/* Intel Broxton */
 	{ PCI_VDEVICE(INTEL, 0x1ad6), .driver_data = PCIE_PORT_SPT },
 	{ PCI_VDEVICE(INTEL, 0x1ad7), .driver_data = PCIE_PORT_SPT },
-- 
2.5.0