diff mbox series

[5/6] irqchip/apple-aic: Support multiple dies

Message ID 20211209043249.65474-6-marcan@marcan.st (mailing list archive)
State New, archived
Headers show
Series irqchip/apple-aic: Add support for AICv2 | expand

Commit Message

Hector Martin Dec. 9, 2021, 4:32 a.m. UTC
Multi-die support in AICv2 uses several sets of IRQ registers. Introduce
a die count and compute the register group offset based on the die ID
field of the hwirq number, as reported by the hardware.

Signed-off-by: Hector Martin <marcan@marcan.st>
---
 drivers/irqchip/irq-apple-aic.c | 75 +++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 22 deletions(-)

Comments

Marc Zyngier Dec. 13, 2021, 4:10 p.m. UTC | #1
On Thu, 09 Dec 2021 04:32:48 +0000,
Hector Martin <marcan@marcan.st> wrote:
> 
> Multi-die support in AICv2 uses several sets of IRQ registers. Introduce
> a die count and compute the register group offset based on the die ID
> field of the hwirq number, as reported by the hardware.
> 
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  drivers/irqchip/irq-apple-aic.c | 75 +++++++++++++++++++++++----------
>  1 file changed, 53 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
> index d03caed51d56..46b7750548a0 100644
> --- a/drivers/irqchip/irq-apple-aic.c
> +++ b/drivers/irqchip/irq-apple-aic.c

[...]

