On 2018-10-18 19:28, Mark Cave-Ayland wrote:
> From: Laurent Vivier <laur...@vivier.eu>
>
> Co-developed-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
> Signed-off-by: Laurent Vivier <laur...@vivier.eu>
> ---
> arch_init.c | 4 +
> hw/display/Makefile.objs | 1 +
> hw/display/macfb-template.h | 158 +++++++++++++++++++++++++++
> hw/display/macfb.c | 252
> ++++++++++++++++++++++++++++++++++++++++++++
> include/hw/display/macfb.h | 42 ++++++++
> qemu-options.hx | 2 +-
> vl.c | 3 +-
> 7 files changed, 460 insertions(+), 2 deletions(-)
> create mode 100644 hw/display/macfb-template.h
> create mode 100644 hw/display/macfb.c
> create mode 100644 include/hw/display/macfb.h
[...]
> +typedef void (*macfb_draw_line_func_t)(MacfbState *, uint8_t *, uint8_t *,
> int);
> +
> +static macfb_draw_line_func_t macfb_draw_line[24][32] = {
> + [0] = { [7] = draw_line1_8, [15] = draw_line1_16,
> + [23] = draw_line1_24, [31] = draw_line1_32 },
> + [1] = { [7] = draw_line2_8, [15] = draw_line2_16,
> + [23] = draw_line2_24, [31] = draw_line2_32 },
> + [3] = { [7] = draw_line4_8, [15] = draw_line4_16,
> + [23] = draw_line4_24, [31] = draw_line4_32 },
> + [7] = { [7] = draw_line8_8, [15] = draw_line8_16,
> + [23] = draw_line8_24, [31] = draw_line8_32 },
> + [15] = { [7] = draw_line16_8, [15] = draw_line16_16,
> + [23] = draw_line16_24, [31] = draw_line16_32 },
> + [23] = { [7] = draw_line24_8, [15] = draw_line24_16,
> + [23] = draw_line24_24, [31] = draw_line24_32 },
> +};
May I suggest to define the array as macfb_draw_line[24][4] instead of
macfb_draw_line[24][32] here...
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> new file mode 100644
> index 0000000000..54472c1cbb
> --- /dev/null
> +++ b/hw/display/macfb.c
> @@ -0,0 +1,252 @@
> +/*
> + * QEMU Motorola 680x0 Macintosh Video Card Emulation
> + * Copyright (c) 2012-2018 Laurent Vivier
> + *
> + * some parts from QEMU G364 framebuffer Emulator.
> + * Copyright (c) 2007-2011 Herve Poussineau
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/sysbus.h"
> +#include "ui/console.h"
> +#include "ui/pixel_ops.h"
> +#include "hw/display/macfb.h"
> +
> +#define VIDEO_BASE 0x00001000
> +#define DAFB_BASE 0x00800000
> +
> +#define MACFB_PAGE_SIZE 4096
> +#define MACFB_VRAM_SIZE (4 * 1024 * 1024)
> +
> +#define DAFB_RESET 0x200
> +#define DAFB_LUT 0x213
> +
> +#include "macfb-template.h"
> +
> +static int macfb_check_dirty(MacfbState *s, DirtyBitmapSnapshot *snap,
> + ram_addr_t addr, int len)
> +{
> + return memory_region_snapshot_get_dirty(&s->mem_vram, snap, addr, len);
> +}
> +
> +static void macfb_draw_graphic(MacfbState *s)
> +{
> + DisplaySurface *surface = qemu_console_surface(s->con);
> + DirtyBitmapSnapshot *snap = NULL;
> + ram_addr_t page;
> + int y, ymin;
> + int macfb_stride = (s->depth * s->width + 7) / 8;
> + macfb_draw_line_func_t draw_line;
> +
> + if (s->depth > 24) {
> + hw_error("macfb: unknown guest depth %d", s->depth);
I think this should be checked in the realize() function instead (and
maybe only do an assert() here).
> + return;
> + }
> + if (surface_bits_per_pixel(surface) > 32) {
> + hw_error("macfb: unknown host depth %d",
> + surface_bits_per_pixel(surface));
Simply assert(surface_bits_per_pixel(surface) <= 32) ?
> + return;
> + }
> + draw_line = macfb_draw_line[s->depth - 1][surface_bits_per_pixel(surface)
> + - 1];
... If you change func_t macfb_draw_line[24][32] to func_t
macfb_draw_line[24][4], you could then use
surface_bits_per_pixel(surface) / 8 - 1 here instead.
Also, do we have still to care about host bit depths < 32 at all these
days? If not, I think the code could be simplified quite a bit.
> + if (draw_line == NULL) {
> + hw_error("macfb: unknown guest/host depth combination %d/%d",
> s->depth,
> + surface_bits_per_pixel(surface));
hw_error() is meant for CPU errors only (it prints out a CPU register
dump), please don't use it in the framebuffer code.
> + return;
> + }
Thomas
