Re: [PATCH 6/7] dmaengine: dw-edma: Snapshot the v0 interrupt status once per handler pass

[Frank Li]

On Fri, Jul 10, 2026 at 05:09:02PM +0900, Koichiro Den wrote:
> The v0 interrupt handler reads the interrupt status register twice per
> invocation, once through the DONE accessor and once through the ABORT
> accessor, although both fields live in the same 32-bit register. On
> remote setups (dw-edma-pcie) each read is a non-posted round trip across
> the PCIe link costing on the order of a microsecond, and with one
> completion interrupt per element the duplicate adds up. As an example,
> profiling the R-Car S4 remote path put the handler at ~7us per
> invocation, dominated by such reads.
>
> Read the register once and derive the DONE and ABORT views from the
> snapshot. No abort is lost to this because the pass only clears status
> bits it observed, so an abort raised after the snapshot keeps its status
> and its own interrupt delivery brings it to the next pass. A second
> abort on an observed channel cannot race the clear either, as an aborted
> channel stays halted until software restarts it, and any restart follows
> the abort() handling, which comes after
> dw_edma_v0_core_clear_abort_int().
>
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> ---

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

> Changes in v2:
>   - New patch in v2, posted as part of this preparation series.
>
>  drivers/dma/dw-edma/dw-edma-v0-core.c | 28 +++++++++++++--------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index cfdd6463252e..377812eaa110 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -218,18 +218,6 @@ static void dw_edma_v0_core_clear_abort_int(struct dw_edma_chan *chan)
>  		  FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id)));
>  }
>
> -static u32 dw_edma_v0_core_status_done_int(struct dw_edma *dw, enum dw_edma_dir dir)
> -{
> -	return FIELD_GET(EDMA_V0_DONE_INT_MASK,
> -			 GET_RW_32(dw, dir, int_status));
> -}
> -
> -static u32 dw_edma_v0_core_status_abort_int(struct dw_edma *dw, enum dw_edma_dir dir)
> -{
> -	return FIELD_GET(EDMA_V0_ABORT_INT_MASK,
> -			 GET_RW_32(dw, dir, int_status));
> -}
> -
>  static irqreturn_t
>  dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
>  			   dw_edma_handler_t done, dw_edma_handler_t abort)
> @@ -239,7 +227,7 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
>  	irqreturn_t ret = IRQ_NONE;
>  	struct dw_edma_chan *chan;
>  	unsigned long off;
> -	u32 mask;
> +	u32 mask, sts;
>
>  	if (dir == EDMA_DIR_WRITE) {
>  		total = dw->wr_ch_cnt;
> @@ -251,7 +239,17 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
>  		mask = dw_irq->rd_mask;
>  	}
>
> -	val = dw_edma_v0_core_status_done_int(dw, dir);
> +	/*
> +	 * DONE and ABORT status share one register, and on remote setups
> +	 * every read is a non-posted round trip across the PCIe link. Take
> +	 * one snapshot and derive both views from it. An abort raised
> +	 * after the snapshot is deferred, not lost: only bits observed in
> +	 * the snapshot are ever cleared below, so its status survives for
> +	 * the next invocation, which its own interrupt delivery triggers.
> +	 */
> +	sts = GET_RW_32(dw, dir, int_status);
> +
> +	val = FIELD_GET(EDMA_V0_DONE_INT_MASK, sts);
>  	val &= mask;
>  	for_each_set_bit(pos, &val, total) {
>  		chan = &dw->chan[pos + off];
> @@ -262,7 +260,7 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
>  		ret = IRQ_HANDLED;
>  	}
>
> -	val = dw_edma_v0_core_status_abort_int(dw, dir);
> +	val = FIELD_GET(EDMA_V0_ABORT_INT_MASK, sts);
>  	val &= mask;
>  	for_each_set_bit(pos, &val, total) {
>  		chan = &dw->chan[pos + off];
> --
> 2.51.0
>