2019-02-20 14:37:47

by Gerd Hoffmann

[permalink] [raw]
Subject: [PATCH] drm/qxl: unbind vgacon

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 <[email protected]>
---
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



2019-02-21 09:58:59

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] drm/qxl: unbind vgacon

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 <[email protected]>
> ---
> 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

2019-02-21 11:28:34

by Gerd Hoffmann

[permalink] [raw]
Subject: Re: [PATCH] drm/qxl: unbind vgacon

> > + /* 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


2019-02-21 23:46:01

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] drm/qxl: unbind vgacon

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


Attachments:
(No filename) (907.00 B)
.config.gz (29.17 kB)
Download all attachments

2019-02-22 01:41:29

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] drm/qxl: unbind vgacon

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


Attachments:
(No filename) (2.50 kB)
.config.gz (32.80 kB)
Download all attachments