diff mbox

[v9,25/28] rcar-vin: extend {start,stop}_streaming to work with media controller

Message ID 20171208010842.20047-26-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State New, archived
Headers show

Commit Message

Niklas Söderlund Dec. 8, 2017, 1:08 a.m. UTC 
The procedure to start or stop streaming using the non-MC single
subdevice and the MC graph and multiple subdevices are quite different.
Create a new function to abstract which method is used based on which
mode the driver is running in and add logic to start the MC graph.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/platform/rcar-vin/rcar-dma.c | 112 +++++++++++++++++++++++++++--
 1 file changed, 105 insertions(+), 7 deletions(-)

Comments

Laurent Pinchart Dec. 8, 2017, 8:45 p.m. UTC | #1 
Hi Niklas,

Thank you for the patch.

On Friday, 8 December 2017 03:08:39 EET Niklas Söderlund wrote:
> The procedure to start or stop streaming using the non-MC single
> subdevice and the MC graph and multiple subdevices are quite different.
> Create a new function to abstract which method is used based on which
> mode the driver is running in and add logic to start the MC graph.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c | 112 ++++++++++++++++++++++++--
>  1 file changed, 105 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c
> b/drivers/media/platform/rcar-vin/rcar-dma.c index
> 6c5df13b30d6dd14..8a6674a891aab357 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -1087,15 +1087,115 @@ static void rvin_buffer_queue(struct vb2_buffer
> *vb) spin_unlock_irqrestore(&vin->qlock, flags);
>  }
> 
> +static int rvin_set_stream(struct rvin_dev *vin, int on)
> +{
> +	struct v4l2_subdev_format fmt = {
> +		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
> +	};
> +	struct media_pipeline *pipe;
> +	struct  v4l2_subdev *sd;
> +	struct media_pad *pad;
> +	int ret;
> +
> +	/* No media controller used, simply pass operation to subdevice */
> +	if (!vin->info->use_mc) {
> +		ret = v4l2_subdev_call(vin->digital->subdev, video, s_stream,
> +				       on);
> +
> +		return ret == -ENOIOCTLCMD ? 0 : ret;
> +	}
> +
> +	pad = media_entity_remote_pad(&vin->pad);
> +	if (!pad)
> +		return -EPIPE;
> +
> +	sd = media_entity_to_v4l2_subdev(pad->entity);
> +	if (!sd)
> +		return -EPIPE;

Can a pad have no entity ?

> +	if (!on) {
> +		media_pipeline_stop(&vin->vdev.entity);
> +		return v4l2_subdev_call(sd, video, s_stream, 0);
> +	}
> +
> +	fmt.pad = pad->index;
> +	if (v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt))
> +		return -EPIPE;
> +
> +	switch (fmt.format.code) {
> +	case MEDIA_BUS_FMT_YUYV8_1X16:
> +	case MEDIA_BUS_FMT_UYVY8_2X8:
> +	case MEDIA_BUS_FMT_UYVY10_2X10:
> +	case MEDIA_BUS_FMT_RGB888_1X24:
> +		vin->code = fmt.format.code;
> +		break;
> +	default:
> +		return -EPIPE;
> +	}
> +
> +	switch (fmt.format.field) {
> +	case V4L2_FIELD_TOP:
> +	case V4L2_FIELD_BOTTOM:
> +	case V4L2_FIELD_NONE:
> +	case V4L2_FIELD_INTERLACED_TB:
> +	case V4L2_FIELD_INTERLACED_BT:
> +	case V4L2_FIELD_INTERLACED:
> +	case V4L2_FIELD_SEQ_TB:
> +	case V4L2_FIELD_SEQ_BT:
> +		/* Supported natively */
> +		break;
> +	case V4L2_FIELD_ALTERNATE:
> +		switch (vin->format.field) {
> +		case V4L2_FIELD_TOP:
> +		case V4L2_FIELD_BOTTOM:
> +		case V4L2_FIELD_NONE:
> +			break;
> +		case V4L2_FIELD_INTERLACED_TB:
> +		case V4L2_FIELD_INTERLACED_BT:
> +		case V4L2_FIELD_INTERLACED:
> +		case V4L2_FIELD_SEQ_TB:
> +		case V4L2_FIELD_SEQ_BT:
> +			/* Use VIN hardware to combine the two fields */
> +			fmt.format.height *= 2;
> +			break;
> +		default:
> +			return -EPIPE;
> +		}
> +		break;
> +	default:
> +		return -EPIPE;
> +	}
> +
> +	if (fmt.format.width != vin->format.width ||
> +	    fmt.format.height != vin->format.height)
> +		return -EPIPE;

