On Mon, 13 Jul 2020 at 13:30, Hans Verkuil <hverkuil-cisco@xxxxxxxxx> wrote:
>
> Use vb2_video_unregister_device() to automatically stop streaming
> at unregister time.
>
> This avoids the use of vb2_queue_release() which should not be
> called by drivers that set vdev->queue.
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> Cc: Robert Foss <robert.foss@xxxxxxxxxx>
> Cc: Andrey Konovalov <andrey.konovalov@xxxxxxxxxx>
> ---
> drivers/media/platform/qcom/camss/camss-vfe.c | 8 --------
> drivers/media/platform/qcom/camss/camss-vfe.h | 2 --
> drivers/media/platform/qcom/camss/camss-video.c | 12 ++----------
> drivers/media/platform/qcom/camss/camss-video.h | 2 --
> drivers/media/platform/qcom/camss/camss.c | 5 -----
> 5 files changed, 2 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c
> index fc31c2c169cd..b7d2293a5004 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.c
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.c
> @@ -2205,14 +2205,6 @@ static const struct camss_video_ops camss_vfe_video_ops = {
> .flush_buffers = vfe_flush_buffers,
> };
>
> -void msm_vfe_stop_streaming(struct vfe_device *vfe)
> -{
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(vfe->line); i++)
> - msm_video_stop_streaming(&vfe->line[i].video_out);
> -}
> -
> /*
> * msm_vfe_register_entities - Register subdev node for VFE module
> * @vfe: VFE device
> diff --git a/drivers/media/platform/qcom/camss/camss-vfe.h b/drivers/media/platform/qcom/camss/camss-vfe.h
> index 0d10071ae881..a90b0d2cc6de 100644
> --- a/drivers/media/platform/qcom/camss/camss-vfe.h
> +++ b/drivers/media/platform/qcom/camss/camss-vfe.h
> @@ -178,8 +178,6 @@ void msm_vfe_unregister_entities(struct vfe_device *vfe);
> void msm_vfe_get_vfe_id(struct media_entity *entity, u8 *id);
> void msm_vfe_get_vfe_line_id(struct media_entity *entity, enum vfe_line_id *id);
>
> -void msm_vfe_stop_streaming(struct vfe_device *vfe);
> -
> extern const struct vfe_hw_ops vfe_ops_4_1;
> extern const struct vfe_hw_ops vfe_ops_4_7;
>
> diff --git a/drivers/media/platform/qcom/camss/camss-video.c b/drivers/media/platform/qcom/camss/camss-video.c
> index cdbd6dba1122..0e2fcee97eeb 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.c
> +++ b/drivers/media/platform/qcom/camss/camss-video.c
> @@ -879,7 +879,7 @@ int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev,
> if (ret < 0) {
> dev_err(v4l2_dev->dev, "Failed to init video entity: %d\n",
> ret);
> - goto error_media_init;
> + goto error_vb2_init;
> }
>
> mutex_init(&video->lock);
> @@ -936,23 +936,15 @@ int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev,
> error_video_register:
> media_entity_cleanup(&vdev->entity);
> mutex_destroy(&video->lock);
> -error_media_init:
> - vb2_queue_release(&video->vb2_q);
> error_vb2_init:
> mutex_destroy(&video->q_lock);
>
> return ret;
> }
>
> -void msm_video_stop_streaming(struct camss_video *video)
> -{
> - if (vb2_is_streaming(&video->vb2_q))
> - vb2_queue_release(&video->vb2_q);
> -}
> -
> void msm_video_unregister(struct camss_video *video)
> {
> atomic_inc(&video->camss->ref_count);
> - video_unregister_device(&video->vdev);
> + vb2_video_unregister_device(&video->vdev);
> atomic_dec(&video->camss->ref_count);
> }
> diff --git a/drivers/media/platform/qcom/camss/camss-video.h b/drivers/media/platform/qcom/camss/camss-video.h
> index aa35e8cc6fd5..bdbae8424140 100644
> --- a/drivers/media/platform/qcom/camss/camss-video.h
> +++ b/drivers/media/platform/qcom/camss/camss-video.h
> @@ -52,8 +52,6 @@ struct camss_video {
> unsigned int nformats;
> };
>
> -void msm_video_stop_streaming(struct camss_video *video);
> -
> int msm_video_register(struct camss_video *video, struct v4l2_device *v4l2_dev,
> const char *name, int is_pix);
>
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 3fdc9f964a3c..d0f4360eb9a0 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -964,13 +964,8 @@ void camss_delete(struct camss *camss)
> */
> static int camss_remove(struct platform_device *pdev)
> {
> - unsigned int i;
> -
> struct camss *camss = platform_get_drvdata(pdev);
>
> - for (i = 0; i < camss->vfe_num; i++)
> - msm_vfe_stop_streaming(&camss->vfe[i]);
> -
I'm trying to understand this change, with msm_vfe_stop_streaming
removed, are we relying on vb2_video_unregister_device being called
through some other path?
> v4l2_async_notifier_unregister(&camss->notifier);
> v4l2_async_notifier_cleanup(&camss->notifier);
> camss_unregister_entities(camss);
> --
> 2.27.0
>
