| Message ID | a9d502cce05cfa57895c415164efd288f4926fb6.1528935420.git.balaton@eik.bme.hu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote: > From: Sebastian Bauer <mail@sebastianbauer.info> > > Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info> > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Commit message. Why is this necessary? > --- > hw/display/sm501.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/hw/display/sm501.c b/hw/display/sm501.c > index 0625cf5..a2ee6e3 100644 > --- a/hw/display/sm501.c > +++ b/hw/display/sm501.c > @@ -479,6 +479,7 @@ typedef struct SM501State { > MemoryRegion twoD_engine_region; > uint32_t last_width; > uint32_t last_height; > + uint32_t do_full_update; /* perform a full update next time */ > I2CBus *i2c_bus; > > /* mmio registers */ > @@ -1032,6 +1033,7 @@ static void sm501_palette_write(void *opaque, hwaddr addr, > > assert(range_covers_byte(0, 0x400 * 3, addr)); > *(uint32_t *)&s->dc_palette[addr] = value; > + s->do_full_update = 1; > } > > static uint64_t sm501_disp_ctrl_read(void *opaque, hwaddr addr, > @@ -1620,6 +1622,12 @@ static void sm501_update_display(void *opaque) > full_update = 1; > } > > + /* someone else requested a full update */ > + if (s->do_full_update) { > + s->do_full_update = 0; > + full_update = 1; > + } > + > /* draw each line according to conditions */ > snap = memory_region_snapshot_and_clear_dirty(&s->local_mem_region, > offset, width * height * src_bpp, DIRTY_MEMORY_VGA);
On Thu, 14 Jun 2018, David Gibson wrote: > On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote: >> From: Sebastian Bauer <mail@sebastianbauer.info> >> >> Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > > Commit message. Why is this necessary? Maybe because changing palette does not change display unless an update is done and dirty tracking which is used in update_display does not detect changes in device registers where palette is stored. I'm not sure this is needed in all modes but I guess palette is not used in modes that are not indexed so unecessary updates should not happen evem in those cases because if palette is not used anyway, guest is unlikely to change it unless doing something really strange but we haven't seen any guests yet that would do that. Therefore this simple way of handling this should be OK. Regards, BALATON Zoltan >> --- >> hw/display/sm501.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/hw/display/sm501.c b/hw/display/sm501.c >> index 0625cf5..a2ee6e3 100644 >> --- a/hw/display/sm501.c >> +++ b/hw/display/sm501.c >> @@ -479,6 +479,7 @@ typedef struct SM501State { >> MemoryRegion twoD_engine_region; >> uint32_t last_width; >> uint32_t last_height; >> + uint32_t do_full_update; /* perform a full update next time */ >> I2CBus *i2c_bus; >> >> /* mmio registers */ >> @@ -1032,6 +1033,7 @@ static void sm501_palette_write(void *opaque, hwaddr addr, >> >> assert(range_covers_byte(0, 0x400 * 3, addr)); >> *(uint32_t *)&s->dc_palette[addr] = value; >> + s->do_full_update = 1; >> } >> >> static uint64_t sm501_disp_ctrl_read(void *opaque, hwaddr addr, >> @@ -1620,6 +1622,12 @@ static void sm501_update_display(void *opaque) >> full_update = 1; >> } >> >> + /* someone else requested a full update */ >> + if (s->do_full_update) { >> + s->do_full_update = 0; >> + full_update = 1; >> + } >> + >> /* draw each line according to conditions */ >> snap = memory_region_snapshot_and_clear_dirty(&s->local_mem_region, >> offset, width * height * src_bpp, DIRTY_MEMORY_VGA); > >
On Thu, Jun 14, 2018 at 10:00:50AM +0200, BALATON Zoltan wrote: > On Thu, 14 Jun 2018, David Gibson wrote: > > On Thu, Jun 14, 2018 at 02:17:00AM +0200, BALATON Zoltan wrote: > > > From: Sebastian Bauer <mail@sebastianbauer.info> > > > > > > Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info> > > > Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> > > > > Commit message. Why is this necessary? > > Maybe because changing palette does not change display unless an update is > done and dirty tracking which is used in update_display does not detect > changes in device registers where palette is stored. > > I'm not sure this is needed in all modes but I guess palette is not used in > modes that are not indexed so unecessary updates should not happen evem in > those cases because if palette is not used anyway, guest is unlikely to > change it unless doing something really strange but we haven't seen any > guests yet that would do that. Therefore this simple way of handling this > should be OK. Sure, so, put that into a commit message in the next spin. > > Regards, > BALATON Zoltan > > > > --- > > > hw/display/sm501.c | 8 ++++++++ > > > 1 file changed, 8 insertions(+) > > > > > > diff --git a/hw/display/sm501.c b/hw/display/sm501.c > > > index 0625cf5..a2ee6e3 100644 > > > --- a/hw/display/sm501.c > > > +++ b/hw/display/sm501.c > > > @@ -479,6 +479,7 @@ typedef struct SM501State { > > > MemoryRegion twoD_engine_region; > > > uint32_t last_width; > > > uint32_t last_height; > > > + uint32_t do_full_update; /* perform a full update next time */ > > > I2CBus *i2c_bus; > > > > > > /* mmio registers */ > > > @@ -1032,6 +1033,7 @@ static void sm501_palette_write(void *opaque, hwaddr addr, > > > > > > assert(range_covers_byte(0, 0x400 * 3, addr)); > > > *(uint32_t *)&s->dc_palette[addr] = value; > > > + s->do_full_update = 1; > > > } > > > > > > static uint64_t sm501_disp_ctrl_read(void *opaque, hwaddr addr, > > > @@ -1620,6 +1622,12 @@ static void sm501_update_display(void *opaque) > > > full_update = 1; > > > } > > > > > > + /* someone else requested a full update */ > > > + if (s->do_full_update) { > > > + s->do_full_update = 0; > > > + full_update = 1; > > > + } > > > + > > > /* draw each line according to conditions */ > > > snap = memory_region_snapshot_and_clear_dirty(&s->local_mem_region, > > > offset, width * height * src_bpp, DIRTY_MEMORY_VGA); > > > > >
diff --git a/hw/display/sm501.c b/hw/display/sm501.c index 0625cf5..a2ee6e3 100644 --- a/hw/display/sm501.c +++ b/hw/display/sm501.c @@ -479,6 +479,7 @@ typedef struct SM501State { MemoryRegion twoD_engine_region; uint32_t last_width; uint32_t last_height; + uint32_t do_full_update; /* perform a full update next time */ I2CBus *i2c_bus; /* mmio registers */ @@ -1032,6 +1033,7 @@ static void sm501_palette_write(void *opaque, hwaddr addr, assert(range_covers_byte(0, 0x400 * 3, addr)); *(uint32_t *)&s->dc_palette[addr] = value; + s->do_full_update = 1; } static uint64_t sm501_disp_ctrl_read(void *opaque, hwaddr addr, @@ -1620,6 +1622,12 @@ static void sm501_update_display(void *opaque) full_update = 1; } + /* someone else requested a full update */ + if (s->do_full_update) { + s->do_full_update = 0; + full_update = 1; + } + /* draw each line according to conditions */ snap = memory_region_snapshot_and_clear_dirty(&s->local_mem_region, offset, width * height * src_bpp, DIRTY_MEMORY_VGA);