[PATCH 0/3] Split VGA default selection from VGA arbiter

[PATCH 0/3] Split VGA default selection from VGA arbiter

[Daniel Axtens]

This is approach 3 of my patch series to sort out Xorg
autoconfiguration for the Hibmc card beind a Hisilicon bridge on
arm64.

Approach 1 was a simple quirk for the card+bridge to mark it as
default. This higlighted the fact that the default card was picked by
the arbiter, which assumed legacy resources. The lack of legacy
resources leads to quirks in ppc and concerns in arm land, so a more
generic approach was desired.

Approach 2 allowed platforms to opt in to a class enable hook that
added a card as default if there was no default. This:

 - was possibly racy as ACPI PCI init and vgaarb are both subsys
   initcalls.

 - didn't check to see if a card had a driver.

 - meant that platforms for which the vga arbiter didn't make sense
   still needed it.

This is approach 3. It pulls the default handling out of the arbiter,
into its own file and behind its own Kconfig option. It adds the extra
detection as a late initcall and an enable hook that only operates
after the initcall, so it's not racy. It checks for drivers. It means
people can turn off the vga arbiter. It works sensibly for modules
too.

Patch 1 cleans up the powerpc fixup, as with approach 2.
Patch 2 is the big split.
Patch 3 moves ppc over, as with approach 2.
There is no need for an arm-specific patch this time as the Kconfig option
is on by default.

Regards,
Daniel

Daniel Axtens (3):
  powerpc: simplify and fix VGA default device behaviour
  Split VGA default device handler out of VGA arbiter
  powerpc: replace vga_fixup() with generic code

 arch/ia64/pci/fixup.c            |   2 +-
 arch/powerpc/kernel/pci-common.c |  13 ----
 arch/x86/pci/fixup.c             |   2 +-
 arch/x86/video/fbdev.c           |   2 +-
 drivers/gpu/vga/Kconfig          |  12 +++
 drivers/gpu/vga/Makefile         |   1 +
 drivers/gpu/vga/vga_default.c    | 159 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/vga/vga_switcheroo.c |   2 +-
 drivers/gpu/vga/vgaarb.c         |  41 +---------
 drivers/pci/pci-sysfs.c          |   2 +-
 include/linux/vga_default.h      |  44 +++++++++++
 include/linux/vgaarb.h           |  14 ----
 12 files changed, 224 insertions(+), 70 deletions(-)
 create mode 100644 drivers/gpu/vga/vga_default.c
 create mode 100644 include/linux/vga_default.h

-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 0/3] Split VGA default selection from VGA arbiter

[Daniel Axtens]

This is approach 3 of my patch series to sort out Xorg
autoconfiguration for the Hibmc card beind a Hisilicon bridge on
arm64.

Approach 1 was a simple quirk for the card+bridge to mark it as
default. This higlighted the fact that the default card was picked by
the arbiter, which assumed legacy resources. The lack of legacy
resources leads to quirks in ppc and concerns in arm land, so a more
generic approach was desired.

Approach 2 allowed platforms to opt in to a class enable hook that
added a card as default if there was no default. This:

 - was possibly racy as ACPI PCI init and vgaarb are both subsys
   initcalls.

 - didn't check to see if a card had a driver.

 - meant that platforms for which the vga arbiter didn't make sense
   still needed it.

This is approach 3. It pulls the default handling out of the arbiter,
into its own file and behind its own Kconfig option. It adds the extra
detection as a late initcall and an enable hook that only operates
after the initcall, so it's not racy. It checks for drivers. It means
people can turn off the vga arbiter. It works sensibly for modules
too.

Patch 1 cleans up the powerpc fixup, as with approach 2.
Patch 2 is the big split.
Patch 3 moves ppc over, as with approach 2.
There is no need for an arm-specific patch this time as the Kconfig option
is on by default.

Regards,
Daniel

Daniel Axtens (3):
  powerpc: simplify and fix VGA default device behaviour
  Split VGA default device handler out of VGA arbiter
  powerpc: replace vga_fixup() with generic code

 arch/ia64/pci/fixup.c            |   2 +-
 arch/powerpc/kernel/pci-common.c |  13 ----
 arch/x86/pci/fixup.c             |   2 +-
 arch/x86/video/fbdev.c           |   2 +-
 drivers/gpu/vga/Kconfig          |  12 +++
 drivers/gpu/vga/Makefile         |   1 +
 drivers/gpu/vga/vga_default.c    | 159 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/vga/vga_switcheroo.c |   2 +-
 drivers/gpu/vga/vgaarb.c         |  41 +---------
 drivers/pci/pci-sysfs.c          |   2 +-
 include/linux/vga_default.h      |  44 +++++++++++
 include/linux/vgaarb.h           |  14 ----
 12 files changed, 224 insertions(+), 70 deletions(-)
 create mode 100644 drivers/gpu/vga/vga_default.c
 create mode 100644 include/linux/vga_default.h

-- 
2.11.0

[PATCH 0/3] Split VGA default selection from VGA arbiter

[Daniel Axtens]

This is approach 3 of my patch series to sort out Xorg
autoconfiguration for the Hibmc card beind a Hisilicon bridge on
arm64.

Approach 1 was a simple quirk for the card+bridge to mark it as
default. This higlighted the fact that the default card was picked by
the arbiter, which assumed legacy resources. The lack of legacy
resources leads to quirks in ppc and concerns in arm land, so a more
generic approach was desired.

Approach 2 allowed platforms to opt in to a class enable hook that
added a card as default if there was no default. This:

 - was possibly racy as ACPI PCI init and vgaarb are both subsys
   initcalls.

 - didn't check to see if a card had a driver.

 - meant that platforms for which the vga arbiter didn't make sense
   still needed it.

This is approach 3. It pulls the default handling out of the arbiter,
into its own file and behind its own Kconfig option. It adds the extra
detection as a late initcall and an enable hook that only operates
after the initcall, so it's not racy. It checks for drivers. It means
people can turn off the vga arbiter. It works sensibly for modules
too.

Patch 1 cleans up the powerpc fixup, as with approach 2.
Patch 2 is the big split.
Patch 3 moves ppc over, as with approach 2.
There is no need for an arm-specific patch this time as the Kconfig option
is on by default.

Regards,
Daniel

Daniel Axtens (3):
  powerpc: simplify and fix VGA default device behaviour
  Split VGA default device handler out of VGA arbiter
  powerpc: replace vga_fixup() with generic code

 arch/ia64/pci/fixup.c            |   2 +-
 arch/powerpc/kernel/pci-common.c |  13 ----
 arch/x86/pci/fixup.c             |   2 +-
 arch/x86/video/fbdev.c           |   2 +-
 drivers/gpu/vga/Kconfig          |  12 +++
 drivers/gpu/vga/Makefile         |   1 +
 drivers/gpu/vga/vga_default.c    | 159 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/vga/vga_switcheroo.c |   2 +-
 drivers/gpu/vga/vgaarb.c         |  41 +---------
 drivers/pci/pci-sysfs.c          |   2 +-
 include/linux/vga_default.h      |  44 +++++++++++
 include/linux/vgaarb.h           |  14 ----
 12 files changed, 224 insertions(+), 70 deletions(-)
 create mode 100644 drivers/gpu/vga/vga_default.c
 create mode 100644 include/linux/vga_default.h

-- 
2.11.0

[PATCH 1/3] powerpc: simplify and fix VGA default device behaviour

[Daniel Axtens]

Some powerpc devices provide a PCI display that isn't picked up by
the VGA arbiter, presumably because it doesn't support the PCI
legacy VGA ranges.

Commit c2e1d84523ad ("powerpc: Set default VGA device") introduced
an arch quirk to mark these devices as default to fix X autoconfig.

The commit message stated that the patch:

    Ensures a default VGA is always set if a graphics adapter is present,
    even if firmware did not initialize it. If more than one graphics
    adapter is present, ensure the one initialized by firmware is set
    as the default VGA device.

The patch used the following test to decide whether or not to mark
a device as default:

  pci_read_config_word(pdev, PCI_COMMAND, &cmd);
  if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
          vga_set_default_device(pdev);

This doesn't seem like it works quite as intended. Because of the
logical OR, the default device will be set in 2 cases:

 1) if there is no default device
OR
 2) if this device has normal memory/IO decoding turned on

This will work as intended if there is only one device, but if
there are multiple devices, we may override the device the VGA
arbiter picked.

Instead, set a device as default if there is no default device AND
this device decodes.

This will not change behaviour on single-headed systems.

Cc: Brian King <brking@linux.vnet.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>

---

Tested in TCG (the card provided by qemu doesn't automatically
register with vgaarb, so the relevant code path has been tested)
but I would appreciate any tests on real hardware.

Informal benh ack: https://patchwork.kernel.org/patch/9850235/
---
 arch/powerpc/kernel/pci-common.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 341a7469cab8..c95fdda3a2dc 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1746,8 +1746,11 @@ static void fixup_vga(struct pci_dev *pdev)
 {
 	u16 cmd;
 
+	if (vga_default_device())
+		return;
+
 	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
-	if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
+	if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
 		vga_set_default_device(pdev);
 
 }
-- 
2.11.0

[PATCH 1/3] powerpc: simplify and fix VGA default device behaviour

[Daniel Axtens]

Some powerpc devices provide a PCI display that isn't picked up by
the VGA arbiter, presumably because it doesn't support the PCI
legacy VGA ranges.

Commit c2e1d84523ad ("powerpc: Set default VGA device") introduced
an arch quirk to mark these devices as default to fix X autoconfig.

The commit message stated that the patch:

    Ensures a default VGA is always set if a graphics adapter is present,
    even if firmware did not initialize it. If more than one graphics
    adapter is present, ensure the one initialized by firmware is set
    as the default VGA device.

The patch used the following test to decide whether or not to mark
a device as default:

  pci_read_config_word(pdev, PCI_COMMAND, &cmd);
  if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
          vga_set_default_device(pdev);

This doesn't seem like it works quite as intended. Because of the
logical OR, the default device will be set in 2 cases:

 1) if there is no default device
OR
 2) if this device has normal memory/IO decoding turned on

This will work as intended if there is only one device, but if
there are multiple devices, we may override the device the VGA
arbiter picked.

Instead, set a device as default if there is no default device AND
this device decodes.

This will not change behaviour on single-headed systems.

Cc: Brian King <brking@linux.vnet.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>

---

Tested in TCG (the card provided by qemu doesn't automatically
register with vgaarb, so the relevant code path has been tested)
but I would appreciate any tests on real hardware.

Informal benh ack: https://patchwork.kernel.org/patch/9850235/
---
 arch/powerpc/kernel/pci-common.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 341a7469cab8..c95fdda3a2dc 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1746,8 +1746,11 @@ static void fixup_vga(struct pci_dev *pdev)
 {
 	u16 cmd;
 
+	if (vga_default_device())
+		return;
+
 	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
-	if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
+	if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
 		vga_set_default_device(pdev);
 
 }
-- 
2.11.0

Re: [PATCH 1/3] powerpc: simplify and fix VGA default device behaviour

[Bjorn Helgaas]

On Fri, Aug 04, 2017 at 08:20:31PM +1000, Daniel Axtens wrote:
> Some powerpc devices provide a PCI display that isn't picked up by
> the VGA arbiter, presumably because it doesn't support the PCI
> legacy VGA ranges.
> 
> Commit c2e1d84523ad ("powerpc: Set default VGA device") introduced
> an arch quirk to mark these devices as default to fix X autoconfig.
> 
> The commit message stated that the patch:
> 
>     Ensures a default VGA is always set if a graphics adapter is present,
>     even if firmware did not initialize it. If more than one graphics
>     adapter is present, ensure the one initialized by firmware is set
>     as the default VGA device.
> 
> The patch used the following test to decide whether or not to mark
> a device as default:
> 
>   pci_read_config_word(pdev, PCI_COMMAND, &cmd);
>   if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
>           vga_set_default_device(pdev);
> 
> This doesn't seem like it works quite as intended. Because of the
> logical OR, the default device will be set in 2 cases:
> 
>  1) if there is no default device
> OR
>  2) if this device has normal memory/IO decoding turned on
> 
> This will work as intended if there is only one device, but if
> there are multiple devices, we may override the device the VGA
> arbiter picked.

