[PATCH 1/3] agp/intel: remove restore in resume

[PATCH 1/3] agp/intel: remove restore in resume

[Zhenyu Wang]

As early pci resume has already restored config for host
bridge and graphics device, don't need to restore it again,
which might cause problem on some chips, like 845G tested by
Alan Stern.

Cc: Stable Team <stable@kernel.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/char/agp/intel-agp.c |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index c585577..dee0f1f 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -2313,15 +2313,6 @@ static int agp_intel_resume(struct pci_dev *pdev)
 	struct agp_bridge_data *bridge = pci_get_drvdata(pdev);
 	int ret_val;
 
-	pci_restore_state(pdev);
-
-	/* We should restore our graphics device's config space,
-	 * as host bridge (00:00) resumes before graphics device (02:00),
-	 * then our access to its pci space can work right.
-	 */
-	if (intel_private.pcidev)
-		pci_restore_state(intel_private.pcidev);
-
 	if (bridge->driver == &intel_generic_driver)
 		intel_configure();
 	else if (bridge->driver == &intel_850_driver)
-- 
1.6.3.3

[PATCH 2/3] drm/i915: remove restore in resume

[Zhenyu Wang]

Don't need extra restore like for intel_agp.

Cc: Stable Team <stable@kernel.org>
Cc: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index dbe568c..e8cf18d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -97,8 +97,6 @@ static int i915_resume(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int ret = 0;
 
-	pci_set_power_state(dev->pdev, PCI_D0);
-	pci_restore_state(dev->pdev);
 	if (pci_enable_device(dev->pdev))
 		return -1;
 	pci_set_master(dev->pdev);
-- 
1.6.3.3

Re: [PATCH 1/3] agp/intel: remove restore in resume

[Andrew Morton]

On Fri,  4 Sep 2009 09:43:34 +0800
Zhenyu Wang <zhenyuw@linux.intel.com> wrote:

> As early pci resume has already restored config for host
> bridge and graphics device, don't need to restore it again,
> which might cause problem on some chips, like 845G tested by
> Alan Stern.
> 
> Cc: Stable Team <stable@kernel.org>
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>

Why were these patches cc'ed to stable@kernel.org?  There's nothing in
the changelog which explains why the problem which is being fixed is
sufficiently serious to warrant backporting the patch.

Was there a [patch 3/3]?  I didn't receive it.

Re: [PATCH 1/3] agp/intel: remove restore in resume

[Zhenyu Wang]

[-- Attachment #1: Type: text/plain, Size: 1342 bytes --]

On 2009.09.10 13:42:52 -0700, Andrew Morton wrote:
> On Fri,  4 Sep 2009 09:43:34 +0800
> Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> 
> > As early pci resume has already restored config for host
> > bridge and graphics device, don't need to restore it again,
> > which might cause problem on some chips, like 845G tested by
> > Alan Stern.
> > 
> > Cc: Stable Team <stable@kernel.org>
> > Cc: Alan Stern <stern@rowland.harvard.edu>
> > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> 
> Why were these patches cc'ed to stable@kernel.org?  There's nothing in
> the changelog which explains why the problem which is being fixed is
> sufficiently serious to warrant backporting the patch.

Sorry, Andrew. I wasn't awared that an earlier version of this patch
has already been merged, and this patch fixed resume issue on Alan's
845G, which was discussed on linux-pm list, I should have made more
clear note about that. We've also seen other resume failure bugs, which
might be relate to this one. So I think it should be fine for stable.

> 
> Was there a [patch 3/3]?  I didn't receive it.

sorry, my mistake, the 3/3 commit on my tree is not ready to send, 
forget to make git format-patch happy.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH 1/3] agp/intel: remove restore in resume

[Andrew Morton]

On Fri, 11 Sep 2009 09:03:42 +0800 Zhenyu Wang <zhenyuw@linux.intel.com> wrote:

> On 2009.09.10 13:42:52 -0700, Andrew Morton wrote:
> > On Fri,  4 Sep 2009 09:43:34 +0800
> > Zhenyu Wang <zhenyuw@linux.intel.com> wrote:
> > 
> > > As early pci resume has already restored config for host
> > > bridge and graphics device, don't need to restore it again,
> > > which might cause problem on some chips, like 845G tested by
> > > Alan Stern.
> > > 
> > > Cc: Stable Team <stable@kernel.org>
> > > Cc: Alan Stern <stern@rowland.harvard.edu>
> > > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
> > 
> > Why were these patches cc'ed to stable@kernel.org?  There's nothing in
> > the changelog which explains why the problem which is being fixed is
> > sufficiently serious to warrant backporting the patch.
> 
> Sorry, Andrew. I wasn't awared that an earlier version of this patch
> has already been merged, and this patch fixed resume issue on Alan's
> 845G, which was discussed on linux-pm list, I should have made more
> clear note about that. We've also seen other resume failure bugs, which
> might be relate to this one. So I think it should be fine for stable.

We still don't have a changelog for this patch which explains to the
-stable maintainers (and those who follow -stable commits):

a) what bug this patch fixes and

b) why they should merge it - this should be ovbious if a) is provided.

this isn't pointless paperwork - it matters to those who maintain and
use the -stable tree.  Vague references to traffic on the linux-pm list
aren't at all useful.

>From my reading of the above, it appears that Alan was experiencing
<secretbug> and this patch was tested and fixed it, yes?

> > 
> > Was there a [patch 3/3]?  I didn't receive it.
> 
> sorry, my mistake, the 3/3 commit on my tree is not ready to send, 
> forget to make git format-patch happy.

OK.

Re: [PATCH 1/3] agp/intel: remove restore in resume

[Zhenyu Wang]

[-- Attachment #1: Type: text/plain, Size: 1310 bytes --]

On 2009.09.10 18:22:57 -0700, Andrew Morton wrote:
> 
> We still don't have a changelog for this patch which explains to the
> -stable maintainers (and those who follow -stable commits):
> 
> a) what bug this patch fixes and
> 
> b) why they should merge it - this should be ovbious if a) is provided.
> 
> this isn't pointless paperwork - it matters to those who maintain and
> use the -stable tree.  Vague references to traffic on the linux-pm list
> aren't at all useful.

Yes, thanks. I'll try to make it clear the bug it fixes. 

> >From my reading of the above, it appears that Alan was experiencing
> <secretbug> and this patch was tested and fixed it, yes?

secretbug? Alan found resume hang on his 845G machine during his pm test,
and root cause that to extra pci config restore in agp/intel resume function,
which was having a fix to workaround resume order problem for host bridge and
gfx device for pci restore. But now this is handled by early pci resume already,
but the extra restore in resume caused machine hang. So remove them fixed
Alan's hang issue.

Sorry, I might misread on something, "early version merged" seems nonsense.
I'll try to send them again.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]