Re: VT console blank ignored by DRM drivers on QEMU

[Takashi Iwai <tiwai@suse.de>]

On Mon, 10 Jul 2017 11:27:01 +0200,
Daniel Vetter wrote:
> 
> On Mon, Jul 10, 2017 at 10:53 AM, Takashi Iwai <tiwai@suse.de> wrote:
> > we've casually found a weird behavior of DRM drivers on QEMU (cirrus,
> > bochs, virtio) via openQA: namely, VT console blank is ignored on such
> > drivers.  The whole screen is kept shown while the cursor blinking
> > stops.
> >
> > I took a closer look, and it seems that drm_fb_helper_blank() just
> > calls drm_fb_helper_dpms(), by a naive assumption that every driver
> > properly implements DPMS handling.  Meanwhile bochs driver has a
> > code to ignore the whole DPMS hilariously.  Ditto for virtio.
> >
> > (The cirrus is a bit different story; it has a DPMS implementation,
> >  but QEMU side seems ignoring it.)
> >
> > In the fbcon side, there is a fallback to the explicit clear when the
> > fb_blank() returns an error, so we should be able to handle it if we
> > return an error properly.  But the dpms callback is a void function,
> > so the driver doesn't tell it for now.
> >
> >
> > I guess we have several options to address it.  An easy one would be
> > to provide an own fb_blank function for returning an error and use it
> > for the drivers for VM.  The driver can't use any longer
> > DRM_FB_HELPER_DEFAULT_OPS, thus it'll a bit ugly, though.
> >
> > Another is to change dpms callback allowing to return an error.  But
> > it'd affect so many codes.
> >
> > Yet another option would be to define some flag and let
> > drm_fb_helper_blank() returns an error.  But I also hesitate to do it
> > just for such a workaround.
> 
> DPMS should be an error anyway, we want that to be able to properly
> thread the acquire_ctx EDEADLK backoff stuff through that we need for
> atomic. That would be the best long-term plan I think.

So it implies the conversions of the whole legacy stuff?
That'd be great but take a long way :)

> But aside from that, can't we just teach these drivers to properly do
> dpms? With the atomic framework dpms is implement as simply turning
> the screen off, any driver should be able to support that properly.

It seems that QEMU doesn't support it yet?  We'd need to implement it
at first there.

> For the fbcon issue, can we perhaps just unconditionally ask fbcon to
> clear the screen when blanking? It's not really perf critical, so
> doing that for everyone shouldn't hurt.

I quickly hacked the code and the patch below seems working.
If this kind of change is acceptable, I'll cook up and submit a proper
patch.


thanks,

Takashi

--- a/drivers/gpu/drm/bochs/bochs_fbdev.c
+++ b/drivers/gpu/drm/bochs/bochs_fbdev.c
@@ -22,7 +22,17 @@ static int bochsfb_mmap(struct fb_info *info,
 
 static struct fb_ops bochsfb_ops = {
 	.owner = THIS_MODULE,
-	DRM_FB_HELPER_DEFAULT_OPS,
+
+	/* can't use DRM_FB_HELPER_DEFAULT_OPS due to lack of fb_blank */
+	.fb_check_var	= drm_fb_helper_check_var,
+	.fb_set_par	= drm_fb_helper_set_par,
+	.fb_setcmap	= drm_fb_helper_setcmap,
+	.fb_blank	= NULL, /* DPMS not working on QEMU */
+	.fb_pan_display	= drm_fb_helper_pan_display,
+	.fb_debug_enter = drm_fb_helper_debug_enter,
+	.fb_debug_leave = drm_fb_helper_debug_leave,
+	.fb_ioctl	= drm_fb_helper_ioctl,
+
 	.fb_fillrect = drm_fb_helper_sys_fillrect,
 	.fb_copyarea = drm_fb_helper_sys_copyarea,
 	.fb_imageblit = drm_fb_helper_sys_imageblit,
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 7fa58eeadc9d..b0e057628157 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -128,7 +128,7 @@ static struct fb_ops cirrusfb_ops = {
 	.fb_copyarea = cirrus_copyarea,
 	.fb_imageblit = cirrus_imageblit,
 	.fb_pan_display = drm_fb_helper_pan_display,
-	.fb_blank = drm_fb_helper_blank,
+	.fb_blank = NULL, /* DPMS not working on QEMU */
 	.fb_setcmap = drm_fb_helper_setcmap,
 };
 
diff --git a/drivers/gpu/drm/virtio/virtgpu_fb.c b/drivers/gpu/drm/virtio/virtgpu_fb.c
index 33df067b11c1..ee3a33ce257f 100644
--- a/drivers/gpu/drm/virtio/virtgpu_fb.c
+++ b/drivers/gpu/drm/virtio/virtgpu_fb.c
@@ -200,7 +200,17 @@ static void virtio_gpu_3d_imageblit(struct fb_info *info,
 
 static struct fb_ops virtio_gpufb_ops = {
 	.owner = THIS_MODULE,
-	DRM_FB_HELPER_DEFAULT_OPS,
+
+	/* can't use DRM_FB_HELPER_DEFAULT_OPS due lack of fb_blank */
+	.fb_check_var	= drm_fb_helper_check_var,
+	.fb_set_par	= drm_fb_helper_set_par,
+	.fb_setcmap	= drm_fb_helper_setcmap,
+	.fb_blank	= NULL, /* DPMS not working on QEMU */
+	.fb_pan_display	= drm_fb_helper_pan_display,
+	.fb_debug_enter = drm_fb_helper_debug_enter,
+	.fb_debug_leave = drm_fb_helper_debug_leave,
+	.fb_ioctl	= drm_fb_helper_ioctl,
+
 	.fb_fillrect = virtio_gpu_3d_fillrect,
 	.fb_copyarea = virtio_gpu_3d_copyarea,
 	.fb_imageblit = virtio_gpu_3d_imageblit,
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel