All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Rodin <mrodin@de.adit-jv.com>
To: "Mauro Carvalho Chehab" <mchehab@kernel.org>,
	"Niklas Söderlund" <niklas.soderlund@ragnatech.se>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org
Cc: Michael Rodin <mrodin@de.adit-jv.com>, <michael@rodin.online>,
	<erosca@de.adit-jv.com>
Subject: [PATCH 3/3] rcar-vin: handle transfer errors from subdevices and stop streaming if required
Date: Thu, 19 May 2022 20:00:09 +0200	[thread overview]
Message-ID: <1652983210-1194-4-git-send-email-mrodin@de.adit-jv.com> (raw)
In-Reply-To: <1652983210-1194-1-git-send-email-mrodin@de.adit-jv.com>

When a subdevice sends a transfer error event during streaming and we can
not capture new frames, then we know for sure that this is an unrecoverable
failure and not just a temporary glitch. In this case we can not ignore the
transfer error any more and have to notify userspace. In response to the
transfer error event userspace can try to restart streaming and hope that
it works again.

This patch is based on the patch [1] from Niklas Söderlund, however it adds
more logic to check whether the VIN hardware module is actually affected by
the transfer errors reported by the usptream device. For this it takes some
ideas from the imx driver where EOF interrupts are monitored by the
eof_timeout_timer added by commit 4a34ec8e470c ("[media] media: imx: Add
CSI subdev driver").

[1] https://lore.kernel.org/linux-renesas-soc/20211108160220.767586-4-niklas.soderlund+renesas@ragnatech.se/

Signed-off-by: Michael Rodin <mrodin@de.adit-jv.com>
---
 drivers/media/platform/renesas/rcar-vin/rcar-dma.c | 34 ++++++++++++++++++++++
 .../media/platform/renesas/rcar-vin/rcar-v4l2.c    | 18 +++++++++++-
 drivers/media/platform/renesas/rcar-vin/rcar-vin.h |  7 +++++
 3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
index 2272f1c..596a367 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
@@ -13,6 +13,7 @@
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/pm_runtime.h>
+#include <media/v4l2-event.h>
 
 #include <media/videobuf2-dma-contig.h>
 
@@ -1060,6 +1061,9 @@ static irqreturn_t rvin_irq(int irq, void *data)
 		vin_dbg(vin, "Dropping frame %u\n", vin->sequence);
 	}
 
+	cancel_delayed_work(&vin->frame_timeout);
+	schedule_delayed_work(&vin->frame_timeout, msecs_to_jiffies(FRAME_TIMEOUT_MS));
+
 	vin->sequence++;
 
 	/* Prepare for next frame */
@@ -1283,6 +1287,7 @@ int rvin_start_streaming(struct rvin_dev *vin)
 	spin_lock_irqsave(&vin->qlock, flags);
 
 	vin->sequence = 0;
+	vin->xfer_error = false;
 
 	ret = rvin_capture_start(vin);
 	if (ret)
@@ -1290,6 +1295,10 @@ int rvin_start_streaming(struct rvin_dev *vin)
 
 	spin_unlock_irqrestore(&vin->qlock, flags);
 
+	/* We start the frame watchdog only after we have successfully started streaming */
+	if (!ret)
+		schedule_delayed_work(&vin->frame_timeout, msecs_to_jiffies(FRAME_TIMEOUT_MS));
+
 	return ret;
 }
 
@@ -1332,6 +1341,12 @@ void rvin_stop_streaming(struct rvin_dev *vin)
 	}
 
 	vin->state = STOPPING;
+	/*
+	 * Since we are now stopping and don't expect more frames to be captured, make sure that
+	 * there is no pending work for error handling.
+	 */
+	cancel_delayed_work_sync(&vin->frame_timeout);
+	vin->xfer_error = false;
 
 	/* Wait until only scratch buffer is used, max 3 interrupts. */
 	retries = 0;
@@ -1424,6 +1439,23 @@ void rvin_dma_unregister(struct rvin_dev *vin)
 	v4l2_device_unregister(&vin->v4l2_dev);
 }
 
+static void rvin_frame_timeout(struct work_struct *work)
+{
+	struct delayed_work *dwork = to_delayed_work(work);
+	struct rvin_dev *vin = container_of(dwork, struct rvin_dev, frame_timeout);
+	struct v4l2_event event = {
+		.type = V4L2_EVENT_XFER_ERROR,
+	};
+
+	vin_dbg(vin, "Frame timeout!\n");
+
+	if (!vin->xfer_error)
+		return;
+	vin_err(vin, "Unrecoverable transfer error detected, stopping streaming\n");
+	vb2_queue_error(&vin->queue);
+	v4l2_event_queue(&vin->vdev, &event);
+}
+
 int rvin_dma_register(struct rvin_dev *vin, int irq)
 {
 	struct vb2_queue *q = &vin->queue;
@@ -1470,6 +1502,8 @@ int rvin_dma_register(struct rvin_dev *vin, int irq)
 		goto error;
 	}
 
