[PATCH RFC v2 0/2] irqchip: irq-ti-sci-inta: Add initial IRQ affinity support

[PATCH RFC v2 0/2] irqchip: irq-ti-sci-inta: Add initial IRQ affinity support

[Vignesh Raghavendra]

Interrupt Aggregator (INTA) IP INTA on TI's K3 SoCs convert DMA global
events (MSI like) to wired interrupts (VINT). 64 events can be mapped to
single VINT. Currently driver maps multiple events to single wired
interrupt line.  This makes setting IRQ affinity impossible as
migrating wired interrupt to different core will end up migrating all
events to that core.  And since DMA events related to networking IPs and
other high IRQ load IPs are behind this INTA logic, it creates load on a
single CPU, thus limiting overall performance of these peripherals

This series add ability to reserve have 1:1 mapping for certain events
(typically networking peripherals) using static soc specific data. These
VINTs are reserved at boot. IRQ affinity is handled at parent IRQ chip
(GIC or INTR - GIC).
This will provide consistent userspace irrespective of module
load/unload or probe order.

Based on discussions at [0]

Since RFC v1:
Rewrite patches to reserve few VINTs for direct mapping.

[0] v1:https://lore.kernel.org/linux-arm-kernel/20230122081607.959474-1-vigneshr@ti.com/#r

---
Vignesh Raghavendra (2):
      irqchip: irq-ti-sci-inta: Allocates VINTs at probe
      irqchip: irq-ti-sci-inta: Add direct mapped interrupts

 drivers/irqchip/irq-ti-sci-inta.c | 289 +++++++---
 1 file changed, 212 insertions(+), 77 deletions(-)
---
base-commit: 011eb7443621f49ca1e8cdf9c74c215f25019118
change-id: 20230327-irq-affinity-upstream-a14c85a5e177

Best regards,
-- 
Vignesh

[PATCH RFC v2 1/2] irqchip: irq-ti-sci-inta: Allocates VINTs at probe

[Vignesh Raghavendra]

Simplify driver by allocating all VINTs at probe instead of
allocating on IRQ request. This will allow dedicating few VINTs as
direct IRQs without aggregation in future.

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
---
 drivers/irqchip/irq-ti-sci-inta.c | 144 +++++-----
 1 file changed, 67 insertions(+), 77 deletions(-)

