On Fri, 14 Jul 2017 12:26:05 +0000 Gabriele Paoloni <gabriele.paoloni@xxxxxxxxxx> wrote: > Hi Ben, Alex > > > -----Original Message----- > > From: Benjamin Herrenschmidt [mailto:benh@xxxxxxxxxxxxxxxxxxx] > > Sent: 13 July 2017 22:21 > > To: Alex Williamson; Bjorn Helgaas > > Cc: Gabriele Paoloni; Daniel Axtens; linux-pci@xxxxxxxxxxxxxxx; > > Liuxinliang (Matthew Liu); Rongrong Zou; Catalin Marinas; Will Deacon; > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; 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? I'd suggest at least looking for an enabled device as fixup_vga() does, perhaps that would even be enough to remove that arch quirk. Thanks, Alex
