Re: [Qemu-devel] PATCH: fix bgr color mapping on qemu on Solaris/SPARC

[Leonardo E. Reiter]

This was the original Solaris patch that I based my own patch on.  At
Paul Brook's suggestion, I made the translations more static (and as you
are suggesting now, Anthony.)  However, Fabrice commented that my patch
was not correct for various reasons (quote from Fabrice on 2006-05-01):

"Hi,

I did not accept your patch for two reasons:

1) You changed the guest vga frame buffer format and I don't think this
is what you wanted (it can be useful to emulate VGA on big endian guests
though - the current implementation for ppc guests is a hack). What is
needed is to swap the host pixel format.

2) You added support for RGB swapping for 8/15/16 depths and I am not
sure this is useful. Endianness swapping would be more useful for 15/16
depths.

Fabrice."

I don't know how to get around #1, and certainly not without some
serious performance penalty.  I only implemented the #2 bit because it
was part of the original patch that I based mine on.  I agree that it's
not nearly as relevant as 24-bit.  In any case:

http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00195.html

http://lists.gnu.org/archive/html/qemu-devel/2006-04/msg00230.html

Just FYI in case it helps anyone.  There is no performance degradation
at all with that version of the patch, and it's helping some of my
SunRay customers with the colors in 24-bit mode just fine when running
Windows XP guests off a Linux server.  It does not address the VNC bits
however, as it was developed before the built-in VNC was available.

Regards,

Leo Reiter

