[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] vga: Implement blinking of text cursor
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] [PATCH v2] vga: Implement blinking of text cursor |
Date: |
Wed, 04 Jul 2012 19:45:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
On 2012-07-04 18:26, Stefan Weil wrote:
> Hi Jan,
>
> please see my comments below.
>
>
> Am 04.07.2012 15:40, schrieb Jan Kiszka:
>> Let the text cursor blink at 5 Hz. No timer is used, instead we rely on
>> the fact that the display is updated periodically.
>>
>> Signed-off-by: Jan Kiszka<address@hidden>
>> ---
>>
>> Changes in v2:
>> - avoid full screen updates for cursor blinking
>> - convert cursor_blink_state to cursor_visible_phase
>>
>> hw/vga.c | 14 ++++++++++++--
>> hw/vga_int.h | 2 ++
>> 2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/vga.c b/hw/vga.c
>> index acb3f7d..a1644f3 100644
>> --- a/hw/vga.c
>> +++ b/hw/vga.c
>> @@ -38,6 +38,8 @@
>>
>> //#define DEBUG_BOCHS_VBE
>>
>> +#define VGA_TEXT_CURSOR_PERIOD 200
>> +
>> /*
>> * Video Graphics Array (VGA)
>> *
>> @@ -1300,6 +1302,7 @@ static void vga_draw_text(VGACommonState *s, int
>> full_update)
>> uint32_t *ch_attr_ptr;
>> vga_draw_glyph8_func *vga_draw_glyph8;
>> vga_draw_glyph9_func *vga_draw_glyph9;
>> + int64_t now = qemu_get_clock_ms(vm_clock);
>>
>> /* compute font data address (in plane 2) */
>> v = s->sr[VGA_SEQ_CHARACTER_MAP];
>> @@ -1370,6 +1373,10 @@ static void vga_draw_text(VGACommonState *s, int
>> full_update)
>> s->cursor_end = s->cr[VGA_CRTC_CURSOR_END];
>> }
>> cursor_ptr = s->vram_ptr + (s->start_addr + cursor_offset) * 4;
>> + if (now>= s->cursor_blink_time) {
>> + s->cursor_blink_time = now + VGA_TEXT_CURSOR_PERIOD;
>> + s->cursor_visible_phase = !s->cursor_visible_phase;
>> + }
>>
>
> It's named VGA_TEXT_CURSOR_PERIOD, but it looks like it is not the
> period, but the time for each of two states (on/off), so it's 1/2 the
> period.
Err, of course.
>
> With your patch, I'd expect a blink frequency of 2.5 Hz.
>
> According to the VGA documentation (I used
> http://www.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/textcur.htm#blink),
> the time for each state should be 16 / (60 Hz) or 266 ms resulting in
> a blink frequency of 1.875 Hz.
Interesting. The real hardware I was looking at is using a rate
definitely larger than 2 HZ. But I can change this to the original rate.
>
>
>>
>> depth_index = get_depth_index(s->ds);
>> if (cw == 16)
>> @@ -1390,7 +1397,7 @@ static void vga_draw_text(VGACommonState *s, int
>> full_update)
>> cx_max = -1;
>> for(cx = 0; cx< width; cx++) {
>> ch_attr = *(uint16_t *)src;
>> - if (full_update || ch_attr != *ch_attr_ptr) {
>> + if (full_update || ch_attr != *ch_attr_ptr || src ==
>> cursor_ptr) {
>> if (cx< cx_min)
>> cx_min = cx;
>> if (cx> cx_max)
>> @@ -1420,7 +1427,8 @@ static void vga_draw_text(VGACommonState *s, int
>> full_update)
>> font_ptr, cheight, fgcol, bgcol, dup9);
>> }
>> if (src == cursor_ptr&&
>> - !(s->cr[VGA_CRTC_CURSOR_START]& 0x20)) {
>> + !(s->cr[VGA_CRTC_CURSOR_START]& 0x20)&&
>> + s->cursor_visible_phase) {
>> int line_start, line_last, h;
>> /* draw the cursor */
>> line_start = s->cr[VGA_CRTC_CURSOR_START]& 0x1f;
>> @@ -1884,6 +1892,8 @@ static void vga_update_display(void *opaque)
>> }
>> if (graphic_mode != s->graphic_mode) {
>> s->graphic_mode = graphic_mode;
>> + s->cursor_blink_time =
>> + qemu_get_clock_ms(vm_clock) + VGA_TEXT_CURSOR_PERIOD;
>> full_update = 1;
>> }
>> switch(graphic_mode) {
>> diff --git a/hw/vga_int.h b/hw/vga_int.h
>> index 3b38764..8938093 100644
>> --- a/hw/vga_int.h
>> +++ b/hw/vga_int.h
>> @@ -156,6 +156,8 @@ typedef struct VGACommonState {
>> uint32_t last_scr_width, last_scr_height; /* in pixels */
>> uint32_t last_depth; /* in bits */
>> uint8_t cursor_start, cursor_end;
>> + bool cursor_visible_phase;
>> + int64_t cursor_blink_time;
>> uint32_t cursor_offset;
>> unsigned int (*rgb_to_pixel)(unsigned int r,
>> unsigned int g, unsigned b);
>>
>
> As far as I can tell, there are more cursor features missing:
> do we support an invisible cursor (your patch doesn't) or
> cursor shape? See also this URL for more information:
> http://www.stanford.edu/class/cs140/projects/pintos/specs/freevga/vga/textcur.htm
>
> Blinking is also missing for text with the blink attribute.
> I don't know whether real hardware synchronizes text and
> cursor blinking. If it does, we should do that, too.
>
> What about vertical blank interrupt? It is the base of text
> cursor blinking. We don't support it currently.
As always: patches are welcome :). This covers at least the most
prominent use case.
Thanks,
Jan
--
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux