All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gabriele Paoloni <gabriele.paoloni@huawei.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Alex Williamson <alex.williamson@redhat.com>,
	Bjorn Helgaas <helgaas@kernel.org>
Cc: David Airlie <airlied@linux.ie>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	Will Deacon <will.deacon@arm.com>,
	"Liuxinliang \(Matthew Liu\)" <z.liuxinliang@hisilicon.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Rongrong Zou <zourongrong@gmail.com>,
	Daniel Vetter <daniel.vetter@intel.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Daniel Axtens <dja@axtens.net>
Subject: RE: [PATCH v4] PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
Date: Fri, 14 Jul 2017 17:03:59 +0000	[thread overview]
Message-ID: <EE11001F9E5DDD47B7634E2F8A612F2E40B39FC4@FRAEML521-MBX.china.huawei.com> (raw)
In-Reply-To: <1500040204.2865.90.camel@kernel.crashing.org>

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==

WARNING: multiple messages have this Message-ID (diff)
From: gabriele.paoloni@huawei.com (Gabriele Paoloni)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4] PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge
Date: Fri, 14 Jul 2017 17:03:59 +0000	[thread overview]
Message-ID: <EE11001F9E5DDD47B7634E2F8A612F2E40B39FC4@FRAEML521-MBX.china.huawei.com> (raw)
In-Reply-To: <1500040204.2865.90.camel@kernel.crashing.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.

  reply	other threads:[~2017-07-14 17:03 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-12  5:08 [PATCH v4] PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge Daniel Axtens
2017-07-12 20:04 ` Bjorn Helgaas
2017-07-12 20:04   ` Bjorn Helgaas
2017-07-13 10:29   ` Gabriele Paoloni
2017-07-13 10:29     ` Gabriele Paoloni
2017-07-13 11:29     ` Bjorn Helgaas
2017-07-13 11:29       ` Bjorn Helgaas
2017-07-13 20:45       ` Benjamin Herrenschmidt
2017-07-13 20:45         ` Benjamin Herrenschmidt
2017-07-14 12:14         ` Gabriele Paoloni
2017-07-14 12:14           ` Gabriele Paoloni
2017-07-13 21:11       ` Alex Williamson
2017-07-13 21:11         ` Alex Williamson
2017-07-13 21:21         ` Benjamin Herrenschmidt
2017-07-13 21:21           ` Benjamin Herrenschmidt
2017-07-14 12:26           ` Gabriele Paoloni
2017-07-14 12:26             ` Gabriele Paoloni
2017-07-14 13:50             ` Benjamin Herrenschmidt
2017-07-14 13:50               ` Benjamin Herrenschmidt
2017-07-14 17:03               ` Gabriele Paoloni [this message]
2017-07-14 17:03                 ` Gabriele Paoloni
2017-07-14 23:54                 ` Benjamin Herrenschmidt
2017-07-14 23:54                   ` Benjamin Herrenschmidt
2017-07-14 14:43             ` Alex Williamson
2017-07-14 14:43               ` Alex Williamson
2017-07-14  1:35   ` Will Deacon
2017-07-14  1:35     ` Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=EE11001F9E5DDD47B7634E2F8A612F2E40B39FC4@FRAEML521-MBX.china.huawei.com \
    --to=gabriele.paoloni@huawei.com \
    --cc=airlied@linux.ie \
    --cc=alex.williamson@redhat.com \
    --cc=benh@kernel.crashing.org \
    --cc=catalin.marinas@arm.com \
    --cc=daniel.vetter@intel.com \
    --cc=dja@axtens.net \
    --cc=helgaas@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=will.deacon@arm.com \
    --cc=z.liuxinliang@hisilicon.com \
    --cc=zourongrong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.