Re: [RFCv11 PATCH 27/29] vim2m: support requests

[Hans Verkuil]

On 04/17/2018 01:42 PM, Paul Kocialkowski wrote:
> Hi,
> 
> On Mon, 2018-04-09 at 16:20 +0200, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Add support for requests to vim2m.
> 
> Please find a nit below.
> 
>> Signed-off-by: Hans Verkuil 
>> ---
>>  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 */
> 
> This comment suggests that this is where request submission is handled.
> I suggest making it clear that this the place where the *controls*
> attached to the request are applied, instead.

True. Fixed here and below.

Thanks!

Hans

> 
>> +if (src_buf->vb2_buf.req_obj.req)
>> +v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req,
>> +>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,
>> +>hdl);
>> +
>>  device_process(ctx, src_buf, dst_buf);
>>  
>> +/* Complete request if needed */
> 
> Ditto here.
> 
>> +if (src_buf->vb2_buf.req_obj.req)
>> +v4l2_ctrl_request_complete(src_buf-
>>> vb2_buf.req_obj.req,
>> +>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,
>> +>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,
>> +   >hdl);
>>  spin_lock_irqsave(>dev->irqlock, flags);
>>  v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
>>  spin_unlock_irqrestore(>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 = >dev;
>>  strlcpy(dev->mdev.model, "vim2m", sizeof(dev->mdev.model));
>>  media_device_init(>mdev);
>> +dev->mdev.ops = _media_ops;
>>  dev->v4l2_dev.mdev = >mdev;
>>  dev->pad[0].flags = MEDIA_PAD_FL_SINK;
>>  dev->pad[1].flags = MEDIA_PAD_FL_SOURCE;

Re: [RFCv11 PATCH 27/29] vim2m: support requests

[Hans Verkuil]

On 04/17/2018 06:37 AM, Alexandre Courbot wrote:
> On Mon, Apr 9, 2018 at 11:20 PM Hans Verkuil  wrote:
> 
>> From: Hans Verkuil 
> 
>> Add support for requests to vim2m.
> 
>> Signed-off-by: Hans Verkuil 
>> ---
>>   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,
>> +   >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,
>> +   >hdl);
> 
> This implies that we can have two different requests active at the same time
> for the same device. Do we want to open that can of worms?
> 
> Valid scenarios for requests should be clearly defined. In the case of a
> simple
> buffer processor like vim2m, something like "request required for source
> buffer,
> optional and must be same as source if specified for destination", looks
> simple
> and sane.

Is it? It is sane for this specific use-case, but for other m2m devices you
might require the request for the sink buffer instead of the source buffer.
Or both. So for one driver you would have one set of rules, and for another
driver another set of rules.

That said, this probably requires some more discussion. I went with the generic
case here, and it was really very little code to make that happen. And it
avoids creating ad-hoc 'rules'.

Regards,

Hans

> 
>> +
>>  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,
>> +   >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,
>> +   >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,
>> +  >hdl);
>>  spin_lock_irqsave(>dev->irqlock, flags);
>>  v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
>>  spin_unlock_irqrestore(>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 = >dev;
>>  strlcpy(dev->mdev.model, "vim2m", sizeof(dev->mdev.model));
>>  media_device_init(>mdev);
>> +   dev->mdev.ops = _media_ops;
>>  dev->v4l2_dev.mdev = >mdev;
>>  dev->pad[0].flags = MEDIA_PAD_FL_SINK;
>>  dev->pad[1].flags = MEDIA_PAD_FL_SOURCE;
>> --
>> 2.16.3

Re: [RFCv11 PATCH 27/29] vim2m: support requests

[Hans Verkuil]

On 04/12/2018 11:15 AM, Tomasz Figa wrote:
> Hi Hans,
> 
> On Mon, Apr 9, 2018 at 11:20 PM Hans Verkuil  wrote:
> 
>> From: Hans Verkuil 
> 
>> Add support for requests to vim2m.
> 
>> Signed-off-by: Hans Verkuil 
>> ---
>>   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,
>> +   >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,
>> +   >hdl);
> 
> I'm not sure I understand what's going on here. How is it possible that we
> have 2 different requests?

You can have one request with buffers for both queues, or two requests, one
for each queue. Or a request for just one of the queues and the other without
any requests.

So you can have 0, 1 or 2 requests associated with these two queues.

