Re: [PATCH] hw/display/tcx: Avoid clearing dirty bitmap in DeviceReset()

[Mark Cave-Ayland]

On 22/01/2022 00:07, Philippe Mathieu-Daudé via wrote:

> Commit 2dd285b5f3 ("tcx: make display updates thread safe")
> converted this model to use the DirtyBitmapSnapshot API,
> resetting the dirty bitmap in tcx_update_display(). There
> is no need to do it again in the DeviceReset handler.
>
> See more details in commit fec5e8c92b ("vga: make display
> updates thread safe.").
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   hw/display/tcx.c | 2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index d4d09d0df8..22b0ae4761 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -371,8 +371,6 @@ static void tcx_reset(DeviceState *d)
>       s->r[258] = s->g[258] = s->b[258] = 255;
>       update_palette_entries(s, 0, 260);
>       memset(s->vram, 0, MAXX*MAXY);
> -    memory_region_reset_dirty(&s->vram_mem, 0, MAXX * MAXY * (1 + 4 + 4),
> -                              DIRTY_MEMORY_VGA);
>       s->dac_index = 0;
>       s->dac_state = 0;
>       s->cursx = 0xf000; /* Put cursor off screen */

I think the issue here is that tcx_reset() should be marking the entire VRAM dirty 
due to the memset() i.e. we should be setting the dirty bitmap rather than resetting 
it. Perhaps memory_region_reset_dirty() should be replaced with 
tcx_invalidate_display() instead?


ATB,

Mark.