Problem: qxl switches from native mode back into vga compatibility mode when it notices someone is accessing vga registers. And vgacon does exactly that before fbcon takes over. Before qxl switched to the generic fbdev emulation that didn't cause any problems. With the generic fbdev emulation the switch to vga mode happens now and then, probably caused by a initialization order change and triggered by a printk in a bad moment. So make sure we take vgacon out of the picture by making dummycon taking over the console early enough. Not entriely happy with the approach, I'm open to better ideas. Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> --- drivers/gpu/drm/qxl/qxl_drv.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c index bb81e310eb..88349dc13e 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.c +++ b/drivers/gpu/drm/qxl/qxl_drv.c @@ -30,6 +30,7 @@ #include <linux/module.h> #include <linux/console.h> +#include <linux/vt_kern.h> #include <drm/drmP.h> #include <drm/drm.h> @@ -89,6 +90,11 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) drm_kms_helper_poll_init(&qdev->ddev); + /* unbind vgacon to make sure it doesn't touch our vga registers */ + console_lock(); + ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, true); + console_unlock(); + /* Complete initialization. */ ret = drm_dev_register(&qdev->ddev, ent->driver_data); if (ret) -- 2.9.3
On Wed, Feb 20, 2019 at 03:36:40PM +0100, Gerd Hoffmann wrote:> Problem: qxl switches from native mode back into vga compatibility mode > when it notices someone is accessing vga registers. And vgacon does > exactly that before fbcon takes over. > > Before qxl switched to the generic fbdev emulation that didn't cause any > problems. With the generic fbdev emulation the switch to vga mode > happens now and then, probably caused by a initialization order change > and triggered by a printk in a bad moment. > > So make sure we take vgacon out of the picture by making dummycon > taking over the console early enough. > > Not entriely happy with the approach, I'm open to better ideas. > > Signed-off-by: Gerd Hoffmann <kraxel at redhat.com> > --- > drivers/gpu/drm/qxl/qxl_drv.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c > index bb81e310eb..88349dc13e 100644 > --- a/drivers/gpu/drm/qxl/qxl_drv.c > +++ b/drivers/gpu/drm/qxl/qxl_drv.c > @@ -30,6 +30,7 @@ > > #include <linux/module.h> > #include <linux/console.h> > +#include <linux/vt_kern.h> > > #include <drm/drmP.h> > #include <drm/drm.h> > @@ -89,6 +90,11 @@ qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > drm_kms_helper_poll_init(&qdev->ddev); > > + /* unbind vgacon to make sure it doesn't touch our vga registers */ > + console_lock(); > + ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, true); > + console_unlock();Still seems very late, in i915 we kick out vgacon as pretty much the first thing in driver load. See i915_kick_out_vgacon. I wonder whether we should integrate that logic into drm_fb_helper_remove_conflicting_pci_framebuffers, by checking whether that pci device can decode VGA and kick out vgacon in that case. Instead of sprinkling the same logic over all drivers. -Daniel> + > /* Complete initialization. */ > ret = drm_dev_register(&qdev->ddev, ent->driver_data); > if (ret) > -- > 2.9.3 >-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
> > + /* unbind vgacon to make sure it doesn't touch our vga registers */ > > + console_lock(); > > + ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1, true); > > + console_unlock(); > > Still seems very late, in i915 we kick out vgacon as pretty much the first > thing in driver load. See i915_kick_out_vgacon.So the idea isn't completely silly ... thanks for the pointer, Gerd
Hi Gerd,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.0-rc4 next-20190221]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Gerd-Hoffmann/drm-qxl-unbind-vgacon/20190222-030117
config: x86_64-randconfig-l3-02212045 (attached as .config)
compiler: gcc-5 (Debian 5.5.0-3) 5.4.1 20171010
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
>> ERROR: "dummy_con" [drivers/gpu/drm/qxl/qxl.ko] undefined!
>> ERROR: "do_take_over_console" [drivers/gpu/drm/qxl/qxl.ko]
undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 29867 bytes
Desc: not available
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20190222/c4b0fd62/attachment-0001.bin>
Hi Gerd,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.0-rc4 next-20190221]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Gerd-Hoffmann/drm-qxl-unbind-vgacon/20190222-030117
config: x86_64-randconfig-m2-02211051 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
ld: drivers/gpu/drm/qxl/qxl_drv.o: in function
`qxl_pci_probe':>> drivers/gpu/drm/qxl/qxl_drv.c:94: undefined reference to
`dummy_con'
>> ld: drivers/gpu/drm/qxl/qxl_drv.c:94: undefined reference to
`do_take_over_console'
vim +94 drivers/gpu/drm/qxl/qxl_drv.c
61
62 static int
63 qxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
64 {
65 struct qxl_device *qdev;
66 int ret;
67
68 if (pdev->revision < 4) {
69 DRM_ERROR("qxl too old, doesn't support
client_monitors_config,"
70 " use xf86-video-qxl in user mode");
71 return -EINVAL; /* TODO: ENODEV ? */
72 }
73
74 qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
75 if (!qdev)
76 return -ENOMEM;
77
78 ret = pci_enable_device(pdev);
79 if (ret)
80 goto free_dev;
81
82 ret = qxl_device_init(qdev, &qxl_driver, pdev);
83 if (ret)
84 goto disable_pci;
85
86 ret = qxl_modeset_init(qdev);
87 if (ret)
88 goto unload;
89
90 drm_kms_helper_poll_init(&qdev->ddev);
91
92 /* unbind vgacon to make sure it doesn't touch our vga registers */
93 console_lock();
> 94 ret = do_take_over_console(&dummy_con, 0, MAX_NR_CONSOLES - 1,
true);
95 console_unlock();
96
97 /* Complete initialization. */
98 ret = drm_dev_register(&qdev->ddev, ent->driver_data);
99 if (ret)
100 goto modeset_cleanup;
101
102 return 0;
103
104 modeset_cleanup:
105 qxl_modeset_fini(qdev);
106 unload:
107 qxl_device_fini(qdev);
108 disable_pci:
109 pci_disable_device(pdev);
110 free_dev:
111 kfree(qdev);
112 return ret;
113 }
114
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: .config.gz
Type: application/gzip
Size: 33583 bytes
Desc: not available
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20190222/5fc8d86d/attachment-0001.bin>