On 2017-04-21 12:31, BALATON Zoltan wrote: > Rework HWC handling to simplify it and fix cursor not updating on > screen as needed. Previously cursor was not updated because checking > for changes in a line overrode the update flag set for the cursor but > fixing this is not enough because the cursor should also be updated if > its shape or location changes. Introduce hwc_invalidate() function to > handle that similar to other display controller models. > > Signed-off-by: BALATON Zoltan <bala...@eik.bme.hu> > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > --- > > v3: simplify return expression in get_bpp > > hw/display/sm501.c | 169 > +++++++++++++++++++++++++------------------- > hw/display/sm501_template.h | 25 +++---- > 2 files changed, 107 insertions(+), 87 deletions(-) > > diff --git a/hw/display/sm501.c b/hw/display/sm501.c > index a628ef1..dc806a3 100644 > --- a/hw/display/sm501.c > +++ b/hw/display/sm501.c > @@ -1339,14 +1362,16 @@ static void sm501_draw_crt(SM501State *s) > /* draw each line according to conditions */ > memory_region_sync_dirty_bitmap(&s->local_mem_region); > for (y = 0; y < height; y++) { > - int update_hwc = draw_hwc_line ? within_hwc_y_range(s, y, 1) : 0; > - int update = full_update || update_hwc; > + int update, update_hwc; > ram_addr_t page0 = offset; > ram_addr_t page1 = offset + width * src_bpp - 1; > > + /* check if hardware cursor is enabled and we're within its range */ > + update_hwc = draw_hwc_line && c_y <= y && y < c_y + SM501_HWC_HEIGHT; > + update = full_update || update_hwc; > /* check dirty flags for each line */ > - update = memory_region_get_dirty(&s->local_mem_region, page0, > - page1 - page0, DIRTY_MEMORY_VGA); > + update |= memory_region_get_dirty(&s->local_mem_region, page0, > + page1 - page0, DIRTY_MEMORY_VGA); > > /* draw line and change status */ > if (update) { > @@ -1358,7 +1383,7 @@ static void sm501_draw_crt(SM501State *s) > > /* draw hardware cursor */ > if (update_hwc) { > - draw_hwc_line(s, 1, hwc_palette, y - get_hwc_y(s, 1), d, > width); > + draw_hwc_line(d, hwc_src, width, hwc_palette, c_x, y - c_y); > } > > if (y_start < 0) {
The above change causes a warning with at least GCC 6 and GCC 7, which causes a build failure when using -Werror: | CC sh4-softmmu/hw/display/sm501.o | /home/aurel32/git/qemu/hw/display/sm501.c: In function ‘sm501_update_display’: | /home/aurel32/git/qemu/hw/display/sm501.c:1504:17: warning: ‘hwc_src’ may be used uninitialized in this function [-Wmaybe-uninitialized] | draw_hwc_line(d, hwc_src, width, hwc_palette, c_x, y - c_y); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | /home/aurel32/git/qemu/hw/display/sm501.c:1488:59: warning: ‘c_y’ may be used uninitialized in this function [-Wmaybe-uninitialized] | update_hwc = draw_hwc_line && c_y <= y && y < c_y + SM501_HWC_HEIGHT; | ^ | /home/aurel32/git/qemu/hw/display/sm501.c:1504:17: warning: ‘c_x’ may be used uninitialized in this function [-Wmaybe-uninitialized] | draw_hwc_line(d, hwc_src, width, hwc_palette, c_x, y - c_y); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ GCC fails to notice that hwc_src, c_x and c_y are always defined if draw_hwc_line is not NULL. This is obviously not a bug in the code, but it might be a good idea to put a workaround for example by defining default values for those variables. Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net