[Qemu-devel] [PATCH] cocoa: Fix VBE function Set Display Start

[Qemu-devel] [PATCH] cocoa: Fix VBE function Set Display Start

[Henry Harrington]

Register a dpy_gfx_setdata callback so that the Cocoa code
is notified whenever the screen start address changes.

Signed-off-by: Henry Harrington <henry.harrington@gmail.com>
---
 ui/cocoa.m |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 87d2e44..97010bc 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -265,6 +265,7 @@ static int cocoa_keycode_to_qemu(int keycode)
     BOOL isTabletEnabled;
 }
 - (void) resizeContentToWidth:(int)w height:(int)h displayState:(DisplayState *)ds;
+- (void) updateDataOffset:(DisplayState *)ds;
 - (void) grabMouse;
 - (void) ungrabMouse;
 - (void) toggleFullScreen:(id)sender;
@@ -429,6 +430,17 @@ QemuCocoaView *cocoaView;
     [self setFrame:NSMakeRect(cx, cy, cw, ch)];
 }
 
+- (void) updateDataOffset:(DisplayState *)ds
+{
+    COCOA_DEBUG("QemuCocoaView: UpdateDataOffset\n");
+
+    // update screenBuffer
+    if (dataProviderRef)
+        CGDataProviderRelease(dataProviderRef);
+
+    dataProviderRef = CGDataProviderCreateWithData(NULL, ds_get_data(ds), ds_get_width(ds) * 4 * ds_get_height(ds), NULL);
+}
+
 - (void) toggleFullScreen:(id)sender
 {
     COCOA_DEBUG("QemuCocoaView: toggleFullScreen\n");
@@ -1004,6 +1016,11 @@ static void cocoa_refresh(DisplayState *ds)
     vga_hw_update();
 }
 
