On Mon, Apr 18, 2016 at 10:04:20AM +0300, Imre Deak wrote: > During system resume we depended on pci_enable_device() also putting the > device into PCI D0 state. This won't work if the PCI device was already > enabled but still in D3 state. This is because pci_enable_device() is > refcounted and will not change the HW state if called with a non-zero > refcount. Leaving the device in D3 will make all subsequent device > accesses fail. > > This didn't cause a problem most of the time, since we resumed with an > enable refcount of 0. But it fails at least after module reload because > after that we also happen to leak a PCI device enable reference: During > probing we call drm_get_pci_dev() which will enable the PCI device, but > during device removal drm_put_dev() won't disable it. This is a bug of > its own in DRM core, but without much harm as it only leaves the PCI > device enabled. Fixing it is also a bit more involved, due to DRM > mid-layering and because it affects non-i915 drivers too. The fix in > this patch is valid regardless of the problem in DRM core. > > CC: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > CC: stable@xxxxxxxxxxxxxxx > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_drv.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index d550ae2..7eaa93e 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -803,7 +803,7 @@ static int i915_drm_resume(struct drm_device *dev) > static int i915_drm_resume_early(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > - int ret = 0; > + int ret; > > /* > * We have a resume ordering issue with the snd-hda driver also > @@ -814,6 +814,13 @@ static int i915_drm_resume_early(struct drm_device *dev) > * FIXME: This should be solved with a special hdmi sink device or > * similar so that power domains can be employed. > */ > + > + ret = pci_set_power_state(dev->pdev, PCI_D0); > + if (ret) { > + DRM_ERROR("failed to set PCI D0 power state (%d)\n", ret); > + goto out; > + } Hmm. Doesn't this already happen from pci bus resume_noirq hook? > + > if (pci_enable_device(dev->pdev)) { > ret = -EIO; > goto out; > -- > 2.5.0 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx