Hi, Bjorn,
On Fri, Aug 20, 2021 at 5:52 AM Bjorn Helgaas helgaas@kernel.org wrote:
On Mon, Aug 09, 2021 at 01:59:01PM -0500, Bjorn Helgaas wrote:
On Tue, Aug 03, 2021 at 12:06:44PM -0500, Bjorn Helgaas wrote:
On Sat, Jul 24, 2021 at 05:30:02PM +0800, Huacai Chen wrote:
On Sat, Jul 24, 2021 at 8:10 AM Bjorn Helgaas helgaas@kernel.org wrote:
Thanks for the above; that was helpful. To summarize:
On your 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 your MIPS system, they are enumerated by pcibios_init(), which typically runs *after* vga_arb_device_init().
So I think the critical change is actually that you made vga_arb_update_default_device(), which you 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, i.e.,
(vgadev->owns & VGA_RSRC_LEGACY_MASK) != VGA_RSRC_LEGACY_MASK
Does that seem right?
Yes, that's right.
I think that means I screwed up. I somehow had it in my head that the hot-add path would never set the default VGA device. But that is false.
I still think we should move vgaarb.c to drivers/pci/ and get it more tightly integrated into the PCI core.
BUT that's a lot of churn and obscures the simple change that fixes the problem for you. So I think the first step should be the change to vga_arb_update_default_device() so it sets the default device even when the upstream bridge doesn't implement PCI_BRIDGE_CTL_VGA.
That should be a relatively small change, and I think it's better to make the fix before embarking on major restructuring.
To make sure this doesn't get lost: I'm hoping you can separate out and post the small patch to vga_arb_update_default_device().
I can look at the move/restructure stuff later.
What's happening with this? I'm still assuming you can post a small patch to vga_arb_update_default_device() that's suitable for v5.15, Huacai.
Otherwise I'm afraid we won't make any forward progress this cycle.
In my opinion these patches (including the last one) are small enough, so can I update the commit message of the last one and keep the patch content as is and send V3?
Huacai
Bjorn