Re: [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init()

[Philippe Mathieu-Daudé]

On 11/19/21 19:20, BALATON Zoltan wrote:
> On Fri, 19 Nov 2021, Philippe Mathieu-Daudé wrote:
>> Inline vga_mm_init() in vga_mmio_init() to simplify the
>> next patch review. Kind of.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> hw/display/vga-mmio.c | 27 ++++++++++-----------------
>> 1 file changed, 10 insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/display/vga-mmio.c b/hw/display/vga-mmio.c
>> index 8aaf44e7b1d..0aefbcf53a0 100644
>> --- a/hw/display/vga-mmio.c
>> +++ b/hw/display/vga-mmio.c
>> @@ -65,12 +65,19 @@ static const MemoryRegionOps vga_mm_ctrl_ops = {
>>     .endianness = DEVICE_NATIVE_ENDIAN,
>> };
>>
>> -static void vga_mm_init(VGAMmioState *s, hwaddr vram_base,
>> -                        hwaddr ctrl_base, int it_shift,
>> -                        MemoryRegion *address_space)
>> +int vga_mmio_init(hwaddr vram_base,
>> +                    hwaddr ctrl_base, int it_shift,
>> +                    MemoryRegion *address_space)
> 
> Indentation? (But it's removed later so does not really matter.)

Oops I didn't notice.

>> {
>> +    VGAMmioState *s;
>>     MemoryRegion *s_ioport_ctrl, *vga_io_memory;
>>
>> +    s = g_malloc0(sizeof(*s));
>> +
>> +    s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
>> +    s->vga.global_vmstate = true;
>> +    vga_common_init(&s->vga, NULL);

        ^---- here is vga_mm_init() inlined [*]

>>     s->it_shift = it_shift;
>>     s_ioport_ctrl = g_malloc(sizeof(*s_ioport_ctrl));
>>     memory_region_init_io(s_ioport_ctrl, NULL, &vga_mm_ctrl_ops, s,
>> @@ -89,20 +96,6 @@ static void vga_mm_init(VGAMmioState *s, hwaddr
>> vram_base,
>>     memory_region_add_subregion(address_space,
>>                                 vram_base + 0x000a0000, vga_io_memory);
>>     memory_region_set_coalescing(vga_io_memory);
>> -}
>> -
>> -int vga_mmio_init(hwaddr vram_base,
>> -                    hwaddr ctrl_base, int it_shift,
>> -                    MemoryRegion *address_space)
>> -{
>> -    VGAMmioState *s;
>> -
>> -    s = g_malloc0(sizeof(*s));
>> -
>> -    s->vga.vram_size_mb = VGA_RAM_SIZE / MiB;
>> -    s->vga.global_vmstate = true;
>> -    vga_common_init(&s->vga, NULL);
>> -    vga_mm_init(s, vram_base, ctrl_base, it_shift, address_space);
> 
> Where did this vga_mm_init() go?

Earlier in [*].

> Regards,
> BALATON Zoltan
> 
>>     s->vga.con = graphic_console_init(NULL, 0, s->vga.hw_ops, s);
>>
>>