1. Patchew
  2. QEMU
  3. [PATCH-for-7.0 0/5] hw/display: QOM'ify vga_mmio_init() as TYPE_VGA_MMIO
  4. View patch

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

Philippe Mathieu-Daudé posted 5 patches 4 years, 2 months ago
Maintainers: Kamil Rytarowski <kamil@netbsd.org>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Ryo ONODERA <ryoon@netbsd.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Reinoud Zandijk <reinoud@netbsd.org>
There is a newer version of this series
[PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init()
Posted by Philippe Mathieu-Daudé 4 years, 2 months ago 
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)
 {
+    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);
+
     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);
 
     s->vga.con = graphic_console_init(NULL, 0, s->vga.hw_ops, s);
 
-- 
2.31.1
Re: [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init()
Posted by BALATON Zoltan 4 years, 2 months ago 
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.)

> {
> +    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);
> +
>     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?

Regards,
BALATON Zoltan

>     s->vga.con = graphic_console_init(NULL, 0, s->vga.hw_ops, s);
>
>
Re: [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init()
Posted by BALATON Zoltan 4 years, 2 months ago 
On Fri, 19 Nov 2021, 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.)
>
>> {
>> +    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);
>> +
>>     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?

Sorry, this is what's being inlined... So I mean

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

> Regards,
> BALATON Zoltan
>
>>     s->vga.con = graphic_console_init(NULL, 0, s->vga.hw_ops, s);
>> 
>
Re: [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init()
Posted by Philippe Mathieu-Daudé 4 years, 2 months ago 
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);
>>
>>
Re: [PATCH-for-7.0 2/5] hw/display/vga-mmio: Inline vga_mm_init()
Posted by Thomas Huth 4 years, 2 months ago 
On 19/11/2021 18.11, 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)
>   {
> +    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);
> +
>       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)

Preferably with the indentation fixed (already in the first patch):

Reviewed-by: Thomas Huth <thuth@redhat.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.