From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: Date: Sat, 28 May 2016 14:27:01 +0200 From: Lukas Wunner To: Peter Wu Cc: Emil Velikov , ML nouveau , ML dri-devel , Dave Airlie , linux-pm@vger.kernel.org, Bjorn Helgaas , Mika Westerberg , linux-pci@vger.kernel.org Subject: Re: [Nouveau] [PATCH 4/4] drm/nouveau/acpi: fix lockup with PCIe runtime PM Message-ID: <20160528122701.GA11464@wunner.de> References: <1464130381-4797-1-git-send-email-peter@lekensteyn.nl> <1464130381-4797-5-git-send-email-peter@lekensteyn.nl> <20160527213123.GB1777@al> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20160527213123.GB1777@al> List-ID: Hi Peter, On Fri, May 27, 2016 at 11:31:23PM +0200, Peter Wu wrote: > On Fri, May 27, 2016 at 02:01:39PM +0100, Emil Velikov wrote: > > On 24 May 2016 at 23:53, Peter Wu wrote: [snip] > > > @@ -273,14 +296,14 @@ static bool nouveau_dsm_detect(void) > > > vga_count++; > > > > > > nouveau_dsm_pci_probe(pdev, &has_mux, &has_optimus, > > > - &has_optimus_flags); > > > + &has_optimus_flags, &has_power_resources); > > > } > > > > > > while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_3D << 8, pdev)) != NULL) { > > > vga_count++; > > > > > > nouveau_dsm_pci_probe(pdev, &has_mux, &has_optimus, > > > - &has_optimus_flags); > > > + &has_optimus_flags, &has_power_resources); > > > } > > > > > This and earlier patch break things in a subtle way. > > > > Namely: upon the second (and any later) call into the > > nouveau_dsm_pci_probe() function, the had_foo flags are reset. Thus > > only the specifics of the _final_ device are being used (at a later > > stage). IMHO one should change that to "_any_ device", which will > > match the original code and the actual intent further down in the > > file. > > The flags are only reset if any of the MUX or Optimus handles are found. > If both are missing, the flags are not overridden. This is from patch 1: > > + /* Does not look like a Nvidia device. */ > + if (!supports_mux && !supports_opt) > + return; > > The reason why later calls override early ones is because some Optimus > laptops have the _DSM method on both the Intel GPU (00:02.0) and the > Nvidia one (01:00.0). Sounds like you may want to check for pdev->vendor == PCI_VENDOR_ID_NVIDIA or export pci_get_dev_by_id() and use that to match for class and vendor. Best regards, Lukas From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukas Wunner Subject: Re: [PATCH 4/4] drm/nouveau/acpi: fix lockup with PCIe runtime PM Date: Sat, 28 May 2016 14:27:01 +0200 Message-ID: <20160528122701.GA11464@wunner.de> References: <1464130381-4797-1-git-send-email-peter@lekensteyn.nl> <1464130381-4797-5-git-send-email-peter@lekensteyn.nl> <20160527213123.GB1777@al> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20160527213123.GB1777@al> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: nouveau-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "Nouveau" To: Peter Wu Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, ML nouveau , Emil Velikov , ML dri-devel , Bjorn Helgaas , linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Dave Airlie , Mika Westerberg List-Id: linux-pm@vger.kernel.org SGkgUGV0ZXIsCgpPbiBGcmksIE1heSAyNywgMjAxNiBhdCAxMTozMToyM1BNICswMjAwLCBQZXRl ciBXdSB3cm90ZToKPiBPbiBGcmksIE1heSAyNywgMjAxNiBhdCAwMjowMTozOVBNICswMTAwLCBF bWlsIFZlbGlrb3Ygd3JvdGU6Cj4gPiBPbiAyNCBNYXkgMjAxNiBhdCAyMzo1MywgUGV0ZXIgV3Ug PHBldGVyQGxla2Vuc3RleW4ubmw+IHdyb3RlOgpbc25pcF0KPiA+ID4gQEAgLTI3MywxNCArMjk2 LDE0IEBAIHN0YXRpYyBib29sIG5vdXZlYXVfZHNtX2RldGVjdCh2b2lkKQo+ID4gPiAgICAgICAg ICAgICAgICAgdmdhX2NvdW50Kys7Cj4gPiA+Cj4gPiA+ICAgICAgICAgICAgICAgICBub3V2ZWF1 X2RzbV9wY2lfcHJvYmUocGRldiwgJmhhc19tdXgsICZoYXNfb3B0aW11cywKPiA+ID4gLSAgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAmaGFzX29wdGltdXNfZmxhZ3MpOwo+ID4g PiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICZoYXNfb3B0aW11c19mbGFn cywgJmhhc19wb3dlcl9yZXNvdXJjZXMpOwo+ID4gPiAgICAgICAgIH0KPiA+ID4KPiA+ID4gICAg ICAgICB3aGlsZSAoKHBkZXYgPSBwY2lfZ2V0X2NsYXNzKFBDSV9DTEFTU19ESVNQTEFZXzNEIDw8 IDgsIHBkZXYpKSAhPSBOVUxMKSB7Cj4gPiA+ICAgICAgICAgICAgICAgICB2Z2FfY291bnQrKzsK PiA+ID4KPiA+ID4gICAgICAgICAgICAgICAgIG5vdXZlYXVfZHNtX3BjaV9wcm9iZShwZGV2LCAm aGFzX211eCwgJmhhc19vcHRpbXVzLAo+ID4gPiAtICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICZoYXNfb3B0aW11c19mbGFncyk7Cj4gPiA+ICsgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAgJmhhc19vcHRpbXVzX2ZsYWdzLCAmaGFzX3Bvd2VyX3Jlc291cmNl cyk7Cj4gPiA+ICAgICAgICAgfQo+ID4gPgo+ID4gVGhpcyBhbmQgZWFybGllciBwYXRjaCBicmVh ayB0aGluZ3MgaW4gYSBzdWJ0bGUgd2F5Lgo+ID4gCj4gPiBOYW1lbHk6IHVwb24gdGhlIHNlY29u ZCAoYW5kIGFueSBsYXRlcikgY2FsbCBpbnRvIHRoZQo+ID4gbm91dmVhdV9kc21fcGNpX3Byb2Jl KCkgZnVuY3Rpb24sIHRoZSBoYWRfZm9vIGZsYWdzIGFyZSByZXNldC4gVGh1cwo+ID4gb25seSB0 aGUgc3BlY2lmaWNzIG9mIHRoZSBfZmluYWxfIGRldmljZSBhcmUgYmVpbmcgdXNlZCAoYXQgYSBs YXRlcgo+ID4gc3RhZ2UpLiBJTUhPIG9uZSBzaG91bGQgY2hhbmdlIHRoYXQgdG8gIl9hbnlfIGRl dmljZSIsIHdoaWNoIHdpbGwKPiA+IG1hdGNoIHRoZSBvcmlnaW5hbCBjb2RlIGFuZCB0aGUgYWN0 dWFsIGludGVudCBmdXJ0aGVyIGRvd24gaW4gdGhlCj4gPiBmaWxlLgo+IAo+IFRoZSBmbGFncyBh cmUgb25seSByZXNldCBpZiBhbnkgb2YgdGhlIE1VWCBvciBPcHRpbXVzIGhhbmRsZXMgYXJlIGZv dW5kLgo+IElmIGJvdGggYXJlIG1pc3NpbmcsIHRoZSBmbGFncyBhcmUgbm90IG92ZXJyaWRkZW4u IFRoaXMgaXMgZnJvbSBwYXRjaCAxOgo+IAo+ICsgICAgICAgLyogRG9lcyBub3QgbG9vayBsaWtl IGEgTnZpZGlhIGRldmljZS4gKi8KPiArICAgICAgIGlmICghc3VwcG9ydHNfbXV4ICYmICFzdXBw b3J0c19vcHQpCj4gKyAgICAgICAgICAgICAgIHJldHVybjsKPiAKPiBUaGUgcmVhc29uIHdoeSBs YXRlciBjYWxscyBvdmVycmlkZSBlYXJseSBvbmVzIGlzIGJlY2F1c2Ugc29tZSBPcHRpbXVzCj4g bGFwdG9wcyBoYXZlIHRoZSBfRFNNIG1ldGhvZCBvbiBib3RoIHRoZSBJbnRlbCBHUFUgKDAwOjAy LjApIGFuZCB0aGUKPiBOdmlkaWEgb25lICgwMTowMC4wKS4KClNvdW5kcyBsaWtlIHlvdSBtYXkg d2FudCB0byBjaGVjayBmb3IgcGRldi0+dmVuZG9yID09IFBDSV9WRU5ET1JfSURfTlZJRElBCm9y IGV4cG9ydCBwY2lfZ2V0X2Rldl9ieV9pZCgpIGFuZCB1c2UgdGhhdCB0byBtYXRjaCBmb3IgY2xh c3MgYW5kIHZlbmRvci4KCkJlc3QgcmVnYXJkcywKCkx1a2FzCl9fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fCk5vdXZlYXUgbWFpbGluZyBsaXN0Ck5vdXZlYXVA bGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxt YW4vbGlzdGluZm8vbm91dmVhdQo=