+static void cocoa_setdata(DisplayState *ds)
+{
+  [cocoaView updateDataOffset:ds];
+}
+
 static void cocoa_cleanup(void)
 {
     COCOA_DEBUG("qemu_cocoa: cocoa_cleanup\n");
@@ -1020,6 +1037,7 @@ void cocoa_display_init(DisplayState *ds, int full_screen)
     dcl->dpy_gfx_update = cocoa_update;
     dcl->dpy_gfx_resize = cocoa_resize;
     dcl->dpy_refresh = cocoa_refresh;
+    dcl->dpy_gfx_setdata = cocoa_setdata;
 
 	register_displaychangelistener(ds, dcl);
 
-- 
1.7.6.1

Re: [Qemu-devel] [PATCH] cocoa: Fix VBE function Set Display Start

[Andreas Färber]

Am 21.11.2012 20:08, schrieb Henry Harrington:
> Register a dpy_gfx_setdata callback so that the Cocoa code
> is notified whenever the screen start address changes.
> 
> Signed-off-by: Henry Harrington <henry.harrington@gmail.com>
> ---
>  ui/cocoa.m |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 87d2e44..97010bc 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -265,6 +265,7 @@ static int cocoa_keycode_to_qemu(int keycode)
>      BOOL isTabletEnabled;
>  }
>  - (void) resizeContentToWidth:(int)w height:(int)h displayState:(DisplayState *)ds;
> +- (void) updateDataOffset:(DisplayState *)ds;
>  - (void) grabMouse;
>  - (void) ungrabMouse;
>  - (void) toggleFullScreen:(id)sender;
> @@ -429,6 +430,17 @@ QemuCocoaView *cocoaView;
>      [self setFrame:NSMakeRect(cx, cy, cw, ch)];
>  }
>  
> +- (void) updateDataOffset:(DisplayState *)ds
> +{
> +    COCOA_DEBUG("QemuCocoaView: UpdateDataOffset\n");
> +
> +    // update screenBuffer
> +    if (dataProviderRef)
> +        CGDataProviderRelease(dataProviderRef);

Please add braces for if.

> +
> +    dataProviderRef = CGDataProviderCreateWithData(NULL, ds_get_data(ds), ds_get_width(ds) * 4 * ds_get_height(ds), NULL);

This line seems overly long, please keep within 80 chars.

> +}
> +
>  - (void) toggleFullScreen:(id)sender
>  {
>      COCOA_DEBUG("QemuCocoaView: toggleFullScreen\n");
> @@ -1004,6 +1016,11 @@ static void cocoa_refresh(DisplayState *ds)
>      vga_hw_update();
>  }
>  
> +static void cocoa_setdata(DisplayState *ds)
> +{
> +  [cocoaView updateDataOffset:ds];

Please indent using 4 spaces.

> +}
> +
>  static void cocoa_cleanup(void)
>  {
>      COCOA_DEBUG("qemu_cocoa: cocoa_cleanup\n");
> @@ -1020,6 +1037,7 @@ void cocoa_display_init(DisplayState *ds, int full_screen)
>      dcl->dpy_gfx_update = cocoa_update;
>      dcl->dpy_gfx_resize = cocoa_resize;
>      dcl->dpy_refresh = cocoa_refresh;
> +    dcl->dpy_gfx_setdata = cocoa_setdata;
>  
>  	register_displaychangelistener(ds, dcl);
>  

Apart from the style issues above (you can use scripts/checkpatch.pl to
verify) this looks okay. Unfortunately I am not getting pixman built on
OSX so am still unable to test...

Am I understanding correctly that this is an optional hook and not a bug
fix for 1.3? How can it be verified?

Regards,
Andreas

Re: [Qemu-devel] [PATCH] cocoa: Fix VBE function Set Display Start

[Henry Harrington]

[-- Attachment #1: Type: text/plain, Size: 905 bytes --]

On Tue, Nov 27, 2012 at 5:03 AM, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 21.11.2012 20:08, schrieb Henry Harrington:
> Apart from the style issues above (you can use scripts/checkpatch.pl to
> verify) this looks okay. Unfortunately I am not getting pixman built on
> OSX so am still unable to test...

Thanks, I'll fix the style issues and submit a new patch.

> Am I understanding correctly that this is an optional hook and not a bug
> fix for 1.3? How can it be verified?
>
> Regards,
> Andreas

The setdata callback is required, even though the Cocoa UI works reasonably
well without it. I've attached a simple testcase, which rapidly switches
between two virtual screens using VBE (one is blue/green, and the other
is green/red). Without the fix Qemu does not switch between screens.

There was a similar bug in the VNC UI which was fixed in commit 1d3323d.

Henry

[-- Attachment #2: display-test.asm --]
[-- Type: application/octet-stream, Size: 4519 bytes --]

[ORG 0x7c00]
[BITS 16]
        jmp 0:entry
entry:
        mov ax, 0
        mov ds, ax
        mov es, ax
        ;; stack goes just below the boot sector.
        mov ss, ax
        mov sp, 0x7c00

        ;; Get info for mode 111 (640x480x2)
        mov ax, 0x4f01
        mov cx, 0x0111
        mov di, mode_info
        int 0x10
        ;; Switch to mode 111, with linear framebuffer.
        mov ax, 0x4f02
        mov bx, 0x4111
        int 0x10

        ;; Enter unreal mode so memory above 1MB can be accessed.
        cli
        push es
        push ds
        lgdt [gdtinfo]
        mov eax, cr0
        or al, 1
        mov cr0, eax
        mov bx, 0x8
        mov ds, bx
        mov es, bx
        and al, 0xfe
        mov cr0, eax
        pop ds
        pop es
        sti

        ;; Fill a 640x960 area with equal parts blue, green and red.
        ;; This is twice as large as the visible screen, so only the
        ;; blue area and part of the green area will be displayed initially.
        cld
        mov edi, [PhysBasePtr]
        mov eax, 0x1f           ; blue
        mov ecx, 640*320
        rep a32 stosw
        mov eax, 0x7e0          ; green
        mov ecx, 640*320
        rep a32 stosw
        mov eax, 0xf800         ; red
        mov ecx, 640*320
        rep a32 stosw

again:
        ;; Use VBE function 7 Set Display Start to flip between
        ;; the blue/green and green/red regions.
        mov ax, 0x4f07
        mov bx, 0x0
        mov cx, 0x0
        mov dx, [offset1]
        int 0x10
        mov dx, [offset1]
        mov ax, [offset2]
        mov [offset1], ax
        mov [offset2], dx
        jmp again

offset1:        dw 0
offset2:        dw 480

gdtinfo:
   dw gdt_end - gdt - 1   ;last byte in table
   dd gdt                 ;start of table

gdt         dd 0,0        ; entry 0 is always unused
flatdesc    db 0xff, 0xff, 0, 0, 0, 10010010b, 11001111b, 0
gdt_end:

; Make the file 512 bytes long
TIMES 510-($-$$) DB 0

; Add the boot signature
dw 0AA55h

        [section .bss]
mode_info:
             ; Mandatory information for all VBE revisions
ModeAttributes          resw 1         ; mode attributes
WinAAttributes          resb 1         ; window A attributes
WinBAttributes          resb 1         ; window B attributes
WinGranularity          resw 1         ; window granularity
WinSize                 resw 1         ; window size
WinASegment             resw 1         ; window A start segment
WinBSegment             resw 1         ; window B start segment
WinFuncPtr              resd 1         ; pointer to window function
BytesPerScanLine        resw 1         ; bytes per scan line

        ; Mandatory information for VBE 1.2 and above
XResolution             resw 1         ; horizontal resolution in pixels or characters3
YResolution             resw 1         ; vertical resolution in pixels or characters
XCharSize               resb 1         ; character cell width in pixels
YCharSize               resb 1         ; character cell height in pixels
NumberOfPlanes          resb 1         ; number of memory planes
BitsPerPixel            resb 1         ; bits per pixel
NumberOfBanks           resb 1         ; number of banks
MemoryModel             resb 1         ; memory model type
BankSize                resb 1         ; bank size in KB
NumberOfImagePages      resb 1         ; number of images
Reserved                resb 1         ; reserved for page function

        ; Direct Color fields (required for direct/6 and YUV/7 memory models)
RedMaskSize             resb 1         ; size of direct color red mask in bits
RedFieldPosition        resb 1         ; bit position of lsb of red mask
GreenMaskSize           resb 1         ; size of direct color green mask in bits
GreenFieldPosition      resb 1         ; bit position of lsb of green mask
BlueMaskSize            resb 1         ; size of direct color blue mask in bits
BlueFieldPosition       resb 1         ; bit position of lsb of blue mask
RsvdMaskSize            resb 1         ; size of direct color reserved mask in bits
RsvdFieldPosition       resb 1         ; bit position of lsb of reserved mask
DirectColorModeInfo     resb 1         ; direct color mode attributes
        ; Mandatory information for VBE 2.0 and above
PhysBasePtr             resd 1         ; physical address for flat memory frame buffer
OffScreenMemOffset      resd 1         ; pointer to start of off screen memory
OffScreenMemSize        resw 1         ; amount of off screen memory in 1k units