Hi,
On Mon, 2018-04-09 at 16:20 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>
> Add support for requests to vim2m.
Depending on where STREAMON happens in the sequence, there is a
possibility that v4l2_m2m_try_schedule is never called and thus that
device_run never runs when submitting the request.
More specifically, the m2m fashion of the STREAMON ioctl handler will
normally call v4l2_m2m_try_schedule. Hence, there is no issue if it's
called after submitting the request. On the other hand, if the request
was not submitted when calling STREAMON (which is a valid use case),
buffers are not available and v4l2_m2m_try_schedule won't schedule
anything. Once the request is submitted, the internal plumbing will
detect that STREAMON was requested but did not take effect, so it will
call vb2_streamon. And of course, vb2_streamon has no provision for
trying to schedule a m2m device run.
One way to fix this is to call v4l2_m2m_try_schedule from a workqueue
triggered in the driver's start_streaming qop. The same is also probably
necessary in buf_queue and buf_prepare since those qops are not called
from the m2m ioctl handler either.
What do you think?
> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> ---
> drivers/media/platform/vim2m.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/media/platform/vim2m.c
> b/drivers/media/platform/vim2m.c
> index 9b18b32c255d..2dcf0ea85705 100644
> --- a/drivers/media/platform/vim2m.c
> +++ b/drivers/media/platform/vim2m.c
> @@ -387,8 +387,26 @@ static void device_run(void *priv)
> src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>
> + /* Apply request if needed */
> + if (src_buf->vb2_buf.req_obj.req)
> + v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req,
> + &ctx->hdl);
> + if (dst_buf->vb2_buf.req_obj.req &&
> + dst_buf->vb2_buf.req_obj.req != src_buf-
> >vb2_buf.req_obj.req)
> + v4l2_ctrl_request_setup(dst_buf->vb2_buf.req_obj.req,
> + &ctx->hdl);
> +
> device_process(ctx, src_buf, dst_buf);
>
> + /* Complete request if needed */
> + if (src_buf->vb2_buf.req_obj.req)
> + v4l2_ctrl_request_complete(src_buf-
> >vb2_buf.req_obj.req,
> + &ctx->hdl);
> + if (dst_buf->vb2_buf.req_obj.req &&
> + dst_buf->vb2_buf.req_obj.req != src_buf-
> >vb2_buf.req_obj.req)
> + v4l2_ctrl_request_complete(dst_buf-
> >vb2_buf.req_obj.req,
> + &ctx->hdl);
> +
> /* Run a timer, which simulates a hardware irq */
> schedule_irq(dev, ctx->transtime);
> }
> @@ -823,6 +841,8 @@ static void vim2m_stop_streaming(struct vb2_queue
> *q)
> vbuf = v4l2_m2m_dst_buf_remove(ctx-
> >fh.m2m_ctx);
> if (vbuf == NULL)
> return;
> + v4l2_ctrl_request_complete(vbuf->vb2_buf.req_obj.req,
> + &ctx->hdl);
> spin_lock_irqsave(&ctx->dev->irqlock, flags);
> v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
> spin_unlock_irqrestore(&ctx->dev->irqlock, flags);
> @@ -1003,6 +1023,10 @@ static const struct v4l2_m2m_ops m2m_ops = {
> .job_abort = job_abort,
> };
>
> +static const struct media_device_ops m2m_media_ops = {
> + .req_queue = vb2_request_queue,
> +};
> +
> static int vim2m_probe(struct platform_device *pdev)
> {
> struct vim2m_dev *dev;
> @@ -1027,6 +1051,7 @@ static int vim2m_probe(struct platform_device
> *pdev)
> dev->mdev.dev = &pdev->dev;
> strlcpy(dev->mdev.model, "vim2m", sizeof(dev->mdev.model));
> media_device_init(&dev->mdev);
> + dev->mdev.ops = &m2m_media_ops;
> dev->v4l2_dev.mdev = &dev->mdev;
> dev->pad[0].flags = MEDIA_PAD_FL_SINK;
> dev->pad[1].flags = MEDIA_PAD_FL_SOURCE;
--
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.comAttachment:
signature.asc
Description: This is a digitally signed message part
