VT console blank ignored by DRM drivers on QEMU

Commit Message

Takashi Iwai July 10, 2017, 2:41 p.m. UTC

On Mon, 10 Jul 2017 13:47:57 +0200,
Gerd Hoffmann wrote:
> 
>   Hi,
> 
> > 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.
> 
> Well, the virtual hardware simply has no dpms support, except maybe for
> cirrus which mimics physical hardware.
> 
> bochs could toggle the blank bit in vga register space.
> 
> virtio and qxl could unmap the plane, but that might have unwanted
> effects on the host side because qemu thinks the guest turned off the
> display altogether.
> 
> > 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.
> 
> Sounds good to me.
> 
> I've seen this on real hardware too btw (arm board with non-working
> dpms).

So something like below?
(Adding Bartlomiej to Cc, as it's fbcon stuff)


Takashi

-- 8< --
From: Takashi Iwai <[e-mail]>
Subject: [PATCH] fbcon: Perform generic blank unconditionally

Currently fbcon performs the manual clearance of console as a fallback
only when fb_blank() returns an error.  Unfortunately, all DRM fbcons
running on QEMU don't return the error but only adjust the non-working
DPMS, we end up just having the frozen screen upon blank call.
Also Gerd suggested that a similar issue could have seen on the bare
metal, too.

As a simple workaround suggested by Daniel, let's call
fbcon_generic_blank() unconditionally at fbcon_blank() so that it
always clears the console.

Suggested-by: Daniel Vetter <[e-mail]>
Cc: Gerd Hoffmann <[e-mail]>
Cc: Alexander Graf <[e-mail]>
Signed-off-by: Takashi Iwai <[e-mail]>
---
 drivers/video/console/fbcon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Daniel Vetter July 10, 2017, 2:57 p.m. UTC | #1

On Mon, Jul 10, 2017 at 4:41 PM, Takashi Iwai <[e-mail]> wrote:
> diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
> index 12ded23f1aaf..65169a5a1bca 100644
> --- a/drivers/video/console/fbcon.c
> +++ b/drivers/video/console/fbcon.c
> @@ -2347,8 +2347,8 @@ static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
>                         ops->cursor_flash = (!blank);
>
>                         if (!(info->flags & FBINFO_MISC_USEREVENT))
> -                               if (fb_blank(info, blank))
> -                                       fbcon_generic_blank(vc, info, blank);
> +                               fb_blank(info, blank);
> +                               fbcon_generic_blank(vc, info, blank);

My idea was more to do this in the drm_fb_helper.c layer if the driver
can't do this, since your DE might also expect that the screen goes
blank for real.

Or we fix the drivers to do a black screen by essentially switching it
off (but that might have implications again on the UI for virtual
machines, so dunno ...).
-Daniel

kernel test robot July 10, 2017, 8:08 p.m. UTC | #2

Hi Takashi,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.12 next-20170710]
[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/Takashi-Iwai/fbcon-Perform-generic-blank-unconditionally/20170711-033549
config: x86_64-randconfig-x008-201728 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/video/console/fbcon.c: In function 'fbcon_blank':
>> drivers/video/console/fbcon.c:2349:4: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
       if (!(info->flags & FBINFO_MISC_USEREVENT))
       ^~
   drivers/video/console/fbcon.c:2351:5: note: ...this statement, but the latter is misleadingly indented as if it is guarded by the 'if'
        fbcon_generic_blank(vc, info, blank);
        ^~~~~~~~~~~~~~~~~~~

vim +/if +2349 drivers/video/console/fbcon.c


:::::: The code at line 2349 was first introduced by commit
:::::: bca404afdc5206c3bb30168315ee8a98a579ec65 fbdev: fix FB console blanking

:::::: TO: Dmitry Baryshkov <[e-mail]>
:::::: CC: Linus Torvalds <[e-mail]>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch

diff --git a/drivers/video/console/fbcon.c b/drivers/video/console/fbcon.c
index 12ded23f1aaf..65169a5a1bca 100644
--- a/drivers/video/console/fbcon.c
+++ b/drivers/video/console/fbcon.c
@@ -2347,8 +2347,8 @@  static int fbcon_blank(struct vc_data *vc, int blank, int mode_switch)
 			ops->cursor_flash = (!blank);
 
 			if (!(info->flags & FBINFO_MISC_USEREVENT))
-				if (fb_blank(info, blank))
-					fbcon_generic_blank(vc, info, blank);
+				fb_blank(info, blank);
+				fbcon_generic_blank(vc, info, blank);
 		}
 
 		if (!blank)