Shouldn't you also validate the media bus pixel code against the pixel format 
?

> +	pipe = sd->entity.pipe ? sd->entity.pipe : &vin->vdev.pipe;

How is this protected against race conditions if you call VIDIOC_STREAMON on 
two VIN instances connected to the same CSI-2 receiver ?

And how does this handle two VIN instances connected to two different CSI-2 
receivers that are both connected to the same source ? We will use two 
different pipeline objects, is that a problem ?

> +	if (media_pipeline_start(&vin->vdev.entity, pipe))
> +		return -EPIPE;
> +
> +	ret = v4l2_subdev_call(sd, video, s_stream, 1);
> +	if (ret == -ENOIOCTLCMD)
> +		ret = 0;
> +	if (ret)
> +		media_pipeline_stop(&vin->vdev.entity);
> +
> +	return ret;
> +}
> +
>  static int rvin_start_streaming(struct vb2_queue *vq, unsigned int count)
>  {
>  	struct rvin_dev *vin = vb2_get_drv_priv(vq);
> -	struct v4l2_subdev *sd;
>  	unsigned long flags;
>  	int ret;
> 
> -	sd = vin_to_source(vin);
> -	v4l2_subdev_call(sd, video, s_stream, 1);
> +	ret = rvin_set_stream(vin, 1);
> +	if (ret) {
> +		spin_lock_irqsave(&vin->qlock, flags);
> +		return_all_buffers(vin, VB2_BUF_STATE_QUEUED);
> +		spin_unlock_irqrestore(&vin->qlock, flags);
> +		return ret;
> +	}
> 
>  	spin_lock_irqsave(&vin->qlock, flags);
> 
> @@ -1104,7 +1204,7 @@ static int rvin_start_streaming(struct vb2_queue *vq,
> unsigned int count) ret = rvin_capture_start(vin);
>  	if (ret) {
>  		return_all_buffers(vin, VB2_BUF_STATE_QUEUED);
> -		v4l2_subdev_call(sd, video, s_stream, 0);
> +		rvin_set_stream(vin, 0);
>  	}
> 
>  	spin_unlock_irqrestore(&vin->qlock, flags);
> @@ -1115,7 +1215,6 @@ static int rvin_start_streaming(struct vb2_queue *vq,
> unsigned int count) static void rvin_stop_streaming(struct vb2_queue *vq)
>  {
>  	struct rvin_dev *vin = vb2_get_drv_priv(vq);
> -	struct v4l2_subdev *sd;
>  	unsigned long flags;
>  	int retries = 0;
> 
> @@ -1154,8 +1253,7 @@ static void rvin_stop_streaming(struct vb2_queue *vq)
> 
>  	spin_unlock_irqrestore(&vin->qlock, flags);
> 
> -	sd = vin_to_source(vin);
> -	v4l2_subdev_call(sd, video, s_stream, 0);
> +	rvin_set_stream(vin, 0);
> 
>  	/* disable interrupts */
>  	rvin_disable_interrupts(vin);
diff mbox

Patch

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index 6c5df13b30d6dd14..8a6674a891aab357 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -1087,15 +1087,115 @@  static void rvin_buffer_queue(struct vb2_buffer *vb)
 	spin_unlock_irqrestore(&vin->qlock, flags);
 }
 
+static int rvin_set_stream(struct rvin_dev *vin, int on)
+{
+	struct v4l2_subdev_format fmt = {
+		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
+	};
+	struct media_pipeline *pipe;
+	struct  v4l2_subdev *sd;
+	struct media_pad *pad;
+	int ret;
+
+	/* No media controller used, simply pass operation to subdevice */
+	if (!vin->info->use_mc) {
+		ret = v4l2_subdev_call(vin->digital->subdev, video, s_stream,
+				       on);
+
+		return ret == -ENOIOCTLCMD ? 0 : ret;
+	}
+
+	pad = media_entity_remote_pad(&vin->pad);
+	if (!pad)
+		return -EPIPE;
+
+	sd = media_entity_to_v4l2_subdev(pad->entity);
+	if (!sd)
+		return -EPIPE;
+
+	if (!on) {
+		media_pipeline_stop(&vin->vdev.entity);
+		return v4l2_subdev_call(sd, video, s_stream, 0);
+	}
+
+	fmt.pad = pad->index;
+	if (v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt))
+		return -EPIPE;
+
+	switch (fmt.format.code) {
+	case MEDIA_BUS_FMT_YUYV8_1X16:
+	case MEDIA_BUS_FMT_UYVY8_2X8:
+	case MEDIA_BUS_FMT_UYVY10_2X10:
+	case MEDIA_BUS_FMT_RGB888_1X24:
+		vin->code = fmt.format.code;
+		break;
+	default:
+		return -EPIPE;
+	}
+
+	switch (fmt.format.field) {
+	case V4L2_FIELD_TOP:
+	case V4L2_FIELD_BOTTOM:
+	case V4L2_FIELD_NONE:
+	case V4L2_FIELD_INTERLACED_TB:
+	case V4L2_FIELD_INTERLACED_BT:
+	case V4L2_FIELD_INTERLACED:
+	case V4L2_FIELD_SEQ_TB:
+	case V4L2_FIELD_SEQ_BT:
+		/* Supported natively */
+		break;
+	case V4L2_FIELD_ALTERNATE:
+		switch (vin->format.field) {
+		case V4L2_FIELD_TOP:
+		case V4L2_FIELD_BOTTOM:
+		case V4L2_FIELD_NONE:
+			break;
+		case V4L2_FIELD_INTERLACED_TB:
+		case V4L2_FIELD_INTERLACED_BT:
+		case V4L2_FIELD_INTERLACED:
+		case V4L2_FIELD_SEQ_TB:
+		case V4L2_FIELD_SEQ_BT:
+			/* Use VIN hardware to combine the two fields */
+			fmt.format.height *= 2;
+			break;
+		default:
+			return -EPIPE;
+		}
+		break;
+	default:
+		return -EPIPE;
+	}
+
+	if (fmt.format.width != vin->format.width ||
+	    fmt.format.height != vin->format.height)
+		return -EPIPE;
+
+	pipe = sd->entity.pipe ? sd->entity.pipe : &vin->vdev.pipe;
+	if (media_pipeline_start(&vin->vdev.entity, pipe))
+		return -EPIPE;
+
+	ret = v4l2_subdev_call(sd, video, s_stream, 1);
+	if (ret == -ENOIOCTLCMD)
+		ret = 0;
+	if (ret)
+		media_pipeline_stop(&vin->vdev.entity);
+
+	return ret;
+}
+
 static int rvin_start_streaming(struct vb2_queue *vq, unsigned int count)
 {
 	struct rvin_dev *vin = vb2_get_drv_priv(vq);
-	struct v4l2_subdev *sd;
 	unsigned long flags;
 	int ret;
 
-	sd = vin_to_source(vin);
-	v4l2_subdev_call(sd, video, s_stream, 1);
+	ret = rvin_set_stream(vin, 1);
+	if (ret) {
+		spin_lock_irqsave(&vin->qlock, flags);
+		return_all_buffers(vin, VB2_BUF_STATE_QUEUED);
+		spin_unlock_irqrestore(&vin->qlock, flags);
+		return ret;
+	}
 
 	spin_lock_irqsave(&vin->qlock, flags);
 
@@ -1104,7 +1204,7 @@  static int rvin_start_streaming(struct vb2_queue *vq, unsigned int count)
 	ret = rvin_capture_start(vin);
 	if (ret) {
 		return_all_buffers(vin, VB2_BUF_STATE_QUEUED);
-		v4l2_subdev_call(sd, video, s_stream, 0);
+		rvin_set_stream(vin, 0);
 	}
 
 	spin_unlock_irqrestore(&vin->qlock, flags);
@@ -1115,7 +1215,6 @@  static int rvin_start_streaming(struct vb2_queue *vq, unsigned int count)
 static void rvin_stop_streaming(struct vb2_queue *vq)
 {
 	struct rvin_dev *vin = vb2_get_drv_priv(vq);
-	struct v4l2_subdev *sd;
 	unsigned long flags;
 	int retries = 0;
 
@@ -1154,8 +1253,7 @@  static void rvin_stop_streaming(struct vb2_queue *vq)
 
 	spin_unlock_irqrestore(&vin->qlock, flags);
 
-	sd = vin_to_source(vin);
-	v4l2_subdev_call(sd, video, s_stream, 0);
+	rvin_set_stream(vin, 0);
 
 	/* disable interrupts */
 	rvin_disable_interrupts(vin);