This quirk only runs on VGA class devices.  If there's more than one
VGA device in the system, and we assume that firmware only enables
PCI_COMMAND_IO or PCI_COMMAND_MEMORY on "the one initialized by
firmware", which seems reasonable to me, I think the existing code
does match the commit message.

We set the first VGA device we find to be the default.  Then, if we
find another VGA device that's enabled, we make *it* the default
instead.

> Instead, set a device as default if there is no default device AND
> this device decodes.
> 
> This will not change behaviour on single-headed systems.

If there is no enabled VGA device on the system, your new code means
there will be no default VGA device.

It's not clear from this changelog what problem this patch solves.
Maybe it's the "some displays not being picked up by the VGA arbiter"
you mentioned, but there's not enough detail to connect it with the
patch, especially since the patch means we'll set the default device
in fewer cases than we did before.

With the patch, we only set the default if we find an enabled VGA
device.  Previously we also set the default if we found a VGA device
that had not been enabled.

> Cc: Brian King <brking@linux.vnet.ibm.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> 
> ---
> 
> Tested in TCG (the card provided by qemu doesn't automatically
> register with vgaarb, so the relevant code path has been tested)
> but I would appreciate any tests on real hardware.
> 
> Informal benh ack: https://patchwork.kernel.org/patch/9850235/
> ---
>  arch/powerpc/kernel/pci-common.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 341a7469cab8..c95fdda3a2dc 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1746,8 +1746,11 @@ static void fixup_vga(struct pci_dev *pdev)
>  {
>  	u16 cmd;
>  
> +	if (vga_default_device())
> +		return;
> +
>  	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> -	if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
> +	if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
>  		vga_set_default_device(pdev);
>  
>  }
> -- 
> 2.11.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] powerpc: simplify and fix VGA default device behaviour

[Bjorn Helgaas]

On Fri, Aug 04, 2017 at 08:20:31PM +1000, Daniel Axtens wrote:
> Some powerpc devices provide a PCI display that isn't picked up by
> the VGA arbiter, presumably because it doesn't support the PCI
> legacy VGA ranges.
> 
> Commit c2e1d84523ad ("powerpc: Set default VGA device") introduced
> an arch quirk to mark these devices as default to fix X autoconfig.
> 
> The commit message stated that the patch:
> 
>     Ensures a default VGA is always set if a graphics adapter is present,
>     even if firmware did not initialize it. If more than one graphics
>     adapter is present, ensure the one initialized by firmware is set
>     as the default VGA device.
> 
> The patch used the following test to decide whether or not to mark
> a device as default:
> 
>   pci_read_config_word(pdev, PCI_COMMAND, &cmd);
>   if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
>           vga_set_default_device(pdev);
> 
> This doesn't seem like it works quite as intended. Because of the
> logical OR, the default device will be set in 2 cases:
> 
>  1) if there is no default device
> OR
>  2) if this device has normal memory/IO decoding turned on
> 
> This will work as intended if there is only one device, but if
> there are multiple devices, we may override the device the VGA
> arbiter picked.

This quirk only runs on VGA class devices.  If there's more than one
VGA device in the system, and we assume that firmware only enables
PCI_COMMAND_IO or PCI_COMMAND_MEMORY on "the one initialized by
firmware", which seems reasonable to me, I think the existing code
does match the commit message.

We set the first VGA device we find to be the default.  Then, if we
find another VGA device that's enabled, we make *it* the default
instead.

> Instead, set a device as default if there is no default device AND
> this device decodes.
> 
> This will not change behaviour on single-headed systems.

If there is no enabled VGA device on the system, your new code means
there will be no default VGA device.

It's not clear from this changelog what problem this patch solves.
Maybe it's the "some displays not being picked up by the VGA arbiter"
you mentioned, but there's not enough detail to connect it with the
patch, especially since the patch means we'll set the default device
in fewer cases than we did before.

With the patch, we only set the default if we find an enabled VGA
device.  Previously we also set the default if we found a VGA device
that had not been enabled.

> Cc: Brian King <brking@linux.vnet.ibm.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> 
> ---
> 
> Tested in TCG (the card provided by qemu doesn't automatically
> register with vgaarb, so the relevant code path has been tested)
> but I would appreciate any tests on real hardware.
> 
> Informal benh ack: https://patchwork.kernel.org/patch/9850235/
> ---
>  arch/powerpc/kernel/pci-common.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 341a7469cab8..c95fdda3a2dc 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1746,8 +1746,11 @@ static void fixup_vga(struct pci_dev *pdev)
>  {
>  	u16 cmd;
>  
> +	if (vga_default_device())
> +		return;
> +
>  	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> -	if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
> +	if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
>  		vga_set_default_device(pdev);
>  
>  }
> -- 
> 2.11.0
> 

Re: [PATCH 1/3] powerpc: simplify and fix VGA default device behaviour

[Bjorn Helgaas]

On Fri, Aug 04, 2017 at 08:20:31PM +1000, Daniel Axtens wrote:
> Some powerpc devices provide a PCI display that isn't picked up by
> the VGA arbiter, presumably because it doesn't support the PCI
> legacy VGA ranges.
> 
> Commit c2e1d84523ad ("powerpc: Set default VGA device") introduced
> an arch quirk to mark these devices as default to fix X autoconfig.
> 
> The commit message stated that the patch:
> 
>     Ensures a default VGA is always set if a graphics adapter is present,
>     even if firmware did not initialize it. If more than one graphics
>     adapter is present, ensure the one initialized by firmware is set
>     as the default VGA device.
> 
> The patch used the following test to decide whether or not to mark
> a device as default:
> 
>   pci_read_config_word(pdev, PCI_COMMAND, &cmd);
>   if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
>           vga_set_default_device(pdev);
> 
> This doesn't seem like it works quite as intended. Because of the
> logical OR, the default device will be set in 2 cases:
> 
>  1) if there is no default device
> OR
>  2) if this device has normal memory/IO decoding turned on
> 
> This will work as intended if there is only one device, but if
> there are multiple devices, we may override the device the VGA
> arbiter picked.

This quirk only runs on VGA class devices.  If there's more than one
VGA device in the system, and we assume that firmware only enables
PCI_COMMAND_IO or PCI_COMMAND_MEMORY on "the one initialized by
firmware", which seems reasonable to me, I think the existing code
does match the commit message.

We set the first VGA device we find to be the default.  Then, if we
find another VGA device that's enabled, we make *it* the default
instead.

> Instead, set a device as default if there is no default device AND
> this device decodes.
> 
> This will not change behaviour on single-headed systems.

If there is no enabled VGA device on the system, your new code means
there will be no default VGA device.

It's not clear from this changelog what problem this patch solves.
Maybe it's the "some displays not being picked up by the VGA arbiter"
you mentioned, but there's not enough detail to connect it with the
patch, especially since the patch means we'll set the default device
in fewer cases than we did before.

With the patch, we only set the default if we find an enabled VGA
device.  Previously we also set the default if we found a VGA device
that had not been enabled.

> Cc: Brian King <brking@linux.vnet.ibm.com>
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> 
> ---
> 
> Tested in TCG (the card provided by qemu doesn't automatically
> register with vgaarb, so the relevant code path has been tested)
> but I would appreciate any tests on real hardware.
> 
> Informal benh ack: https://patchwork.kernel.org/patch/9850235/
> ---
>  arch/powerpc/kernel/pci-common.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 341a7469cab8..c95fdda3a2dc 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1746,8 +1746,11 @@ static void fixup_vga(struct pci_dev *pdev)
>  {
>  	u16 cmd;
>  
> +	if (vga_default_device())
> +		return;
> +
>  	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> -	if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
> +	if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
>  		vga_set_default_device(pdev);
>  
>  }
> -- 
> 2.11.0
> 

Re: [PATCH 1/3] powerpc: simplify and fix VGA default device behaviour

[Daniel Axtens]

Hi Bjorn,

Thanks for reading the patch and providing some feedback.

>> This will work as intended if there is only one device, but if
>> there are multiple devices, we may override the device the VGA
>> arbiter picked.
>
> This quirk only runs on VGA class devices.  If there's more than one
> VGA device in the system, and we assume that firmware only enables
> PCI_COMMAND_IO or PCI_COMMAND_MEMORY on "the one initialized by
> firmware", which seems reasonable to me, I think the existing code
> does match the commit message.
>
> We set the first VGA device we find to be the default.  Then, if we
> find another VGA device that's enabled, we make *it* the default
> instead.
>
>> Instead, set a device as default if there is no default device AND
>> this device decodes.
>> 
>> This will not change behaviour on single-headed systems.
>
> If there is no enabled VGA device on the system, your new code means
> there will be no default VGA device.

Ah. Right you are.

>
> It's not clear from this changelog what problem this patch solves.
> Maybe it's the "some displays not being picked up by the VGA arbiter"
> you mentioned, but there's not enough detail to connect it with the
> patch, especially since the patch means we'll set the default device
> in fewer cases than we did before.
>
> With the patch, we only set the default if we find an enabled VGA
> device.  Previously we also set the default if we found a VGA device
> that had not been enabled.

My overall problem is not having default devices on some
machines. Initially, to solve this, I wanted to make this code
generic. To do that I wanted to make sure it played nice with the vga
arbiter, so not overriding default devices willy-nilly was a
requirement. Evidently, I was overly keen and restricted its behaviour
too much.

Regardless, in this current approach I don't make this powerpc code
generic after all, so this patch is unnecessary. I will remove it for
v2, which I will post after further testing.

I document the effects of the new approach for powerpc in more detail in
patch 3 of this series. If powerpc wants to keep the existing approach
we can arrange for that too; it's a simple matter of ifdefs.

Regards,
Daniel
>
>> Cc: Brian King <brking@linux.vnet.ibm.com>
>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>> 
>> ---
>> 
>> Tested in TCG (the card provided by qemu doesn't automatically
>> register with vgaarb, so the relevant code path has been tested)
>> but I would appreciate any tests on real hardware.
>> 
>> Informal benh ack: https://patchwork.kernel.org/patch/9850235/
>> ---
>>  arch/powerpc/kernel/pci-common.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>> index 341a7469cab8..c95fdda3a2dc 100644
>> --- a/arch/powerpc/kernel/pci-common.c
>> +++ b/arch/powerpc/kernel/pci-common.c
>> @@ -1746,8 +1746,11 @@ static void fixup_vga(struct pci_dev *pdev)
>>  {
>>  	u16 cmd;
>>  
>> +	if (vga_default_device())
>> +		return;
>> +
>>  	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
>> -	if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
>> +	if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
>>  		vga_set_default_device(pdev);
>>  
>>  }
>> -- 
>> 2.11.0
>> 

[PATCH 1/3] powerpc: simplify and fix VGA default device behaviour

[Daniel Axtens]

Hi Bjorn,

Thanks for reading the patch and providing some feedback.

>> This will work as intended if there is only one device, but if
>> there are multiple devices, we may override the device the VGA
>> arbiter picked.
>
> This quirk only runs on VGA class devices.  If there's more than one
> VGA device in the system, and we assume that firmware only enables
> PCI_COMMAND_IO or PCI_COMMAND_MEMORY on "the one initialized by
> firmware", which seems reasonable to me, I think the existing code
> does match the commit message.
>
> We set the first VGA device we find to be the default.  Then, if we
> find another VGA device that's enabled, we make *it* the default
> instead.
>
>> Instead, set a device as default if there is no default device AND
>> this device decodes.
>> 
>> This will not change behaviour on single-headed systems.
>
> If there is no enabled VGA device on the system, your new code means
> there will be no default VGA device.

Ah. Right you are.

