[PATCH 0/3] artist: minor fixes and performance improvement

[PATCH 0/3] artist: minor fixes and performance improvement

[Mark Cave-Ayland]

Patch 1 updates artist.c to fix some style issues and ensure that artist.c passes
checkpatch for the remainder of the patchset, whilst patch 2 removes the ROP8OFF()
macro which is currently unused.

Finally patch 3 is the main reason for this patchset and improves the performance
of the artist framebuffer by only rendering the dirty scanlines to the display
surface, which noticeably improves boot times and makes the GTK UI usable on my
fairly modest laptop.

[Helge/Sven: if you can provide a Tested-by or Reviewed-by tag for this patchset
before the weekend, I can include it in my PR containing all the other HPPA
updates]

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


Mark Cave-Ayland (3):
  artist: checkpatch and newline style fixes
  artist: remove unused ROP8OFF() macro
  artist: only render dirty scanlines on the display surface

 hw/display/artist.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

-- 
2.20.1

[PATCH 1/3] artist: checkpatch and newline style fixes

[Mark Cave-Ayland]

Ensure that subsequent patches do not cause checkpatch to fail and also tidy up
extra/missing newlines.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/display/artist.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/hw/display/artist.c b/hw/display/artist.c
index 8e121bb0b4..be92113494 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -211,8 +211,9 @@ static void artist_invalidate_lines(struct vram_buffer *buf,
     int start = starty * buf->width;
     int size;
 
-    if (starty + height > buf->height)
+    if (starty + height > buf->height) {
         height = buf->height - starty;
+    }
 
     size = height * buf->width;
 
@@ -321,8 +322,9 @@ static void artist_get_cursor_pos(ARTISTState *s, int *x, int *y)
     }
 
     lx = artist_get_x(s->cursor_pos);
-    if (lx < offset)
+    if (lx < offset) {
         offset = lx;
+    }
     *x = (lx - offset) / 2;
 
     *y = 1146 - artist_get_y(s->cursor_pos);
@@ -343,6 +345,7 @@ static void artist_get_cursor_pos(ARTISTState *s, int *x, int *y)
 static void artist_invalidate_cursor(ARTISTState *s)
 {
     int x, y;
+
     artist_get_cursor_pos(s, &x, &y);
     artist_invalidate_lines(&s->vram_buffer[ARTIST_BUFFER_AP],
                             y, s->cursor_height);
@@ -470,10 +473,9 @@ static void draw_line(ARTISTState *s,
 
     if ((x1 >= buf->width && x2 >= buf->width) ||
         (y1 >= buf->height && y2 >= buf->height)) {
-	return;
+        return;
     }
 
-
     if (update_start) {
         s->vram_start = (x2 << 16) | y2;
     }
@@ -553,15 +555,15 @@ static void draw_line(ARTISTState *s,
         x++;
     } while (x <= x2 && (max_pix == -1 || --max_pix > 0));
 
-    if (c1)
+    if (c1) {
         artist_invalidate_lines(buf, x1, x2 - x1);
-    else
+    } else {
         artist_invalidate_lines(buf, y1 > y2 ? y2 : y1, x2 - x1);
+    }
 }
 
 static void draw_line_pattern_start(ARTISTState *s)
 {
-
     int startx = artist_get_x(s->vram_start);
     int starty = artist_get_y(s->vram_start);
     int endx = artist_get_x(s->blockmove_size);
@@ -574,7 +576,6 @@ static void draw_line_pattern_start(ARTISTState *s)
 
 static void draw_line_pattern_next(ARTISTState *s)
 {
-
     int startx = artist_get_x(s->vram_start);
     int starty = artist_get_y(s->vram_start);
     int endx = artist_get_x(s->blockmove_size);
@@ -589,7 +590,6 @@ static void draw_line_pattern_next(ARTISTState *s)
 
 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);
@@ -600,7 +600,6 @@ static void draw_line_size(ARTISTState *s, bool update_start)
 
 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);
@@ -650,7 +649,6 @@ static void draw_line_xy(ARTISTState *s, bool update_start)
 
 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);