Anthony Liguori wrote:
> Ben Taylor wrote:
> 
>> Enclosed is a patch that fixes the color mapping when running qemu on
>> a Solaris/Sparc
>> system.  To enable the color mapping bgr, call qemu with the flag "-bgr".
>>
>> This patch *requires* the qemu-0.8.1cvs-sparc-solaris.patch diff that
>> was posted
>> earlier today.
>>
>> I separated out this patch as there has been much discussion about
>> fixing the bgr
>> problem, but I haven't seen any other code posted.  This has been
>> tested with
>> Win98SE and DamnSmallLinux on an Ultra 80 running Solaris 10.
>>
>> Note:  VNC and Sparc are still not playing nicely, so you will see
>> inverted colors if
>> you run qemu on a Solaris Sparc host and display with vnc.
>>   
> 
> 
> This is going to almost certainly going to have a negative performance
> consequence.  A better solution would be introduce a second set of
> functions that were byte swapped and set the rgb_to_pixel function
> pointer appropriately if bgr is enabled.
> 
> I would expect that you can automate the selecting of bgr too based on
> the host architecture and the target architecture.  There shouldn't be a
> need for a new command line option.
> 
> If you added a byte_order member to the DisplayState, this would fix the
> vnc case too.
> 
> Regards,
> 
> Anthony Liguori
> 
>> Ben
>> ------------------------------------------------------------------------
>>
>> diff -ruN qemu-solaris-sparc/hw/vga.c qemu/hw/vga.c
>> --- qemu-solaris-sparc/hw/vga.c    2006-04-08 21:06:34.000000000 -0400
>> +++ qemu/hw/vga.c    2006-05-10 12:31:31.059297000 -0400
>> @@ -790,23 +790,43 @@
>>  
>>  static inline unsigned int rgb_to_pixel8(unsigned int r, unsigned int
>> g, unsigned b)
>>  {
>> +if (bgr_display_enabled) {
>> +    return ((b >> 5) << 5) | ((g >> 5) << 2) | (r >> 6);
>> +}
>> +else {
>>      return ((r >> 5) << 5) | ((g >> 5) << 2) | (b >> 6);
>>  }
>> +}
>>  
>>  static inline unsigned int rgb_to_pixel15(unsigned int r, unsigned
>> int g, unsigned b)
>>  {
>> +if (bgr_display_enabled) {
>> +    return ((b >> 3) << 10) | ((g >> 3) << 5) | (r >> 3);
>> +}
>> +else {
>>      return ((r >> 3) << 10) | ((g >> 3) << 5) | (b >> 3);
>>  }
>> +}
>>  
>>  static inline unsigned int rgb_to_pixel16(unsigned int r, unsigned
>> int g, unsigned b)
>>  {
>> +if (bgr_display_enabled) {
>> +    return ((b >> 3) << 11) | ((g >> 2) << 5) | (r >> 3);
>> +}
>> +else {
>>      return ((r >> 3) << 11) | ((g >> 2) << 5) | (b >> 3);
>>  }
>> +}
>>  
>>  static inline unsigned int rgb_to_pixel32(unsigned int r, unsigned
>> int g, unsigned b)
>>  {
>> +if (bgr_display_enabled) {
>> +    return (b << 16) | (g << 8) | r;
>> +}
>> +else {
>>      return (r << 16) | (g << 8) | b;
>>  }
>> +}
>>  
>>  #define DEPTH 8
>>  #include "vga_template.h"
>> @@ -1366,6 +1386,8 @@
>>  
>>      if (disp_width != s->last_width ||
>>          height != s->last_height) {
>> +        if (cirrus_vga_enabled && s->get_bpp(s) >= 8)
>> +            memset(s->vram_ptr, 0xff, s->vram_size);
>>          dpy_resize(s->ds, disp_width, height);
>>          s->last_scr_width = disp_width;
>>          s->last_scr_height = height;
>> diff -ruN qemu-solaris-sparc/vl.c qemu/vl.c
>> --- qemu-solaris-sparc/vl.c    2006-05-10 12:37:40.467126000 -0400
>> +++ qemu/vl.c    2006-05-10 12:31:46.354278000 -0400
>> @@ -130,6 +130,7 @@
>>  int vm_running;
>>  int rtc_utc = 1;
>>  int cirrus_vga_enabled = 1;
>> +int bgr_display_enabled = 0;
>>  #ifdef TARGET_SPARC
>>  int graphic_width = 1024;
>>  int graphic_height = 768;
>> @@ -4626,6 +4627,7 @@
>>             "-m megs         set virtual RAM size to megs MB
>> [default=%d]\n"
>>             "-smp n          set the number of CPUs to 'n' [default=1]\n"
>>             "-nographic      disable graphical output and redirect
>> serial I/Os to console\n"
>> +           "-bgr            invert colors for HOSTS using certain
>> SPARC frame buffers\n"
>>  #ifndef _WIN32
>>         "-k language     use keyboard layout (for example \"fr\" for
>> French)\n"
>>  #endif
>> @@ -4782,6 +4784,7 @@
>>      QEMU_OPTION_cirrusvga,
>>      QEMU_OPTION_g,
>>      QEMU_OPTION_std_vga,
>> +    QEMU_OPTION_bgr,
>>      QEMU_OPTION_monitor,
>>      QEMU_OPTION_serial,
>>      QEMU_OPTION_parallel,
>> @@ -4861,6 +4864,7 @@
>>      { "full-screen", 0, QEMU_OPTION_full_screen },
>>      { "pidfile", HAS_ARG, QEMU_OPTION_pidfile },
>>      { "win2k-hack", 0, QEMU_OPTION_win2k_hack },
>> +    { "bgr", 0, QEMU_OPTION_bgr },
>>      { "usbdevice", HAS_ARG, QEMU_OPTION_usbdevice },
>>      { "smp", HAS_ARG, QEMU_OPTION_smp },
>>      { "vnc", HAS_ARG, QEMU_OPTION_vnc },
>> @@ -5365,6 +5369,9 @@
>>              case QEMU_OPTION_std_vga:
>>                  cirrus_vga_enabled = 0;
>>                  break;
>> +            case QEMU_OPTION_bgr:
>> +                bgr_display_enabled = 1;
>> +                break;
>>              case QEMU_OPTION_g:
>>                  {
>>                      const char *p;
>> diff -ruN qemu-solaris-sparc/vl.h qemu/vl.h
>> --- qemu-solaris-sparc/vl.h    2006-05-03 18:02:44.000000000 -0400
>> +++ qemu/vl.h    2006-05-10 12:31:51.368923000 -0400
>> @@ -135,6 +135,7 @@
>>  extern int bios_size;
>>  extern int rtc_utc;
>>  extern int cirrus_vga_enabled;
>> +extern int bgr_display_enabled;
>>  extern int graphic_width;
>>  extern int graphic_height;
>>  extern int graphic_depth;
>>  
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> Qemu-devel mailing list
>> address@hidden
>> http://lists.nongnu.org/mailman/listinfo/qemu-devel
>>   
> 
> 
> 
> 
> _______________________________________________
> Qemu-devel mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/qemu-devel

-- 
Leonardo E. Reiter
Vice President of Product Development, CTO

Win4Lin, Inc.
Virtual Computing that means Business
Main: +1 512 339 7979
Fax: +1 512 532 6501
http://www.win4lin.com