Re: [PATCH v3 9/9] dmaengine: dw-edma: Defer channel IRQ handling to workqueue

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Jul 16, 2026 at 02:57:40AM +0900, Koichiro Den wrote:
> On some SoCs (e.g. R-Car S4) the endpoint-side eDMA raises a single
> fixed SPI that is hardwired to CPU0 and covers every read and write
> channel. Handling channel events directly in that hard IRQ context
> serializes the completion processing of all channels on one CPU:
> descriptor recycling and refill, client callbacks (the vchan tasklet
> runs on the scheduling CPU) and the doorbell writes all funnel through
> CPU0, while the handler additionally spins on each channel's vc.lock.
> Especially under heavy multichannel load, this contention becomes a
> performance bottleneck.
>
> Keep the hard IRQ handler minimal: clear the status, dispatch channel
> events, and defer per-channel processing to work items. A work item per
> channel preserves ordering while allowing different channels to run in
> parallel on any CPU.
>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v3:
>   - Clear irq_pending in dw_edma_device_synchronize(). (Sashiko)
>   - Drop Frank's Reviewed-by tag due to this change.
>   - Refine the commit message and source comments.
>
>  drivers/dma/dw-edma/dw-edma-core.c | 69 +++++++++++++++++++++++++++---
>  drivers/dma/dw-edma/dw-edma-core.h | 11 +++++
>  2 files changed, 75 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 2369b4c4630b..4ee7f14c3a82 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -30,6 +30,11 @@ struct dw_edma_desc *vd2dw_edma_desc(struct virt_dma_desc *vd)
>  	return container_of(vd, struct dw_edma_desc, vd);
>  }
>
> +enum dw_edma_irq_event {
> +	DW_EDMA_IRQ_DONE	= BIT(0),
> +	DW_EDMA_IRQ_ABORT	= BIT(1),
> +};
> +
>  static inline
>  u64 dw_edma_get_pci_address(struct dw_edma_chan *chan, phys_addr_t cpu_addr)
>  {
> @@ -759,6 +764,39 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
>  	spin_unlock_irqrestore(&chan->vc.lock, flags);
>  }
>
> +static void dw_edma_irq_work(struct work_struct *work)
> +{
> +	struct dw_edma_chan *chan = container_of(work, struct dw_edma_chan,
> +						 irq_work);
> +	unsigned int events;
> +
> +	do {
> +		events = atomic_xchg(&chan->irq_pending, 0);
> +
> +		if (events & DW_EDMA_IRQ_DONE)
> +			dw_edma_done_interrupt(chan);
> +		if (events & DW_EDMA_IRQ_ABORT)
> +			dw_edma_abort_interrupt(chan);
> +	} while (atomic_read(&chan->irq_pending));
> +}
> +
> +static void dw_edma_queue_irq_work(struct dw_edma_chan *chan,
> +				   enum dw_edma_irq_event event)
> +{
> +	atomic_or(event, &chan->irq_pending);
> +	queue_work(chan->dw->wq, &chan->irq_work);
> +}
> +
> +static void dw_edma_done_interrupt_deferred(struct dw_edma_chan *chan)
> +{
> +	dw_edma_queue_irq_work(chan, DW_EDMA_IRQ_DONE);
> +}
> +
> +static void dw_edma_abort_interrupt_deferred(struct dw_edma_chan *chan)
> +{
> +	dw_edma_queue_irq_work(chan, DW_EDMA_IRQ_ABORT);
> +}
> +
>  static void dw_edma_emul_irq_ack(struct irq_data *d)
>  {
>  	struct dw_edma *dw = irq_data_get_irq_chip_data(d);
> @@ -853,8 +891,8 @@ static inline irqreturn_t dw_edma_interrupt_write_inner(int irq, void *data)
>  	struct dw_edma_irq *dw_irq = data;
>
>  	return dw_edma_core_handle_int(dw_irq, EDMA_DIR_WRITE,
> -				       dw_edma_done_interrupt,
> -				       dw_edma_abort_interrupt);
> +				       dw_edma_done_interrupt_deferred,
> +				       dw_edma_abort_interrupt_deferred);
>  }
>
>  static inline irqreturn_t dw_edma_interrupt_read_inner(int irq, void *data)
> @@ -862,8 +900,8 @@ static inline irqreturn_t dw_edma_interrupt_read_inner(int irq, void *data)
>  	struct dw_edma_irq *dw_irq = data;
>
>  	return dw_edma_core_handle_int(dw_irq, EDMA_DIR_READ,
> -				       dw_edma_done_interrupt,
> -				       dw_edma_abort_interrupt);
> +				       dw_edma_done_interrupt_deferred,
> +				       dw_edma_abort_interrupt_deferred);
>  }
>
>  static inline irqreturn_t dw_edma_interrupt_write(int irq, void *data)
> @@ -940,6 +978,8 @@ static void dw_edma_device_synchronize(struct dma_chan *dchan)
>  	struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>
>  	dw_edma_wait_termination(dchan);
> +	cancel_work_sync(&chan->irq_work);
> +	atomic_set(&chan->irq_pending, 0);
>  	vchan_synchronize(&chan->vc);
>  }
>
> @@ -982,6 +1022,8 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
>  		chan->configured = false;
>  		chan->request = EDMA_REQ_NONE;
>  		chan->status = EDMA_ST_IDLE;
> +		INIT_WORK(&chan->irq_work, dw_edma_irq_work);
> +		atomic_set(&chan->irq_pending, 0);
>
>  		if (chan->dir == EDMA_DIR_WRITE)
>  			chan->ll_max = (chip->ll_region_wr[chan->id].sz / EDMA_LL_SZ);
> @@ -1195,10 +1237,21 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>  	/* Disable eDMA, only to establish the ideal initial conditions */
>  	dw_edma_core_off(dw);
>
> +	/*
> +	 * Deferred IRQ works are queued from the hard IRQ handlers, so the
> +	 * workqueue must exist before any IRQ is requested.
> +	 */
> +	dw->wq = alloc_workqueue("dw-edma:%s", WQ_UNBOUND | WQ_HIGHPRI, 0,
> +				 dev_name(chip->dev));
> +	if (!dw->wq)
> +		return -ENOMEM;
> +
>  	/* Request IRQs */
>  	err = dw_edma_irq_request(dw, &wr_alloc, &rd_alloc);
> -	if (err)
> +	if (err) {
> +		destroy_workqueue(dw->wq);
>  		return err;
> +	}
>
>  	/* Allocate a dedicated virtual IRQ for interrupt-emulation doorbells */
>  	err = dw_edma_emul_irq_alloc(dw);
> @@ -1221,6 +1274,7 @@ int dw_edma_probe(struct dw_edma_chip *chip)
>  	for (i = (dw->nr_irqs - 1); i >= 0; i--)
>  		free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]);
>  	dw_edma_emul_irq_free(dw);
> +	destroy_workqueue(dw->wq);
>
>  	return err;
>  }
> @@ -1245,6 +1299,11 @@ int dw_edma_remove(struct dw_edma_chip *chip)
>  		free_irq(chip->ops->irq_vector(dev, i), &dw->irq[i]);
>  	dw_edma_emul_irq_free(dw);
>
> +	for (i = 0; i < dw->wr_ch_cnt + dw->rd_ch_cnt; i++)
> +		cancel_work_sync(&dw->chan[i].irq_work);
> +
> +	destroy_workqueue(dw->wq);
> +
>  	/* Deregister eDMA device */
>  	dma_async_device_unregister(&dw->dma);
>  	list_for_each_entry_safe(chan, _chan, &dw->dma.channels,
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 6474cacf7195..7f6301572fa4 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -9,8 +9,10 @@
>  #ifndef _DW_EDMA_CORE_H
>  #define _DW_EDMA_CORE_H
>
> +#include <linux/atomic.h>
>  #include <linux/msi.h>
>  #include <linux/dma/edma.h>
> +#include <linux/workqueue.h>
>
>  #include "../virt-dma.h"
>
> @@ -87,6 +89,9 @@ struct dw_edma_chan {
>
>  	struct dma_slave_config		config;
>  	bool				non_ll;
> +
> +	struct work_struct		irq_work;
> +	atomic_t			irq_pending;
>  };
>
>  struct dw_edma_irq {
> @@ -109,6 +114,12 @@ struct dw_edma {
>
>  	struct dw_edma_chan		*chan;
>
> +	/*
> +	 * WQ_HIGHPRI keeps completion processing responsive under heavy load;
> +	 * WQ_UNBOUND lets different channels run on different CPUs.
> +	 */
> +	struct workqueue_struct		*wq;
> +
>  	raw_spinlock_t			lock;		/* Protect v0 shared registers */
>
>  	struct dw_edma_chip             *chip;
> --
> 2.51.0
>