@@ -835,6 +833,7 @@ static void artist_vram_write(void *opaque, hwaddr addr, uint64_t val,
                               unsigned size)
 {
     ARTISTState *s = opaque;
+
     s->vram_char_y = 0;
     trace_artist_vram_write(size, addr, val);
     vram_bit_write(opaque, addr, 0, val, size);
@@ -1244,7 +1243,6 @@ static void artist_update_display(void *opaque)
     DisplaySurface *surface = qemu_console_surface(s->con);
     int first = 0, last;
 
-
     framebuffer_update_display(surface, &s->fbsection, s->width, s->height,
                                s->width, s->width * 4, 0, 0, artist_draw_line,
                                s, &first, &last);
@@ -1258,6 +1256,7 @@ static void artist_invalidate(void *opaque)
 {
     ARTISTState *s = ARTIST(opaque);
     struct vram_buffer *buf = &s->vram_buffer[ARTIST_BUFFER_AP];
+
     memory_region_set_dirty(&buf->mr, 0, buf->size);
 }
 
-- 
2.20.1

[PATCH 2/3] artist: remove unused ROP8OFF() macro

[Mark Cave-Ayland]

This macro is unused and so can simply be removed.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/display/artist.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/hw/display/artist.c b/hw/display/artist.c
index be92113494..4a9f7b5e59 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -25,12 +25,6 @@
 #define TYPE_ARTIST "artist"
 OBJECT_DECLARE_SIMPLE_TYPE(ARTISTState, ARTIST)
 
-#if HOST_BIG_ENDIAN
-#define ROP8OFF(_i) (3 - (_i))
-#else
-#define ROP8OFF
-#endif
-
 struct vram_buffer {
     MemoryRegion mr;
     uint8_t *data;
-- 
2.20.1

[PATCH 3/3] artist: only render dirty scanlines on the display surface

[Mark Cave-Ayland]

The framebuffer_update_display() function returns the dirty scanlines that were
touched since the last display update, however artist_update_display() always calls
dpy_gfx_update() with start and end scanlines of 0 and s->height causing the
entire display surface to be rendered on every update.

Update artist_update_display() so that dpy_gfx_update() only renders the dirty
scanlines on the display surface, bypassing the display surface rendering
completely if framebuffer_update_display() indicates no changes occurred.

This noticeably improves boot performance when the framebuffer is enabled on my
rather modest laptop here, including making the GTK UI usable.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/display/artist.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/display/artist.c b/hw/display/artist.c
index 4a9f7b5e59..39fc0c4ca5 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -1243,7 +1243,9 @@ static void artist_update_display(void *opaque)
 
     artist_draw_cursor(s);
 
-    dpy_gfx_update(s->con, 0, 0, s->width, s->height);
+    if (first >= 0) {
+        dpy_gfx_update(s->con, 0, first, s->width, last - first + 1);
+    }
 }
 
 static void artist_invalidate(void *opaque)
-- 
2.20.1

Re: [PATCH 2/3] artist: remove unused ROP8OFF() macro

[Peter Maydell]

On Wed, 4 May 2022 at 16:41, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> This macro is unused and so can simply be removed.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 1/3] artist: checkpatch and newline style fixes

[Peter Maydell]

On Wed, 4 May 2022 at 16:39, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> Ensure that subsequent patches do not cause checkpatch to fail and also tidy up
> extra/missing newlines.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 0/3] artist: minor fixes and performance improvement

[Helge Deller]

