Hi Malathi,
On 1/21/19 1:20 PM, mgottam@xxxxxxxxxxxxxx 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@xxxxxxxxxx>
>> ---
>> 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
