[05/14] tcx: alter tcx_set_dirty() to accept address and length parameters

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/display/tcx.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Hi Mark,

On 04/05/2017 05:35 AM, Mark Cave-Ayland wrote:
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/display/tcx.c |   12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 8e26aae..d24466f 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -93,9 +93,9 @@ typedef struct TCXState {
>      uint16_t cursy;
>  } TCXState;
>
> -static void tcx_set_dirty(TCXState *s)
> +static void tcx_set_dirty(TCXState *s, ram_addr_t addr, int len)

len is uint64_t

with this change:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

This function can also be inlined.

>  {
> -    memory_region_set_dirty(&s->vram_mem, 0, MAXX * MAXY);
> +    memory_region_set_dirty(&s->vram_mem, addr, len);
>  }
>
>  static inline int tcx24_check_dirty(TCXState *s, ram_addr_t page,
> @@ -156,7 +156,7 @@ static void update_palette_entries(TCXState *s, int start, int end)
>              break;
>          }
>      }
> -    tcx_set_dirty(s);
> +    tcx_set_dirty(s, 0, memory_region_size(&s->vram_mem));
>  }
>
>  static void tcx_draw_line32(TCXState *s1, uint8_t *d,
> @@ -526,7 +526,7 @@ static void tcx_invalidate_display(void *opaque)
>  {
>      TCXState *s = opaque;
>
> -    tcx_set_dirty(s);
> +    tcx_set_dirty(s, 0, memory_region_size(&s->vram_mem));
>      qemu_console_resize(s->con, s->width, s->height);
>  }
>
> @@ -534,7 +534,7 @@ static void tcx24_invalidate_display(void *opaque)
>  {
>      TCXState *s = opaque;
>
> -    tcx_set_dirty(s);
> +    tcx_set_dirty(s, 0, memory_region_size(&s->vram_mem));
>      qemu_console_resize(s->con, s->width, s->height);
>  }
>
> @@ -543,7 +543,7 @@ static int vmstate_tcx_post_load(void *opaque, int version_id)
>      TCXState *s = opaque;
>
>      update_palette_entries(s, 0, 256);
> -    tcx_set_dirty(s);
> +    tcx_set_dirty(s, 0, memory_region_size(&s->vram_mem));
>      return 0;
>  }
>
>

On 15/04/17 15:27, Philippe Mathieu-Daudé wrote:

Hi Philippe,

> Hi Mark,
> 
> On 04/05/2017 05:35 AM, Mark Cave-Ayland wrote:
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>  hw/display/tcx.c |   12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
>> index 8e26aae..d24466f 100644
>> --- a/hw/display/tcx.c
>> +++ b/hw/display/tcx.c
>> @@ -93,9 +93,9 @@ typedef struct TCXState {
>>      uint16_t cursy;
>>  } TCXState;
>>
>> -static void tcx_set_dirty(TCXState *s)
>> +static void tcx_set_dirty(TCXState *s, ram_addr_t addr, int len)
> 
> len is uint64_t

Yes, this was deliberate in order to match the existing tcx_* functions
in the same file which tend to use int for len/width parameters. Note
that TCX is fixed 1024x768 resolution giving a maximum len of ~800K so
there is no risk of overflow here.

> with this change:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> This function can also be inlined.

My current understanding is that these days compilers tend to handle
inlining themselves and inline is either a no-op or a very weak hint?

>>  {
>> -    memory_region_set_dirty(&s->vram_mem, 0, MAXX * MAXY);
>> +    memory_region_set_dirty(&s->vram_mem, addr, len);
>>  }
>>
>>  static inline int tcx24_check_dirty(TCXState *s, ram_addr_t page,
>> @@ -156,7 +156,7 @@ static void update_palette_entries(TCXState *s,
>> int start, int end)
>>              break;
>>          }
>>      }
>> -    tcx_set_dirty(s);
>> +    tcx_set_dirty(s, 0, memory_region_size(&s->vram_mem));
>>  }
>>
>>  static void tcx_draw_line32(TCXState *s1, uint8_t *d,
>> @@ -526,7 +526,7 @@ static void tcx_invalidate_display(void *opaque)
>>  {
>>      TCXState *s = opaque;
>>
>> -    tcx_set_dirty(s);
>> +    tcx_set_dirty(s, 0, memory_region_size(&s->vram_mem));
>>      qemu_console_resize(s->con, s->width, s->height);
>>  }
>>
>> @@ -534,7 +534,7 @@ static void tcx24_invalidate_display(void *opaque)
>>  {
>>      TCXState *s = opaque;
>>
>> -    tcx_set_dirty(s);
>> +    tcx_set_dirty(s, 0, memory_region_size(&s->vram_mem));
>>      qemu_console_resize(s->con, s->width, s->height);
>>  }
>>
>> @@ -543,7 +543,7 @@ static int vmstate_tcx_post_load(void *opaque, int
>> version_id)
>>      TCXState *s = opaque;
>>
>>      update_palette_entries(s, 0, 256);
>> -    tcx_set_dirty(s);
>> +    tcx_set_dirty(s, 0, memory_region_size(&s->vram_mem));
>>      return 0;
>>  }

ATB,

Mark.