On 5/4/22 17:37, Mark Cave-Ayland wrote:
> Patch 1 updates artist.c to fix some style issues and ensure that artist.c passes
> checkpatch for the remainder of the patchset, whilst patch 2 removes the ROP8OFF()
> macro which is currently unused.
>
> Finally patch 3 is the main reason for this patchset and improves the performance
> of the artist framebuffer by only rendering the dirty scanlines to the display
> surface, which noticeably improves boot times and makes the GTK UI usable on my
> fairly modest laptop.
>
> [Helge/Sven: if you can provide a Tested-by or Reviewed-by tag for this patchset
> before the weekend, I can include it in my PR containing all the other HPPA
> updates]
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

I tested it and indeed, the GTK GUI is now really much faster, even with the
GTK window forwarded over X11.

Ideally, can the patch #3 (the GTK speedup) be backported?

I applied, reviewed and tested it. Please add:

Reviewed-by: Helge Deller <deller@gmx.de>

Thanks!
Helge


>
> Mark Cave-Ayland (3):
>   artist: checkpatch and newline style fixes
>   artist: remove unused ROP8OFF() macro
>   artist: only render dirty scanlines on the display surface
>
>  hw/display/artist.c | 33 ++++++++++++++-------------------
>  1 file changed, 14 insertions(+), 19 deletions(-)
>

Re: [PATCH 0/3] artist: minor fixes and performance improvement

[Sven Schnelle]

Hi Mark,

Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:

> Patch 1 updates artist.c to fix some style issues and ensure that artist.c passes
> checkpatch for the remainder of the patchset, whilst patch 2 removes the ROP8OFF()
> macro which is currently unused.
>
> Finally patch 3 is the main reason for this patchset and improves the performance
> of the artist framebuffer by only rendering the dirty scanlines to the display
> surface, which noticeably improves boot times and makes the GTK UI usable on my
> fairly modest laptop.
>
> [Helge/Sven: if you can provide a Tested-by or Reviewed-by tag for this patchset
> before the weekend, I can include it in my PR containing all the other HPPA
> updates]
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>
>
> Mark Cave-Ayland (3):
>   artist: checkpatch and newline style fixes
>   artist: remove unused ROP8OFF() macro
>   artist: only render dirty scanlines on the display surface
>
>  hw/display/artist.c | 33 ++++++++++++++-------------------
>  1 file changed, 14 insertions(+), 19 deletions(-)

I reviewed and tested the patches with HP-UX 10.20 and HP-UX 11.11. Feel
free to add my

Reviewed-by: Sven Schnelle <svens@stackframe.org>

Re: [PATCH 0/3] artist: minor fixes and performance improvement

[Mark Cave-Ayland]

On 05/05/2022 13:26, Helge Deller wrote:

> On 5/4/22 17:37, Mark Cave-Ayland wrote:
>> Patch 1 updates artist.c to fix some style issues and ensure that artist.c passes
>> checkpatch for the remainder of the patchset, whilst patch 2 removes the ROP8OFF()
>> macro which is currently unused.
>>
>> Finally patch 3 is the main reason for this patchset and improves the performance
>> of the artist framebuffer by only rendering the dirty scanlines to the display
>> surface, which noticeably improves boot times and makes the GTK UI usable on my
>> fairly modest laptop.
>>
>> [Helge/Sven: if you can provide a Tested-by or Reviewed-by tag for this patchset
>> before the weekend, I can include it in my PR containing all the other HPPA
>> updates]
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> 
> I tested it and indeed, the GTK GUI is now really much faster, even with the
> GTK window forwarded over X11.

Excellent!

> Ideally, can the patch #3 (the GTK speedup) be backported?

I'm not sure that it would really make a difference in this case: if this were queued 
for 7.0 then that wouldn't be released until a few weeks after 7.1, and even then its 
likely that the stable distros would be using something older regardless so it 
wouldn't get picked up anyhow.

> I applied, reviewed and tested it. Please add:
> 
> Reviewed-by: Helge Deller <deller@gmx.de>

Thanks - I'll add your R-B to the artist series.


ATB,

Mark.