diff --git a/drivers/irqchip/irq-ti-sci-inta.c b/drivers/irqchip/irq-ti-sci-inta.c
index a6ecc53d055c..a3a22edbe0f0 100644
--- a/drivers/irqchip/irq-ti-sci-inta.c
+++ b/drivers/irqchip/irq-ti-sci-inta.c
@@ -96,6 +96,7 @@ struct ti_sci_inta_vint_desc {
  *			device identifier in place of the source device
  *			identifier to let sysfw know where it has to program the
  *			Global Event number.
+ * @num_vints:		Number of VINT lines allocated to this IA.
  */
 struct ti_sci_inta_irq_domain {
 	const struct ti_sci_handle *sci;
@@ -110,6 +111,7 @@ struct ti_sci_inta_irq_domain {
 
 	int unmapped_cnt;
 	u16 *unmapped_dev_ids;
+	int num_vints;
 };
 
 #define to_vint_desc(e, i) container_of(e, struct ti_sci_inta_vint_desc, \
@@ -197,12 +199,12 @@ static int ti_sci_inta_xlate_irq(struct ti_sci_inta_irq_domain *inta,
 }
 
 /**
- * ti_sci_inta_alloc_parent_irq() - Allocate parent irq to Interrupt aggregator
+ * ti_sci_inta_alloc_parent_irqs() - Allocate parent irqs to Interrupt aggregator
  * @domain:	IRQ domain corresponding to Interrupt Aggregator
  *
  * Return 0 if all went well else corresponding error value.
  */
-static struct ti_sci_inta_vint_desc *ti_sci_inta_alloc_parent_irq(struct irq_domain *domain)
+static int ti_sci_inta_alloc_parent_irqs(struct irq_domain *domain)
 {
 	struct ti_sci_inta_irq_domain *inta = domain->host_data;
 	struct ti_sci_inta_vint_desc *vint_desc;
@@ -211,61 +213,69 @@ static struct ti_sci_inta_vint_desc *ti_sci_inta_alloc_parent_irq(struct irq_dom
 	unsigned int parent_virq;
 	int p_hwirq, ret;
 	u16 vint_id;
+	int i;
 
-	vint_id = ti_sci_get_free_resource(inta->vint);
-	if (vint_id == TI_SCI_RESOURCE_NULL)
-		return ERR_PTR(-EINVAL);
-
-	p_hwirq = ti_sci_inta_xlate_irq(inta, vint_id);
-	if (p_hwirq < 0) {
-		ret = p_hwirq;
-		goto free_vint;
-	}
+	parent_node = of_irq_find_parent(dev_of_node(&inta->pdev->dev));
+	parent_fwspec.fwnode = of_node_to_fwnode(parent_node);
 
-	vint_desc = kzalloc(sizeof(*vint_desc), GFP_KERNEL);
-	if (!vint_desc) {
-		ret = -ENOMEM;
-		goto free_vint;
-	}
+	for (i = 0; i < inta->num_vints; i++) {
+		vint_id = ti_sci_get_free_resource(inta->vint);
+		if (vint_id == TI_SCI_RESOURCE_NULL)
+			break;
 
-	vint_desc->domain = domain;
-	vint_desc->vint_id = vint_id;
-	INIT_LIST_HEAD(&vint_desc->list);
+		p_hwirq = ti_sci_inta_xlate_irq(inta, vint_id);
+		if (p_hwirq < 0) {
+			ret = p_hwirq;
+			goto free_vint;
+		}
 
-	parent_node = of_irq_find_parent(dev_of_node(&inta->pdev->dev));
-	parent_fwspec.fwnode = of_node_to_fwnode(parent_node);
+		vint_desc = kzalloc(sizeof(*vint_desc), GFP_KERNEL);
+		if (!vint_desc) {
+			ret = -ENOMEM;
+			goto free_vint;
+		}
 
-	if (of_device_is_compatible(parent_node, "arm,gic-v3")) {
-		/* Parent is GIC */
-		parent_fwspec.param_count = 3;
-		parent_fwspec.param[0] = 0;
-		parent_fwspec.param[1] = p_hwirq - 32;
-		parent_fwspec.param[2] = IRQ_TYPE_LEVEL_HIGH;
-	} else {
-		/* Parent is Interrupt Router */
-		parent_fwspec.param_count = 1;
-		parent_fwspec.param[0] = p_hwirq;
-	}
+		INIT_LIST_HEAD(&vint_desc->list);
+		vint_desc->domain = domain;
+		vint_desc->vint_id = vint_id;
+
+		if (of_device_is_compatible(parent_node, "arm,gic-v3")) {
+			/* Parent is GIC */
+			parent_fwspec.param_count = 3;
+			parent_fwspec.param[0] = 0;
+			parent_fwspec.param[1] = p_hwirq - 32;
+			parent_fwspec.param[2] = IRQ_TYPE_LEVEL_HIGH;
+		} else {
+			/* Parent is Interrupt Router */
+			parent_fwspec.param_count = 1;
+			parent_fwspec.param[0] = p_hwirq;
+		}
 
-	parent_virq = irq_create_fwspec_mapping(&parent_fwspec);
-	if (parent_virq == 0) {
-		dev_err(&inta->pdev->dev, "Parent IRQ allocation failed\n");
-		ret = -EINVAL;
-		goto free_vint_desc;
+		parent_virq = irq_create_fwspec_mapping(&parent_fwspec);
+		if (parent_virq == 0) {
+			dev_dbg(&inta->pdev->dev, "Parent IRQ allocation failed\n");
+			/*
+			 * Certain versions of firmwares don't have
+			 * enough INTR lines allocated to match number
+			 * of VINTs, hence don't error out to maintain
+			 * backward compatibility.
+			 */
+			ret = 0;
+			goto free_vint_desc;
+		}
+		vint_desc->parent_virq = parent_virq;
 
+		list_add_tail(&vint_desc->list, &inta->vint_list);
+		irq_set_chained_handler_and_data(vint_desc->parent_virq,
+						 ti_sci_inta_irq_handler, vint_desc);
 	}
-	vint_desc->parent_virq = parent_virq;
 
-	list_add_tail(&vint_desc->list, &inta->vint_list);
-	irq_set_chained_handler_and_data(vint_desc->parent_virq,
-					 ti_sci_inta_irq_handler, vint_desc);
-
-	return vint_desc;
+	return 0;
 free_vint_desc:
 	kfree(vint_desc);
 free_vint:
 	ti_sci_release_resource(inta->vint, vint_id);
-	return ERR_PTR(ret);
+	return ret;
 }
 
 /**
@@ -335,22 +345,14 @@ static struct ti_sci_inta_event_desc *ti_sci_inta_alloc_irq(struct irq_domain *d
 					       MAX_EVENTS_PER_VINT);
 		if (free_bit != MAX_EVENTS_PER_VINT) {
 			set_bit(free_bit, vint_desc->event_map);
-			goto alloc_event;
+			break;
 		}
 	}
-
-	/* No free bits available. Allocate a new vint */
-	vint_desc = ti_sci_inta_alloc_parent_irq(domain);
-	if (IS_ERR(vint_desc)) {
-		event_desc = ERR_CAST(vint_desc);
+	if (free_bit == MAX_EVENTS_PER_VINT) {
+		event_desc = NULL;
 		goto unlock;
 	}
 
-	free_bit = find_first_zero_bit(vint_desc->event_map,
-				       MAX_EVENTS_PER_VINT);
-	set_bit(free_bit, vint_desc->event_map);
-
-alloc_event:
 	event_desc = ti_sci_inta_alloc_event(vint_desc, free_bit, hwirq);
 	if (IS_ERR(event_desc))
 		clear_bit(free_bit, vint_desc->event_map);
@@ -360,22 +362,6 @@ static struct ti_sci_inta_event_desc *ti_sci_inta_alloc_irq(struct irq_domain *d
 	return event_desc;
 }
 
-/**
- * ti_sci_inta_free_parent_irq() - Free a parent irq to INTA
- * @inta:	Pointer to inta domain.
- * @vint_desc:	Pointer to vint_desc that needs to be freed.
- */
-static void ti_sci_inta_free_parent_irq(struct ti_sci_inta_irq_domain *inta,
-					struct ti_sci_inta_vint_desc *vint_desc)
-{
-	if (find_first_bit(vint_desc->event_map, MAX_EVENTS_PER_VINT) == MAX_EVENTS_PER_VINT) {
-		list_del(&vint_desc->list);
-		ti_sci_release_resource(inta->vint, vint_desc->vint_id);
-		irq_dispose_mapping(vint_desc->parent_virq);
-		kfree(vint_desc);
-	}
-}
-
 /**
  * ti_sci_inta_free_irq() - Free an IRQ within INTA domain
  * @event_desc:	Pointer to event_desc that needs to be freed.
@@ -405,7 +391,6 @@ static void ti_sci_inta_free_irq(struct ti_sci_inta_event_desc *event_desc,
 	event_desc->global_event = TI_SCI_RESOURCE_NULL;
 	event_desc->hwirq = 0;
 
-	ti_sci_inta_free_parent_irq(inta, vint_desc);
 	mutex_unlock(&inta->vint_mutex);
 }
 
@@ -414,8 +399,7 @@ static void ti_sci_inta_free_irq(struct ti_sci_inta_event_desc *event_desc,
  * @data: Pointer to corresponding irq_data
  *
  * Note: This is the core api where the actual allocation happens for input
- *	 hwirq. This allocation involves creating a parent irq for vint.
- *	 If this is done in irq_domain_ops.alloc() then a deadlock is reached
+ *	 hwirq. If this is done in irq_domain_ops.alloc() then a deadlock is reached
  *	 for allocation. So this allocation is being done in request_resources()
  *
  * Return: 0 if all went well else corresponding error.
@@ -437,8 +421,7 @@ static int ti_sci_inta_request_resources(struct irq_data *data)
  * ti_sci_inta_release_resources - Release resources for input irq
  * @data: Pointer to corresponding irq_data
  *
- * Note: Corresponding to request_resources(), all the unmapping and deletion
- *	 of parent vint irqs happens in this api.
+ * Note: Corresponding to request_resources(), release event mapping
  */
 static void ti_sci_inta_release_resources(struct irq_data *data)
 {
@@ -701,8 +684,9 @@ static int ti_sci_inta_irq_domain_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	inta->num_vints = ti_sci_get_num_resources(inta->vint);
 	domain = irq_domain_add_linear(dev_of_node(dev),
-				       ti_sci_get_num_resources(inta->vint),
+				       inta->num_vints,
 				       &ti_sci_inta_irq_domain_ops, inta);
 	if (!domain) {
 		dev_err(dev, "Failed to allocate IRQ domain\n");
@@ -721,6 +705,12 @@ static int ti_sci_inta_irq_domain_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&inta->vint_list);
 	mutex_init(&inta->vint_mutex);
 
+	ret = ti_sci_inta_alloc_parent_irqs(domain);
+	if (ret) {
+		irq_domain_remove(msi_domain);
+		irq_domain_remove(domain);
+	}
+
 	dev_info(dev, "Interrupt Aggregator domain %d created\n", inta->ti_sci_id);
 
 	return 0;

-- 
2.40.0

Re: [PATCH RFC v2 1/2] irqchip: irq-ti-sci-inta: Allocates VINTs at probe

[Marc Zyngier]

On Mon, 27 Mar 2023 16:04:26 +0100,
Vignesh Raghavendra <vigneshr@ti.com> wrote:
> 
> Simplify driver by allocating all VINTs at probe instead of
> allocating on IRQ request. This will allow dedicating few VINTs as
> direct IRQs without aggregation in future.

I think this is going in the wrong direction. Eager allocation is
wasting memory, slowing down boot, and in general a bad idea.

Why can't you just pre-allocate *one* interrupt that serves as a
chained handler for everything, and then use the rest of the interrupt
space for "direct" interrupts?

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH RFC v2 1/2] irqchip: irq-ti-sci-inta: Allocates VINTs at probe

[Raghavendra, Vignesh]

On 4/8/2023 4:09 PM, Marc Zyngier wrote:
> On Mon, 27 Mar 2023 16:04:26 +0100,
> Vignesh Raghavendra <vigneshr@ti.com> wrote:
>>
>> Simplify driver by allocating all VINTs at probe instead of
>> allocating on IRQ request. This will allow dedicating few VINTs as
>> direct IRQs without aggregation in future.
> 
> I think this is going in the wrong direction. Eager allocation is
> wasting memory, slowing down boot, and in general a bad idea.
> 
> Why can't you just pre-allocate *one* interrupt that serves as a
> chained handler for everything, and then use the rest of the interrupt
> space for "direct" interrupts?
> 


I may need more than one (at least 2 or 3), but I get the point.
Will rework accordingly.

Regards
Vignesh

[PATCH RFC v2 2/2] irqchip: irq-ti-sci-inta: Add direct mapped interrupts

[Vignesh Raghavendra]

Certain high throughput peripherals such as ethernet etc may need
dedicated IRQ lines so the IRQ load can be balanced across cores using
IRQ affinity controls. Current driver aggregates multiple events/IRQ to
single wired IRQ line/VINT thus making it impossible to migrate events
selectively.
In order to overcome this limitation, reserve a set of VINTs as direct
interrupts and map them to events that are known to generate high IRQ
load. Use SoC specific table to determine such events.

This allows affinity management at parent irqchip level (GIC or
ti-sci-intr + GIC).

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
---
 drivers/irqchip/irq-ti-sci-inta.c | 149 +++++++++-
 1 file changed, 147 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-ti-sci-inta.c b/drivers/irqchip/irq-ti-sci-inta.c
index a3a22edbe0f0..db098bd95edc 100644
--- a/drivers/irqchip/irq-ti-sci-inta.c
+++ b/drivers/irqchip/irq-ti-sci-inta.c
@@ -21,6 +21,7 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/soc/ti/ti_sci_inta_msi.h>
 #include <linux/soc/ti/ti_sci_protocol.h>
+#include <linux/sys_soc.h>
 #include <asm-generic/msi.h>
 
 #define TI_SCI_DEV_ID_MASK	0xffff
@@ -40,6 +41,20 @@
 #define VINT_STATUS_OFFSET	0x18
 #define VINT_STATUS_MASKED_OFFSET	0x20
 
+#define DEV_DMASS0_INTAGGR_0 28
+#define DEV_DMASS0_PKTDMA_0 30
+
+/**
+ * struct ti_sci_inta_soc_data - Description of SoC specific data
+ * @events_list:		Pointer to array of events requiring direct IRQ
+ *                              mapping
+ * @events_list_size:		size of event_list array
+ */
+struct ti_sci_inta_soc_data {
+	unsigned int *events_list;
+	int events_list_size;
+};
+
 /**
  * struct ti_sci_inta_event_desc - Description of an event coming to
  *				   Interrupt Aggregator. This serves
@@ -64,6 +79,7 @@ struct ti_sci_inta_event_desc {
  * @events:		Array of event descriptors assigned to this vint.
  * @parent_virq:	Linux IRQ number that gets attached to parent
  * @vint_id:		TISCI vint ID
+ * @reserved:		Indicate if this VINT is reserved for direct mapping
  */
 struct ti_sci_inta_vint_desc {
 	struct irq_domain *domain;
@@ -72,6 +88,7 @@ struct ti_sci_inta_vint_desc {
 	struct ti_sci_inta_event_desc events[MAX_EVENTS_PER_VINT];
 	unsigned int parent_virq;
 	u16 vint_id;
+	bool reserved;
 };
 
 /**
@@ -81,6 +98,8 @@ struct ti_sci_inta_vint_desc {
  * @vint:		TISCI resource pointer representing IA interrupts.
  * @global_event:	TISCI resource pointer representing global events.
  * @vint_list:		List of the vints active in the system
+ * @resv_vint_list:	List of the direct vints active in the system.
+ * @resv_events:	Pointer to list of direct events
  * @vint_mutex:		Mutex to protect vint_list
  * @base:		Base address of the memory mapped IO registers
  * @pdev:		Pointer to platform device.
@@ -97,12 +116,15 @@ struct ti_sci_inta_vint_desc {
  *			identifier to let sysfw know where it has to program the
  *			Global Event number.
  * @num_vints:		Number of VINT lines allocated to this IA.
+ * @num_resv_vints:	Number of VINT lines reserved for direct mapping.
  */
 struct ti_sci_inta_irq_domain {
 	const struct ti_sci_handle *sci;
 	struct ti_sci_resource *vint;
 	struct ti_sci_resource *global_event;
 	struct list_head vint_list;
+	struct list_head resv_vint_list;
+	unsigned int *resv_events;
 	/* Mutex to protect vint list */
 	struct mutex vint_mutex;
 	void __iomem *base;
@@ -112,6 +134,7 @@ struct ti_sci_inta_irq_domain {
 	int unmapped_cnt;
 	u16 *unmapped_dev_ids;
 	int num_vints;
+	int num_resv_vints;
 };
 
 #define to_vint_desc(e, i) container_of(e, struct ti_sci_inta_vint_desc, \
@@ -265,7 +288,11 @@ static int ti_sci_inta_alloc_parent_irqs(struct irq_domain *domain)
 		}
 		vint_desc->parent_virq = parent_virq;
 
-		list_add_tail(&vint_desc->list, &inta->vint_list);
+		if (i < inta->num_resv_vints)
+			list_add_tail(&vint_desc->list, &inta->resv_vint_list);
+		else
+			list_add_tail(&vint_desc->list, &inta->vint_list);
+
 		irq_set_chained_handler_and_data(vint_desc->parent_virq,
 						 ti_sci_inta_irq_handler, vint_desc);
 	}
@@ -390,8 +417,58 @@ static void ti_sci_inta_free_irq(struct ti_sci_inta_event_desc *event_desc,
 	ti_sci_release_resource(inta->global_event, event_desc->global_event);
 	event_desc->global_event = TI_SCI_RESOURCE_NULL;
 	event_desc->hwirq = 0;
+	vint_desc->reserved = false;
+}
+
+/**
+ * ti_sci_inta_alloc_direct_irq - Allocate non-aggregated events/IRQs
+ * @data: Pointer to corresponding irq_data
+ *
+ * Returns allocated event_desc, ERR_PTR otherwise.
+ */
+static struct ti_sci_inta_event_desc *ti_sci_inta_alloc_direct_irq(struct irq_data *data)
+{
+	struct ti_sci_inta_irq_domain *inta = data->domain->host_data;
+	struct ti_sci_inta_event_desc *event_desc = NULL;
+	struct ti_sci_inta_vint_desc *vint_desc;
+
+	mutex_lock(&inta->vint_mutex);
+	list_for_each_entry(vint_desc, &inta->resv_vint_list, list) {
+		if (vint_desc->reserved)
+			continue;
+
+		vint_desc->reserved = true;
+		event_desc = ti_sci_inta_alloc_event(vint_desc, 0, data->hwirq);
+		if (IS_ERR(event_desc))
+			goto unlock;
 
+		data->parent_data = irq_get_irq_data(vint_desc->parent_virq);
+		break;
+	}
+unlock:
 	mutex_unlock(&inta->vint_mutex);
+
+	return event_desc;
+}
+
+/**
+ * ti_sci_inta_direct_irq - Allocate non-aggregated events/IRQs
+ * @data: Pointer to corresponding irq_data
+ *
+ * Returns true if this IRQ is to be direct mapped on given platform,
+ * false otherwise.
+ */
+static bool ti_sci_inta_direct_irq(struct irq_data *data)
+{
+	struct ti_sci_inta_irq_domain *inta = data->domain->host_data;
+	int i;
+
+	for (i = 0; i < inta->num_resv_vints; i++) {
+		if (data->hwirq == inta->resv_events[i])
+			return true;
+	}
+
+	return false;
 }
 
 /**
@@ -408,7 +485,11 @@ static int ti_sci_inta_request_resources(struct irq_data *data)
 {
 	struct ti_sci_inta_event_desc *event_desc;
 
-	event_desc = ti_sci_inta_alloc_irq(data->domain, data->hwirq);
+	if (ti_sci_inta_direct_irq(data))
+		event_desc = ti_sci_inta_alloc_direct_irq(data);
+	else
+		event_desc = ti_sci_inta_alloc_irq(data->domain, data->hwirq);
+
 	if (IS_ERR(event_desc))
 		return PTR_ERR(event_desc);
 
@@ -485,6 +566,15 @@ static void ti_sci_inta_ack_irq(struct irq_data *data)
 static int ti_sci_inta_set_affinity(struct irq_data *d,
 				    const struct cpumask *mask_val, bool force)
 {
+	int ret;
+
+	if (d->parent_data) {
+		ret =  irq_chip_set_affinity_parent(d, mask_val, force);
+		irq_data_update_effective_affinity(d, mask_val);
+
+		return ret;
+	}
+
 	return -EINVAL;
 }
 
@@ -627,9 +717,54 @@ static int ti_sci_inta_get_unmapped_sources(struct ti_sci_inta_irq_domain *inta)
 	return 0;
 }
 
+static unsigned int ti_sci_inta_direct_events_am62x[] = {
+	/* CPSW etherenti DMA events */
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4627),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4635),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4643),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4651),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4659),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4667),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4675),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4683),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 5651),
+};
+
+static struct ti_sci_inta_soc_data soc_data_am62x = {
+	.events_list = ti_sci_inta_direct_events_am62x,
+	.events_list_size = ARRAY_SIZE(ti_sci_inta_direct_events_am62x),
+};
+
+static unsigned int ti_sci_inta_direct_events_am64x[] = {
+	/* CPSW etherenti DMA events */
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4624),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4632),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4640),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4648),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4656),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4664),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4672),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4680),
+	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 5648),
+};
+
+static struct ti_sci_inta_soc_data soc_data_am64x = {
+	.events_list = ti_sci_inta_direct_events_am64x,
+	.events_list_size = ARRAY_SIZE(ti_sci_inta_direct_events_am64x),
+};
+
+static const struct soc_device_attribute k3_soc_devices[] = {
+	{ .family = "AM64X", .data = &soc_data_am64x },
+	{ .family = "AM62X", .data = &soc_data_am62x },
+	{ .family = "AM62AX", .data = &soc_data_am62x },
+	{ /* sentinel */ }
+};
+
 static int ti_sci_inta_irq_domain_probe(struct platform_device *pdev)
 {
 	struct irq_domain *parent_domain, *domain, *msi_domain;
+	const struct ti_sci_inta_soc_data *soc_data;
+	const struct soc_device_attribute *soc;
 	struct device_node *parent_node, *node;
 	struct ti_sci_inta_irq_domain *inta;
 	struct device *dev = &pdev->dev;
@@ -703,8 +838,18 @@ static int ti_sci_inta_irq_domain_probe(struct platform_device *pdev)
 	}
 
 	INIT_LIST_HEAD(&inta->vint_list);
