Re: [PATCH v3 04/24] dmaengine: dw-edma: Make DMA link list work as a circular buffer

On Tue, Jul 28, 2026 at 02:03:03AM +0900, Koichiro Den wrote:
> From: Frank Li <[email protected]>
>
> The driver currently rebuilds the whole linked list for every transfer.
>
> Use it as a circular ring instead. Append entries at ll_head with the
> current cycle bit, and reserve the final entry for the link back to the
> start.
>
> Clear control words before first use so stale cycle bits cannot become
> valid entries. Reject rings without usable data slots and, until reclaim
> support lands, descriptors that exceed the usable ring capacity.
>
> Termination and abort can discard descriptors while ll_done still trails
> ll_head. Reset the ring after the channel has stopped so the next transfer
> does not inherit occupied slots.
>
> This prepares the driver for appending requests while the engine runs.
>
> Signed-off-by: Frank Li <[email protected]>
> Co-developed-by: Koichiro Den <[email protected]>
> Signed-off-by: Koichiro Den <[email protected]>
> ---

Reviewed-by: Frank Li <[email protected]>

> Changes in v3:
>   - Reset ring state after termination or abort. (Frank, Sashiko)
>   - Use ll_done as the consumer boundary from the beginning and move the
>     ring accounting helpers here, so the later progress-reclamation
>     patch can focus on consuming IRQ-paired progress.
>   - Calculate free space once per ring-fill pass.
>
>  drivers/dma/dw-edma/dw-edma-core.c | 125 +++++++++++++++++++++++------
>  drivers/dma/dw-edma/dw-edma-core.h |  27 ++++++-
>  2 files changed, 125 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index 6a25a050b89c..d7a8a43b71d6 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
> @@ -51,13 +51,19 @@ 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;
>
>  	desc->chan = chan;
>  	desc->nburst = nburst;
> -	desc->cb = true;
>
>  	return desc;
>  }
> @@ -67,30 +73,75 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
>  	kfree(vd2dw_edma_desc(vdesc));
>  }
>
> +static void dw_edma_core_reset_ll(struct dw_edma_chan *chan)
> +{
> +	u32 i;
> +
> +	chan->ll_head = 0;
> +	chan->ll_done = 0;
> +	/* Drop stale CB bits before reusing the circular LL ring. */
> +	for (i = 0; i < chan->ll_max; i++)
> +		dw_edma_core_ll_clear(chan, i);
> +	chan->cb = true;
> +
> +	dw_edma_core_ll_link(chan, chan->ll_max, chan->cb,
> +			     chan->ll_region.paddr);
> +
> +	dw_edma_core_ch_enable(chan);
> +	chan->ll_valid = true;
> +}
> +
> +static u32 dw_edma_core_get_ll_dist(struct dw_edma_chan *chan, u32 from, u32 to)
> +{
> +	return (to + chan->ll_max - from) % chan->ll_max;
> +}
> +
> +static u32 dw_edma_core_get_used_num(struct dw_edma_chan *chan)
> +{
> +	return dw_edma_core_get_ll_dist(chan, chan->ll_done, chan->ll_head);
> +}
> +
> +static u32 dw_edma_core_get_free_num(struct dw_edma_chan *chan)
> +{
> +	/* Keep one data entry free so equal indices mean an empty ring. */
> +	return chan->ll_max - 1 - dw_edma_core_get_used_num(chan);
> +}
> +
> +static bool dw_edma_ll_pending(struct dw_edma_chan *chan)
> +{
> +	return chan->ll_head != chan->ll_done;
> +}
> +
>  static void dw_edma_core_ll_start(struct dw_edma_desc *desc)
>  {
>  	struct dw_edma_chan *chan = desc->chan;
>  	size_t i;
> -	bool first = !desc->start_burst;
> +	u32 free;
> +
> +	free = dw_edma_core_get_free_num(chan);
> +	for (i = desc->start_burst; i < desc->nburst && free; i++, free--) {
> +		/*
> +		 * Refresh the link element before filling the last data slot so
> +		 * the next lap has the updated CB value.
> +		 */
> +		if (chan->ll_head == chan->ll_max - 1)
> +			dw_edma_core_ll_link(chan, chan->ll_max, chan->cb,
> +					     chan->ll_region.paddr);
>
> -	for (i = 0; i + desc->start_burst < desc->nburst; i++) {
> -		u32 idx = i + desc->start_burst;
> +		dw_edma_core_ll_data(chan, &desc->burst[i],
> +				     chan->ll_head, chan->cb,
> +				     i == desc->nburst - 1 || free == 1);
>
> -		if (i == chan->ll_max)
> -			break;
> +		chan->ll_head++;
>
> -		dw_edma_core_ll_data(chan, &desc->burst[idx],
> -				     i, desc->cb,
> -				     idx == desc->nburst - 1 || i == chan->ll_max - 1);
> +		if (chan->ll_head == chan->ll_max) {
> +			chan->cb = !chan->cb;
> +			chan->ll_head = 0;
> +		}
>  	}
>
>  	desc->done_burst = desc->start_burst;
> -	desc->start_burst += i;
> -
> -	dw_edma_core_ll_link(chan, i, desc->cb, chan->ll_region.paddr);
> -
> -	if (first)
> -		dw_edma_core_ch_enable(chan);
> +	desc->start_burst = i;
>
>  	dw_edma_core_ch_doorbell(chan);
>  }
> @@ -123,9 +174,10 @@ static int dw_edma_start_transfer(struct dw_edma_chan *chan)
>  	if (!desc)
>  		return 0;
>
> -	dw_edma_core_start(desc);
> +	if (!chan->non_ll && !chan->ll_valid)
> +		dw_edma_core_reset_ll(chan);
>
> -	desc->cb = !desc->cb;
> +	dw_edma_core_start(desc);
>
>  	return 1;
>  }
> @@ -159,6 +211,19 @@ static void dw_edma_terminate_all_descs(struct dw_edma_chan *chan)
>  	dw_edma_terminate_vdesc_list(&chan->vc.desc_submitted);
>  }
>
> +/* Must be called with vc.lock held after the channel has stopped. */
> +static void dw_edma_finish_termination(struct dw_edma_chan *chan)
> +{
> +	dw_edma_terminate_all_descs(chan);
> +
> +	/* Preserve a clean ring; resync only if entries remain published. */
> +	if (!chan->non_ll && dw_edma_ll_pending(chan))
> +		dw_edma_core_reset_ll(chan);
> +
> +	chan->request = EDMA_REQ_NONE;
> +	chan->status = EDMA_ST_IDLE;
> +}
> +
>  static void dw_edma_device_caps(struct dma_chan *dchan,
>  				struct dma_slave_caps *caps)
>  {
> @@ -299,17 +364,15 @@ static int dw_edma_device_terminate_all(struct dma_chan *dchan)
>  	if (!chan->configured) {
>  		dw_edma_terminate_all_descs(chan);
>  	} else if (chan->status == EDMA_ST_PAUSE) {
> -		dw_edma_terminate_all_descs(chan);
> -		chan->status = EDMA_ST_IDLE;
> +		dw_edma_finish_termination(chan);
>  	} else if (chan->status == EDMA_ST_IDLE) {
> -		dw_edma_terminate_all_descs(chan);
> +		dw_edma_finish_termination(chan);
>  	} else if (dw_edma_core_ch_status(chan) == DMA_COMPLETE) {
>  		/*
>  		 * The channel is in a false BUSY state, probably didn't
>  		 * receive or lost an interrupt
>  		 */
> -		dw_edma_terminate_all_descs(chan);
> -		chan->status = EDMA_ST_IDLE;
> +		dw_edma_finish_termination(chan);
>  	} else if (chan->request > EDMA_REQ_PAUSE) {
>  		err = -EPERM;
>  	} else {
> @@ -645,6 +708,8 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
>  							    DMA_TRANS_NOERROR);
>  				list_del(&vd->node);
>  				vchan_cookie_complete(vd);
> +				if (!chan->non_ll)
> +					chan->ll_done = chan->ll_head;
>  			}
>
>  			if (chan->request == EDMA_REQ_PAUSE) {
> @@ -659,9 +724,7 @@ static void dw_edma_done_interrupt(struct dw_edma_chan *chan)
>  			break;
>
>  		case EDMA_REQ_STOP:
> -			dw_edma_terminate_all_descs(chan);
> -			chan->request = EDMA_REQ_NONE;
> -			chan->status = EDMA_ST_IDLE;
> +			dw_edma_finish_termination(chan);
>  			break;
>
>  		default:
> @@ -685,6 +748,8 @@ static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
>  		list_del(&vd->node);
>  		vchan_cookie_complete(vd);
>  	}
> +	if (!chan->non_ll)
> +		dw_edma_core_reset_ll(chan);
>  	chan->request = EDMA_REQ_NONE;
>  	chan->status = EDMA_ST_IDLE;
>  	spin_unlock_irqrestore(&chan->vc.lock, flags);
> @@ -871,6 +936,9 @@ static int dw_edma_alloc_chan_resources(struct dma_chan *dchan)
>  	if (chan->status != EDMA_ST_IDLE)
>  		return -EBUSY;
>
> +	/* The hardware context may have been invalidated while unowned. */
> +	chan->ll_valid = false;
> +
>  	return 0;
>  }
>
> @@ -962,6 +1030,13 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
>  		else
>  			chan->ll_region = chip->ll_region_rd[chan->id];
>
> +		if (!chip->cfg_non_ll && chan->ll_region.sz < 3 * EDMA_LL_SZ) {
> +			dev_err(dev,
> +				"channel %s[%u]: LL region has fewer than 2 data entries\n",
> +				str_write_read(chan->dir == EDMA_DIR_WRITE),
> +				chan->id);
> +			return -EINVAL;
> +		}
>  		chan->ll_max = chan->ll_region.sz / EDMA_LL_SZ - 1;
>
>  		dev_vdbg(dev, "L. List:\tChannel %s[%u] max_cnt=%u\n",
> diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
> index 089f913fd247..761a5ab4bbb5 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.h
> +++ b/drivers/dma/dw-edma/dw-edma-core.h
> @@ -60,7 +60,6 @@ struct dw_edma_desc {
>
>  	size_t				done_burst;
>  	size_t				start_burst;
> -	u8				cb;
>  	size_t				nburst;
>  	struct dw_edma_burst            burst[] __counted_by(nburst);
>  };
> @@ -72,8 +71,32 @@ struct dw_edma_chan {
>  	enum dw_edma_dir		dir;
>  	u8				func_no;
>
> -	u32				ll_max;
> +	/*
> +	 * New LL entries are appended at ll_head. Entries between ll_done
> +	 * and ll_head, modulo the LL ring, are owned by DMA; the rest are
> +	 * owned by software.
> +	 *
> +	 *   software-owned      DMA-owned       software-owned
> +	 * +---------------+-------------------+---------------+
> +	 * ^               ^                   ^
> +	 * 0             ll_done             ll_head
> +	 *
> +	 * The link entry points back to the region start. ll_head == ll_done
> +	 * means all entries are software-owned and previous DMA work is
> +	 * done.
> +	 *
> +	 * Software always keeps at least one free entry, so the ring is
> +	 * never completely DMA-owned. That keeps a hardware-reported physical
> +	 * LL index unique within the current ll_done..ll_head producer window.
> +	 */
> +	u32				ll_head;
> +	u32				ll_done;
> +
> +	u32				ll_max;		/* Data entries */
>  	struct dw_edma_region		ll_region;	/* Linked list */
> +	bool				ll_valid;	/* LL context programmed */
> +
> +	bool				cb;
>
>  	struct msi_msg			msi;
>
> --
> 2.51.0
>