media: apple: avd: safety fixes for the current driver (mailbox, early completion, reset, AV1, FFmpeg HEVC)

drivers/media/platform/apple/avd/avd-av1.c

@@ -246,6 +246,19 @@ struct avd_av1_ctx {
 	} bufs;
 };
 
+/*
+ * Timestamp of the reference a ref_frame_idx entry names. The entries are
+ * -1 on frames that use no references (FFmpeg fills key frames that way);
+ * indexing reference_frame_ts with them read out of bounds. Return a
+ * timestamp no buffer has, so the lookup takes the missing-reference path.
+ */
+static u64 avd_av1_ref_ts(const struct v4l2_ctrl_av1_frame *frame, int idx)
+{
+	if (idx < 0 || idx >= V4L2_AV1_TOTAL_REFS_PER_FRAME)
+		return U64_MAX;
+	return frame->reference_frame_ts[idx];
+}
+
 /*
  * from verisilicon/rockchip_vpu981_hw_av1_dec.c
  */
@@ -367,8 +380,7 @@ static void set_refs(struct avd_ctx *ctx, struct avd_av1_run *run)
 			ref_idx = i + V4L2_AV1_REF_LAST_FRAME;
 			ref = avd_get_ref_buf(
 				ctx, &dst->base.vb,
-				frame->reference_frame_ts
-					[frame->ref_frame_idx[i]]);
+				avd_av1_ref_ts(frame, frame->ref_frame_idx[i]));
 		}
 		addr = vb2_dma_contig_plane_dma_addr(&ref->base.vb.vb2_buf, 0) +
 		       ref->comp.start_offset;
