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

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

On Fri, Jul 10, 2026 at 05:09:03PM +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 multi-channels heavy load, the contention becomes a
> performance bottleneck.
>
> Keep the hard IRQ handler minimal and have it just clear the status and
> dispatch, defer the per-channel processing to work items. A work item
> per channel preserves per-channel ordering while letting the channels be
> processed in parallel on any CPU.
>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---

It looks good, allen try convert bh tasklet to workqueue.
https://lore.kernel.org/dmaengine/CAOMdWSLVk136RzEyiN76zqk65VLwYms0hCDi5Kww9FQppie12A@mail.gmail.com/

Please double check if it can apply to dw-edma?

Frank

> Changes in v2:
>   - New patch in v2, posted as part of this preparation series.
>
>  drivers/dma/dw-edma/dw-edma-core.c | 73 ++++++++++++++++++++++++++++--
>  drivers/dma/dw-edma/dw-edma-core.h | 12 +++++
>  2 files changed, 80 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 5664421c6f15..704d8f9746e8 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -31,6 +31,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)
>  {
> @@ -748,6 +753,44 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
>  	chan->status = EDMA_ST_IDLE;
>  }
>
> +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);
> +		/*
> +		 * Correctness does not depend on this loop: queue_work() can
> +		 * requeue once the work item starts running. Staying here just
> +		 * coalesces back-to-back channel events into one wakeup.
> +		 */
> +	} 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);
> @@ -842,8 +885,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)
> @@ -851,8 +894,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)
> @@ -930,6 +973,7 @@ 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);
>  	vchan_synchronize(&chan->vc);
>  }
>
> @@ -972,6 +1016,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);
> @@ -1185,10 +1231,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);
> @@ -1211,6 +1268,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;
>  }
> @@ -1235,6 +1293,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..a6a9ed09fe1b 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,13 @@ struct dw_edma {
>
>  	struct dw_edma_chan		*chan;
>
> +	/*
> +	 * Deferred channel IRQ handling. WQ_HIGHPRI keeps
> +	 * completion processing from starving behind saturated user load;
> +	 * WQ_UNBOUND spreads per-channel works across CPUs.
> +	 */
> +	struct workqueue_struct		*wq;
> +
>  	raw_spinlock_t			lock;		/* Protect v0 shared registers */
>
>  	struct dw_edma_chip             *chip;
> --
> 2.51.0
>