Re: [PATCH 1/2] g364fb: use RAM memory region for framebuffer

[Mark Cave-Ayland]

On 25/06/2021 09:37, Philippe Mathieu-Daudé wrote:

> On 6/25/21 9:38 AM, Mark Cave-Ayland wrote:
> > Since the migration stream is already broken, we can use this opportunity to
> > change the framebuffer so that it is migrated as a RAM memory region rather
> > than as an array of bytes.
> >
> > In particular this helps the output of the analyze-migration.py tool which
> > no longer contains a huge array representing the framebuffer contents.
> >
> > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> > ---
> >   hw/display/g364fb.c | 15 +++++++--------
> >   1 file changed, 7 insertions(+), 8 deletions(-)
>
> > @@ -454,7 +457,6 @@ static const VMStateDescription vmstate_g364fb = {
> >       .minimum_version_id = 1,
>
> Even if broken, I'd increase to version as good practice.

Sure - I'll go ahead and do that for a v2. The reason I wasn't too worried was 
because since the wrong struct is used for saving data, the outgoing stream is 
corrupted so all bets are off on being able to reload it(!).

> >       .post_load = g364fb_post_load,
> >       .fields = (VMStateField[]) {
> > -        VMSTATE_VBUFFER_UINT32(vram, G364State, 1, NULL, vram_size),
> >           VMSTATE_BUFFER_UNSAFE(color_palette, G364State, 0, 256 * 3),
> >           VMSTATE_BUFFER_UNSAFE(cursor_palette, G364State, 0, 9),
> >           VMSTATE_UINT16_ARRAY(cursor, G364State, 512),
>
> The vram pointer is now unused, we can remove it:

Indeed. I remember thinking I had to do this, but clearly forgot. Will fix.

> -- >8 --
> diff --git a/hw/display/g364fb.c b/hw/display/g364fb.c
> index 990ef3afdd8..11ad61fa73a 100644
> --- a/hw/display/g364fb.c
> +++ b/hw/display/g364fb.c
> @@ -34,7 +34,6 @@
>
>   typedef struct G364State {
>       /* hardware */
> -    uint8_t *vram;
>       uint32_t vram_size;
>       qemu_irq irq;
>       MemoryRegion mem_vram;
> ---
>
> Removing 'uint8_t *vram':
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


ATB,

Mark.