From: Huacai Chen <chenhuacai@kernel.org>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Huacai Chen <chenhuacai@loongson.cn>,
David Airlie <airlied@linux.ie>,
Maling list - DRI developers <dri-devel@lists.freedesktop.org>,
linux-pci <linux-pci@vger.kernel.org>,
Bjorn Helgaas <bhelgaas@google.com>,
Xuefeng Li <lixuefeng@loongson.cn>,
Christoph Hellwig <hch@infradead.org>,
Daniel Vetter <daniel@ffwll.ch>
Subject: Re: [PATCH v2 0/9] PCI/VGA: Rework default VGA device selection
Date: Fri, 20 Aug 2021 12:07:05 +0800 [thread overview]
Message-ID: <CAAhV-H4qqAaCoi6_kVnwXCMjBqfen5iH583tQ07bxK5P5zpYeQ@mail.gmail.com> (raw)
In-Reply-To: <20210819215240.GA3241693@bjorn-Precision-5520>
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
prev parent reply other threads:[~2021-08-20 4:07 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-22 21:29 [PATCH v2 0/9] PCI/VGA: Rework default VGA device selection Bjorn Helgaas
2021-07-22 21:29 ` [PATCH v2 1/9] PCI/VGA: Move vgaarb to drivers/pci Bjorn Helgaas
2021-07-22 21:38 ` Randy Dunlap
2021-07-22 21:55 ` Bjorn Helgaas
2021-07-22 21:29 ` [PATCH v2 2/9] PCI/VGA: Replace full MIT license text with SPDX identifier Bjorn Helgaas
2021-07-22 21:29 ` [PATCH v2 3/9] PCI/VGA: Use unsigned format string to print lock counts Bjorn Helgaas
2021-07-22 21:29 ` [PATCH v2 4/9] PCI/VGA: Remove empty vga_arb_device_card_gone() Bjorn Helgaas
2021-07-22 21:29 ` [PATCH v2 5/9] PCI/VGA: Move vga_arb_integrated_gpu() earlier in file Bjorn Helgaas
2021-07-22 21:29 ` [PATCH v2 6/9] PCI/VGA: Prefer vga_default_device() Bjorn Helgaas
2021-07-22 21:29 ` [PATCH v2 7/9] PCI/VGA: Split out vga_arb_update_default_device() Bjorn Helgaas
2021-07-22 21:29 ` [PATCH v2 8/9] PCI/VGA: Log bridge control messages when adding devices Bjorn Helgaas
2021-07-22 21:29 ` [PATCH v2 9/9] PCI/VGA: Rework default VGA device selection Bjorn Helgaas
2021-07-23 5:51 ` [PATCH v2 0/9] " Christoph Hellwig
2021-07-23 8:27 ` Daniel Vetter
2021-07-23 22:43 ` Bjorn Helgaas
2021-07-27 9:32 ` Daniel Vetter
2021-07-23 9:53 ` Huacai Chen
2021-07-24 0:10 ` Bjorn Helgaas
2021-07-24 9:30 ` Huacai Chen
2021-08-03 17:06 ` Bjorn Helgaas
2021-08-09 18:59 ` Bjorn Helgaas
2021-08-19 21:52 ` Bjorn Helgaas
2021-08-20 4:07 ` Huacai Chen [this message]
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=CAAhV-H4qqAaCoi6_kVnwXCMjBqfen5iH583tQ07bxK5P5zpYeQ@mail.gmail.com \
--to=chenhuacai@kernel.org \
--cc=airlied@linux.ie \
--cc=bhelgaas@google.com \
--cc=chenhuacai@loongson.cn \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=hch@infradead.org \
--cc=helgaas@kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lixuefeng@loongson.cn \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).