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

[Koichiro Den]

On Sat, Jul 11, 2026 at 09:55:04AM -0500, Frank Li wrote:
> 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@xxxxxxxxxxxxx>
> > ---
> 
> It looks good, allen try convert bh tasklet to workqueue.
> https://lore.kernel.org/dmaengine/CAOMdWSLVk136RzEyiN76zqk65VLwYms0hCDi5Kww9FQppie12A@xxxxxxxxxxxxxx/
> 
> Please double check if it can apply to dw-edma?

Thanks for the pointer. IIUC, Allen and Arnd had agreed there that the common
per-channel BH path work is for the vchan client callbacks, and tasklets/works
serving other purposes are to be dealt with case-by-case (hardirq, threaded irq
or a regular workqueue, depending on the driver). dw->wq here is the latter
kind, it exists mostly to relieve the hard IRQ handler, and also to spread the
per-channel servicing across CPUs. I don't think this one gets commonized as
well.

That conversion itself should apply to dw-edma fine on top of this patch, so
I'll keep an eye on it.

Best regards,
Koichiro

> 
> 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
> >