mbox series

[v2,0/8] Venus stateful encoder compliance

Message ID 20201111143755.24541-1-stanimir.varbanov@linaro.org
Headers show
Series Venus stateful encoder compliance | expand

Message

Stanimir Varbanov Nov. 11, 2020, 2:37 p.m. UTC 
Hello,

Here is v2 of the subject patchset. 

The patchset starts with few small preparation and fix patches, 1/8 to 5/8.
6/8 is redesigned Dikshita's patch and 7/8 add Reset encoder state handling.
The last 8/8 just delete not needed helper function.

The major changes are:
 * An attempt to reuse m2m helpers for drain and reset state in 6/8 and 7/8.
 * Use null encoder buffer to signal end-of-stream in 6/8.

Comments are welcome!

regards,
Stan

Dikshita Agarwal (1):
  venus: venc: add handling for VIDIOC_ENCODER_CMD

Stanimir Varbanov (7):
  venus: hfi: Use correct state in unload resources
  venus: helpers: Add a new helper for buffer processing
  venus: hfi_cmds: Allow null buffer address on encoder input
  venus: helpers: Calculate properly compressed buffer size
  venus: pm_helpers: Check instance state when calculate instance
    frequency
  venus: venc: Handle reset encoder state
  venus: helpers: Delete unused stop streaming helper

 drivers/media/platform/qcom/venus/helpers.c   |  65 ++---
 drivers/media/platform/qcom/venus/helpers.h   |   2 +-
 drivers/media/platform/qcom/venus/hfi.c       |   2 +-
 drivers/media/platform/qcom/venus/hfi.h       |   1 -
 drivers/media/platform/qcom/venus/hfi_cmds.c  |   2 +-
 .../media/platform/qcom/venus/pm_helpers.c    |   3 +
 drivers/media/platform/qcom/venus/venc.c      | 232 +++++++++++++++---
 7 files changed, 226 insertions(+), 81 deletions(-)

-- 
2.17.1

Comments

