[PATCH v3 11/24] dmaengine: dw-edma: Serialize LL event capture with channel kicks

[Koichiro Den]

The hard IRQ path clears interrupt status before deferred handling takes
vc.lock. If a doorbell starts a new hardware run in between, the
deferred handler can apply an event from the previous run.

Use the event lock to serialize the interrupt status read and clear,
snapshot recording, and LL doorbell writes. Do not write another
doorbell while a snapshot is pending. A ring reset discards the snapshot
under the same lock. When events coalesce, keep STOP over PROGRESS.

Changing chan->request alone neither changes the ring nor writes a
doorbell, so it does not invalidate the snapshot. Preserve it across
request changes. A later patch uses PROGRESS and STOP to decide whether
a pending STOP or PAUSE request can finish.

Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
---
Changes in v3:
  - New patch to serialize IRQ event capture with LL doorbell writes and
    defer doorbells while an LL event is pending. (Sashiko)
  - Add explicit PROGRESS and STOP types for deferred LL snapshots.
    (Frank)

 drivers/dma/dw-edma/dw-edma-core.c    | 101 +++++++++++++++++++++++++-
 drivers/dma/dw-edma/dw-edma-core.h    |  37 ++++++++++
 drivers/dma/dw-edma/dw-edma-v0-core.c |  64 ++++++++--------
 drivers/dma/dw-edma/dw-hdma-v0-core.c |  32 +++++---
 4 files changed, 187 insertions(+), 47 deletions(-)

diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
index e3d0559206d8..3970b066dca2 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -33,7 +33,8 @@ struct dw_edma_desc *vd2dw_edma_desc(struct virt_dma_desc *vd)
 
 enum dw_edma_deferred_event {
 	DW_EDMA_DEFERRED_DONE	= BIT(0),
-	DW_EDMA_DEFERRED_ABORT	= BIT(1),
+	DW_EDMA_DEFERRED_LL	= BIT(1),
+	DW_EDMA_DEFERRED_ABORT	= BIT(2),
 };
 
 static inline
@@ -74,6 +75,52 @@ static void vchan_free_desc(struct virt_dma_desc *vdesc)
 	kfree(vd2dw_edma_desc(vdesc));
 }
 
+static void dw_edma_ll_snapshot_discard_locked(struct dw_edma_chan *chan)
+{
+	lockdep_assert_held(dw_edma_event_lock(chan));
+
+	chan->ll_irq.event = DW_EDMA_LL_EVENT_NONE;
+}
+
+static void dw_edma_ll_snapshot_discard(struct dw_edma_chan *chan)
+{
+	guard(raw_spinlock_irqsave)(dw_edma_event_lock(chan));
+
+	dw_edma_ll_snapshot_discard_locked(chan);
+}
+
+/* Must be called with vc.lock held. */
+static bool
+dw_edma_ll_snapshot_take(struct dw_edma_chan *chan,
+			 struct dw_edma_ll_snapshot *snapshot)
+{
+	guard(raw_spinlock_irqsave)(dw_edma_event_lock(chan));
+
+	if (chan->ll_irq.event == DW_EDMA_LL_EVENT_NONE)
+		return false;
+
+	*snapshot = chan->ll_irq;
+	dw_edma_ll_snapshot_discard_locked(chan);
+
+	return true;
+}
+
+static void dw_edma_ll_event_discard_locked(struct dw_edma_chan *chan)
+{
+	lockdep_assert_held(dw_edma_event_lock(chan));
+
+	dw_edma_core_ll_irq_clear(chan);
+	dw_edma_ll_snapshot_discard_locked(chan);
+}
+
+/* Must be called with vc.lock held for an LL channel. */
+static void dw_edma_ll_event_discard(struct dw_edma_chan *chan)
+{
+	guard(raw_spinlock_irqsave)(dw_edma_event_lock(chan));
+
+	dw_edma_ll_event_discard_locked(chan);
+}
+
 /* Must be called with vc.lock held. */
 static void
 dw_edma_set_request(struct dw_edma_chan *chan, enum dw_edma_request request)
@@ -120,6 +167,7 @@ static void dw_edma_core_reset_ll(struct dw_edma_chan *chan)
 	dw_edma_core_ll_link(chan, chan->ll_max, chan->cb,
 			     chan->ll_region.paddr);
 
+	dw_edma_ll_event_discard(chan);
 	dw_edma_core_ch_enable(chan);
 	chan->ll_valid = true;
 }
