How should we resolve this patch in relation to the "venus: venc: Init the session only once in queue_setup" [1] patch? "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 >