From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E6B60C4338F for ; Sat, 24 Jul 2021 00:10:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C12F860EAF for ; Sat, 24 Jul 2021 00:10:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233064AbhGWXaM (ORCPT ); Fri, 23 Jul 2021 19:30:12 -0400 Received: from mail.kernel.org ([198.145.29.99]:41918 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233059AbhGWXaM (ORCPT ); Fri, 23 Jul 2021 19:30:12 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id A70DE60E9C; Sat, 24 Jul 2021 00:10:44 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1627085445; bh=nmR91NX8dtR/WS09amQLZqUt+/m8Wb7iJ02RfzJAtFc=; h=Date:From:To:Cc:Subject:In-Reply-To:From; b=JsJj+8wel8HfmmDGkGH3fpGS/8TfmHsYFyw54/ryOZjI80oygMYExuOLRwbEftaPq QREazNjDblLVnX5fQNEJ/lB8PpkKuOZB3DjXAtPlssbtBsmHQ78NtUr0X5/JhR/8h2 jBlwVf0+EMncCVIiol3MXWLSkOMghkjHVkP35MOdYKQY0bcOyy/UrpbvMRaJBQfr3K KK2MRIY+386tVRBwrupXgB7LCNQC0VxTn1V042gIVe+LvKOutZpZnTM1AEINYgqh7k akCDg0bD9ClRn5Jws5KIaZCLPAbLm3SIBaOAjUgWD+mOI558RKbOMfi8eGUQCd19WL kNbTCddP5djdg== Date: Fri, 23 Jul 2021 19:10:43 -0500 From: Bjorn Helgaas To: Huacai Chen Cc: Huacai Chen , David Airlie , dri-devel@lists.freedesktop.org, linux-pci@vger.kernel.org, Bjorn Helgaas , Xuefeng Li , Christoph Hellwig , Daniel Vetter Subject: Re: [PATCH v2 0/9] PCI/VGA: Rework default VGA device selection Message-ID: <20210724001043.GA448782@bjorn-Precision-5520> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Fri, Jul 23, 2021 at 05:53:36PM +0800, Huacai Chen wrote: > Hi, Bjorn, > > On Fri, Jul 23, 2021 at 5:29 AM Bjorn Helgaas wrote: > > > > From: Bjorn Helgaas > > > > This is a little bit of rework and extension of Huacai's nice work at [1]. > > > > It moves the VGA arbiter to the PCI subsystem, fixes a few nits, and breaks > > a few pieces off Huacai's patch to make the main patch a little smaller. > > > > That last patch is still not very small, and it needs a commit log, as I > > mentioned at [2]. > > > > All comments welcome! > > > > [1] https://lore.kernel.org/dri-devel/20210705100503.1120643-1-chenhuacai@loongson.cn/ > > [2] https://lore.kernel.org/r/20210720221923.GA43331@bjorn-Precision-5520 > Thank you for your splitting. Your two questions are answered in the following. > > (1) explain why your initcall ordering is unusual. > The original problem happens on MIPS. vga_arb_device_init() and > pcibios_init() are both wrapped by subsys_initcall(). The order of > functions in the same level depends on the Makefile. > > TOP level Makefile: > drivers-y := drivers/ sound/ > .... > include arch/$(SRCARCH)/Makefile > > drivers/Makefile: > obj-$(CONFIG_ACPI) += acpi/ > .... > obj-y += gpu/ > > arch/mips/Makefile: > drivers-$(CONFIG_PCI) += arch/mips/pci/ > > This makes pcibios_init() in arch/mips/pci/ placed after > vga_arb_device_init() in drivers/gpu. ACPI-based systems have no > problems because acpi_init() in drivers/acpi is placed before > vga_arb_device_init(). 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? [1] https://lore.kernel.org/r/CAAhV-H4pn53XC7qVvwM792ppkQRnjWpPDwmrhBv8twgQu0eabQ@mail.gmail.com > (2) explain the approach, which IIUC is basically to add the > vga_arb_select_default_device() functionality to > vga_arbiter_add_pci_device(). > vga_arb_select_default_device() has only one chance to be called, we > want to make it be called every time a new vga device is added. So > rename it to vga_arb_update_default_device() and move the callsite to > vga_arbiter_add_pci_device(). > > I think you know all the information which you need now. And you can > reorganize the commit message based on the existing one. As English is > not my first language, the updated commit message written by me may > still not be as good as you want.:) > > Huacai > > > Bjorn Helgaas (4): > > PCI/VGA: Move vgaarb to drivers/pci > > PCI/VGA: Replace full MIT license text with SPDX identifier > > PCI/VGA: Use unsigned format string to print lock counts > > PCI/VGA: Remove empty vga_arb_device_card_gone() > > > > Huacai Chen (5): > > PCI/VGA: Move vga_arb_integrated_gpu() earlier in file > > PCI/VGA: Prefer vga_default_device() > > PCI/VGA: Split out vga_arb_update_default_device() > > PCI/VGA: Log bridge control messages when adding devices > > PCI/VGA: Rework default VGA device selection > > > > 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.25.1 > >