From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/4] drm/i915: Fix system resume if PCI device remained enabled Date: Mon, 18 Apr 2016 11:28:22 +0300 Message-ID: <20160418082822.GY4329@intel.com> References: <1460963062-13211-1-git-send-email-imre.deak@intel.com> <1460963062-13211-3-git-send-email-imre.deak@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id 0D30A6E0CF for ; Mon, 18 Apr 2016 08:28:26 +0000 (UTC) Content-Disposition: inline In-Reply-To: <1460963062-13211-3-git-send-email-imre.deak@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Imre Deak Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gTW9uLCBBcHIgMTgsIDIwMTYgYXQgMTA6MDQ6MjBBTSArMDMwMCwgSW1yZSBEZWFrIHdyb3Rl Ogo+IER1cmluZyBzeXN0ZW0gcmVzdW1lIHdlIGRlcGVuZGVkIG9uIHBjaV9lbmFibGVfZGV2aWNl KCkgYWxzbyBwdXR0aW5nIHRoZQo+IGRldmljZSBpbnRvIFBDSSBEMCBzdGF0ZS4gVGhpcyB3b24n dCB3b3JrIGlmIHRoZSBQQ0kgZGV2aWNlIHdhcyBhbHJlYWR5Cj4gZW5hYmxlZCBidXQgc3RpbGwg aW4gRDMgc3RhdGUuIFRoaXMgaXMgYmVjYXVzZSBwY2lfZW5hYmxlX2RldmljZSgpIGlzCj4gcmVm Y291bnRlZCBhbmQgd2lsbCBub3QgY2hhbmdlIHRoZSBIVyBzdGF0ZSBpZiBjYWxsZWQgd2l0aCBh IG5vbi16ZXJvCj4gcmVmY291bnQuIExlYXZpbmcgdGhlIGRldmljZSBpbiBEMyB3aWxsIG1ha2Ug YWxsIHN1YnNlcXVlbnQgZGV2aWNlCj4gYWNjZXNzZXMgZmFpbC4KPiAKPiBUaGlzIGRpZG4ndCBj YXVzZSBhIHByb2JsZW0gbW9zdCBvZiB0aGUgdGltZSwgc2luY2Ugd2UgcmVzdW1lZCB3aXRoIGFu Cj4gZW5hYmxlIHJlZmNvdW50IG9mIDAuIEJ1dCBpdCBmYWlscyBhdCBsZWFzdCBhZnRlciBtb2R1 bGUgcmVsb2FkIGJlY2F1c2UKPiBhZnRlciB0aGF0IHdlIGFsc28gaGFwcGVuIHRvIGxlYWsgYSBQ Q0kgZGV2aWNlIGVuYWJsZSByZWZlcmVuY2U6IER1cmluZwo+IHByb2Jpbmcgd2UgY2FsbCBkcm1f Z2V0X3BjaV9kZXYoKSB3aGljaCB3aWxsIGVuYWJsZSB0aGUgUENJIGRldmljZSwgYnV0Cj4gZHVy aW5nIGRldmljZSByZW1vdmFsIGRybV9wdXRfZGV2KCkgd29uJ3QgZGlzYWJsZSBpdC4gVGhpcyBp cyBhIGJ1ZyBvZgo+IGl0cyBvd24gaW4gRFJNIGNvcmUsIGJ1dCB3aXRob3V0IG11Y2ggaGFybSBh cyBpdCBvbmx5IGxlYXZlcyB0aGUgUENJCj4gZGV2aWNlIGVuYWJsZWQuIEZpeGluZyBpdCBpcyBh bHNvIGEgYml0IG1vcmUgaW52b2x2ZWQsIGR1ZSB0byBEUk0KPiBtaWQtbGF5ZXJpbmcgYW5kIGJl Y2F1c2UgaXQgYWZmZWN0cyBub24taTkxNSBkcml2ZXJzIHRvby4gVGhlIGZpeCBpbgo+IHRoaXMg cGF0Y2ggaXMgdmFsaWQgcmVnYXJkbGVzcyBvZiB0aGUgcHJvYmxlbSBpbiBEUk0gY29yZS4KPiAK PiBDQzogVmlsbGUgU3lyasOkbMOkIDx2aWxsZS5zeXJqYWxhQGxpbnV4LmludGVsLmNvbT4KPiBD Qzogc3RhYmxlQHZnZXIua2VybmVsLm9yZwo+IFNpZ25lZC1vZmYtYnk6IEltcmUgRGVhayA8aW1y ZS5kZWFrQGludGVsLmNvbT4KPiAtLS0KPiAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9kcnYu YyB8IDkgKysrKysrKystCj4gIDEgZmlsZSBjaGFuZ2VkLCA4IGluc2VydGlvbnMoKyksIDEgZGVs ZXRpb24oLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9kcnYu YyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZHJ2LmMKPiBpbmRleCBkNTUwYWUyLi43ZWFh OTNlIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZHJ2LmMKPiArKysg Yi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2Rydi5jCj4gQEAgLTgwMyw3ICs4MDMsNyBAQCBz dGF0aWMgaW50IGk5MTVfZHJtX3Jlc3VtZShzdHJ1Y3QgZHJtX2RldmljZSAqZGV2KQo+ICBzdGF0 aWMgaW50IGk5MTVfZHJtX3Jlc3VtZV9lYXJseShzdHJ1Y3QgZHJtX2RldmljZSAqZGV2KQo+ICB7 Cj4gIAlzdHJ1Y3QgZHJtX2k5MTVfcHJpdmF0ZSAqZGV2X3ByaXYgPSBkZXYtPmRldl9wcml2YXRl Owo+IC0JaW50IHJldCA9IDA7Cj4gKwlpbnQgcmV0Owo+ICAKPiAgCS8qCj4gIAkgKiBXZSBoYXZl IGEgcmVzdW1lIG9yZGVyaW5nIGlzc3VlIHdpdGggdGhlIHNuZC1oZGEgZHJpdmVyIGFsc28KPiBA QCAtODE0LDYgKzgxNCwxMyBAQCBzdGF0aWMgaW50IGk5MTVfZHJtX3Jlc3VtZV9lYXJseShzdHJ1 Y3QgZHJtX2RldmljZSAqZGV2KQo+ICAJICogRklYTUU6IFRoaXMgc2hvdWxkIGJlIHNvbHZlZCB3 aXRoIGEgc3BlY2lhbCBoZG1pIHNpbmsgZGV2aWNlIG9yCj4gIAkgKiBzaW1pbGFyIHNvIHRoYXQg cG93ZXIgZG9tYWlucyBjYW4gYmUgZW1wbG95ZWQuCj4gIAkgKi8KPiArCj4gKwlyZXQgPSBwY2lf c2V0X3Bvd2VyX3N0YXRlKGRldi0+cGRldiwgUENJX0QwKTsKPiArCWlmIChyZXQpIHsKPiArCQlE Uk1fRVJST1IoImZhaWxlZCB0byBzZXQgUENJIEQwIHBvd2VyIHN0YXRlICglZClcbiIsIHJldCk7 Cj4gKwkJZ290byBvdXQ7Cj4gKwl9CgpIbW0uIERvZXNuJ3QgdGhpcyBhbHJlYWR5IGhhcHBlbiBm cm9tIHBjaSBidXMgcmVzdW1lX25vaXJxIGhvb2s/Cgo+ICsKPiAgCWlmIChwY2lfZW5hYmxlX2Rl dmljZShkZXYtPnBkZXYpKSB7Cj4gIAkJcmV0ID0gLUVJTzsKPiAgCQlnb3RvIG91dDsKPiAtLSAK PiAyLjUuMAoKLS0gClZpbGxlIFN5cmrDpGzDpApJbnRlbCBPVEMKX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJbnRl bC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3Jn L21haWxtYW4vbGlzdGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com ([192.55.52.120]:49479 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751036AbcDRI20 (ORCPT ); Mon, 18 Apr 2016 04:28:26 -0400 Date: Mon, 18 Apr 2016 11:28:22 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Imre Deak Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org Subject: Re: [PATCH 2/4] drm/i915: Fix system resume if PCI device remained enabled Message-ID: <20160418082822.GY4329@intel.com> References: <1460963062-13211-1-git-send-email-imre.deak@intel.com> <1460963062-13211-3-git-send-email-imre.deak@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1460963062-13211-3-git-send-email-imre.deak@intel.com> Sender: stable-owner@vger.kernel.org List-ID: 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� > CC: stable@vger.kernel.org > Signed-off-by: Imre Deak > --- > 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