> @@ -535,28 +545,41 @@ static int aic_irq_domain_translate(struct irq_domain *id,
>  				    unsigned int *type)
>  {
>  	struct aic_irq_chip *ic = id->host_data;
> +	u32 *args;
> +	u32 die = 0;
>  
> -	if (fwspec->param_count != 3 || !is_of_node(fwspec->fwnode))
> +	if (fwspec->param_count < 3 || fwspec->param_count > 4 ||
> +	    !is_of_node(fwspec->fwnode))
>  		return -EINVAL;
>  
> +	args = &fwspec->param[1];
> +
> +	if (fwspec->param_count == 4) {
> +		die = args[0];
> +		args++;
> +	}
> +
>  	switch (fwspec->param[0]) {
>  	case AIC_IRQ:
> -		if (fwspec->param[1] >= ic->nr_irq)
> +		if (die >= ic->nr_die)
> +			return -EINVAL;
> +		if (args[0] >= ic->nr_irq)
>  			return -EINVAL;
> -		*hwirq = (FIELD_PREP(AIC_EVENT_TYPE, AIC_EVENT_TYPE_HW) |
> -			  FIELD_PREP(AIC_EVENT_NUM, fwspec->param[1]));
> +		*hwirq = AIC_IRQ_HWIRQ(die, args[0]);
>  		break;

A side issue with this is that it breaks MSIs, due to the way we
construct the intspec (I did hit that when upgrading the M1 intspec to
4 cells for the PMU). I have the following hack locally:

diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
index b090924b41fe..f7b4a67b13cf 100644
--- a/drivers/pci/controller/pcie-apple.c
+++ b/drivers/pci/controller/pcie-apple.c
@@ -218,7 +218,7 @@ static int apple_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
 	if (hwirq < 0)
 		return -ENOSPC;
 
-	fwspec.param[1] += hwirq;
+	fwspec.param[1 + (fwspec.param_count == 4)] += hwirq;
 
 	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &fwspec);
 	if (ret)

Thanks,

	M.
Hector Martin Dec. 18, 2021, 5:39 a.m. UTC | #2
On 14/12/2021 01.10, Marc Zyngier wrote:
>>  	switch (fwspec->param[0]) {
>>  	case AIC_IRQ:
>> -		if (fwspec->param[1] >= ic->nr_irq)
>> +		if (die >= ic->nr_die)
>> +			return -EINVAL;
>> +		if (args[0] >= ic->nr_irq)
>>  			return -EINVAL;
>> -		*hwirq = (FIELD_PREP(AIC_EVENT_TYPE, AIC_EVENT_TYPE_HW) |
>> -			  FIELD_PREP(AIC_EVENT_NUM, fwspec->param[1]));
>> +		*hwirq = AIC_IRQ_HWIRQ(die, args[0]);
>>  		break;
> 
> A side issue with this is that it breaks MSIs, due to the way we
> construct the intspec (I did hit that when upgrading the M1 intspec to
> 4 cells for the PMU). I have the following hack locally:
> 
> diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
> index b090924b41fe..f7b4a67b13cf 100644
> --- a/drivers/pci/controller/pcie-apple.c
> +++ b/drivers/pci/controller/pcie-apple.c
> @@ -218,7 +218,7 @@ static int apple_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
>  	if (hwirq < 0)
>  		return -ENOSPC;
>  
> -	fwspec.param[1] += hwirq;
> +	fwspec.param[1 + (fwspec.param_count == 4)] += hwirq;
>  
>  	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &fwspec);
>  	if (ret)
> 

Heh, I never noticed; probably because I guess the SD card reader on the
machine I've been testing this on doesn't use MSIs, and I haven't tried
WiFi yet.

Perhaps (fwspec.param_count - 2)? It's probably a safer long-term
assumption that the last two cells will always be leaf IRQ number and type.
Marc Zyngier Dec. 20, 2021, 1:38 p.m. UTC | #3
On Sat, 18 Dec 2021 05:39:04 +0000,
Hector Martin <marcan@marcan.st> wrote:
> 
> On 14/12/2021 01.10, Marc Zyngier wrote:
> >>  	switch (fwspec->param[0]) {
> >>  	case AIC_IRQ:
> >> -		if (fwspec->param[1] >= ic->nr_irq)
> >> +		if (die >= ic->nr_die)
> >> +			return -EINVAL;
> >> +		if (args[0] >= ic->nr_irq)
> >>  			return -EINVAL;
> >> -		*hwirq = (FIELD_PREP(AIC_EVENT_TYPE, AIC_EVENT_TYPE_HW) |
> >> -			  FIELD_PREP(AIC_EVENT_NUM, fwspec->param[1]));
> >> +		*hwirq = AIC_IRQ_HWIRQ(die, args[0]);
> >>  		break;
> > 
> > A side issue with this is that it breaks MSIs, due to the way we
> > construct the intspec (I did hit that when upgrading the M1 intspec to
> > 4 cells for the PMU). I have the following hack locally:
> > 
> > diff --git a/drivers/pci/controller/pcie-apple.c b/drivers/pci/controller/pcie-apple.c
> > index b090924b41fe..f7b4a67b13cf 100644
> > --- a/drivers/pci/controller/pcie-apple.c
> > +++ b/drivers/pci/controller/pcie-apple.c
> > @@ -218,7 +218,7 @@ static int apple_msi_domain_alloc(struct irq_domain *domain, unsigned int virq,
> >  	if (hwirq < 0)
> >  		return -ENOSPC;
> >  
> > -	fwspec.param[1] += hwirq;
> > +	fwspec.param[1 + (fwspec.param_count == 4)] += hwirq;
> >  
> >  	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &fwspec);
> >  	if (ret)
> > 
> 
> Heh, I never noticed; probably because I guess the SD card reader on the
> machine I've been testing this on doesn't use MSIs, and I haven't tried
> WiFi yet.
> 
> Perhaps (fwspec.param_count - 2)? It's probably a safer long-term
> assumption that the last two cells will always be leaf IRQ number and type.

Yup, that'd work as well, as long as we make this assumption explicit.

	M.
diff mbox series

Patch

diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
index d03caed51d56..46b7750548a0 100644
--- a/drivers/irqchip/irq-apple-aic.c
+++ b/drivers/irqchip/irq-apple-aic.c
@@ -72,7 +72,8 @@ 
 
 #define AIC_WHOAMI		0x2000
 #define AIC_EVENT		0x2004
-#define AIC_EVENT_TYPE		GENMASK(31, 16)
+#define AIC_EVENT_DIE		GENMASK(31, 24)
+#define AIC_EVENT_TYPE		GENMASK(23, 16)
 #define AIC_EVENT_NUM		GENMASK(15, 0)
 
 #define AIC_EVENT_TYPE_FIQ	0 /* Software use */
@@ -157,6 +158,9 @@ 
 #define MPIDR_CPU			GENMASK(7, 0)
 #define MPIDR_CLUSTER			GENMASK(15, 8)
 
+#define AIC_IRQ_HWIRQ(die, irq)	(FIELD_PREP(AIC_EVENT_DIE, die) | \
+				 FIELD_PREP(AIC_EVENT_TYPE, AIC_EVENT_TYPE_HW) | \
+				 FIELD_PREP(AIC_EVENT_NUM, irq))
 #define AIC_FIQ_HWIRQ(x)	(FIELD_PREP(AIC_EVENT_TYPE, AIC_EVENT_TYPE_FIQ) | \
 				 FIELD_PREP(AIC_EVENT_NUM, x))
 #define AIC_NR_FIQ		4