+	INIT_DELAYED_WORK(&vin->frame_timeout, rvin_frame_timeout);
+
 	return 0;
 error:
 	rvin_dma_unregister(vin);
diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
index 2e2aa9d..bd7f6fe2 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-v4l2.c
@@ -648,6 +648,8 @@ static int rvin_subscribe_event(struct v4l2_fh *fh,
 	switch (sub->type) {
 	case V4L2_EVENT_SOURCE_CHANGE:
 		return v4l2_event_subscribe(fh, sub, 4, NULL);
+	case V4L2_EVENT_XFER_ERROR:
+		return v4l2_event_subscribe(fh, sub, 1, NULL);
 	}
 	return v4l2_ctrl_subscribe_event(fh, sub);
 }
@@ -1000,9 +1002,23 @@ void rvin_v4l2_unregister(struct rvin_dev *vin)
 static void rvin_notify_video_device(struct rvin_dev *vin,
 				     unsigned int notification, void *arg)
 {
+	const struct v4l2_event *event;
+
 	switch (notification) {
 	case V4L2_DEVICE_NOTIFY_EVENT:
-		v4l2_event_queue(&vin->vdev, arg);
+		event = arg;
+
+		switch (event->type) {
+		case V4L2_EVENT_XFER_ERROR:
+			if (vin->state != STOPPED && vin->state != STOPPING) {
+				vin_dbg(vin, "Subdevice signaled transfer error.\n");
+				vin->xfer_error = true;
+			}
+			break;
+		default:
+			break;
+		}
+
 		break;
 	default:
 		break;
diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
index 1f94589..4726a69 100644
--- a/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
+++ b/drivers/media/platform/renesas/rcar-vin/rcar-vin.h
@@ -31,6 +31,9 @@
 /* Max number on VIN instances that can be in a system */
 #define RCAR_VIN_NUM 32
 
+/* maximum time we wait before signalling an error to userspace */
+#define FRAME_TIMEOUT_MS 1000
+
 struct rvin_group;
 
 enum model_id {
@@ -207,6 +210,8 @@ struct rvin_info {
  * @std:		active video standard of the video source
  *
  * @alpha:		Alpha component to fill in for supported pixel formats
+ * @xfer_error:		Indicates if any transfer errors occurred in the current streaming session.
+ * @frame_timeout:	Watchdog for monitoring regular capturing of frames in rvin_irq.
  */
 struct rvin_dev {
 	struct device *dev;
@@ -251,6 +256,8 @@ struct rvin_dev {
 	v4l2_std_id std;
 
 	unsigned int alpha;
+	bool xfer_error;
+	struct delayed_work frame_timeout;
 };
 
 #define vin_to_source(vin)		((vin)->parallel.subdev)
-- 
2.7.4


  parent reply	other threads:[~2022-05-19 18:02 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-19 18:00 Improve error handling in the rcar-vin driver Michael Rodin
2022-05-19 18:00 ` [PATCH 1/3] media: videobuf2: Add a transfer error event Michael Rodin
2022-05-19 18:00 ` [PATCH 2/3] rcar-csi2: Do not try to recover after transfer error Michael Rodin
2022-05-19 18:00 ` Michael Rodin [this message]
2022-05-19 21:00   ` [PATCH 3/3] rcar-vin: handle transfer errors from subdevices and stop streaming if required Niklas Söderlund
2022-05-20 19:50     ` Michael Rodin
2022-06-08 21:04       ` Niklas Söderlund
2022-06-28 18:00         ` [PATCH v2 0/3] Improve error handling in the rcar-vin driver Michael Rodin
2022-06-28 18:00           ` [PATCH 1/3] media: videobuf2: Add a transfer error event Michael Rodin
2022-07-04 15:59             ` Nicolas Dufresne
2022-07-15 16:15               ` Michael Rodin
2022-08-02  9:32                 ` Hans Verkuil
2022-08-08 17:03                   ` Michael Rodin
2022-08-09  7:12                     ` Hans Verkuil
2022-06-28 18:00           ` [PATCH 2/3] rcar-csi2: Do not try to recover after transfer error Michael Rodin
2022-06-28 18:00           ` [PATCH 3/3] media: rcar-vin: Allow userspace to subscribe to V4L2_EVENT_XFER_ERROR Michael Rodin
2022-07-05  9:46           ` [PATCH v2 0/3] Improve error handling in the rcar-vin driver Niklas Söderlund
2022-07-15 13:42             ` Michael Rodin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1652983210-1194-4-git-send-email-mrodin@de.adit-jv.com \
    --to=mrodin@de.adit-jv.com \
    --cc=erosca@de.adit-jv.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=michael@rodin.online \
    --cc=niklas.soderlund@ragnatech.se \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.