[Sound-open-firmware] [PATCH_V3 4/8] cnl-interrupt: refine interrupt setting for change in irq_desc

Jie, Yang yang.jie at intel.com
Thu May 10 16:25:04 CEST 2018


>-----Original Message-----
>From: sound-open-firmware-bounces at alsa-project.org [mailto:sound-open-
>firmware-bounces at alsa-project.org] On Behalf Of Rander Wang
>Sent: Thursday, May 10, 2018 1:32 PM
>To: sound-open-firmware at alsa-project.org; marcin.maka at linux.intel.com
>Cc: Liam Girdwood <liam.r.girdwood at linux.intel.com>; Rander Wang
><rander.wang at linux.intel.com>
>Subject: [Sound-open-firmware] [PATCH_V3 4/8] cnl-interrupt: refine interrupt
>setting for change in irq_desc
>
>(1)Add core id check in interrupt register setting.
>(2)Check child list in interrupt function (3)Extend root irq_desc to support 4 core
>on cnl.
>(4)Refine macro for interrupt setting
>
>Signed-off-by: Rander Wang <rander.wang at linux.intel.com>
>Signed-off-by: Liam Girdwood <liam.r.girdwood at linux.intel.com>
>
>---
>V2: call cpu_get_id to get core id
>V3: remove duplication in handler function
>
>test on CNL & APL & BYT, pass
>SOF: master c1f2682c210201
>kernel: v4.14 d09db67c5a9d6d
>SOF-tools: master 13b56fa6047c566a
>---
> src/platform/cannonlake/dma.c                      |   2 +-
> .../cannonlake/include/platform/interrupt.h        |  16 +-
> src/platform/cannonlake/include/platform/memory.h  |   2 +-
> .../cannonlake/include/platform/platform.h         |   2 +
> src/platform/cannonlake/interrupt.c                | 258 ++++++++-------------
> 5 files changed, 108 insertions(+), 172 deletions(-)
>
>diff --git a/src/platform/cannonlake/dma.c b/src/platform/cannonlake/dma.c
>index b606d1e..3f9d6cb 100644
>--- a/src/platform/cannonlake/dma.c
>+++ b/src/platform/cannonlake/dma.c
>@@ -125,7 +125,7 @@ static struct dma dma[] = {
> 		.id		= DMA_GP_LP_DMAC1,
> 		.base		= LP_GP_DMA_BASE(1),
> 		.channels	= 8,
>-		.irq = IRQ_EXT_LP_GPDMA0_LVL5(0, 0),
>+		.irq = IRQ_EXT_LP_GPDMA1_LVL5(0, 0),
> 		.drv_plat_data	= &dmac1,
> 	},
> 	.ops		= &dw_dma_ops,
>diff --git a/src/platform/cannonlake/include/platform/interrupt.h
>b/src/platform/cannonlake/include/platform/interrupt.h
>index 03857a7..b14cfdf 100644
>--- a/src/platform/cannonlake/include/platform/interrupt.h
>+++ b/src/platform/cannonlake/include/platform/interrupt.h
>@@ -88,7 +88,7 @@
> #define IRQ_BIT_LVL5_DMIC		6
> #define IRQ_BIT_LVL5_SSP(x)		(0 + x)
>
>-/* Level 2 Peripheral IRQ mappings */
>+/* Priority 2 Peripheral IRQ mappings */
> #define IRQ_EXT_HP_GPDMA_LVL2(xcpu) \
> 	SOF_IRQ(IRQ_BIT_LVL2_HP_GP_DMA0(0), 2, xcpu,
>IRQ_NUM_EXT_LEVEL2)  #define IRQ_EXT_IDC_LVL2(xcpu) \ @@ -106,7 +106,7
>@@  #define IRQ_EXT_SHA256_LVL2(xcpu) \
> 	SOF_IRQ(IRQ_BIT_LVL2_SHA256, 2, xcpu, IRQ_NUM_EXT_LEVEL2)
>
>-/* Level 3 Peripheral IRQ mappings */
>+/* Priority 3 Peripheral IRQ mappings */
> #define IRQ_EXT_CODE_DMA_LVL3(xcpu) \
> 	SOF_IRQ(IRQ_BIT_LVL3_CODE_LOADER, 3, xcpu, IRQ_NUM_EXT_LEVEL3)
>#define IRQ_EXT_HOST_DMA_IN_LVL3(xcpu, channel) \ @@ -114,17 +114,17
>@@  #define IRQ_EXT_HOST_DMA_OUT_LVL3(xcpu, channel) \
> 	SOF_IRQ(IRQ_BIT_LVL3_HOST_STREAM_OUT(channel), 3, xcpu,
>IRQ_NUM_EXT_LEVEL3)
>
>-/* Level 4 Peripheral IRQ mappings */
>+/* Priority 4 Peripheral IRQ mappings */
> #define IRQ_EXT_LINK_DMA_IN_LVL4(xcpu, channel) \
> 	SOF_IRQ(IRQ_BIT_LVL4_LINK_STREAM_IN(channel), 4, xcpu,
>IRQ_NUM_EXT_LEVEL4)  #define IRQ_EXT_LINK_DMA_OUT_LVL4(xcpu, channel)
>\
> 	SOF_IRQ(IRQ_BIT_LVL4_LINK_STREAM_OUT(channel), 4, xcpu,
>IRQ_NUM_EXT_LEVEL4)
>
>-/* Level 5 Peripheral IRQ mappings */
>+/* Priority 5 Peripheral IRQ mappings */
> #define IRQ_EXT_LP_GPDMA0_LVL5(xcpu, channel) \
>-	SOF_IRQ(IRQ_BIT_LVL5_LP_GP_DMA0, 5, xcpu, IRQ_NUM_EXT_LEVEL5)
>-#define IRQ_EXT_LP_GPDMA1_LVL4(xcpu, channel) \
>-	SOF_IRQ(IRQ_BIT_LVL5_LP_GP_DMA1, 4, xcpu, IRQ_NUM_EXT_LEVEL4)
>+	SOF_ID_IRQ(0, IRQ_BIT_LVL5_LP_GP_DMA0, 5, xcpu,
>IRQ_NUM_EXT_LEVEL5)
>+#define IRQ_EXT_LP_GPDMA1_LVL5(xcpu, channel) \
>+	SOF_ID_IRQ(1, IRQ_BIT_LVL5_LP_GP_DMA0, 5, xcpu,
>IRQ_NUM_EXT_LEVEL5)
> #define IRQ_EXT_SSP0_LVL5(xcpu) \
> 	SOF_IRQ(IRQ_BIT_LVL5_SSP(0), 5, xcpu, IRQ_NUM_EXT_LEVEL5)
>#define IRQ_EXT_SSP1_LVL5(xcpu) \ @@ -161,7 +161,7 @@
>
> void platform_interrupt_init(void);
>
>-struct irq_parent *platform_irq_get_parent(uint32_t irq);
>+struct irq_desc *platform_irq_get_parent(uint32_t irq);
> void platform_interrupt_set(int irq);
> void platform_interrupt_clear(uint32_t irq, uint32_t mask);  uint32_t
>platform_interrupt_get_enabled(void);
>diff --git a/src/platform/cannonlake/include/platform/memory.h
>b/src/platform/cannonlake/include/platform/memory.h
>index c7f4750..67f574a 100644
>--- a/src/platform/cannonlake/include/platform/memory.h
>+++ b/src/platform/cannonlake/include/platform/memory.h
>@@ -240,7 +240,7 @@
> #define SOF_TEXT_SIZE		0x18000
>
> /* initialized data */
>-#define SOF_DATA_SIZE		0x18000
>+#define SOF_DATA_SIZE		0x19000
>
> /* bss data */
> #define SOF_BSS_DATA_SIZE		0x8000
>diff --git a/src/platform/cannonlake/include/platform/platform.h
>b/src/platform/cannonlake/include/platform/platform.h
>index dc9a963..73d57ec 100644
>--- a/src/platform/cannonlake/include/platform/platform.h
>+++ b/src/platform/cannonlake/include/platform/platform.h
>@@ -43,6 +43,8 @@ struct sof;
> #define PLATFORM_SSP_COUNT 3
> #define MAX_GPDMA_COUNT 2
>
>+#define MAX_CORE_COUNT 4
>+
> /* Host page size */
> #define HOST_PAGE_SIZE		4096
> #define PLATFORM_PAGE_TABLE_SIZE	256
>diff --git a/src/platform/cannonlake/interrupt.c
>b/src/platform/cannonlake/interrupt.c
>index 8c47e14..949b923 100644
>--- a/src/platform/cannonlake/interrupt.c
>+++ b/src/platform/cannonlake/interrupt.c
>@@ -34,24 +34,31 @@
> #include <sof/interrupt.h>
> #include <sof/interrupt-map.h>
> #include <arch/interrupt.h>
>+#include <arch/cpu.h>
> #include <platform/interrupt.h>
> #include <platform/shim.h>
> #include <stdint.h>
> #include <stdlib.h>
>
>-static void parent_level2_handler(void *data)
>+static inline void irq_lvl2_handler(void *data,
>+				    int level,
>+				    uint32_t ilxsd,
>+				    uint32_t ilxmsd,
>+				    uint32_t ilxmcd)
> {
>-	struct irq_parent *parent = (struct irq_parent *)data;
>-	struct irq_child * child = NULL;
>+	struct irq_desc *parent = (struct irq_desc *)data;
>+	struct irq_desc *child = NULL;
>+	struct list_item *clist;
> 	uint32_t status;
> 	uint32_t i = 0;
>+	uint32_t unmask = 0;
>
> 	/* mask the parent IRQ */
>-	arch_interrupt_disable_mask(1 << IRQ_NUM_EXT_LEVEL2);
>+	arch_interrupt_disable_mask(1 << level);
>
> 	/* mask all child interrupts */
>-	status = irq_read(REG_IRQ_IL2SD(0));
>-	irq_write(REG_IRQ_IL2MSD(0), status);
>+	status = irq_read(ilxsd);
>+	irq_write(ilxmsd, status);
>
> 	/* handle each child */
> 	while (status) {
>@@ -61,175 +68,95 @@ static void parent_level2_handler(void *data)
> 			goto next;
>
> 		/* get child if any and run handler */
>-		child = parent->child[i];
>-		if (child && child->handler) {
>-			child->handler(child->handler_arg);
>-
>-			/* unmask this bit i interrupt */
>-			irq_write(REG_IRQ_IL2MCD(0), 0x1 << i);
>-		} else {
>-			/* nobody cared ? */
>-			trace_irq_error("nbc");
>+		list_for_item(clist, &parent->child[i]) {
>+			child = container_of(clist, struct irq_desc, irq_list);
>+
>+			if (child && child->handler) {
>+				child->handler(child->handler_arg);
>+				unmask = 1;
>+			} else {
>+				/* nobody cared ? */
>+				trace_irq_error("nbc");
>+			}
> 		}
>
>+		/* unmask this bit i interrupt */
>+		if (unmask)
>+			irq_write(ilxmcd, 0x1 << i);
>+
> next:
> 		status >>= 1;
> 		i++;
> 	}
>
> 	/* clear parent and unmask */
>-	arch_interrupt_clear(IRQ_NUM_EXT_LEVEL2);
>-	arch_interrupt_enable_mask(1 << IRQ_NUM_EXT_LEVEL2);
>+	arch_interrupt_clear(level);
>+	arch_interrupt_enable_mask(1 << level);
> }
>
>-static void parent_level3_handler(void *data) -{
>-	struct irq_parent *parent = (struct irq_parent *)data;
>-	struct irq_child * child = NULL;
>-	uint32_t status;
>-	uint32_t i = 0;
>-
>-	/* mask the parent IRQ */
>-	arch_interrupt_disable_mask(1 << IRQ_NUM_EXT_LEVEL3);
>-
>-	/* mask all child interrupts */
>-	status = irq_read(REG_IRQ_IL3SD(0));
>-	irq_write(REG_IRQ_IL3MSD(0), status);
>-
>-	/* handle each child */
>-	while (status) {
>-
>-		/* any IRQ for this child bit ? */
>-		if ((status & 0x1) == 0)
>-			goto next;
>-
>-		/* get child if any and run handler */
>-		child = parent->child[i];
>-		if (child && child->handler) {
>-			child->handler(child->handler_arg);
>-
>-			/* unmask this bit i interrupt */
>-			irq_write(REG_IRQ_IL3MCD(0), 0x1 << i);
>-		} else {
>-			/* nobody cared ? */
>-			trace_irq_error("nbc");
>-		}
>-
>-next:
>-		status >>= 1;
>-		i++;
>-	}
>+#define IRQ_LVL2_HANDLER(n) int core = cpu_get_id(); \
>+				irq_lvl2_handler(data, \
>+				 IRQ_NUM_EXT_LEVEL##n, \
>+				 REG_IRQ_IL##n##SD(core), \
>+				 REG_IRQ_IL##n##MSD(core), \
>+				 REG_IRQ_IL##n##MCD(core))
>
>-	/* clear parent and unmask */
>-	arch_interrupt_clear(IRQ_NUM_EXT_LEVEL3);
>-	arch_interrupt_enable_mask(1 << IRQ_NUM_EXT_LEVEL3);
>+static void irq_lvl2_level2_handler(void *data) {
>+	IRQ_LVL2_HANDLER(2);
> }
>
>-static void parent_level4_handler(void *data)
>+static void irq_lvl2_level3_handler(void *data)
> {
>-	struct irq_parent *parent = (struct irq_parent *)data;
>-	struct irq_child * child = NULL;
>-	uint32_t status;
>-	uint32_t i = 0;
>-
>-	/* mask the parent IRQ */
>-	arch_interrupt_disable_mask(1 << IRQ_NUM_EXT_LEVEL4);
>-
>-	/* mask all child interrupts */
>-	status = irq_read(REG_IRQ_IL4SD(0));
>-	irq_write(REG_IRQ_IL4MSD(0), status);
>-
>-	/* handle each child */
>-	while (status) {
>-
>-		/* any IRQ for this child bit ? */
>-		if ((status & 0x1) == 0)
>-			goto next;
>-
>-		/* get child if any and run handler */
>-		child = parent->child[i];
>-		if (child && child->handler) {
>-			child->handler(child->handler_arg);
>-
>-			/* unmask this bit i interrupt */
>-			irq_write(REG_IRQ_IL4MCD(0), 0x1 << i);
>-		} else {
>-			/* nobody cared ? */
>-			trace_irq_error("nbc");
>-		}
>-
>-next:
>-		status >>= 1;
>-		i++;
>-	}
>-
>-	/* clear parent and unmask */
>-	arch_interrupt_clear(IRQ_NUM_EXT_LEVEL4);
>-	arch_interrupt_enable_mask(1 << IRQ_NUM_EXT_LEVEL4);
>+	IRQ_LVL2_HANDLER(3);
> }
>
>-static void parent_level5_handler(void *data)
>+static void irq_lvl2_level4_handler(void *data)
> {
>-	struct irq_parent *parent = (struct irq_parent *)data;
>-	struct irq_child * child = NULL;
>-	uint32_t status;
>-	uint32_t i = 0;
>-
>-	/* mask the parent IRQ */
>-	arch_interrupt_disable_mask(1 << IRQ_NUM_EXT_LEVEL5);
>-
>-	/* mask all child interrupts */
>-	status = irq_read(REG_IRQ_IL5SD(0));
>-	irq_write(REG_IRQ_IL5MSD(0), status);
>-
>-	/* handle each child */
>-	while (status) {
>-
>-		/* any IRQ for this child bit ? */
>-		if ((status & 0x1) == 0)
>-			goto next;
>-
>-		/* get child if any and run handler */
>-		child = parent->child[i];
>-		if (child && child->handler) {
>-			child->handler(child->handler_arg);
>-
>-			/* unmask this bit i interrupt */
>-			irq_write(REG_IRQ_IL5MCD(0), 0x1 << i);
>-		} else {
>-			/* nobody cared ? */
>-			trace_irq_error("nbc");
>-		}
>-
>-next:
>-		status >>= 1;
>-		i++;
>-	}
>+	IRQ_LVL2_HANDLER(4);
>+}
>
>-	/* clear parent and unmask */
>-	arch_interrupt_clear(IRQ_NUM_EXT_LEVEL5);
>-	arch_interrupt_enable_mask(1 << IRQ_NUM_EXT_LEVEL5);
>+static void irq_lvl2_level5_handler(void *data) {
>+	IRQ_LVL2_HANDLER(5);
> }
>
> /* DSP internal interrupts */
>-static struct irq_parent dsp_irq[4] = {
>-	{IRQ_NUM_EXT_LEVEL2, parent_level2_handler, },
>-	{IRQ_NUM_EXT_LEVEL3, parent_level3_handler, },
>-	{IRQ_NUM_EXT_LEVEL4, parent_level4_handler, },
>-	{IRQ_NUM_EXT_LEVEL5, parent_level5_handler, },
>+static struct irq_desc dsp_irq[MAX_CORE_COUNT][4] = {
>+	{{IRQ_NUM_EXT_LEVEL2, irq_lvl2_level2_handler, },
>+	{IRQ_NUM_EXT_LEVEL3, irq_lvl2_level3_handler, },
>+	{IRQ_NUM_EXT_LEVEL4, irq_lvl2_level4_handler, },
>+	{IRQ_NUM_EXT_LEVEL5, irq_lvl2_level5_handler, } },
>+
>+	{{IRQ_NUM_EXT_LEVEL2, irq_lvl2_level2_handler, },
>+	{IRQ_NUM_EXT_LEVEL3, irq_lvl2_level3_handler, },
>+	{IRQ_NUM_EXT_LEVEL4, irq_lvl2_level4_handler, },
>+	{IRQ_NUM_EXT_LEVEL5, irq_lvl2_level5_handler, } },
>+
>+	{{IRQ_NUM_EXT_LEVEL2, irq_lvl2_level2_handler, },
>+	{IRQ_NUM_EXT_LEVEL3, irq_lvl2_level3_handler, },
>+	{IRQ_NUM_EXT_LEVEL4, irq_lvl2_level4_handler, },
>+	{IRQ_NUM_EXT_LEVEL5, irq_lvl2_level5_handler, } },
>+
>+	{{IRQ_NUM_EXT_LEVEL2, irq_lvl2_level2_handler, },
>+	{IRQ_NUM_EXT_LEVEL3, irq_lvl2_level3_handler, },
>+	{IRQ_NUM_EXT_LEVEL4, irq_lvl2_level4_handler, },
>+	{IRQ_NUM_EXT_LEVEL5, irq_lvl2_level5_handler, } },
> };

Can we share this irq_desc array among 4 cores? They looks exactly same and are static.

Thanks,
~Keyon

>
>-struct irq_parent *platform_irq_get_parent(uint32_t irq)
>+struct irq_desc *platform_irq_get_parent(uint32_t irq)
> {
>+	int core = cpu_get_id();
>+
> 	switch (SOF_IRQ_NUMBER(irq)) {
> 	case IRQ_NUM_EXT_LEVEL2:
>-		return &dsp_irq[0];
>+		return &dsp_irq[core][0];
> 	case IRQ_NUM_EXT_LEVEL3:
>-		return &dsp_irq[1];
>+		return &dsp_irq[core][1];
> 	case IRQ_NUM_EXT_LEVEL4:
>-		return &dsp_irq[2];
>+		return &dsp_irq[core][2];
> 	case IRQ_NUM_EXT_LEVEL5:
>-		return &dsp_irq[3];
>+		return &dsp_irq[core][3];
> 	default:
> 		return NULL;
> 	}
>@@ -242,19 +169,21 @@ uint32_t platform_interrupt_get_enabled(void)
>
> void platform_interrupt_mask(uint32_t irq, uint32_t mask)  {
>+	int core = cpu_get_id();
>+
> 	/* mask external interrupt bit */
> 	switch (SOF_IRQ_NUMBER(irq)) {
> 	case IRQ_NUM_EXT_LEVEL5:
>-		irq_write(REG_IRQ_IL5MSD(0), 1 << SOF_IRQ_BIT(irq));
>+		irq_write(REG_IRQ_IL5MSD(core), 1 << SOF_IRQ_BIT(irq));
> 		break;
> 	case IRQ_NUM_EXT_LEVEL4:
>-		irq_write(REG_IRQ_IL4MSD(0), 1 << SOF_IRQ_BIT(irq));
>+		irq_write(REG_IRQ_IL4MSD(core), 1 << SOF_IRQ_BIT(irq));
> 		break;
> 	case IRQ_NUM_EXT_LEVEL3:
>-		irq_write(REG_IRQ_IL3MSD(0), 1 << SOF_IRQ_BIT(irq));
>+		irq_write(REG_IRQ_IL3MSD(core), 1 << SOF_IRQ_BIT(irq));
> 		break;
> 	case IRQ_NUM_EXT_LEVEL2:
>-		irq_write(REG_IRQ_IL2MSD(0), 1 << SOF_IRQ_BIT(irq));
>+		irq_write(REG_IRQ_IL2MSD(core), 1 << SOF_IRQ_BIT(irq));
> 		break;
> 	default:
> 		break;
>@@ -264,19 +193,21 @@ void platform_interrupt_mask(uint32_t irq, uint32_t
>mask)
>
> void platform_interrupt_unmask(uint32_t irq, uint32_t mask)  {
>+	int core = cpu_get_id();
>+
> 	/* unmask external interrupt bit */
> 	switch (SOF_IRQ_NUMBER(irq)) {
> 	case IRQ_NUM_EXT_LEVEL5:
>-		irq_write(REG_IRQ_IL5MCD(0), 1 << SOF_IRQ_BIT(irq));
>+		irq_write(REG_IRQ_IL5MCD(core), 1 << SOF_IRQ_BIT(irq));
> 		break;
> 	case IRQ_NUM_EXT_LEVEL4:
>-		irq_write(REG_IRQ_IL4MCD(0), 1 << SOF_IRQ_BIT(irq));
>+		irq_write(REG_IRQ_IL4MCD(core), 1 << SOF_IRQ_BIT(irq));
> 		break;
> 	case IRQ_NUM_EXT_LEVEL3:
>-		irq_write(REG_IRQ_IL3MCD(0), 1 << SOF_IRQ_BIT(irq));
>+		irq_write(REG_IRQ_IL3MCD(core), 1 << SOF_IRQ_BIT(irq));
> 		break;
> 	case IRQ_NUM_EXT_LEVEL2:
>-		irq_write(REG_IRQ_IL2MCD(0), 1 << SOF_IRQ_BIT(irq));
>+		irq_write(REG_IRQ_IL2MCD(core), 1 << SOF_IRQ_BIT(irq));
> 		break;
> 	default:
> 		break;
>@@ -290,15 +221,18 @@ void platform_interrupt_clear(uint32_t irq, uint32_t
>mask)
>
> void platform_interrupt_init(void)
> {
>-	int i;
>+	int i, j;
>+	int core = cpu_get_id();
>
> 	/* mask all external IRQs by default */
>-	irq_write(REG_IRQ_IL2MSD(0), REG_IRQ_IL2MD_ALL);
>-	irq_write(REG_IRQ_IL3MSD(0), REG_IRQ_IL3MD_ALL);
>-	irq_write(REG_IRQ_IL4MSD(0), REG_IRQ_IL4MD_ALL);
>-	irq_write(REG_IRQ_IL5MSD(0), REG_IRQ_IL5MD_ALL);
>-
>-	for (i = 0; i < ARRAY_SIZE(dsp_irq); i++) {
>-		spinlock_init(&dsp_irq[i].lock);
>+	irq_write(REG_IRQ_IL2MSD(core), REG_IRQ_IL2MD_ALL);
>+	irq_write(REG_IRQ_IL3MSD(core), REG_IRQ_IL3MD_ALL);
>+	irq_write(REG_IRQ_IL4MSD(core), REG_IRQ_IL4MD_ALL);
>+	irq_write(REG_IRQ_IL5MSD(core), REG_IRQ_IL5MD_ALL);
>+
>+	for (i = 0; i < ARRAY_SIZE(dsp_irq[core]); i++) {
>+		spinlock_init(&dsp_irq[core][i].lock);
>+		for (j = 0; j < PLATFORM_IRQ_CHILDREN; j++)
>+			list_init(&dsp_irq[core][i].child[j]);
> 	}
> }
>--
>2.14.1
>
>_______________________________________________
>Sound-open-firmware mailing list
>Sound-open-firmware at alsa-project.org
>http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware


More information about the Sound-open-firmware mailing list