Hi Fritz, On 1/2/21 2:13 AM, Fritz Koenig wrote: > How should we resolve this patch in relation to the "venus: venc: Init > the session only once in queue_setup" [1] patch? I plan to make a pull request including 4/8 and 5/8 patches from this series which are infact fixes. After that I will send v2 on top of this pull-request. Unfortunately I have to postpone this series because I see issues when execute in a row my tests for drain and reset encoder states. > > "venus: venc: Init the session only once in queue_setup" comes after > and reworks |venc_start_streaming| substantially. This patch > implements |venc_stop_streaming|, but maybe that is not needed with > the newer patch? Can this one just be dropped, or does it need > rework? > > -Fritz > > [1]: https://lore.kernel.org/patchwork/patch/1349416/ > > On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov > <stanimir.varbanov@xxxxxxxxxx> wrote: >> >> Redesign the encoder driver to be compliant with stateful encoder >> spec - specifically adds handling of Reset state. >> >> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@xxxxxxxxxx> >> --- >> drivers/media/platform/qcom/venus/venc.c | 155 ++++++++++++++++++----- >> 1 file changed, 122 insertions(+), 33 deletions(-) >> >> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c >> index 7512e4a16270..f1ae89d45a54 100644 >> --- a/drivers/media/platform/qcom/venus/venc.c >> +++ b/drivers/media/platform/qcom/venus/venc.c >> @@ -907,6 +907,54 @@ static int venc_queue_setup(struct vb2_queue *q, >> return ret; >> } >> >> +static void venc_release_session(struct venus_inst *inst) >> +{ >> + int ret, abort = 0; >> + >> + mutex_lock(&inst->lock); >> + >> + ret = hfi_session_deinit(inst); >> + abort = (ret && ret != -EINVAL) ? 1 : 0; >> + >> + if (inst->session_error) >> + abort = 1; >> + >> + if (abort) >> + hfi_session_abort(inst); >> + >> + venus_pm_load_scale(inst); >> + INIT_LIST_HEAD(&inst->registeredbufs); >> + mutex_unlock(&inst->lock); >> + >> + venus_pm_release_core(inst); >> +} >> + >> +static int venc_buf_init(struct vb2_buffer *vb) >> +{ >> + struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue); >> + >> + inst->buf_count++; >> + >> + return venus_helper_vb2_buf_init(vb); >> +} >> + >> +static void venc_buf_cleanup(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 venus_buffer *buf = to_venus_buffer(vbuf); >> + >> + mutex_lock(&inst->lock); >> + if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) >> + if (!list_empty(&inst->registeredbufs)) >> + list_del_init(&buf->reg_list); >> + mutex_unlock(&inst->lock); >> + >> + inst->buf_count--; >> + if (!inst->buf_count) >> + venc_release_session(inst); >> +} >> + >> static int venc_verify_conf(struct venus_inst *inst) >> { >> enum hfi_version ver = inst->core->res->hfi_version; >> @@ -938,49 +986,57 @@ static int venc_verify_conf(struct venus_inst *inst) >> static int venc_start_streaming(struct vb2_queue *q, unsigned int count) >> { >> struct venus_inst *inst = vb2_get_drv_priv(q); >> + struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx; >> int ret; >> >> mutex_lock(&inst->lock); >> >> - if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) >> + v4l2_m2m_update_start_streaming_state(m2m_ctx, q); >> + >> + if (V4L2_TYPE_IS_OUTPUT(q->type)) >> inst->streamon_out = 1; >> else >> inst->streamon_cap = 1; >> >> - if (!(inst->streamon_out & inst->streamon_cap)) { >> - mutex_unlock(&inst->lock); >> - return 0; >> - } >> + if (inst->streamon_out && inst->streamon_cap && >> + inst->state == INST_UNINIT) { >> + venus_helper_init_instance(inst); >> >> - venus_helper_init_instance(inst); >> + inst->sequence_cap = 0; >> + inst->sequence_out = 0; >> >> - inst->sequence_cap = 0; >> - inst->sequence_out = 0; >> + ret = venc_init_session(inst); >> + if (ret) >> + goto bufs_done; >> >> - ret = venc_init_session(inst); >> - if (ret) >> - goto bufs_done; >> + ret = venus_pm_acquire_core(inst); >> + if (ret) >> + goto deinit_sess; >> >> - ret = venus_pm_acquire_core(inst); >> - if (ret) >> - goto deinit_sess; >> + ret = venc_verify_conf(inst); >> + if (ret) >> + goto deinit_sess; >> >> - ret = venc_set_properties(inst); >> - if (ret) >> - goto deinit_sess; >> + ret = venus_helper_set_num_bufs(inst, inst->num_input_bufs, >> + inst->num_output_bufs, 0); >> + if (ret) >> + goto deinit_sess; >> >> - ret = venc_verify_conf(inst); >> - if (ret) >> - goto deinit_sess; >> + ret = venus_helper_vb2_start_streaming(inst); >> + if (ret) >> + goto deinit_sess; >> >> - ret = venus_helper_set_num_bufs(inst, inst->num_input_bufs, >> - inst->num_output_bufs, 0); >> - if (ret) >> - goto deinit_sess; >> + venus_helper_process_initial_out_bufs(inst); >> + venus_helper_process_initial_cap_bufs(inst); >> + } else if (V4L2_TYPE_IS_CAPTURE(q->type) && inst->streamon_cap && >> + inst->streamon_out) { >> + ret = venus_helper_vb2_start_streaming(inst); >> + if (ret) >> + goto bufs_done; >> >> - ret = venus_helper_vb2_start_streaming(inst); >> - if (ret) >> - goto deinit_sess; >> + venus_helper_process_initial_out_bufs(inst); >> + venus_helper_process_initial_cap_bufs(inst); >> + } >> >> mutex_unlock(&inst->lock); >> >> @@ -990,15 +1046,43 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count) >> hfi_session_deinit(inst); >> bufs_done: >> venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_QUEUED); >> - if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) >> + if (V4L2_TYPE_IS_OUTPUT(q->type)) >> inst->streamon_out = 0; >> else >> inst->streamon_cap = 0; >> + >> mutex_unlock(&inst->lock); >> return ret; >> } >> >> -static void venc_vb2_buf_queue(struct vb2_buffer *vb) >> +static void venc_stop_streaming(struct vb2_queue *q) >> +{ >> + struct venus_inst *inst = vb2_get_drv_priv(q); >> + struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx; >> + int ret = -EINVAL; >> + >> + mutex_lock(&inst->lock); >> + >> + v4l2_m2m_clear_state(m2m_ctx); >> + >> + if (V4L2_TYPE_IS_CAPTURE(q->type)) { >> + ret = hfi_session_stop(inst); >> + ret |= hfi_session_unload_res(inst); >> + ret |= venus_helper_unregister_bufs(inst); >> + ret |= venus_helper_intbufs_free(inst); >> + } >> + >> + venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR); >> + >> + if (V4L2_TYPE_IS_OUTPUT(q->type)) >> + inst->streamon_out = 0; >> + else >> + inst->streamon_cap = 0; >> + >> + mutex_unlock(&inst->lock); >> +} >> + >> +static void venc_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); >> @@ -1022,11 +1106,12 @@ static void venc_vb2_buf_queue(struct vb2_buffer *vb) >> >> static const struct vb2_ops venc_vb2_ops = { >> .queue_setup = venc_queue_setup, >> - .buf_init = venus_helper_vb2_buf_init, >> + .buf_init = venc_buf_init, >> + .buf_cleanup = venc_buf_cleanup, >> .buf_prepare = venus_helper_vb2_buf_prepare, >> .start_streaming = venc_start_streaming, >> - .stop_streaming = venus_helper_vb2_stop_streaming, >> - .buf_queue = venc_vb2_buf_queue, >> + .stop_streaming = venc_stop_streaming, >> + .buf_queue = venc_buf_queue, >> }; >> >> static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type, >> @@ -1084,8 +1169,12 @@ static const struct hfi_inst_ops venc_hfi_ops = { >> .event_notify = venc_event_notify, >> }; >> >> +static void venc_m2m_device_run(void *priv) >> +{ >> +} >> + >> static const struct v4l2_m2m_ops venc_m2m_ops = { >> - .device_run = venus_helper_m2m_device_run, >> + .device_run = venc_m2m_device_run, >> .job_abort = venus_helper_m2m_job_abort, >> }; >> >> -- >> 2.17.1 >> -- regards, Stan
