[PATCH v2] drm/bochs: Add screen blanking support
Takashi Iwai
tiwai at suse.de
Tue Apr 27 06:18:22 UTC 2021
On Mon, 26 Apr 2021 20:44:55 +0200,
Thomas Zimmermann wrote:
>
> Hi
>
> Am 21.04.21 um 10:08 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 access via VGA_ATT_IW is done in
> > "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.
>
> I read up on the details of what the attribute registers do and what
> you're modifying is the PAS field in the attribute index register. It
> controls write access to the attribute fields.
>
>
> https://web.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/attrreg.htm#3C0
>
> It's located in the index register and cleared while
> attributes/palettes are updated. I guess that in this mode the stdvga
> disables the palette entirely (hence the screen turns dark).
>
> While it works, it feels wrong to do this.
>
> I to do blanking/unblanking with the SR field in SEQ0
>
>
> https://web.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/seqreg.htm#00
>
> That's what drivers usually do AFAICT. I think the 'unblank' comment
> next to the existing code might be misleading.
Yeah, when you look at the existing vga16fb.c in kernel, we can find a
relatively complex blanking procedure. OTOH, what I changed is rather
based on the the actual behavior of bochs is more or less simplified.
QEMU hw/display/vga.c contains a code like:
static void vga_update_display(void *opaque)
{
VGACommonState *s = opaque;
DisplaySurface *surface = qemu_console_surface(s->con);
int full_update, graphic_mode;
qemu_flush_coalesced_mmio_buffer();
if (surface_bits_per_pixel(surface) == 0) {
/* nothing to do */
} else {
full_update = 0;
if (!(s->ar_index & 0x20)) {
graphic_mode = GMODE_BLANK;
} else {
graphic_mode = s->gr[VGA_GFX_MISC] & VGA_GR06_GRAPHICS_MODE;
}
if (graphic_mode != s->graphic_mode) {
s->graphic_mode = graphic_mode;
s->cursor_blink_time = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
full_update = 1;
}
switch(graphic_mode) {
case GMODE_TEXT:
vga_draw_text(s, full_update);
break;
case GMODE_GRAPH:
vga_draw_graphic(s, full_update);
break;
case GMODE_BLANK:
default:
vga_draw_blank(s, full_update);
break;
}
}
}
So, it simply checks the ar_index () at each update and switches
from/to the blank mode depending on the bit 0x20.
I'm fine to change in any better way, of course, so feel free to
modify the patch.
thanks,
Takashi
>
> Best regards
> Thomas
>
> >
> > The spec and the actual VGA implementation in QEMU suggests that the
> > flip flop flag is discarded by reading the CRTC index register
> > (VGA_IS1_RC, 0x3da). So, in this patch, we add the helper to read a
> > byte and the call to clear the flip flop flag before changing the
> > blank / unblank setup via VGA_ATT_IW register.
> >
> > v1->v2:
> > * discard ar_flip_flop by reading 0x3da, add bochs_vga_readb()
> > * include video/vga.h for VGA register definitions
> > * move the blank/unblank code to bochs_hw_blank()
> >
> > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > ---
> > drivers/gpu/drm/bochs/bochs.h | 1 +
> > drivers/gpu/drm/bochs/bochs_hw.c | 25 ++++++++++++++++++++++++-
> > drivers/gpu/drm/bochs/bochs_kms.c | 8 ++++++++
> > 3 files changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/bochs/bochs.h b/drivers/gpu/drm/bochs/bochs.h
> > index e5bd1d517a18..e9645c612aff 100644
> > --- a/drivers/gpu/drm/bochs/bochs.h
> > +++ b/drivers/gpu/drm/bochs/bochs.h
> > @@ -78,6 +78,7 @@ struct bochs_device {
> > int bochs_hw_init(struct drm_device *dev);
> > void bochs_hw_fini(struct drm_device *dev);
> > +void bochs_hw_blank(struct bochs_device *bochs, bool blank);
> > void bochs_hw_setmode(struct bochs_device *bochs,
> > struct drm_display_mode *mode);
> > void bochs_hw_setformat(struct bochs_device *bochs,
> > diff --git a/drivers/gpu/drm/bochs/bochs_hw.c b/drivers/gpu/drm/bochs/bochs_hw.c
> > index 2d7380a9890e..7d3426d8cc69 100644
> > --- a/drivers/gpu/drm/bochs/bochs_hw.c
> > +++ b/drivers/gpu/drm/bochs/bochs_hw.c
> > @@ -7,6 +7,7 @@
> > #include <drm/drm_drv.h>
> > #include <drm/drm_fourcc.h>
> > +#include <video/vga.h>
> > #include "bochs.h"
> > /*
> > ----------------------------------------------------------------------
> > */
> > @@ -24,6 +25,19 @@ static void bochs_vga_writeb(struct bochs_device *bochs, u16 ioport, u8 val)
> > }
> > }
> > +static u8 bochs_vga_readb(struct bochs_device *bochs, u16 ioport)
> > +{
> > + if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df))
> > + return 0xff;
> > +
> > + if (bochs->mmio) {
> > + int offset = ioport - 0x3c0 + 0x400;
> > + return readb(bochs->mmio + offset);
> > + } else {
> > + return inb(ioport);
> > + }
> > +}
> > +
> > static u16 bochs_dispi_read(struct bochs_device *bochs, u16 reg)
> > {
> > u16 ret = 0;
> > @@ -205,6 +219,15 @@ void bochs_hw_fini(struct drm_device *dev)
> > kfree(bochs->edid);
> > }
> > +void bochs_hw_blank(struct bochs_device *bochs, bool blank)
> > +{
> > + DRM_DEBUG_DRIVER("hw_blank %d\n", blank);
> > + /* discard ar_flip_flop */
> > + (void)bochs_vga_readb(bochs, VGA_IS1_RC);
> > + /* blank or unblank; we need only update index and set 0x20 */
> > + bochs_vga_writeb(bochs, VGA_ATT_W, blank ? 0 : 0x20);
> > +}
> > +
> > void bochs_hw_setmode(struct bochs_device *bochs,
> > struct drm_display_mode *mode)
> > {
> > @@ -223,7 +246,7 @@ void bochs_hw_setmode(struct bochs_device *bochs,
> > bochs->xres, bochs->yres, bochs->bpp,
> > bochs->yres_virtual);
> > - bochs_vga_writeb(bochs, 0x3c0, 0x20); /* unblank */
> > + bochs_hw_blank(bochs, false);
> > bochs_dispi_write(bochs, VBE_DISPI_INDEX_ENABLE, 0);
> > bochs_dispi_write(bochs, VBE_DISPI_INDEX_BPP, bochs->bpp);
> > diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c
> > index 853081d186d5..99410e77d51a 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_blank(bochs, true);
> > +}
> > +
> > 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
>
> [2 OpenPGP digital signature <application/pgp-signature (7bit)>]
>
More information about the dri-devel
mailing list