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

Submitted by Daniel Axtens on Aug. 4, 2017, 10:20 a.m.

Details

Message ID 20170804102033.27731-4-dja@axtens.net
State Changes Requested
Headers show

Commit Message

Daniel Axtens Aug. 4, 2017, 10:20 a.m.
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(-)

Comments

Michael Ellerman Aug. 7, 2017, 7:18 a.m.
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
Daniel Axtens Aug. 7, 2017, 11:01 p.m.
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
Michael Ellerman Aug. 8, 2017, 10:12 a.m.
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

Patch hide | download patch | download mbox

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);