From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: From: Gabriele Paoloni To: Benjamin Herrenschmidt , Alex Williamson , Bjorn Helgaas Subject: RE: [PATCH v4] PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge Date: Fri, 14 Jul 2017 17:03:59 +0000 Message-ID: References: <20170712050811.3620-1-dja@axtens.net> <20170712200430.GI14614@bhelgaas-glaptop.roam.corp.google.com> <20170713112938.GI4486@bhelgaas-glaptop.roam.corp.google.com> <20170713151146.53e9644c@w520.home> <1499980882.2865.65.camel@kernel.crashing.org> <1500040204.2865.90.camel@kernel.crashing.org> In-Reply-To: <1500040204.2865.90.camel@kernel.crashing.org> MIME-Version: 1.0 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: David Airlie , "linux-pci@vger.kernel.org" , Will Deacon , "Liuxinliang \(Matthew Liu\)" , Catalin Marinas , Rongrong Zou , Daniel Vetter , "linux-arm-kernel@lists.infradead.org" , Daniel Axtens Content-Type: text/plain; charset="utf-8" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+bjorn=helgaas.com@lists.infradead.org List-ID: SGkgQWxleCwgQmVuDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQmVu amFtaW4gSGVycmVuc2NobWlkdCBbbWFpbHRvOmJlbmhAa2VybmVsLmNyYXNoaW5nLm9yZ10NCj4g U2VudDogMTQgSnVseSAyMDE3IDE0OjUwDQo+IFRvOiBHYWJyaWVsZSBQYW9sb25pOyBBbGV4IFdp bGxpYW1zb247IEJqb3JuIEhlbGdhYXMNCj4gQ2M6IERhbmllbCBBeHRlbnM7IGxpbnV4LXBjaUB2 Z2VyLmtlcm5lbC5vcmc7IExpdXhpbmxpYW5nIChNYXR0aGV3DQo+IExpdSk7IFJvbmdyb25nIFpv dTsgQ2F0YWxpbiBNYXJpbmFzOyBXaWxsIERlYWNvbjsgbGludXgtYXJtLQ0KPiBrZXJuZWxAbGlz dHMuaW5mcmFkZWFkLm9yZzsgRGF2aWQgQWlybGllOyBEYW5pZWwgVmV0dGVyDQo+IFN1YmplY3Q6 IFJlOiBbUEFUQ0ggdjRdIFBDSTogU3VwcG9ydCBoaWJtYyBWR0EgY2FyZHMgYmVoaW5kIGENCj4g bWlzYmVoYXZpbmcgSGlTaWxpY29uIGJyaWRnZQ0KPiANCj4gT24gRnJpLCAyMDE3LTA3LTE0IGF0 IDEyOjI2ICswMDAwLCBHYWJyaWVsZSBQYW9sb25pIHdyb3RlOg0KPiA+IGRpZmYgLS1naXQgYS9k cml2ZXJzL2dwdS92Z2EvdmdhYXJiLmMgYi9kcml2ZXJzL2dwdS92Z2EvdmdhYXJiLmMNCj4gPiBp bmRleCA5MmYxNDUyLi5hYjNhZDlhIDEwMDY0NA0KPiA+IC0tLSBhL2RyaXZlcnMvZ3B1L3ZnYS92 Z2FhcmIuYw0KPiA+ICsrKyBiL2RyaXZlcnMvZ3B1L3ZnYS92Z2FhcmIuYw0KPiA+IEBAIC0xNDI0 LDYgKzE0MjQsMTQgQEAgc3RhdGljIGludCBfX2luaXQgdmdhX2FyYl9kZXZpY2VfaW5pdCh2b2lk KQ0KPiA+DQo+ID4gwqDCoMKgwqDCoMKgwqDCoGxpc3RfZm9yX2VhY2hfZW50cnkodmdhZGV2LCAm dmdhX2xpc3QsIGxpc3QpIHsNCj4gPiDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHN0 cnVjdCBkZXZpY2UgKmRldiA9ICZ2Z2FkZXYtPnBkZXYtPmRldjsNCj4gPiArDQo+ID4gK8KgwqDC oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoC8qIGlmIG5vIGxlZ2FjeSBkZXZpY2UgaGFzIGJlZW4g c2V0IGFzIGRlZmF1bHQgVkdBDQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoCAq IGRldmljZSwganVzdCBwaWNrIHVwIHRoZSBmaXJzdCBvbmUgaW4gdGhlIGxpc3QgKi8NCj4gPiAr wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgaWYgKHZnYV9kZWZhdWx0ID09IE5VTEwpIHsN Cj4gPiArwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHZnYWFy Yl9pbmZvKGRldiwgInNldHRpbmcgYXMgYm9vdCBWR0ENCj4gZGV2aWNlXG4iKTsNCj4gPiArwqDC oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHZnYV9zZXRfZGVmYXVs dF9kZXZpY2UodmdhZGV2LT5wZGV2KTsNCj4gPiArwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC oMKgfQ0KPiA+ICsNCj4gPiDCoCNpZiBkZWZpbmVkKENPTkZJR19YODYpIHx8IGRlZmluZWQoQ09O RklHX0lBNjQpDQo+ID4gwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqAvKg0KPiA+IMKg wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgICogT3ZlcnJpZGUgdmdhX2FyYml0ZXJfYWRk X3BjaV9kZXZpY2UoKSdzIEkvTyBiYXNlZA0KPiBkZXRlY3Rpb24NCj4gPg0KPiA+IEFib3ZlIGFm dGVyIHdlIGhhdmUgZmlsbGVkIHRoZSBsaXN0IG9mIFZHQSBkZXZpY2VzIGJ5IGl0ZXJhdGluZyBv dmVyDQo+IGFsbA0KPiA+IFBDSSBkZXZpY2VzIHdlIGNoZWNrIGlmIG5vIGxlZ2FjeSBvbmUgaGFz IGJlZW4gc2V0IGFzIGRlZmF1bHQgVkdBDQo+IGRldmljZQ0KPiA+IHlldDogdGhlcmVmb3JlIHdl IHNldCB0aGUgZmlyc3QgVkdBIGRldmljZSBpbiB0aGUgbGlzdCBhcyBkZWZhdWx0DQo+IG9uZS4u Lg0KPiA+DQo+ID4gRG8geW91IHRoaW5rIGl0IHdvdWxkIHdvcms/DQo+IA0KPiBJIGhvbmVzdGx5 IGRvbid0IHJlbWVtYmVyIGFsbCBvZiB0aGUgZGV0YWlscyBvZiB0aGUgYXJiaXRlciBidXQganVz dA0KPiBtYWtlIHN1cmUgdGhhdCBpdCB3b24ndCB0aGluayB0aGF0IGRldmljZSBpcyBlbmFibGVk IGZvciB0aGluZ3MgbGlrZQ0KPiBWR0EgZXRjLi4uIGlmIGl0J3MgbWVtb3J5L0lPIGRlY29kZSB3 ZXJlbid0IGVuYWJsZWQuDQo+IA0KDQpJIHRoaW5rIHRoZSBmb2xsb3dpbmcgb25lIHNob3VsZCBt YWtlIHN1cmUgdGhhdCBNRU0vSU8gcmVzcG9uc2UgYXJlDQplbmFibGVkIChhbHNvIGFzIHN1Z2dl c3RlZCBieSBBbGV4KQ0KDQpkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvdmdhL3ZnYWFyYi5jIGIv ZHJpdmVycy9ncHUvdmdhL3ZnYWFyYi5jDQppbmRleCA5MmYxNDUyLi5hOTBjNDhjIDEwMDY0NA0K LS0tIGEvZHJpdmVycy9ncHUvdmdhL3ZnYWFyYi5jDQorKysgYi9kcml2ZXJzL2dwdS92Z2Evdmdh YXJiLmMNCkBAIC0xNDI0LDYgKzE0MjQsMjEgQEAgc3RhdGljIGludCBfX2luaXQgdmdhX2FyYl9k ZXZpY2VfaW5pdCh2b2lkKQ0KIA0KIAlsaXN0X2Zvcl9lYWNoX2VudHJ5KHZnYWRldiwgJnZnYV9s aXN0LCBsaXN0KSB7DQogCQlzdHJ1Y3QgZGV2aWNlICpkZXYgPSAmdmdhZGV2LT5wZGV2LT5kZXY7 DQorDQorCQkvKiBpZiBubyBsZWdhY3kgZGV2aWNlIGhhcyBiZWVuIHNldCBhcyBkZWZhdWx0IFZH QSBkZXZpY2UsDQorCQkgKiBqdXN0cGljayB1cCB0aGUgZmlyc3Qgb25lIGluIHRoZSBsaXN0IGNh cGFibGUgb2YgcmVzcG9uZGluZyB0bw0KKwkJICogbWVtIGFuZCBpbyByZXF1ZXN0cyovDQorCQlp ZiAodmdhX2RlZmF1bHQgPT0gTlVMTCkgew0KKwkJCXUxNiBjbWQ7DQorDQorCQkJcGNpX3JlYWRf Y29uZmlnX3dvcmQodmdhZGV2LT5wZGV2LCBQQ0lfQ09NTUFORCwgJmNtZCk7DQorCQkJaWYgKChj bWQgJiAoUENJX0NPTU1BTkRfSU8gfCBQQ0lfQ09NTUFORF9NRU1PUlkpKSB8fA0KKwkJCQkJIXZn YV9kZWZhdWx0X2RldmljZSgpKSB7DQorCQkJCXZnYV9zZXRfZGVmYXVsdF9kZXZpY2UodmdhZGV2 LT5wZGV2KTsNCisJCQkJdmdhYXJiX2luZm8oZGV2LCAic2V0dGluZyBhcyBib290IFZHQSBkZXZp Y2VcbiIpOw0KKwkJCX0NCisJCX0NCisNCiAjaWYgZGVmaW5lZChDT05GSUdfWDg2KSB8fCBkZWZp bmVkKENPTkZJR19JQTY0KQ0KIAkJLyoNCiAJCSAqIE92ZXJyaWRlIHZnYV9hcmJpdGVyX2FkZF9w Y2lfZGV2aWNlKCkncyBJL08gYmFzZWQgZGV0ZWN0aW9uDQoNCg0KQWxzbyB0aGUgYWJvdmUgb25l IGNvdWxkIGFsbG93IHVzIHRvIGdldCByaWQgb2YgZml4dXBfdmdhIGluIHBvd2VycGMuLi4uDQoN Cg0KPiBJJ2QgcmF0aGVyIHdlIGhhdmUgbm8gZGVmYXVsdCBkZXZpY2UgdW50aWwgYSBkcml2ZXIg YWN0dWFsbHkgcGlja3MgdXANCj4gdGhvdWdoLCBhbmQgdGhlbiwgaWYgd2Ugc3RpbGwgaGF2ZSBu byBkZWZhdWx0LCB1c2UgdGhlIGZpcnN0IGRyaXZlciB0bw0KPiBwaWNrIHVwLg0KDQpXZWxsIGZy b20gbXkgdW5kZXJzdGFuZGluZyB0aGUgUENJIGhvc3QgY29udHJvbGxlciBkcml2ZXIgd2lsbCBl bnVtZXJhdGUNCmFsbCB0aGUgZGV2aWNlcyBpbiB0aGUgUENJIGhpZXJhcmNoeSBhbmQgY2FsbCBw Y2lfZGV2aWNlX2FkZCgpIGZvciBlYWNoIG9mDQp0aGVtLCB0aGF0IGluIHR1cm4gd2lsbCBjYWxs IGRldmljZV9hZGQoKSwgYXQgdGhpcyBzdGFnZSBpZiB0aGVyZSBpcyBhDQpkcml2ZXIgYXZhaWxh YmxlIGZvciB0aGUgZGV2aWNlIHN1Y2ggZHJpdmVyIHdpbGwgcHJvYmUgb3RoZXJ3aXNlIGl0IHdp bGwgbm90Lg0KDQpBcmUgeW91IHN1Z2dlc3RpbmcgdG8gYWRkIHRoZSBjb2RlIGFib3ZlIGluIHBj aV9kZXZpY2VfYWRkKCkgYWZ0ZXIgZGV2aWNlX2FkZCgpDQphbmQgYWZ0ZXIgY2hlY2tpbmcgdGhh dCBhIGRyaXZlciBoYXMgYmVlbiBib3VuZCBmb3Igc3VjaCBkZXY/DQoNClRoYW5rcw0KR2FiDQoN Cg0KPiANCj4gQmVuLg0KDQpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0ta2VybmVsQGxp c3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0 aW5mby9saW51eC1hcm0ta2VybmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: gabriele.paoloni@huawei.com (Gabriele Paoloni) Date: Fri, 14 Jul 2017 17:03:59 +0000 Subject: [PATCH v4] PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge In-Reply-To: <1500040204.2865.90.camel@kernel.crashing.org> References: <20170712050811.3620-1-dja@axtens.net> <20170712200430.GI14614@bhelgaas-glaptop.roam.corp.google.com> <20170713112938.GI4486@bhelgaas-glaptop.roam.corp.google.com> <20170713151146.53e9644c@w520.home> <1499980882.2865.65.camel@kernel.crashing.org> <1500040204.2865.90.camel@kernel.crashing.org> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Alex, Ben > -----Original Message----- > From: Benjamin Herrenschmidt [mailto:benh at kernel.crashing.org] > Sent: 14 July 2017 14:50 > To: Gabriele Paoloni; Alex Williamson; Bjorn Helgaas > Cc: Daniel Axtens; linux-pci at vger.kernel.org; Liuxinliang (Matthew > Liu); Rongrong Zou; Catalin Marinas; Will Deacon; linux-arm- > kernel at lists.infradead.org; David Airlie; Daniel Vetter > Subject: Re: [PATCH v4] PCI: Support hibmc VGA cards behind a > misbehaving HiSilicon bridge > > On Fri, 2017-07-14 at 12:26 +0000, Gabriele Paoloni wrote: > > diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c > > index 92f1452..ab3ad9a 100644 > > --- a/drivers/gpu/vga/vgaarb.c > > +++ b/drivers/gpu/vga/vgaarb.c > > @@ -1424,6 +1424,14 @@ static int __init vga_arb_device_init(void) > > > > ????????list_for_each_entry(vgadev, &vga_list, list) { > > ????????????????struct device *dev = &vgadev->pdev->dev; > > + > > +???????????????/* if no legacy device has been set as default VGA > > +??????????????? * device, just pick up the first one in the list */ > > +???????????????if (vga_default == NULL) { > > +???????????????????????vgaarb_info(dev, "setting as boot VGA > device\n"); > > +???????????????????????vga_set_default_device(vgadev->pdev); > > +???????????????} > > + > > ?#if defined(CONFIG_X86) || defined(CONFIG_IA64) > > ????????????????/* > > ???????????????? * Override vga_arbiter_add_pci_device()'s I/O based > detection > > > > Above after we have filled the list of VGA devices by iterating over > all > > PCI devices we check if no legacy one has been set as default VGA > device > > yet: therefore we set the first VGA device in the list as default > one... > > > > Do you think it would work? > > I honestly don't remember all of the details of the arbiter but just > make sure that it won't think that device is enabled for things like > VGA etc... if it's memory/IO decode weren't enabled. > I think the following one should make sure that MEM/IO response are enabled (also as suggested by Alex) diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c index 92f1452..a90c48c 100644 --- a/drivers/gpu/vga/vgaarb.c +++ b/drivers/gpu/vga/vgaarb.c @@ -1424,6 +1424,21 @@ static int __init vga_arb_device_init(void) list_for_each_entry(vgadev, &vga_list, list) { struct device *dev = &vgadev->pdev->dev; + + /* if no legacy device has been set as default VGA device, + * justpick up the first one in the list capable of responding to + * mem and io requests*/ + if (vga_default == NULL) { + u16 cmd; + + pci_read_config_word(vgadev->pdev, PCI_COMMAND, &cmd); + if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || + !vga_default_device()) { + vga_set_default_device(vgadev->pdev); + vgaarb_info(dev, "setting as boot VGA device\n"); + } + } + #if defined(CONFIG_X86) || defined(CONFIG_IA64) /* * Override vga_arbiter_add_pci_device()'s I/O based detection Also the above one could allow us to get rid of fixup_vga in powerpc.... > I'd rather we have no default device until a driver actually picks up > though, and then, if we still have no default, use the first driver to > pick up. Well from my understanding the PCI host controller driver will enumerate all the devices in the PCI hierarchy and call pci_device_add() for each of them, that in turn will call device_add(), at this stage if there is a driver available for the device such driver will probe otherwise it will not. Are you suggesting to add the code above in pci_device_add() after device_add() and after checking that a driver has been bound for such dev? Thanks Gab > > Ben.