drivers/pci/pci.c | 70 -------------------------------------------- drivers/pci/vgaarb.c | 70 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 3 -- 3 files changed, 70 insertions(+), 73 deletions(-)
pci_set_vga_state() is only used in vgaarb.c. Move it from pci.c to
vgaarb.c, make it static, and remove the declaration from
include/linux/pci.h.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/pci.c | 70 --------------------------------------------
drivers/pci/vgaarb.c | 70 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/pci.h | 3 --
3 files changed, 70 insertions(+), 73 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index f3244630bfd0..50935eef4d54 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -6134,76 +6134,6 @@ int pci_select_bars(struct pci_dev *dev, unsigned long flags)
}
EXPORT_SYMBOL(pci_select_bars);
-/* Some architectures require additional programming to enable VGA */
-static arch_set_vga_state_t arch_set_vga_state;
-
-void __init pci_register_set_vga_state(arch_set_vga_state_t func)
-{
- arch_set_vga_state = func; /* NULL disables */
-}
-
-static int pci_set_vga_state_arch(struct pci_dev *dev, bool decode,
- unsigned int command_bits, u32 flags)
-{
- if (arch_set_vga_state)
- return arch_set_vga_state(dev, decode, command_bits,
- flags);
- return 0;
-}
-
-/**
- * pci_set_vga_state - set VGA decode state on device and parents if requested
- * @dev: the PCI device
- * @decode: true = enable decoding, false = disable decoding
- * @command_bits: PCI_COMMAND_IO and/or PCI_COMMAND_MEMORY
- * @flags: traverse ancestors and change bridges
- * CHANGE_BRIDGE_ONLY / CHANGE_BRIDGE
- */
-int pci_set_vga_state(struct pci_dev *dev, bool decode,
- unsigned int command_bits, u32 flags)
-{
- struct pci_bus *bus;
- struct pci_dev *bridge;
- u16 cmd;
- int rc;
-
- WARN_ON((flags & PCI_VGA_STATE_CHANGE_DECODES) && (command_bits & ~(PCI_COMMAND_IO|PCI_COMMAND_MEMORY)));
-
- /* ARCH specific VGA enables */
- rc = pci_set_vga_state_arch(dev, decode, command_bits, flags);
- if (rc)
- return rc;
-
- if (flags & PCI_VGA_STATE_CHANGE_DECODES) {
- pci_read_config_word(dev, PCI_COMMAND, &cmd);
- if (decode)
- cmd |= command_bits;
- else
- cmd &= ~command_bits;
- pci_write_config_word(dev, PCI_COMMAND, cmd);
- }
-
- if (!(flags & PCI_VGA_STATE_CHANGE_BRIDGE))
- return 0;
-
- bus = dev->bus;
- while (bus) {
- bridge = bus->self;
- if (bridge) {
- pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
- &cmd);
- if (decode)
- cmd |= PCI_BRIDGE_CTL_VGA;
- else
- cmd &= ~PCI_BRIDGE_CTL_VGA;
- pci_write_config_word(bridge, PCI_BRIDGE_CONTROL,
- cmd);
- }
- bus = bus->parent;
- }
- return 0;
-}
-
#ifdef CONFIG_ACPI
bool pci_pr3_present(struct pci_dev *pdev)
{
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index 87143e235033..cdda530521ae 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -207,6 +207,76 @@ static void vga_check_first_use(void)
}
}
+/* Some architectures require additional programming to enable VGA */
+static arch_set_vga_state_t arch_set_vga_state;
+
+void __init pci_register_set_vga_state(arch_set_vga_state_t func)
+{
+ arch_set_vga_state = func; /* NULL disables */
+}
+
+static int pci_set_vga_state_arch(struct pci_dev *dev, bool decode,
+ unsigned int command_bits, u32 flags)
+{
+ if (arch_set_vga_state)
+ return arch_set_vga_state(dev, decode, command_bits,
+ flags);
+ return 0;
+}
+
+/**
+ * pci_set_vga_state - set VGA decode state on device and parents if requested
+ * @dev: the PCI device
+ * @decode: true = enable decoding, false = disable decoding
+ * @command_bits: PCI_COMMAND_IO and/or PCI_COMMAND_MEMORY
+ * @flags: traverse ancestors and change bridges
+ * CHANGE_BRIDGE_ONLY / CHANGE_BRIDGE
+ */
+static int pci_set_vga_state(struct pci_dev *dev, bool decode,
+ unsigned int command_bits, u32 flags)
+{
+ struct pci_bus *bus;
+ struct pci_dev *bridge;
+ u16 cmd;
+ int rc;
+
+ WARN_ON((flags & PCI_VGA_STATE_CHANGE_DECODES) && (command_bits & ~(PCI_COMMAND_IO|PCI_COMMAND_MEMORY)));
+
+ /* ARCH specific VGA enables */
+ rc = pci_set_vga_state_arch(dev, decode, command_bits, flags);
+ if (rc)
+ return rc;
+
+ if (flags & PCI_VGA_STATE_CHANGE_DECODES) {
+ pci_read_config_word(dev, PCI_COMMAND, &cmd);
+ if (decode)
+ cmd |= command_bits;
+ else
+ cmd &= ~command_bits;
+ pci_write_config_word(dev, PCI_COMMAND, cmd);
+ }
+
+ if (!(flags & PCI_VGA_STATE_CHANGE_BRIDGE))
+ return 0;
+
+ bus = dev->bus;
+ while (bus) {
+ bridge = bus->self;
+ if (bridge) {
+ pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
+ &cmd);
+ if (decode)
+ cmd |= PCI_BRIDGE_CTL_VGA;
+ else
+ cmd &= ~PCI_BRIDGE_CTL_VGA;
+ pci_write_config_word(bridge, PCI_BRIDGE_CONTROL,
+ cmd);
+ }
+ bus = bus->parent;
+ }
+ return 0;
+}
+
static struct vga_device *__vga_tryget(struct vga_device *vgadev,
unsigned int rsrc)
{
diff --git a/include/linux/pci.h b/include/linux/pci.h
index edf792a79193..8e16c496606c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1714,9 +1714,6 @@ resource_size_t pcibios_window_alignment(struct pci_bus *bus,
#define PCI_VGA_STATE_CHANGE_BRIDGE (1 << 0)
#define PCI_VGA_STATE_CHANGE_DECODES (1 << 1)
-int pci_set_vga_state(struct pci_dev *pdev, bool decode,
- unsigned int command_bits, u32 flags);
-
/*
* Virtual interrupts allow for more interrupts to be allocated
* than the device has interrupts for. These are not programmed
--
2.51.0Hi,
On 2/21/26 4:49 AM, Bjorn Helgaas wrote:
> pci_set_vga_state() is only used in vgaarb.c. Move it from pci.c to
> vgaarb.c, make it static, and remove the declaration from
> include/linux/pci.h.
pci_register_set_vga_state() would then be exported from vgaarb.c -- is
it confusing to have a function with a "pci_" prefix inside a file where
all others have a "vga_" prefix?
pci_notify() in vgaarb.c is static, so there is obvious precedent for a
"pci_" prefix, but that's not externally visible.
Other than that, I think it's neater overall.
Simon
On Thu, Mar 12, 2026 at 08:39:29AM +0900, Simon Richter wrote: > On 2/21/26 4:49 AM, Bjorn Helgaas wrote: > > > pci_set_vga_state() is only used in vgaarb.c. Move it from pci.c to > > vgaarb.c, make it static, and remove the declaration from > > include/linux/pci.h. > > pci_register_set_vga_state() would then be exported from vgaarb.c -- is it > confusing to have a function with a "pci_" prefix inside a file where all > others have a "vga_" prefix? > > pci_notify() in vgaarb.c is static, so there is obvious precedent for a > "pci_" prefix, but that's not externally visible. Thanks for taking a look! I don't care too much about the "pci_" vs "vga_" prefix. But it *is* a problem to move pci_register_set_vga_state() to vgaarb.c, which is only built when CONFIG_VGA_ARB is enabled, because it's called from arch/x86/kernel/apic/x2apic_uv_x.c, which doesn't depend on CONFIG_VGA_ARB.
© 2016 - 2026 Red Hat, Inc.