[PATCH v1] PCI/VGA: Move pci_set_vga_state() to vgaarb.c

[PATCH v1] PCI/VGA: Move pci_set_vga_state() to vgaarb.c

[Bjorn Helgaas]

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.0

Re: [PATCH v1] PCI/VGA: Move pci_set_vga_state() to vgaarb.c

[Simon Richter]

Hi,

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

Re: [PATCH v1] PCI/VGA: Move pci_set_vga_state() to vgaarb.c

[Bjorn Helgaas]

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.