Fritz Koenig Nov. 29, 2020, 6:03 a.m. UTC | #1 
On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> The new helper will be used from encoder and decoder drivers
> to enqueue buffers for processing by firmware.
>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/helpers.c | 20 ++++++++++++++++++++
>  drivers/media/platform/qcom/venus/helpers.h |  1 +
>  2 files changed, 21 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index efa2781d6f55..688e3e3e8362 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -1369,6 +1369,26 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer *vb)
>  }
>  EXPORT_SYMBOL_GPL(venus_helper_vb2_buf_queue);
>
> +void venus_helper_process_buf(struct vb2_buffer *vb)
> +{
> +       struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +       struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
> +       int ret;
> +
> +       cache_payload(inst, vb);
> +
> +       if (vb2_start_streaming_called(vb->vb2_queue)) {
> +               ret = is_buf_refed(inst, vbuf);
> +               if (ret)
> +                       return;
> +
> +               ret = session_process_buf(inst, vbuf);
> +               if (ret)
> +                       return_buf_error(inst, vbuf);
> +       }
> +}
> +EXPORT_SYMBOL_GPL(venus_helper_process_buf);
> +
>  void venus_helper_buffers_done(struct venus_inst *inst, unsigned int type,
>                                enum vb2_buffer_state state)
>  {
> diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
> index f36c9f717798..231af29667e7 100644
> --- a/drivers/media/platform/qcom/venus/helpers.h
> +++ b/drivers/media/platform/qcom/venus/helpers.h
> @@ -19,6 +19,7 @@ void venus_helper_buffers_done(struct venus_inst *inst, unsigned int type,
>  int venus_helper_vb2_buf_init(struct vb2_buffer *vb);
>  int venus_helper_vb2_buf_prepare(struct vb2_buffer *vb);
>  void venus_helper_vb2_buf_queue(struct vb2_buffer *vb);
> +void venus_helper_process_buf(struct vb2_buffer *vb);
>  void venus_helper_vb2_stop_streaming(struct vb2_queue *q);
>  int venus_helper_vb2_start_streaming(struct venus_inst *inst);
>  void venus_helper_m2m_device_run(void *priv);
> --
> 2.17.1
>

Reviewed-by: Fritz Koenig <frkoenig@chromium.org>
Fritz Koenig Nov. 29, 2020, 6:08 a.m. UTC | #2 
On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>

> Skip calculating instance frequency if it is not in running state.

>

> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>

> ---

>  drivers/media/platform/qcom/venus/pm_helpers.c | 3 +++

>  1 file changed, 3 insertions(+)

>

> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c

> index ca99908ca3d3..cc84dc5e371b 100644

> --- a/drivers/media/platform/qcom/venus/pm_helpers.c

> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c

> @@ -940,6 +940,9 @@ static unsigned long calculate_inst_freq(struct venus_inst *inst,

>

>         mbs_per_sec = load_per_instance(inst);

>

> +       if (inst->state != INST_START)

> +               return 0;

> +

>         vpp_freq = mbs_per_sec * inst->clk_data.codec_freq_data->vpp_freq;

>         /* 21 / 20 is overhead factor */

>         vpp_freq += vpp_freq / 20;

> --

> 2.17.1

>


Reviewed-by: Fritz Koenig <frkoenig@chromium.org>
Fritz Koenig Nov. 29, 2020, 6:12 a.m. UTC | #3 
On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>

> From: Dikshita Agarwal <dikshita@codeaurora.org>

>

> Add handling for below commands in encoder:

> 1. V4L2_ENC_CMD_STOP

> 2. V4L2_ENC_CMD_START

>

> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>

> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>

> ---

>  drivers/media/platform/qcom/venus/venc.c | 77 +++++++++++++++++++++++-

>  1 file changed, 76 insertions(+), 1 deletion(-)

>

> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c

> index 99bfabf90bd2..7512e4a16270 100644

> --- a/drivers/media/platform/qcom/venus/venc.c

> +++ b/drivers/media/platform/qcom/venus/venc.c

> @@ -507,6 +507,59 @@ static int venc_enum_frameintervals(struct file *file, void *fh,

>         return 0;

>  }

>

> +static int venc_encoder_cmd(struct file *file, void *fh,

> +                           struct v4l2_encoder_cmd *ec)

> +{

> +       struct venus_inst *inst = to_inst(file);

> +       struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;

> +       struct hfi_frame_data fdata = {0};

> +       int ret = 0;

> +

> +       ret = v4l2_m2m_ioctl_try_encoder_cmd(file, fh, ec);

> +       if (ret < 0)

> +               return ret;

> +

> +       mutex_lock(&inst->lock);

> +

> +       if (!vb2_is_streaming(&m2m_ctx->cap_q_ctx.q) ||

> +           !vb2_is_streaming(&m2m_ctx->out_q_ctx.q))

> +               goto unlock;

> +

> +       if (m2m_ctx->is_draining) {

> +               ret = -EBUSY;

> +               goto unlock;

> +       }

> +

> +       if (ec->cmd == V4L2_ENC_CMD_STOP) {

> +               if (v4l2_m2m_has_stopped(m2m_ctx)) {

> +                       ret = 0;

> +                       goto unlock;

> +               }

> +

> +               m2m_ctx->is_draining = true;

> +

> +               fdata.buffer_type = HFI_BUFFER_INPUT;

> +               fdata.flags |= HFI_BUFFERFLAG_EOS;

> +               fdata.device_addr = 0;

> +               fdata.clnt_data = (u32)-1;

> +

> +               ret = hfi_session_process_buf(inst, &fdata);

> +               if (ret)

> +                       goto unlock;

> +       }

> +

> +       if (ec->cmd == V4L2_ENC_CMD_START && v4l2_m2m_has_stopped(m2m_ctx)) {

> +               vb2_clear_last_buffer_dequeued(&m2m_ctx->cap_q_ctx.q);

> +               inst->m2m_ctx->has_stopped = false;

> +               venus_helper_process_initial_out_bufs(inst);

> +               venus_helper_process_initial_cap_bufs(inst);

> +       }

> +

> +unlock:

> +       mutex_unlock(&inst->lock);

> +       return ret;

> +}

> +

>  static const struct v4l2_ioctl_ops venc_ioctl_ops = {

>         .vidioc_querycap = venc_querycap,

>         .vidioc_enum_fmt_vid_cap = venc_enum_fmt,

> @@ -534,6 +587,8 @@ static const struct v4l2_ioctl_ops venc_ioctl_ops = {

>         .vidioc_enum_frameintervals = venc_enum_frameintervals,

>         .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,

>         .vidioc_unsubscribe_event = v4l2_event_unsubscribe,

> +       .vidioc_try_encoder_cmd = v4l2_m2m_ioctl_try_encoder_cmd,

> +       .vidioc_encoder_cmd = venc_encoder_cmd,

>  };

>

>  static int venc_set_properties(struct venus_inst *inst)

> @@ -946,9 +1001,22 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)

>  static void venc_vb2_buf_queue(struct vb2_buffer *vb)

>  {

>         struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);

> +       struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);

> +       struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;

>

>         mutex_lock(&inst->lock);

> -       venus_helper_vb2_buf_queue(vb);

> +

> +       v4l2_m2m_buf_queue(m2m_ctx, vbuf);

> +

> +       if (!(inst->streamon_out && inst->streamon_cap))

> +               goto unlock;

> +

> +       if (v4l2_m2m_has_stopped(m2m_ctx))

> +               goto unlock;

> +

> +       venus_helper_process_buf(vb);

> +

> +unlock:

>         mutex_unlock(&inst->lock);

>  }

>

> @@ -968,6 +1036,7 @@ static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,

>         struct vb2_v4l2_buffer *vbuf;

>         struct vb2_buffer *vb;

>         unsigned int type;

> +       struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;

>

>         if (buf_type == HFI_BUFFER_INPUT)

>                 type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;

> @@ -986,6 +1055,12 @@ static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,

>                 vb->planes[0].data_offset = data_offset;

>                 vb->timestamp = timestamp_us * NSEC_PER_USEC;

>                 vbuf->sequence = inst->sequence_cap++;

> +

> +               if ((!bytesused && m2m_ctx->is_draining) ||

> +                   (vbuf->flags & V4L2_BUF_FLAG_LAST)) {

> +                       vbuf->flags |= V4L2_BUF_FLAG_LAST;

> +                       v4l2_m2m_mark_stopped(inst->m2m_ctx);

> +               }

>         } else {

>                 vbuf->sequence = inst->sequence_out++;

>         }

