Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753995AbbBZSux (ORCPT ); Thu, 26 Feb 2015 13:50:53 -0500 Received: from mga14.intel.com ([192.55.52.115]:64695 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752748AbbBZSuw (ORCPT ); Thu, 26 Feb 2015 13:50:52 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,654,1418112000"; d="scan'208,223";a="683954864" Message-ID: <1424976648.17078.1.camel@intel.com> Subject: Re: [PATCH] drm/i915: fix failure to power off after hibernate From: Imre Deak Reply-To: imre.deak@intel.com To: =?ISO-8859-1?Q?Bj=F8rn?= Mork Cc: linux-kernel@vger.kernel.org, Daniel Vetter , Jani Nikula , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, stable@vger.kernel.org, Ville =?ISO-8859-1?Q?Syrj=E4l=E4?= Date: Thu, 26 Feb 2015 20:50:48 +0200 In-Reply-To: <87bnkhqan8.fsf@nemi.mork.no> References: <87bnkjcqjt.fsf@nemi.mork.no> <1424789904-26699-1-git-send-email-bjorn@mork.no> <1424794340.15554.3.camel@intel.com> <878ufnjfrr.fsf@nemi.mork.no> <1424889214.5991.4.camel@intel.com> <87bnkhqan8.fsf@nemi.mork.no> Organization: Intel Content-Type: multipart/mixed; boundary="=-fCz18JHchsK1tZeBD0WI" X-Mailer: Evolution 3.12.7-0ubuntu1 Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9096 Lines: 161 --=-fCz18JHchsK1tZeBD0WI Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit On to, 2015-02-26 at 10:34 +0100, Bjørn Mork wrote: > Imre Deak writes: > > >> That patch fixes the problem, with only pci_set_power_state commented > >> out. Do you still want me to try with pci_disable_device() commented > >> out as well? > > > > No, but it would help if you could still try the two attached patch > > separately, without any of the previous workarounds. Based on the > > result, we'll follow up with a fix that adds for your specific platform > > either of the attached workarounds or simply avoids putting the device > > into D3 (corresponding to the patch you already tried). > > None of those patches made any difference. The laptop still hangs at > power-off. > > Not really surprising, is it? Previous testing shows that the hang > occurs at the pci_set_power_state(drm_dev->pdev, PCI_D3hot) call in the > poweroff_late hook. It is hard to see how replacing it by an attempt to > set D3cold, or adding any call after this point, could possibly change > anything. The system is stil hanging at the pci_set_power_state() call. Judging from the blinking LED, I assume that it's not pci_set_power_state() that hangs the machine, but the hang happens in BIOS code. > The generic pci-driver code will put the i915 device into PCI_D3hot for > you, won't it? Why do you need to duplicate that in the driver, > *knowing* that doing so breaks (at least some) systems? Letting the pci core put the device into D3 wouldn't get rid of the problem. It's putting the device into D3 in the first place what causes it. > I honestly don't think this "let's try some random code" is the proper > way to fix this bug (or any other bug for that matter). You need to > start understanding the code you write, and the first step is by > actually explaining the changes you make. We have a good understanding about the issue: the BIOS on your platform does something unexpected behind the back of the driver/kernel. In that sense the patches were not random, for instance the first one is based on an existing workaround for an issue that resembles quite a lot yours, see pci_pm_poweroff_noirq(). > I also believe that you completely miss the fact that this bug has > survived a full release cycle before you became aware of it, and the > implications this has wrt other affected systems/users. I assume you > understand that my system isn't one-of-a-kind, This means that there are > other affected users with identical/similar systems. Now, if none of > those users reported the bug to you (we all know why: Linux kernel > development is currently limited by the available testing resources, NOT > by the available developer resources), then how do you know that there > aren't a number of other systems affected as well? > > Let me answer that for you: You don't. > > Which is why you must explain the mechanism triggering the bug, proving > that it is a chipset/system specific issue. Because that's the only way > you will *know* that you have solved the problem not only for me, but for > all affected users. > > IMHO, the only safe and sane solution at the moment is the revert patch > I posted. It's a simple fix, reverting back to the *known* working > state before this regression was introduced. > > Then you can start over from there, trying to implement this properly. The current way is the proper one that we want for the generic case. The issue on your platform is the exception, so working around that is a sensible choice. Attached is the proposed fix for this issue. --Imre --=-fCz18JHchsK1tZeBD0WI Content-Disposition: attachment; filename="0001-drm-i915-gm45-work-around-hang-during-hibernation.patch" Content-Transfer-Encoding: base64 Content-Type: text/x-patch; name="0001-drm-i915-gm45-work-around-hang-during-hibernation.patch"; charset="UTF-8" RnJvbSA1YzIzNjU3YmMxNjhkYjEyYTFiYTEwMGFiNjVmYWJkMzA1Yzg5YzhhIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBJbXJlIERlYWsgPGltcmUuZGVha0BpbnRlbC5jb20+CkRhdGU6 IFRodSwgMjYgRmViIDIwMTUgMTg6Mzg6NTMgKzAyMDAKU3ViamVjdDogW1BBVENIXSBkcm0vaTkx NTogZ200NTogd29yayBhcm91bmQgaGFuZyBkdXJpbmcgaGliZXJuYXRpb24KTUlNRS1WZXJzaW9u OiAxLjAKQ29udGVudC1UeXBlOiB0ZXh0L3BsYWluOyBjaGFyc2V0PVVURi04CkNvbnRlbnQtVHJh bnNmZXItRW5jb2Rpbmc6IDhiaXQKCkJqw7hybiByZXBvcnRlZCB0aGF0IGhpcyBtYWNoaW5lIGhh bmcgZHVyaW5nIGhpYmVybmF0aW9uIGFuZCBldmVudHVhbGx5CmJpc2VjdGVkIHRoZSBwcm9ibGVt IHRvIHRoZSBmb2xsb3dpbmcgY29tbWl0OgoKY29tbWl0IGRhMmJjMWI5ZGIzMzUxYWRkZDI5M2U1 YjgyNzU3ZWZlMWY3N2VkMWQKQXV0aG9yOiBJbXJlIERlYWsgPGltcmUuZGVha0BpbnRlbC5jb20+ CkRhdGU6ICAgVGh1IE9jdCAyMyAxOToyMzoyNiAyMDE0ICswMzAwCgogICAgZHJtL2k5MTU6IGFk ZCBwb3dlcm9mZl9sYXRlIGhhbmRsZXIKClRoZSBwcm9ibGVtIHNlZW1zIHRvIGJlIHRoYXQgYWZ0 ZXIgdGhlIGtlcm5lbCBwdXRzIHRoZSBkZXZpY2UgaW50byBEMwp0aGUgQklPUyBzdGlsbCB0cmll cyB0byBhY2Nlc3MgaXQsIG9yIG90aGVyd2lzZSBhc3N1bWVzIHRoYXQgaXQncyBpbiBEMC4KVGhp cyBpcyBjbGVhcmx5IGJvZ3VzLCBzaW5jZSBBQ1BJIG1hbmRhdGVzIHRoYXQgZGV2aWNlcyBhcmUg cHV0IGludG8gRDMKYnkgdGhlIE9TUE0gaWYgdGhleSBhcmUgbm90IHdha2UtdXAgc291cmNlcy4g SW4gdGhlIGZ1dHVyZSB3ZSB3YW50IHRvCnVuaWZ5IG1vcmUgb2YgdGhlIGRyaXZlcidzIHJ1bnRp bWUgYW5kIHN5c3RlbSBzdXNwZW5kIHBhdGhzLCBmb3IgZXhhbXBsZQpieSBza2lwcGluZyBhbGwg dGhlIHN5c3RlbSBzdXNwZW5kL2hpYmVybmF0aW9uIGhvb2tzIGlmIHRoZSBkZXZpY2UgaXMKcnVu dGltZSBzdXNwZW5kZWQgYWxyZWFkeS4gQWNjb3JkaW5nbHkgZm9yIGFsbCBvdGhlciBwbGF0Zm9y bXMgdGhlIGdvYWwKaXMgc3RpbGwgdG8gcHJvcGVybHkgcG93ZXIgZG93biB0aGUgZGV2aWNlIGR1 cmluZyBoaWJlcm5hdGlvbi4KClJlZmVyZW5jZTogaHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9y Zy9hcmNoaXZlcy9pbnRlbC1nZngvMjAxNS1GZWJydWFyeS8wNjA2MzMuaHRtbApSZXBvcnRlZC1h bmQtYmlzZWN0ZWQtYnk6IEJqw7hybiBNb3JrIDxiam9ybkBtb3JrLm5vPgpTaWduZWQtb2ZmLWJ5 OiBJbXJlIERlYWsgPGltcmUuZGVha0BpbnRlbC5jb20+Ci0tLQogZHJpdmVycy9ncHUvZHJtL2k5 MTUvaTkxNV9kcnYuYyB8IDI3ICsrKysrKysrKysrKysrKysrKysrKystLS0tLQogMSBmaWxlIGNo YW5nZWQsIDIyIGluc2VydGlvbnMoKyksIDUgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvZHJp dmVycy9ncHUvZHJtL2k5MTUvaTkxNV9kcnYuYyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVf ZHJ2LmMKaW5kZXggNGJhZGIyMy4uNjdkMjEyYiAxMDA2NDQKLS0tIGEvZHJpdmVycy9ncHUvZHJt L2k5MTUvaTkxNV9kcnYuYworKysgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2Rydi5jCkBA IC02MzcsNyArNjM3LDcgQEAgc3RhdGljIGludCBpOTE1X2RybV9zdXNwZW5kKHN0cnVjdCBkcm1f ZGV2aWNlICpkZXYpCiAJcmV0dXJuIDA7CiB9CiAKLXN0YXRpYyBpbnQgaTkxNV9kcm1fc3VzcGVu ZF9sYXRlKHN0cnVjdCBkcm1fZGV2aWNlICpkcm1fZGV2KQorc3RhdGljIGludCBpOTE1X2RybV9z dXNwZW5kX2xhdGUoc3RydWN0IGRybV9kZXZpY2UgKmRybV9kZXYsIGJvb2wgaGliZXJuYXRpb24p CiB7CiAJc3RydWN0IGRybV9pOTE1X3ByaXZhdGUgKmRldl9wcml2ID0gZHJtX2Rldi0+ZGV2X3By aXZhdGU7CiAJaW50IHJldDsKQEAgLTY1MSw3ICs2NTEsMTQgQEAgc3RhdGljIGludCBpOTE1X2Ry bV9zdXNwZW5kX2xhdGUoc3RydWN0IGRybV9kZXZpY2UgKmRybV9kZXYpCiAJfQogCiAJcGNpX2Rp c2FibGVfZGV2aWNlKGRybV9kZXYtPnBkZXYpOwotCXBjaV9zZXRfcG93ZXJfc3RhdGUoZHJtX2Rl di0+cGRldiwgUENJX0QzaG90KTsKKwkvKgorCSAqIER1cmluZyBoaWJlcm5hdGlvbiBvbiBzb21l IEdNNDUgcGxhdGZvcm1zIHRoZSBCSU9TIG1heSB0cnkgdG8gYWNjZXNzCisJICogdGhlIGRldmlj ZSBldmVuIHRob3VnaCBpdCdzIGFscmVhZHkgaW4gRDMgYW5kIGhhbmcgdGhlIG1hY2hpbmUuIFNv CisJICogbGVhdmUgdGhlIGRldmljZSBpbiBEMCBvbiB0aG9zZSBwbGF0Zm9ybXMgYW5kIGhvcGUg dGhlIEJJT1Mgd2lsbAorCSAqIHBvd2VyIGRvd24gdGhlIGRldmljZSBwcm9wZXJseS4KKwkgKi8K KwlpZiAoIShoaWJlcm5hdGlvbiAmJiBJU19HTTQ1KGRldl9wcml2KSkpCisJCXBjaV9zZXRfcG93 ZXJfc3RhdGUoZHJtX2Rldi0+cGRldiwgUENJX0QzaG90KTsKIAogCXJldHVybiAwOwogfQpAQCAt Njc3LDcgKzY4NCw3IEBAIGludCBpOTE1X3N1c3BlbmRfbGVnYWN5KHN0cnVjdCBkcm1fZGV2aWNl ICpkZXYsIHBtX21lc3NhZ2VfdCBzdGF0ZSkKIAlpZiAoZXJyb3IpCiAJCXJldHVybiBlcnJvcjsK IAotCXJldHVybiBpOTE1X2RybV9zdXNwZW5kX2xhdGUoZGV2KTsKKwlyZXR1cm4gaTkxNV9kcm1f c3VzcGVuZF9sYXRlKGRldiwgZmFsc2UpOwogfQogCiBzdGF0aWMgaW50IGk5MTVfZHJtX3Jlc3Vt ZShzdHJ1Y3QgZHJtX2RldmljZSAqZGV2KQpAQCAtOTY1LDcgKzk3MiwxNyBAQCBzdGF0aWMgaW50 IGk5MTVfcG1fc3VzcGVuZF9sYXRlKHN0cnVjdCBkZXZpY2UgKmRldikKIAlpZiAoZHJtX2Rldi0+ c3dpdGNoX3Bvd2VyX3N0YXRlID09IERSTV9TV0lUQ0hfUE9XRVJfT0ZGKQogCQlyZXR1cm4gMDsK IAotCXJldHVybiBpOTE1X2RybV9zdXNwZW5kX2xhdGUoZHJtX2Rldik7CisJcmV0dXJuIGk5MTVf ZHJtX3N1c3BlbmRfbGF0ZShkcm1fZGV2LCBmYWxzZSk7Cit9CisKK3N0YXRpYyBpbnQgaTkxNV9w bV9wb3dlcm9mZl9sYXRlKHN0cnVjdCBkZXZpY2UgKmRldikKK3sKKwlzdHJ1Y3QgZHJtX2Rldmlj ZSAqZHJtX2RldiA9IGRldl90b19pOTE1KGRldiktPmRldjsKKworCWlmIChkcm1fZGV2LT5zd2l0 Y2hfcG93ZXJfc3RhdGUgPT0gRFJNX1NXSVRDSF9QT1dFUl9PRkYpCisJCXJldHVybiAwOworCisJ cmV0dXJuIGk5MTVfZHJtX3N1c3BlbmRfbGF0ZShkcm1fZGV2LCB0cnVlKTsKIH0KIAogc3RhdGlj IGludCBpOTE1X3BtX3Jlc3VtZV9lYXJseShzdHJ1Y3QgZGV2aWNlICpkZXYpCkBAIC0xNTM1LDcg KzE1NTIsNyBAQCBzdGF0aWMgY29uc3Qgc3RydWN0IGRldl9wbV9vcHMgaTkxNV9wbV9vcHMgPSB7 CiAJLnRoYXdfZWFybHkgPSBpOTE1X3BtX3Jlc3VtZV9lYXJseSwKIAkudGhhdyA9IGk5MTVfcG1f cmVzdW1lLAogCS5wb3dlcm9mZiA9IGk5MTVfcG1fc3VzcGVuZCwKLQkucG93ZXJvZmZfbGF0ZSA9 IGk5MTVfcG1fc3VzcGVuZF9sYXRlLAorCS5wb3dlcm9mZl9sYXRlID0gaTkxNV9wbV9wb3dlcm9m Zl9sYXRlLAogCS5yZXN0b3JlX2Vhcmx5ID0gaTkxNV9wbV9yZXN1bWVfZWFybHksCiAJLnJlc3Rv cmUgPSBpOTE1X3BtX3Jlc3VtZSwKIAotLSAKMi4xLjAKCg== --=-fCz18JHchsK1tZeBD0WI-- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/