Re: [PATCH v2 09/14] drm/i915: disable/re-enable PCI device around S4 freeze/thaw

[Imre Deak <imre.deak@intel.com>]

On Fri, 2014-10-24 at 10:51 +0200, Daniel Vetter wrote:
> On Thu, Oct 23, 2014 at 07:23:23PM +0300, Imre Deak wrote:
> > We already disable everything during S4 freeze, except the PCI device
> > itself. There is no reason why we couldn't disable that too and doing
> > so allows us to unify these handlers in the next patch with the
> > corresponding S3 suspend/resume handlers.
> > 
> > Signed-off-by: Imre Deak <imre.deak@intel.com>
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> It feels like I've asked you this already, but could this perhaps fix the
> ominous S4 issues we're seeing on hsw+? There should be a few QA reports
> in bugzilla about it.

Yes, it's possible. I still think that if it fixes things then it's just
a workaround, since by the time we disable the PCI device everything
should be quiescent anyway. Or the reinitialization during thaw depends
on an explicit HW reset (which we'll get now with the PCI
disable/enable), but imo this shouldn't be needed either.

Anyway there is:
https://bugzilla.kernel.org/show_bug.cgi?id=59321
I can ask the reporter to test again -nightly and also test with the
patchset reverted. I'll check also fdo for similar bugs and ask for
retesting.

--Imre

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 4524c61..be4cb1d 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -1019,12 +1019,11 @@ static int i915_pm_freeze_late(struct device *dev)
> >  {
> >  	struct pci_dev *pdev = to_pci_dev(dev);
> >  	struct drm_device *drm_dev = pci_get_drvdata(pdev);
> > -	struct drm_i915_private *dev_priv = drm_dev->dev_private;
> >  
> >  	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> >  		return 0;
> >  
> > -	return intel_suspend_complete(dev_priv);
> > +	return i915_drm_suspend_late(drm_dev);
> >  }
> >  
> >  static int i915_pm_thaw_early(struct device *dev)
> > @@ -1035,7 +1034,7 @@ static int i915_pm_thaw_early(struct device *dev)
> >  	if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> >  		return 0;
> >  
> > -	return i915_drm_thaw_early(drm_dev);
> > +	return i915_resume_early(drm_dev);
> >  }
> >  
> >  static int i915_pm_thaw(struct device *dev)
> > -- 
> > 1.8.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx