[PATCH v4] PCI: Support hibmc VGA cards behind a misbehaving HiSilicon bridge

[Gabriele Paoloni]

Hi Ben, Alex

> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh at kernel.crashing.org]
> Sent: 13 July 2017 22:21
> To: Alex Williamson; Bjorn Helgaas
> Cc: Gabriele Paoloni; 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 Thu, 2017-07-13 at 15:11 -0600, Alex Williamson wrote:
> > > >       */
> > > > -   if (vga_default == NULL &&
> > > > -       ((vgadev->owns & VGA_RSRC_LEGACY_MASK) ==
> VGA_RSRC_LEGACY_MASK)) {
> > > > +   if (vga_default == NULL) {
> > > >              vgaarb_info(&pdev->dev, "setting as boot VGA
> device\n");
> > > >              vga_set_default_device(pdev);
> > > >      }
> >
> >
> > "Legacy" is the breadcrumb we use to try to pick the same device for
> > default VGA as the system firmware used as the boot VGA.  There can
> be
> > multiple VGA devices in the system, the change above would simply
> make
> > the first discovered device be the default VGA.  That would break
> many,
> > many systems.  If legacy VGA ranges mean nothing on ARM64, then
> follow
> > the powerpc lead and make an arch quirk that simply selects the first
> > enabled VGA device as the default.  VGA routing is part of the PCI
> spec
> > though, so the default of selecting the device with routing enabled
> > makes sense.  Thanks,
> 
> "Legacy" there iirc also means that it has decoding enabled at boot. If
> you pick the first one you may pick a device that doesn't and hasn't
> been initialized by any driver (good old BIOS-initialized text mode).

Right so effectively if there is a legacy dev we should pick that up;
however what about the following code:

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?

Thanks
Gab

> 
> Cheers,
> Ben.