Hi, Bjorn, On Wed, Sep 15, 2021 at 3:18 AM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > On Sat, Sep 11, 2021 at 05:30:45PM +0800, Huacai Chen wrote: > > My original work is at [1]. > > > > Current default VGA device selection fails in some cases: > > > > - On BMC system, the AST2500 bridge [1a03:1150] does not implement > > PCI_BRIDGE_CTL_VGA [1]. This is perfectly legal but means the > > legacy VGA resources won't reach downstream devices unless they're > > included in the usual bridge windows. > > > > - vga_arb_select_default_device() will set a device below such a > > bridge as the default VGA device as long as it has PCI_COMMAND_IO > > and PCI_COMMAND_MEMORY enabled. > > > > - vga_arbiter_add_pci_device() is called for every VGA device, > > either at boot-time or at hot-add time, and it will also set the > > device as the default VGA device, but ONLY if all bridges leading > > to it implement PCI_BRIDGE_CTL_VGA. > > > > - This difference between vga_arb_select_default_device() and > > vga_arbiter_add_pci_device() means that a device below an AST2500 > > or similar bridge can only be set as the default if it is > > enumerated before vga_arb_device_init(). > > > > - On ACPI-based systems, PCI devices are enumerated by acpi_init(), > > which runs before vga_arb_device_init(). > > > > - On non-ACPI systems, like on MIPS system, they are enumerated by > > pcibios_init(), which typically runs *after* > > vga_arb_device_init(). > > > > So I made vga_arb_update_default_device() to replace the current vga_ > > arb_select_default_device(), which will be call from vga_arbiter_add_ > > pci_device(), set the default device even if it does not own the VGA > > resources because an upstream bridge doesn't implement PCI_BRIDGE_CTL_ > > VGA. And the default VGA device is updated if a better one is found > > (device with legacy resources enabled is better, device owns the > > firmware framebuffer is even better). > > > > Bjorn do some rework and extension in V2. It moves the VGA arbiter to > > the PCI subsystem, fixes a few nits, and breaks a few pieces to make > > the main patch a little smaller. > > > > V3 rewrite the commit log of the last patch (which is also summarized > > by Bjorn). > > > > V4 split the last patch to two steps. > > > > V5 split big patches again and sort the patches. > > Not sure if I'm missing something, or if this is an interim version > and you're working on a v6. > > From https://lore.kernel.org/r/20210909175926.GA996660@bjorn-Precision-5520, > I was looking for: > > BUT as I mentioned, I want the very first patch to be the very > simple 2-line change to vga_arb_update_default_device() that actually > fixes your problem. > > That doesn't seem to be what we have here. I try my best to split patches. But it seems very difficult to achieve the goal of "simple 2-line change" because I cannot break a single functional change. E.g., I think I can try to split Patch-5 again, but Patch-6 seems impossible. Huacai > > > All comments welcome! > > > > [1] https://lore.kernel.org/dri-devel/20210705100503.1120643-1-chenhuacai@xxxxxxxxxxx/ > > > > Bjorn Helgaas (4): > > PCI/VGA: Move vgaarb to drivers/pci > > PCI/VGA: Remove empty vga_arb_device_card_gone() > > PCI/VGA: Use unsigned format string to print lock counts > > PCI/VGA: Replace full MIT license text with SPDX identifier > > > > Huacai Chen (7): > > PCI/VGA: Prefer vga_default_device() > > PCI/VGA: Move vga_arb_integrated_gpu() earlier in file > > PCI/VGA: Split out vga_arb_update_default_device() > > PCI/VGA: Update default VGA device if a better one found > > PCI/VGA: Update default VGA device again for X86/IA64 > > PCI/VGA: Remove vga_arb_select_default_device() > > PCI/VGA: Log bridge control messages when adding devices > > > > Signed-off-by: Huacai Chen <chenhuacai@xxxxxxxxxxx> > > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > --- > > drivers/gpu/vga/Kconfig | 19 --- > > drivers/gpu/vga/Makefile | 1 - > > drivers/pci/Kconfig | 19 +++ > > drivers/pci/Makefile | 1 + > > drivers/{gpu/vga => pci}/vgaarb.c | 269 ++++++++++++------------------ > > 5 files changed, 126 insertions(+), 183 deletions(-) > > rename drivers/{gpu/vga => pci}/vgaarb.c (90%) > > -- > > 2.27.0 > >