>
> It's not clear from this changelog what problem this patch solves.
> Maybe it's the "some displays not being picked up by the VGA arbiter"
> you mentioned, but there's not enough detail to connect it with the
> patch, especially since the patch means we'll set the default device
> in fewer cases than we did before.
>
> With the patch, we only set the default if we find an enabled VGA
> device.  Previously we also set the default if we found a VGA device
> that had not been enabled.

My overall problem is not having default devices on some
machines. Initially, to solve this, I wanted to make this code
generic. To do that I wanted to make sure it played nice with the vga
arbiter, so not overriding default devices willy-nilly was a
requirement. Evidently, I was overly keen and restricted its behaviour
too much.

Regardless, in this current approach I don't make this powerpc code
generic after all, so this patch is unnecessary. I will remove it for
v2, which I will post after further testing.

I document the effects of the new approach for powerpc in more detail in
patch 3 of this series. If powerpc wants to keep the existing approach
we can arrange for that too; it's a simple matter of ifdefs.

Regards,
Daniel
>
>> Cc: Brian King <brking@linux.vnet.ibm.com>
>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>> 
>> ---
>> 
>> Tested in TCG (the card provided by qemu doesn't automatically
>> register with vgaarb, so the relevant code path has been tested)
>> but I would appreciate any tests on real hardware.
>> 
>> Informal benh ack: https://patchwork.kernel.org/patch/9850235/
>> ---
>>  arch/powerpc/kernel/pci-common.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>> index 341a7469cab8..c95fdda3a2dc 100644
>> --- a/arch/powerpc/kernel/pci-common.c
>> +++ b/arch/powerpc/kernel/pci-common.c
>> @@ -1746,8 +1746,11 @@ static void fixup_vga(struct pci_dev *pdev)
>>  {
>>  	u16 cmd;
>>  
>> +	if (vga_default_device())
>> +		return;
>> +
>>  	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
>> -	if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
>> +	if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
>>  		vga_set_default_device(pdev);
>>  
>>  }
>> -- 
>> 2.11.0
>> 

[PATCH 2/3] Split VGA default device handler out of VGA arbiter

[Daniel Axtens]

A system without PCI legacy resources (e.g. ARM64, powerpc) may
find that no default/boot VGA device has been marked, because the
VGA arbiter checks for legacy resource decoding before marking a
card as default.

Split the small bit of code that does default VGA handling out from
the arbiter. Add a Kconfig option to allow the kernel to be built
with just the default handling, or the arbiter and default handling.
(You can try with the arbiter and no default handling - I can't see
why that wouldn't work but it seems a bit odd to try.)

Add handling for devices that should be marked as default but aren't
handled by the vga arbiter by adding a late initcall and a class
enable hook. If there is no default from vgaarb then the first card
that is enabled, has a driver bound, and can decode memory or I/O
will be marked as default.

Signed-off-by: Daniel Axtens <dja@axtens.net>

---

I haven't tested this particularly deeply just yet as I wanted to
see if anyone had Strong Feelings before I put too much time into
testing. I have verified that it works on ppc with qemu TCG, both
with and without the following patch.

I know this adds another config option and that's a bit sad, but
we can't include it unconditionally as it depends on PCI.
Suggestions welcome.
---
 arch/ia64/pci/fixup.c            |   2 +-
 arch/powerpc/kernel/pci-common.c |   2 +-
 arch/x86/pci/fixup.c             |   2 +-
 arch/x86/video/fbdev.c           |   2 +-
 drivers/gpu/vga/Kconfig          |  12 +++
 drivers/gpu/vga/Makefile         |   1 +
 drivers/gpu/vga/vga_default.c    | 159 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/vga/vga_switcheroo.c |   2 +-
 drivers/gpu/vga/vgaarb.c         |  41 +---------
 drivers/pci/pci-sysfs.c          |   2 +-
 include/linux/vga_default.h      |  44 +++++++++++
 include/linux/vgaarb.h           |  14 ----
 12 files changed, 225 insertions(+), 58 deletions(-)
 create mode 100644 drivers/gpu/vga/vga_default.c
 create mode 100644 include/linux/vga_default.h

diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
index 41caa99add51..b35d1cf4501a 100644
--- a/arch/ia64/pci/fixup.c
+++ b/arch/ia64/pci/fixup.c
@@ -5,7 +5,7 @@
 
 #include <linux/pci.h>
 #include <linux/init.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 #include <linux/screen_info.h>
 
 #include <asm/machvec.h>
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index c95fdda3a2dc..6cfaec107374 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -31,7 +31,7 @@
 #include <linux/irq.h>
 #include <linux/vmalloc.h>
 #include <linux/slab.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 
 #include <asm/processor.h>
 #include <asm/io.h>
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 11e407489db0..b1254bc09a45 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -5,7 +5,7 @@
 #include <linux/delay.h>
 #include <linux/dmi.h>
 #include <linux/pci.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 #include <asm/hpet.h>
 #include <asm/pci_x86.h>
 
diff --git a/arch/x86/video/fbdev.c b/arch/x86/video/fbdev.c
index 9fd24846d094..62cfa74ea86e 100644
--- a/arch/x86/video/fbdev.c
+++ b/arch/x86/video/fbdev.c
@@ -9,7 +9,7 @@
 #include <linux/fb.h>
 #include <linux/pci.h>
 #include <linux/module.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 
 int fb_is_primary_device(struct fb_info *info)
 {
diff --git a/drivers/gpu/vga/Kconfig b/drivers/gpu/vga/Kconfig
index 29437eabe095..81d4105aecf6 100644
--- a/drivers/gpu/vga/Kconfig
+++ b/drivers/gpu/vga/Kconfig
@@ -1,3 +1,14 @@
+config VGA_DEFAULT
+	bool "VGA Default Device Support" if EXPERT
+	default y
+	depends on PCI
+	help
+	  Some programs find it helpful to know what VGA device is the default.
+	  On platforms like x86 this means the device used by the BIOS to show
+	  early boot messages. On other platforms this may be an arbitrary PCI
+	  graphics card. Select this to have a default device recorded within
+	  the kernel and exposed to userspace through sysfs.
+
 config VGA_ARB
 	bool "VGA Arbitration" if EXPERT
 	default y
@@ -22,6 +33,7 @@ config VGA_SWITCHEROO
 	depends on X86
 	depends on ACPI
 	select VGA_ARB
+	select VGA_DEFAULT
 	help
 	  Many laptops released in 2008/9/10 have two GPUs with a multiplexer
 	  to switch between them. This adds support for dynamic switching when
diff --git a/drivers/gpu/vga/Makefile b/drivers/gpu/vga/Makefile
index 14ca30b75d0a..1e30f90d40fb 100644
--- a/drivers/gpu/vga/Makefile
+++ b/drivers/gpu/vga/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_VGA_ARB)  += vgaarb.o
+obj-$(CONFIG_VGA_DEFAULT) += vga_default.o
 obj-$(CONFIG_VGA_SWITCHEROO) += vga_switcheroo.o
diff --git a/drivers/gpu/vga/vga_default.c b/drivers/gpu/vga/vga_default.c
new file mode 100644
index 000000000000..f6fcb0eb1507
--- /dev/null
+++ b/drivers/gpu/vga/vga_default.c
@@ -0,0 +1,159 @@
+/*
+ * vga_default.c: What is the default/boot PCI VGA device?
+ *
+ * (C) Copyright 2005 Benjamin Herrenschmidt <benh@kernel.crashing.org>
+ * (C) Copyright 2007 Paulo R. Zanoni <przanoni@gmail.com>
+ * (C) Copyright 2007, 2009 Tiago Vignatti <vignatti@freedesktop.org>
+ * (C) Copyright 2017 Canonical Ltd. (Author: Daniel Axtens <dja@axtens.net>)
+ *
+ * (License from vgaarb.c)
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/*
+ * What device should a graphics system draw to? In order of priority:
+ *
+ *  1) Any devices configured specifically by the user (think
+ *     xorg.conf).
+ *
+ *  2) If the platform has a concept of a boot device for early boot
+ *     messages (think BIOS displays on x86), that device.
+ *
+ *  3) If the platform does not have the concept of a boot device,
+ *     then we still want to pick something. For now, pick the first
+ *     PCI VGA device with a driver bound and with memory or I/O
+ *     control on.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+
+#include <linux/vga_default.h>
+
+static struct pci_dev *vga_default;
+/*
+ * only go active after the late initcall so as not to interfere with
+ * the arbiter
+ */
+static bool vga_default_active = false;
+
+/**
+ * vga_default_device - return the default VGA device
+ *
+ * This can be defined by the platform. The default implementation
+ * is rather dumb and will probably only work properly on single
+ * vga card setups and/or x86 platforms.
+ *
+ * If your VGA default device is not PCI, you'll have to return
+ * NULL here. In this case, I assume it will not conflict with
+ * any PCI card. If this is not true, I'll have to define two archs
+ * hooks for enabling/disabling the VGA default device if that is
+ * possible. This may be a problem with real _ISA_ VGA cards, in
+ * addition to a PCI one. I don't know at this point how to deal
+ * with that card. Can theirs IOs be disabled at all ? If not, then
+ * I suppose it's a matter of having the proper arch hook telling
+ * us about it, so we basically never allow anybody to succeed a
+ * vga_get()...
+ */
+
+struct pci_dev *vga_default_device(void)
+{
+	return vga_default;
+}
+EXPORT_SYMBOL_GPL(vga_default_device);
+
+void vga_set_default_device(struct pci_dev *pdev)
+{
+	if (vga_default == pdev)
+		return;
+
+	pci_dev_put(vga_default);
+	vga_default = pci_dev_get(pdev);
+}
+
+static bool vga_default_try_device(struct pci_dev *pdev)
+{
+	u16 cmd;
+
+	/* Only deal with VGA class devices */
+	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+		return false;
+
+	/* Only deal with devices with drivers bound */
+	if (!pdev->driver)
+		return false;
+
+	/* Require I/O or memory control */
+	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
+	if (!(cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)))
+		return false;
+
+	dev_info(&pdev->dev, "vga_default: setting as default device\n");
+	vga_set_default_device(pdev);
+	return true;
+}
+
+static int __init vga_default_init(void)
+{
+	struct pci_dev *pdev;
+
+	vga_default_active = true;
+
+	if (vga_default_device())
+		return 0;
+
+	pdev = NULL;
+	while ((pdev =
+		pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
+			       PCI_ANY_ID, pdev)) != NULL) {
+		if (vga_default_try_device(pdev))
+			return 0;
+	}
+
+	return 0;
+}
+late_initcall(vga_default_init);
+
+/*
+ * A driver could be loaded much later than late_initcall, for example
+ * if it's in a module.
+ *
+ * We want to pick that up. However, we want to make sure this does
+ * not interfere with the arbiter - it should only activate if the
+ * arbiter has already had a chance to operate. To ensure this, we set
+ * vga_default_active in the late_initcall: as the vga arbiter is a
+ * subsys initcall, it is guaranteed to fire first.
+ */
+static void vga_default_enable_hook(struct pci_dev *pdev)
+{
+       if (!vga_default_active)
+	       return;
+
+       if (vga_default_device())
+               return;
+
+       vga_default_try_device(pdev);
+}
+DECLARE_PCI_FIXUP_CLASS_ENABLE(PCI_ANY_ID, PCI_ANY_ID,
+			       PCI_CLASS_DISPLAY_VGA, 8,
+			       vga_default_enable_hook)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 3cd153c6d271..6612ec7981b6 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -41,7 +41,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/seq_file.h>
 #include <linux/uaccess.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 #include <linux/vga_switcheroo.h>
 
 /**
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index 76875f6299b8..74683286f5f8 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -51,6 +51,7 @@
 
 #include <linux/uaccess.h>
 
+#include <linux/vga_default.h>
 #include <linux/vgaarb.h>
 
 static void vga_arbiter_notify_clients(void);
@@ -119,9 +120,6 @@ static int vga_str_to_iostate(char *buf, int str_size, int *io_state)
 	return 1;
 }
 
-/* this is only used a cookie - it should not be dereferenced */
-static struct pci_dev *vga_default;
-
 static void vga_arb_device_card_gone(struct pci_dev *pdev);
 
 /* Find somebody in our list */