@@ -269,6 +317,17 @@ static void dw_edma_core_ll_sync(struct dw_edma_chan *chan)
 static void dw_edma_core_ch_kick(struct dw_edma_chan *chan)
 {
 	dw_edma_core_ll_sync(chan);
+
+	guard(raw_spinlock_irqsave)(dw_edma_event_lock(chan));
+
+	/*
+	 * A recorded event belongs to the current ring and must be consumed
+	 * before starting another hardware run.
+	 */
+	if (chan->ll_irq.event != DW_EDMA_LL_EVENT_NONE)
+		return;
+
+	dw_edma_ll_event_discard_locked(chan);
 	dw_edma_core_ch_doorbell(chan);
 }
 
@@ -779,6 +838,18 @@ 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)
+{
+	struct dw_edma_ll_snapshot snapshot;
+
+	guard(spinlock_irqsave)(&chan->vc.lock);
+
+	if (!dw_edma_ll_snapshot_take(chan, &snapshot))
+		return;
+
+	dw_edma_done_interrupt_locked(chan);
+}
+
 static void dw_edma_abort_interrupt(struct dw_edma_chan *chan)
 {
 	struct virt_dma_desc *vd;
@@ -811,6 +882,8 @@ static void dw_edma_irq_work(struct work_struct *work)
 
 		if (events & DW_EDMA_DEFERRED_DONE)
 			dw_edma_done_interrupt(chan);
+		if (events & DW_EDMA_DEFERRED_LL)
+			dw_edma_ll_interrupt(chan);
 		if (events & DW_EDMA_DEFERRED_ABORT)
 			dw_edma_abort_interrupt(chan);
 	} while (atomic_read(&chan->irq_pending));
@@ -825,14 +898,29 @@ static void dw_edma_queue_irq_work(struct dw_edma_chan *chan,
 
 static void dw_edma_record_irq(struct dw_edma_chan *chan, unsigned int events)
 {
+	struct dw_edma_ll_snapshot snapshot = {
+		.event = events & DW_EDMA_IRQ_STOP ?
+			 DW_EDMA_LL_EVENT_STOP : DW_EDMA_LL_EVENT_PROGRESS,
+	};
 	unsigned int pending = 0;
 
-	if (events & (DW_EDMA_IRQ_DONE | DW_EDMA_IRQ_PROGRESS |
-		      DW_EDMA_IRQ_STOP))
-		pending |= DW_EDMA_DEFERRED_DONE;
+	lockdep_assert_held(dw_edma_event_lock(chan));
+
 	if (events & DW_EDMA_IRQ_ABORT)
 		pending |= DW_EDMA_DEFERRED_ABORT;
 
+	if (chan->non_ll) {
+		if (events & (DW_EDMA_IRQ_DONE | DW_EDMA_IRQ_STOP))
+			pending |= DW_EDMA_DEFERRED_DONE;
+	} else if (events & (DW_EDMA_IRQ_DONE | DW_EDMA_IRQ_PROGRESS |
+			     DW_EDMA_IRQ_STOP)) {
+		/* STOP is final for this run; do not replace it with progress. */
+		if (chan->ll_irq.event != DW_EDMA_LL_EVENT_STOP ||
+		    snapshot.event == DW_EDMA_LL_EVENT_STOP)
+			chan->ll_irq = snapshot;
+		pending |= DW_EDMA_DEFERRED_LL;
+	}
+
 	if (pending)
 		dw_edma_queue_irq_work(chan, pending);
 }
@@ -1020,6 +1108,7 @@ static void dw_edma_device_synchronize(struct dma_chan *dchan)
 	dw_edma_wait_termination(dchan);
 	cancel_work_sync(&chan->irq_work);
 	atomic_set(&chan->irq_pending, 0);
+	dw_edma_ll_snapshot_discard(chan);
 	vchan_synchronize(&chan->vc);
 }
 
@@ -1071,6 +1160,8 @@ static int dw_edma_channel_setup(struct dw_edma *dw, u32 wr_alloc, u32 rd_alloc)
 		chan->irq_mode = dw_edma_get_default_irq_mode(chan);
 		INIT_WORK(&chan->irq_work, dw_edma_irq_work);
 		atomic_set(&chan->irq_pending, 0);
+		chan->ll_irq.event = DW_EDMA_LL_EVENT_NONE;
+		raw_spin_lock_init(&chan->event_lock);
 
 		if (chan->dir == EDMA_DIR_WRITE)
 			chan->ll_region = chip->ll_region_wr[chan->id];
@@ -1307,6 +1398,8 @@ int dw_edma_probe(struct dw_edma_chip *chip)
 	}
 
 	raw_spin_lock_init(&dw->lock);
