diff mbox series

[v2,08/11] rockchip/vpu: Support the Request API

Message ID 20190304192529.14200-9-ezequiel@collabora.com (mailing list archive)
State New, archived
Headers show
Series Add MPEG-2 decoding to Rockchip VPU | expand

Commit Message

Ezequiel Garcia March 4, 2019, 7:25 p.m. UTC
Introduce support for the Request API. Although the JPEG encoder
does not mandate using the Request API, it's perfectly possible to
use it, if the application wants to.

In addition, add helpers that will be used by video decoders.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 .../rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c     |  6 ++++
 .../media/rockchip/vpu/rockchip_vpu_common.h  |  3 ++
 .../media/rockchip/vpu/rockchip_vpu_drv.c     | 29 ++++++++++++++++++-
 .../media/rockchip/vpu/rockchip_vpu_enc.c     | 18 ++++++++++++
 4 files changed, 55 insertions(+), 1 deletion(-)

Comments

Tomasz Figa March 28, 2019, 7:20 a.m. UTC | #1
On Tue, Mar 5, 2019 at 4:27 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> Introduce support for the Request API. Although the JPEG encoder
> does not mandate using the Request API, it's perfectly possible to
> use it, if the application wants to.
>
> In addition, add helpers that will be used by video decoders.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  .../rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c     |  6 ++++
>  .../media/rockchip/vpu/rockchip_vpu_common.h  |  3 ++
>  .../media/rockchip/vpu/rockchip_vpu_drv.c     | 29 ++++++++++++++++++-
>  .../media/rockchip/vpu/rockchip_vpu_enc.c     | 18 ++++++++++++
>  4 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c b/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c
> index fe92c40d0a84..e11ee152b8ce 100644
> --- a/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c
> +++ b/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c
> @@ -113,11 +113,15 @@ void rk3399_vpu_jpeg_enc_run(struct rockchip_vpu_ctx *ctx)
>         struct rockchip_vpu_dev *vpu = ctx->dev;
>         struct vb2_v4l2_buffer *src_buf, *dst_buf;
>         struct rockchip_vpu_jpeg_ctx jpeg_ctx;
> +       struct media_request *src_req;
>         u32 reg;
>
>         src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
>         dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>
> +       src_req = src_buf->vb2_buf.req_obj.req;
> +       v4l2_ctrl_request_setup(src_req, &ctx->ctrl_handler);
> +
>         memset(&jpeg_ctx, 0, sizeof(jpeg_ctx));
>         jpeg_ctx.buffer = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
>         jpeg_ctx.width = ctx->dst_fmt.width;
> @@ -153,6 +157,8 @@ void rk3399_vpu_jpeg_enc_run(struct rockchip_vpu_ctx *ctx)
>                 | VEPU_REG_ENCODE_FORMAT_JPEG
>                 | VEPU_REG_ENCODE_ENABLE;
>
> +       v4l2_ctrl_request_complete(src_req, &ctx->ctrl_handler);
> +
>         /* Kick the watchdog and start encoding */
>         schedule_delayed_work(&vpu->watchdog_work, msecs_to_jiffies(2000));
>         vepu_write(vpu, reg, VEPU_REG_ENCODE_START);
> diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h b/drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h
> index ca77668d9579..ac018136a7bc 100644
> --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h
> +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h
> @@ -26,4 +26,7 @@ void rockchip_vpu_enc_reset_src_fmt(struct rockchip_vpu_dev *vpu,
>  void rockchip_vpu_enc_reset_dst_fmt(struct rockchip_vpu_dev *vpu,
>                                     struct rockchip_vpu_ctx *ctx);
>
> +void *rockchip_vpu_get_ctrl(struct rockchip_vpu_ctx *ctx, u32 id);
> +dma_addr_t rockchip_vpu_get_ref(struct vb2_queue *q, u64 ts);
> +
>  #endif /* ROCKCHIP_VPU_COMMON_H_ */
> diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c
> index af2481ca2228..937e9cfb4568 100644
> --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c
> +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c
> @@ -36,6 +36,24 @@ module_param_named(debug, rockchip_vpu_debug, int, 0644);
>  MODULE_PARM_DESC(debug,
>                  "Debug level - higher value produces more verbose messages");
>
> +void *rockchip_vpu_get_ctrl(struct rockchip_vpu_ctx *ctx, u32 id)
> +{
> +       struct v4l2_ctrl *ctrl;
> +
> +       ctrl = v4l2_ctrl_find(&ctx->ctrl_handler, id);
> +       return ctrl ? ctrl->p_cur.p : NULL;
> +}
> +
> +dma_addr_t rockchip_vpu_get_ref(struct vb2_queue *q, u64 ts)
> +{
> +       int index;
> +
> +       index = vb2_find_timestamp(q, ts, 0);
> +       if (index >= 0)
> +               return vb2_dma_contig_plane_dma_addr(q->bufs[index], 0);
> +       return 0;
> +}
> +

Hmm, I assume these 2 are the "helpers that will be used by video
decoders"? Probably overly nitpicking, but these are relatively small,
so why couldn't they be just added in the patch that actually adds the
code using them? Otherwise, if for some reason we separate the series,
we end up with dead code.

>  static void rockchip_vpu_job_finish(struct rockchip_vpu_dev *vpu,
>                                     struct rockchip_vpu_ctx *ctx,
>                                     unsigned int bytesused,
> @@ -164,6 +182,7 @@ enc_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
>         src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
>         src_vq->lock = &ctx->dev->vpu_mutex;
>         src_vq->dev = ctx->dev->v4l2_dev.dev;
> +       src_vq->supports_requests = true;
>
>         ret = vb2_queue_init(src_vq);
>         if (ret)
> @@ -328,6 +347,11 @@ static const struct of_device_id of_rockchip_vpu_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, of_rockchip_vpu_match);
>
> +static const struct media_device_ops rockchip_m2m_media_ops = {
> +       .req_validate = vb2_request_validate,
> +       .req_queue = v4l2_m2m_request_queue,
> +};
> +
>  static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu)
>  {
>         const struct of_device_id *match;
> @@ -610,8 +634,11 @@ static int rockchip_vpu_probe(struct platform_device *pdev)
>         }
>
>         vpu->mdev.dev = vpu->dev;
> -       strlcpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model));
> +       strscpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model));
> +       strscpy(vpu->mdev.bus_info, "platform:" DRIVER_NAME,
> +               sizeof(vpu->mdev.bus_info));

