Re: [Qemu-devel] [PATCH 2/2] framebuffer: set DIRTY_MEMORY_VGA on RAM that is used for the framebuffer

[Peter Maydell]

On 23 July 2015 at 12:02, Paolo Bonzini <address@hidden> wrote:
> The MemoryRegionSection contains enough information to access the
> RAM region underlying the framebuffer, and can be cached inside the
> display device.
>
> By doing this, the new framebuffer_update_memory_section function can
> enable dirty memory logging on the relevant RAM region.  The function
> must be called whenever the stride or base of the framebuffer changes;
> a simple way to cover these cases is to call it on every full frame
> invalidation, which is a rare case.
>
> framebuffer_update_display now works entirely on a MemoryRegionSection,
> without going through cpu_physical_memory_map/unmap.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> --- a/hw/display/framebuffer.h
> +++ b/hw/display/framebuffer.h
> @@ -7,10 +7,50 @@
>
>  typedef void (*drawfn)(void *, uint8_t *, const uint8_t *, int, int);
>
> +/* framebuffer_update_memory_section: Update framebuffer
> + * #MemoryRegionSection, for example if the framebuffer is switched to
> + * a different memory area.
> + *
> + * @mem_section: Output #MemoryRegionSection, to be passed to
> + * framebuffer_update_display().
> + * @root: #MemoryRegion within which the framebuffer lies
> + * @base: Base address of the framebuffer within @root.
> + * @rows: Height of the screen.
> + * @src_width: Number of bytes in framebuffer memory between two rows.
> + */
> +void framebuffer_update_memory_section(
> +    MemoryRegionSection *mem_section,
> +    MemoryRegion *root,
> +    hwaddr base,
> +    unsigned rows,
> +    unsigned src_width);
> +
> +/* framebuffer_update_display: Draw the framebuffer on a surface.
> + *
> + * @ds: #DisplaySurface to draw to.
> + * @mem_section: #MemoryRegionSection provded by

"provided"

> + * framebuffer_update_memory_section().
> + * @cols: Width the screen.
> + * @rows: Height of the screen.
> + * @src_width: Number of bytes in framebuffer memory between two rows.
> + * @dest_row_pitch: Number of bytes in the surface data between two rows.
> + * Negative if the framebuffer is stored in the opposite order (e.g.

should these be "i.e.", or do we allow the source framebuffer to be
either way round?

> + * bottom-to-top) compared to the framebuffer.
> + * @dest_col_pitch: Number of bytes in the surface data between two pixels.
> + * Negative if the framebuffer is stored in the opposite order (e.g.
> + * right-to-left) compared to the framebuffer.
> + * @invalidate: True if the function should redraw the whole screen
> + * without checking the DIRTY_MEMORY_VGA dirty bitmap.
> + * @fn: Drawing function to be called for each row that has to be drawn.
> + * @opaque: Opaque pointer passed to @fn.
> + * @first_row: Pointer to an integer, receives the number of the first row
> + * that was drawn (either the first dirty row, or 0 if @invalidate is true).
> + * @last_row: Pointer to an integer, receives the number of the last row that
> + * was drawn (either the last dirty row, or @rows-1 if @invalidate is true).
> + */

Otherwise
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM