Re: [PATCH v5 5/6] hppa: Add emulation of Artist graphics

2020-02-12 Thread Philippe Mathieu-Daudé

Hi Sven,

On 12/20/19 10:15 PM, Sven Schnelle wrote:

This adds emulation of Artist graphics good enough
to get a Text console on both Linux and HP-UX. The
X11 server from HP-UX also works.

Signed-off-by: Sven Schnelle 
---
  hw/display/Kconfig   |4 +
  hw/display/Makefile.objs |1 +
  hw/display/artist.c  | 1450 ++
  hw/display/trace-events  |9 +
  hw/hppa/Kconfig  |1 +
  hw/hppa/hppa_hardware.h  |1 +
  hw/hppa/machine.c|9 +
  7 files changed, 1475 insertions(+)
  create mode 100644 hw/display/artist.c

diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index c500d1fc6d..15d59e10dc 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -91,6 +91,10 @@ config TCX
  config CG3
  bool
  
+config ARTIST

+bool
+select FRAMEBUFFER
+
  config VGA
  bool
  
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs

index f2182e3bef..5f03dfdcc4 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -40,6 +40,7 @@ common-obj-$(CONFIG_SM501) += sm501.o
  common-obj-$(CONFIG_TCX) += tcx.o
  common-obj-$(CONFIG_CG3) += cg3.o
  common-obj-$(CONFIG_NEXTCUBE) += next-fb.o
+common-obj-$(CONFIG_ARTIST) += artist.o
  
  obj-$(CONFIG_VGA) += vga.o
  
diff --git a/hw/display/artist.c b/hw/display/artist.c

[...]

+static void draw_line_size(ARTISTState *s, bool update_start)
+{
+
+int startx = artist_get_x(s->vram_start);
+int starty = artist_get_y(s->vram_start);
+int endx = artist_get_x(s->line_size);
+int endy = artist_get_y(s->line_size);
+
+trace_artist_draw_line(startx, starty, endx, endy);
+draw_line(s, startx, starty, endx, endy, update_start, -1, -1);
+}
+
+static void draw_line_xy(ARTISTState *s, bool update_start)
+{
+
+int startx = artist_get_x(s->vram_start);
+int starty = artist_get_y(s->vram_start);
+int sizex = artist_get_x(s->blockmove_size);
+int sizey = artist_get_y(s->blockmove_size);
+int linexy = s->line_xy >> 16;
+int endx, endy;
+
+endx = startx;
+endy = starty;
+
+if (sizex > 0) {
+endx = startx + linexy;
+}
+
+if (sizex < 0) {
+endx = startx;
+startx -= linexy;
+}
+
+if (sizey > 0) {
+endy = starty + linexy;
+}
+
+if (sizey < 0) {
+endy = starty;
+starty -= linexy;
+}
+
+if (startx < 0) {
+startx = 0;
+}
+
+if (endx < 0) {
+endx = 0;


If negative, set to zero.


+}
+
+if (starty < 0) {
+starty = 0;
+}
+
+if (endy < 0) {
+endy = 0;


Ditto.


+}
+
+
+if (endx < 0) {
+return;


Here Coverity points:

>>> CID 1419388:  Control flow issues  (DEADCODE)
>>> Execution cannot reach this statement: "return;".


+}
+
+if (endy < 0) {
+return;


Here again.


+}
+
+trace_artist_draw_line(startx, starty, endx, endy);
+draw_line(s, startx, starty, endx, endy, false, -1, -1);
+}
+
+static void draw_line_end(ARTISTState *s, bool update_start)
+{
+
+int startx = artist_get_x(s->vram_start);
+int starty = artist_get_y(s->vram_start);
+int endx = artist_get_x(s->line_end);
+int endy = artist_get_y(s->line_end);
+
+trace_artist_draw_line(startx, starty, endx, endy);
+draw_line(s, startx, starty, endx, endy, update_start, -1, -1);
+}





Re: [PATCH v5 5/6] hppa: Add emulation of Artist graphics

