On Mon, 10 Jul 2017 11:27:01 +0200,
Daniel Vetter wrote:
>
> On Mon, Jul 10, 2017 at 10:53 AM, Takashi Iwai <tiwai@xxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