@@ -188,6 +192,8 @@  struct aic_info {
 	u32 mask_set;
 	u32 mask_clr;
 
+	u32 die_stride;
+
 	/* Features */
 	bool fast_ipi;
 };
@@ -227,6 +233,8 @@  struct aic_irq_chip {
 
 	int nr_irq;
 	int max_irq;
+	int nr_die;
+	int max_die;
 
 	struct aic_info info;
 };
@@ -259,9 +267,10 @@  static void aic_irq_mask(struct irq_data *d)
 	irq_hw_number_t hwirq = irqd_to_hwirq(d);
 	struct aic_irq_chip *ic = irq_data_get_irq_chip_data(d);
 
+	u32 off = FIELD_GET(AIC_EVENT_DIE, hwirq) * ic->info.die_stride;
 	u32 irq = FIELD_GET(AIC_EVENT_NUM, hwirq);
 
-	aic_ic_write(ic, ic->info.mask_set + MASK_REG(irq), MASK_BIT(irq));
+	aic_ic_write(ic, ic->info.mask_set + off + MASK_REG(irq), MASK_BIT(irq));
 }
 
 static void aic_irq_unmask(struct irq_data *d)
@@ -269,9 +278,10 @@  static void aic_irq_unmask(struct irq_data *d)
 	irq_hw_number_t hwirq = irqd_to_hwirq(d);
 	struct aic_irq_chip *ic = irq_data_get_irq_chip_data(d);
 
+	u32 off = FIELD_GET(AIC_EVENT_DIE, hwirq) * ic->info.die_stride;
 	u32 irq = FIELD_GET(AIC_EVENT_NUM, hwirq);
 
-	aic_ic_write(ic, ic->info.mask_clr + MASK_REG(irq), MASK_BIT(irq));
+	aic_ic_write(ic, ic->info.mask_clr + off + MASK_REG(irq), MASK_BIT(irq));
 }
 
 static void aic_irq_eoi(struct irq_data *d)
