[PATCH 0/2] media: qcom: camss: vfe-17x: fix wm_done dispatch and silence unmapped-WM noise

[PATCH 0/2] media: qcom: camss: vfe-17x: fix wm_done dispatch and silence unmapped-WM noise

[Herman van Hazendonk]

The vfe-17x wm_done IRQ path has two independent bugs fixed here:

PATCH 1/2 fixes a gate condition in the wm_done dispatch loop that
used a constant BIT(9) — STATUS_0_IMAGE_MASTER_PING_PONG(1) — instead
of the per-WM macro STATUS_0_IMAGE_MASTER_PING_PONG(wm). As a result
only write master 1 ever had its wm_done callback fired; every other
write master's interrupt was silently dropped, stalling any video
stream not routed through WM 1.

PATCH 2/2 moves the VFE_LINE_NONE check ahead of the output spinlock
in vfe_isr_wm_done(). Write masters allocated to statistics or other
secondary paths are not mapped to an output line; the existing code
took the spinlock before noticing this and printed a rate-limited
error each time. The fix returns silently before acquiring the lock,
eliminating harmless but noisy dmesg spam on such configurations.

Herman van Hazendonk (2):
  media: qcom: camss: vfe-17x: dispatch wm_done per write master
  media: qcom: camss: vfe-17x: silence wm-done IRQ on unmapped WMs

 .../media/platform/qcom/camss/camss-vfe-17x.c | 31 ++++++++++++++-----
 1 file changed, 23 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH 1/2] media: qcom: camss: vfe-17x: dispatch wm_done per write master

[Herman van Hazendonk]

The wm_done dispatch loop checks a constant BIT(9) inside the
per-write-master iteration:

  for (wm = 0; wm < MSM_VFE_IMAGE_MASTERS_NUM; wm++)
          if (status0 & BIT(9))
                  if (vfe_bus_status[1] & STATUS1_WM_CLIENT_BUF_DONE(wm))
                          vfe->isr_ops.wm_done(vfe, wm);

BIT(9) is STATUS_0_IMAGE_MASTER_PING_PONG(1), so the outer check only
fires for write master 1. Any wm_done interrupt raised for a different
write master is dropped: the per-WM bit in vfe_bus_status[1] may be
set, but the gating status0 read does not match for that index and
isr_ops.wm_done() is never called. That stalls every video stream not
routed through WM 1.

Use the per-wm STATUS_0_IMAGE_MASTER_PING_PONG(wm) macro so the gate
tracks the loop iterator.

Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
 drivers/media/platform/qcom/camss/camss-vfe-17x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe-17x.c b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
index e5ee7e717b3b..3ad0c3be005f 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-17x.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
@@ -364,7 +364,7 @@ static irqreturn_t vfe_isr(int irq, void *dev)
 			vfe->isr_ops.comp_done(vfe, i);
 
 	for (wm = 0; wm < MSM_VFE_IMAGE_MASTERS_NUM; wm++)
-		if (status0 & BIT(9))
+		if (status0 & STATUS_0_IMAGE_MASTER_PING_PONG(wm))
 			if (vfe_bus_status[1] & STATUS1_WM_CLIENT_BUF_DONE(wm))
 				vfe->isr_ops.wm_done(vfe, wm);
 
-- 
2.43.0

[PATCH 2/2] media: qcom: camss: vfe-17x: silence wm-done IRQ on unmapped WMs

[Herman van Hazendonk]

vfe_isr_wm_done() handles the per-write-master "frame complete" IRQ.
On VFE 1.7x configurations where a Write Master is allocated to a
secondary purpose (e.g. statistics or DMI buffers) it is not mapped
to any output line, so vfe->wm_output_map[wm] is VFE_LINE_NONE.

Today the function takes the output spinlock first and only then
checks for VFE_LINE_NONE, emitting a rate-limited "Received wm done
for unmapped index" error each time. The error is harmless but it
spams dmesg when a stats path is configured.

Move the VFE_LINE_NONE test in front of the spinlock and return
silently: an unmapped WM simply has nothing to deliver to userspace.
While at it, hoist the wm_output_map[] dereference into a local for
readability.

No functional change for mapped Write Masters.

Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
 .../media/platform/qcom/camss/camss-vfe-17x.c | 29 ++++++++++++++-----
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/qcom/camss/camss-vfe-17x.c b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
index 3ad0c3be005f..f639ac02410b 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe-17x.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe-17x.c
@@ -507,21 +507,36 @@ static void vfe_isr_reg_update(struct vfe_device *vfe, enum vfe_line_id line_id)
  */
 static void vfe_isr_wm_done(struct vfe_device *vfe, u8 wm)
 {
-	struct vfe_line *line = &vfe->line[vfe->wm_output_map[wm]];
+	struct vfe_line *line;
 	struct camss_buffer *ready_buf;
 	struct vfe_output *output;
 	unsigned long flags;
+	int wm_output;
 	u32 index;
 	u64 ts = ktime_get_ns();
 
+	/*
+	 * Some VFE modes route data to secondary WMs that aren't mapped
+	 * to output lines. Silently ignore their IRQs.
+	 *
+	 * vfe->wm_output_map[wm] is written from the stream stop/start
+	 * paths under output_lock held as a mutex; this ISR runs in
+	 * atomic context and cannot take it. Snapshot the value once
+	 * with READ_ONCE() so the check below and the array index that
+	 * follows operate on the same value: otherwise a concurrent
+	 * write of VFE_LINE_NONE (-1) between the two loads would let
+	 * the function pass the check and then dereference
+	 * &vfe->line[-1].
+	 */
+	wm_output = READ_ONCE(vfe->wm_output_map[wm]);
+	if (wm_output == VFE_LINE_NONE)
+		return;
+
+	line = &vfe->line[wm_output];
+
 	spin_lock_irqsave(&vfe->output_lock, flags);
 
-	if (vfe->wm_output_map[wm] == VFE_LINE_NONE) {
-		dev_err_ratelimited(vfe->camss->dev,
-				    "Received wm done for unmapped index\n");
-		goto out_unlock;
-	}
-	output = &vfe->line[vfe->wm_output_map[wm]].output;
+	output = &line->output;
 
 	ready_buf = output->buf[0];
 	if (!ready_buf) {
-- 
2.43.0