@@ -455,9 +467,8 @@ static bool is_valid_ref_frame(struct avd_ctx *ctx, struct avd_av1_run *run,
 	const struct v4l2_ctrl_av1_frame *frame = run->frame;
 	struct avd_decoded_buffer *ref = avd_get_ref_buf(
 		ctx, &dst->base.vb,
-		frame->reference_frame_ts
-			[frame->ref_frame_idx[ref_frame -
-					      V4L2_AV1_REF_LAST_FRAME]]);
+		avd_av1_ref_ts(frame, frame->ref_frame_idx[ref_frame -
+						      V4L2_AV1_REF_LAST_FRAME]));
 	int ref_mi_cols = DIV_ROUND_UP(ref->av1.width + 1, 8);
 	int ref_mi_rows = DIV_ROUND_UP(ref->av1.height + 1, 8);
 	bool intra_only = ((ref->av1.frame_type == V4L2_AV1_KEY_FRAME) ||
@@ -485,7 +496,7 @@ static int avd_select_refs(struct avd_ctx *ctx, struct avd_av1_run *run,
 	if (seq->flags & (V4L2_AV1_SEQUENCE_FLAG_ENABLE_ORDER_HINT |
 			  V4L2_AV1_SEQUENCE_FLAG_ENABLE_REF_FRAME_MVS)) {
 		ref = avd_get_ref_buf(ctx, &dst->base.vb,
-				      frame->reference_frame_ts[last_ref_idx]);
+				      avd_av1_ref_ts(frame, last_ref_idx));
 		if (dst != ref) {
 			use_last =
 				(ref->av1.order_hints[V4L2_AV1_REF_ALTREF_FRAME] !=
@@ -554,7 +565,7 @@ static void set_ref_hints(struct avd_ctx *ctx, struct avd_av1_run *run,
 						       V4L2_AV1_REF_LAST_FRAME];
 			ref = avd_get_ref_buf(
 				ctx, &dst->base.vb,
-				frame->reference_frame_ts[ref_idx]);
+				avd_av1_ref_ts(frame, ref_idx));
 
 			push(AV1_ORDER_HINT0(ref->av1.order_hints[3]) |
 				     AV1_ORDER_HINT1(ref->av1.order_hints[2]) |
@@ -585,8 +596,8 @@ static void set_ref_hdr(struct avd_ctx *ctx, struct avd_av1_run *run)
 	/* point to the first index where the frames equal */
 	for (i = 0; i < 6; i++) {
 		for (j = i + 1; j < 7; j++) {
-			if (frame->reference_frame_ts[frame->ref_frame_idx[i]] ==
-			    frame->reference_frame_ts[frame->ref_frame_idx[j]]) {
+			if (avd_av1_ref_ts(frame, frame->ref_frame_idx[i]) ==
+			    avd_av1_ref_ts(frame, frame->ref_frame_idx[j])) {
 				ref_slots[j] = ref_slots[i];
 				break;
 			}
@@ -831,7 +842,7 @@ static void set_header(struct avd_ctx *ctx, struct avd_av1_run *run)
 						       V4L2_AV1_REF_LAST_FRAME];
 			ref = avd_get_ref_buf(
 				ctx, &dst->base.vb,
-				frame->reference_frame_ts[ref_idx]);
+				avd_av1_ref_ts(frame, ref_idx));
 			ref_addr =
 				vb2_dma_contig_plane_dma_addr(
 					&ref->base.vb.vb2_buf, 0) +
@@ -1075,9 +1086,8 @@ static void avd_av1_set_prob(struct avd_ctx *ctx, struct avd_av1_run *run)
 	} else {
 		ref = avd_get_ref_buf(
 			ctx, &dst->base.vb,
-			run->frame->reference_frame_ts
-				[run->frame->ref_frame_idx
-					 [run->frame->primary_ref_frame]]);
+			avd_av1_ref_ts(run->frame,
+				       run->frame->ref_frame_idx[run->frame->primary_ref_frame]));
 		memcpy(av1_ctx->bufs.probs.cpu,
 		       vb2_plane_vaddr(&ref->base.vb.vb2_buf, 0) +
 			       AVD_AV1_CDFS_OFFSET(
@@ -1118,8 +1128,8 @@ static void avd_av1_set_seg(struct avd_ctx *ctx, struct avd_av1_run *run)
 	if (frame->primary_ref_frame != AV1_PRIMARY_REF_NONE) {
 		ref = avd_get_ref_buf(
 			ctx, &dst->base.vb,
-			frame->reference_frame_ts
-				[frame->ref_frame_idx[frame->primary_ref_frame]]);
+			avd_av1_ref_ts(frame,
+				       frame->ref_frame_idx[frame->primary_ref_frame]));
 		/* avd_get_ref_buf() returns dst for a missing reference */
 		/* same MiCols and MiRows, as libdav1d compares them */
 		if (ref == dst ||
@@ -1384,9 +1394,9 @@ static int avd_av1_run(struct avd_ctx *ctx)
 	ref = dst;
 	if (run.frame->primary_ref_frame != AV1_PRIMARY_REF_NONE)
 		ref = avd_get_ref_buf(ctx, &dst->base.vb,
-				      run.frame->reference_frame_ts
-					      [run.frame->ref_frame_idx
-						       [run.frame->primary_ref_frame]]);
+				      avd_av1_ref_ts(run.frame,
+						     run.frame->ref_frame_idx
+							[run.frame->primary_ref_frame]));
 	if (!vb2_plane_vaddr(&dst->base.vb.vb2_buf, 0) ||
 	    !vb2_plane_vaddr(&ref->base.vb.vb2_buf, 0)) {
 		avd_run_postamble(ctx, &run.base);

drivers/media/platform/apple/avd/avd-drv.c

@@ -15,6 +15,7 @@
 #include <linux/iommu.h>
 #include <linux/reset.h>
 #include <linux/delay.h>
+#include <linux/iopoll.h>
 
 #include <media/videobuf2-dma-contig.h>
 #include <media/videobuf2-v4l2.h>
@@ -222,29 +223,88 @@ int avd_submit_job(struct avd_ctx *ctx)
 	return 0;
 }
 
+/* Recover at most twice a minute, never twice within this gap. */
+#define AVD_RECOVER_MAX		2
+#define AVD_RECOVER_WINDOW	msecs_to_jiffies(60000)
+#define AVD_RECOVER_GAP_MS	20
+
+/*
+ * Reset the decoder after a timeout or a VP error. Every reset is a risk
+ * to the whole machine (the J616s hung hard after timeouts and resets
+ * three times in one night, and the Neo after a burst of one reset per
+ * frame), so: stop the interrupt handler and the CM3 first, block the
+ * decoder's DMA at its DART so a transfer still in flight faults instead
+ * of reaching memory that may already be reused, give such transfers time
+ * to drain, and only then assert the pmgr reset. Recover sparingly, and
+ * retire the decoder after a failed or a third recovery within a minute.
+ */
 static int avd_reset(struct avd_dev *avd)
 {
 	int ret = 0;
 
+	if (avd->retired)
+		return -ENODEV;
+	if (!avd->recoveries ||
+	    time_after(jiffies, avd->recover_window_start + AVD_RECOVER_WINDOW)) {
+		avd->recover_window_start = jiffies;
+		avd->recoveries = 0;
+	}
+	if (++avd->recoveries > AVD_RECOVER_MAX) {
+		ret = -EBUSY;
+		goto retire;
+	}
+	if (avd->last_recover &&
+	    time_before(jiffies, avd->last_recover + msecs_to_jiffies(AVD_RECOVER_GAP_MS)))
+		msleep(AVD_RECOVER_GAP_MS);
+
 	ret = pm_runtime_resume_and_get(avd->dev);
 	if (ret < 0)
-		return ret;
+		goto retire;
 
+	/* No handler may touch the mailbox while the block is in reset. */
+	disable_irq(avd->irq);
+	dev_warn(avd->dev, "recover: stop firmware\n");
+	avd_shutdown(avd);
+	if (avd->empty_domain) {
+		dev_warn(avd->dev, "recover: block DMA\n");
+		ret = iommu_attach_device(avd->empty_domain, avd->dev);
+		if (ret) {
+			dev_err(avd->dev, "recover: DART empty attach failed: %d\n", ret);
+			goto put;
+		}
+		usleep_range(5000, 6000);
+	}
+	dev_warn(avd->dev, "recover: reset\n");
 	ret = reset_control_reset(avd->rstc);
 	if (ret)
-		dev_err(avd->dev, "reset: failed: %d", ret);
-
+		dev_err(avd->dev, "recover: reset failed: %d\n", ret);
 	if (avd->empty_domain) {
-		iommu_attach_device(avd->empty_domain, avd->dev);
+		dev_warn(avd->dev, "recover: restore DMA\n");
 		iommu_detach_device(avd->empty_domain, avd->dev);
+		if (avd->domain && iommu_get_domain_for_dev(avd->dev) != avd->domain) {
+			dev_err(avd->dev, "recover: DART domain not restored\n");
+			ret = -EIO;
+		}
+	}
+	if (!ret) {
+		dev_warn(avd->dev, "recover: boot firmware\n");
+		ret = avd_boot(avd);
+		if (ret)
+			dev_err(avd->dev, "recover: boot failed: %d\n", ret);
+	}
+put:
+	avd->last_recover = jiffies ?: 1;
+	if (!ret) {
+		enable_irq(avd->irq);
+		dev_warn(avd->dev, "recover: done\n");
 	}
-
-	ret = avd_boot(avd);
-	if (ret)
-		dev_err(avd->dev, "reset: failed to boot");
-
 	pm_runtime_put_autosuspend(avd->dev);
-
+	if (!ret)
+		return 0;
+retire:
+	avd->retired = true;
+	dev_err(avd->dev, "decoder retired after %s (%d): reload the module to use it again\n",
+		ret == -EBUSY ? "repeated resets" : "a failed reset", ret);
 	return ret;
 }
 
@@ -284,6 +344,12 @@ static void avd_watchdog_func(struct work_struct *work)
 	ret = avd_reset(avd);
 	if (ret)
 		dev_err(avd->dev, "failed to reset: %d", ret);
+	/*
+	 * A stream whose frame failed in hardware usually fails the next one
+	 * too (a reset per frame took the Neo down), so stop it here.
+	 */
+	ctx->broken = true;
+	dev_err(avd->dev, "stream stopped after a failed frame; restart streaming to retry\n");
 
 	avd_job_finish(ctx, VB2_BUF_STATE_ERROR);
 }
@@ -300,6 +366,19 @@ static irqreturn_t avd_irq_handler(int irq, void *data)
 	if (!ctx)
 		return IRQ_HANDLED;
 
+	/*
+	 * Reading RETRIEVE on an empty mailbox returns the previous message
+	 * again. A repeated PP DONE finishes the next frame before its PP has
+	 * run: userspace gets a wrong picture, the frame's own PP DONE never
+	 * comes, and the watchdog resets the decoder. Give a late message
+	 * 10 us, otherwise acknowledge and ignore the interrupt.
+	 */
+	if (readl_poll_timeout_atomic(avd->mbox + AVD_REG_MBOX1_STATUS, status,
+				      !(status & AVD_MBOX1_STATUS_EMPTY), 1, 10)) {
+		writel(AVD_MBOX1_NOT_EMPTY, avd->mbox + AVD_REG_MBOX_IRQ_CLR);
+		dev_dbg_ratelimited(avd->dev, "spurious interrupt, mailbox empty\n");
+		return IRQ_HANDLED;
+	}
 	status = readl(avd->mbox + AVD_REG_MBOX1_RETRIEVE);
 
 	writel(AVD_MBOX1_NOT_EMPTY, avd->mbox + AVD_REG_MBOX_IRQ_CLR);
@@ -317,6 +396,10 @@ static irqreturn_t avd_irq_handler(int irq, void *data)
 		/* a vp is done, kick the pp and hope for the best */
 		if(ctx->coded_fmt_desc->ops->submit)
 			ctx->coded_fmt_desc->ops->submit(ctx);
+		spin_lock_irqsave(&avd->job_lock, flags);
+		if (avd->job_ctx == ctx)
+			avd->job_vp_done = true;
+		spin_unlock_irqrestore(&avd->job_lock, flags);
 		goto done;
 	} else {
 		dev_err(avd->dev, "H%d error", status);
@@ -338,7 +421,13 @@ static irqreturn_t avd_irq_handler(int irq, void *data)
 	 * let avd_device_run() finish the job when it returns.
 	 */
 	spin_lock_irqsave(&avd->job_lock, flags);
-	if (avd->job_ctx != ctx) {
+	if (avd->job_ctx != ctx || !avd->job_vp_done) {
+		/*
+		 * Only the PP kicked for this job's VP can finish it. A surplus
+		 * PP DONE of a multi-kick picture used to finish the next job
+		 * early, so userspace reused its buffers while the decoder was
+		 * still writing them.
+		 */
 		stale = true;
 	} else if (avd->job_state == AVD_JOB_SUBMITTED) {
 		avd->job_state = AVD_JOB_FINISHING;
@@ -378,6 +467,16 @@ static void avd_device_run(void *priv)
 	if (WARN_ON(!desc))
 		return;
 
+	if (READ_ONCE(avd->retired) || ctx->broken) {
+		struct vb2_v4l2_buffer *src = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
+
+		/* Complete the request's controls too, or its fd never signals. */
+		if (src && src->vb2_buf.req_obj.req)
+			v4l2_ctrl_request_complete(src->vb2_buf.req_obj.req, &ctx->ctrl_hdl);
+		avd_job_finish_no_pm(ctx, VB2_BUF_STATE_ERROR);
+		return;
+	}
+
 	/*
 	 * IRQ completion cannot wait for an already running watchdog. Drain it
 	 * here, before reusing wd_seq, so an old callback cannot time out this
@@ -401,6 +500,7 @@ static void avd_device_run(void *priv)
 	avd->job_ctx = ctx;
 	avd->job_state = AVD_JOB_RUNNING;
 	avd->job_end_sent = false;
+	avd->job_vp_done = false;
 	avd->job_pending = false;
 	spin_unlock_irqrestore(&avd->job_lock, flags);
 	dev_dbg(avd->dev, "avd-job: run seq %llu\n", seq);
@@ -954,6 +1054,7 @@ static int avd_probe(struct platform_device *pdev)
 	irq = platform_get_irq(pdev, 1);
 	if (irq < 0)
 		return irq;
+	avd->irq = irq;
 	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
 			avd_irq_handler, IRQF_ONESHOT,
 			dev_name(&pdev->dev), avd);

drivers/media/platform/apple/avd/avd-hevc.c

@@ -1282,6 +1282,51 @@ static int avd_hevc_compute_tiles(struct avd_ctx *ctx, struct avd_hevc_run *run)
 	return 0;
 }
 
+/*
+ * The slice builder below expects the GStreamer layout: the slices tile the
+ * OUTPUT buffer as whole NALs, bit_size covers the whole NAL and
+ * data_byte_offset is the slice data's offset inside it. FFmpeg's request
+ * API backend instead gives each slice's data_byte_offset from the start of
+ * the buffer and a bit_size of the slice data only, so every slice was given
+ * a wrong address and size and the VP raised its error interrupt on every
+ * frame. Detect the absolute layout (complete and unambiguous) and rewrite
+ * the run's private copy into the equivalent NAL-relative description.
+ */
+static void avd_hevc_normalize_slices(struct avd_hevc_run *run, u32 payload)
+{
+	struct v4l2_ctrl_hevc_slice_params *sl =
+		(struct v4l2_ctrl_hevc_slice_params *)run->sl;
+	u64 nal_end = 0, data_end = 0, prev_end = 0;
+	bool nal = true, absolute = true;
+	int i;
+
+	if (!run->num_slices || !payload)
+		return;
+	for (i = 0; i < run->num_slices; i++) {
+		u64 bytes = DIV_ROUND_UP((u64)sl[i].bit_size, 8);
+		u64 start = sl[i].data_byte_offset;
+
+		if (!bytes || start >= bytes || nal_end + bytes > payload)
+			nal = false;
+		nal_end += bytes;
+		if (!bytes || start < data_end || start + bytes > payload)
+			absolute = false;
+		data_end = start + bytes;
+	}
+	nal &= nal_end == payload;
+	absolute &= data_end == payload;
+	if (!absolute || nal)
+		return;
+	for (i = 0; i < run->num_slices; i++) {
+		u64 bytes = DIV_ROUND_UP((u64)sl[i].bit_size, 8);
+		u64 start = sl[i].data_byte_offset;
+
+		sl[i].data_byte_offset = start - prev_end;
+		sl[i].bit_size = (start - prev_end + bytes) * 8;
+		prev_end = start + bytes;
+	}
+}
+
 static int avd_hevc_run_preamble(struct avd_ctx *ctx, struct avd_hevc_run *run)
 {
 	struct v4l2_ctrl *ctrl, *sl, *ep;
@@ -1353,6 +1398,8 @@ static int avd_hevc_run(struct avd_ctx *ctx)
 	if (ret)
 		return ret;
 
+	avd_hevc_normalize_slices(&run,
+				  vb2_get_plane_payload(&run.base.bufs.src->vb2_buf, 0));
 	dst = vb2_to_avd_decoded_buf(&run.base.bufs.dst->vb2_buf);
 	update_dec_buf_info(dst, &run.sl[0]);
 

drivers/media/platform/apple/avd/avd-regs.h

@@ -13,6 +13,8 @@
 #define AVD_MBOX1_NOT_EMPTY	BIT(3)
 
 #define AVD_REG_MBOX1_STATUS	0x5c
+/* Read side: bits 8-11 write pointer, bits 12-15 read pointer, bit 17 empty. */
+#define AVD_MBOX1_STATUS_EMPTY	BIT(17)
 #define AVD_REG_MBOX1_RETRIEVE	0x64
 #define AVD_MBOX_ENABLE	BIT(0)
 

drivers/media/platform/apple/avd/avd-v4l2.c

@@ -889,6 +889,10 @@ static void avd_buf_request_complete(struct vb2_buffer *vb)
 static int avd_start_streaming(struct vb2_queue *q, unsigned int count)
 {
 	struct avd_ctx *ctx = vb2_get_drv_priv(q);
+
+	/* A new stream gets a fresh start after a hardware failure. */
+	if (!V4L2_TYPE_IS_CAPTURE(q->type))
+		ctx->broken = false;
 	const struct avd_coded_fmt_desc *desc;
 	struct vb2_v4l2_buffer *vbuf;
 	int ret;