> --

> 2.17.1

>


Reviewed-by: Fritz Koenig <frkoenig@chromium.org>
Fritz Koenig Nov. 29, 2020, 7:17 p.m. UTC | #4 
Since this patchset adds support for V4L2_ENC_CMD_STOP and
VENUS_ENC_STATE_ENCODING it should also add support for
VIDIOC_TRY_ENCODER_CMD so that those commands are discoverable.  I've
made an attempt at that here:
https://www.spinics.net/lists/linux-media/msg182319.html

On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> Hello,
>
> Here is v2 of the subject patchset.
>
> The patchset starts with few small preparation and fix patches, 1/8 to 5/8.
> 6/8 is redesigned Dikshita's patch and 7/8 add Reset encoder state handling.
> The last 8/8 just delete not needed helper function.
>
> The major changes are:
>  * An attempt to reuse m2m helpers for drain and reset state in 6/8 and 7/8.
>  * Use null encoder buffer to signal end-of-stream in 6/8.
>
> Comments are welcome!
>
> regards,
> Stan
>
> Dikshita Agarwal (1):
>   venus: venc: add handling for VIDIOC_ENCODER_CMD
>
> Stanimir Varbanov (7):
>   venus: hfi: Use correct state in unload resources
>   venus: helpers: Add a new helper for buffer processing
>   venus: hfi_cmds: Allow null buffer address on encoder input
>   venus: helpers: Calculate properly compressed buffer size
>   venus: pm_helpers: Check instance state when calculate instance
>     frequency
>   venus: venc: Handle reset encoder state
>   venus: helpers: Delete unused stop streaming helper
>
>  drivers/media/platform/qcom/venus/helpers.c   |  65 ++---
>  drivers/media/platform/qcom/venus/helpers.h   |   2 +-
>  drivers/media/platform/qcom/venus/hfi.c       |   2 +-
>  drivers/media/platform/qcom/venus/hfi.h       |   1 -
>  drivers/media/platform/qcom/venus/hfi_cmds.c  |   2 +-
>  .../media/platform/qcom/venus/pm_helpers.c    |   3 +
>  drivers/media/platform/qcom/venus/venc.c      | 232 +++++++++++++++---
>  7 files changed, 226 insertions(+), 81 deletions(-)
>
> --
> 2.17.1
>
Stanimir Varbanov Nov. 30, 2020, 7:55 a.m. UTC | #5 
Hi Fritz,

On 11/29/20 9:17 PM, Fritz Koenig wrote:
> Since this patchset adds support for V4L2_ENC_CMD_STOP and

> VENUS_ENC_STATE_ENCODING it should also add support for

> VIDIOC_TRY_ENCODER_CMD so that those commands are discoverable.  I've


6/8 is adding it:

+	.vidioc_try_encoder_cmd = v4l2_m2m_ioctl_try_encoder_cmd,

> made an attempt at that here:

> https://www.spinics.net/lists/linux-media/msg182319.html

> 

> On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov

> <stanimir.varbanov@linaro.org> wrote:

>>

>> Hello,

>>

>> Here is v2 of the subject patchset.

>>

>> The patchset starts with few small preparation and fix patches, 1/8 to 5/8.

>> 6/8 is redesigned Dikshita's patch and 7/8 add Reset encoder state handling.