+	for (i = 0; i < ARRAY_SIZE(dw->event_lock); i++)
+		raw_spin_lock_init(&dw->event_lock[i]);
 
 	/*
 	 * chip->ll_*_cnt describes the channels exposed by this instance. Keep
diff --git a/drivers/dma/dw-edma/dw-edma-core.h b/drivers/dma/dw-edma/dw-edma-core.h
index 6b0d475bb8b5..9f528f160cea 100644
--- a/drivers/dma/dw-edma/dw-edma-core.h
+++ b/drivers/dma/dw-edma/dw-edma-core.h
@@ -48,6 +48,12 @@ enum dw_edma_irq_event {
 	DW_EDMA_IRQ_ABORT	= BIT(3),
 };
 
+enum dw_edma_ll_event {
+	DW_EDMA_LL_EVENT_NONE,
+	DW_EDMA_LL_EVENT_PROGRESS,
+	DW_EDMA_LL_EVENT_STOP,
+};
+
 enum dw_edma_event_scope {
 	DW_EDMA_EVENT_PER_CHAN,
 	DW_EDMA_EVENT_PER_DIR,
@@ -76,6 +82,10 @@ struct dw_edma_desc {
 	struct dw_edma_burst            burst[] __counted_by(nburst);
 };
 
+struct dw_edma_ll_snapshot {
+	enum dw_edma_ll_event		event;
+};
+
 struct dw_edma_chan {
 	struct virt_dma_chan		vc;
 	struct dw_edma			*dw;
@@ -104,6 +114,14 @@ struct dw_edma_chan {
 	u32				ll_head;
 	u32				ll_done;
 
+	/*
+	 * LL event recorded by the hard IRQ handler. The event scope lock
+	 * serializes its capture with a new hardware run; vc.lock serializes
+	 * its consumption with LL state.
+	 */
+	struct dw_edma_ll_snapshot	ll_irq;
+	raw_spinlock_t			event_lock;
+
 	u32				ll_max;		/* Data entries */
 	struct dw_edma_region		ll_region;	/* Linked list */
 	bool				ll_valid;	/* LL context programmed */
