lkml.org 
[lkml]   [2019]   [Jan]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH 10/10] venus: dec: make decoder compliant with stateful codec API
    From
    Date
    Hi Malathi,

    On 1/21/19 1:20 PM, mgottam@codeaurora.org wrote:
    > On 2019-01-17 21:50, Stanimir Varbanov wrote:
    >> This refactored code for start/stop streaming vb2 operations and
    >> adds a state machine handling similar to the one in stateful codec
    >> API documentation. One major change is that now the HFI session is
    >> started on STREAMON(OUTPUT) and stopped on REQBUF(OUTPUT,count=0),
    >> during that time streamoff(cap,out) just flush buffers but doesn't
    >> stop the session. The other major change is that now the capture
    >> and output queues are completely separated.
    >>
    >> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
    >> ---
    >>  drivers/media/platform/qcom/venus/core.h    |  20 +-
    >>  drivers/media/platform/qcom/venus/helpers.c |  23 +-
    >>  drivers/media/platform/qcom/venus/helpers.h |   5 +
    >>  drivers/media/platform/qcom/venus/vdec.c    | 449 ++++++++++++++++----
    >>  4 files changed, 389 insertions(+), 108 deletions(-)
    >>
    >> diff --git a/drivers/media/platform/qcom/venus/core.h
    >> b/drivers/media/platform/qcom/venus/core.h
    >> index 79c7e816c706..5a133c203455 100644
    >> --- a/drivers/media/platform/qcom/venus/core.h
    >> +++ b/drivers/media/platform/qcom/venus/core.h
    >> @@ -218,6 +218,15 @@ struct venus_buffer {
    >>
    >>  #define to_venus_buffer(ptr)    container_of(ptr, struct
    >> venus_buffer, vb)
    >>
    >> +#define DEC_STATE_UNINIT        0
    >> +#define DEC_STATE_INIT            1
    >> +#define DEC_STATE_CAPTURE_SETUP        2
    >> +#define DEC_STATE_STOPPED        3
    >> +#define DEC_STATE_SEEK            4
    >> +#define DEC_STATE_DRAIN            5
    >> +#define DEC_STATE_DECODING        6
    >> +#define DEC_STATE_DRC            7
    >> +
    >>  /**
    >>   * struct venus_inst - holds per instance paramerters
    >>   *
    >> @@ -241,6 +250,10 @@ struct venus_buffer {
    >>   * @colorspace:    current color space
    >>   * @quantization:    current quantization
    >>   * @xfer_func:    current xfer function
    >> + * @codec_state:    current codec API state (see DEC/ENC_STATE_)
    >> + * @reconf_wait:    wait queue for resolution change event
    >> + * @ten_bits:        does new stream is 10bits depth
    >> + * @buf_count:        used to count number number of buffers (reqbuf(0))
    >>   * @fps:        holds current FPS
    >>   * @timeperframe:    holds current time per frame structure
    >>   * @fmt_out:    a reference to output format structure
    >> @@ -255,8 +268,6 @@ struct venus_buffer {
    >>   * @opb_buftype:    output picture buffer type
    >>   * @opb_fmt:        output picture buffer raw format
    >>   * @reconfig:    a flag raised by decoder when the stream resolution
    >> changed
    >> - * @reconfig_width:    holds the new width
    >> - * @reconfig_height:    holds the new height
    >>   * @hfi_codec:        current codec for this instance in HFI space
    >>   * @sequence_cap:    a sequence counter for capture queue
    >>   * @sequence_out:    a sequence counter for output queue
    >> @@ -296,6 +307,9 @@ struct venus_inst {
    >>      u8 ycbcr_enc;
    >>      u8 quantization;
    >>      u8 xfer_func;
    >> +    unsigned int codec_state;
    >> +    wait_queue_head_t reconf_wait;
    >> +    int buf_count;
    >>      u64 fps;
    >>      struct v4l2_fract timeperframe;
    >>      const struct venus_format *fmt_out;
    >> @@ -310,8 +324,6 @@ struct venus_inst {
    >>      u32 opb_buftype;
    >>      u32 opb_fmt;
    >>      bool reconfig;
    >> -    u32 reconfig_width;
    >> -    u32 reconfig_height;
    >>      u32 hfi_codec;
    >>      u32 sequence_cap;
    >>      u32 sequence_out;
    >> diff --git a/drivers/media/platform/qcom/venus/helpers.c
    >> b/drivers/media/platform/qcom/venus/helpers.c
    >> index 637ce7b82d94..25d8cceccae4 100644
    >> --- a/drivers/media/platform/qcom/venus/helpers.c
    >> +++ b/drivers/media/platform/qcom/venus/helpers.c
    >> @@ -1030,16 +1030,15 @@ void venus_helper_vb2_buf_queue(struct
    >> vb2_buffer *vb)
    >>
    >>      v4l2_m2m_buf_queue(m2m_ctx, vbuf);
    >>
    >> -    if (!(inst->streamon_out & inst->streamon_cap))
    >> -        goto unlock;
    >> -
    >> -    ret = is_buf_refed(inst, vbuf);
    >> -    if (ret)
    >> -        goto unlock;
    >> +    if (IS_OUT(vb->vb2_queue, inst) || IS_CAP(vb->vb2_queue, inst)) {
    >> +        ret = is_buf_refed(inst, vbuf);
    >> +        if (ret)
    >> +            goto unlock;
    >>
    >> -    ret = session_process_buf(inst, vbuf);
    >> -    if (ret)
    >> -        return_buf_error(inst, vbuf);
    >> +        ret = session_process_buf(inst, vbuf);
    >> +        if (ret)
    >> +            return_buf_error(inst, vbuf);
    >> +    }
    >>
    >>  unlock:
    >>      mutex_unlock(&inst->lock);
    >
    > Hi Stan,
    >
    > In case of encoder, we are queuing buffers only after both planes are
    > streamed on.
    > As we don’t have any reconfig event in case of encoder,
    > it’s better if we stick to the earlier implementation of queuing buffers.
    >
    > So I would recommend to add a check for the same in the below way :
    >
    > diff --git a/drivers/media/platform/qcom/venus/helpers.c
    > b/drivers/media/platform/qcom/venus/helpers.c
    > index 25d8cce..cc490fe2 100644
    > --- a/drivers/media/platform/qcom/venus/helpers.c
    > +++ b/drivers/media/platform/qcom/venus/helpers.c
    > @@ -1029,6 +1029,8 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer
    > *vb)
    >         mutex_lock(&inst->lock);
    >
    >         v4l2_m2m_buf_queue(m2m_ctx, vbuf);
    > +       if (inst->session_type == VIDC_SESSION_TYPE_ENC &&
    > !(inst->streamon_out & inst->streamon_cap))
    > +               goto unlock;
    >
    >         if (IS_OUT(vb->vb2_queue, inst) || IS_CAP(vb->vb2_queue, inst)) {
    >                 ret = is_buf_refed(inst, vbuf);
    >
    > Please provide your view.

    I agree that this change is needed for encoder and will incorporate such
    a change in next version.

    --
    regards,
    Stan

    \
     
     \ /
      Last update: 2019-01-24 11:24    [W:4.103 / U:0.180 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site