| Message ID | 1430152117-100558-8-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, 04/27 18:28, Paolo Bonzini wrote: > framebuffer.c expects DIRTY_MEMORY_VGA logging to be always on, but that > will not be the case soon. Because framebuffer.c computes the memory > region on the fly for every update (with memory_region_find), it cannot > enable/disable logging by itself. > > Instead, always treat updates as invalidations if dirty logging is > not enabled, assuming that the board will enable logging on the > RAM region that includes the framebuffer. > > To simplify the code, replace memory_region_get_dirty with > memory_region_test_and_clear_dirty. Then memory_region_reset_dirty > is only needed in the invalidate case. The commit message seems out-of-date, but I think it's better to keep the v1 hunks which skip memory_region_get_dirty if invalidate == true, otherwise use memory_region_test_and_clear_dirty. Fam
On 26/05/2015 10:02, Fam Zheng wrote: > On Mon, 04/27 18:28, Paolo Bonzini wrote: >> framebuffer.c expects DIRTY_MEMORY_VGA logging to be always on, but that >> will not be the case soon. Because framebuffer.c computes the memory >> region on the fly for every update (with memory_region_find), it cannot >> enable/disable logging by itself. >> >> Instead, always treat updates as invalidations if dirty logging is >> not enabled, assuming that the board will enable logging on the >> RAM region that includes the framebuffer. >> >> To simplify the code, replace memory_region_get_dirty with >> memory_region_test_and_clear_dirty. Then memory_region_reset_dirty >> is only needed in the invalidate case. > > The commit message seems out-of-date, but I think it's better to keep the v1 > hunks which skip memory_region_get_dirty if invalidate == true, otherwise use > memory_region_test_and_clear_dirty. Unfortunately that is wrong. See the first paragraph of the commit message: "because framebuffer.c computes the memory region on the fly for every update (with memory_region_find), it cannot enable/disable logging by itself". Only the last paragraph of the commit message is out-of-date, and I will remove it. Paolo
diff --git a/hw/display/framebuffer.c b/hw/display/framebuffer.c index 4546e42..2cabced 100644 --- a/hw/display/framebuffer.c +++ b/hw/display/framebuffer.c @@ -63,6 +63,10 @@ void framebuffer_update_display( assert(mem_section.offset_within_address_space == base); memory_region_sync_dirty_bitmap(mem); + if (!memory_region_is_logging(mem, DIRTY_MEMORY_VGA)) { + invalidate = true; + } + src_base = cpu_physical_memory_map(base, &src_len, 0); /* If we can't map the framebuffer then bail. We could try harder, but it's not really worth it as dirty flag tracking will probably
framebuffer.c expects DIRTY_MEMORY_VGA logging to be always on, but that will not be the case soon. Because framebuffer.c computes the memory region on the fly for every update (with memory_region_find), it cannot enable/disable logging by itself. Instead, always treat updates as invalidations if dirty logging is not enabled, assuming that the board will enable logging on the RAM region that includes the framebuffer. To simplify the code, replace memory_region_get_dirty with memory_region_test_and_clear_dirty. Then memory_region_reset_dirty is only needed in the invalidate case. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/display/framebuffer.c | 4 ++++ 1 file changed, 4 insertions(+)