>> The last 8/8 just delete not needed helper function.

>>

>> The major changes are:

>>  * An attempt to reuse m2m helpers for drain and reset state in 6/8 and 7/8.

>>  * Use null encoder buffer to signal end-of-stream in 6/8.

>>

>> Comments are welcome!

>>

>> regards,

>> Stan

>>

>> Dikshita Agarwal (1):

>>   venus: venc: add handling for VIDIOC_ENCODER_CMD

>>

>> Stanimir Varbanov (7):

>>   venus: hfi: Use correct state in unload resources

>>   venus: helpers: Add a new helper for buffer processing

>>   venus: hfi_cmds: Allow null buffer address on encoder input

>>   venus: helpers: Calculate properly compressed buffer size

>>   venus: pm_helpers: Check instance state when calculate instance

>>     frequency

>>   venus: venc: Handle reset encoder state

>>   venus: helpers: Delete unused stop streaming helper

>>

>>  drivers/media/platform/qcom/venus/helpers.c   |  65 ++---

>>  drivers/media/platform/qcom/venus/helpers.h   |   2 +-

>>  drivers/media/platform/qcom/venus/hfi.c       |   2 +-

>>  drivers/media/platform/qcom/venus/hfi.h       |   1 -

>>  drivers/media/platform/qcom/venus/hfi_cmds.c  |   2 +-

>>  .../media/platform/qcom/venus/pm_helpers.c    |   3 +

>>  drivers/media/platform/qcom/venus/venc.c      | 232 +++++++++++++++---

>>  7 files changed, 226 insertions(+), 81 deletions(-)

>>

>> --

>> 2.17.1

>>


-- 
regards,
Stan
Fritz Koenig Nov. 30, 2020, 5:28 p.m. UTC | #6 
On Sun, Nov 29, 2020 at 11:55 PM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> Hi Fritz,
>
> On 11/29/20 9:17 PM, Fritz Koenig wrote:
> > Since this patchset adds support for V4L2_ENC_CMD_STOP and
> > VENUS_ENC_STATE_ENCODING it should also add support for
> > VIDIOC_TRY_ENCODER_CMD so that those commands are discoverable.  I've
>
> 6/8 is adding it:
>
> +       .vidioc_try_encoder_cmd = v4l2_m2m_ioctl_try_encoder_cmd,
>

Ahh, thanks.  I need to work on my reading comprehension.

> > made an attempt at that here:
> > https://www.spinics.net/lists/linux-media/msg182319.html
> >
> > On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
> > <stanimir.varbanov@linaro.org> wrote:
> >>
> >> Hello,
> >>
> >> Here is v2 of the subject patchset.
> >>
> >> The patchset starts with few small preparation and fix patches, 1/8 to 5/8.
> >> 6/8 is redesigned Dikshita's patch and 7/8 add Reset encoder state handling.
> >> The last 8/8 just delete not needed helper function.
> >>
> >> The major changes are:
> >>  * An attempt to reuse m2m helpers for drain and reset state in 6/8 and 7/8.
> >>  * Use null encoder buffer to signal end-of-stream in 6/8.
> >>
> >> Comments are welcome!
> >>
> >> regards,
> >> Stan
> >>
> >> Dikshita Agarwal (1):
> >>   venus: venc: add handling for VIDIOC_ENCODER_CMD
> >>
> >> Stanimir Varbanov (7):
> >>   venus: hfi: Use correct state in unload resources
> >>   venus: helpers: Add a new helper for buffer processing
> >>   venus: hfi_cmds: Allow null buffer address on encoder input
> >>   venus: helpers: Calculate properly compressed buffer size
> >>   venus: pm_helpers: Check instance state when calculate instance
> >>     frequency
> >>   venus: venc: Handle reset encoder state
> >>   venus: helpers: Delete unused stop streaming helper
> >>
> >>  drivers/media/platform/qcom/venus/helpers.c   |  65 ++---
> >>  drivers/media/platform/qcom/venus/helpers.h   |   2 +-
> >>  drivers/media/platform/qcom/venus/hfi.c       |   2 +-
> >>  drivers/media/platform/qcom/venus/hfi.h       |   1 -
> >>  drivers/media/platform/qcom/venus/hfi_cmds.c  |   2 +-
> >>  .../media/platform/qcom/venus/pm_helpers.c    |   3 +
> >>  drivers/media/platform/qcom/venus/venc.c      | 232 +++++++++++++++---
> >>  7 files changed, 226 insertions(+), 81 deletions(-)
> >>
> >> --
> >> 2.17.1
> >>
>
> --
> regards,
> Stan