Hi Tomi,
Thank you for the patch.
On Wed, Aug 10, 2022 at 03:11:01PM +0300, Tomi Valkeinen wrote:
> Use video_device_pipeline_alloc_start() instead of manually
> allocating/managing the media pipeline storage.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/platform/renesas/rcar-vin/rcar-dma.c | 14 +-------------
> drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c | 2 +-
> .../media/platform/sunxi/sun6i-csi/sun6i_video.c | 2 +-
> drivers/media/platform/ti/cal/cal-video.c | 2 +-
> drivers/media/platform/ti/cal/cal.h | 1 -
> 5 files changed, 4 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> index 548067f19576..884875600231 100644
> --- a/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> +++ b/drivers/media/platform/renesas/rcar-vin/rcar-dma.c
> @@ -1244,8 +1244,6 @@ static int rvin_mc_validate_format(struct rvin_dev *vin, struct v4l2_subdev *sd,
>
> 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;
> @@ -1273,17 +1271,7 @@ static int rvin_set_stream(struct rvin_dev *vin, int on)
> 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(&mdev->graph_mutex);
> - pipe = media_entity_pipeline(&sd->entity) ? : &vin->vdev.pipe;
> - ret = __video_device_pipeline_start(&vin->vdev, pipe);
> - mutex_unlock(&mdev->graph_mutex);
> + ret = video_device_pipeline_alloc_start(&vin->vdev);
This doesn't look right, it will use different pipeline instances for
difference video devices, that's a change in behaviour.
> if (ret)
> return ret;
>
> diff --git a/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c b/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
> index 17ad9a3caaa5..a3e826a755fc 100644
> --- a/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
> +++ b/drivers/media/platform/sunxi/sun4i-csi/sun4i_dma.c
> @@ -266,7 +266,7 @@ static int sun4i_csi_start_streaming(struct vb2_queue *vq, unsigned int count)
> goto err_clear_dma_queue;
> }
>
> - ret = video_device_pipeline_start(&csi->vdev, &csi->vdev.pipe);
What ? There is a pipe embedded in video_device ? Oh my, I didn't
realize how messed up the V4L2 core had become :-(
> + ret = video_device_pipeline_alloc_start(&csi->vdev);
> if (ret < 0)
> goto err_free_scratch_buffer;
>
> diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> index de4c0d47240f..436922503ece 100644
> --- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> +++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_video.c
> @@ -141,7 +141,7 @@ static int sun6i_video_start_streaming(struct vb2_queue *vq, unsigned int count)
>
> video->sequence = 0;
>
> - ret = video_device_pipeline_start(&video->vdev, &video->vdev.pipe);
> + ret = video_device_pipeline_alloc_start(&video->vdev);
> if (ret < 0)
> goto clear_dma_queue;
>
> diff --git a/drivers/media/platform/ti/cal/cal-video.c b/drivers/media/platform/ti/cal/cal-video.c
> index ae29130df819..e8122e7ec944 100644
> --- a/drivers/media/platform/ti/cal/cal-video.c
> +++ b/drivers/media/platform/ti/cal/cal-video.c
> @@ -707,7 +707,7 @@ static int cal_start_streaming(struct vb2_queue *vq, unsigned int count)
> dma_addr_t addr;
> int ret;
>
> - ret = video_device_pipeline_start(&ctx->vdev, &ctx->phy->pipe);
> + ret = video_device_pipeline_alloc_start(&ctx->vdev);
> if (ret < 0) {
> ctx_err(ctx, "Failed to start media pipeline: %d\n", ret);
> goto error_release_buffers;
> diff --git a/drivers/media/platform/ti/cal/cal.h b/drivers/media/platform/ti/cal/cal.h
> index 61409ddced98..648cec43c3fc 100644
> --- a/drivers/media/platform/ti/cal/cal.h
> +++ b/drivers/media/platform/ti/cal/cal.h
> @@ -174,7 +174,6 @@ struct cal_camerarx {
> struct device_node *source_ep_node;
> struct device_node *source_node;
> struct v4l2_subdev *source;
> - struct media_pipeline pipe;
>
> struct v4l2_subdev subdev;
> struct media_pad pads[CAL_CAMERARX_NUM_PADS];
--
Regards,
Laurent Pinchart