But you don't want to call v4l2_ctrl_request_setup twice if the same request
is associated with both queues. (Well, you can call it twice and the second
call would not do anything, but that's a waste of CPU cycles)

Regards,

Hans

Re: [RFCv11 PATCH 27/29] vim2m: support requests

[Paul Kocialkowski]

Hi,

On Mon, 2018-04-09 at 16:20 +0200, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Add support for requests to vim2m.

Please find a nit below.

> Signed-off-by: Hans Verkuil 
> ---
>  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 */

This comment suggests that this is where request submission is handled.
I suggest making it clear that this the place where the *controls*
attached to the request are applied, instead.

> + if (src_buf->vb2_buf.req_obj.req)
> + v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req,
> + >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,
> + >hdl);
> +
>   device_process(ctx, src_buf, dst_buf);
>  
> + /* Complete request if needed */

Ditto here.

> + if (src_buf->vb2_buf.req_obj.req)
> + v4l2_ctrl_request_complete(src_buf-
> >vb2_buf.req_obj.req,
> + >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,
> + >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,
> +>hdl);
>   spin_lock_irqsave(>dev->irqlock, flags);
>   v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
>   spin_unlock_irqrestore(>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 = >dev;
>   strlcpy(dev->mdev.model, "vim2m", sizeof(dev->mdev.model));
>   media_device_init(>mdev);
> + dev->mdev.ops = _media_ops;
>   dev->v4l2_dev.mdev = >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.com

signature.asc
Description: This is a digitally signed message part

Re: [RFCv11 PATCH 27/29] vim2m: support requests

[Alexandre Courbot]

On Mon, Apr 9, 2018 at 11:20 PM Hans Verkuil  wrote:

> From: Hans Verkuil 

> Add support for requests to vim2m.

> Signed-off-by: Hans Verkuil 
> ---
>   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,
> +   >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,
> +   >hdl);

This implies that we can have two different requests active at the same time
for the same device. Do we want to open that can of worms?

Valid scenarios for requests should be clearly defined. In the case of a
simple
buffer processor like vim2m, something like "request required for source
buffer,
optional and must be same as source if specified for destination", looks
simple
and sane.

> +
>  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,
> +   >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,
> +   >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,
> +  >hdl);
>  spin_lock_irqsave(>dev->irqlock, flags);
>  v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
>  spin_unlock_irqrestore(>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 = >dev;
>  strlcpy(dev->mdev.model, "vim2m", sizeof(dev->mdev.model));
>  media_device_init(>mdev);
> +   dev->mdev.ops = _media_ops;
>  dev->v4l2_dev.mdev = >mdev;
>  dev->pad[0].flags = MEDIA_PAD_FL_SINK;
>  dev->pad[1].flags = MEDIA_PAD_FL_SOURCE;
> --
> 2.16.3

Re: [RFCv11 PATCH 27/29] vim2m: support requests

[Tomasz Figa]

Hi Hans,

On Mon, Apr 9, 2018 at 11:20 PM Hans Verkuil  wrote:

> From: Hans Verkuil 

> Add support for requests to vim2m.

> Signed-off-by: Hans Verkuil 
> ---
>   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,
> +   >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,
> +   >hdl);

I'm not sure I understand what's going on here. How is it possible that we
have 2 different requests?

Best regards,
Tomasz

Re: [RFCv11 PATCH 27/29] vim2m: support requests

[Paul Kocialkowski]

Hi,

On Mon, 2018-04-09 at 16:20 +0200, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> 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 
> ---
>  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,
> + >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,
> + >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,
> + >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,
> + >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,
> +>hdl);
>   spin_lock_irqsave(>dev->irqlock, flags);
>   v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
>   spin_unlock_irqrestore(>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 = >dev;
>   strlcpy(dev->mdev.model, "vim2m", sizeof(dev->mdev.model));
>   media_device_init(>mdev);
> + dev->mdev.ops = _media_ops;
>   dev->v4l2_dev.mdev = >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.com

signature.asc
Description: This is a digitally signed message part

[RFCv11 PATCH 27/29] vim2m: support requests

[Hans Verkuil]

From: Hans Verkuil 

Add support for requests to vim2m.

Signed-off-by: Hans Verkuil 
---
 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,
+   >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,
+   >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,
+   >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,
+   >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,
+  >hdl);
spin_lock_irqsave(>dev->irqlock, flags);
v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
spin_unlock_irqrestore(>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 = >dev;
strlcpy(dev->mdev.model, "vim2m", sizeof(dev->mdev.model));
media_device_init(>mdev);
+   dev->mdev.ops = _media_ops;
dev->v4l2_dev.mdev = >mdev;
dev->pad[0].flags = MEDIA_PAD_FL_SINK;
dev->pad[1].flags = MEDIA_PAD_FL_SOURCE;
-- 
2.16.3