@@ -535,28 +545,41 @@  static int aic_irq_domain_translate(struct irq_domain *id,
 				    unsigned int *type)
 {
 	struct aic_irq_chip *ic = id->host_data;
+	u32 *args;
+	u32 die = 0;
 
-	if (fwspec->param_count != 3 || !is_of_node(fwspec->fwnode))
+	if (fwspec->param_count < 3 || fwspec->param_count > 4 ||
+	    !is_of_node(fwspec->fwnode))
 		return -EINVAL;
 
+	args = &fwspec->param[1];
+
+	if (fwspec->param_count == 4) {
+		die = args[0];
+		args++;
+	}
+
 	switch (fwspec->param[0]) {
 	case AIC_IRQ:
-		if (fwspec->param[1] >= ic->nr_irq)
+		if (die >= ic->nr_die)
+			return -EINVAL;
+		if (args[0] >= ic->nr_irq)
 			return -EINVAL;
-		*hwirq = (FIELD_PREP(AIC_EVENT_TYPE, AIC_EVENT_TYPE_HW) |
-			  FIELD_PREP(AIC_EVENT_NUM, fwspec->param[1]));
+		*hwirq = AIC_IRQ_HWIRQ(die, args[0]);
 		break;
 	case AIC_FIQ:
-		if (fwspec->param[1] >= AIC_NR_FIQ)
+		if (die != 0)
 			return -EINVAL;
-		*hwirq = AIC_FIQ_HWIRQ(fwspec->param[1]);
+		if (args[0] >= AIC_NR_FIQ)
+			return -EINVAL;
+		*hwirq = AIC_FIQ_HWIRQ(args[0]);
 
 		/*
 		 * In EL1 the non-redirected registers are the guest's,
 		 * not EL2's, so remap the hwirqs to match.
 		 */
 		if (!is_kernel_in_hyp_mode()) {
-			switch (fwspec->param[1]) {
+			switch (args[0]) {
 			case AIC_TMR_GUEST_PHYS:
 				*hwirq = AIC_FIQ_HWIRQ(AIC_TMR_EL0_PHYS);
 				break;
@@ -575,7 +598,7 @@  static int aic_irq_domain_translate(struct irq_domain *id,
 		return -EINVAL;
 	}
 
-	*type = fwspec->param[2] & IRQ_TYPE_SENSE_MASK;
+	*type = args[1] & IRQ_TYPE_SENSE_MASK;
 
 	return 0;
 }
@@ -892,8 +915,8 @@  static struct gic_kvm_info vgic_info __initdata = {
 
 static int __init aic_of_ic_init(struct device_node *node, struct device_node *parent)
 {
-	int i;
-	u32 off;
+	int i, die;
+	u32 off, start_off;
 	void __iomem *regs;
 	struct aic_irq_chip *irqc;
 	const struct of_device_id *match;
@@ -923,8 +946,9 @@  static int __init aic_of_ic_init(struct device_node *node, struct device_node *p
 		info = aic_ic_read(irqc, AIC_INFO);
 		irqc->nr_irq = FIELD_GET(AIC_INFO_NR_IRQ, info);
 		irqc->max_irq = AIC_MAX_IRQ;
+		irqc->nr_die = irqc->max_die = 1;
 
-		off = irqc->info.target_cpu;
+		off = start_off = irqc->info.target_cpu;
 		off += sizeof(u32) * irqc->max_irq; /* TARGET_CPU */
 
 		break;
@@ -941,6 +965,8 @@  static int __init aic_of_ic_init(struct device_node *node, struct device_node *p
 	off += sizeof(u32) * (irqc->max_irq >> 5); /* MASK_CLR */
 	off += sizeof(u32) * (irqc->max_irq >> 5); /* HW_STATE */
 
+	irqc->info.die_stride = off - start_off;
+
 	irqc->hw_domain = irq_domain_create_tree(of_node_to_fwnode(node),
 						 &aic_irq_domain_ops, irqc);
 	if (WARN_ON(!irqc->hw_domain)) {
@@ -961,12 +987,17 @@  static int __init aic_of_ic_init(struct device_node *node, struct device_node *p
 	set_handle_irq(aic_handle_irq);
 	set_handle_fiq(aic_handle_fiq);
 
-	for (i = 0; i < BITS_TO_U32(irqc->nr_irq); i++)
-		aic_ic_write(irqc, irqc->info.mask_set + i * 4, U32_MAX);
-	for (i = 0; i < BITS_TO_U32(irqc->nr_irq); i++)
-		aic_ic_write(irqc, irqc->info.sw_clr + i * 4, U32_MAX);
-	for (i = 0; i < irqc->nr_irq; i++)
-		aic_ic_write(irqc, irqc->info.target_cpu + i * 4, 1);
+	off = 0;
+	for (die = 0; die < irqc->nr_die; die++) {
+		for (i = 0; i < BITS_TO_U32(irqc->nr_irq); i++)
+			aic_ic_write(irqc, irqc->info.mask_set + off + i * 4, U32_MAX);
+		for (i = 0; i < BITS_TO_U32(irqc->nr_irq); i++)
+			aic_ic_write(irqc, irqc->info.sw_clr + off + i * 4, U32_MAX);
+		if (irqc->info.target_cpu)
+			for (i = 0; i < irqc->nr_irq; i++)
+				aic_ic_write(irqc, irqc->info.target_cpu + off + i * 4, 1);
+		off += irqc->info.die_stride;
+	}
 
 	if (!is_kernel_in_hyp_mode())
 		pr_info("Kernel running in EL1, mapping interrupts");
@@ -980,8 +1011,8 @@  static int __init aic_of_ic_init(struct device_node *node, struct device_node *p
 
 	vgic_set_kvm_info(&vgic_info);
 
-	pr_info("Initialized with %d/%d IRQs, %d FIQs, %d vIPIs",
-		irqc->nr_irq, irqc->max_irq, AIC_NR_FIQ, AIC_NR_SWIPI);
+	pr_info("Initialized with %d/%d IRQs * %d/%d die(s), %d FIQs, %d vIPIs",
+		irqc->nr_irq, irqc->max_irq, irqc->nr_die, irqc->max_die, AIC_NR_FIQ, AIC_NR_SWIPI);
 
 	return 0;
 }