From patchwork Mon Jul 10 09:37:36 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Takashi Iwai X-Patchwork-Id: 9832631 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id E46E460318 for ; Mon, 10 Jul 2017 09:37:40 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D6097283DA for ; Mon, 10 Jul 2017 09:37:40 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C9E202847A; Mon, 10 Jul 2017 09:37:40 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 3E673283DA for ; Mon, 10 Jul 2017 09:37:40 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id AE9F56E12A; Mon, 10 Jul 2017 09:37:39 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mx1.suse.de (mx2.suse.de [195.135.220.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id E55176E12A for ; Mon, 10 Jul 2017 09:37:37 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 8724AABB1; Mon, 10 Jul 2017 09:37:36 +0000 (UTC) Date: Mon, 10 Jul 2017 11:37:36 +0200 Message-ID: From: Takashi Iwai To: Daniel Vetter Subject: Re: VT console blank ignored by DRM drivers on QEMU In-Reply-To: References: User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.2 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Cc: Gerd Hoffmann , dri-devel , Alexander Graf X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP On Mon, 10 Jul 2017 11:27:01 +0200, Daniel Vetter wrote: > > On Mon, Jul 10, 2017 at 10:53 AM, Takashi Iwai 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,