@@ -135,39 +133,6 @@ static struct vga_device *vgadev_find(struct pci_dev *pdev)
 	return NULL;
 }
 
-/**
- * vga_default_device - return the default VGA device, for vgacon
- *
- * This can be defined by the platform. The default implementation
- * is rather dumb and will probably only work properly on single
- * vga card setups and/or x86 platforms.
- *
- * If your VGA default device is not PCI, you'll have to return
- * NULL here. In this case, I assume it will not conflict with
- * any PCI card. If this is not true, I'll have to define two archs
- * hooks for enabling/disabling the VGA default device if that is
- * possible. This may be a problem with real _ISA_ VGA cards, in
- * addition to a PCI one. I don't know at this point how to deal
- * with that card. Can theirs IOs be disabled at all ? If not, then
- * I suppose it's a matter of having the proper arch hook telling
- * us about it, so we basically never allow anybody to succeed a
- * vga_get()...
- */
-struct pci_dev *vga_default_device(void)
-{
-	return vga_default;
-}
-EXPORT_SYMBOL_GPL(vga_default_device);
-
-void vga_set_default_device(struct pci_dev *pdev)
-{
-	if (vga_default == pdev)
-		return;
-
-	pci_dev_put(vga_default);
-	vga_default = pci_dev_get(pdev);
-}
-
 static inline void vga_irq_set_state(struct vga_device *vgadev, bool state)
 {
 	if (vgadev->irq_set_state)
@@ -667,7 +632,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
 	/* Deal with VGA default device. Use first enabled one
 	 * by default if arch doesn't have it's own hook
 	 */
-	if (vga_default == NULL &&
+	if (vga_default_device() == NULL &&
 	    ((vgadev->owns & VGA_RSRC_LEGACY_MASK) == VGA_RSRC_LEGACY_MASK)) {
 		vgaarb_info(&pdev->dev, "setting as boot VGA device\n");
 		vga_set_default_device(pdev);
@@ -704,7 +669,7 @@ static bool vga_arbiter_del_pci_device(struct pci_dev *pdev)
 		goto bail;
 	}
 
-	if (vga_default == pdev)
+	if (vga_default_device() == pdev)
 		vga_set_default_device(NULL);
 
 	if (vgadev->decodes & (VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM))
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 2f3780b50723..c174b427ea2b 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -27,7 +27,7 @@
 #include <linux/security.h>
 #include <linux/pci-aspm.h>
 #include <linux/slab.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
 #include "pci.h"
diff --git a/include/linux/vga_default.h b/include/linux/vga_default.h
new file mode 100644
index 000000000000..78df6a2a194c
--- /dev/null
+++ b/include/linux/vga_default.h
@@ -0,0 +1,44 @@
+/*
+ * vga_default.h: What is the default/boot PCI VGA device?
+ *
+ * (C) Copyright 2005 Benjamin Herrenschmidt <benh@kernel.crashing.org>
+ * (C) Copyright 2007 Paulo R. Zanoni <przanoni@gmail.com>
+ * (C) Copyright 2007, 2009 Tiago Vignatti <vignatti@freedesktop.org>
+ * (C) Copyright 2017 Canonical Ltd. (Author: Daniel Axtens <dja@axtens.net>)
+ *
+ * (License from vgaarb.h)
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef LINUX_VGA_DEFAULT_H
+#define LINUX_VGA_DEFAULT_H
+
+struct pci_dev;
+
+#ifdef CONFIG_VGA_DEFAULT
+extern struct pci_dev *vga_default_device(void);
+extern void vga_set_default_device(struct pci_dev *pdev);
+#else
+static inline struct pci_dev *vga_default_device(void) { return NULL; };
+static inline void vga_set_default_device(struct pci_dev *pdev) { };
+#endif
+
+#endif
diff --git a/include/linux/vgaarb.h b/include/linux/vgaarb.h
index ee162e3e879b..953e1955efbe 100644
--- a/include/linux/vgaarb.h
+++ b/include/linux/vgaarb.h
@@ -42,12 +42,6 @@
 #define VGA_RSRC_NORMAL_IO     0x04
 #define VGA_RSRC_NORMAL_MEM    0x08
 
-/* Passing that instead of a pci_dev to use the system "default"
- * device, that is the one used by vgacon. Archs will probably
- * have to provide their own vga_default_device();
- */
-#define VGA_DEFAULT_DEVICE     (NULL)
-
 struct pci_dev;
 
 /* For use by clients */
@@ -122,14 +116,6 @@ extern void vga_put(struct pci_dev *pdev, unsigned int rsrc);
 #endif
 
 
-#ifdef CONFIG_VGA_ARB
-extern struct pci_dev *vga_default_device(void);
-extern void vga_set_default_device(struct pci_dev *pdev);
-#else
-static inline struct pci_dev *vga_default_device(void) { return NULL; };
-static inline void vga_set_default_device(struct pci_dev *pdev) { };
-#endif
-
 /*
  * Architectures should define this if they have several
  * independent PCI domains that can afford concurrent VGA
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] Split VGA default device handler out of VGA arbiter

[Daniel Axtens]

A system without PCI legacy resources (e.g. ARM64, powerpc) may
find that no default/boot VGA device has been marked, because the
VGA arbiter checks for legacy resource decoding before marking a
card as default.

Split the small bit of code that does default VGA handling out from
the arbiter. Add a Kconfig option to allow the kernel to be built
with just the default handling, or the arbiter and default handling.
(You can try with the arbiter and no default handling - I can't see
why that wouldn't work but it seems a bit odd to try.)

Add handling for devices that should be marked as default but aren't
handled by the vga arbiter by adding a late initcall and a class
enable hook. If there is no default from vgaarb then the first card
that is enabled, has a driver bound, and can decode memory or I/O
will be marked as default.

Signed-off-by: Daniel Axtens <dja@axtens.net>

---

I haven't tested this particularly deeply just yet as I wanted to
see if anyone had Strong Feelings before I put too much time into
testing. I have verified that it works on ppc with qemu TCG, both
with and without the following patch.

I know this adds another config option and that's a bit sad, but
we can't include it unconditionally as it depends on PCI.
Suggestions welcome.
---
 arch/ia64/pci/fixup.c            |   2 +-
 arch/powerpc/kernel/pci-common.c |   2 +-
 arch/x86/pci/fixup.c             |   2 +-
 arch/x86/video/fbdev.c           |   2 +-
 drivers/gpu/vga/Kconfig          |  12 +++
 drivers/gpu/vga/Makefile         |   1 +
 drivers/gpu/vga/vga_default.c    | 159 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/vga/vga_switcheroo.c |   2 +-
 drivers/gpu/vga/vgaarb.c         |  41 +---------
 drivers/pci/pci-sysfs.c          |   2 +-
 include/linux/vga_default.h      |  44 +++++++++++
 include/linux/vgaarb.h           |  14 ----
 12 files changed, 225 insertions(+), 58 deletions(-)
 create mode 100644 drivers/gpu/vga/vga_default.c
 create mode 100644 include/linux/vga_default.h

diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
index 41caa99add51..b35d1cf4501a 100644
--- a/arch/ia64/pci/fixup.c
+++ b/arch/ia64/pci/fixup.c
@@ -5,7 +5,7 @@
 
 #include <linux/pci.h>
 #include <linux/init.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 #include <linux/screen_info.h>
 
 #include <asm/machvec.h>
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index c95fdda3a2dc..6cfaec107374 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -31,7 +31,7 @@
 #include <linux/irq.h>
 #include <linux/vmalloc.h>
 #include <linux/slab.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 
 #include <asm/processor.h>
 #include <asm/io.h>
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 11e407489db0..b1254bc09a45 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -5,7 +5,7 @@
 #include <linux/delay.h>
 #include <linux/dmi.h>
 #include <linux/pci.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 #include <asm/hpet.h>
 #include <asm/pci_x86.h>
 
diff --git a/arch/x86/video/fbdev.c b/arch/x86/video/fbdev.c
index 9fd24846d094..62cfa74ea86e 100644
--- a/arch/x86/video/fbdev.c
+++ b/arch/x86/video/fbdev.c
@@ -9,7 +9,7 @@
 #include <linux/fb.h>
 #include <linux/pci.h>
 #include <linux/module.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 
 int fb_is_primary_device(struct fb_info *info)
 {
diff --git a/drivers/gpu/vga/Kconfig b/drivers/gpu/vga/Kconfig
index 29437eabe095..81d4105aecf6 100644
--- a/drivers/gpu/vga/Kconfig
+++ b/drivers/gpu/vga/Kconfig
@@ -1,3 +1,14 @@
+config VGA_DEFAULT
+	bool "VGA Default Device Support" if EXPERT
+	default y
+	depends on PCI
+	help
+	  Some programs find it helpful to know what VGA device is the default.
+	  On platforms like x86 this means the device used by the BIOS to show
+	  early boot messages. On other platforms this may be an arbitrary PCI
+	  graphics card. Select this to have a default device recorded within
+	  the kernel and exposed to userspace through sysfs.
+
 config VGA_ARB
 	bool "VGA Arbitration" if EXPERT
 	default y
@@ -22,6 +33,7 @@ config VGA_SWITCHEROO
 	depends on X86
 	depends on ACPI
 	select VGA_ARB
+	select VGA_DEFAULT
 	help
 	  Many laptops released in 2008/9/10 have two GPUs with a multiplexer
 	  to switch between them. This adds support for dynamic switching when
diff --git a/drivers/gpu/vga/Makefile b/drivers/gpu/vga/Makefile
index 14ca30b75d0a..1e30f90d40fb 100644
--- a/drivers/gpu/vga/Makefile
+++ b/drivers/gpu/vga/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_VGA_ARB)  += vgaarb.o
+obj-$(CONFIG_VGA_DEFAULT) += vga_default.o
 obj-$(CONFIG_VGA_SWITCHEROO) += vga_switcheroo.o
diff --git a/drivers/gpu/vga/vga_default.c b/drivers/gpu/vga/vga_default.c
new file mode 100644
index 000000000000..f6fcb0eb1507
--- /dev/null
+++ b/drivers/gpu/vga/vga_default.c
@@ -0,0 +1,159 @@
+/*
+ * vga_default.c: What is the default/boot PCI VGA device?
+ *
+ * (C) Copyright 2005 Benjamin Herrenschmidt <benh@kernel.crashing.org>
+ * (C) Copyright 2007 Paulo R. Zanoni <przanoni@gmail.com>
+ * (C) Copyright 2007, 2009 Tiago Vignatti <vignatti@freedesktop.org>
+ * (C) Copyright 2017 Canonical Ltd. (Author: Daniel Axtens <dja@axtens.net>)
+ *
+ * (License from vgaarb.c)
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/*
+ * What device should a graphics system draw to? In order of priority:
+ *
+ *  1) Any devices configured specifically by the user (think
+ *     xorg.conf).
+ *
+ *  2) If the platform has a concept of a boot device for early boot
+ *     messages (think BIOS displays on x86), that device.
+ *
+ *  3) If the platform does not have the concept of a boot device,
+ *     then we still want to pick something. For now, pick the first
+ *     PCI VGA device with a driver bound and with memory or I/O
+ *     control on.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+
+#include <linux/vga_default.h>
+
+static struct pci_dev *vga_default;
+/*
+ * only go active after the late initcall so as not to interfere with
+ * the arbiter
+ */
+static bool vga_default_active = false;
+
+/**
+ * vga_default_device - return the default VGA device
+ *
+ * This can be defined by the platform. The default implementation
+ * is rather dumb and will probably only work properly on single
+ * vga card setups and/or x86 platforms.
+ *
+ * If your VGA default device is not PCI, you'll have to return
+ * NULL here. In this case, I assume it will not conflict with
+ * any PCI card. If this is not true, I'll have to define two archs
+ * hooks for enabling/disabling the VGA default device if that is
+ * possible. This may be a problem with real _ISA_ VGA cards, in
+ * addition to a PCI one. I don't know at this point how to deal
+ * with that card. Can theirs IOs be disabled at all ? If not, then
+ * I suppose it's a matter of having the proper arch hook telling
+ * us about it, so we basically never allow anybody to succeed a
+ * vga_get()...
+ */
+
+struct pci_dev *vga_default_device(void)
+{
+	return vga_default;
+}
+EXPORT_SYMBOL_GPL(vga_default_device);
+
+void vga_set_default_device(struct pci_dev *pdev)
+{
+	if (vga_default == pdev)
+		return;
+
+	pci_dev_put(vga_default);
+	vga_default = pci_dev_get(pdev);
+}
+
+static bool vga_default_try_device(struct pci_dev *pdev)
+{
+	u16 cmd;
+
+	/* Only deal with VGA class devices */
+	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+		return false;
+
+	/* Only deal with devices with drivers bound */
+	if (!pdev->driver)
+		return false;
+
+	/* Require I/O or memory control */
+	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
+	if (!(cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)))
+		return false;
+
+	dev_info(&pdev->dev, "vga_default: setting as default device\n");
+	vga_set_default_device(pdev);
+	return true;
+}
+
+static int __init vga_default_init(void)
+{
+	struct pci_dev *pdev;
+
+	vga_default_active = true;
+
+	if (vga_default_device())
+		return 0;
+
+	pdev = NULL;
+	while ((pdev =
+		pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
+			       PCI_ANY_ID, pdev)) != NULL) {
+		if (vga_default_try_device(pdev))
+			return 0;
+	}
+
+	return 0;
+}
+late_initcall(vga_default_init);
+
+/*
+ * A driver could be loaded much later than late_initcall, for example
+ * if it's in a module.
+ *
+ * We want to pick that up. However, we want to make sure this does
+ * not interfere with the arbiter - it should only activate if the
+ * arbiter has already had a chance to operate. To ensure this, we set
+ * vga_default_active in the late_initcall: as the vga arbiter is a
+ * subsys initcall, it is guaranteed to fire first.
+ */
+static void vga_default_enable_hook(struct pci_dev *pdev)
+{
+       if (!vga_default_active)
+	       return;
+
+       if (vga_default_device())
+               return;
+
+       vga_default_try_device(pdev);
+}
+DECLARE_PCI_FIXUP_CLASS_ENABLE(PCI_ANY_ID, PCI_ANY_ID,
+			       PCI_CLASS_DISPLAY_VGA, 8,
+			       vga_default_enable_hook)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 3cd153c6d271..6612ec7981b6 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -41,7 +41,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/seq_file.h>
 #include <linux/uaccess.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 #include <linux/vga_switcheroo.h>
 
 /**
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index 76875f6299b8..74683286f5f8 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -51,6 +51,7 @@
 
 #include <linux/uaccess.h>
 
+#include <linux/vga_default.h>
 #include <linux/vgaarb.h>
 
 static void vga_arbiter_notify_clients(void);
@@ -119,9 +120,6 @@ static int vga_str_to_iostate(char *buf, int str_size, int *io_state)
 	return 1;
 }
 
-/* this is only used a cookie - it should not be dereferenced */
-static struct pci_dev *vga_default;
-
 static void vga_arb_device_card_gone(struct pci_dev *pdev);
 
 /* Find somebody in our list */
