[PATCH] drm/qxl: unbind vgacon

[PATCH] drm/qxl: unbind vgacon

[Gerd Hoffmann]

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@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

Re: [PATCH] drm/qxl: unbind vgacon

[Daniel Vetter]

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@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

Re: [PATCH] drm/qxl: unbind vgacon

[Gerd Hoffmann]

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

Re: [PATCH] drm/qxl: unbind vgacon

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29867 bytes --]

Re: [PATCH] drm/qxl: unbind vgacon

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33583 bytes --]