On 07/10/2017 11:37 AM, Takashi Iwai wrote:
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
I think that would be a really good change, yes.
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 */
Is DPMS even specified in the BOCHS VGA adapter? If it's not in the
spec, there's not a lot QEMU can do about it :).
+ .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 */
I'm torn on this one. For Cirrus, it might be better to fix QEMU and
support power saving there. If nothing else at least by switching to a
blank pane.
.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 */
The same spec argument applies here. If the virtio-gpu spec doesn't
specific DPMS, there's not a lot we can do about it in QEMU today.
Alex
+ .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