@@ -153,6 +171,7 @@ struct dw_edma {
 	struct workqueue_struct		*wq;
 
 	raw_spinlock_t			lock;		/* Protect v0 shared registers */
+	raw_spinlock_t			event_lock[2];	/* Per-direction event scopes */
 
 	struct dw_edma_chip             *chip;
 
@@ -177,7 +196,9 @@ struct dw_edma_core_ops {
 	void (*ll_link)(struct dw_edma_chan *chan, u32 idx, bool cb, u64 addr);
 	void (*ll_clear)(struct dw_edma_chan *chan, u32 idx);
 	int (*ll_cur_idx)(struct dw_edma_chan *chan);
+	/* Called with the event scope locked. */
 	void (*ll_irq_clear)(struct dw_edma_chan *chan);
+	/* Called with the event scope locked for an LL channel. */
 	void (*ch_doorbell)(struct dw_edma_chan *chan);
 	void (*ch_enable)(struct dw_edma_chan *chan);
 	void (*ch_config)(struct dw_edma_chan *chan);
@@ -221,6 +242,22 @@ struct dw_edma_chan *dchan2dw_edma_chan(struct dma_chan *dchan)
 	return vc2dw_edma_chan(to_virt_chan(dchan));
 }
 
+/*
+ * Lock ordering:
+ *
+ *   chan->vc.lock -> dw_edma_event_lock(chan)
+ *
+ * Interrupt providers invoke the dw_edma_handler_t callback with the event
+ * lock held. The callback must not take vc.lock.
+ */
+static inline raw_spinlock_t *dw_edma_event_lock(struct dw_edma_chan *chan)
+{
+	if (chan->dw->core->event_scope == DW_EDMA_EVENT_PER_DIR)
+		return &chan->dw->event_lock[chan->dir];
+
+	return &chan->event_lock;
+}
+
 /*
  * Return the current LL entry index. A negative value means that the channel
  * context is not initialized or was lost after a link reset.
diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c b/drivers/dma/dw-edma/dw-edma-v0-core.c
index b3098eb83442..43e33a400a3e 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
@@ -339,8 +339,8 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
 			   dw_edma_handler_t handler)
 {
 	struct dw_edma *dw = dw_irq->dw;
-	unsigned long total, pos, val;
-	irqreturn_t ret = IRQ_NONE;
+	u8 events[EDMA_V0_MAX_NR_CH] = {};
+	unsigned long total, pos, val, active = 0;
 	struct dw_edma_chan *chan;
 	unsigned long off;
 	unsigned long *mask;
@@ -356,45 +356,45 @@ dw_edma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
 		mask = dw_irq->rd_mask;
 	}
 
-	/*
-	 * 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 remains set and
-	 * triggers another handler pass.
-	 */
-	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];
+	scoped_guard(raw_spinlock_irqsave, &dw->event_lock[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.
+		 */
+		sts = GET_RW_32(dw, dir, int_status);
 
-		if (unlikely(dw_edma_core_ch_ignore_irq(chan)))
-			continue;
+		val = FIELD_GET(EDMA_V0_DONE_INT_MASK, sts);
+		val &= *mask;
+		for_each_set_bit(pos, &val, total) {
+			chan = &dw->chan[pos + off];
 
-		dw_edma_v0_core_clear_done_int(chan);
-		handler(chan, DW_EDMA_IRQ_DONE);
+			if (unlikely(dw_edma_core_ch_ignore_irq(chan)))
+				continue;
 
-		ret = IRQ_HANDLED;
-	}
+			events[pos] |= DW_EDMA_IRQ_DONE;
+			active |= BIT(pos);
+			dw_edma_v0_core_clear_done_int(chan);
+		}
 
-	val = FIELD_GET(EDMA_V0_ABORT_INT_MASK, sts);
-	val &= *mask;
-	for_each_set_bit(pos, &val, total) {
-		chan = &dw->chan[pos + off];
+		val = FIELD_GET(EDMA_V0_ABORT_INT_MASK, sts);
+		val &= *mask;
+		for_each_set_bit(pos, &val, total) {
+			chan = &dw->chan[pos + off];
 
-		if (unlikely(dw_edma_core_ch_ignore_irq(chan)))
-			continue;
+			if (unlikely(dw_edma_core_ch_ignore_irq(chan)))
+				continue;
 
-		dw_edma_v0_core_clear_abort_int(chan);
-		handler(chan, DW_EDMA_IRQ_ABORT);
+			events[pos] |= DW_EDMA_IRQ_ABORT;
+			active |= BIT(pos);
+			dw_edma_v0_core_clear_abort_int(chan);
+		}
 
-		ret = IRQ_HANDLED;
+		for_each_set_bit(pos, &active, total)
+			handler(&dw->chan[pos + off], events[pos]);
 	}
 
-	return ret;
+	return active ? IRQ_HANDLED : IRQ_NONE;
 }
 
 static void dw_edma_v0_write_ll_data(struct dw_edma_chan *chan, int i,
diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
index 64a07fbc5be3..bbf9ee0392f8 100644
--- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
+++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
@@ -177,10 +177,12 @@ dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
 			   dw_edma_handler_t handler)
 {
 	struct dw_edma *dw = dw_irq->dw;
-	unsigned long total, pos, val;
+	unsigned int events;
+	unsigned long total, pos;
 	irqreturn_t ret = IRQ_NONE;
 	struct dw_edma_chan *chan;
 	unsigned long off, *mask;
+	u32 val;
 
 	if (dir == EDMA_DIR_WRITE) {
 		total = dw->wr_ch_cnt;
@@ -197,20 +199,28 @@ dw_hdma_v0_core_handle_int(struct dw_edma_irq *dw_irq, enum dw_edma_dir dir,
 		if (unlikely(dw_edma_core_ch_ignore_irq(chan)))
 			continue;
 
-		val = dw_hdma_v0_core_status_int(chan);
-		if (FIELD_GET(HDMA_V0_STOP_INT_MASK, val)) {
-			dw_hdma_v0_core_clear_done_int(chan);
-			handler(chan, DW_EDMA_IRQ_STOP);
+		events = 0;
+		scoped_guard(raw_spinlock_irqsave, dw_edma_event_lock(chan)) {
+			val = dw_hdma_v0_core_status_int(chan);
 
-			ret = IRQ_HANDLED;
-		}
+			if (FIELD_GET(HDMA_V0_STOP_INT_MASK, val)) {
+				events |= DW_EDMA_IRQ_STOP;
+				dw_hdma_v0_core_clear_done_int(chan);
+			}
 
-		if (FIELD_GET(HDMA_V0_ABORT_INT_MASK, val)) {
-			dw_hdma_v0_core_clear_abort_int(chan);
-			handler(chan, DW_EDMA_IRQ_ABORT);
+			if (FIELD_GET(HDMA_V0_ABORT_INT_MASK, val)) {
+				events |= DW_EDMA_IRQ_ABORT;
+				dw_hdma_v0_core_clear_abort_int(chan);
+			}
 
-			ret = IRQ_HANDLED;
+			if (events)
+				handler(chan, events);
 		}
+
+		if (!events)
+			continue;
+
+		ret = IRQ_HANDLED;
 	}
 
 	return ret;
-- 
2.51.0