Re: [PATCH v2 06/10] hw/arm/bcm2836: Create VideoCore address space in the SoC

[Philippe Mathieu-Daudé]

On 10/18/19 12:57 AM, Philippe Mathieu-Daudé wrote:
> Currently the VideoCore is created in the Peripheral container
> as the 'GPU bus'. It is created there because the peripherals
> using DMA use physical addresses from the VideoCore bus.
> However the VideoCore is a GPU core placed at the same
> hierarchical level than the ARM cores.
>
> To match the datasheet design, create the VideoCore container
> in the SoC, and link it to the peripheral container.
>
> The VideoCore bus is 1GiB wide, accessible at 4 regions in
> different cache configurations. Add the full mapping.
[...]
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 019e67b906..d712f36052 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -9,6 +9,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qemu/units.h"
>   #include "qapi/error.h"
>   #include "qemu/module.h"
>   #include "cpu.h"
> @@ -16,6 +17,9 @@
>   #include "hw/arm/raspi_platform.h"
>   #include "hw/sysbus.h"
>   
> +/* Peripheral base address on the VC (GPU) system bus */
> +#define BCM2835_VC_PERI_BASE    0x3e000000
> +
>   struct BCM283XInfo {
>       const char *name;
>       const char *cpu_type;
> @@ -50,6 +54,21 @@ static void bcm2836_init(Object *obj)
>       const BCM283XInfo *info = bc->info;
>       int n;
>   
> +    /* VideoCore memory region */
> +    memory_region_init(&s->videocore.mr[0], obj, "videocore-bus", 1 * GiB);
> +    object_property_add_child(obj, "videocore",
> +                              OBJECT(&s->videocore.mr[0]), NULL);
> +    for (n = 1; n < BCM283X_NCPUS; n++) {
> +        static const char *alias_name[] = {
> +            NULL, "cached-coherent", "cached", "uncached"
> +        };
> +        memory_region_init_alias(&s->videocore.mr[n], obj,
> +                                 alias_name[n], &s->videocore.mr[0],
> +                                 0, 1 * GiB);

Please disregard this patch, something is incorrect here and I'll respin.

> +        memory_region_add_subregion_overlap(&s->videocore.mr[0], n * GiB,
> +                                            &s->videocore.mr[n], 0);
> +    }
> +
>       for (n = 0; n < BCM283X_NCPUS; n++) {
>           object_initialize_child(obj, "cpu[*]", &s->cpus[n], 
> sizeof(s->cpus[n]),
>                                   info->cpu_type, &error_abort, NULL);
> @@ -71,6 +90,7 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
>       BCM283XState *s = BCM283X(dev);
>       BCM283XClass *bc = BCM283X_GET_CLASS(dev);
>       const BCM283XInfo *info = bc->info;
> +    MemoryRegion *ram_mr, *peri_mr;
>       Object *obj;
>       Error *err = NULL;
>       int n;
> @@ -83,26 +103,45 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
>                      __func__, error_get_pretty(err));
>           return;
>       }
[...]