Re: [PATCH v7 4/7] dmaengine: dw-edma: Reclaim LL entries from hardware progress

[Frank Li]

On Thu, Nov 05, 2026 at 01:12:24AM +0900, Koichiro Den wrote:
> With dynamic appends, a DONE interrupt no longer means that all
> published LL entries have completed. Prepare for this by using LLP to
> advance ll_done and complete descriptors in issue order.
>
> Reclaim completed entries and refill the ring as space becomes
> available. Restart a stopped channel when published work remains. This
> also allows descriptors larger than the ring to run in batches.
>
> Hardware can keep running while the driver handles an interrupt, and
> LLP does not always track payload completion. Use the following
> exclusive completion boundaries, where idx is the raw LLP entry index
> and prev(idx) is the previous data entry in the ring:
>
>   State/event          Meaning of raw LLP               Boundary
>   -------------------  -------------------------------  ---------
>   eDMA/HDMA stopped    Next entry to process             idx
>
>   eDMA running         Observed to advance before        prev(idx)
>                        payload completion
>
>   HDMA watermark       Inclusive LLE recycling limit     prev(idx)
>
> A stopped sample requires CS=STOPPED and a zero transfer size.
>
> While running, HDMA updates LLP only at watermark events. Its documented
> recycling limit translates to an exclusive boundary of next(idx).
> However, tests using that boundary for dmaengine completion reported
> IOMMU faults on HDMA 6.30a SpacemiT K3. Use prev(idx) for both engines
> while running to keep the handling simple and conservative. Entries held
> back by this rule are eventually reclaimed on later progress or a
> confirmed stop, where idx gives the exact boundary.
>
> Handle termination and pause as follows:
> - On termination, stop publishing new entries and drain those already
>   published, restarting the channel if needed. Terminate descriptors
>   without callbacks, as in the non-LL path.
> - On pause, stop publishing new entries and enter the paused state once
>   the channel stops. Keep unfinished work for resume.
>
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> ---

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

