headers
Rafael J. Wysocki | 1 Jan 2010 02:29
Picon
Gravatar

[PATCH][RFC] e1000e: Add basic runtime PM support (was: [PATCH 0/12] PCI run-time PM support (rev. 2))

On Sunday 27 December 2009, Rafael J. Wysocki wrote:
> Hi,
> 
> The following (updated) series of patches provides preliminary run-time power
> management support for PCI devices through ACPI and/or the native PCIe PME.
> 
> Some patches have been modified since the previous iteration, one patch has
> been merged and there's one more.
> 
> I've tested this patchset with the native PCIe PME mechanism using the r8169
> driver on the MSI Wind U-100 (see the last patch for details) and with the ACPI
> mechanism using the e1000e driver on the Toshiba Portege R500 (the patch still
> requires some work to be shown in public ;-)).

The e1000e patch is now in a better shape IMO, at least it worked for me in all
conditions I tested it, so it is appended below.

> [1/12] - Add function for checking PME status of devices
>  
> [2/12] - Modify wake-up enable propagation so that it's done for PCIe devices
>          too (this one is in the Jesse's tree already, but it's reproduced here
>          for completness)
>  
> [3/12] - PCIe PME root port service driver
> 
> [4/12] - "Don't use MSIs for PME signaling" switch for PCIe
>  
> [5/12] - ACPI GPE refcounting, from Matthew
>  
> [6/12] - ACPI drivers support for GPE refcounting, from Matthew
(Continue reading)

Permalink | Reply | 0 comments |
headers
Andreas Mohr | 1 Jan 2010 18:07
Picon
Favicon

Re: [PATCH] PCI / PM: Use per-device D3 delays

Hi,

While the bug report mentions "So it's just quirky hardware.",
the implementation of your patch makes it seem like this delay attribute is
totally "norm"al behaviour - I'm missing some more aggressive wording.

Perhaps rename d3_delay to d3_delay__quirk or add something to the
comment, like "D3->D0 transition time in ms (out-of-spec PCI device quirk)"?
Or maybe something like
"custom D3->D0 transition time in ms (for quirky hardware violating the PCI spec's <= 100ms)".

Thanks for your ongoing great PCI efforts,

Andreas Mohr
Permalink | Reply | 0 comments |
headers
Rafael J. Wysocki | 1 Jan 2010 19:55
Picon
Gravatar

Re: [PATCH] PCI / PM: Use per-device D3 delays

On Friday 01 January 2010, Andreas Mohr wrote:
> Hi,
> 
> While the bug report mentions "So it's just quirky hardware.",
> the implementation of your patch makes it seem like this delay attribute is
> totally "norm"al behaviour - I'm missing some more aggressive wording.

That's because it works both ways (please look at the changelog).

I know of a few devices that don't need the PCI-prescribed 10 ms wait when
going from D3 to D0 and their drivers may use the d3_delay field to actually
set a _shorter_ delay.

> Perhaps rename d3_delay to d3_delay__quirk or add something to the
> comment, like "D3->D0 transition time in ms (out-of-spec PCI device quirk)"?
> Or maybe something like
> "custom D3->D0 transition time in ms (for quirky hardware violating the PCI spec's <= 100ms)".
> 
> Thanks for your ongoing great PCI efforts,

You're welcome.

Rafael
Permalink | Reply | 0 comments |
headers
Rafael J. Wysocki | 1 Jan 2010 20:03
Picon
Gravatar

[PATCH][RFC] e1000e: Add basic runtime PM support (rev. 2) (was: [PATCH 0/12] PCI run-time PM support (rev. 2))

On Friday 01 January 2010, Rafael J. Wysocki wrote:
> On Sunday 27 December 2009, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > The following (updated) series of patches provides preliminary run-time power
> > management support for PCI devices through ACPI and/or the native PCIe PME.
> > 
> > Some patches have been modified since the previous iteration, one patch has
> > been merged and there's one more.
> > 
> > I've tested this patchset with the native PCIe PME mechanism using the r8169
> > driver on the MSI Wind U-100 (see the last patch for details) and with the ACPI
> > mechanism using the e1000e driver on the Toshiba Portege R500 (the patch still
> > requires some work to be shown in public ;-)).
> 
> The e1000e patch is now in a better shape IMO, at least it worked for me in all
> conditions I tested it, so it is appended below.