@@ -135,39 +133,6 @@ static struct vga_device *vgadev_find(struct pci_dev *pdev)
 	return NULL;
 }
 
-/**
- * vga_default_device - return the default VGA device, for vgacon
- *
- * This can be defined by the platform. The default implementation
- * is rather dumb and will probably only work properly on single
- * vga card setups and/or x86 platforms.
- *
- * If your VGA default device is not PCI, you'll have to return
- * NULL here. In this case, I assume it will not conflict with
- * any PCI card. If this is not true, I'll have to define two archs
- * hooks for enabling/disabling the VGA default device if that is
- * possible. This may be a problem with real _ISA_ VGA cards, in
- * addition to a PCI one. I don't know at this point how to deal
- * with that card. Can theirs IOs be disabled at all ? If not, then
- * I suppose it's a matter of having the proper arch hook telling
- * us about it, so we basically never allow anybody to succeed a
- * vga_get()...
- */
-struct pci_dev *vga_default_device(void)
-{
-	return vga_default;
-}
-EXPORT_SYMBOL_GPL(vga_default_device);
-
-void vga_set_default_device(struct pci_dev *pdev)
-{
-	if (vga_default == pdev)
-		return;
-
-	pci_dev_put(vga_default);
-	vga_default = pci_dev_get(pdev);
-}
-
 static inline void vga_irq_set_state(struct vga_device *vgadev, bool state)
 {
 	if (vgadev->irq_set_state)
@@ -667,7 +632,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
 	/* Deal with VGA default device. Use first enabled one
 	 * by default if arch doesn't have it's own hook
 	 */
-	if (vga_default == NULL &&
+	if (vga_default_device() == NULL &&
 	    ((vgadev->owns & VGA_RSRC_LEGACY_MASK) == VGA_RSRC_LEGACY_MASK)) {
 		vgaarb_info(&pdev->dev, "setting as boot VGA device\n");
 		vga_set_default_device(pdev);
@@ -704,7 +669,7 @@ static bool vga_arbiter_del_pci_device(struct pci_dev *pdev)
 		goto bail;
 	}
 
-	if (vga_default == pdev)
+	if (vga_default_device() == pdev)
 		vga_set_default_device(NULL);
 
 	if (vgadev->decodes & (VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM))
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 2f3780b50723..c174b427ea2b 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -27,7 +27,7 @@
 #include <linux/security.h>
 #include <linux/pci-aspm.h>
 #include <linux/slab.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
 #include "pci.h"
diff --git a/include/linux/vga_default.h b/include/linux/vga_default.h
new file mode 100644
index 000000000000..78df6a2a194c
--- /dev/null
+++ b/include/linux/vga_default.h
@@ -0,0 +1,44 @@
+/*
+ * vga_default.h: What is the default/boot PCI VGA device?
+ *
+ * (C) Copyright 2005 Benjamin Herrenschmidt <benh@kernel.crashing.org>
+ * (C) Copyright 2007 Paulo R. Zanoni <przanoni@gmail.com>
+ * (C) Copyright 2007, 2009 Tiago Vignatti <vignatti@freedesktop.org>
+ * (C) Copyright 2017 Canonical Ltd. (Author: Daniel Axtens <dja@axtens.net>)
+ *
+ * (License from vgaarb.h)
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef LINUX_VGA_DEFAULT_H
+#define LINUX_VGA_DEFAULT_H
+
+struct pci_dev;
+
+#ifdef CONFIG_VGA_DEFAULT
+extern struct pci_dev *vga_default_device(void);
+extern void vga_set_default_device(struct pci_dev *pdev);
+#else
+static inline struct pci_dev *vga_default_device(void) { return NULL; };
+static inline void vga_set_default_device(struct pci_dev *pdev) { };
+#endif
+
+#endif
diff --git a/include/linux/vgaarb.h b/include/linux/vgaarb.h
index ee162e3e879b..953e1955efbe 100644
--- a/include/linux/vgaarb.h
+++ b/include/linux/vgaarb.h
@@ -42,12 +42,6 @@
 #define VGA_RSRC_NORMAL_IO     0x04
 #define VGA_RSRC_NORMAL_MEM    0x08
 
-/* Passing that instead of a pci_dev to use the system "default"
- * device, that is the one used by vgacon. Archs will probably
- * have to provide their own vga_default_device();
- */
-#define VGA_DEFAULT_DEVICE     (NULL)
-
 struct pci_dev;
 
 /* For use by clients */
@@ -122,14 +116,6 @@ extern void vga_put(struct pci_dev *pdev, unsigned int rsrc);
 #endif
 
 
-#ifdef CONFIG_VGA_ARB
-extern struct pci_dev *vga_default_device(void);
-extern void vga_set_default_device(struct pci_dev *pdev);
-#else
-static inline struct pci_dev *vga_default_device(void) { return NULL; };
-static inline void vga_set_default_device(struct pci_dev *pdev) { };
-#endif
-
 /*
  * Architectures should define this if they have several
  * independent PCI domains that can afford concurrent VGA
-- 
2.11.0

[PATCH 2/3] Split VGA default device handler out of VGA arbiter

[Daniel Axtens]

A system without PCI legacy resources (e.g. ARM64, powerpc) may
find that no default/boot VGA device has been marked, because the
VGA arbiter checks for legacy resource decoding before marking a
card as default.

Split the small bit of code that does default VGA handling out from
the arbiter. Add a Kconfig option to allow the kernel to be built
with just the default handling, or the arbiter and default handling.
(You can try with the arbiter and no default handling - I can't see
why that wouldn't work but it seems a bit odd to try.)

Add handling for devices that should be marked as default but aren't
handled by the vga arbiter by adding a late initcall and a class
enable hook. If there is no default from vgaarb then the first card
that is enabled, has a driver bound, and can decode memory or I/O
will be marked as default.

Signed-off-by: Daniel Axtens <dja@axtens.net>

---

I haven't tested this particularly deeply just yet as I wanted to
see if anyone had Strong Feelings before I put too much time into
testing. I have verified that it works on ppc with qemu TCG, both
with and without the following patch.

I know this adds another config option and that's a bit sad, but
we can't include it unconditionally as it depends on PCI.
Suggestions welcome.
---
 arch/ia64/pci/fixup.c            |   2 +-
 arch/powerpc/kernel/pci-common.c |   2 +-
 arch/x86/pci/fixup.c             |   2 +-
 arch/x86/video/fbdev.c           |   2 +-
 drivers/gpu/vga/Kconfig          |  12 +++
 drivers/gpu/vga/Makefile         |   1 +
 drivers/gpu/vga/vga_default.c    | 159 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/vga/vga_switcheroo.c |   2 +-
 drivers/gpu/vga/vgaarb.c         |  41 +---------
 drivers/pci/pci-sysfs.c          |   2 +-
 include/linux/vga_default.h      |  44 +++++++++++
 include/linux/vgaarb.h           |  14 ----
 12 files changed, 225 insertions(+), 58 deletions(-)
 create mode 100644 drivers/gpu/vga/vga_default.c
 create mode 100644 include/linux/vga_default.h

diff --git a/arch/ia64/pci/fixup.c b/arch/ia64/pci/fixup.c
index 41caa99add51..b35d1cf4501a 100644
--- a/arch/ia64/pci/fixup.c
+++ b/arch/ia64/pci/fixup.c
@@ -5,7 +5,7 @@
 
 #include <linux/pci.h>
 #include <linux/init.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 #include <linux/screen_info.h>
 
 #include <asm/machvec.h>
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index c95fdda3a2dc..6cfaec107374 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -31,7 +31,7 @@
 #include <linux/irq.h>
 #include <linux/vmalloc.h>
 #include <linux/slab.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 
 #include <asm/processor.h>
 #include <asm/io.h>
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 11e407489db0..b1254bc09a45 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -5,7 +5,7 @@
 #include <linux/delay.h>
 #include <linux/dmi.h>
 #include <linux/pci.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 #include <asm/hpet.h>
 #include <asm/pci_x86.h>
 
diff --git a/arch/x86/video/fbdev.c b/arch/x86/video/fbdev.c
index 9fd24846d094..62cfa74ea86e 100644
--- a/arch/x86/video/fbdev.c
+++ b/arch/x86/video/fbdev.c
@@ -9,7 +9,7 @@
 #include <linux/fb.h>
 #include <linux/pci.h>
 #include <linux/module.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 
 int fb_is_primary_device(struct fb_info *info)
 {
diff --git a/drivers/gpu/vga/Kconfig b/drivers/gpu/vga/Kconfig
index 29437eabe095..81d4105aecf6 100644
--- a/drivers/gpu/vga/Kconfig
+++ b/drivers/gpu/vga/Kconfig
@@ -1,3 +1,14 @@
+config VGA_DEFAULT
+	bool "VGA Default Device Support" if EXPERT
+	default y
+	depends on PCI
+	help
+	  Some programs find it helpful to know what VGA device is the default.
+	  On platforms like x86 this means the device used by the BIOS to show
+	  early boot messages. On other platforms this may be an arbitrary PCI
+	  graphics card. Select this to have a default device recorded within
+	  the kernel and exposed to userspace through sysfs.
+
 config VGA_ARB
 	bool "VGA Arbitration" if EXPERT
 	default y
@@ -22,6 +33,7 @@ config VGA_SWITCHEROO
 	depends on X86
 	depends on ACPI
 	select VGA_ARB
+	select VGA_DEFAULT
 	help
 	  Many laptops released in 2008/9/10 have two GPUs with a multiplexer
 	  to switch between them. This adds support for dynamic switching when
diff --git a/drivers/gpu/vga/Makefile b/drivers/gpu/vga/Makefile
index 14ca30b75d0a..1e30f90d40fb 100644
--- a/drivers/gpu/vga/Makefile
+++ b/drivers/gpu/vga/Makefile
@@ -1,2 +1,3 @@
 obj-$(CONFIG_VGA_ARB)  += vgaarb.o
+obj-$(CONFIG_VGA_DEFAULT) += vga_default.o
 obj-$(CONFIG_VGA_SWITCHEROO) += vga_switcheroo.o
diff --git a/drivers/gpu/vga/vga_default.c b/drivers/gpu/vga/vga_default.c
new file mode 100644
index 000000000000..f6fcb0eb1507
--- /dev/null
+++ b/drivers/gpu/vga/vga_default.c
@@ -0,0 +1,159 @@
+/*
+ * vga_default.c: What is the default/boot PCI VGA device?
+ *
+ * (C) Copyright 2005 Benjamin Herrenschmidt <benh@kernel.crashing.org>
+ * (C) Copyright 2007 Paulo R. Zanoni <przanoni@gmail.com>
+ * (C) Copyright 2007, 2009 Tiago Vignatti <vignatti@freedesktop.org>
+ * (C) Copyright 2017 Canonical Ltd. (Author: Daniel Axtens <dja@axtens.net>)
+ *
+ * (License from vgaarb.c)
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/*
+ * What device should a graphics system draw to? In order of priority:
+ *
+ *  1) Any devices configured specifically by the user (think
+ *     xorg.conf).
+ *
+ *  2) If the platform has a concept of a boot device for early boot
+ *     messages (think BIOS displays on x86), that device.
+ *
+ *  3) If the platform does not have the concept of a boot device,
+ *     then we still want to pick something. For now, pick the first
+ *     PCI VGA device with a driver bound and with memory or I/O
+ *     control on.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+
+#include <linux/vga_default.h>
+
+static struct pci_dev *vga_default;
+/*
+ * only go active after the late initcall so as not to interfere with
+ * the arbiter
+ */
+static bool vga_default_active = false;
+
+/**
+ * vga_default_device - return the default VGA device
+ *
+ * This can be defined by the platform. The default implementation
+ * is rather dumb and will probably only work properly on single
+ * vga card setups and/or x86 platforms.
+ *
+ * If your VGA default device is not PCI, you'll have to return
+ * NULL here. In this case, I assume it will not conflict with
+ * any PCI card. If this is not true, I'll have to define two archs
+ * hooks for enabling/disabling the VGA default device if that is
+ * possible. This may be a problem with real _ISA_ VGA cards, in
+ * addition to a PCI one. I don't know at this point how to deal
+ * with that card. Can theirs IOs be disabled at all ? If not, then
+ * I suppose it's a matter of having the proper arch hook telling
+ * us about it, so we basically never allow anybody to succeed a
+ * vga_get()...
+ */
+
+struct pci_dev *vga_default_device(void)
+{
+	return vga_default;
+}
+EXPORT_SYMBOL_GPL(vga_default_device);
+
+void vga_set_default_device(struct pci_dev *pdev)
+{
+	if (vga_default == pdev)
+		return;
+
+	pci_dev_put(vga_default);
+	vga_default = pci_dev_get(pdev);
+}
+
+static bool vga_default_try_device(struct pci_dev *pdev)
+{
+	u16 cmd;
+
+	/* Only deal with VGA class devices */
+	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
+		return false;
+
+	/* Only deal with devices with drivers bound */
+	if (!pdev->driver)
+		return false;
+
+	/* Require I/O or memory control */
+	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
+	if (!(cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)))
+		return false;
+
+	dev_info(&pdev->dev, "vga_default: setting as default device\n");
+	vga_set_default_device(pdev);
+	return true;
+}
+
+static int __init vga_default_init(void)
+{
+	struct pci_dev *pdev;
+
+	vga_default_active = true;
+
+	if (vga_default_device())
+		return 0;
+
+	pdev = NULL;
+	while ((pdev =
+		pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
+			       PCI_ANY_ID, pdev)) != NULL) {
+		if (vga_default_try_device(pdev))
+			return 0;
+	}
+
+	return 0;
+}
+late_initcall(vga_default_init);
+
+/*
+ * A driver could be loaded much later than late_initcall, for example
+ * if it's in a module.
+ *
+ * We want to pick that up. However, we want to make sure this does
+ * not interfere with the arbiter - it should only activate if the
+ * arbiter has already had a chance to operate. To ensure this, we set
+ * vga_default_active in the late_initcall: as the vga arbiter is a
+ * subsys initcall, it is guaranteed to fire first.
+ */
+static void vga_default_enable_hook(struct pci_dev *pdev)
+{
+       if (!vga_default_active)
+	       return;
+
+       if (vga_default_device())
+               return;
+
+       vga_default_try_device(pdev);
+}
+DECLARE_PCI_FIXUP_CLASS_ENABLE(PCI_ANY_ID, PCI_ANY_ID,
+			       PCI_CLASS_DISPLAY_VGA, 8,
+			       vga_default_enable_hook)
diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index 3cd153c6d271..6612ec7981b6 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -41,7 +41,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/seq_file.h>
 #include <linux/uaccess.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 #include <linux/vga_switcheroo.h>
 
 /**
diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
index 76875f6299b8..74683286f5f8 100644
--- a/drivers/gpu/vga/vgaarb.c
+++ b/drivers/gpu/vga/vgaarb.c
@@ -51,6 +51,7 @@
 
 #include <linux/uaccess.h>
 
+#include <linux/vga_default.h>
 #include <linux/vgaarb.h>
 
 static void vga_arbiter_notify_clients(void);
@@ -119,9 +120,6 @@ static int vga_str_to_iostate(char *buf, int str_size, int *io_state)
 	return 1;
 }
 
-/* this is only used a cookie - it should not be dereferenced */
-static struct pci_dev *vga_default;
-
 static void vga_arb_device_card_gone(struct pci_dev *pdev);
 
 /* Find somebody in our list */
