Re: [PATCH RFC v2 2/2] irqchip: irq-ti-sci-inta: Add direct mapped interrupts
From: Marc Zyngier
Date: Sat Apr 08 2023 - 06:43:36 EST
On Mon, 27 Mar 2023 16:04:27 +0100,
Vignesh Raghavendra <vigneshr@xxxxxx> 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@xxxxxx>
> ---
> 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.