[PATCH] drm/bochs: Add screen blanking support

[PATCH] drm/bochs: Add screen blanking support

[Takashi Iwai]

On bochs DRM driver, the execution of "setterm --blank force" results
in a frozen screen instead of a blank screen.  It's due to the lack of
the screen blanking support in its code.

Actually, the QEMU bochs vga side can switch to the blanking mode when
the bit 0x20 is cleared on VGA_ATT_IW register (0x3c0), which updates
ar_index in QEMU side.  So, essentially, we'd just need to clear the
bit at pipe disable callback; that's what this patch does essentially.

However, a tricky part is that the QEMU vga code does treat VGA_ATT_IW
register always as "flip-flop"; the first write is for index and the
second write is for the data like palette.  Meanwhile, in the current
bochs DRM driver, the flip-flop wasn't considered, and it calls only
the register update once with the value 0x20.

So, in this patch, we fix the behavior by simply writing the
VGA_ATT_IW register value twice at each place as well.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/gpu/drm/bochs/bochs_hw.c  | 13 ++++++++++++-
 drivers/gpu/drm/bochs/bochs_kms.c |  8 ++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bochs/bochs_hw.c b/drivers/gpu/drm/bochs/bochs_hw.c
index 2d7380a9890e..9a6f90216d6c 100644
--- a/drivers/gpu/drm/bochs/bochs_hw.c
+++ b/drivers/gpu/drm/bochs/bochs_hw.c
@@ -213,6 +213,14 @@ void bochs_hw_setmode(struct bochs_device *bochs,
 	if (!drm_dev_enter(bochs->dev, &idx))
 		return;
 
+	if (!mode) {
+		DRM_DEBUG_DRIVER("crtc disabled\n");
+		/* set to blank mode; send twice for ar_flip_flop */
+		bochs_vga_writeb(bochs, 0x3c0, 0);
+		bochs_vga_writeb(bochs, 0x3c0, 0);
+		goto exit;
+	}
+
 	bochs->xres = mode->hdisplay;
 	bochs->yres = mode->vdisplay;
 	bochs->bpp = 32;
@@ -223,7 +231,9 @@ void bochs_hw_setmode(struct bochs_device *bochs,
 			 bochs->xres, bochs->yres, bochs->bpp,
 			 bochs->yres_virtual);
 
-	bochs_vga_writeb(bochs, 0x3c0, 0x20); /* unblank */
+	/* unblank; send twice for ar_flip_flop */
+	bochs_vga_writeb(bochs, 0x3c0, 0x20);
+	bochs_vga_writeb(bochs, 0x3c0, 0x20);
 
 	bochs_dispi_write(bochs, VBE_DISPI_INDEX_ENABLE,      0);
 	bochs_dispi_write(bochs, VBE_DISPI_INDEX_BPP,         bochs->bpp);
@@ -239,6 +249,7 @@ void bochs_hw_setmode(struct bochs_device *bochs,
 	bochs_dispi_write(bochs, VBE_DISPI_INDEX_ENABLE,
 			  VBE_DISPI_ENABLED | VBE_DISPI_LFB_ENABLED);
 
+ exit:
 	drm_dev_exit(idx);
 }
 
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
index 853081d186d5..b0d77d6d3ae4 100644
--- a/drivers/gpu/drm/bochs/bochs_kms.c
+++ b/drivers/gpu/drm/bochs/bochs_kms.c
@@ -57,6 +57,13 @@ static void bochs_pipe_enable(struct drm_simple_display_pipe *pipe,
 	bochs_plane_update(bochs, plane_state);
 }
 
+static void bochs_pipe_disable(struct drm_simple_display_pipe *pipe)
+{
+	struct bochs_device *bochs = pipe->crtc.dev->dev_private;
+
+	bochs_hw_setmode(bochs, NULL);
+}
+
 static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
 			      struct drm_plane_state *old_state)
 {
@@ -67,6 +74,7 @@ static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
 
 static const struct drm_simple_display_pipe_funcs bochs_pipe_funcs = {
 	.enable	    = bochs_pipe_enable,
+	.disable    = bochs_pipe_disable,
 	.update	    = bochs_pipe_update,
 	.prepare_fb = drm_gem_vram_simple_display_pipe_prepare_fb,
 	.cleanup_fb = drm_gem_vram_simple_display_pipe_cleanup_fb,
-- 
2.26.2

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

Re: [PATCH] drm/bochs: Add screen blanking support

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 4204 bytes --]

Hi

Am 20.04.21 um 18:56 schrieb Takashi Iwai:
> On bochs DRM driver, the execution of "setterm --blank force" results
> in a frozen screen instead of a blank screen.  It's due to the lack of
> the screen blanking support in its code.
> 
> Actually, the QEMU bochs vga side can switch to the blanking mode when
> the bit 0x20 is cleared on VGA_ATT_IW register (0x3c0), which updates
> ar_index in QEMU side.  So, essentially, we'd just need to clear the
> bit at pipe disable callback; that's what this patch does essentially.
> 
> However, a tricky part is that the QEMU vga code does treat VGA_ATT_IW
> register always as "flip-flop"; the first write is for index and the
> second write is for the data like palette.  Meanwhile, in the current
> bochs DRM driver, the flip-flop wasn't considered, and it calls only
> the register update once with the value 0x20.
> 

Unless bochs does things very different, the index should first be reset 
by reading 0x3da. Then write the index, then the data.

https://web.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/vgareg.htm#attribute

Best regards
Thomas

> So, in this patch, we fix the behavior by simply writing the
> VGA_ATT_IW register value twice at each place as well.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   drivers/gpu/drm/bochs/bochs_hw.c  | 13 ++++++++++++-
>   drivers/gpu/drm/bochs/bochs_kms.c |  8 ++++++++
>   2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bochs/bochs_hw.c b/drivers/gpu/drm/bochs/bochs_hw.c
> index 2d7380a9890e..9a6f90216d6c 100644
> --- a/drivers/gpu/drm/bochs/bochs_hw.c
> +++ b/drivers/gpu/drm/bochs/bochs_hw.c
> @@ -213,6 +213,14 @@ void bochs_hw_setmode(struct bochs_device *bochs,
>   	if (!drm_dev_enter(bochs->dev, &idx))
>   		return;
>   
> +	if (!mode) {
> +		DRM_DEBUG_DRIVER("crtc disabled\n");
> +		/* set to blank mode; send twice for ar_flip_flop */
> +		bochs_vga_writeb(bochs, 0x3c0, 0);
> +		bochs_vga_writeb(bochs, 0x3c0, 0);
> +		goto exit;
> +	}
> +
>   	bochs->xres = mode->hdisplay;
>   	bochs->yres = mode->vdisplay;
>   	bochs->bpp = 32;
> @@ -223,7 +231,9 @@ void bochs_hw_setmode(struct bochs_device *bochs,
>   			 bochs->xres, bochs->yres, bochs->bpp,
>   			 bochs->yres_virtual);
>   
> -	bochs_vga_writeb(bochs, 0x3c0, 0x20); /* unblank */
> +	/* unblank; send twice for ar_flip_flop */
> +	bochs_vga_writeb(bochs, 0x3c0, 0x20);
> +	bochs_vga_writeb(bochs, 0x3c0, 0x20);
>   
>   	bochs_dispi_write(bochs, VBE_DISPI_INDEX_ENABLE,      0);
>   	bochs_dispi_write(bochs, VBE_DISPI_INDEX_BPP,         bochs->bpp);
> @@ -239,6 +249,7 @@ void bochs_hw_setmode(struct bochs_device *bochs,
>   	bochs_dispi_write(bochs, VBE_DISPI_INDEX_ENABLE,
>   			  VBE_DISPI_ENABLED | VBE_DISPI_LFB_ENABLED);
>   
> + exit:
>   	drm_dev_exit(idx);
>   }
>   
> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
> index 853081d186d5..b0d77d6d3ae4 100644
> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> @@ -57,6 +57,13 @@ static void bochs_pipe_enable(struct drm_simple_display_pipe *pipe,
>   	bochs_plane_update(bochs, plane_state);
>   }
>   
> +static void bochs_pipe_disable(struct drm_simple_display_pipe *pipe)
> +{
> +	struct bochs_device *bochs = pipe->crtc.dev->dev_private;
> +
> +	bochs_hw_setmode(bochs, NULL);
> +}
> +
>   static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
>   			      struct drm_plane_state *old_state)
>   {
> @@ -67,6 +74,7 @@ static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
>   
>   static const struct drm_simple_display_pipe_funcs bochs_pipe_funcs = 
{
>   	.enable	    = bochs_pipe_enable,
> +	.disable    = bochs_pipe_disable,
>   	.update	    = bochs_pipe_update,
>   	.prepare_fb = drm_gem_vram_simple_display_pipe_prepare_fb,
>   	.cleanup_fb = drm_gem_vram_simple_display_pipe_cleanup_fb,
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] drm/bochs: Add screen blanking support

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 4204 bytes --]

Hi

Am 20.04.21 um 18:56 schrieb Takashi Iwai:
> On bochs DRM driver, the execution of "setterm --blank force" results
> in a frozen screen instead of a blank screen.  It's due to the lack of
> the screen blanking support in its code.
> 
> Actually, the QEMU bochs vga side can switch to the blanking mode when
> the bit 0x20 is cleared on VGA_ATT_IW register (0x3c0), which updates
> ar_index in QEMU side.  So, essentially, we'd just need to clear the
> bit at pipe disable callback; that's what this patch does essentially.
> 
> However, a tricky part is that the QEMU vga code does treat VGA_ATT_IW
> register always as "flip-flop"; the first write is for index and the
> second write is for the data like palette.  Meanwhile, in the current
> bochs DRM driver, the flip-flop wasn't considered, and it calls only
> the register update once with the value 0x20.
> 

Unless bochs does things very different, the index should first be reset 
by reading 0x3da. Then write the index, then the data.

https://web.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/vgareg.htm#attribute

Best regards
Thomas

> So, in this patch, we fix the behavior by simply writing the
> VGA_ATT_IW register value twice at each place as well.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   drivers/gpu/drm/bochs/bochs_hw.c  | 13 ++++++++++++-
>   drivers/gpu/drm/bochs/bochs_kms.c |  8 ++++++++
>   2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bochs/bochs_hw.c b/drivers/gpu/drm/bochs/bochs_hw.c
> index 2d7380a9890e..9a6f90216d6c 100644
> --- a/drivers/gpu/drm/bochs/bochs_hw.c
> +++ b/drivers/gpu/drm/bochs/bochs_hw.c
> @@ -213,6 +213,14 @@ void bochs_hw_setmode(struct bochs_device *bochs,
>   	if (!drm_dev_enter(bochs->dev, &idx))
>   		return;
>   
> +	if (!mode) {
> +		DRM_DEBUG_DRIVER("crtc disabled\n");
> +		/* set to blank mode; send twice for ar_flip_flop */
> +		bochs_vga_writeb(bochs, 0x3c0, 0);
> +		bochs_vga_writeb(bochs, 0x3c0, 0);
> +		goto exit;
> +	}
> +
>   	bochs->xres = mode->hdisplay;
>   	bochs->yres = mode->vdisplay;
>   	bochs->bpp = 32;
> @@ -223,7 +231,9 @@ void bochs_hw_setmode(struct bochs_device *bochs,
>   			 bochs->xres, bochs->yres, bochs->bpp,
>   			 bochs->yres_virtual);
>   
> -	bochs_vga_writeb(bochs, 0x3c0, 0x20); /* unblank */
> +	/* unblank; send twice for ar_flip_flop */
> +	bochs_vga_writeb(bochs, 0x3c0, 0x20);
> +	bochs_vga_writeb(bochs, 0x3c0, 0x20);
>   
>   	bochs_dispi_write(bochs, VBE_DISPI_INDEX_ENABLE,      0);
>   	bochs_dispi_write(bochs, VBE_DISPI_INDEX_BPP,         bochs->bpp);
> @@ -239,6 +249,7 @@ void bochs_hw_setmode(struct bochs_device *bochs,
>   	bochs_dispi_write(bochs, VBE_DISPI_INDEX_ENABLE,
>   			  VBE_DISPI_ENABLED | VBE_DISPI_LFB_ENABLED);
>   
> + exit:
>   	drm_dev_exit(idx);
>   }
>   
> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
> index 853081d186d5..b0d77d6d3ae4 100644
> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> @@ -57,6 +57,13 @@ static void bochs_pipe_enable(struct drm_simple_display_pipe *pipe,
>   	bochs_plane_update(bochs, plane_state);
>   }
>   
> +static void bochs_pipe_disable(struct drm_simple_display_pipe *pipe)
> +{
> +	struct bochs_device *bochs = pipe->crtc.dev->dev_private;
> +
> +	bochs_hw_setmode(bochs, NULL);
> +}
> +
>   static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
>   			      struct drm_plane_state *old_state)
>   {
> @@ -67,6 +74,7 @@ static void bochs_pipe_update(struct drm_simple_display_pipe *pipe,
>   
>   static const struct drm_simple_display_pipe_funcs bochs_pipe_funcs = 
{
>   	.enable	    = bochs_pipe_enable,
> +	.disable    = bochs_pipe_disable,
>   	.update	    = bochs_pipe_update,
>   	.prepare_fb = drm_gem_vram_simple_display_pipe_prepare_fb,
>   	.cleanup_fb = drm_gem_vram_simple_display_pipe_cleanup_fb,
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

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

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

Re: [PATCH] drm/bochs: Add screen blanking support

[Gerd Hoffmann]

> > However, a tricky part is that the QEMU vga code does treat VGA_ATT_IW
> > register always as "flip-flop"; the first write is for index and the
> > second write is for the data like palette.  Meanwhile, in the current
> > bochs DRM driver, the flip-flop wasn't considered, and it calls only
> > the register update once with the value 0x20.
> > 
> 
> Unless bochs does things very different, the index should first be reset by
> reading 0x3da. Then write the index, then the data.
> 
> https://web.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/vgareg.htm#attribute

bochs should follow standard vga logic here.
Also a bochs_set_blank(true/false) helper function probably makes sense.

take care,
  Gerd

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] drm/bochs: Add screen blanking support

[Gerd Hoffmann]

> > However, a tricky part is that the QEMU vga code does treat VGA_ATT_IW
> > register always as "flip-flop"; the first write is for index and the
> > second write is for the data like palette.  Meanwhile, in the current
> > bochs DRM driver, the flip-flop wasn't considered, and it calls only
> > the register update once with the value 0x20.
> > 
> 
> Unless bochs does things very different, the index should first be reset by
> reading 0x3da. Then write the index, then the data.
> 
> https://web.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/vgareg.htm#attribute

bochs should follow standard vga logic here.
Also a bochs_set_blank(true/false) helper function probably makes sense.

take care,
  Gerd

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

Re: [PATCH] drm/bochs: Add screen blanking support

[Takashi Iwai]

On Tue, 20 Apr 2021 19:47:31 +0200,
Thomas Zimmermann wrote:
> 
> Hi
> 
> Am 20.04.21 um 18:56 schrieb Takashi Iwai:
> > On bochs DRM driver, the execution of "setterm --blank force" results
> > in a frozen screen instead of a blank screen.  It's due to the lack of
> > the screen blanking support in its code.
> >
> > Actually, the QEMU bochs vga side can switch to the blanking mode when
> > the bit 0x20 is cleared on VGA_ATT_IW register (0x3c0), which updates
> > ar_index in QEMU side.  So, essentially, we'd just need to clear the
> > bit at pipe disable callback; that's what this patch does essentially.
> >
> > However, a tricky part is that the QEMU vga code does treat VGA_ATT_IW
> > register always as "flip-flop"; the first write is for index and the
> > second write is for the data like palette.  Meanwhile, in the current
> > bochs DRM driver, the flip-flop wasn't considered, and it calls only
> > the register update once with the value 0x20.
> >
> 
> Unless bochs does things very different, the index should first be
> reset by reading 0x3da. Then write the index, then the data.
> 
> https://web.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/vgareg.htm#attribute

Thanks for the pointer!

It seems that QEMU stdvga actually implements the discard of flip flop
bit by reading 0x3da.  Meanwhile, the write of the data isn't needed
in this case because we do care only about the enablement bit 0x20 of
ar_index.

I'll resubmit v2 patch.


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

Re: [PATCH] drm/bochs: Add screen blanking support

[Takashi Iwai]

On Wed, 21 Apr 2021 09:19:42 +0200,
Gerd Hoffmann wrote:
> 
> > > However, a tricky part is that the QEMU vga code does treat VGA_ATT_IW
> > > register always as "flip-flop"; the first write is for index and the
> > > second write is for the data like palette.  Meanwhile, in the current
> > > bochs DRM driver, the flip-flop wasn't considered, and it calls only
> > > the register update once with the value 0x20.
> > > 
> > 
> > Unless bochs does things very different, the index should first be reset by
> > reading 0x3da. Then write the index, then the data.
> > 
> > https://web.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/vgareg.htm#attribute
> 
> bochs should follow standard vga logic here.
> Also a bochs_set_blank(true/false) helper function probably makes sense.

OK, I factored it out to bochs_hw_blank() in v2 patch.


thanks,

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