| Message ID | 1603117737-16965-1-git-send-email-dikshita@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | venus: venc: add handling for VIDIOC_ENCODER_CMD | expand |
Hi Dikshita, On Mon, Oct 19, 2020 at 11:29 PM Dikshita Agarwal <dikshita@codeaurora.org> wrote: > > Add handling for below commands in encoder: > 1. V4L2_ENC_CMD_STOP > 2. V4L2_ENC_CMD_START I suspect this can be implemented more easily (and more safely) using the m2m encoder helpers introduced recently. Please see this commit for details: https://github.com/torvalds/linux/commit/2b48e113866a6735de3a99531183afb6217c2a60 By making use of this you can probably get rid of venus_enc_state entirely. Also this generic implementation should take care of corner cases that this patch does not address (like streaming off while a drain is in progress). > > Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org> > --- > drivers/media/platform/qcom/venus/core.h | 9 +++++ > drivers/media/platform/qcom/venus/venc.c | 64 +++++++++++++++++++++++++++++++- > 2 files changed, 72 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h > index e30eeaf..5c46936 100644 > --- a/drivers/media/platform/qcom/venus/core.h > +++ b/drivers/media/platform/qcom/venus/core.h > @@ -276,6 +276,14 @@ enum venus_dec_state { > VENUS_DEC_STATE_DRC = 7, > }; > > +enum venus_enc_state { > + VENUS_ENC_STATE_DEINIT = 0, > + VENUS_ENC_STATE_INIT = 1, > + VENUS_ENC_STATE_ENCODING = 2, > + VENUS_ENC_STATE_STOPPED = 3, > + VENUS_ENC_STATE_DRAIN = 4, > +}; > + > struct venus_ts_metadata { > bool used; > u64 ts_ns; > @@ -367,6 +375,7 @@ struct venus_inst { > u8 quantization; > u8 xfer_func; > enum venus_dec_state codec_state; > + enum venus_enc_state enc_state; > wait_queue_head_t reconf_wait; > unsigned int subscriptions; > int buf_count; > diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c > index f7fb6e3..c6143b0 100644 > --- a/drivers/media/platform/qcom/venus/venc.c > +++ b/drivers/media/platform/qcom/venus/venc.c > @@ -498,6 +498,46 @@ 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 *cmd) > +{ > + struct venus_inst *inst = to_inst(file); > + struct hfi_frame_data fdata = {0}; > + int ret = 0; > + > + ret = v4l2_m2m_ioctl_try_encoder_cmd(file, fh, cmd); > + if (ret) > + return ret; > + > + mutex_lock(&inst->lock); > + > + if (cmd->cmd == V4L2_ENC_CMD_STOP && > + inst->enc_state == VENUS_ENC_STATE_ENCODING) { > + /* > + * Implement V4L2_ENC_CMD_STOP by enqueue an empty buffer on > + * encoder input to signal EOS. > + */ > + if (!(inst->streamon_out && inst->streamon_cap)) > + goto unlock; > + > + fdata.buffer_type = HFI_BUFFER_INPUT; > + fdata.flags |= HFI_BUFFERFLAG_EOS; > + fdata.device_addr = 0xdeadb000; > + > + ret = hfi_session_process_buf(inst, &fdata); > + > + inst->enc_state = VENUS_ENC_STATE_DRAIN; > + } else if (cmd->cmd == V4L2_ENC_CMD_START && > + inst->enc_state == VENUS_ENC_STATE_STOPPED) { > + vb2_clear_last_buffer_dequeued(&inst->fh.m2m_ctx->cap_q_ctx.q); > + inst->enc_state = VENUS_ENC_STATE_ENCODING; > + } > + > +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, > @@ -525,6 +565,7 @@ static int venc_enum_frameintervals(struct file *file, void *fh, > .vidioc_enum_frameintervals = venc_enum_frameintervals, > .vidioc_subscribe_event = v4l2_ctrl_subscribe_event, > .vidioc_unsubscribe_event = v4l2_event_unsubscribe, > + .vidioc_encoder_cmd = venc_encoder_cmd, > }; > > static int venc_set_properties(struct venus_inst *inst) > @@ -884,6 +925,8 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count) > if (ret) > goto deinit_sess; > > + inst->enc_state = VENUS_ENC_STATE_ENCODING; > + > mutex_unlock(&inst->lock); > > return 0; > @@ -903,8 +946,19 @@ 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); > > mutex_lock(&inst->lock); > + > + if (inst->enc_state == VENUS_ENC_STATE_STOPPED) { > + vbuf->sequence = inst->sequence_cap++; > + vbuf->field = V4L2_FIELD_NONE; > + vb2_set_plane_payload(vb, 0, 0); > + v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_DONE); > + mutex_unlock(&inst->lock); > + return; > + } > + > venus_helper_vb2_buf_queue(vb); > mutex_unlock(&inst->lock); > } > @@ -943,6 +997,11 @@ 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 ((vbuf->flags & V4L2_BUF_FLAG_LAST) && > + inst->enc_state == VENUS_ENC_STATE_DRAIN) { > + inst->enc_state = VENUS_ENC_STATE_STOPPED; > + } > } else { > vbuf->sequence = inst->sequence_out++; > } > @@ -1041,6 +1100,9 @@ static int venc_open(struct file *file) > inst->clk_data.core_id = VIDC_CORE_ID_DEFAULT; > inst->core_acquired = false; > > + if (inst->enc_state == VENUS_ENC_STATE_DEINIT) > + inst->enc_state = VENUS_ENC_STATE_INIT; > + > venus_helper_init_instance(inst); > > ret = pm_runtime_get_sync(core->dev_enc); > @@ -1105,7 +1167,7 @@ static int venc_close(struct file *file) > mutex_destroy(&inst->lock); > v4l2_fh_del(&inst->fh); > v4l2_fh_exit(&inst->fh); > - > + inst->enc_state = VENUS_ENC_STATE_DEINIT; > pm_runtime_put_sync(inst->core->dev_enc); > > kfree(inst); > -- > 1.9.1 >
Hi Alex, Thanks for your suggestion, the helpers are good but it is complicated to use them in video deriver as video driver needs to deal with FW interface and wait for buffer processing from FW. So these helpers can't be used directly. For example in case of B frames, to encode such frame FW expect one more buffer to be queued on src queue which needs to be handled by queueing an empty buffer with EOS flag. Without EOS, we can't handle B frames. Thanks, Dikshita On 2020-10-20 14:27, Alexandre Courbot wrote: > Hi Dikshita, > > On Mon, Oct 19, 2020 at 11:29 PM Dikshita Agarwal > <dikshita@codeaurora.org> wrote: >> >> Add handling for below commands in encoder: >> 1. V4L2_ENC_CMD_STOP >> 2. V4L2_ENC_CMD_START > > I suspect this can be implemented more easily (and more safely) using > the m2m encoder helpers introduced recently. Please see this commit > for details: > > https://github.com/torvalds/linux/commit/2b48e113866a6735de3a99531183afb6217c2a60 > > By making use of this you can probably get rid of venus_enc_state > entirely. Also this generic implementation should take care of corner > cases that this patch does not address (like streaming off while a > drain is in progress). > >> >> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org> >> --- >> drivers/media/platform/qcom/venus/core.h | 9 +++++ >> drivers/media/platform/qcom/venus/venc.c | 64 >> +++++++++++++++++++++++++++++++- >> 2 files changed, 72 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/media/platform/qcom/venus/core.h >> b/drivers/media/platform/qcom/venus/core.h >> index e30eeaf..5c46936 100644 >> --- a/drivers/media/platform/qcom/venus/core.h >> +++ b/drivers/media/platform/qcom/venus/core.h >> @@ -276,6 +276,14 @@ enum venus_dec_state { >> VENUS_DEC_STATE_DRC = 7, >> }; >> >> +enum venus_enc_state { >> + VENUS_ENC_STATE_DEINIT = 0, >> + VENUS_ENC_STATE_INIT = 1, >> + VENUS_ENC_STATE_ENCODING = 2, >> + VENUS_ENC_STATE_STOPPED = 3, >> + VENUS_ENC_STATE_DRAIN = 4, >> +}; >> + >> struct venus_ts_metadata { >> bool used; >> u64 ts_ns; >> @@ -367,6 +375,7 @@ struct venus_inst { >> u8 quantization; >> u8 xfer_func; >> enum venus_dec_state codec_state; >> + enum venus_enc_state enc_state; >> wait_queue_head_t reconf_wait; >> unsigned int subscriptions; >> int buf_count; >> diff --git a/drivers/media/platform/qcom/venus/venc.c >> b/drivers/media/platform/qcom/venus/venc.c >> index f7fb6e3..c6143b0 100644 >> --- a/drivers/media/platform/qcom/venus/venc.c >> +++ b/drivers/media/platform/qcom/venus/venc.c >> @@ -498,6 +498,46 @@ 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 >> *cmd) >> +{ >> + struct venus_inst *inst = to_inst(file); >> + struct hfi_frame_data fdata = {0}; >> + int ret = 0; >> + >> + ret = v4l2_m2m_ioctl_try_encoder_cmd(file, fh, cmd); >> + if (ret) >> + return ret; >> + >> + mutex_lock(&inst->lock); >> + >> + if (cmd->cmd == V4L2_ENC_CMD_STOP && >> + inst->enc_state == VENUS_ENC_STATE_ENCODING) { >> + /* >> + * Implement V4L2_ENC_CMD_STOP by enqueue an empty >> buffer on >> + * encoder input to signal EOS. >> + */ >> + if (!(inst->streamon_out && inst->streamon_cap)) >> + goto unlock; >> + >> + fdata.buffer_type = HFI_BUFFER_INPUT; >> + fdata.flags |= HFI_BUFFERFLAG_EOS; >> + fdata.device_addr = 0xdeadb000; >> + >> + ret = hfi_session_process_buf(inst, &fdata); >> + >> + inst->enc_state = VENUS_ENC_STATE_DRAIN; >> + } else if (cmd->cmd == V4L2_ENC_CMD_START && >> + inst->enc_state == VENUS_ENC_STATE_STOPPED) { >> + >> vb2_clear_last_buffer_dequeued(&inst->fh.m2m_ctx->cap_q_ctx.q); >> + inst->enc_state = VENUS_ENC_STATE_ENCODING; >> + } >> + >> +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, >> @@ -525,6 +565,7 @@ static int venc_enum_frameintervals(struct file >> *file, void *fh, >> .vidioc_enum_frameintervals = venc_enum_frameintervals, >> .vidioc_subscribe_event = v4l2_ctrl_subscribe_event, >> .vidioc_unsubscribe_event = v4l2_event_unsubscribe, >> + .vidioc_encoder_cmd = venc_encoder_cmd, >> }; >> >> static int venc_set_properties(struct venus_inst *inst) >> @@ -884,6 +925,8 @@ static int venc_start_streaming(struct vb2_queue >> *q, unsigned int count) >> if (ret) >> goto deinit_sess; >> >> + inst->enc_state = VENUS_ENC_STATE_ENCODING; >> + >> mutex_unlock(&inst->lock); >> >> return 0; >> @@ -903,8 +946,19 @@ 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); >> >> mutex_lock(&inst->lock); >> + >> + if (inst->enc_state == VENUS_ENC_STATE_STOPPED) { >> + vbuf->sequence = inst->sequence_cap++; >> + vbuf->field = V4L2_FIELD_NONE; >> + vb2_set_plane_payload(vb, 0, 0); >> + v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_DONE); >> + mutex_unlock(&inst->lock); >> + return; >> + } >> + >> venus_helper_vb2_buf_queue(vb); >> mutex_unlock(&inst->lock); >> } >> @@ -943,6 +997,11 @@ 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 ((vbuf->flags & V4L2_BUF_FLAG_LAST) && >> + inst->enc_state == VENUS_ENC_STATE_DRAIN) { >> + inst->enc_state = VENUS_ENC_STATE_STOPPED; >> + } >> } else { >> vbuf->sequence = inst->sequence_out++; >> } >> @@ -1041,6 +1100,9 @@ static int venc_open(struct file *file) >> inst->clk_data.core_id = VIDC_CORE_ID_DEFAULT; >> inst->core_acquired = false; >> >> + if (inst->enc_state == VENUS_ENC_STATE_DEINIT) >> + inst->enc_state = VENUS_ENC_STATE_INIT; >> + >> venus_helper_init_instance(inst); >> >> ret = pm_runtime_get_sync(core->dev_enc); >> @@ -1105,7 +1167,7 @@ static int venc_close(struct file *file) >> mutex_destroy(&inst->lock); >> v4l2_fh_del(&inst->fh); >> v4l2_fh_exit(&inst->fh); >> - >> + inst->enc_state = VENUS_ENC_STATE_DEINIT; >> pm_runtime_put_sync(inst->core->dev_enc); >> >> kfree(inst); >> -- >> 1.9.1 >>
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h index e30eeaf..5c46936 100644 --- a/drivers/media/platform/qcom/venus/core.h +++ b/drivers/media/platform/qcom/venus/core.h @@ -276,6 +276,14 @@ enum venus_dec_state { VENUS_DEC_STATE_DRC = 7, }; +enum venus_enc_state { + VENUS_ENC_STATE_DEINIT = 0, + VENUS_ENC_STATE_INIT = 1, + VENUS_ENC_STATE_ENCODING = 2, + VENUS_ENC_STATE_STOPPED = 3, + VENUS_ENC_STATE_DRAIN = 4, +}; + struct venus_ts_metadata { bool used; u64 ts_ns; @@ -367,6 +375,7 @@ struct venus_inst { u8 quantization; u8 xfer_func; enum venus_dec_state codec_state; + enum venus_enc_state enc_state; wait_queue_head_t reconf_wait; unsigned int subscriptions; int buf_count; diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c index f7fb6e3..c6143b0 100644 --- a/drivers/media/platform/qcom/venus/venc.c +++ b/drivers/media/platform/qcom/venus/venc.c @@ -498,6 +498,46 @@ 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 *cmd) +{ + struct venus_inst *inst = to_inst(file); + struct hfi_frame_data fdata = {0}; + int ret = 0; + + ret = v4l2_m2m_ioctl_try_encoder_cmd(file, fh, cmd); + if (ret) + return ret; + + mutex_lock(&inst->lock); + + if (cmd->cmd == V4L2_ENC_CMD_STOP && + inst->enc_state == VENUS_ENC_STATE_ENCODING) { + /* + * Implement V4L2_ENC_CMD_STOP by enqueue an empty buffer on + * encoder input to signal EOS. + */ + if (!(inst->streamon_out && inst->streamon_cap)) + goto unlock; + + fdata.buffer_type = HFI_BUFFER_INPUT; + fdata.flags |= HFI_BUFFERFLAG_EOS; + fdata.device_addr = 0xdeadb000; + + ret = hfi_session_process_buf(inst, &fdata); + + inst->enc_state = VENUS_ENC_STATE_DRAIN; + } else if (cmd->cmd == V4L2_ENC_CMD_START && + inst->enc_state == VENUS_ENC_STATE_STOPPED) { + vb2_clear_last_buffer_dequeued(&inst->fh.m2m_ctx->cap_q_ctx.q); + inst->enc_state = VENUS_ENC_STATE_ENCODING; + } + +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, @@ -525,6 +565,7 @@ static int venc_enum_frameintervals(struct file *file, void *fh, .vidioc_enum_frameintervals = venc_enum_frameintervals, .vidioc_subscribe_event = v4l2_ctrl_subscribe_event, .vidioc_unsubscribe_event = v4l2_event_unsubscribe, + .vidioc_encoder_cmd = venc_encoder_cmd, }; static int venc_set_properties(struct venus_inst *inst) @@ -884,6 +925,8 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count) if (ret) goto deinit_sess; + inst->enc_state = VENUS_ENC_STATE_ENCODING; + mutex_unlock(&inst->lock); return 0; @@ -903,8 +946,19 @@ 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); mutex_lock(&inst->lock); + + if (inst->enc_state == VENUS_ENC_STATE_STOPPED) { + vbuf->sequence = inst->sequence_cap++; + vbuf->field = V4L2_FIELD_NONE; + vb2_set_plane_payload(vb, 0, 0); + v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_DONE); + mutex_unlock(&inst->lock); + return; + } + venus_helper_vb2_buf_queue(vb); mutex_unlock(&inst->lock); } @@ -943,6 +997,11 @@ 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 ((vbuf->flags & V4L2_BUF_FLAG_LAST) && + inst->enc_state == VENUS_ENC_STATE_DRAIN) { + inst->enc_state = VENUS_ENC_STATE_STOPPED; + } } else { vbuf->sequence = inst->sequence_out++; } @@ -1041,6 +1100,9 @@ static int venc_open(struct file *file) inst->clk_data.core_id = VIDC_CORE_ID_DEFAULT; inst->core_acquired = false; + if (inst->enc_state == VENUS_ENC_STATE_DEINIT) + inst->enc_state = VENUS_ENC_STATE_INIT; + venus_helper_init_instance(inst); ret = pm_runtime_get_sync(core->dev_enc); @@ -1105,7 +1167,7 @@ static int venc_close(struct file *file) mutex_destroy(&inst->lock); v4l2_fh_del(&inst->fh); v4l2_fh_exit(&inst->fh); - + inst->enc_state = VENUS_ENC_STATE_DEINIT; pm_runtime_put_sync(inst->core->dev_enc); kfree(inst);
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> --- drivers/media/platform/qcom/venus/core.h | 9 +++++ drivers/media/platform/qcom/venus/venc.c | 64 +++++++++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 1 deletion(-)