Re: [PATCH v12 30/33] rcar-vin: extend {start,stop}_streaming to work with media controller

[Hans Verkuil]

On 07/03/18 23:05, 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 
> Reviewed-by: Laurent Pinchart 

Reviewed-by: Hans Verkuil 

Regards,

Hans

> ---
>  drivers/media/platform/rcar-vin/rcar-dma.c | 133 
> +++--
>  1 file changed, 126 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c 
> b/drivers/media/platform/rcar-vin/rcar-dma.c
> index da113531f0ce7dc0..580b286acbf2dab6 100644
> --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> @@ -1082,15 +1082,136 @@ static void rvin_buffer_queue(struct vb2_buffer *vb)
>   spin_unlock_irqrestore(>qlock, flags);
>  }
>  
> +static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev 
> *sd,
> +struct media_pad *pad)
> +{
> + struct v4l2_subdev_format fmt = {
> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> + };
> +
> + fmt.pad = pad->index;
> + if (v4l2_subdev_call(sd, pad, get_fmt, NULL, ))
> + 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->mbus_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 ||
> + fmt.format.code != vin->mbus_code)
> + return -EPIPE;
> +
> + return 0;
> +}
> +
> +static int rvin_set_stream(struct rvin_dev *vin, int on)
> +{
> + struct media_pipeline *pipe;
> + struct media_device *mdev;
> + 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(>pad);
> + if (!pad)
> + return -EPIPE;
> +
> + sd = media_entity_to_v4l2_subdev(pad->entity);
> +
> + if (!on) {
> + media_pipeline_stop(>vdev.entity);
> + return v4l2_subdev_call(sd, video, s_stream, 0);
> + }
> +
> + ret = rvin_mc_validate_format(vin, sd, pad);
> + if (ret)
> + return ret;
> +
> + /*
> +  * The graph lock needs to be taken to protect concurrent
> +  * starts of multiple VIN instances as they might share
> +  * a common subdevice down the line and then should use
> +  * the same pipe.
> +  */
> + mdev = vin->vdev.entity.graph_obj.mdev;
> + mutex_lock(>graph_mutex);
> + pipe = sd->entity.pipe ? sd->entity.pipe : >vdev.pipe;
> + ret = __media_pipeline_start(>vdev.entity, pipe);
> + mutex_unlock(>graph_mutex);
> + if (ret)
> + return ret;
> +
> + ret = v4l2_subdev_call(sd, video, s_stream, 1);
> + if (ret == -ENOIOCTLCMD)
> + ret = 0;
> + if (ret)
> + media_pipeline_stop(>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;
>   

[PATCH v12 30/33] rcar-vin: extend {start,stop}_streaming to work with media controller

[Niklas Söderlund]

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 
Reviewed-by: Laurent Pinchart 
---
 drivers/media/platform/rcar-vin/rcar-dma.c | 133 +++--
 1 file changed, 126 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c 
b/drivers/media/platform/rcar-vin/rcar-dma.c
index da113531f0ce7dc0..580b286acbf2dab6 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -1082,15 +1082,136 @@ static void rvin_buffer_queue(struct vb2_buffer *vb)
spin_unlock_irqrestore(>qlock, flags);
 }
 
+static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev 
*sd,
+  struct media_pad *pad)
+{
+   struct v4l2_subdev_format fmt = {
+   .which = V4L2_SUBDEV_FORMAT_ACTIVE,
+   };
+
+   fmt.pad = pad->index;
+   if (v4l2_subdev_call(sd, pad, get_fmt, NULL, ))
+   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->mbus_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 ||
+   fmt.format.code != vin->mbus_code)
+   return -EPIPE;
+
+   return 0;
+}
+
+static int rvin_set_stream(struct rvin_dev *vin, int on)
+{
+   struct media_pipeline *pipe;
+   struct media_device *mdev;
+   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(>pad);
+   if (!pad)
+   return -EPIPE;
+
+   sd = media_entity_to_v4l2_subdev(pad->entity);
+
+   if (!on) {
+   media_pipeline_stop(>vdev.entity);
+   return v4l2_subdev_call(sd, video, s_stream, 0);
+   }
+
+   ret = rvin_mc_validate_format(vin, sd, pad);
+   if (ret)
+   return ret;
+
+   /*
+* The graph lock needs to be taken to protect concurrent
+* starts of multiple VIN instances as they might share
+* a common subdevice down the line and then should use
+* the same pipe.
+*/
+   mdev = vin->vdev.entity.graph_obj.mdev;
+   mutex_lock(>graph_mutex);
+   pipe = sd->entity.pipe ? sd->entity.pipe : >vdev.pipe;
+   ret = __media_pipeline_start(>vdev.entity, pipe);
+   mutex_unlock(>graph_mutex);
+   if (ret)
+   return ret;
+
+   ret = v4l2_subdev_call(sd, video, s_stream, 1);
+   if (ret == -ENOIOCTLCMD)
+   ret = 0;
+   if (ret)
+   media_pipeline_stop(>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) {
+