Re: [PATCH v6 2/5] dmaengine: dw-edma: Reclaim LL entries from hardware progress

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

On Sat, Oct 31, 2026 at 12:07:39AM +0900, Koichiro Den wrote:
> Once requests are appended while the channel runs, a DONE interrupt no
> longer means that all published entries have completed.
>
> Use DMA_LLP to advance ll_done and complete descriptors in issue order.
> Reclaim entries as they complete so descriptors larger than the ring
> can be submitted in batches.
>
> DMA_LLP may advance before payload completion, so keep the running
> completion boundary one entry behind it. Use DMA_LLP itself once the
> channel is stopped with a zero transfer size. Hold vc.lock to serialize
> progress with ring resets and doorbells.
>
> Refill reclaimed entries and restart a stopped channel if work remains.
> Defer STOP and PAUSE completion until the channel stops.
>
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---
> Reworked v5 patch 13:
> https://lore.kernel.org/r/20260812155721.2807506-14-den@valinux.co.jp/
>
> Changes since v5:
>   - Read the registers in the progress handler instead of retaining an
>     IRQ-paired LLP snapshot.
>   - Allow index 0 to wrap. Reject running samples that would reclaim all
>     pending entries, so an unchanged start pointer cannot look like a
>     full lap.
>
>  drivers/dma/dw-edma/dw-edma-core.c | 140 ++++++++++++++++++++++++++---
>  1 file changed, 126 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index b08bb31657c9..74613c95ae11 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -53,13 +53,6 @@ dw_edma_alloc_desc(struct dw_edma_chan *chan, size_t nburst)
>  {
>  	struct dw_edma_desc *desc;
>
> -	/*
> -	 * For now, a descriptor that does not fit would stall the channel
> -	 * forever: reject it up front.
> -	 */
> -	if (!chan->non_ll && nburst > chan->ll_max - 1)
> -		return NULL;
> -
>  	desc = kzalloc_flex(*desc, burst, nburst, GFP_NOWAIT);
>  	if (unlikely(!desc))
>  		return NULL;
> @@ -146,6 +139,76 @@ static bool dw_edma_ll_pending(struct dw_edma_chan *chan)
>  	return chan->ll_head != chan->ll_done;
>  }
>
> +static bool dw_edma_ll_stopped(struct dw_edma_chan *chan)
> +{
> +	return dw_edma_core_ch_status(chan) == DMA_COMPLETE &&
> +	       !dw_edma_core_ch_transfer_size(chan);
> +}
> +
> +/* Return the first entry we cannot reclaim yet. */
> +static int dw_edma_ll_boundary(struct dw_edma_chan *chan, int idx,
> +			       bool stopped)
> +{
> +	if (idx < 0 || (u32)idx > chan->ll_max)
> +		return -EINVAL;
> +
> +	/* After a stop, LLP points past the completed entries. */
> +	if (stopped)
> +		return idx == chan->ll_max ? 0 : idx;
> +
> +	/*
> +	 * DMA_LLP can advance before payload completion. Keep the boundary
> +	 * one entry behind it, wrapping at the start of the ring.
> +	 */
> +	return idx ? idx - 1 : chan->ll_max - 1;

use mod

	(idx + (chan->ll_max - 1) - 1) % chan->ll_max - 1;


> +}
> +
> +/* Must be called with vc.lock held. */
> +static void dw_edma_ll_consume(struct dw_edma_chan *chan, int idx,
> +			       bool stopped)
> +{
> +	struct virt_dma_desc *vd, *_vd;
> +	u32 done, used;

lockdep_assert_held()

> +
> +	idx = dw_edma_ll_boundary(chan, idx, stopped);
> +	if (idx < 0)
> +		return;
> +
> +	done = dw_edma_core_get_ll_dist(chan, chan->ll_done, idx);
> +	used = dw_edma_core_get_used_num(chan);
> +
> +	/*
> +	 * LLP may still point to the first pending entry. Subtracting one
> +	 * can wrap the index and falsely mark all pending entries complete.
> +	 * Reclaim them all only after the channel has stopped.
> +	 */

Do you means !done for this comments?

> +	if (!done || done > used || (!stopped && done == used))
> +		return;
> +
> +	chan->ll_done = idx;
> +
> +	/* Match reclaimed entries to descriptors in issue order. */
> +	list_for_each_entry_safe(vd, _vd, &chan->vc.desc_issued, node) {
> +		struct dw_edma_desc *desc = vd2dw_edma_desc(vd);
> +		u32 consumed = min_t(u32, done,
> +				     desc->start_burst - desc->done_burst);
> +
> +		if (!consumed)
> +			break;
> +
> +		desc->done_burst += consumed;
> +		done -= consumed;
> +		if (desc->done_burst != desc->nburst)
> +			break;
> +
> +		dw_hdma_set_callback_result(vd, DMA_TRANS_NOERROR);
> +		list_del(&vd->node);
> +		vchan_cookie_complete(vd);
> +	}
> +
> +	WARN_ON_ONCE(done);
> +}
> +
>  static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
>  {
>  	struct dw_edma_chan *chan = desc->chan;
> @@ -174,7 +237,6 @@ static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
>  		}
>  	}
>
> -	desc->done_burst = desc->start_burst;
>  	desc->start_burst = i;
>  }
>
> @@ -283,6 +345,45 @@ static void dw_edma_core_ch_maybe_doorbell(struct dw_edma_chan *chan)
>  	dw_edma_core_ch_doorbell(chan);
>  }
>
> +/*
> + * Must be called with vc.lock held.
> + *
> + * HDMA does not update LLP continuously. Only use it to track running
> + * transfers when handling an interrupt.
> + */
> +static void dw_edma_ll_advance(struct dw_edma_chan *chan, bool event)
> +{
> +	bool stopped = dw_edma_ll_stopped(chan);
> +
> +	if (stopped || event)
> +		dw_edma_ll_consume(chan, dw_edma_core_ll_cur_idx(chan),
> +				   stopped);
> +
> +	if (chan->request != EDMA_REQ_NONE) {
> +		/* Wait for the channel to stop before completing STOP or PAUSE. */
> +		if (!stopped)
> +			return;
> +
> +		if (chan->request == EDMA_REQ_STOP) {
> +			dw_edma_finish_termination(chan);
> +		} else if (chan->request == EDMA_REQ_PAUSE) {
> +			dw_edma_set_request(chan, EDMA_REQ_NONE);
> +			chan->status = EDMA_ST_PAUSE;
> +		}
> +		return;
> +	}
> +
> +	dw_edma_start_transfer(chan);
> +	if (!dw_edma_ll_pending(chan)) {
> +		chan->status = EDMA_ST_IDLE;
> +		return;
> +	}
> +
> +	chan->status = EDMA_ST_BUSY;
> +	if (stopped)
> +		dw_edma_core_ch_doorbell(chan);
> +}
> +
>  static void dw_edma_device_caps(struct dma_chan *dchan,
>  				struct dma_slave_caps *caps)
>  {
> @@ -749,7 +850,7 @@ dw_edma_device_prep_interleaved_dma(struct dma_chan *dchan,
>  	return dw_edma_device_transfer(&xfer, dw_edma_device_get_config(dchan, NULL));
>  }
>
> -/* Must be called with vc.lock held. */
> +/* Non-LL completion; vc.lock must be held. */
>  static void dw_edma_done_interrupt_locked(struct dw_edma_chan *chan)
>  {
>  	struct dw_edma_desc *desc;
> @@ -772,8 +873,6 @@ static void dw_edma_done_interrupt_locked(struct dw_edma_chan *chan)
>  			dw_hdma_set_callback_result(vd, DMA_TRANS_NOERROR);
>  			list_del(&vd->node);
>  			vchan_cookie_complete(vd);

why not use dw_edma_ll_consume(), let vchan_cookie_complete() at one
place only

Frank

> -			if (!chan->non_ll)
> -				chan->ll_done = chan->ll_head;
>  		}
>
>  		if (chan->request == EDMA_REQ_PAUSE) {
> @@ -796,7 +895,6 @@ static void dw_edma_done_interrupt_locked(struct dw_edma_chan *chan)
>  	default:
>  		break;
>  	}
> -	dw_edma_core_ch_maybe_doorbell(chan);
>  }
>
>  static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> @@ -808,6 +906,16 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
>  	spin_unlock_irqrestore(&chan->vc.lock, flags);
>  }
>
> +static void dw_edma_ll_interrupt(struct dw_edma_chan *chan)
> +{
> +	guard(spinlock_irqsave)(&chan->vc.lock);
> +
> +	if (chan->status == EDMA_ST_PAUSE)
> +		return;
> +
> +	dw_edma_ll_advance(chan, true);
> +}
> +
>  static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
>  {
>  	struct virt_dma_desc *vd;
> @@ -838,8 +946,12 @@ static void dw_edma_irq_work(struct work_struct *work)
>  	do {
>  		events = atomic_xchg(&chan->irq_pending, 0);
>
> -		if (events & DW_EDMA_DEFERRED_DONE)
> -			dw_edma_done_interrupt(chan);
> +		if (events & DW_EDMA_DEFERRED_DONE) {
> +			if (chan->non_ll)
> +				dw_edma_done_interrupt(chan);
> +			else
> +				dw_edma_ll_interrupt(chan);
> +		}
>  		if (events & DW_EDMA_DEFERRED_ABORT)
>  			dw_edma_abort_interrupt(chan);
>  	} while (atomic_read(&chan->irq_pending));
> --
> 2.54.0
>