+	INIT_LIST_HEAD(&inta->resv_vint_list);
 	mutex_init(&inta->vint_mutex);
 
+	soc = soc_device_match(k3_soc_devices);
+	if (soc) {
+		soc_data = soc->data;
+		if (inta->num_vints > soc_data->events_list_size) {
+			inta->num_resv_vints = soc_data->events_list_size;
+			inta->resv_events = soc_data->events_list;
+		}
+	}
+
 	ret = ti_sci_inta_alloc_parent_irqs(domain);
 	if (ret) {
 		irq_domain_remove(msi_domain);

-- 
2.40.0

Re: [PATCH RFC v2 2/2] irqchip: irq-ti-sci-inta: Add direct mapped interrupts

[Marc Zyngier]

On Mon, 27 Mar 2023 16:04:27 +0100,
Vignesh Raghavendra <vigneshr@ti.com> wrote:
> 
> Certain high throughput peripherals such as ethernet etc may need
> dedicated IRQ lines so the IRQ load can be balanced across cores using
> IRQ affinity controls. Current driver aggregates multiple events/IRQ to
> single wired IRQ line/VINT thus making it impossible to migrate events
> selectively.
> In order to overcome this limitation, reserve a set of VINTs as direct
> interrupts and map them to events that are known to generate high IRQ
> load. Use SoC specific table to determine such events.
> 
> This allows affinity management at parent irqchip level (GIC or
> ti-sci-intr + GIC).
> 
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
>  drivers/irqchip/irq-ti-sci-inta.c | 149 +++++++++-
>  1 file changed, 147 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-ti-sci-inta.c b/drivers/irqchip/irq-ti-sci-inta.c
> index a3a22edbe0f0..db098bd95edc 100644
> --- a/drivers/irqchip/irq-ti-sci-inta.c
> +++ b/drivers/irqchip/irq-ti-sci-inta.c
> @@ -21,6 +21,7 @@
>  #include <linux/irqchip/chained_irq.h>
>  #include <linux/soc/ti/ti_sci_inta_msi.h>
>  #include <linux/soc/ti/ti_sci_protocol.h>
> +#include <linux/sys_soc.h>
>  #include <asm-generic/msi.h>
>  
>  #define TI_SCI_DEV_ID_MASK	0xffff
> @@ -40,6 +41,20 @@
>  #define VINT_STATUS_OFFSET	0x18
>  #define VINT_STATUS_MASKED_OFFSET	0x20
>  
> +#define DEV_DMASS0_INTAGGR_0 28
> +#define DEV_DMASS0_PKTDMA_0 30
> +
> +/**
> + * struct ti_sci_inta_soc_data - Description of SoC specific data
> + * @events_list:		Pointer to array of events requiring direct IRQ
> + *                              mapping
> + * @events_list_size:		size of event_list array
> + */
> +struct ti_sci_inta_soc_data {
> +	unsigned int *events_list;
> +	int events_list_size;
> +};
> +
>  /**
>   * struct ti_sci_inta_event_desc - Description of an event coming to
>   *				   Interrupt Aggregator. This serves
> @@ -64,6 +79,7 @@ struct ti_sci_inta_event_desc {
>   * @events:		Array of event descriptors assigned to this vint.
>   * @parent_virq:	Linux IRQ number that gets attached to parent
>   * @vint_id:		TISCI vint ID
> + * @reserved:		Indicate if this VINT is reserved for direct mapping
>   */
>  struct ti_sci_inta_vint_desc {
>  	struct irq_domain *domain;
> @@ -72,6 +88,7 @@ struct ti_sci_inta_vint_desc {
>  	struct ti_sci_inta_event_desc events[MAX_EVENTS_PER_VINT];
>  	unsigned int parent_virq;
>  	u16 vint_id;
> +	bool reserved;
>  };
>  
>  /**
> @@ -81,6 +98,8 @@ struct ti_sci_inta_vint_desc {
>   * @vint:		TISCI resource pointer representing IA interrupts.
>   * @global_event:	TISCI resource pointer representing global events.
>   * @vint_list:		List of the vints active in the system
> + * @resv_vint_list:	List of the direct vints active in the system.
> + * @resv_events:	Pointer to list of direct events
>   * @vint_mutex:		Mutex to protect vint_list
>   * @base:		Base address of the memory mapped IO registers
>   * @pdev:		Pointer to platform device.
> @@ -97,12 +116,15 @@ struct ti_sci_inta_vint_desc {
>   *			identifier to let sysfw know where it has to program the
>   *			Global Event number.
>   * @num_vints:		Number of VINT lines allocated to this IA.
> + * @num_resv_vints:	Number of VINT lines reserved for direct mapping.
>   */
>  struct ti_sci_inta_irq_domain {
>  	const struct ti_sci_handle *sci;
>  	struct ti_sci_resource *vint;
>  	struct ti_sci_resource *global_event;
>  	struct list_head vint_list;
> +	struct list_head resv_vint_list;
> +	unsigned int *resv_events;
>  	/* Mutex to protect vint list */
>  	struct mutex vint_mutex;
>  	void __iomem *base;
> @@ -112,6 +134,7 @@ struct ti_sci_inta_irq_domain {
>  	int unmapped_cnt;
>  	u16 *unmapped_dev_ids;
>  	int num_vints;
> +	int num_resv_vints;
>  };
>  
>  #define to_vint_desc(e, i) container_of(e, struct ti_sci_inta_vint_desc, \
> @@ -265,7 +288,11 @@ static int ti_sci_inta_alloc_parent_irqs(struct irq_domain *domain)
>  		}
>  		vint_desc->parent_virq = parent_virq;
>  
> -		list_add_tail(&vint_desc->list, &inta->vint_list);
> +		if (i < inta->num_resv_vints)
> +			list_add_tail(&vint_desc->list, &inta->resv_vint_list);
> +		else
> +			list_add_tail(&vint_desc->list, &inta->vint_list);
> +
>  		irq_set_chained_handler_and_data(vint_desc->parent_virq,
>  						 ti_sci_inta_irq_handler, vint_desc);
>  	}
> @@ -390,8 +417,58 @@ static void ti_sci_inta_free_irq(struct ti_sci_inta_event_desc *event_desc,
>  	ti_sci_release_resource(inta->global_event, event_desc->global_event);
>  	event_desc->global_event = TI_SCI_RESOURCE_NULL;
>  	event_desc->hwirq = 0;
> +	vint_desc->reserved = false;
> +}
> +
> +/**
> + * ti_sci_inta_alloc_direct_irq - Allocate non-aggregated events/IRQs
> + * @data: Pointer to corresponding irq_data
> + *
> + * Returns allocated event_desc, ERR_PTR otherwise.
> + */
> +static struct ti_sci_inta_event_desc *ti_sci_inta_alloc_direct_irq(struct irq_data *data)
> +{
> +	struct ti_sci_inta_irq_domain *inta = data->domain->host_data;
> +	struct ti_sci_inta_event_desc *event_desc = NULL;
> +	struct ti_sci_inta_vint_desc *vint_desc;
> +
> +	mutex_lock(&inta->vint_mutex);
> +	list_for_each_entry(vint_desc, &inta->resv_vint_list, list) {
> +		if (vint_desc->reserved)
> +			continue;
> +
> +		vint_desc->reserved = true;
> +		event_desc = ti_sci_inta_alloc_event(vint_desc, 0, data->hwirq);
> +		if (IS_ERR(event_desc))
> +			goto unlock;
>  
> +		data->parent_data = irq_get_irq_data(vint_desc->parent_virq);
> +		break;
> +	}
> +unlock:
>  	mutex_unlock(&inta->vint_mutex);
> +
> +	return event_desc;
> +}
> +
> +/**
> + * ti_sci_inta_direct_irq - Allocate non-aggregated events/IRQs
> + * @data: Pointer to corresponding irq_data
> + *
> + * Returns true if this IRQ is to be direct mapped on given platform,
> + * false otherwise.
> + */
> +static bool ti_sci_inta_direct_irq(struct irq_data *data)
> +{
> +	struct ti_sci_inta_irq_domain *inta = data->domain->host_data;
> +	int i;
> +
> +	for (i = 0; i < inta->num_resv_vints; i++) {
> +		if (data->hwirq == inta->resv_events[i])
> +			return true;
> +	}
> +
> +	return false;
>  }
>  
>  /**
> @@ -408,7 +485,11 @@ static int ti_sci_inta_request_resources(struct irq_data *data)
>  {
>  	struct ti_sci_inta_event_desc *event_desc;
>  
> -	event_desc = ti_sci_inta_alloc_irq(data->domain, data->hwirq);
> +	if (ti_sci_inta_direct_irq(data))
> +		event_desc = ti_sci_inta_alloc_direct_irq(data);
> +	else
> +		event_desc = ti_sci_inta_alloc_irq(data->domain, data->hwirq);
> +
>  	if (IS_ERR(event_desc))
>  		return PTR_ERR(event_desc);
>  
> @@ -485,6 +566,15 @@ static void ti_sci_inta_ack_irq(struct irq_data *data)
>  static int ti_sci_inta_set_affinity(struct irq_data *d,
>  				    const struct cpumask *mask_val, bool force)
>  {
> +	int ret;
> +
> +	if (d->parent_data) {
> +		ret =  irq_chip_set_affinity_parent(d, mask_val, force);
> +		irq_data_update_effective_affinity(d, mask_val);
> +
> +		return ret;
> +	}
> +
>  	return -EINVAL;
>  }
>  
> @@ -627,9 +717,54 @@ static int ti_sci_inta_get_unmapped_sources(struct ti_sci_inta_irq_domain *inta)
>  	return 0;
>  }
>  
> +static unsigned int ti_sci_inta_direct_events_am62x[] = {
> +	/* CPSW etherenti DMA events */
> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4627),
> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4635),
> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4643),
> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4651),
> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4659),
> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4667),
> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4675),
> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4683),
> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 5651),
> +};
> +
> +static struct ti_sci_inta_soc_data soc_data_am62x = {
> +	.events_list = ti_sci_inta_direct_events_am62x,
> +	.events_list_size = ARRAY_SIZE(ti_sci_inta_direct_events_am62x),
> +};