@@ -135,39 +133,6 @@ static struct vga_device *vgadev_find(struct pci_dev *pdev)
 	return NULL;
 }
 
-/**
- * vga_default_device - return the default VGA device, for vgacon
- *
- * This can be defined by the platform. The default implementation
- * is rather dumb and will probably only work properly on single
- * vga card setups and/or x86 platforms.
- *
- * If your VGA default device is not PCI, you'll have to return
- * NULL here. In this case, I assume it will not conflict with
- * any PCI card. If this is not true, I'll have to define two archs
- * hooks for enabling/disabling the VGA default device if that is
- * possible. This may be a problem with real _ISA_ VGA cards, in
- * addition to a PCI one. I don't know at this point how to deal
- * with that card. Can theirs IOs be disabled at all ? If not, then
- * I suppose it's a matter of having the proper arch hook telling
- * us about it, so we basically never allow anybody to succeed a
- * vga_get()...
- */
-struct pci_dev *vga_default_device(void)
-{
-	return vga_default;
-}
-EXPORT_SYMBOL_GPL(vga_default_device);
-
-void vga_set_default_device(struct pci_dev *pdev)
-{
-	if (vga_default == pdev)
-		return;
-
-	pci_dev_put(vga_default);
-	vga_default = pci_dev_get(pdev);
-}
-
 static inline void vga_irq_set_state(struct vga_device *vgadev, bool state)
 {
 	if (vgadev->irq_set_state)
@@ -667,7 +632,7 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
 	/* Deal with VGA default device. Use first enabled one
 	 * by default if arch doesn't have it's own hook
 	 */
-	if (vga_default == NULL &&
+	if (vga_default_device() == NULL &&
 	    ((vgadev->owns & VGA_RSRC_LEGACY_MASK) == VGA_RSRC_LEGACY_MASK)) {
 		vgaarb_info(&pdev->dev, "setting as boot VGA device\n");
 		vga_set_default_device(pdev);
@@ -704,7 +669,7 @@ static bool vga_arbiter_del_pci_device(struct pci_dev *pdev)
 		goto bail;
 	}
 
-	if (vga_default == pdev)
+	if (vga_default_device() == pdev)
 		vga_set_default_device(NULL);
 
 	if (vgadev->decodes & (VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM))
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 2f3780b50723..c174b427ea2b 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -27,7 +27,7 @@
 #include <linux/security.h>
 #include <linux/pci-aspm.h>
 #include <linux/slab.h>
-#include <linux/vgaarb.h>
+#include <linux/vga_default.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
 #include "pci.h"
diff --git a/include/linux/vga_default.h b/include/linux/vga_default.h
new file mode 100644
index 000000000000..78df6a2a194c
--- /dev/null
+++ b/include/linux/vga_default.h
@@ -0,0 +1,44 @@
+/*
+ * vga_default.h: What is the default/boot PCI VGA device?
+ *
+ * (C) Copyright 2005 Benjamin Herrenschmidt <benh@kernel.crashing.org>
+ * (C) Copyright 2007 Paulo R. Zanoni <przanoni@gmail.com>
+ * (C) Copyright 2007, 2009 Tiago Vignatti <vignatti@freedesktop.org>
+ * (C) Copyright 2017 Canonical Ltd. (Author: Daniel Axtens <dja@axtens.net>)
+ *
+ * (License from vgaarb.h)
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef LINUX_VGA_DEFAULT_H
+#define LINUX_VGA_DEFAULT_H
+
+struct pci_dev;
+
+#ifdef CONFIG_VGA_DEFAULT
+extern struct pci_dev *vga_default_device(void);
+extern void vga_set_default_device(struct pci_dev *pdev);
+#else
+static inline struct pci_dev *vga_default_device(void) { return NULL; };
+static inline void vga_set_default_device(struct pci_dev *pdev) { };
+#endif
+
+#endif
diff --git a/include/linux/vgaarb.h b/include/linux/vgaarb.h
index ee162e3e879b..953e1955efbe 100644
--- a/include/linux/vgaarb.h
+++ b/include/linux/vgaarb.h
@@ -42,12 +42,6 @@
 #define VGA_RSRC_NORMAL_IO     0x04
 #define VGA_RSRC_NORMAL_MEM    0x08
 
-/* Passing that instead of a pci_dev to use the system "default"
- * device, that is the one used by vgacon. Archs will probably
- * have to provide their own vga_default_device();
- */
-#define VGA_DEFAULT_DEVICE     (NULL)
-
 struct pci_dev;
 
 /* For use by clients */
@@ -122,14 +116,6 @@ extern void vga_put(struct pci_dev *pdev, unsigned int rsrc);
 #endif
 
 
-#ifdef CONFIG_VGA_ARB
-extern struct pci_dev *vga_default_device(void);
-extern void vga_set_default_device(struct pci_dev *pdev);
-#else
-static inline struct pci_dev *vga_default_device(void) { return NULL; };
-static inline void vga_set_default_device(struct pci_dev *pdev) { };
-#endif
-
 /*
  * Architectures should define this if they have several
  * independent PCI domains that can afford concurrent VGA
-- 
2.11.0

Re: [PATCH 2/3] Split VGA default device handler out of VGA arbiter

[Lukas Wunner]

On Fri, Aug 04, 2017 at 08:20:32PM +1000, Daniel Axtens wrote:
> A system without PCI legacy resources (e.g. ARM64, powerpc) may
> find that no default/boot VGA device has been marked, because the
> VGA arbiter checks for legacy resource decoding before marking a
> card as default.

Would it be feasible to just sprinkle some "if (IS_ENABLED(CONFIG_X86))"
over the portions of vgaarb.c that deal with legacy resources?

I'm missing some context as to the negative consequences you're
experiencing on other arches (the cover letter merely refers to
"quirks in ppc and concerns in arm land" and is missing links to
archived versions of v1 and v2), but clearly legacy resources are
x86-specific so any code dealing with them seems superfluous on
other arches and should be marked as such.

Please cc dri-devel when proposing changes to vgaarb.c (see
MAINTAINERS).

Thanks,

Lukas

[PATCH 2/3] Split VGA default device handler out of VGA arbiter

[Lukas Wunner]

On Fri, Aug 04, 2017 at 08:20:32PM +1000, Daniel Axtens wrote:
> A system without PCI legacy resources (e.g. ARM64, powerpc) may
> find that no default/boot VGA device has been marked, because the
> VGA arbiter checks for legacy resource decoding before marking a
> card as default.

Would it be feasible to just sprinkle some "if (IS_ENABLED(CONFIG_X86))"
over the portions of vgaarb.c that deal with legacy resources?

I'm missing some context as to the negative consequences you're
experiencing on other arches (the cover letter merely refers to
"quirks in ppc and concerns in arm land" and is missing links to
archived versions of v1 and v2), but clearly legacy resources are
x86-specific so any code dealing with them seems superfluous on
other arches and should be marked as such.

Please cc dri-devel when proposing changes to vgaarb.c (see
MAINTAINERS).

Thanks,

Lukas

Re: [PATCH 2/3] Split VGA default device handler out of VGA arbiter

[Daniel Axtens]

Hi Lukas,

Let me reply "back-to-front":

> Please cc dri-devel when proposing changes to vgaarb.c (see
> MAINTAINERS).

Sorry, will do in future.

> I'm missing some context as to the negative consequences you're
> experiencing on other arches (the cover letter merely refers to
> "quirks in ppc and concerns in arm land" and is missing links to
> archived versions of v1 and v2), but clearly legacy resources are
> x86-specific so any code dealing with them seems superfluous on
> other arches and should be marked as such.

Quirk/v1: https://www.spinics.net/lists/linux-pci/msg62865.html
Approach 2: https://www.spinics.net/lists/linux-pci/msg63092.html
            https://www.spinics.net/lists/linux-pci/msg63083.html

The quirk in powerpc land that I'm referring to is
arch/powerpc/kernel/pci-common.c::fixup_vga() which is a class enable
hook that marks the first VGA device it comes across as default if the
arbiter has not marked one.

The arm concerns are ventaliated in the threads I linked - they boil
down to "why do we need the arbiter at all on platforms without legacy
resources? Can't we do this more simply?"

> Would it be feasible to just sprinkle some "if (IS_ENABLED(CONFIG_X86))"
> over the portions of vgaarb.c that deal with legacy resources?

Yes and no. That would disable the unnecessary chunks of the VGA
arbiter, but we then still have to figure out how to mark devices as
default if not with the arbiter.

Regards,
Daniel

>
> Thanks,
>
> Lukas

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] Split VGA default device handler out of VGA arbiter

[Daniel Axtens]

Hi Lukas,

Let me reply "back-to-front":

> Please cc dri-devel when proposing changes to vgaarb.c (see
> MAINTAINERS).

Sorry, will do in future.

> I'm missing some context as to the negative consequences you're
> experiencing on other arches (the cover letter merely refers to
> "quirks in ppc and concerns in arm land" and is missing links to
> archived versions of v1 and v2), but clearly legacy resources are
> x86-specific so any code dealing with them seems superfluous on
> other arches and should be marked as such.

Quirk/v1: https://www.spinics.net/lists/linux-pci/msg62865.html
Approach 2: https://www.spinics.net/lists/linux-pci/msg63092.html
            https://www.spinics.net/lists/linux-pci/msg63083.html

The quirk in powerpc land that I'm referring to is
arch/powerpc/kernel/pci-common.c::fixup_vga() which is a class enable
hook that marks the first VGA device it comes across as default if the
arbiter has not marked one.

The arm concerns are ventaliated in the threads I linked - they boil
down to "why do we need the arbiter at all on platforms without legacy
resources? Can't we do this more simply?"

> Would it be feasible to just sprinkle some "if (IS_ENABLED(CONFIG_X86))"
> over the portions of vgaarb.c that deal with legacy resources?

Yes and no. That would disable the unnecessary chunks of the VGA
arbiter, but we then still have to figure out how to mark devices as
default if not with the arbiter.

Regards,
Daniel

>
> Thanks,
>
> Lukas

Re: [PATCH 2/3] Split VGA default device handler out of VGA arbiter

[Daniel Axtens]

Hi Lukas,

Let me reply "back-to-front":

> Please cc dri-devel when proposing changes to vgaarb.c (see
> MAINTAINERS).

Sorry, will do in future.

> I'm missing some context as to the negative consequences you're
> experiencing on other arches (the cover letter merely refers to
> "quirks in ppc and concerns in arm land" and is missing links to
> archived versions of v1 and v2), but clearly legacy resources are
> x86-specific so any code dealing with them seems superfluous on
> other arches and should be marked as such.

Quirk/v1: https://www.spinics.net/lists/linux-pci/msg62865.html
Approach 2: https://www.spinics.net/lists/linux-pci/msg63092.html
            https://www.spinics.net/lists/linux-pci/msg63083.html

The quirk in powerpc land that I'm referring to is
arch/powerpc/kernel/pci-common.c::fixup_vga() which is a class enable
hook that marks the first VGA device it comes across as default if the
arbiter has not marked one.

The arm concerns are ventaliated in the threads I linked - they boil
down to "why do we need the arbiter at all on platforms without legacy
resources? Can't we do this more simply?"

> Would it be feasible to just sprinkle some "if (IS_ENABLED(CONFIG_X86))"
> over the portions of vgaarb.c that deal with legacy resources?

Yes and no. That would disable the unnecessary chunks of the VGA
arbiter, but we then still have to figure out how to mark devices as
default if not with the arbiter.

Regards,
Daniel

>
> Thanks,
>
> Lukas

[PATCH 3/3] powerpc: replace vga_fixup() with generic code

[Daniel Axtens]

Currently, we do a PCI fixup to mark a default card so that Xorg
autoconfiguration works.

There is a new generic method to do this sort of vga fixup, and
it occurs by default.

Drop our old method.

This method is different:
 - it will only mark a card as default if a driver is bound
 - the marking will happen at late_initcall time, or even later
   if a card is enabled later on (via an ENABLE hook). Currently
   things are enabled in a FINAL hook.

This *does* change behaviour under some circumstances.

For example, pseries_le_defconfig doesn't have DRM drivers for
many of the qemu GPU models, including the 'standard' vga. So
when a VM with that GPU boots, no driver binds the GPU, and
it does *not* get marked as default. Previously, it would have
been marked as default.

As it turns out Xorg (at least Xorg v1.19.3) can still
autoconfigure it, as Xorg is smart about OpenFirmware
framebuffer devices.

If the right GPU driver is available, and the OpenFirmware fb
driver is removed, the device *is* marked as a boot GPU. (If the
OpenFirmware driver is around, it enables the PCI device but
doesn't bind to it, making it ineligible to be the default card.
Then, when the right driver is loaded, the enable hook doesn't fire
because the card has already been enabled. Fun!) So everything
works as intended, I guess.

Cc: Brian King <brking@linux.vnet.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>

---

This would benefit from some tests on real hardware.
---
 arch/powerpc/kernel/pci-common.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 6cfaec107374..65cd5bad5ad6 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -31,7 +31,6 @@
 #include <linux/irq.h>
 #include <linux/vmalloc.h>
 #include <linux/slab.h>
-#include <linux/vga_default.h>
 
 #include <asm/processor.h>
 #include <asm/io.h>
@@ -1741,18 +1740,3 @@ static void fixup_hide_host_resource_fsl(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl);
-
-static void fixup_vga(struct pci_dev *pdev)
-{
-	u16 cmd;
-
-	if (vga_default_device())
-		return;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
-	if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
-		vga_set_default_device(pdev);
-
-}
-DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
-			      PCI_CLASS_DISPLAY_VGA, 8, fixup_vga);
-- 
2.11.0

[PATCH 3/3] powerpc: replace vga_fixup() with generic code

[Daniel Axtens]

Currently, we do a PCI fixup to mark a default card so that Xorg
autoconfiguration works.

There is a new generic method to do this sort of vga fixup, and
it occurs by default.

Drop our old method.

This method is different:
 - it will only mark a card as default if a driver is bound
 - the marking will happen at late_initcall time, or even later
   if a card is enabled later on (via an ENABLE hook). Currently
   things are enabled in a FINAL hook.

This *does* change behaviour under some circumstances.

For example, pseries_le_defconfig doesn't have DRM drivers for
many of the qemu GPU models, including the 'standard' vga. So
when a VM with that GPU boots, no driver binds the GPU, and
it does *not* get marked as default. Previously, it would have
been marked as default.

As it turns out Xorg (at least Xorg v1.19.3) can still
autoconfigure it, as Xorg is smart about OpenFirmware
framebuffer devices.

If the right GPU driver is available, and the OpenFirmware fb
driver is removed, the device *is* marked as a boot GPU. (If the
OpenFirmware driver is around, it enables the PCI device but
doesn't bind to it, making it ineligible to be the default card.
Then, when the right driver is loaded, the enable hook doesn't fire
because the card has already been enabled. Fun!) So everything
works as intended, I guess.

Cc: Brian King <brking@linux.vnet.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>

---

This would benefit from some tests on real hardware.
---
 arch/powerpc/kernel/pci-common.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 6cfaec107374..65cd5bad5ad6 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -31,7 +31,6 @@
 #include <linux/irq.h>
 #include <linux/vmalloc.h>
 #include <linux/slab.h>
-#include <linux/vga_default.h>
 
 #include <asm/processor.h>
 #include <asm/io.h>
@@ -1741,18 +1740,3 @@ static void fixup_hide_host_resource_fsl(struct pci_dev *dev)
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl);
 DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl);
-
-static void fixup_vga(struct pci_dev *pdev)
-{
-	u16 cmd;
-
-	if (vga_default_device())
-		return;
-
-	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
-	if (cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY))
-		vga_set_default_device(pdev);
-
-}
-DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
-			      PCI_CLASS_DISPLAY_VGA, 8, fixup_vga);
-- 
2.11.0

Re: [PATCH 3/3] powerpc: replace vga_fixup() with generic code

[Michael Ellerman]

Daniel Axtens <dja@axtens.net> writes:

> Currently, we do a PCI fixup to mark a default card so that Xorg
> autoconfiguration works.
>
> There is a new generic method to do this sort of vga fixup, and
> it occurs by default.
>
> Drop our old method.
>
> This method is different:
>  - it will only mark a card as default if a driver is bound
>  - the marking will happen at late_initcall time, or even later
>    if a card is enabled later on (via an ENABLE hook). Currently
>    things are enabled in a FINAL hook.
>
> This *does* change behaviour under some circumstances.
>
> For example, pseries_le_defconfig doesn't have DRM drivers for
> many of the qemu GPU models, including the 'standard' vga.

Should we enable them/it?

cheers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] powerpc: replace vga_fixup() with generic code

[Michael Ellerman]

Daniel Axtens <dja@axtens.net> writes:

> Currently, we do a PCI fixup to mark a default card so that Xorg
> autoconfiguration works.
>
> There is a new generic method to do this sort of vga fixup, and
> it occurs by default.
>
> Drop our old method.
>
> This method is different:
>  - it will only mark a card as default if a driver is bound
>  - the marking will happen at late_initcall time, or even later
>    if a card is enabled later on (via an ENABLE hook). Currently
>    things are enabled in a FINAL hook.
>
> This *does* change behaviour under some circumstances.
>
> For example, pseries_le_defconfig doesn't have DRM drivers for
> many of the qemu GPU models, including the 'standard' vga.

Should we enable them/it?

cheers

Re: [PATCH 3/3] powerpc: replace vga_fixup() with generic code

[Michael Ellerman]

Daniel Axtens <dja@axtens.net> writes:

> Currently, we do a PCI fixup to mark a default card so that Xorg
> autoconfiguration works.
>
> There is a new generic method to do this sort of vga fixup, and
> it occurs by default.
>
> Drop our old method.
>
> This method is different:
>  - it will only mark a card as default if a driver is bound
>  - the marking will happen at late_initcall time, or even later
>    if a card is enabled later on (via an ENABLE hook). Currently
>    things are enabled in a FINAL hook.
>
> This *does* change behaviour under some circumstances.
>
> For example, pseries_le_defconfig doesn't have DRM drivers for
> many of the qemu GPU models, including the 'standard' vga.

Should we enable them/it?

cheers

Re: [PATCH 3/3] powerpc: replace vga_fixup() with generic code

[Daniel Axtens]

Michael Ellerman <mpe@ellerman.id.au> writes:

> Daniel Axtens <dja@axtens.net> writes:
>
>> Currently, we do a PCI fixup to mark a default card so that Xorg
>> autoconfiguration works.
>>
>> There is a new generic method to do this sort of vga fixup, and
>> it occurs by default.
>>
>> Drop our old method.
>>
>> This method is different:
>>  - it will only mark a card as default if a driver is bound
>>  - the marking will happen at late_initcall time, or even later
>>    if a card is enabled later on (via an ENABLE hook). Currently
>>    things are enabled in a FINAL hook.
>>
>> This *does* change behaviour under some circumstances.
>>
>> For example, pseries_le_defconfig doesn't have DRM drivers for
>> many of the qemu GPU models, including the 'standard' vga.
>
> Should we enable them/it?

Hard to say.

The 'standard' vga module (bochs_drm) was blacklisted by Ubuntu -
apparently at IBM's request [0] - some years back. Even if you
un-blacklist it, I had trouble with getting it and the openfirmware
framebuffer driver to play nicely together. It may not be worth the
trouble for bochs_drm.

There's a better case for including some of the more modern drivers -
maybe QXL and virtio - but I wasn't able to test them: my particular
build of qemu/TCG refused to start with them and I didn't feel like
rebuilding/debugging qemu.

It would also be legitmate to say that you're focussing on headless use
with pseries_*defconfig and not include them: you need to bring in the
DRM core if you want these drivers.

HTH.

Regards,
Daniel

[0] https://bugs.launchpad.net/ubuntu/+source/kmod/+bug/1378648

>
> cheers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] powerpc: replace vga_fixup() with generic code

[Daniel Axtens]

Michael Ellerman <mpe@ellerman.id.au> writes:

> Daniel Axtens <dja@axtens.net> writes:
>
>> Currently, we do a PCI fixup to mark a default card so that Xorg
>> autoconfiguration works.
>>
>> There is a new generic method to do this sort of vga fixup, and
>> it occurs by default.
>>
>> Drop our old method.
>>
>> This method is different:
>>  - it will only mark a card as default if a driver is bound
>>  - the marking will happen at late_initcall time, or even later
>>    if a card is enabled later on (via an ENABLE hook). Currently
>>    things are enabled in a FINAL hook.
>>
>> This *does* change behaviour under some circumstances.
>>
>> For example, pseries_le_defconfig doesn't have DRM drivers for
>> many of the qemu GPU models, including the 'standard' vga.
>
> Should we enable them/it?

Hard to say.

The 'standard' vga module (bochs_drm) was blacklisted by Ubuntu -
apparently at IBM's request [0] - some years back. Even if you
un-blacklist it, I had trouble with getting it and the openfirmware
framebuffer driver to play nicely together. It may not be worth the
trouble for bochs_drm.

There's a better case for including some of the more modern drivers -
maybe QXL and virtio - but I wasn't able to test them: my particular
build of qemu/TCG refused to start with them and I didn't feel like
rebuilding/debugging qemu.

It would also be legitmate to say that you're focussing on headless use
with pseries_*defconfig and not include them: you need to bring in the
DRM core if you want these drivers.

HTH.

Regards,
Daniel

[0] https://bugs.launchpad.net/ubuntu/+source/kmod/+bug/1378648

>
> cheers

Re: [PATCH 3/3] powerpc: replace vga_fixup() with generic code

[Daniel Axtens]

Michael Ellerman <mpe@ellerman.id.au> writes:

> Daniel Axtens <dja@axtens.net> writes:
>
>> Currently, we do a PCI fixup to mark a default card so that Xorg
>> autoconfiguration works.
>>
>> There is a new generic method to do this sort of vga fixup, and
>> it occurs by default.
>>
>> Drop our old method.
>>
>> This method is different:
>>  - it will only mark a card as default if a driver is bound
>>  - the marking will happen at late_initcall time, or even later
>>    if a card is enabled later on (via an ENABLE hook). Currently
>>    things are enabled in a FINAL hook.
>>
>> This *does* change behaviour under some circumstances.
>>
>> For example, pseries_le_defconfig doesn't have DRM drivers for
>> many of the qemu GPU models, including the 'standard' vga.
>
> Should we enable them/it?

Hard to say.

The 'standard' vga module (bochs_drm) was blacklisted by Ubuntu -
apparently at IBM's request [0] - some years back. Even if you
un-blacklist it, I had trouble with getting it and the openfirmware
framebuffer driver to play nicely together. It may not be worth the
trouble for bochs_drm.

There's a better case for including some of the more modern drivers -
maybe QXL and virtio - but I wasn't able to test them: my particular
build of qemu/TCG refused to start with them and I didn't feel like
rebuilding/debugging qemu.

It would also be legitmate to say that you're focussing on headless use
with pseries_*defconfig and not include them: you need to bring in the
DRM core if you want these drivers.

HTH.

Regards,
Daniel

[0] https://bugs.launchpad.net/ubuntu/+source/kmod/+bug/1378648

>
> cheers

Re: [PATCH 3/3] powerpc: replace vga_fixup() with generic code

[Michael Ellerman]

Daniel Axtens <dja@axtens.net> writes:

> Michael Ellerman <mpe@ellerman.id.au> writes:
>
>> Daniel Axtens <dja@axtens.net> writes:
>>
>>> Currently, we do a PCI fixup to mark a default card so that Xorg
>>> autoconfiguration works.
>>>
>>> There is a new generic method to do this sort of vga fixup, and
>>> it occurs by default.
>>>
>>> Drop our old method.
>>>
>>> This method is different:
>>>  - it will only mark a card as default if a driver is bound
>>>  - the marking will happen at late_initcall time, or even later
>>>    if a card is enabled later on (via an ENABLE hook). Currently
>>>    things are enabled in a FINAL hook.
>>>
>>> This *does* change behaviour under some circumstances.
>>>
>>> For example, pseries_le_defconfig doesn't have DRM drivers for
>>> many of the qemu GPU models, including the 'standard' vga.
>>
>> Should we enable them/it?
>
> Hard to say.
>
> The 'standard' vga module (bochs_drm) was blacklisted by Ubuntu -
> apparently at IBM's request [0] - some years back. Even if you
> un-blacklist it, I had trouble with getting it and the openfirmware
> framebuffer driver to play nicely together. It may not be worth the
> trouble for bochs_drm.
>
> There's a better case for including some of the more modern drivers -
> maybe QXL and virtio - but I wasn't able to test them: my particular
> build of qemu/TCG refused to start with them and I didn't feel like
> rebuilding/debugging qemu.

Yeah OK. Sounds like a bit of mess :)

I'll leave it unless someone who knows Qemu/Gfx etc. tells me otherwise.

> It would also be legitmate to say that you're focussing on headless use
> with pseries_*defconfig and not include them: you need to bring in the
> DRM core if you want these drivers.

True. There's a bit of a tension there between making them useful
configs for developers and also turning on as much code as possible so
it gets tested.

Arguably we should have DRM enabled because the distros will.

cheers

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] powerpc: replace vga_fixup() with generic code

[Michael Ellerman]

Daniel Axtens <dja@axtens.net> writes:

> Michael Ellerman <mpe@ellerman.id.au> writes:
>
>> Daniel Axtens <dja@axtens.net> writes:
>>
>>> Currently, we do a PCI fixup to mark a default card so that Xorg
>>> autoconfiguration works.
>>>
>>> There is a new generic method to do this sort of vga fixup, and
>>> it occurs by default.
>>>
>>> Drop our old method.
>>>
>>> This method is different:
>>>  - it will only mark a card as default if a driver is bound
>>>  - the marking will happen at late_initcall time, or even later
>>>    if a card is enabled later on (via an ENABLE hook). Currently
>>>    things are enabled in a FINAL hook.
>>>
>>> This *does* change behaviour under some circumstances.
>>>
>>> For example, pseries_le_defconfig doesn't have DRM drivers for
>>> many of the qemu GPU models, including the 'standard' vga.
>>
>> Should we enable them/it?
>
> Hard to say.
>
> The 'standard' vga module (bochs_drm) was blacklisted by Ubuntu -
> apparently at IBM's request [0] - some years back. Even if you
> un-blacklist it, I had trouble with getting it and the openfirmware
> framebuffer driver to play nicely together. It may not be worth the
> trouble for bochs_drm.
>
> There's a better case for including some of the more modern drivers -
> maybe QXL and virtio - but I wasn't able to test them: my particular
> build of qemu/TCG refused to start with them and I didn't feel like
> rebuilding/debugging qemu.

Yeah OK. Sounds like a bit of mess :)

I'll leave it unless someone who knows Qemu/Gfx etc. tells me otherwise.

> It would also be legitmate to say that you're focussing on headless use
> with pseries_*defconfig and not include them: you need to bring in the
> DRM core if you want these drivers.

True. There's a bit of a tension there between making them useful
configs for developers and also turning on as much code as possible so
it gets tested.

Arguably we should have DRM enabled because the distros will.

cheers

Re: [PATCH 3/3] powerpc: replace vga_fixup() with generic code

[Michael Ellerman]

Daniel Axtens <dja@axtens.net> writes:

> Michael Ellerman <mpe@ellerman.id.au> writes:
>
>> Daniel Axtens <dja@axtens.net> writes:
>>
>>> Currently, we do a PCI fixup to mark a default card so that Xorg
>>> autoconfiguration works.
>>>
>>> There is a new generic method to do this sort of vga fixup, and
>>> it occurs by default.
>>>
>>> Drop our old method.
>>>
>>> This method is different:
>>>  - it will only mark a card as default if a driver is bound
>>>  - the marking will happen at late_initcall time, or even later
>>>    if a card is enabled later on (via an ENABLE hook). Currently
>>>    things are enabled in a FINAL hook.
>>>
>>> This *does* change behaviour under some circumstances.
>>>
>>> For example, pseries_le_defconfig doesn't have DRM drivers for
>>> many of the qemu GPU models, including the 'standard' vga.
>>
>> Should we enable them/it?
>
> Hard to say.
>
> The 'standard' vga module (bochs_drm) was blacklisted by Ubuntu -
> apparently at IBM's request [0] - some years back. Even if you
> un-blacklist it, I had trouble with getting it and the openfirmware
> framebuffer driver to play nicely together. It may not be worth the
> trouble for bochs_drm.
>
> There's a better case for including some of the more modern drivers -
> maybe QXL and virtio - but I wasn't able to test them: my particular
> build of qemu/TCG refused to start with them and I didn't feel like
> rebuilding/debugging qemu.

Yeah OK. Sounds like a bit of mess :)

I'll leave it unless someone who knows Qemu/Gfx etc. tells me otherwise.

> It would also be legitmate to say that you're focussing on headless use
> with pseries_*defconfig and not include them: you need to bring in the
> DRM core if you want these drivers.

True. There's a bit of a tension there between making them useful
configs for developers and also turning on as much code as possible so
it gets tested.

Arguably we should have DRM enabled because the distros will.

cheers