[PATCH v3 0/2] rcar-csi2: Update handling of transfer error

[PATCH v3 0/2] rcar-csi2: Update handling of transfer error

[Niklas Söderlund]

Hello,

This series adapts the R-Car CSI-2 receiver recovery logic to match
updates in the datasheet. The later datasheets recommend that the whole
video pipeline shall be stopped if an transmission error is detected
instead of just restarting the CSI-2 receiver.

This is tested on M3-N and a fault is injected by quickly removing and
re-inserting the HDMI cable while streaming. This method does not always
hit and is time consuming. To consistently prove correctness of handling
a fake fault was introduced by a HACK and a debugfs entry.

Niklas Söderlund (2):
  rcar-vin: Stop stream when subdevice signal EOS
  rcar-csi2: Do not try to recover after transfer error

 drivers/media/platform/rcar-vin/rcar-csi2.c | 14 ++++++--------
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 16 +++++++++++++++-
 2 files changed, 21 insertions(+), 9 deletions(-)

-- 
2.30.1

[PATCH v3 1/2] rcar-vin: Stop stream when subdevice signal EOS

[Niklas Söderlund]

When a subdevice signals end of stream stop the VIN in addition to
informing user-space of the event.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
* Changes since v2
- Log using vin_dbg() instead of v4l2_info().
---
 drivers/media/platform/rcar-vin/rcar-v4l2.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
index 457a65bf6b664f05..76f6f46799e95791 100644
--- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
+++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
@@ -969,9 +969,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_EOS:
+			rvin_stop_streaming(vin);
+			vin_dbg(vin,
+				"Subdevice signaled end of stream, stopping.\n");
+			break;
+		default:
+			break;
+		}
+
+		v4l2_event_queue(&vin->vdev, event);
 		break;
 	default:
 		break;
-- 
2.30.1

[PATCH v3 2/2] rcar-csi2: Do not try to recover after transfer error

[Niklas Söderlund]

Instead of restarting the R-Car CSI-2 receiver if a transmission error
is detected, inform the R-Car VIN driver of the error so it can stop the
whole pipeline and inform user-space. This is done to reflect a updated
usage recommendation in later versions of the datasheet.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
---
* Changes since v2
- Update spelling in commit message.
---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index e06cd512aba207a4..594d583921df2495 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -773,21 +773,19 @@ static irqreturn_t rcsi2_irq(int irq, void *data)
 
 	rcsi2_write(priv, INTERRSTATE_REG, err_status);
 
-	dev_info(priv->dev, "Transfer error, restarting CSI-2 receiver\n");
-
 	return IRQ_WAKE_THREAD;
 }
 
 static irqreturn_t rcsi2_irq_thread(int irq, void *data)
 {
 	struct rcar_csi2 *priv = data;
+	struct v4l2_event event = {
+		.type = V4L2_EVENT_EOS,
+	};
 
-	mutex_lock(&priv->lock);
-	rcsi2_stop(priv);
-	usleep_range(1000, 2000);
-	if (rcsi2_start(priv))
-		dev_warn(priv->dev, "Failed to restart CSI-2 receiver\n");
-	mutex_unlock(&priv->lock);
+	dev_err(priv->dev, "Transfer error detected.\n");
+
+	v4l2_subdev_notify_event(&priv->subdev, &event);
 
 	return IRQ_HANDLED;
 }
-- 
2.30.1

Re: [PATCH v3 1/2] rcar-vin: Stop stream when subdevice signal EOS

[Hans Verkuil]

Hi Niklas,

On 10/03/2021 17:45, Niklas Söderlund wrote:
> When a subdevice signals end of stream stop the VIN in addition to
> informing user-space of the event.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
> * Changes since v2
> - Log using vin_dbg() instead of v4l2_info().
> ---
>  drivers/media/platform/rcar-vin/rcar-v4l2.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-v4l2.c b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> index 457a65bf6b664f05..76f6f46799e95791 100644
> --- a/drivers/media/platform/rcar-vin/rcar-v4l2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-v4l2.c
> @@ -969,9 +969,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_EOS:

This is not the right event for this. EVENT_EOS is specific to codecs and does
not signal an error condition.

I think we need a new event for this, I've seen similar situations with HDMI
receivers. The SOURCE_CHANGE event was (ab)used for that, but I think a new
event (V4L2_EVENT_XFER_ERROR?) should be created for these situations.

> +			rvin_stop_streaming(vin);

In addition to this you should call vb2_queue_error(), that's typically used
in situations where there is an unrecoverable transfer error.

Regards,

	Hans

> +			vin_dbg(vin,
> +				"Subdevice signaled end of stream, stopping.\n");
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		v4l2_event_queue(&vin->vdev, event);
>  		break;
>  	default:
>  		break;
>