I don't think these tables belong in a driver, and they are bound to
grow without any obvious limits. You have firmware tables that can
express these things. Surely they can be put to a good use.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH RFC v2 2/2] irqchip: irq-ti-sci-inta: Add direct mapped interrupts

[Raghavendra, Vignesh]

Hi,

On 4/8/2023 4:10 PM, Marc Zyngier wrote:
>> +static unsigned int ti_sci_inta_direct_events_am62x[] = {
>> +	/* CPSW etherenti DMA events */
>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4627),
>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4635),
>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4643),
>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4651),
>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4659),
>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4667),
>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4675),
>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4683),
>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 5651),
>> +};
>> +
>> +static struct ti_sci_inta_soc_data soc_data_am62x = {
>> +	.events_list = ti_sci_inta_direct_events_am62x,
>> +	.events_list_size = ARRAY_SIZE(ti_sci_inta_direct_events_am62x),
>> +};
> I don't think these tables belong in a driver, and they are bound to
> grow without any obvious limits. 

Fair point.

> You have firmware tables that can express these things. Surely they can be put to a good use.

By firmware tables you mean device tree?

Regards
Vignesh

Re: [PATCH RFC v2 2/2] irqchip: irq-ti-sci-inta: Add direct mapped interrupts

[Marc Zyngier]