> Changes from v6:
>   - Handle STOP before consuming LL entries (Sashiko).
>   - Use modulo, assert vc.lock, and share descriptor completion code
>     (Frank).
>   - (*) Restart stopped channels during termination. Tests under DMA
>     load exposed the missing restart. In fact, v5 patch 19 had similar
>     handling and I mistakenly dropped that logic while doing major
>     v5->v6 reconstruction/simplification:
>     https://lore.kernel.org/r/20260812155721.2807506-20-den@xxxxxxxxxxxxx/
>   - Substantially reword/polish the commit message.
>   - Link to v6:
>     https://lore.kernel.org/r/20261030150742.2586776-3-den@xxxxxxxxxxxxx/
>
> Note on (*): A channel can still report STOPPED just after a doorbell.
> Another doorbell in this window may trigger the class B stall observed
> on R-Car S4 [1]. That stall appears specific to S4 or its DWC IP
> revision, and IMHO should not block this series any longer.
>
> [1] https://lore.kernel.org/all/tkyu5rsxfa7i6rxxw6qxmmwke5qnlxptwx4j2s24fhftsmmf26@gamef2ec4wwm/
>
>  drivers/dma/dw-edma/dw-edma-core.c | 156 +++++++++++++++++++++++++----
>  1 file changed, 134 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index e2f7af5969c6..e2f2a535d5ad 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;
> @@ -161,17 +154,80 @@ static bool dw_edma_ll_stopped(struct dw_edma_chan *chan)
>  	       !dw_edma_core_ch_transfer_size(chan);
>  }
>
> -static bool dw_edma_ll_drained(struct dw_edma_chan *chan)
> +/* Return the first entry we cannot reclaim yet. */
> +static int dw_edma_ll_boundary(struct dw_edma_chan *chan, int idx,
> +			       bool stopped)
>  {
> -	int idx = dw_edma_core_ll_cur_idx(chan);
> +	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;
>
> -	if (idx == chan->ll_max)
> -		idx = 0;
> +	/*
> +	 * DMA_LLP can advance before payload completion. Keep the boundary
> +	 * one entry behind it, wrapping at the start of the ring.
> +	 */
> +	return (idx + chan->ll_max - 1) % chan->ll_max;
> +}
> +
> +static bool dw_edma_ll_drained(struct dw_edma_chan *chan)
> +{
> +	int idx = dw_edma_ll_boundary(chan, dw_edma_core_ll_cur_idx(chan), true);
>
>  	return idx >= 0 && (u32)idx == chan->ll_head &&
>  	       dw_edma_ll_stopped(chan);
>  }
>
> +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(&chan->vc.lock);
> +
> +	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);
> +
> +	if (!done || done > used)
> +		return;
> +
> +	/*
> +	 * 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.
> +	 */
> +	if (!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_edma_complete_vdesc(vd, DMA_TRANS_NOERROR);
> +	}
> +
> +	WARN_ON_ONCE(done);
> +}
> +
>  static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
>  {
>  	struct dw_edma_chan *chan = desc->chan;
> @@ -200,7 +256,6 @@ static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
>  		}
>  	}
>
> -	desc->done_burst = desc->start_burst;
>  	desc->start_burst = i;
>  }
>
> @@ -309,6 +364,52 @@ 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. */
> +static void dw_edma_ll_advance(struct dw_edma_chan *chan, bool event)
> +{
> +	bool stopped;
> +
> +	lockdep_assert_held(&chan->vc.lock);
> +
> +	/* Terminate without scheduling completion callbacks. */
> +	if (chan->request == EDMA_REQ_STOP) {
> +		if (dw_edma_ll_drained(chan))
> +			dw_edma_finish_termination(chan);
> +		else if (dw_edma_ll_stopped(chan))
> +			/* A newly appended tail may have missed the last stop. */
> +			dw_edma_core_ch_doorbell(chan);
> +		return;
> +	}
> +
> +	stopped = dw_edma_ll_stopped(chan);
> +	/*
> +	 * HDMA updates LLP at watermark events while running. Use running
> +	 * samples only when processing an LL interrupt.
> +	 */
> +	if (stopped || event)
> +		dw_edma_ll_consume(chan, dw_edma_core_ll_cur_idx(chan),
> +				   stopped);
> +
> +	if (chan->request == EDMA_REQ_PAUSE) {
> +		if (!stopped)
> +			return;
> +
> +		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)
>  {
> @@ -493,6 +594,8 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
>  		err = -EPERM;
>  	} else {
>  		dw_edma_set_request(chan, EDMA_REQ_STOP);
> +		if (!chan->non_ll)
> +			dw_edma_ll_advance(chan, false);
>  	}
>  	if (chan->status == EDMA_ST_IDLE)
>  		dw_edma_set_request(chan, EDMA_REQ_NONE);
> @@ -777,7 +880,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;
> @@ -796,11 +899,8 @@ static void dw_edma_done_interrupt_locked(struct dw_edma_chan *chan)
>  			break;
>
>  		desc = vd2dw_edma_desc(vd);
> -		if (desc->start_burst >= desc->nburst) {
> +		if (desc->start_burst >= desc->nburst)
>  			dw_edma_complete_vdesc(vd, DMA_TRANS_NOERROR);
> -			if (!chan->non_ll)
> -				chan->ll_done = chan->ll_head;
> -		}
>
>  		if (chan->request == EDMA_REQ_PAUSE) {
>  			dw_edma_set_request(chan, EDMA_REQ_NONE);
> @@ -816,14 +916,12 @@ static void dw_edma_done_interrupt_locked(struct dw_edma_chan *chan)
>  		if (!vd)
>  			break;
>
> -		if (chan->non_ll || dw_edma_ll_drained(chan))
> -			dw_edma_finish_termination(chan);
> +		dw_edma_finish_termination(chan);
>  		break;
>
>  	default:
>  		break;
>  	}
> -	dw_edma_core_ch_maybe_doorbell(chan);
>  }
>
>  static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
> @@ -835,6 +933,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;
> @@ -863,8 +971,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
>