Unrelated changes?

>         media_device_init(&vpu->mdev);
> +       vpu->mdev.ops = &rockchip_m2m_media_ops;
>         vpu->v4l2_dev.mdev = &vpu->mdev;
>
>         ret = rockchip_vpu_video_device_register(vpu);
> diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c b/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c
> index 2b28403314bc..1b5a675ef24f 100644
> --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c
> +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c
> @@ -555,14 +555,32 @@ static void rockchip_vpu_stop_streaming(struct vb2_queue *q)
>                         vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
>                 if (!vbuf)
>                         break;
> +               v4l2_ctrl_request_complete(vbuf->vb2_buf.req_obj.req, &ctx->ctrl_handler);
>                 v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
>         }
>  }
>
> +static void rockchip_vpu_buf_request_complete(struct vb2_buffer *vb)
> +{
> +       struct rockchip_vpu_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +
> +       v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->ctrl_handler);
> +}
> +
> +static int rockchip_vpu_buf_out_validate(struct vb2_buffer *vb)
> +{
> +       struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +
> +       vbuf->field = V4L2_FIELD_NONE;

Hmm, "validate" in the name of this callback would suggest that we
should just check the contents, not change them. Hans, what was the
intention when adding this callback? Are we missing the const
specifier in the argument?

Best regards,
Tomasz
Hans Verkuil March 28, 2019, 1:59 p.m. UTC | #2
On 3/28/19 8:20 AM, Tomasz Figa wrote:

<snip>

>> +static int rockchip_vpu_buf_out_validate(struct vb2_buffer *vb)
>> +{
>> +       struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>> +
>> +       vbuf->field = V4L2_FIELD_NONE;
> 
> Hmm, "validate" in the name of this callback would suggest that we
> should just check the contents, not change them. Hans, what was the
> intention when adding this callback? Are we missing the const
> specifier in the argument?

See the original commit log:

https://www.mail-archive.com/linux-media@vger.kernel.org/msg143163.html

It is allowed to either just validate and return an error if wrong,
or change it to something known to be valid. In particular, userspace
can set this to FIELD_ANY, and in that case the driver must replace
it with something valid.

Most drivers just support FIELD_NONE, and just set it.

That said, I think we should tighten the spec for this as this is
not well documented.

I propose that if vbuf->field == FIELD_ANY, then replace it with something
sane. Otherwise validate it and return an error if the field value
is not supported.

And FIELD_ALTERNATE is never allowed here, that's always wrong.

Regards,

	Hans
Ezequiel Garcia March 28, 2019, 7:07 p.m. UTC | #3
On Thu, 2019-03-28 at 16:20 +0900, Tomasz Figa wrote:
> On Tue, Mar 5, 2019 at 4:27 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > Introduce support for the Request API. Although the JPEG encoder
> > does not mandate using the Request API, it's perfectly possible to
> > use it, if the application wants to.
> > 
> > In addition, add helpers that will be used by video decoders.
> > 
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> >  .../rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c     |  6 ++++
> >  .../media/rockchip/vpu/rockchip_vpu_common.h  |  3 ++
> >  .../media/rockchip/vpu/rockchip_vpu_drv.c     | 29 ++++++++++++++++++-
> >  .../media/rockchip/vpu/rockchip_vpu_enc.c     | 18 ++++++++++++
> >  4 files changed, 55 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c b/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c
> > index fe92c40d0a84..e11ee152b8ce 100644
> > --- a/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c
> > +++ b/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c
> > @@ -113,11 +113,15 @@ void rk3399_vpu_jpeg_enc_run(struct rockchip_vpu_ctx *ctx)
> >         struct rockchip_vpu_dev *vpu = ctx->dev;
> >         struct vb2_v4l2_buffer *src_buf, *dst_buf;
> >         struct rockchip_vpu_jpeg_ctx jpeg_ctx;
> > +       struct media_request *src_req;
> >         u32 reg;
> > 
> >         src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> >         dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> > 
> > +       src_req = src_buf->vb2_buf.req_obj.req;
> > +       v4l2_ctrl_request_setup(src_req, &ctx->ctrl_handler);
> > +
> >         memset(&jpeg_ctx, 0, sizeof(jpeg_ctx));
> >         jpeg_ctx.buffer = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
> >         jpeg_ctx.width = ctx->dst_fmt.width;
> > @@ -153,6 +157,8 @@ void rk3399_vpu_jpeg_enc_run(struct rockchip_vpu_ctx *ctx)
> >                 | VEPU_REG_ENCODE_FORMAT_JPEG
> >                 | VEPU_REG_ENCODE_ENABLE;
> > 
> > +       v4l2_ctrl_request_complete(src_req, &ctx->ctrl_handler);
> > +
> >         /* Kick the watchdog and start encoding */
> >         schedule_delayed_work(&vpu->watchdog_work, msecs_to_jiffies(2000));
> >         vepu_write(vpu, reg, VEPU_REG_ENCODE_START);
> > diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h b/drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h
> > index ca77668d9579..ac018136a7bc 100644
> > --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h
> > +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h
> > @@ -26,4 +26,7 @@ void rockchip_vpu_enc_reset_src_fmt(struct rockchip_vpu_dev *vpu,
> >  void rockchip_vpu_enc_reset_dst_fmt(struct rockchip_vpu_dev *vpu,
> >                                     struct rockchip_vpu_ctx *ctx);
> > 
> > +void *rockchip_vpu_get_ctrl(struct rockchip_vpu_ctx *ctx, u32 id);
> > +dma_addr_t rockchip_vpu_get_ref(struct vb2_queue *q, u64 ts);
> > +
> >  #endif /* ROCKCHIP_VPU_COMMON_H_ */
> > diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c
> > index af2481ca2228..937e9cfb4568 100644
> > --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c
> > +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c
> > @@ -36,6 +36,24 @@ module_param_named(debug, rockchip_vpu_debug, int, 0644);
> >  MODULE_PARM_DESC(debug,
> >                  "Debug level - higher value produces more verbose messages");
> > 
> > +void *rockchip_vpu_get_ctrl(struct rockchip_vpu_ctx *ctx, u32 id)
> > +{
> > +       struct v4l2_ctrl *ctrl;
> > +
> > +       ctrl = v4l2_ctrl_find(&ctx->ctrl_handler, id);
> > +       return ctrl ? ctrl->p_cur.p : NULL;
> > +}
> > +
> > +dma_addr_t rockchip_vpu_get_ref(struct vb2_queue *q, u64 ts)
> > +{
> > +       int index;
> > +
> > +       index = vb2_find_timestamp(q, ts, 0);
> > +       if (index >= 0)
> > +               return vb2_dma_contig_plane_dma_addr(q->bufs[index], 0);
> > +       return 0;
> > +}
> > +
> 
> Hmm, I assume these 2 are the "helpers that will be used by video
> decoders"? Probably overly nitpicking, but these are relatively small,
> so why couldn't they be just added in the patch that actually adds the
> code using them? Otherwise, if for some reason we separate the series,
> we end up with dead code.
> 

I can't remember why these are here. It was obviously intentional,
as the commit log shows!

Anyway, yeah, I'll move them to the patch that makes use of it.

> >  static void rockchip_vpu_job_finish(struct rockchip_vpu_dev *vpu,
> >                                     struct rockchip_vpu_ctx *ctx,
> >                                     unsigned int bytesused,
> > @@ -164,6 +182,7 @@ enc_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
> >         src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> >         src_vq->lock = &ctx->dev->vpu_mutex;
> >         src_vq->dev = ctx->dev->v4l2_dev.dev;
> > +       src_vq->supports_requests = true;
> > 
> >         ret = vb2_queue_init(src_vq);
> >         if (ret)
> > @@ -328,6 +347,11 @@ static const struct of_device_id of_rockchip_vpu_match[] = {
> >  };
> >  MODULE_DEVICE_TABLE(of, of_rockchip_vpu_match);
> > 
> > +static const struct media_device_ops rockchip_m2m_media_ops = {
> > +       .req_validate = vb2_request_validate,
> > +       .req_queue = v4l2_m2m_request_queue,
> > +};
> > +
> >  static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu)
> >  {
> >         const struct of_device_id *match;
> > @@ -610,8 +634,11 @@ static int rockchip_vpu_probe(struct platform_device *pdev)
> >         }
> > 
> >         vpu->mdev.dev = vpu->dev;
> > -       strlcpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model));
> > +       strscpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model));
> > +       strscpy(vpu->mdev.bus_info, "platform:" DRIVER_NAME,
> > +               sizeof(vpu->mdev.bus_info));
> 
> Unrelated changes?
> 

