VT console blank ignored by DRM drivers on QEMU

VT console blank ignored by DRM drivers on QEMU

[Takashi Iwai]

Hi,

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.


Any suggestions?


thanks,

Takashi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: VT console blank ignored by DRM drivers on QEMU

[Daniel Vetter]

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.

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.

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.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: VT console blank ignored by DRM drivers on QEMU

[Takashi Iwai]

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

Re: VT console blank ignored by DRM drivers on QEMU

[Alexander Graf]

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@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

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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: VT console blank ignored by DRM drivers on QEMU

[Daniel Vetter]

On Mon, Jul 10, 2017 at 11:37 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> 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.

I meant to say that adding an error code to the dpms callback seems
like a good idea, because we need that anyway. You can ignore the
blabla about why exactly atomic drivers need it, and ofc I'm not going
to suggest that you convert all your drivers over to atomic first.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: VT console blank ignored by DRM drivers on QEMU

[Daniel Vetter]

On Mon, Jul 10, 2017 at 4:56 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Jul 10, 2017 at 11:37 AM, Takashi Iwai <tiwai@suse.de> wrote:
>>> 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.
>
> I meant to say that adding an error code to the dpms callback seems
> like a good idea, because we need that anyway. You can ignore the
> blabla about why exactly atomic drivers need it, and ofc I'm not going
> to suggest that you convert all your drivers over to atomic first.

I just realized that we've switched the dpms callback from void to int
return type a while ago. So only thing you'd need to do is wire up the
return code through the fbdev helpers, and fix up the virtual drivers
to not allow dpms.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: VT console blank ignored by DRM drivers on QEMU

[Takashi Iwai]

On Tue, 11 Jul 2017 19:35:36 +0200,
Daniel Vetter wrote:
> 
> On Mon, Jul 10, 2017 at 4:56 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On Mon, Jul 10, 2017 at 11:37 AM, Takashi Iwai <tiwai@suse.de> wrote:
> >>> 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.
> >
> > I meant to say that adding an error code to the dpms callback seems
> > like a good idea, because we need that anyway. You can ignore the
> > blabla about why exactly atomic drivers need it, and ofc I'm not going
> > to suggest that you convert all your drivers over to atomic first.
> 
> I just realized that we've switched the dpms callback from void to int
> return type a while ago. So only thing you'd need to do is wire up the
> return code through the fbdev helpers, and fix up the virtual drivers
> to not allow dpms.

Hmm, as of 4.13-rc1, I see some inconsistencies:

In drm_connector.h:
struct drm_connector_funcs {
	int (*dpms)(struct drm_connector *connector, int mode);

In drm_encoder_slave.h:
struct drm_encoder_slave_funcs {
	void (*dpms)(struct drm_encoder *encoder, int mode);

In drm_modeset_helper_vtables.h:
struct drm_crtc_helper_funcs {
	void (*dpms)(struct drm_crtc *crtc, int mode);

struct drm_encoder_helper_funcs {
	void (*dpms)(struct drm_encoder *encoder, int mode);


Takashi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: VT console blank ignored by DRM drivers on QEMU

[Daniel Vetter]

On Tue, Jul 11, 2017 at 8:10 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Tue, 11 Jul 2017 19:35:36 +0200,
> Daniel Vetter wrote:
>>
>> On Mon, Jul 10, 2017 at 4:56 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> > On Mon, Jul 10, 2017 at 11:37 AM, Takashi Iwai <tiwai@suse.de> wrote:
>> >>> 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.
>> >
>> > I meant to say that adding an error code to the dpms callback seems
>> > like a good idea, because we need that anyway. You can ignore the
>> > blabla about why exactly atomic drivers need it, and ofc I'm not going
>> > to suggest that you convert all your drivers over to atomic first.
>>
>> I just realized that we've switched the dpms callback from void to int
>> return type a while ago. So only thing you'd need to do is wire up the
>> return code through the fbdev helpers, and fix up the virtual drivers
>> to not allow dpms.
>
> Hmm, as of 4.13-rc1, I see some inconsistencies:
>
> In drm_connector.h:
> struct drm_connector_funcs {
>         int (*dpms)(struct drm_connector *connector, int mode);

This is the driver interface.

> In drm_encoder_slave.h:
> struct drm_encoder_slave_funcs {
>         void (*dpms)(struct drm_encoder *encoder, int mode);
>
> In drm_modeset_helper_vtables.h:
> struct drm_crtc_helper_funcs {
>         void (*dpms)(struct drm_crtc *crtc, int mode);
>
> struct drm_encoder_helper_funcs {
>         void (*dpms)(struct drm_encoder *encoder, int mode);

These are just helpers used by the legacy modeset infrastructure and
deprecated in atomic. As long as you overwrite the connector->dmps
function with your own special one you can return an error code.

You still have to carry around the dummy functions doing nothing,
because the legacy helpers suck that way (and I'm definitely not going
to spend timing cleaning them up, just port to atomic instead).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: VT console blank ignored by DRM drivers on QEMU

[Gerd Hoffmann]

  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).

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: VT console blank ignored by DRM drivers on QEMU

[Takashi Iwai]

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 <tiwai@suse.de>
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 <daniel@ffwll.ch>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/video/console/fbcon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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)
-- 
2.13.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: VT console blank ignored by DRM drivers on QEMU

[Daniel Vetter]

On Mon, Jul 10, 2017 at 4:41 PM, Takashi Iwai <tiwai@suse.de> 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
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] fbcon: Perform generic blank unconditionally

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]

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 <dbaryshkov@gmail.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30285 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: VT console blank ignored by DRM drivers on QEMU

[Daniel Vetter]

On Mon, Jul 10, 2017 at 1:47 PM, Gerd Hoffmann <kraxel@redhat.com> 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.

dpms off = turn screen off, except keep some of the resources reserved
to be able to guarantee that you can switch it on again. There is no
difference in behaviour, and I think it'd be the right thing for
virtual screens too. Well there's the mild problem of you can't wiggle
the mouse anymore to wake it up I guess, but from a drm api pov
there's really not supposed to be a difference in behaviour.

>> 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).

Atomic drivers _all_ have working dpms, because atomic simply remaps
that to "everything off". DPMS is purely a legacy thing to keep
existing userspace happy. And it's useful for suspend/resume, to keep
resources reserved and guarantee we can resume.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel