[RFC PATCH 4/5] media: iris: Let a resolution change drain before stopping CAPTURE

[Christian Cortner <ccortner@gmail.com>]

The stateful decoder interface lets a client abort the implicit drain of
a dynamic resolution change by stopping the CAPTURE queue, and ffmpeg's
h264_v4l2m2m decoder does so about 10 us after it dequeues the source
change event. If the stop reaches the firmware before the firmware has
reported the end of the drain (HFI_BUF_FW_FLAG_PSC_LAST), the firmware
returns all capture buffers without that flag and
IRIS_INST_SUB_DRC_LAST is never set. At the next
VIDIOC_STREAMON(CAPTURE), iris_process_streamon_output() then finds no
completed resolution change and resumes the firmware without
reallocating the internal input buffers. The firmware still returns the
PSC_LAST buffer it owes, 1.3 ms later in our trace,
iris_inst_sub_state_change_drc_last() rejects it because the input is no
longer paused, and the instance goes to IRIS_INST_ERROR: no buffer is
returned after that, and the client waits forever. Completing the
resolution change at that STREAMON instead, by reallocating the internal
buffers before resuming the firmware, did not help: the firmware then
returned nothing at all.

Wait in iris_vb2_stop_streaming(), for at most 500 ms, until the
firmware has completed the drain of a pending resolution change before
stopping CAPTURE. The client's STREAMON then takes the normal resolution
change path. The wait queue is woken wherever IRIS_INST_SUB_DRC_LAST is
set, for both HFI generations; gen1 sets it in the same event handler
that sets IRIS_INST_SUB_DRC, so there the wait ends at once.

Fixes: 84e17adae3e3 ("media: iris: add support for dynamic resolution change")
Assisted-by: LLM sparse smatch
---
Reference only, not for merging (no Signed-off-by); see the cover
letter.

 .../media/platform/qcom/iris/iris_instance.h  |  3 +++
 drivers/media/platform/qcom/iris/iris_state.c |  8 +++++++-
 drivers/media/platform/qcom/iris/iris_vb2.c   | 19 +++++++++++++++++++
 drivers/media/platform/qcom/iris/iris_vidc.c  |  1 +
 4 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h
index 771269e705d9..1013ed2911a1 100644
--- a/drivers/media/platform/qcom/iris/iris_instance.h
+++ b/drivers/media/platform/qcom/iris/iris_instance.h
@@ -7,6 +7,7 @@
 #define __IRIS_INSTANCE_H__
 
 #include <linux/kref.h>
+#include <linux/wait.h>
 
 #include <media/v4l2-ctrls.h>
 
@@ -56,6 +57,7 @@ struct iris_hfi_session_ops;
  * @metadata_idx: index for metadata buffer
  * @codec: codec type
  * @last_buffer_dequeued: a flag to indicate that last buffer is sent by driver
+ * @drc_wq: woken when IRIS_INST_SUB_DRC_LAST is set
  * @last_buf_ns: start time of received input buffer for current one second FPS window
  * @frame_counter: input buffer counter for current one second FPS window
  * @frame_rate: frame rate of current instance
@@ -104,6 +106,7 @@ struct iris_inst {
 	u32				metadata_idx;
 	u32				codec;
 	bool				last_buffer_dequeued;
+	wait_queue_head_t		drc_wq;
 	u64				last_buf_ns;
 	u32				frame_counter;
 	u32				frame_rate;
diff --git a/drivers/media/platform/qcom/iris/iris_state.c b/drivers/media/platform/qcom/iris/iris_state.c
index 5552725c614e..dd8e0093b972 100644
--- a/drivers/media/platform/qcom/iris/iris_state.c
+++ b/drivers/media/platform/qcom/iris/iris_state.c
@@ -209,6 +209,7 @@ int iris_inst_sub_state_change_drain_last(struct iris_inst *inst)
 int iris_inst_sub_state_change_drc_last(struct iris_inst *inst)
 {
 	enum iris_inst_sub_state set_sub_state;
+	int ret;
 
 	if (inst->sub_state & IRIS_INST_SUB_DRC_LAST)
 		return -EINVAL;
@@ -222,7 +223,12 @@ int iris_inst_sub_state_change_drc_last(struct iris_inst *inst)
 
 	set_sub_state = IRIS_INST_SUB_DRC_LAST | IRIS_INST_SUB_OUTPUT_PAUSE;
 
-	return iris_inst_change_sub_state(inst, 0, set_sub_state);
+	ret = iris_inst_change_sub_state(inst, 0, set_sub_state);
+	/* a CAPTURE STREAMOFF may be waiting, see iris_vb2_stop_streaming() */
+	if (!ret)
+		wake_up_all(&inst->drc_wq);
+
+	return ret;
 }
 
 int iris_inst_sub_state_change_pause(struct iris_inst *inst, u32 plane)
diff --git a/drivers/media/platform/qcom/iris/iris_vb2.c b/drivers/media/platform/qcom/iris/iris_vb2.c
index b820b3e43910..bc2f201fea0e 100644
--- a/drivers/media/platform/qcom/iris/iris_vb2.c
+++ b/drivers/media/platform/qcom/iris/iris_vb2.c
@@ -228,6 +228,15 @@ int iris_vb2_start_streaming(struct vb2_queue *q, unsigned int count)
 	return ret;
 }
 
+/* a source change whose drain the firmware has not completed yet */
+static bool iris_drc_draining(struct iris_inst *inst)
+{
+	enum iris_inst_sub_state sub_state = READ_ONCE(inst->sub_state);
+
+	return sub_state & IRIS_INST_SUB_DRC &&
+	       !(sub_state & IRIS_INST_SUB_DRC_LAST);
+}
+
 void iris_vb2_stop_streaming(struct vb2_queue *q)
 {
 	struct iris_inst *inst;
@@ -238,6 +247,16 @@ void iris_vb2_stop_streaming(struct vb2_queue *q)
 	if (V4L2_TYPE_IS_CAPTURE(q->type) && inst->state == IRIS_INST_INIT)
 		return;
 
+	/*
+	 * A client may abort the drain of a resolution change by stopping
+	 * CAPTURE, but if the stop reaches the firmware before the drain is
+	 * complete, the resolution change can never finish and decoding
+	 * stalls after the next STREAMON. Let the drain complete first.
+	 */
+	if (V4L2_TYPE_IS_CAPTURE(q->type) && inst->domain == DECODER)
+		wait_event_timeout(inst->drc_wq, !iris_drc_draining(inst),
+				   msecs_to_jiffies(500));
+
 	mutex_lock(&inst->lock);
 
 	if (!V4L2_TYPE_IS_OUTPUT(q->type) &&
diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c
index 05aab6e3005d..0df0a5915e16 100644
--- a/drivers/media/platform/qcom/iris/iris_vidc.c
+++ b/drivers/media/platform/qcom/iris/iris_vidc.c
@@ -201,6 +201,7 @@ int iris_open(struct file *filp)
 	INIT_LIST_HEAD(&inst->buffers[BUF_PARTIAL].list);
 	init_completion(&inst->completion);
 	init_completion(&inst->flush_completion);
+	init_waitqueue_head(&inst->drc_wq);
 
 	iris_v4l2_fh_init(inst, filp);
 
-- 
2.56.0