Hi Sakari,
Thank you for the patch.
On Monday 20 Feb 2017 17:22:20 Sakari Ailus wrote:
> Once the driver is unbound accessing the hardware is not allowed anymore.
> Due to this, disable streaming when the device driver is unbound. The
> states of the associated objects related to Media controller and videobuf2
> frameworks are updated as well, just like if the application disabled
> streaming explicitly.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
This looks mostly good to me, although I'm a bit concerned about race
conditions related to buffer handling. I don't think this patch introduces any
new one though, so
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
We'll have to go through buffer management at some point in the near future,
including from a V4L2 API point of view I think.
> ---
> drivers/media/platform/omap3isp/ispvideo.c | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c index a3ca2a4..c04d357 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -1191,22 +1191,17 @@ isp_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type)
> }
>
> static int
> -isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
> +__isp_video_streamoff(struct isp_video *video)
> {
> - struct isp_video_fh *vfh = to_isp_video_fh(fh);
> - struct isp_video *video = video_drvdata(file);
> struct isp_pipeline *pipe = to_isp_pipeline(&video->video.entity);
> enum isp_pipeline_state state;
> unsigned int streaming;
> unsigned long flags;
>
> - if (type != video->type)
> - return -EINVAL;
> -
> mutex_lock(&video->stream_lock);
>
> mutex_lock(&video->queue_lock);
> - streaming = vb2_is_streaming(&vfh->queue);
> + streaming = video->queue && vb2_is_streaming(video->queue);
> mutex_unlock(&video->queue_lock);
>
> if (!streaming)
> @@ -1229,7 +1224,7 @@ isp_video_streamoff(struct file *file, void *fh, enum
> v4l2_buf_type type) omap3isp_video_cancel_stream(video);
>
> mutex_lock(&video->queue_lock);
> - vb2_streamoff(&vfh->queue, type);
> + vb2_streamoff(video->queue, video->queue->type);
> mutex_unlock(&video->queue_lock);
> video->queue = NULL;
> video->error = false;
> @@ -1245,6 +1240,17 @@ isp_video_streamoff(struct file *file, void *fh, enum
> v4l2_buf_type type) }
>
> static int
> +isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
> +{
> + struct isp_video *video = video_drvdata(file);
> +
> + if (type != video->type)
> + return -EINVAL;
> +
> + return __isp_video_streamoff(video);
> +}
> +
> +static int
> isp_video_enum_input(struct file *file, void *fh, struct v4l2_input *input)
> {
> if (input->index > 0)
> @@ -1494,5 +1500,6 @@ int omap3isp_video_register(struct isp_video *video,
> struct v4l2_device *vdev)
>
> void omap3isp_video_unregister(struct isp_video *video)
> {
> + __isp_video_streamoff(video);
> video_unregister_device(&video->video);
> }
--
Regards,
Laurent Pinchart