Totally.

> >         media_device_init(&vpu->mdev);
> > +       vpu->mdev.ops = &rockchip_m2m_media_ops;
> >         vpu->v4l2_dev.mdev = &vpu->mdev;
> > 
> >         ret = rockchip_vpu_video_device_register(vpu);
> > diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c b/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c
> > index 2b28403314bc..1b5a675ef24f 100644
> > --- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c
> > +++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c
> > @@ -555,14 +555,32 @@ static void rockchip_vpu_stop_streaming(struct vb2_queue *q)
> >                         vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> >                 if (!vbuf)
> >                         break;
> > +               v4l2_ctrl_request_complete(vbuf->vb2_buf.req_obj.req, &ctx->ctrl_handler);
> >                 v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
> >         }
> >  }
> > 
> > +static void rockchip_vpu_buf_request_complete(struct vb2_buffer *vb)
> > +{
> > +       struct rockchip_vpu_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> > +
> > +       v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->ctrl_handler);
> > +}
> > +
> > +static int rockchip_vpu_buf_out_validate(struct vb2_buffer *vb)
> > +{
> > +       struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> > +
> > +       vbuf->field = V4L2_FIELD_NONE;
> 
> Hmm, "validate" in the name of this callback would suggest that we
> should just check the contents, not change them. Hans, what was the
> intention when adding this callback? Are we missing the const
> specifier in the argument?
> 

Hans already replied to this.

Thanks,
Eze
Tomasz Figa March 29, 2019, 3:23 a.m. UTC | #4
On Thu, Mar 28, 2019 at 10:59 PM Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> On 3/28/19 8:20 AM, Tomasz Figa wrote:
>
> <snip>
>
> >> +static int rockchip_vpu_buf_out_validate(struct vb2_buffer *vb)
> >> +{
> >> +       struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> >> +
> >> +       vbuf->field = V4L2_FIELD_NONE;
> >
> > Hmm, "validate" in the name of this callback would suggest that we
> > should just check the contents, not change them. Hans, what was the
> > intention when adding this callback? Are we missing the const
> > specifier in the argument?
>
> See the original commit log:
>
> https://www.mail-archive.com/linux-media@vger.kernel.org/msg143163.html
>
> It is allowed to either just validate and return an error if wrong,
> or change it to something known to be valid. In particular, userspace
> can set this to FIELD_ANY, and in that case the driver must replace
> it with something valid.
>
> Most drivers just support FIELD_NONE, and just set it.
>
> That said, I think we should tighten the spec for this as this is
> not well documented.
>
> I propose that if vbuf->field == FIELD_ANY, then replace it with something
> sane. Otherwise validate it and return an error if the field value
> is not supported.
>
> And FIELD_ALTERNATE is never allowed here, that's always wrong.

Thanks for clarifying. Makes sense.

Best regards,
Tomasz
diff mbox series

Patch

diff --git a/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c b/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c
index fe92c40d0a84..e11ee152b8ce 100644
--- a/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c
+++ b/drivers/staging/media/rockchip/vpu/rk3399_vpu_hw_jpeg_enc.c
@@ -113,11 +113,15 @@  void rk3399_vpu_jpeg_enc_run(struct rockchip_vpu_ctx *ctx)
 	struct rockchip_vpu_dev *vpu = ctx->dev;
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
 	struct rockchip_vpu_jpeg_ctx jpeg_ctx;
+	struct media_request *src_req;
 	u32 reg;
 
 	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
 	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
 
+	src_req = src_buf->vb2_buf.req_obj.req;
+	v4l2_ctrl_request_setup(src_req, &ctx->ctrl_handler);
+
 	memset(&jpeg_ctx, 0, sizeof(jpeg_ctx));
 	jpeg_ctx.buffer = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
 	jpeg_ctx.width = ctx->dst_fmt.width;
@@ -153,6 +157,8 @@  void rk3399_vpu_jpeg_enc_run(struct rockchip_vpu_ctx *ctx)
 		| VEPU_REG_ENCODE_FORMAT_JPEG
 		| VEPU_REG_ENCODE_ENABLE;
 
+	v4l2_ctrl_request_complete(src_req, &ctx->ctrl_handler);
+
 	/* Kick the watchdog and start encoding */
 	schedule_delayed_work(&vpu->watchdog_work, msecs_to_jiffies(2000));
 	vepu_write(vpu, reg, VEPU_REG_ENCODE_START);
diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h b/drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h
index ca77668d9579..ac018136a7bc 100644
--- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h
+++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_common.h
@@ -26,4 +26,7 @@  void rockchip_vpu_enc_reset_src_fmt(struct rockchip_vpu_dev *vpu,
 void rockchip_vpu_enc_reset_dst_fmt(struct rockchip_vpu_dev *vpu,
 				    struct rockchip_vpu_ctx *ctx);
 
+void *rockchip_vpu_get_ctrl(struct rockchip_vpu_ctx *ctx, u32 id);
+dma_addr_t rockchip_vpu_get_ref(struct vb2_queue *q, u64 ts);
+
 #endif /* ROCKCHIP_VPU_COMMON_H_ */
diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c
index af2481ca2228..937e9cfb4568 100644
--- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c
+++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_drv.c
@@ -36,6 +36,24 @@  module_param_named(debug, rockchip_vpu_debug, int, 0644);
 MODULE_PARM_DESC(debug,
 		 "Debug level - higher value produces more verbose messages");
 
+void *rockchip_vpu_get_ctrl(struct rockchip_vpu_ctx *ctx, u32 id)
+{
+	struct v4l2_ctrl *ctrl;
+
+	ctrl = v4l2_ctrl_find(&ctx->ctrl_handler, id);
+	return ctrl ? ctrl->p_cur.p : NULL;
+}
+
+dma_addr_t rockchip_vpu_get_ref(struct vb2_queue *q, u64 ts)
+{
+	int index;
+
+	index = vb2_find_timestamp(q, ts, 0);
+	if (index >= 0)
+		return vb2_dma_contig_plane_dma_addr(q->bufs[index], 0);
+	return 0;
+}
+
 static void rockchip_vpu_job_finish(struct rockchip_vpu_dev *vpu,
 				    struct rockchip_vpu_ctx *ctx,
 				    unsigned int bytesused,
@@ -164,6 +182,7 @@  enc_queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
 	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
 	src_vq->lock = &ctx->dev->vpu_mutex;
 	src_vq->dev = ctx->dev->v4l2_dev.dev;
+	src_vq->supports_requests = true;
 
 	ret = vb2_queue_init(src_vq);
 	if (ret)
@@ -328,6 +347,11 @@  static const struct of_device_id of_rockchip_vpu_match[] = {
 };
 MODULE_DEVICE_TABLE(of, of_rockchip_vpu_match);
 
+static const struct media_device_ops rockchip_m2m_media_ops = {
+	.req_validate = vb2_request_validate,
+	.req_queue = v4l2_m2m_request_queue,
+};
+
 static int rockchip_vpu_video_device_register(struct rockchip_vpu_dev *vpu)
 {
 	const struct of_device_id *match;
@@ -610,8 +634,11 @@  static int rockchip_vpu_probe(struct platform_device *pdev)
 	}
 
 	vpu->mdev.dev = vpu->dev;
-	strlcpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model));
+	strscpy(vpu->mdev.model, DRIVER_NAME, sizeof(vpu->mdev.model));
+	strscpy(vpu->mdev.bus_info, "platform:" DRIVER_NAME,
+		sizeof(vpu->mdev.bus_info));
 	media_device_init(&vpu->mdev);
+	vpu->mdev.ops = &rockchip_m2m_media_ops;
 	vpu->v4l2_dev.mdev = &vpu->mdev;
 
 	ret = rockchip_vpu_video_device_register(vpu);
diff --git a/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c b/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c
index 2b28403314bc..1b5a675ef24f 100644
--- a/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c
+++ b/drivers/staging/media/rockchip/vpu/rockchip_vpu_enc.c
@@ -555,14 +555,32 @@  static void rockchip_vpu_stop_streaming(struct vb2_queue *q)
 			vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 		if (!vbuf)
 			break;
+		v4l2_ctrl_request_complete(vbuf->vb2_buf.req_obj.req, &ctx->ctrl_handler);
 		v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 	}
 }
 
+static void rockchip_vpu_buf_request_complete(struct vb2_buffer *vb)
+{
+	struct rockchip_vpu_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+
+	v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->ctrl_handler);
+}
+
+static int rockchip_vpu_buf_out_validate(struct vb2_buffer *vb)
+{
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+
+	vbuf->field = V4L2_FIELD_NONE;
+	return 0;
+}
+
 const struct vb2_ops rockchip_vpu_enc_queue_ops = {
 	.queue_setup = rockchip_vpu_queue_setup,
 	.buf_prepare = rockchip_vpu_buf_prepare,
 	.buf_queue = rockchip_vpu_buf_queue,
+	.buf_out_validate = rockchip_vpu_buf_out_validate,
+	.buf_request_complete = rockchip_vpu_buf_request_complete,
 	.start_streaming = rockchip_vpu_start_streaming,
 	.stop_streaming = rockchip_vpu_stop_streaming,
 	.wait_prepare = vb2_ops_wait_prepare,