On 2023-04-08 12:27, Raghavendra, Vignesh wrote:
> Hi,
> 
> On 4/8/2023 4:10 PM, Marc Zyngier wrote:
>>> +static unsigned int ti_sci_inta_direct_events_am62x[] = {
>>> +	/* CPSW etherenti DMA events */
>>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4627),
>>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4635),
>>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4643),
>>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4651),
>>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4659),
>>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4667),
>>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4675),
>>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 4683),
>>> +	TO_HWIRQ(DEV_DMASS0_PKTDMA_0, 5651),
>>> +};
>>> +
>>> +static struct ti_sci_inta_soc_data soc_data_am62x = {
>>> +	.events_list = ti_sci_inta_direct_events_am62x,
>>> +	.events_list_size = ARRAY_SIZE(ti_sci_inta_direct_events_am62x),
>>> +};
>> I don't think these tables belong in a driver, and they are bound to
>> grow without any obvious limits.
> 
> Fair point.
> 
>> You have firmware tables that can express these things. Surely they 
>> can be put to a good use.
> 
> By firmware tables you mean device tree?

That, or any other machine-specific mean. From what I get of these
systems, they already make heavy use of some runtime firmware to get
things configured. That side could also provide setup information.

I don't mind either way, as long as we don't end-up with forever
growing in-kernel tables that are just board files in disguise.

           M.
-- 
Jazz is not dead. It just smells funny...