* [Qemu-devel] Updated BGR vs. RGB vga patch... @ 2006-04-10 16:25 Leonardo E. Reiter 2006-04-10 16:35 ` Paul Brook 2006-04-10 23:12 ` Thiemo Seufer 0 siblings, 2 replies; 17+ messages in thread From: Leonardo E. Reiter @ 2006-04-10 16:25 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 965 bytes --] Hello, attached is an updated version (against today's CVS) of a patch to enable B/G/R color encoding rather than R/G/B with the command-line option -bgr. I found the original here (post by Martin Bochnig): http://lists.nongnu.org/archive/html/qemu-devel/2005-09/msg00059.html It's main intention is to deal with some 24-bit Sun/SPARC X servers, including SunRays. But you can also mimic this behavior with a VNC server if you want. Anyway, it may not be too interesting to most, but it is to me at the moment, so I figured I'd share. To use it, you can pass in the -bgr command line option to qemu. You can get some psychadelic colors if you're really bored and want to try it on an R/G/B 24-bit X server, like XFree86 or X.org :) Regards, Leo Reiter -- Leonardo E. Reiter Vice President of Product Development, CTO Win4Lin, Inc. Virtual Computing from Desktop to Data Center Main: +1 512 339 7979 Fax: +1 512 532 6501 http://www.win4lin.com [-- Attachment #2: qemu-vga-bgr.patch --] [-- Type: text/x-patch, Size: 3466 bytes --] Index: vl.c =================================================================== RCS file: /cvsroot/qemu/qemu/vl.c,v retrieving revision 1.168 diff -a -u -r1.168 vl.c --- vl.c 9 Apr 2006 01:32:52 -0000 1.168 +++ vl.c 10 Apr 2006 14:07:26 -0000 @@ -128,6 +128,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; @@ -4131,6 +4132,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 @@ -4283,6 +4285,7 @@ QEMU_OPTION_cirrusvga, QEMU_OPTION_g, QEMU_OPTION_std_vga, + QEMU_OPTION_bgr, QEMU_OPTION_monitor, QEMU_OPTION_serial, QEMU_OPTION_parallel, @@ -4360,6 +4363,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 }, @@ -4850,6 +4854,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; Index: vl.h =================================================================== RCS file: /cvsroot/qemu/qemu/vl.h,v retrieving revision 1.107 diff -a -u -r1.107 vl.h --- vl.h 9 Apr 2006 01:32:52 -0000 1.107 +++ vl.h 10 Apr 2006 14:07:26 -0000 @@ -130,6 +130,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; Index: hw/vga.c =================================================================== RCS file: /cvsroot/qemu/qemu/hw/vga.c,v retrieving revision 1.42 diff -a -u -r1.42 vga.c --- hw/vga.c 9 Apr 2006 01:06:34 -0000 1.42 +++ hw/vga.c 10 Apr 2006 14:07:26 -0000 @@ -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" ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 16:25 [Qemu-devel] Updated BGR vs. RGB vga patch Leonardo E. Reiter @ 2006-04-10 16:35 ` Paul Brook 2006-04-10 16:41 ` Leonardo E. Reiter 2006-04-10 16:44 ` Leonardo E. Reiter 2006-04-10 23:12 ` Thiemo Seufer 1 sibling, 2 replies; 17+ messages in thread From: Paul Brook @ 2006-04-10 16:35 UTC (permalink / raw) To: qemu-devel On Monday 10 April 2006 17:25, Leonardo E. Reiter wrote: > Hello, > > attached is an updated version (against today's CVS) of a patch to > enable B/G/R color encoding rather than R/G/B with the command-line > option -bgr. I found the original here (post by Martin Bochnig): Shouldn't we be able to figure this out by asking SDL and/or the X server? Also, adding an if to the low-level conversion routines seems a bad idea for performance. Paul ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 16:35 ` Paul Brook @ 2006-04-10 16:41 ` Leonardo E. Reiter 2006-04-10 16:46 ` Paul Brook 2006-04-10 16:44 ` Leonardo E. Reiter 1 sibling, 1 reply; 17+ messages in thread From: Leonardo E. Reiter @ 2006-04-10 16:41 UTC (permalink / raw) To: Paul Brook; +Cc: qemu-devel You can definitely figure it out by asking the X server or SDL. I don't know enough SDL to have hacked it in myself. X is pretty simple on the other hand. As for where to add it, I agree that the low level conversions are not a great place to add this. I didn't originate the patch, I just updated it. Unfortunately none of the VGA is optimized - everything happens pixel by pixel already. Adding what probably amounts to 2 instructions (probably a couple of clock cycles) per pixel doesn't seem to do anything for performance really. The VGA is already slow. Unfortunately X11 will not allow you to force a certain color order in 24-bit mode. You can tell it to force byte-order but this only affects 16-bit blits - it's ignored for 24-bit since the individual components of the 24-bit blits, even if packed into 32-bits, are still 8-bits long. That's what the color masks are for. There is no efficient way to do this at the server level that I can see. If you find a better way (within the current scope of the VGA architecture), I'd be glad to try to implement it. I was just sharing something that was useful to me. Thanks, Leo Reiter Paul Brook wrote: > On Monday 10 April 2006 17:25, Leonardo E. Reiter wrote: > >>Hello, >> >>attached is an updated version (against today's CVS) of a patch to >>enable B/G/R color encoding rather than R/G/B with the command-line >>option -bgr. I found the original here (post by Martin Bochnig): > > > Shouldn't we be able to figure this out by asking SDL and/or the X server? > Also, adding an if to the low-level conversion routines seems a bad idea for > performance. > > Paul -- Leonardo E. Reiter Vice President of Product Development, CTO Win4Lin, Inc. Virtual Computing from Desktop to Data Center Main: +1 512 339 7979 Fax: +1 512 532 6501 http://www.win4lin.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 16:41 ` Leonardo E. Reiter @ 2006-04-10 16:46 ` Paul Brook 2006-04-10 16:48 ` Leonardo E. Reiter 0 siblings, 1 reply; 17+ messages in thread From: Paul Brook @ 2006-04-10 16:46 UTC (permalink / raw) To: qemu-devel > Unfortunately X11 will not allow you to force a certain color order in > 24-bit mode. You can tell it to force byte-order but this only affects > 16-bit blits - it's ignored for 24-bit since the individual components > of the 24-bit blits, even if packed into 32-bits, are still 8-bits long. > That's what the color masks are for. There is no efficient way to do > this at the server level that I can see. If you find a better way > (within the current scope of the VGA architecture), I'd be glad to try > to implement it. I was just sharing something that was useful to me. It should be possible to select between different 24-bit packings the same way different bit depths are handled. Paul ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 16:46 ` Paul Brook @ 2006-04-10 16:48 ` Leonardo E. Reiter 0 siblings, 0 replies; 17+ messages in thread From: Leonardo E. Reiter @ 2006-04-10 16:48 UTC (permalink / raw) To: qemu-devel Okay, that makes perfect sense. Let me see if I can figure out how to get SDL to report the ordering, and I'll try to roll it out that way. thanks for clarifying, - Leo Reiter Paul Brook wrote: >>Unfortunately X11 will not allow you to force a certain color order in >>24-bit mode. You can tell it to force byte-order but this only affects >>16-bit blits - it's ignored for 24-bit since the individual components >>of the 24-bit blits, even if packed into 32-bits, are still 8-bits long. >> That's what the color masks are for. There is no efficient way to do >>this at the server level that I can see. If you find a better way >>(within the current scope of the VGA architecture), I'd be glad to try >>to implement it. I was just sharing something that was useful to me. > > > It should be possible to select between different 24-bit packings the same way > different bit depths are handled. > > Paul -- Leonardo E. Reiter Vice President of Product Development, CTO Win4Lin, Inc. Virtual Computing from Desktop to Data Center Main: +1 512 339 7979 Fax: +1 512 532 6501 http://www.win4lin.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 16:35 ` Paul Brook 2006-04-10 16:41 ` Leonardo E. Reiter @ 2006-04-10 16:44 ` Leonardo E. Reiter 2006-04-10 16:49 ` Paul Brook 1 sibling, 1 reply; 17+ messages in thread From: Leonardo E. Reiter @ 2006-04-10 16:44 UTC (permalink / raw) Cc: qemu-devel Actually it should probably be made conditionally compiled for now, until the VGA architecture changes to something more efficient. Would you agree with that? That way, people who know they will need the hack can enable it at configure/compile time, and the rest will not be affected. How does that sound? I'd be happy to post an updated patch that does that. - Leo Paul Brook wrote: > On Monday 10 April 2006 17:25, Leonardo E. Reiter wrote: > >>Hello, >> >>attached is an updated version (against today's CVS) of a patch to >>enable B/G/R color encoding rather than R/G/B with the command-line >>option -bgr. I found the original here (post by Martin Bochnig): > > > Shouldn't we be able to figure this out by asking SDL and/or the X server? > Also, adding an if to the low-level conversion routines seems a bad idea for > performance. > > Paul -- Leonardo E. Reiter Vice President of Product Development, CTO Win4Lin, Inc. Virtual Computing from Desktop to Data Center Main: +1 512 339 7979 Fax: +1 512 532 6501 http://www.win4lin.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 16:44 ` Leonardo E. Reiter @ 2006-04-10 16:49 ` Paul Brook 2006-04-10 16:51 ` Leonardo E. Reiter 0 siblings, 1 reply; 17+ messages in thread From: Paul Brook @ 2006-04-10 16:49 UTC (permalink / raw) To: qemu-devel On Monday 10 April 2006 17:44, Leonardo E. Reiter wrote: > Actually it should probably be made conditionally compiled for now, > until the VGA architecture changes to something more efficient. Would > you agree with that? That way, people who know they will need the hack > can enable it at configure/compile time, and the rest will not be affected. > > How does that sound? I'd be happy to post an updated patch that does that. Making it a configure option sounds a stunningly bad idea. If you're going down that route you may as well make all the output format selection a compile-time option. Paul ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 16:49 ` Paul Brook @ 2006-04-10 16:51 ` Leonardo E. Reiter 2006-04-10 16:59 ` Paul Brook 0 siblings, 1 reply; 17+ messages in thread From: Leonardo E. Reiter @ 2006-04-10 16:51 UTC (permalink / raw) To: Paul Brook; +Cc: qemu-devel I was talking about enabling the use of the -bgr flag at compile time, to save the check at runtime if the user doesn't even care about the flag, not forcing the determination of the blit order altogether at compile-time. In any case, this is moot, your clarification on how to better implement it makes it much more clear to me and is a much better idea. Mails to the list are a bit delayed, that's all ;) Thanks, Leo Paul Brook wrote: > On Monday 10 April 2006 17:44, Leonardo E. Reiter wrote: > >>Actually it should probably be made conditionally compiled for now, >>until the VGA architecture changes to something more efficient. Would >>you agree with that? That way, people who know they will need the hack >>can enable it at configure/compile time, and the rest will not be affected. >> >>How does that sound? I'd be happy to post an updated patch that does that. > > > Making it a configure option sounds a stunningly bad idea. If you're going > down that route you may as well make all the output format selection a > compile-time option. > > Paul -- Leonardo E. Reiter Vice President of Product Development, CTO Win4Lin, Inc. Virtual Computing from Desktop to Data Center Main: +1 512 339 7979 Fax: +1 512 532 6501 http://www.win4lin.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 16:51 ` Leonardo E. Reiter @ 2006-04-10 16:59 ` Paul Brook 2006-04-10 18:24 ` Leonardo E. Reiter 2006-04-10 18:25 ` Leonardo E. Reiter 0 siblings, 2 replies; 17+ messages in thread From: Paul Brook @ 2006-04-10 16:59 UTC (permalink / raw) To: qemu-devel On Monday 10 April 2006 17:51, Leonardo E. Reiter wrote: > I was talking about enabling the use of the -bgr flag at compile time, > to save the check at runtime if the user doesn't even care about the > flag, not forcing the determination of the blit order altogether at > compile-time. In any case, this is moot, your clarification on how to > better implement it makes it much more clear to me and is a much better > idea. Mails to the list are a bit delayed, that's all ;) Ok. For the record I also think it's a bad idea to have features conditionally compiled. Either something is worth including, or we have to ask whether there's any point having it in qemu at all. The exception being debug code, which probably isn't useful unless you're already building qemu from source. Paul ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 16:59 ` Paul Brook @ 2006-04-10 18:24 ` Leonardo E. Reiter 2006-04-10 19:08 ` malc 2006-04-10 18:25 ` Leonardo E. Reiter 1 sibling, 1 reply; 17+ messages in thread From: Leonardo E. Reiter @ 2006-04-10 18:24 UTC (permalink / raw) To: qemu-devel Here's a better patch... I can't seem to validate BGR X servers in 15 or 16-bit mode. This may not make sense (or be prevalent) on little-endian machines anyway (I'm using VNC server on a little endian box to test against.) In any case, 24 and 32-bit works like a charm. I use the same basic logic as the original patch, so it's possible it was always broken for 16-bit... especially given that most Sun X servers run in 24-bit, which is what the originators intended the patch for. Anyway, I didn't spend time figuring out how to query SDL for the ordering. You still have to manually pass in -bgr. The good news is that all the computation is now done at compile-time, with only very few tests done at run-time. I agree with your assessment of compile-time versus run-time options... I was merely suggesting a compromise to fend off the minor performance hit of using the old patch. But the method you suggested is much better and that's how it's implemented now. Thanks, Leo Reiter Paul Brook wrote: > Ok. For the record I also think it's a bad idea to have features conditionally > compiled. Either something is worth including, or we have to ask whether > there's any point having it in qemu at all. > The exception being debug code, which probably isn't useful unless you're > already building qemu from source. > > Paul -- Leonardo E. Reiter Vice President of Product Development, CTO Win4Lin, Inc. Virtual Computing from Desktop to Data Center Main: +1 512 339 7979 Fax: +1 512 532 6501 http://www.win4lin.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 18:24 ` Leonardo E. Reiter @ 2006-04-10 19:08 ` malc 2006-04-10 19:11 ` Leonardo E. Reiter ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: malc @ 2006-04-10 19:08 UTC (permalink / raw) To: qemu-devel On Mon, 10 Apr 2006, Leonardo E. Reiter wrote: > Anyway, I didn't spend time figuring out how to query SDL for the ordering. > You still have to manually pass in -bgr. The good news is that all the > computation is now done at compile-time, with only very few tests done at > run-time. I agree with your assessment of compile-time versus run-time > options... I was merely suggesting a compromise to fend off the minor > performance hit of using the old patch. But the method you suggested is much > better and that's how it's implemented now. I maybe going out on a limb here, but isn't rgb/bgr trivially deducible from: sdl.c:screen->format->[RGBA]shift. -- mailto:malc@pulsesoft.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 19:08 ` malc @ 2006-04-10 19:11 ` Leonardo E. Reiter 2006-04-11 17:06 ` Leonardo E. Reiter 2006-04-11 17:11 ` Leonardo E. Reiter 2 siblings, 0 replies; 17+ messages in thread From: Leonardo E. Reiter @ 2006-04-10 19:11 UTC (permalink / raw) To: qemu-devel Probably... I just don't know anything about SDL and I needed this patch to work right away with at least the command-line option. It's definitely deducible from X11, and that's very easy. - Leo Reiter malc wrote: > On Mon, 10 Apr 2006, Leonardo E. Reiter wrote: > >> Anyway, I didn't spend time figuring out how to query SDL for the >> ordering. You still have to manually pass in -bgr. The good news is >> that all the computation is now done at compile-time, with only very >> few tests done at run-time. I agree with your assessment of >> compile-time versus run-time options... I was merely suggesting a >> compromise to fend off the minor performance hit of using the old >> patch. But the method you suggested is much better and that's how >> it's implemented now. > > > I maybe going out on a limb here, but isn't rgb/bgr trivially deducible > from: sdl.c:screen->format->[RGBA]shift. > -- Leonardo E. Reiter Vice President of Product Development, CTO Win4Lin, Inc. Virtual Computing from Desktop to Data Center Main: +1 512 339 7979 Fax: +1 512 532 6501 http://www.win4lin.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 19:08 ` malc 2006-04-10 19:11 ` Leonardo E. Reiter @ 2006-04-11 17:06 ` Leonardo E. Reiter 2006-04-11 17:11 ` Leonardo E. Reiter 2 siblings, 0 replies; 17+ messages in thread From: Leonardo E. Reiter @ 2006-04-11 17:06 UTC (permalink / raw) To: qemu-devel How's this: Index: sdl.c =================================================================== RCS file: /cvsroot/qemu/qemu/sdl.c,v retrieving revision 1.25 diff -a -u -r1.25 sdl.c --- sdl.c 9 Apr 2006 01:06:34 -0000 1.25 +++ sdl.c 11 Apr 2006 17:03:51 -0000 @@ -510,4 +510,9 @@ gui_fullscreen_initial_grab = 1; sdl_grab_start(); } + + /* guess to use BGR mode by seeing if the blue color mask is greater than + * the red color mask, indicating that blue comes before red */ + if (screen->format->Bmask > screen->format->Rmask) + bgr_display_enabled = 1; } combined with my prior bgr patch of course (bgr_display_enabled doesn't exist otherwise.) If you think it's right, let me know, or post a better one. Either way I can post an updated BGR patch with this hack. You can still manually use -bgr if you want, or perhaps the meaning of -bgr should be to enable this test. It may not be correct in all circumstances. Maybe Paul Brook can weigh in too since he originally suggested that this patch should determine this automatically. Thanks, Leo Reiter malc wrote: > On Mon, 10 Apr 2006, Leonardo E. Reiter wrote: > >> Anyway, I didn't spend time figuring out how to query SDL for the >> ordering. You still have to manually pass in -bgr. The good news is >> that all the computation is now done at compile-time, with only very >> few tests done at run-time. I agree with your assessment of >> compile-time versus run-time options... I was merely suggesting a >> compromise to fend off the minor performance hit of using the old >> patch. But the method you suggested is much better and that's how >> it's implemented now. > > > I maybe going out on a limb here, but isn't rgb/bgr trivially deducible > from: sdl.c:screen->format->[RGBA]shift. > -- Leonardo E. Reiter Vice President of Product Development, CTO Win4Lin, Inc. Virtual Computing from Desktop to Data Center Main: +1 512 339 7979 Fax: +1 512 532 6501 http://www.win4lin.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 19:08 ` malc 2006-04-10 19:11 ` Leonardo E. Reiter 2006-04-11 17:06 ` Leonardo E. Reiter @ 2006-04-11 17:11 ` Leonardo E. Reiter 2 siblings, 0 replies; 17+ messages in thread From: Leonardo E. Reiter @ 2006-04-11 17:11 UTC (permalink / raw) To: qemu-devel By the way, I did verify that the patch works correctly in B/G/R 5/6/5 mode - I wasn't sure about 16-bit BGR before. However, in 16-bit mode, only -std-vga works correctly if the X server is in BGR mode. In 24-bit BGR mode, cirrus has no problem. I think there is some 16-bit trickery going on in the cirrus implementation, but I haven't had much time to figure it out. Also the cirrus implementation is really hard to follow for me, so if anyone has any idea, I'd appreciate it. Thanks, Leo Reiter malc wrote: > On Mon, 10 Apr 2006, Leonardo E. Reiter wrote: > >> Anyway, I didn't spend time figuring out how to query SDL for the >> ordering. You still have to manually pass in -bgr. The good news is >> that all the computation is now done at compile-time, with only very >> few tests done at run-time. I agree with your assessment of >> compile-time versus run-time options... I was merely suggesting a >> compromise to fend off the minor performance hit of using the old >> patch. But the method you suggested is much better and that's how >> it's implemented now. > > > I maybe going out on a limb here, but isn't rgb/bgr trivially deducible > from: sdl.c:screen->format->[RGBA]shift. > -- Leonardo E. Reiter Vice President of Product Development, CTO Win4Lin, Inc. Virtual Computing from Desktop to Data Center Main: +1 512 339 7979 Fax: +1 512 532 6501 http://www.win4lin.com ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 16:59 ` Paul Brook 2006-04-10 18:24 ` Leonardo E. Reiter @ 2006-04-10 18:25 ` Leonardo E. Reiter 1 sibling, 0 replies; 17+ messages in thread From: Leonardo E. Reiter @ 2006-04-10 18:25 UTC (permalink / raw) To: qemu-devel [-- Attachment #1: Type: text/plain, Size: 1560 bytes --] Here's a better patch (attached this time!)... I can't seem to validate BGR X servers in 15 or 16-bit mode. This may not make sense (or be prevalent) on little-endian machines anyway (I'm using VNC server on a little endian box to test against.) In any case, 24 and 32-bit works like a charm. I use the same basic logic as the original patch, so it's possible it was always broken for 16-bit... especially given that most Sun X servers run in 24-bit, which is what the originators intended the patch for. Anyway, I didn't spend time figuring out how to query SDL for the ordering. You still have to manually pass in -bgr. The good news is that all the computation is now done at compile-time, with only very few tests done at run-time. I agree with your assessment of compile-time versus run-time options... I was merely suggesting a compromise to fend off the minor performance hit of using the old patch. But the method you suggested is much better and that's how it's implemented now. Thanks, Leo Reiter Paul Brook wrote: > Ok. For the record I also think it's a bad idea to have features conditionally > compiled. Either something is worth including, or we have to ask whether > there's any point having it in qemu at all. > The exception being debug code, which probably isn't useful unless you're > already building qemu from source. > > Paul -- Leonardo E. Reiter Vice President of Product Development, CTO Win4Lin, Inc. Virtual Computing from Desktop to Data Center Main: +1 512 339 7979 Fax: +1 512 532 6501 http://www.win4lin.com [-- Attachment #2: qemu-vga-bgr-fast.patch --] [-- Type: text/x-patch, Size: 11488 bytes --] Index: vl.c =================================================================== RCS file: /cvsroot/qemu/qemu/vl.c,v retrieving revision 1.168 diff -a -u -r1.168 vl.c --- vl.c 9 Apr 2006 01:32:52 -0000 1.168 +++ vl.c 10 Apr 2006 17:57:12 -0000 @@ -128,6 +128,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; @@ -4131,6 +4132,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 @@ -4283,6 +4285,7 @@ QEMU_OPTION_cirrusvga, QEMU_OPTION_g, QEMU_OPTION_std_vga, + QEMU_OPTION_bgr, QEMU_OPTION_monitor, QEMU_OPTION_serial, QEMU_OPTION_parallel, @@ -4360,6 +4363,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 }, @@ -4850,6 +4854,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; Index: vl.h =================================================================== RCS file: /cvsroot/qemu/qemu/vl.h,v retrieving revision 1.107 diff -a -u -r1.107 vl.h --- vl.h 9 Apr 2006 01:32:52 -0000 1.107 +++ vl.h 10 Apr 2006 17:57:12 -0000 @@ -130,6 +130,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; Index: hw/vga.c =================================================================== RCS file: /cvsroot/qemu/qemu/hw/vga.c,v retrieving revision 1.42 diff -a -u -r1.42 vga.c --- hw/vga.c 9 Apr 2006 01:06:34 -0000 1.42 +++ hw/vga.c 10 Apr 2006 17:57:12 -0000 @@ -810,15 +810,27 @@ #define DEPTH 8 #include "vga_template.h" +#define BGR_DISPLAY_TYPE +#define DEPTH 8 +#include "vga_template.h" #define DEPTH 15 #include "vga_template.h" +#define BGR_DISPLAY_TYPE +#define DEPTH 15 +#include "vga_template.h" #define DEPTH 16 #include "vga_template.h" +#define BGR_DISPLAY_TYPE +#define DEPTH 16 +#include "vga_template.h" #define DEPTH 32 #include "vga_template.h" +#define BGR_DISPLAY_TYPE +#define DEPTH 32 +#include "vga_template.h" static unsigned int rgb_to_pixel8_dup(unsigned int r, unsigned int g, unsigned b) { @@ -829,6 +841,15 @@ return col; } +static unsigned int bgr_to_pixel8_dup(unsigned int r, unsigned int g, unsigned b) +{ + unsigned int col; + col = rgb_to_pixel8(b, g, r); + col |= col << 8; + col |= col << 16; + return col; +} + static unsigned int rgb_to_pixel15_dup(unsigned int r, unsigned int g, unsigned b) { unsigned int col; @@ -837,6 +858,14 @@ return col; } +static unsigned int bgr_to_pixel15_dup(unsigned int r, unsigned int g, unsigned b) +{ + unsigned int col; + col = rgb_to_pixel15(b, g, r); + col |= col << 16; + return col; +} + static unsigned int rgb_to_pixel16_dup(unsigned int r, unsigned int g, unsigned b) { unsigned int col; @@ -845,13 +874,24 @@ return col; } -static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g, unsigned b) +static unsigned int bgr_to_pixel16_dup(unsigned int r, unsigned int g, unsigned b) { unsigned int col; - col = rgb_to_pixel32(r, g, b); + col = rgb_to_pixel16(b, g, r); + col |= col << 16; return col; } +static unsigned int rgb_to_pixel32_dup(unsigned int r, unsigned int g, unsigned b) +{ + return rgb_to_pixel32(r, g, b); +} + +static unsigned int bgr_to_pixel32_dup(unsigned int r, unsigned int g, unsigned b) +{ + return rgb_to_pixel32(b, g, r); +} + /* return true if the palette was modified */ static int update_palette16(VGAState *s) { @@ -1190,9 +1230,13 @@ VGA_DRAW_LINE8D2, VGA_DRAW_LINE8, VGA_DRAW_LINE15, + VGA_DRAW_LINE15BGR, VGA_DRAW_LINE16, + VGA_DRAW_LINE16BGR, VGA_DRAW_LINE24, + VGA_DRAW_LINE24BGR, VGA_DRAW_LINE32, + VGA_DRAW_LINE32BGR, VGA_DRAW_LINE_NB, }; @@ -1232,20 +1276,40 @@ vga_draw_line15_16, vga_draw_line15_32, + vga_draw_line15bgr_8, + vga_draw_line15bgr_15, + vga_draw_line15bgr_16, + vga_draw_line15bgr_32, + vga_draw_line16_8, vga_draw_line16_15, vga_draw_line16_16, vga_draw_line16_32, + vga_draw_line16bgr_8, + vga_draw_line16bgr_15, + vga_draw_line16bgr_16, + vga_draw_line16bgr_32, + vga_draw_line24_8, vga_draw_line24_15, vga_draw_line24_16, vga_draw_line24_32, + vga_draw_line24bgr_8, + vga_draw_line24bgr_15, + vga_draw_line24bgr_16, + vga_draw_line24bgr_32, + vga_draw_line32_8, vga_draw_line32_15, vga_draw_line32_16, vga_draw_line32_32, + + vga_draw_line32bgr_8, + vga_draw_line32bgr_15, + vga_draw_line32bgr_16, + vga_draw_line32bgr_32, }; static int vga_get_bpp(VGAState *s) @@ -1349,16 +1413,16 @@ v = VGA_DRAW_LINE8; break; case 15: - v = VGA_DRAW_LINE15; + v = bgr_display_enabled ? VGA_DRAW_LINE15BGR : VGA_DRAW_LINE15; break; case 16: - v = VGA_DRAW_LINE16; + v = bgr_display_enabled ? VGA_DRAW_LINE16BGR : VGA_DRAW_LINE16; break; case 24: - v = VGA_DRAW_LINE24; + v = bgr_display_enabled ? VGA_DRAW_LINE24BGR : VGA_DRAW_LINE24; break; case 32: - v = VGA_DRAW_LINE32; + v = bgr_display_enabled ? VGA_DRAW_LINE32BGR : VGA_DRAW_LINE32; break; } } @@ -1494,17 +1558,21 @@ } else { switch(s->ds->depth) { case 8: - s->rgb_to_pixel = rgb_to_pixel8_dup; + s->rgb_to_pixel = + bgr_display_enabled ? bgr_to_pixel8_dup : rgb_to_pixel8_dup; break; case 15: - s->rgb_to_pixel = rgb_to_pixel15_dup; + s->rgb_to_pixel = + bgr_display_enabled ? bgr_to_pixel15_dup : rgb_to_pixel15_dup; break; default: case 16: - s->rgb_to_pixel = rgb_to_pixel16_dup; + s->rgb_to_pixel = + bgr_display_enabled ? bgr_to_pixel16_dup : rgb_to_pixel16_dup; break; case 32: - s->rgb_to_pixel = rgb_to_pixel32_dup; + s->rgb_to_pixel = + bgr_display_enabled ? bgr_to_pixel32_dup : rgb_to_pixel32_dup; break; } Index: hw/vga_template.h =================================================================== RCS file: /cvsroot/qemu/qemu/hw/vga_template.h,v retrieving revision 1.11 diff -a -u -r1.11 vga_template.h --- hw/vga_template.h 10 Oct 2004 15:44:19 -0000 1.11 +++ hw/vga_template.h 10 Apr 2006 17:57:13 -0000 @@ -35,6 +35,27 @@ #error unsupport depth #endif +/* draw-line function base names for RGB vs. BGR */ +#ifndef BGR_DISPLAY_TYPE +#define VGA_DRAW_LINE15_ vga_draw_line15_ +#define VGA_DRAW_LINE16_ vga_draw_line16_ +#define VGA_DRAW_LINE24_ vga_draw_line24_ +#define VGA_DRAW_LINE32_ vga_draw_line32_ +#define __R_VALUE r +#define __G_VALUE g +#define __B_VALUE b +#else +#define VGA_DRAW_LINE15_ vga_draw_line15bgr_ +#define VGA_DRAW_LINE16_ vga_draw_line16bgr_ +#define VGA_DRAW_LINE24_ vga_draw_line24bgr_ +#define VGA_DRAW_LINE32_ vga_draw_line32bgr_ +#define __R_VALUE b +#define __G_VALUE g +#define __B_VALUE r +#endif /* !BGR_DISPLAY_TYPE */ + +/* avoid redifining common functions when we generate the BGR-only functions */ +#ifndef BGR_DISPLAY_TYPE #if DEPTH != 15 static inline void glue(vga_draw_glyph_line_, DEPTH)(uint8_t *d, @@ -335,14 +356,14 @@ } #endif /* DEPTH != 15 */ - +#endif /* !BGR_DISPLAY_TYPE */ /* XXX: optimize */ /* * 15 bit color */ -static void glue(vga_draw_line15_, DEPTH)(VGAState *s1, uint8_t *d, +static void glue(VGA_DRAW_LINE15_, DEPTH)(VGAState *s1, uint8_t *d, const uint8_t *s, int width) { #if DEPTH == 15 && defined(WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN) @@ -354,9 +375,9 @@ w = width; do { v = lduw_raw((void *)s); - r = (v >> 7) & 0xf8; - g = (v >> 2) & 0xf8; - b = (v << 3) & 0xf8; + __R_VALUE = (v >> 7) & 0xf8; + __G_VALUE = (v >> 2) & 0xf8; + __B_VALUE = (v << 3) & 0xf8; ((PIXEL_TYPE *)d)[0] = glue(rgb_to_pixel, DEPTH)(r, g, b); s += 2; d += BPP; @@ -367,7 +388,7 @@ /* * 16 bit color */ -static void glue(vga_draw_line16_, DEPTH)(VGAState *s1, uint8_t *d, +static void glue(VGA_DRAW_LINE16_, DEPTH)(VGAState *s1, uint8_t *d, const uint8_t *s, int width) { #if DEPTH == 16 && defined(WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN) @@ -379,9 +400,9 @@ w = width; do { v = lduw_raw((void *)s); - r = (v >> 8) & 0xf8; - g = (v >> 3) & 0xfc; - b = (v << 3) & 0xf8; + __R_VALUE = (v >> 8) & 0xf8; + __G_VALUE = (v >> 3) & 0xfc; + __B_VALUE = (v << 3) & 0xf8; ((PIXEL_TYPE *)d)[0] = glue(rgb_to_pixel, DEPTH)(r, g, b); s += 2; d += BPP; @@ -392,7 +413,7 @@ /* * 24 bit color */ -static void glue(vga_draw_line24_, DEPTH)(VGAState *s1, uint8_t *d, +static void glue(VGA_DRAW_LINE24_, DEPTH)(VGAState *s1, uint8_t *d, const uint8_t *s, int width) { int w; @@ -400,7 +421,7 @@ w = width; do { -#if defined(TARGET_WORDS_BIGENDIAN) +#if defined(TARGET_WORDS_BIGENDIAN) || defined(BGR_DISPLAY_TYPE) r = s[0]; g = s[1]; b = s[2]; @@ -418,7 +439,7 @@ /* * 32 bit color */ -static void glue(vga_draw_line32_, DEPTH)(VGAState *s1, uint8_t *d, +static void glue(VGA_DRAW_LINE32_, DEPTH)(VGAState *s1, uint8_t *d, const uint8_t *s, int width) { #if DEPTH == 32 && defined(WORDS_BIGENDIAN) == defined(TARGET_WORDS_BIGENDIAN) @@ -429,7 +450,7 @@ w = width; do { -#if defined(TARGET_WORDS_BIGENDIAN) +#if defined(TARGET_WORDS_BIGENDIAN) || defined(BGR_DISPLAY_TYPE) r = s[1]; g = s[2]; b = s[3]; @@ -445,6 +466,7 @@ #endif } +#ifndef BGR_DISPLAY_TYPE #if DEPTH != 15 void glue(vga_draw_cursor_line_, DEPTH)(uint8_t *d1, const uint8_t *src1, @@ -512,8 +534,19 @@ } } #endif +#endif /* !BGR_DISPLAY_TYPE */ #undef PUT_PIXEL2 #undef DEPTH #undef BPP #undef PIXEL_TYPE +#undef VGA_DRAW_LINE15_ +#undef VGA_DRAW_LINE16_ +#undef VGA_DRAW_LINE24_ +#undef VGA_DRAW_LINE32_ +#undef __R_VALUE +#undef __G_VALUE +#undef __B_VALUE +#ifdef BGR_DISPLAY_TYPE +#undef BGR_DISPLAY_TYPE +#endif ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 16:25 [Qemu-devel] Updated BGR vs. RGB vga patch Leonardo E. Reiter 2006-04-10 16:35 ` Paul Brook @ 2006-04-10 23:12 ` Thiemo Seufer 2006-04-10 23:22 ` Leonardo E. Reiter 1 sibling, 1 reply; 17+ messages in thread From: Thiemo Seufer @ 2006-04-10 23:12 UTC (permalink / raw) To: qemu-devel Leonardo E. Reiter wrote: > Hello, > > attached is an updated version (against today's CVS) of a patch to > enable B/G/R color encoding rather than R/G/B with the command-line > option -bgr. I found the original here (post by Martin Bochnig): > > http://lists.nongnu.org/archive/html/qemu-devel/2005-09/msg00059.html > > It's main intention is to deal with some 24-bit Sun/SPARC X servers, > including SunRays. But you can also mimic this behavior with a VNC > server if you want. > > Anyway, it may not be too interesting to most, but it is to me at the > moment, so I figured I'd share. To use it, you can pass in the -bgr > command line option to qemu. You can get some psychadelic colors if > you're really bored and want to try it on an R/G/B 24-bit X server, like > XFree86 or X.org :) FWIW, the SGI O2 has the same problem, I heard the Xorg server got fixed recently to take this into account. I don't know if the fix is O2 specific. Btw, wouldn't it be the right thing to add a Sun framebuffer emulation instead of fiddling with VGA? Or is the hardware in question actually VGA compatible? Thiemo ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] Updated BGR vs. RGB vga patch... 2006-04-10 23:12 ` Thiemo Seufer @ 2006-04-10 23:22 ` Leonardo E. Reiter 0 siblings, 0 replies; 17+ messages in thread From: Leonardo E. Reiter @ 2006-04-10 23:22 UTC (permalink / raw) To: qemu-devel Hi Thiemo, the goal of my patch (and the original which I derived from) was to give the VGA device the ability to display correctly on BGR X servers. Most likely this is for target-i386 emulation, such as for running Windows guests. That's my need for the patch anyway. In my case, I'm trying to get the colors to look right on a Sun Ray thin client served up from an x86_64 Linux box running the SRSS software from Sun (SunRay server basically.) The emulator sessions run Windows 2000/XP, hence the need for VGA fixes. If the O2's 24-bit display is similar to the Sun's (i.e. B/G/R, 24-bit depth, 32-bit pixel width), then the patch should fix the problem on the O2 as well when using PC hardware emulation (i.e. i386-softmmu) - Leo Reiter Thiemo Seufer wrote: > > FWIW, the SGI O2 has the same problem, I heard the Xorg server got > fixed recently to take this into account. I don't know if the fix is > O2 specific. > > Btw, wouldn't it be the right thing to add a Sun framebuffer emulation > instead of fiddling with VGA? Or is the hardware in question actually > VGA compatible? > > > Thiemo > > > _______________________________________________ > Qemu-devel mailing list > Qemu-devel@nongnu.org > http://lists.nongnu.org/mailman/listinfo/qemu-devel -- Leonardo E. Reiter Vice President of Product Development, CTO Win4Lin, Inc. Virtual Computing from Desktop to Data Center Main: +1 512 339 7979 Fax: +1 512 532 6501 http://www.win4lin.com ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2006-04-11 17:11 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-04-10 16:25 [Qemu-devel] Updated BGR vs. RGB vga patch Leonardo E. Reiter 2006-04-10 16:35 ` Paul Brook 2006-04-10 16:41 ` Leonardo E. Reiter 2006-04-10 16:46 ` Paul Brook 2006-04-10 16:48 ` Leonardo E. Reiter 2006-04-10 16:44 ` Leonardo E. Reiter 2006-04-10 16:49 ` Paul Brook 2006-04-10 16:51 ` Leonardo E. Reiter 2006-04-10 16:59 ` Paul Brook 2006-04-10 18:24 ` Leonardo E. Reiter 2006-04-10 19:08 ` malc 2006-04-10 19:11 ` Leonardo E. Reiter 2006-04-11 17:06 ` Leonardo E. Reiter 2006-04-11 17:11 ` Leonardo E. Reiter 2006-04-10 18:25 ` Leonardo E. Reiter 2006-04-10 23:12 ` Thiemo Seufer 2006-04-10 23:22 ` Leonardo E. Reiter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).