2019-12-29 Thread Philippe Mathieu-Daudé
On Fri, Dec 27, 2019 at 9:58 PM Helge Deller  wrote:
> On 24.12.19 01:18, Philippe Mathieu-Daudé wrote:
> > On 12/23/19 6:50 PM, Sven Schnelle wrote:
> >> Hi Philippe,
> >>
> >> On Sun, Dec 22, 2019 at 01:37:48PM +0100, Philippe Mathieu-Daudé wrote:
>    +if (vga_interface_type != VGA_NONE) {
>  +dev = qdev_create(NULL, "artist");
>  +qdev_init_nofail(dev);
>  +s = SYS_BUS_DEVICE(dev);
>  +sysbus_mmio_map(s, 0, LASI_GFX_HPA);
>  +sysbus_mmio_map(s, 1, ARTIST_FB_ADDR);
> >>>
> >>> How is this chipset connected on the board?
> >>> If it is a card you can plug on a bus, you can use a condition.
> >>> If it is soldered or part of another chipset, then it has to be mapped
> >>> unconditionally.
> >>
> >> Depends on the Model. Hp 9000 712 and 715 had it onboard, for the B160L
> >> we're emulating and others it was a GSC add-on card.
> >
> > The B160L case is unclear, do you mean this is not the chipset on the 
> > machine, but the software is happy if another chipset is available?
> >
> > Looking at hw/hppa/ I only see one machine:
> >
> >   static void machine_hppa_machine_init(MachineClass *mc)
> >   {
> >   mc->desc = "HPPA generic machine";
> >   ...
> >   }
> >   DEFINE_MACHINE("hppa", machine_hppa_machine_init)
> >
> > Are you saying this generic machine is able to run different physical hw? 
> > Why not add them? This shouldn't take long and it would be clearer, what do 
> > you think?
> >
> > Adding different machines here in QEMU mostly mean add a class which 
> > declare the different properties used by each machine. Igor Mammedov 
> > recently suggested to follow the example of aspeed_machine_types[] in 
> > hw/arm/aspeed.c.
>
> Yes, we plan to add specific machines like 712 (or 715), and maybe a
> C3000 or B2000 over time, as needed device emulations (e.g. tulip, artist)
> gets accepted.
> But for that it would be very beneficial if changes (like the Artist emulation
> here in this thread) would be accepted faster upstream

IMHO the HPPA patches are merged quicker than various other subsystems...
>From the cover, this series contains "3311 insertions". If you do the
ratio lines per patch / time for a patch to get accepted, you are not
that bad ;)

To be constructive, how do you think we can improve?
Looking at the git history, except global refactors, I see only 3
contributors, Richard (who merges your patches), you and Sven.
2 users so far, + Richard as tester.



Re: [PATCH v5 5/6] hppa: Add emulation of Artist graphics