This one was actually buggy, as it triggered a netdevice.h BUG_ON() during
resume from suspend to RAM with network cable attached.

Appended is a new version that doesn't have this problem and enables runtime PM
in _probe(), so the device can be put into the low power state if _open()
hasn't been called yet (or after _close()).

Rafael

---
From: Rafael J. Wysocki <rjw <at> sisk.pl>
Subject: PCI / PM / e1000e: Add basic runtime PM support (rev. 2)
(Continue reading)

Permalink | Reply | 0 comments |
headers
Rafael J. Wysocki | 1 Jan 2010 20:06
Picon
Gravatar

[PATCH 12/12] PM / r8169: Add simplified run-time PM support (rev. 2)

On Sunday 27 December 2009, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw <at> sisk.pl>
> 
> Use the PCI run-time power management framework to add simplified
> run-time PM support to the r8169 driver.  Namely, make the driver
> suspend the device when the link is off and set it up for generating
> wake-up event after the link has been detected again.
> 
> Signed-off-by: Rafael J. Wysocki <rjw <at> sisk.pl>

Appended is an improved (in my opinion) version of this patch that enables
the runtime PM in _init(), so that the device can be put into the low power
state before _open() or after _close().

Rafael

---
From: Rafael J. Wysocki <rjw <at> sisk.pl>
Subject: PCI / PM / r8169: Add simplified run-time PM support (rev. 2)

Use the PCI run-time power management framework to add simplified
run-time PM support to the r8169 driver.  Namely, make the driver
suspend the device when the link is off and set it up for generating
wake-up event after the link has been detected again.

Signed-off-by: Rafael J. Wysocki <rjw <at> sisk.pl>
---
 drivers/net/r8169.c |  151 ++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 124 insertions(+), 27 deletions(-)
(Continue reading)

Permalink | Reply | 0 comments |
headers
Andreas Mohr | 1 Jan 2010 21:12
Picon
Favicon

Re: [PATCH] PCI / PM: Use per-device D3 delays

Hi,

On Fri, Jan 01, 2010 at 07:55:27PM +0100, Rafael J. Wysocki wrote:
> On Friday 01 January 2010, Andreas Mohr wrote:
> > Hi,
> > 
> > While the bug report mentions "So it's just quirky hardware.",
> > the implementation of your patch makes it seem like this delay attribute is
> > totally "norm"al behaviour - I'm missing some more aggressive wording.
> 
> That's because it works both ways (please look at the changelog).

Ah, ok.

> I know of a few devices that don't need the PCI-prescribed 10 ms wait when
> going from D3 to D0 and their drivers may use the d3_delay field to actually
> set a _shorter_ delay.

Then why is the value lower-bounded by pci_pm_d3_delay
(which, puzzlingly, was initialized to PCI_PM_D3_WAIT and thus 10
before, which the patch now removes!), in pci_dev_d3_sleep()?
(and pci_pm_d3_delay is being quirked in drivers/pci/quirks.c only,
to 120)

Confused,

Andreas Mohr
Permalink | Reply | 0 comments |
headers
Rafael J. Wysocki | 1 Jan 2010 22:42
Picon
Gravatar

Re: [PATCH] PCI / PM: Use per-device D3 delays

On Friday 01 January 2010, Andreas Mohr wrote:
> Hi,
> 
> On Fri, Jan 01, 2010 at 07:55:27PM +0100, Rafael J. Wysocki wrote:
> > On Friday 01 January 2010, Andreas Mohr wrote:
> > > Hi,
> > > 
> > > While the bug report mentions "So it's just quirky hardware.",
> > > the implementation of your patch makes it seem like this delay attribute is
> > > totally "norm"al behaviour - I'm missing some more aggressive wording.
> > 
> > That's because it works both ways (please look at the changelog).
> 
> Ah, ok.
> 
> > I know of a few devices that don't need the PCI-prescribed 10 ms wait when
> > going from D3 to D0 and their drivers may use the d3_delay field to actually
> > set a _shorter_ delay.
> 
> Then why is the value lower-bounded by pci_pm_d3_delay
> (which, puzzlingly, was initialized to PCI_PM_D3_WAIT and thus 10
> before, which the patch now removes!),

That's because dev->d3_delay is initialized to PCI_PM_D3_WAIT for all devices.

