| Message ID | 1458971873-2768-1-git-send-email-dhannawatpooja1@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
CCing Eric Blake and Gerd Hoffmann. On Sat, Mar 26, 2016 at 11:27 AM, Pooja Dhannawat <dhannawatpooja1@gmail.com > wrote: > Removing support for DEPTH != 32 from blizzard template header > and file that includes it, as macro DEPTH == 32 only used. > > Reviewed-by: Eric Blake <eblake@redhat.com> > Signed-off-by: Pooja Dhannawat <dhannawatpooja1@gmail.com> > --- > hw/display/blizzard.c | 41 > ++++------------------------------------- > hw/display/blizzard_template.h | 30 +----------------------------- > 2 files changed, 5 insertions(+), 66 deletions(-) > > diff --git a/hw/display/blizzard.c b/hw/display/blizzard.c > index c231960..5684e49 100644 > --- a/hw/display/blizzard.c > +++ b/hw/display/blizzard.c > @@ -925,14 +925,6 @@ static void blizzard_update_display(void *opaque) > s->my[1] = 0; > } > > -#define DEPTH 8 > -#include "blizzard_template.h" > -#define DEPTH 15 > -#include "blizzard_template.h" > -#define DEPTH 16 > -#include "blizzard_template.h" > -#define DEPTH 24 > -#include "blizzard_template.h" > #define DEPTH 32 > #include "blizzard_template.h" > > @@ -951,35 +943,10 @@ void *s1d13745_init(qemu_irq gpio_int) > s->con = graphic_console_init(NULL, 0, &blizzard_ops, s); > surface = qemu_console_surface(s->con); > > - switch (surface_bits_per_pixel(surface)) { > - case 0: > - s->line_fn_tab[0] = s->line_fn_tab[1] = > - g_malloc0(sizeof(blizzard_fn_t) * 0x10); > - break; > - case 8: > - s->line_fn_tab[0] = blizzard_draw_fn_8; > - s->line_fn_tab[1] = blizzard_draw_fn_r_8; > - break; > - case 15: > - s->line_fn_tab[0] = blizzard_draw_fn_15; > - s->line_fn_tab[1] = blizzard_draw_fn_r_15; > - break; > - case 16: > - s->line_fn_tab[0] = blizzard_draw_fn_16; > - s->line_fn_tab[1] = blizzard_draw_fn_r_16; > - break; > - case 24: > - s->line_fn_tab[0] = blizzard_draw_fn_24; > - s->line_fn_tab[1] = blizzard_draw_fn_r_24; > - break; > - case 32: > - s->line_fn_tab[0] = blizzard_draw_fn_32; > - s->line_fn_tab[1] = blizzard_draw_fn_r_32; > - break; > - default: > - fprintf(stderr, "%s: Bad color depth\n", __FUNCTION__); > - exit(1); > - } > + assert(surface_bits_per_pixel(surface) == 32) > + > + s->line_fn_tab[0] = blizzard_draw_fn_32; > + s->line_fn_tab[1] = blizzard_draw_fn_r_32; > > blizzard_reset(s); > > diff --git a/hw/display/blizzard_template.h > b/hw/display/blizzard_template.h > index b7ef27c..bc38d7a 100644 > --- a/hw/display/blizzard_template.h > +++ b/hw/display/blizzard_template.h > @@ -19,31 +19,7 @@ > */ > > #define SKIP_PIXEL(to) (to += deststep) > -#if DEPTH == 8 > -# define PIXEL_TYPE uint8_t > -# define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while > (0) > -# define COPY_PIXEL1(to, from) (*to++ = from) > -#elif DEPTH == 15 || DEPTH == 16 > -# define PIXEL_TYPE uint16_t > -# define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while > (0) > -# define COPY_PIXEL1(to, from) (*to++ = from) > -#elif DEPTH == 24 > -# define PIXEL_TYPE uint8_t > -# define COPY_PIXEL(to, from) \ > - do { \ > - to[0] = from; \ > - to[1] = (from) >> 8; \ > - to[2] = (from) >> 16; \ > - SKIP_PIXEL(to); \ > - } while (0) > - > -# define COPY_PIXEL1(to, from) \ > - do { \ > - *to++ = from; \ > - *to++ = (from) >> 8; \ > - *to++ = (from) >> 16; \ > - } while (0) > -#elif DEPTH == 32 > +#if DEPTH == 32 > # define PIXEL_TYPE uint32_t > # define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while > (0) > # define COPY_PIXEL1(to, from) (*to++ = from) > @@ -58,9 +34,6 @@ > static void glue(blizzard_draw_line16_, DEPTH)(PIXEL_TYPE *dest, > const uint16_t *src, unsigned int width) > { > -#if !defined(SWAP_WORDS) && DEPTH == 16 > - memcpy(dest, src, width); > -#else > uint16_t data; > unsigned int r, g, b; > const uint16_t *end = (const void *) src + width; > @@ -74,7 +47,6 @@ static void glue(blizzard_draw_line16_, > DEPTH)(PIXEL_TYPE *dest, > data >>= 5; > COPY_PIXEL1(dest, glue(rgb_to_pixel, DEPTH)(r, g, b)); > } > -#endif > } > > static void glue(blizzard_draw_line24mode1_, DEPTH)(PIXEL_TYPE *dest, > -- > 2.5.0 > >
On 26 March 2016 at 05:57, Pooja Dhannawat <dhannawatpooja1@gmail.com> wrote: > Removing support for DEPTH != 32 from blizzard template header > and file that includes it, as macro DEPTH == 32 only used. > > Reviewed-by: Eric Blake <eblake@redhat.com> > Signed-off-by: Pooja Dhannawat <dhannawatpooja1@gmail.com> Hi. Apologies for not getting to this patch earlier -- I've been busy with a bunch of other work and the QEMU 2.6 release. In general this patch looks good, but unfortunately it doesn't compile: hw/display/blizzard.c: In function ‘s1d13745_init’: hw/display/blizzard.c:948:5: error: expected ‘;’ before ‘s’ s->line_fn_tab[0] = blizzard_draw_fn_32; ^ In file included from hw/display/blizzard.c:929:0: hw/display/blizzard.c: At top level: hw/display/blizzard_template.h:86:22: error: ‘blizzard_draw_fn_32’ defined but not used [-Werror=unused-variable] static blizzard_fn_t glue(blizzard_draw_fn_, DEPTH)[0x10] = { ^ cc1: all warnings being treated as errors > + assert(surface_bits_per_pixel(surface) == 32) This is because you're missing a semicolon here. It's important to at least compile and preferably also test any patch you send to the list. (Occasionally it's not possible if the code in question requires an obscure host OS or library combination or something, in which case you should note in the patch remarks below the "---" line that you weren't able to compile test.) In this case (and since it took me so long to get back to this patch) I've just fixed up this error and applied the result to target-arm.next. There are a couple of nice further cleanups we can do on top of this to remove blizzard_template.h altogether; I'll post those patches in a moment. thanks -- PMM
diff --git a/hw/display/blizzard.c b/hw/display/blizzard.c index c231960..5684e49 100644 --- a/hw/display/blizzard.c +++ b/hw/display/blizzard.c @@ -925,14 +925,6 @@ static void blizzard_update_display(void *opaque) s->my[1] = 0; } -#define DEPTH 8 -#include "blizzard_template.h" -#define DEPTH 15 -#include "blizzard_template.h" -#define DEPTH 16 -#include "blizzard_template.h" -#define DEPTH 24 -#include "blizzard_template.h" #define DEPTH 32 #include "blizzard_template.h" @@ -951,35 +943,10 @@ void *s1d13745_init(qemu_irq gpio_int) s->con = graphic_console_init(NULL, 0, &blizzard_ops, s); surface = qemu_console_surface(s->con); - switch (surface_bits_per_pixel(surface)) { - case 0: - s->line_fn_tab[0] = s->line_fn_tab[1] = - g_malloc0(sizeof(blizzard_fn_t) * 0x10); - break; - case 8: - s->line_fn_tab[0] = blizzard_draw_fn_8; - s->line_fn_tab[1] = blizzard_draw_fn_r_8; - break; - case 15: - s->line_fn_tab[0] = blizzard_draw_fn_15; - s->line_fn_tab[1] = blizzard_draw_fn_r_15; - break; - case 16: - s->line_fn_tab[0] = blizzard_draw_fn_16; - s->line_fn_tab[1] = blizzard_draw_fn_r_16; - break; - case 24: - s->line_fn_tab[0] = blizzard_draw_fn_24; - s->line_fn_tab[1] = blizzard_draw_fn_r_24; - break; - case 32: - s->line_fn_tab[0] = blizzard_draw_fn_32; - s->line_fn_tab[1] = blizzard_draw_fn_r_32; - break; - default: - fprintf(stderr, "%s: Bad color depth\n", __FUNCTION__); - exit(1); - } + assert(surface_bits_per_pixel(surface) == 32) + + s->line_fn_tab[0] = blizzard_draw_fn_32; + s->line_fn_tab[1] = blizzard_draw_fn_r_32; blizzard_reset(s); diff --git a/hw/display/blizzard_template.h b/hw/display/blizzard_template.h index b7ef27c..bc38d7a 100644 --- a/hw/display/blizzard_template.h +++ b/hw/display/blizzard_template.h @@ -19,31 +19,7 @@ */ #define SKIP_PIXEL(to) (to += deststep) -#if DEPTH == 8 -# define PIXEL_TYPE uint8_t -# define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while (0) -# define COPY_PIXEL1(to, from) (*to++ = from) -#elif DEPTH == 15 || DEPTH == 16 -# define PIXEL_TYPE uint16_t -# define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while (0) -# define COPY_PIXEL1(to, from) (*to++ = from) -#elif DEPTH == 24 -# define PIXEL_TYPE uint8_t -# define COPY_PIXEL(to, from) \ - do { \ - to[0] = from; \ - to[1] = (from) >> 8; \ - to[2] = (from) >> 16; \ - SKIP_PIXEL(to); \ - } while (0) - -# define COPY_PIXEL1(to, from) \ - do { \ - *to++ = from; \ - *to++ = (from) >> 8; \ - *to++ = (from) >> 16; \ - } while (0) -#elif DEPTH == 32 +#if DEPTH == 32 # define PIXEL_TYPE uint32_t # define COPY_PIXEL(to, from) do { *to = from; SKIP_PIXEL(to); } while (0) # define COPY_PIXEL1(to, from) (*to++ = from) @@ -58,9 +34,6 @@ static void glue(blizzard_draw_line16_, DEPTH)(PIXEL_TYPE *dest, const uint16_t *src, unsigned int width) { -#if !defined(SWAP_WORDS) && DEPTH == 16 - memcpy(dest, src, width); -#else uint16_t data; unsigned int r, g, b; const uint16_t *end = (const void *) src + width; @@ -74,7 +47,6 @@ static void glue(blizzard_draw_line16_, DEPTH)(PIXEL_TYPE *dest, data >>= 5; COPY_PIXEL1(dest, glue(rgb_to_pixel, DEPTH)(r, g, b)); } -#endif } static void glue(blizzard_draw_line24mode1_, DEPTH)(PIXEL_TYPE *dest,