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@xxxxxxx> > > --- > > 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)>] > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
