Hi Phil,
Agreed, for v3 I selected naming with "orgated" instead of "shared" because
there is already IRQ line #15 that is called shared which seems like a
completely different thing and it's "not used" according to DTS
(arch/arm/boot/dts/bcm2835-common.dtsi):
interrupts = <1 16>,
<1 17>,
<1 18>,
<1 19>,
<1 20>,
<1 21>,
<1 22>,
<1 23>,
<1 24>,
<1 25>,
<1 26>,
/* dma channel 11-14 share one irq */
<1 27>,
<1 27>,
<1 27>,
<1 27>,
/* unused shared irq for all channels */
<1 28>;
> So before we could trigger IRQ #12, and now it is unbound?
Yeah, well, DMA IRQ #12 is bound to the same line as #11, but DMA IRQ #15 aka
IC IRQ # INTERRUPT_DMA0+12=16+12=28 is not bound anymore indeed (the last one
in the list above). Previously channels 0--12 were bound linearly and it was
unclear what this hard-coded "12" is. (This 12 is never mentioned anywhere
besides Qemu.) So actually 3 DMA IRQ #13-15 were unbound before.
I omitted this last DMA IRQ #15 (IC IRQ #28) — it clearly has some special
status in the spec, e.g. it has a different block of DMA address, but it's not
described otherwise. I assumed that "unused" means off.
Best regards,
Andrey Makarov
Software Team Lead
Auriga LLC
________________________________
From: Philippe Mathieu-Daudé <philippe.mathieu.da...@gmail.com> on behalf of
Philippe Mathieu-Daudé <f4...@amsat.org>
Sent: Wednesday, July 13, 2022 1:45:13 AM
To: Andrey Makarov; qemu-devel@nongnu.org
Cc: Makarov, Andrey
Subject: Re: [PATCH v2] Align Raspberry Pi DMA interrupts with Linux DTS
Hi Andrey,
On 12/7/22 12:45, Andrey Makarov wrote:
> There is nothing in the specs on DMA engine interrupt lines: it should have
> been in the "BCM2835 ARM Peripherals" datasheet but the appropriate
> "ARM peripherals interrupt table" (p.113) is nearly empty.
>
> All Raspberry Pi models 1-3 (based on bcm2835) have
> Linux device tree (arch/arm/boot/dts/bcm2835-common.dtsi +25):
>
> /* dma channel 11-14 share one irq */
>
> This information is repeated in the driver code
> (drivers/dma/bcm2835-dma.c +1344):
>
> /*
> * in case of channel >= 11
> * use the 11th interrupt and that is shared
> */
>
> In this patch channels 0--10 and 11--14 are handled separately.
>
> In version v2:
>
> 1) an OR-gate is added according to review
> 2) a simple qtest is added for testing DMA & its interrupts
>
> Signed-off-by: Andrey Makarov <andrey.maka...@auriga.com>
> ---
> hw/arm/bcm2835_peripherals.c | 21 +++++-
> include/hw/arm/bcm2835_peripherals.h | 2 +
> tests/qtest/bcm2835-dma-test.c | 106 +++++++++++++++++++++++++++
> tests/qtest/meson.build | 3 +-
> 4 files changed, 130 insertions(+), 2 deletions(-)
> create mode 100644 tests/qtest/bcm2835-dma-test.c
>
> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
> index 48538c9360..5a9c472b5a 100644
> --- a/hw/arm/bcm2835_peripherals.c
> +++ b/hw/arm/bcm2835_peripherals.c
> @@ -101,6 +101,11 @@ static void bcm2835_peripherals_init(Object *obj)
> /* DMA Channels */
> object_initialize_child(obj, "dma", &s->dma, TYPE_BCM2835_DMA);
>
> + object_initialize_child(obj, "dma-11-14-irq-orgate",
Maybe name "shared-dma-irq-orgate"?
> + &s->dma_11_14_irq_orgate, TYPE_OR_IRQ);
Similarly 'shared_dma' or 'orgated-dma'? But not _11_14_.
> + object_property_set_int(OBJECT(&s->dma_11_14_irq_orgate), "num-lines", 4,
Instead of using a magic number:
#define BCM2835_SHARED_DMA_COUNT 4
> + &error_abort);
> +
> object_property_add_const_link(OBJECT(&s->dma), "dma-mr",
> OBJECT(&s->gpu_bus_mr));
>
> @@ -322,13 +327,27 @@ static void bcm2835_peripherals_realize(DeviceState
> *dev, Error **errp)
> memory_region_add_subregion(&s->peri_mr, DMA15_OFFSET,
> sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->dma), 1));
>
> - for (n = 0; n <= 12; n++) {
> + for (n = 0; n <= 10; n++) {
So before we could trigger IRQ #12, and now it is unbound?
Also:
#define BCM2835_DMA_CHANNELS 10
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->dma), n,
> qdev_get_gpio_in_named(DEVICE(&s->ic),
> BCM2835_IC_GPU_IRQ,
> INTERRUPT_DMA0 + n));
> }
>
> + /* According to DTS, dma channels 11-14 share one irq */
> + if (!qdev_realize(DEVICE(&s->dma_11_14_irq_orgate), NULL, errp)) {
> + return;
> + }
> + for (n = 11; n <= 14; n++) {
Logic simplified if you use the [0 .. BCM2835_SHARED_DMA_COUNT-1] range:
for (n = 0; n < BCM2835_SHARED_DMA_COUNT; n++) {
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->dma), n,
BCM2835_DMA_CHANNELS + 1 + n,
> + qdev_get_gpio_in(DEVICE(&s->dma_11_14_irq_orgate),
> + n - 11));
n)
> + }
> + qdev_connect_gpio_out(DEVICE(&s->dma_11_14_irq_orgate), 0,
> + qdev_get_gpio_in_named(DEVICE(&s->ic),
> + BCM2835_IC_GPU_IRQ,
> + INTERRUPT_DMA0 + 11));
> +
> /* THERMAL */
> if (!sysbus_realize(SYS_BUS_DEVICE(&s->thermal), errp)) {
> return;
> diff --git a/include/hw/arm/bcm2835_peripherals.h
> b/include/hw/arm/bcm2835_peripherals.h
> index d864879421..79e2f2771a 100644
> --- a/include/hw/arm/bcm2835_peripherals.h
> +++ b/include/hw/arm/bcm2835_peripherals.h
> @@ -17,6 +17,7 @@
> #include "hw/char/bcm2835_aux.h"
> #include "hw/display/bcm2835_fb.h"
> #include "hw/dma/bcm2835_dma.h"
> +#include "hw/or-irq.h"
> #include "hw/intc/bcm2835_ic.h"
> #include "hw/misc/bcm2835_property.h"
> #include "hw/misc/bcm2835_rng.h"
> @@ -55,6 +56,7 @@ struct BCM2835PeripheralState {
> BCM2835AuxState aux;
> BCM2835FBState fb;
> BCM2835DMAState dma;
> + qemu_or_irq dma_11_14_irq_orgate;
> BCM2835ICState ic;
> BCM2835PropertyState property;
> BCM2835RngState rng;
Regards,
Phil.
