Hi
Am 27.04.21 um 08:18 schrieb Takashi Iwai:> 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.
If no one objects, I'll merge it as-is. It's somewhat wrong wrt to VGA,
but apparently what qemu wants.
Best regards
Thomas
>
>
> 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)>]
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
--
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL:
<http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20210427/870e0b44/attachment.sig>