2019-12-27 Thread Helge Deller
On 24.12.19 01:18, Philippe Mathieu-Daudé wrote:
> On 12/23/19 6:50 PM, Sven Schnelle wrote:
>> Hi Philippe,
>>
>> On Sun, Dec 22, 2019 at 01:37:48PM +0100, Philippe Mathieu-Daudé wrote:
   +    if (vga_interface_type != VGA_NONE) {
 +    dev = qdev_create(NULL, "artist");
 +    qdev_init_nofail(dev);
 +    s = SYS_BUS_DEVICE(dev);
 +    sysbus_mmio_map(s, 0, LASI_GFX_HPA);
 +    sysbus_mmio_map(s, 1, ARTIST_FB_ADDR);
>>>
>>> How is this chipset connected on the board?
>>> If it is a card you can plug on a bus, you can use a condition.
>>> If it is soldered or part of another chipset, then it has to be mapped
>>> unconditionally.
>>
>> Depends on the Model. Hp 9000 712 and 715 had it onboard, for the B160L
>> we're emulating and others it was a GSC add-on card.
>
> The B160L case is unclear, do you mean this is not the chipset on the 
> machine, but the software is happy if another chipset is available?
>
> Looking at hw/hppa/ I only see one machine:
>
>   static void machine_hppa_machine_init(MachineClass *mc)
>   {
>   mc->desc = "HPPA generic machine";
>   ...
>   }
>   DEFINE_MACHINE("hppa", machine_hppa_machine_init)
>
> Are you saying this generic machine is able to run different physical hw? Why 
> not add them? This shouldn't take long and it would be clearer, what do you 
> think?
>
> Adding different machines here in QEMU mostly mean add a class which declare 
> the different properties used by each machine. Igor Mammedov recently 
> suggested to follow the example of aspeed_machine_types[] in hw/arm/aspeed.c.

Yes, we plan to add specific machines like 712 (or 715), and maybe a
C3000 or B2000 over time, as needed device emulations (e.g. tulip, artist)
gets accepted.
But for that it would be very beneficial if changes (like the Artist emulation
here in this thread) would be accepted faster upstream

Helge



Re: [PATCH v5 5/6] hppa: Add emulation of Artist graphics

2019-12-23 Thread Philippe Mathieu-Daudé

On 12/23/19 6:50 PM, Sven Schnelle wrote:

Hi Philippe,

On Sun, Dec 22, 2019 at 01:37:48PM +0100, Philippe Mathieu-Daudé wrote:
  
+if (vga_interface_type != VGA_NONE) {

+dev = qdev_create(NULL, "artist");
+qdev_init_nofail(dev);
+s = SYS_BUS_DEVICE(dev);
+sysbus_mmio_map(s, 0, LASI_GFX_HPA);
+sysbus_mmio_map(s, 1, ARTIST_FB_ADDR);


How is this chipset connected on the board?
If it is a card you can plug on a bus, you can use a condition.
If it is soldered or part of another chipset, then it has to be mapped
unconditionally.


Depends on the Model. Hp 9000 712 and 715 had it onboard, for the B160L
we're emulating and others it was a GSC add-on card.


The B160L case is unclear, do you mean this is not the chipset on the 
machine, but the software is happy if another chipset is available?


Looking at hw/hppa/ I only see one machine:

  static void machine_hppa_machine_init(MachineClass *mc)
  {
  mc->desc = "HPPA generic machine";
  ...
  }
  DEFINE_MACHINE("hppa", machine_hppa_machine_init)

Are you saying this generic machine is able to run different physical 
hw? Why not add them? This shouldn't take long and it would be clearer, 
what do you think?


Adding different machines here in QEMU mostly mean add a class which 
declare the different properties used by each machine. Igor Mammedov 
recently suggested to follow the example of aspeed_machine_types[] in 
hw/arm/aspeed.c.





Re: [PATCH v5 5/6] hppa: Add emulation of Artist graphics

2019-12-23 Thread Sven Schnelle
Hi Philippe,

On Sun, Dec 22, 2019 at 01:37:48PM +0100, Philippe Mathieu-Daudé wrote:
> >  
> > +if (vga_interface_type != VGA_NONE) {
> > +dev = qdev_create(NULL, "artist");
> > +qdev_init_nofail(dev);
> > +s = SYS_BUS_DEVICE(dev);
> > +sysbus_mmio_map(s, 0, LASI_GFX_HPA);
> > +sysbus_mmio_map(s, 1, ARTIST_FB_ADDR);
> 
> How is this chipset connected on the board?
> If it is a card you can plug on a bus, you can use a condition.
> If it is soldered or part of another chipset, then it has to be mapped
> unconditionally.

Depends on the Model. Hp 9000 712 and 715 had it onboard, for the B160L
we're emulating and others it was a GSC add-on card.

Regards
Sven



Re: [PATCH v5 5/6] hppa: Add emulation of Artist graphics

2019-12-22 Thread Philippe Mathieu-Daudé
On 12/20/19 10:15 PM, Sven Schnelle wrote:
> This adds emulation of Artist graphics good enough
> to get a Text console on both Linux and HP-UX. The
> X11 server from HP-UX also works.
> 
> Signed-off-by: Sven Schnelle 
> ---
>  hw/display/Kconfig   |4 +
>  hw/display/Makefile.objs |1 +
>  hw/display/artist.c  | 1450 ++
>  hw/display/trace-events  |9 +
>  hw/hppa/Kconfig  |1 +
>  hw/hppa/hppa_hardware.h  |1 +
>  hw/hppa/machine.c|9 +
>  7 files changed, 1475 insertions(+)
>  create mode 100644 hw/display/artist.c
> 
[...]
> diff --git a/hw/hppa/hppa_hardware.h b/hw/hppa/hppa_hardware.h
> index 507f91e05d..4a2fe2df60 100644
> --- a/hw/hppa/hppa_hardware.h
> +++ b/hw/hppa/hppa_hardware.h
> @@ -22,6 +22,7 @@
>  #define LASI_PS2KBD_HPA 0xffd08000
>  #define LASI_PS2MOU_HPA 0xffd08100
>  #define LASI_GFX_HPA0xf800
> +#define ARTIST_FB_ADDR  0xf900
>  #define CPU_HPA 0xfffb
>  #define MEMORY_HPA  0xfffbf000
>  
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index 33e3769d0b..6c67399054 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -75,6 +75,7 @@ static void machine_hppa_init(MachineState *machine)
>  MemoryRegion *cpu_region;
>  long i;
>  unsigned int smp_cpus = machine->smp.cpus;
> +SysBusDevice *s;
>  
>  ram_size = machine->ram_size;
>  
> @@ -127,6 +128,14 @@ static void machine_hppa_init(MachineState *machine)
>  dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a"));
>  lsi53c8xx_handle_legacy_cmdline(dev);
>  
> +if (vga_interface_type != VGA_NONE) {
> +dev = qdev_create(NULL, "artist");
> +qdev_init_nofail(dev);
> +s = SYS_BUS_DEVICE(dev);
> +sysbus_mmio_map(s, 0, LASI_GFX_HPA);
> +sysbus_mmio_map(s, 1, ARTIST_FB_ADDR);

How is this chipset connected on the board?
If it is a card you can plug on a bus, you can use a condition.
If it is soldered or part of another chipset, then it has to be mapped
unconditionally.
IOW I think you should drop the 'if (vga_interface_type != VGA_NONE)' check.

> +}
> +
>  /* Network setup.  e1000 is good enough, failing Tulip support.  */
>  for (i = 0; i < nb_nics; i++) {
>  if (!enable_lasi_lan()) {
> 



[PATCH v5 5/6] hppa: Add emulation of Artist graphics

2019-12-20 Thread Sven Schnelle
This adds emulation of Artist graphics good enough
to get a Text console on both Linux and HP-UX. The
X11 server from HP-UX also works.

Signed-off-by: Sven Schnelle 
---
 hw/display/Kconfig   |4 +
 hw/display/Makefile.objs |1 +
 hw/display/artist.c  | 1450 ++
 hw/display/trace-events  |9 +
 hw/hppa/Kconfig  |1 +
 hw/hppa/hppa_hardware.h  |1 +
 hw/hppa/machine.c|9 +
 7 files changed, 1475 insertions(+)
 create mode 100644 hw/display/artist.c

diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index c500d1fc6d..15d59e10dc 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -91,6 +91,10 @@ config TCX
 config CG3
 bool
 
+config ARTIST
+bool
+select FRAMEBUFFER
+
 config VGA
 bool
 
diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs
index f2182e3bef..5f03dfdcc4 100644
--- a/hw/display/Makefile.objs
+++ b/hw/display/Makefile.objs
@@ -40,6 +40,7 @@ common-obj-$(CONFIG_SM501) += sm501.o
 common-obj-$(CONFIG_TCX) += tcx.o
 common-obj-$(CONFIG_CG3) += cg3.o
 common-obj-$(CONFIG_NEXTCUBE) += next-fb.o
+common-obj-$(CONFIG_ARTIST) += artist.o
 
 obj-$(CONFIG_VGA) += vga.o
 
diff --git a/hw/display/artist.c b/hw/display/artist.c
new file mode 100644
index 00..0885b7b988
--- /dev/null
+++ b/hw/display/artist.c
@@ -0,0 +1,1450 @@
+/*
+ * QEMU HP Artist Emulation
+ *
+ * Copyright (c) 2019 Sven Schnelle 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qemu/error-report.h"
+#include "qemu/typedefs.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/units.h"
+#include "qapi/error.h"
+#include "hw/sysbus.h"
+#include "hw/loader.h"
+#include "hw/qdev-core.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "ui/console.h"
+#include "trace.h"
+#include "hw/display/framebuffer.h"
+
+#define TYPE_ARTIST "artist"
+#define ARTIST(obj) OBJECT_CHECK(ARTISTState, (obj), TYPE_ARTIST)
+
+#ifdef HOST_WORDS_BIGENDIAN
+#define ROP8OFF(_i) (3 - (_i))
+#else
+#define ROP8OFF
+#endif
+
+struct vram_buffer {
+MemoryRegion mr;
+uint8_t *data;
+int size;
+int width;
+int height;
+};
+
+typedef struct ARTISTState {
+SysBusDevice parent_obj;
+
+QemuConsole *con;
+MemoryRegion vram_mem;
+MemoryRegion mem_as_root;
+MemoryRegion reg;
+MemoryRegionSection fbsection;
+
+void *vram_int_mr;
+AddressSpace as;
+
+struct vram_buffer vram_buffer[16];
+
+uint16_t width;
+uint16_t height;
+uint16_t depth;
+
+uint32_t fg_color;
+uint32_t bg_color;
+
+uint32_t vram_char_y;
+uint32_t vram_bitmask;
+
+uint32_t vram_start;
+uint32_t vram_pos;
+
+uint32_t vram_size;
+
+uint32_t blockmove_source;
+uint32_t blockmove_dest;
+uint32_t blockmove_size;
+
+uint32_t line_size;
+uint32_t line_end;
+uint32_t line_xy;
+uint32_t line_pattern_start;
+uint32_t line_pattern_skip;
+
+uint32_t cursor_pos;
+
+uint32_t cursor_height;
+uint32_t cursor_width;
+
+uint32_t plane_mask;
+
+uint32_t reg_100080;
+uint32_t reg_300200;
+uint32_t reg_300208;
+uint32_t reg_300218;
+
+uint32_t cmap_bm_access;
+uint32_t dst_bm_access;
+uint32_t src_bm_access;
+uint32_t control_plane;
+uint32_t transfer_data;
+uint32_t image_bitmap_op;
+
+uint32_t font_write1;
+uint32_t font_write2;
+uint32_t font_write_pos_y;
+
+int draw_line_pattern;
+} ARTISTState;
+
+typedef enum {
+ARTIST_BUFFER_AP = 1,
+ARTIST_BUFFER_OVERLAY = 2,
+ARTIST_BUFFER_CURSOR1 = 6,
+ARTIST_BUFFER_CURSOR2 = 7,
+ARTIST_BUFFER_ATTRIBUTE = 13,
+ARTIST_BUFFER_CMAP = 15,
+} artist_buffer_t;
+
+typedef enum {
+VRAM_IDX = 0x1004a0,
+VRAM_BITMASK = 0x1005a0,
+VRAM_WRITE_INCR_X = 0x100600,
+VRAM_WRITE_INCR_X2 = 0x100604,
+VRAM_WRITE_INCR_Y = 0x100620,
+VRAM_START = 0x100800,
+BLOCK_MOVE_SIZE = 0x100804,
+BLOCK_MOVE_SOURCE = 0x100808,
+TRANSFER_DATA = 0x100820,
+FONT_WRITE_INCR_Y = 0x1008a0,
+VRAM_START_TRIGGER = 0x100a00,
+VRAM_SIZE_TRIGGER = 0x100a04,
+FONT_WRITE_START = 0x100aa0,
+BLOCK_MOVE_DEST_TRIGGER = 0x100b00,
+BLOCK_MOVE_SIZE_TRIGGER = 0x100b04,
+LINE_XY = 0x100ccc,
+PATTERN_LINE_START = 0x100ecc,
+LINE_SIZE = 0x100e04,
+LINE_END = 0x100e44,
+CMAP_BM_ACCESS = 0x118000,
+DST_BM_ACCESS = 0x118004,
+SRC_BM_ACCESS = 0x118008,
+CONTROL_PLANE = 0x11800c,
+FG_COLOR = 0x118010,
+BG_COLOR = 0x118014,
+PLANE_MASK = 0x118018,
+IMAGE_BITMAP_OP = 0x11801c,
+CURSOR_POS = 0x300100,
+CURSOR_CTRL = 0x300104,
+} artist_reg_t;
+
+typedef enum {
+ARTIST_ROP_CLEAR = 0,
+ARTIST_ROP_COPY = 3,
+ARTIST_ROP_XOR = 6,
+ARTIST_ROP_NOT_DST = 10,
+ARTIST_ROP_SET = 15,
+} artist_rop_t;
+
+#define REG_NAME(_x) case _x: return " "#_x;