> in pci_dev_d3_sleep()? (and pci_pm_d3_delay is being quirked in
> drivers/pci/quirks.c only, to 120)

Exactly because pci_pm_d3_delay is only necessary for some quirky chipsets
that require longer delays for _all_ devices (note that this cannot be handled
(Continue reading)

Permalink | Reply | 0 comments |
headers
Rafael J. Wysocki | 1 Jan 2010 22:51
Picon
Gravatar

[PATCH][RFC] e1000e: Add basic runtime PM support (rev. 3) (was: [PATCH 0/12] PCI run-time PM support (rev. 2))

On Friday 01 January 2010, Rafael J. Wysocki wrote:
> On Friday 01 January 2010, Rafael J. Wysocki wrote:
> > On Sunday 27 December 2009, Rafael J. Wysocki wrote:
> > > Hi,
> > > 
> > > The following (updated) series of patches provides preliminary run-time power
> > > management support for PCI devices through ACPI and/or the native PCIe PME.
> > > 
> > > Some patches have been modified since the previous iteration, one patch has
> > > been merged and there's one more.
> > > 
> > > I've tested this patchset with the native PCIe PME mechanism using the r8169
> > > driver on the MSI Wind U-100 (see the last patch for details) and with the ACPI
> > > mechanism using the e1000e driver on the Toshiba Portege R500 (the patch still
> > > requires some work to be shown in public ;-)).
> > 
> > The e1000e patch is now in a better shape IMO, at least it worked for me in all
> > conditions I tested it, so it is appended below.
> 
> This one was actually buggy, as it triggered a netdevice.h BUG_ON() during
> resume from suspend to RAM with network cable attached.
> 
> Appended is a new version that doesn't have this problem and enables runtime PM
> in _probe(), so the device can be put into the low power state if _open()
> hasn't been called yet (or after _close()).

This one also has a few problems (triggers a warning about an already freed
interrupt being freed again during suspend to RAM with the adapter suspended at
run time if it is maganed by NetworkManager and doesn't put the adapter into
the low power state if the link is not present from the start), so appended is
(Continue reading)

Permalink | Reply | 0 comments |
headers
Andreas Mohr | 1 Jan 2010 22:56
Picon
Favicon

Re: [PATCH] PCI / PM: Use per-device D3 delays

On Fri, Jan 01, 2010 at 10:42:28PM +0100, Rafael J. Wysocki wrote:
> On Friday 01 January 2010, Andreas Mohr wrote:
> > Then why is the value lower-bounded by pci_pm_d3_delay
> > (which, puzzlingly, was initialized to PCI_PM_D3_WAIT and thus 10
> > before, which the patch now removes!),
> 
> That's because dev->d3_delay is initialized to PCI_PM_D3_WAIT for all devices.

Doh, I should have viewn the altered mechanism correctly, sorry.
Seems quite correct and useful after all.

Andreas Mohr
Permalink | Reply | 0 comments |
headers
Alan Jenkins | 2 Jan 2010 16:03

s2disk hang update

Hi,

I've been suffering from s2disk hangs again.  This time, the hangs
were always before the hibernation image was written out.

They're still frustratingly random.  I just started trying to work out
whether doubling PAGES_FOR_IO makes them go away, but they went away
on their own again.

I did manage to capture a backtrace with debug info though.  Here it
is for 2.6.33-rc2.  (It has also happened on rc1).  I was able to get
the line numbers (using gdb, e.g.  "info line
*stop_machine_create+0x27"), having built the kernel with debug info.

[top of trace lost due to screen height]
? sync_page	(filemap.c:183)
? wait_on_page_bit	(filemap.c:506)
? wake_bit_function	(wait.c:174)
? shrink_page_list	(vmscan.c:696)
? __delayacct_blkio_end	(delayacct.c:94)
? finish_wait	(list.h:142)
? congestion_wait	(backing-dev.c:761)
? shrink_inactive_list	(vmscan.c:1193)
? scsi_request_fn	(spinlock.h:306)
? blk_run_queue	(blk-core.c:434)
? shrink_zone	(vmscan.c:1484)
? do_try_to_free_pages	(vmscan.c:1684)
? try_to_free_pages	(vmscan.c:1848)
? isolate_pages_global	(vmscan.c:980)
? __alloc_pages_nodemask	(page_alloc.c:1702)
(Continue reading)

Permalink | Reply | 0 comments |

Gmane