Hello,
Đ¢his patchset is an attempt to make Venus encoder driver compliant with
stateful encoder spec. There are still few details which need to be
cleaned up so this can be treated as WIP. For example the usage of m2m
helpers to update the states and handing of the LAST capture buffer for
Drain state. Here mainly I re-designed the driver to able to handle
capture/output queues independently and properly go in and out of Reset
state.
These patches depend on [1].
Comments are welcome!
regards,
Stan
[1] https://lkml.org/lkml/2020/10/19/432
Stanimir Varbanov (4):
venus: hfi: Use correct state in unload resources
venus: helpers: Add a new helper for buffer processing
venus: venc: Handle reset encoder state
venus: helpers: Delete unused stop streaming helper
drivers/media/platform/qcom/venus/core.h | 10 +-
drivers/media/platform/qcom/venus/helpers.c | 63 ++---
drivers/media/platform/qcom/venus/helpers.h | 2 +-
drivers/media/platform/qcom/venus/hfi.c | 2 +-
drivers/media/platform/qcom/venus/hfi.h | 1 -
drivers/media/platform/qcom/venus/venc.c | 242 ++++++++++++++++----
6 files changed, 219 insertions(+), 101 deletions(-)
--
2.17.1
The new helper will be used from encoder and decoder drivers
to enqueue buffers for processing by firmware.
Signed-off-by: Stanimir Varbanov <[email protected]>
---
drivers/media/platform/qcom/venus/helpers.c | 20 ++++++++++++++++++++
drivers/media/platform/qcom/venus/helpers.h | 1 +
2 files changed, 21 insertions(+)
diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 2b6925b6c274..8d0ca70d740d 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -1369,6 +1369,26 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer *vb)
}
EXPORT_SYMBOL_GPL(venus_helper_vb2_buf_queue);
+void venus_helper_process_buf(struct vb2_buffer *vb)
+{
+ struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+ struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
+ int ret;
+
+ cache_payload(inst, vb);
+
+ if (vb2_start_streaming_called(vb->vb2_queue)) {
+ ret = is_buf_refed(inst, vbuf);
+ if (ret)
+ return;
+
+ ret = session_process_buf(inst, vbuf);
+ if (ret)
+ return_buf_error(inst, vbuf);
+ }
+}
+EXPORT_SYMBOL_GPL(venus_helper_process_buf);
+
void venus_helper_buffers_done(struct venus_inst *inst, unsigned int type,
enum vb2_buffer_state state)
{
diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
index a4a0562bc83f..dca4794c05d9 100644
--- a/drivers/media/platform/qcom/venus/helpers.h
+++ b/drivers/media/platform/qcom/venus/helpers.h
@@ -19,6 +19,7 @@ void venus_helper_buffers_done(struct venus_inst *inst, unsigned int type,
int venus_helper_vb2_buf_init(struct vb2_buffer *vb);
int venus_helper_vb2_buf_prepare(struct vb2_buffer *vb);
void venus_helper_vb2_buf_queue(struct vb2_buffer *vb);
+void venus_helper_process_buf(struct vb2_buffer *vb);
void venus_helper_vb2_stop_streaming(struct vb2_queue *q);
int venus_helper_vb2_start_streaming(struct venus_inst *inst);
void venus_helper_m2m_device_run(void *priv);
--
2.17.1
Redesign the encoder driver to be compliant with stateful encoder
spec - specifically adds handling of Reset state.
Signed-off-by: Stanimir Varbanov <[email protected]>
---
drivers/media/platform/qcom/venus/core.h | 10 +-
drivers/media/platform/qcom/venus/venc.c | 242 ++++++++++++++++++-----
2 files changed, 197 insertions(+), 55 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index 5c4693678e3f..294d5467a6a1 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -277,11 +277,11 @@ enum venus_dec_state {
};
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,
+ VENUS_ENC_STATE_INIT = 0,
+ VENUS_ENC_STATE_ENCODING = 1,
+ VENUS_ENC_STATE_STOPPED = 2,
+ VENUS_ENC_STATE_DRAIN = 3,
+ VENUS_ENC_STATE_RESET = 4,
};
struct venus_ts_metadata {
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index c6143b07914c..aa9255ddb0a5 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -565,6 +565,7 @@ static const struct v4l2_ioctl_ops venc_ioctl_ops = {
.vidioc_enum_frameintervals = venc_enum_frameintervals,
.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
+ .vidioc_try_encoder_cmd = v4l2_m2m_ioctl_try_encoder_cmd,
.vidioc_encoder_cmd = venc_encoder_cmd,
};
@@ -850,6 +851,69 @@ static int venc_queue_setup(struct vb2_queue *q,
return ret;
}
+static void venc_release_session(struct venus_inst *inst)
+{
+ struct venus_core *core = inst->core;
+ int ret, abort = 0;
+
+ mutex_lock(&inst->lock);
+
+ if (inst->enc_state != VENUS_ENC_STATE_RESET)
+ dev_dbg(core->dev_enc, VDBGH "wrong state!\n");
+
+ ret = hfi_session_stop(inst);
+ abort |= (ret && ret != -EINVAL) ? 1 : 0;
+ ret = hfi_session_unload_res(inst);
+ abort |= (ret && ret != -EINVAL) ? 1 : 0;
+ ret = venus_helper_unregister_bufs(inst);
+ abort |= (ret && ret != -EINVAL) ? 1 : 0;
+ ret = venus_helper_intbufs_free(inst);
+ abort |= (ret && ret != -EINVAL) ? 1 : 0;
+ 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);
+
+ inst->enc_state = VENUS_ENC_STATE_INIT;
+
+ 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;
@@ -881,61 +945,73 @@ 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);
- int ret;
+ int ret = 0;
mutex_lock(&inst->lock);
- if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+ 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))
+ goto unlock;
- venus_helper_init_instance(inst);
+ if (inst->enc_state == VENUS_ENC_STATE_INIT) {
+ 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_set_properties(inst);
- if (ret)
- goto deinit_sess;
+ ret = venc_verify_conf(inst);
+ if (ret)
+ goto deinit_sess;
- ret = venc_verify_conf(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 = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
- inst->num_output_bufs, 0);
- if (ret)
- goto deinit_sess;
+ ret = venus_helper_vb2_start_streaming(inst);
+ if (ret)
+ goto deinit_sess;
- 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);
- inst->enc_state = VENUS_ENC_STATE_ENCODING;
+ inst->enc_state = VENUS_ENC_STATE_ENCODING;
+ } else if (inst->enc_state == VENUS_ENC_STATE_RESET &&
+ V4L2_TYPE_IS_CAPTURE(q->type)) {
+ ret = venus_helper_vb2_start_streaming(inst);
+ if (ret)
+ goto bufs_done;
- mutex_unlock(&inst->lock);
+ venus_helper_process_initial_out_bufs(inst);
+ venus_helper_process_initial_cap_bufs(inst);
- return 0;
+ inst->enc_state = VENUS_ENC_STATE_ENCODING;
+ } else if (inst->enc_state == VENUS_ENC_STATE_STOPPED &&
+ V4L2_TYPE_IS_OUTPUT(q->type)) {
+ inst->enc_state = VENUS_ENC_STATE_ENCODING;
+ }
+
+unlock:
+ mutex_unlock(&inst->lock);
+ return ret;
deinit_sess:
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;
@@ -943,33 +1019,97 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
return ret;
}
-static void venc_vb2_buf_queue(struct vb2_buffer *vb)
+static int venc_stop_capture(struct venus_inst *inst)
+{
+ int ret;
+
+ switch (inst->enc_state) {
+ case VENUS_ENC_STATE_ENCODING:
+ case VENUS_ENC_STATE_DRAIN:
+ case VENUS_ENC_STATE_STOPPED:
+ inst->enc_state = VENUS_ENC_STATE_RESET;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = hfi_session_stop(inst);
+ ret |= hfi_session_unload_res(inst);
+ ret |= venus_helper_unregister_bufs(inst);
+ ret |= venus_helper_intbufs_free(inst);
+
+ return ret;
+}
+
+static int venc_stop_output(struct venus_inst *inst)
+{
+ switch (inst->enc_state) {
+ case VENUS_ENC_STATE_ENCODING:
+ inst->enc_state = VENUS_ENC_STATE_STOPPED;
+ break;
+ case VENUS_ENC_STATE_DRAIN:
+ inst->enc_state = VENUS_ENC_STATE_STOPPED;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void venc_stop_streaming(struct vb2_queue *q)
+{
+ struct venus_inst *inst = vb2_get_drv_priv(q);
+ int ret = -EINVAL;
+
+ mutex_lock(&inst->lock);
+
+ if (V4L2_TYPE_IS_OUTPUT(q->type))
+ ret = venc_stop_output(inst);
+ else
+ ret = venc_stop_capture(inst);
+
+ venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
+
+ if (ret)
+ goto unlock;
+
+ if (V4L2_TYPE_IS_OUTPUT(q->type))
+ inst->streamon_out = 0;
+ else
+ inst->streamon_cap = 0;
+
+unlock:
+ 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);
+ struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
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;
- }
+ v4l2_m2m_buf_queue(m2m_ctx, vbuf);
+
+ if (!(inst->streamon_out && inst->streamon_cap))
+ goto unlock;
+
+ venus_helper_process_buf(vb);
- venus_helper_vb2_buf_queue(vb);
+unlock:
mutex_unlock(&inst->lock);
}
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,
@@ -1025,8 +1165,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,
};
@@ -1098,11 +1242,9 @@ static int venc_open(struct file *file)
inst->core = core;
inst->session_type = VIDC_SESSION_TYPE_ENC;
inst->clk_data.core_id = VIDC_CORE_ID_DEFAULT;
+ inst->enc_state = VENUS_ENC_STATE_INIT;
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);
@@ -1167,7 +1309,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);
--
2.17.1
On Fri, Oct 23, 2020 at 5:57 AM Stanimir Varbanov
<[email protected]> wrote:
>
> Redesign the encoder driver to be compliant with stateful encoder
> spec - specifically adds handling of Reset state.
>
> Signed-off-by: Stanimir Varbanov <[email protected]>
> ---
> drivers/media/platform/qcom/venus/core.h | 10 +-
> drivers/media/platform/qcom/venus/venc.c | 242 ++++++++++++++++++-----
> 2 files changed, 197 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
> index 5c4693678e3f..294d5467a6a1 100644
> --- a/drivers/media/platform/qcom/venus/core.h
> +++ b/drivers/media/platform/qcom/venus/core.h
> @@ -277,11 +277,11 @@ enum venus_dec_state {
> };
>
> 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,
> + VENUS_ENC_STATE_INIT = 0,
> + VENUS_ENC_STATE_ENCODING = 1,
> + VENUS_ENC_STATE_STOPPED = 2,
> + VENUS_ENC_STATE_DRAIN = 3,
> + VENUS_ENC_STATE_RESET = 4,
> };
>
> struct venus_ts_metadata {
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index c6143b07914c..aa9255ddb0a5 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -565,6 +565,7 @@ static const struct v4l2_ioctl_ops venc_ioctl_ops = {
> .vidioc_enum_frameintervals = venc_enum_frameintervals,
> .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
> .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> + .vidioc_try_encoder_cmd = v4l2_m2m_ioctl_try_encoder_cmd,
> .vidioc_encoder_cmd = venc_encoder_cmd,
> };
>
> @@ -850,6 +851,69 @@ static int venc_queue_setup(struct vb2_queue *q,
> return ret;
> }
>
> +static void venc_release_session(struct venus_inst *inst)
> +{
> + struct venus_core *core = inst->core;
> + int ret, abort = 0;
> +
> + mutex_lock(&inst->lock);
> +
> + if (inst->enc_state != VENUS_ENC_STATE_RESET)
> + dev_dbg(core->dev_enc, VDBGH "wrong state!\n");
> +
> + ret = hfi_session_stop(inst);
> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
> + ret = hfi_session_unload_res(inst);
> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
> + ret = venus_helper_unregister_bufs(inst);
> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
> + ret = venus_helper_intbufs_free(inst);
> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
> + 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);
venus_pm_load_scale depends on inst->clk_data.codec_freq_data being
set up. This occurs in venc_init_session(). I am seeing scenarios
where the encoder is getting setup up, but before it is finished,
teardown occurs. If this teardown occurs before
inst->clk_data.codec_freq_data is initalized, a crash occurs. (also
"wrong state!" is printed out)
[ 106.593198] Unable to handle kernel NULL pointer dereference at
virtual address 0000000000000008
[ 106.603916] Mem abort info:
[ 106.608470] ESR = 0x96000006
[ 106.613802] EC = 0x25: DABT (current EL), IL = 32 bits
[ 106.619426] SET = 0, FnV = 0
[ 106.622619] EA = 0, S1PTW = 0
[ 106.625862] Data abort info:
[ 106.628835] ISV = 0, ISS = 0x00000006
[ 106.632785] CM = 0, WnR = 0
[ 106.635850] user pgtable: 4k pages, 39-bit VAs, pgdp=000000014839f000
[ 106.642472] [0000000000000008] pgd=000000016ab1f003,
pud=000000016ab1f003, pmd=0000000000000000
[ 106.651410] Internal error: Oops: 96000006 [#1] PREEMPT SMP
[ 106.657132] Modules linked in: rfcomm algif_hash algif_skcipher
af_alg uinput venus_dec venus_enc videobuf2_dma_sg hci_uart btqca
venus_core qcom_spmi_adc5 qcom_spmi_temp_alarm qcom_vadc_common
snd_soc_rt5682_i2c v4l2_mem2mem snd_soc_sc7180 snd_soc_rt5682
snd_soc_qcom_common snd_soc_rl6231 bluetooth ecdh_generic ecc
snd_soc_lpass_sc7180 snd_soc_lpass_hdmi snd_soc_lpass_cpu
snd_soc_lpass_platform snd_soc_max98357a xt_MASQUERADE fuse
iio_trig_sysfs cros_ec_lid_angle cros_ec_sensors cros_ec_sensors_core
industrialio_triggered_buffer cros_ec_sensors_ring rmtfs_mem kfifo_buf
cros_ec_sensorhub ath10k_snoc lzo_rle ath10k_core lzo_compress ath
zram mac80211 ipa qmi_helpers cfg80211 qcom_q6v5_mss qcom_pil_info
qcom_q6v5 qcom_common cdc_ether usbnet r8152 mii uvcvideo
videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_common
joydev
[ 106.732576] CPU: 7 PID: 3622 Comm: DrmThread Not tainted 5.4.72 #40
[ 106.739004] Hardware name: Google Lazor (rev1+) (DT)
[ 106.744103] pstate: 60400009 (nZCv daif +PAN -UAO)
[ 106.749027] pc : load_scale_v4+0x160/0x3a4 [venus_core]
[ 106.754396] lr : load_scale_v4+0x154/0x3a4 [venus_core]
[ 106.759766] sp : ffffffc0120ab9e0
[ 106.763171] x29: ffffffc0120ab9e0 x28: 0000000000000005
[ 106.768629] x27: 0000000000000000 x26: 0000000000000000
[ 106.774080] x25: 0000000000000000 x24: 000000000000001e
[ 106.779530] x23: 0000000000000000 x22: ffffff80b41a8000
[ 106.784980] x21: ffffffd344e97e98 x20: ffffff80cb5b8080
[ 106.790431] x19: ffffff80fa3b1410 x18: 00000000ffff0a10
[ 106.795881] x17: 000000000000003c x16: ffffffd398ec2e88
[ 106.801329] x15: 0000000000000006 x14: ffff001000000600
[ 106.806779] x13: 000000000002cca2 x12: 0000000000000000
[ 106.812229] x11: 0000000000000000 x10: 0000000000000000
[ 106.817679] x9 : 472cbd12793c4600 x8 : 0000000000000000
[ 106.823137] x7 : 0000000000000000 x6 : ffffffd399dbcc6c
[ 106.828585] x5 : 0000000000000000 x4 : 0000000000000000
[ 106.834035] x3 : 0000000000000000 x2 : ffffff80ff6ae5c0
[ 106.839487] x1 : ffffff80ff69e018 x0 : ffffffd344e990ac
[ 106.844937] Call trace:
[ 106.847460] load_scale_v4+0x160/0x3a4 [venus_core]
[ 106.852473] venc_buf_cleanup+0x198/0x1f0 [venus_enc]
[ 106.857656] __vb2_queue_free+0x90/0x1f4 [videobuf2_common]
[ 106.863374] vb2_core_queue_release+0x3c/0x50 [videobuf2_common]
[ 106.869541] vb2_queue_release+0x1c/0x28 [videobuf2_v4l2]
[ 106.875082] v4l2_m2m_ctx_release+0x24/0x40 [v4l2_mem2mem]
[ 106.880710] venc_close+0x24/0x78 [venus_enc]
[ 106.885188] v4l2_release+0x8c/0xdc
[ 106.888779] __fput+0xe0/0x214
[ 106.891916] ____fput+0x1c/0x28
[ 106.895148] task_work_run+0x94/0xc4
[ 106.898828] do_exit+0x244/0x7c8
[ 106.902147] do_group_exit+0x88/0x98
[ 106.905825] get_signal+0x1cc/0x674
[ 106.909415] do_notify_resume+0x134/0x1410
[ 106.913619] work_pending+0x8/0x10
[ 106.917119] Code: 97ffd58d f94032c8 90000020 9102b000 (f9400501)
[ 106.923377] ---[ end trace a9caaf72c228e386 ]---
[ 106.928767] Kernel panic - not syncing: Fatal exception
[ 106.934131] SMP: stopping secondary CPUs
[ 106.938347] Kernel Offset: 0x1388a00000 from 0xffffffc010000000
[ 106.944426] PHYS_OFFSET: 0xffffffd900000000
[ 106.948722] CPU features: 0x08102e,2a80aa18
[ 106.953015] Memory Limit: none
This is the debug log before the crash:
[Nov 2 15:33] qcom-venus aa00000.video-codec: VenusLow : venus hw
version 4.44.20a
[ +0.000065] videodev: v4l2_open: video2: open (0)
[ +0.000019] video2: VIDIOC_ENUM_FMT: index=0, type=vid-cap-mplane,
flags=0x1, pixelformat=H264, description='H.264'
[ +0.000017] video2: VIDIOC_ENUM_FMT: index=1, type=vid-cap-mplane,
flags=0x1, pixelformat=VP80, description='VP8'
[ +0.000042] video2: VIDIOC_ENUM_FMT: index=2, type=vid-cap-mplane,
flags=0x1, pixelformat=HEVC, description='HEVC'
[ +0.000030] video2: VIDIOC_ENUM_FMT: error -22: index=3,
type=vid-cap-mplane, flags=0x0, pixelformat=\x00\x00\x00\x00,
description=''
[ +0.000068] videodev: v4l2_release: video2: release
[ +0.002752] qcom-venus aa00000.video-codec: VenusLow : venus hw
version 4.44.20a
[ +0.000062] videodev: v4l2_open: video2: open (0)
[ +0.000071] video2: VIDIOC_ENUM_FRAMESIZES: index=0,
pixelformat=H264, type=3, min=96x96, max=4096x4096, step=16x16
[ +0.000012] video2: VIDIOC_TRY_ENCODER_CMD: cmd=1, flags=0x0
[ +0.000005] video2: VIDIOC_QUERYCAP: driver=qcom-venus,
card=Qualcomm Venus video encoder, bus=platform:qcom-venus,
version=0x00050448, capabilities=0x84204000, device_caps=0x04204000
[ +0.001055] video2: VIDIOC_REQBUFS: count=0, type=vid-out-mplane, memory=mmap
[ +0.000382] video2: VIDIOC_REQBUFS: count=0, type=vid-cap-mplane, memory=mmap
[ +0.000227] video2: VIDIOC_S_FMT: type=vid-cap-mplane, width=96,
height=96, format=H264, field=none, colorspace=0, num_planes=1,
flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
[ +0.000027] plane 0: bytesperline=0 sizeimage=2097152
[ +0.000527] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
height=192, format=NV12, field=none, colorspace=0, num_planes=1,
flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
[ +0.000024] plane 0: bytesperline=384 sizeimage=122880
[ +0.000032] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
height=192, format=NV12, field=none, colorspace=0, num_planes=1,
flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
[ +0.000017] plane 0: bytesperline=384 sizeimage=122880
[ +0.000024] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
height=192, format=NV12, field=none, colorspace=0, num_planes=1,
flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
[ +0.000017] plane 0: bytesperline=384 sizeimage=122880
[ +0.000075] video2: VIDIOC_S_SELECTION: type=vid-out, target=0,
flags=0x0, wxh=320x192, x,y=0,0
[ +0.028300] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
height=192, format=NV12, field=none, colorspace=0, num_planes=1,
flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
[ +0.000036] plane 0: bytesperline=384 sizeimage=122880
[ +0.000033] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
height=192, format=NV12, field=none, colorspace=0, num_planes=1,
flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
[ +0.000014] plane 0: bytesperline=384 sizeimage=122880
[ +0.000020] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
height=192, format=NV12, field=none, colorspace=0, num_planes=1,
flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
[ +0.000120] plane 0: bytesperline=384 sizeimage=122880
[ +0.000157] video2: VIDIOC_S_SELECTION: type=vid-out, target=0,
flags=0x0, wxh=320x192, x,y=0,0
[ +0.000250] video2: VIDIOC_S_EXT_CTRLS: which=0x990000, count=1,
error_idx=0, request_fd=0, id/val=0x9909d7/0x1
[ +0.000120] video2: VIDIOC_QUERYCTRL: error -22: id=0x990b84,
type=0, name=, min/max=0/0, step=0, default=0, flags=0x00000000
[ +0.000368] v4l2-ctrls: try_set_ext_ctrls: video2: video2:
try_set_ext_ctrls_common failed (-22)
[ +0.000087] video2: VIDIOC_S_EXT_CTRLS: error -22: which=0x990000,
count=5, error_idx=5, request_fd=0, id/val=0x9909ca/0x0,
id/val=0x990a62/0x33, id/val=0x990a6b/0x2, id/val=0x990a67/0xb,
id/val=0x9909d8/0x1
[ +0.000290] v4l2-ctrls: prepare_ext_ctrls: video2: cannot find
control id 0x9909da
[ +0.000010] v4l2-ctrls: try_set_ext_ctrls: video2: video2:
try_set_ext_ctrls_common failed (-22)
[ +0.000014] video2: VIDIOC_S_EXT_CTRLS: error -22: which=0x990000,
count=2, error_idx=2, request_fd=0, id/val=0x9909da/0x1,
id/val=0x9909cb/0x0
[ +0.000225] video2: VIDIOC_G_FMT: type=vid-cap-mplane, width=320,
height=192, format=H264, field=none, colorspace=0, num_planes=1,
flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
[ +0.000028] plane 0: bytesperline=0 sizeimage=49152
[ +0.002272] video2: VIDIOC_REQBUFS: count=4, type=vid-cap-mplane, memory=mmap
[ +0.001661] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=0,
type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
sequence=0, memory=mmap
[ +0.000034] plane 0: bytesused=0, data_offset=0x00000000,
offset/userptr=0xee18ad4840000000, length=2097152
[ +0.000009] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
userbits=0x00000000
[ +0.000142] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=1,
type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
sequence=0, memory=mmap
[ +0.000023] plane 0: bytesused=0, data_offset=0x00000000,
offset/userptr=0xee18ad4840200000, length=2097152
[ +0.000008] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
userbits=0x00000000
[ +0.000409] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=2,
type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
sequence=0, memory=mmap
[ +0.000027] plane 0: bytesused=0, data_offset=0x00000000,
offset/userptr=0xee18ad4840400000, length=2097152
[ +0.000007] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
userbits=0x00000000
[ +0.000233] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=3,
type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
sequence=0, memory=mmap
[ +0.000023] plane 0: bytesused=0, data_offset=0x00000000,
offset/userptr=0xee18ad4840600000, length=2097152
[ +0.000015] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
userbits=0x00000000
[ +0.000288] video2: VIDIOC_S_EXT_CTRLS: which=0x990000, count=1,
error_idx=0, request_fd=0, id/val=0x9909cf/0x30d40
[ +0.000184] video2: VIDIOC_S_PARM: type=vid-out-mplane,
capability=0x1000, outputmode=0x0, timeperframe=1/30, extendedmode=0,
writebuffers=0
[ +0.310832] qcom-venus-encoder aa00000.video-codec:video-encoder:
VenusHigh: wrong state!
> + INIT_LIST_HEAD(&inst->registeredbufs);
> +
> + inst->enc_state = VENUS_ENC_STATE_INIT;
> +
> + 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;
> @@ -881,61 +945,73 @@ 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);
> - int ret;
> + int ret = 0;
>
> mutex_lock(&inst->lock);
>
> - if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> + 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))
> + goto unlock;
>
> - venus_helper_init_instance(inst);
> + if (inst->enc_state == VENUS_ENC_STATE_INIT) {
> + 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_set_properties(inst);
> - if (ret)
> - goto deinit_sess;
> + ret = venc_verify_conf(inst);
> + if (ret)
> + goto deinit_sess;
>
> - ret = venc_verify_conf(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 = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
> - inst->num_output_bufs, 0);
> - if (ret)
> - goto deinit_sess;
> + ret = venus_helper_vb2_start_streaming(inst);
> + if (ret)
> + goto deinit_sess;
>
> - 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);
>
> - inst->enc_state = VENUS_ENC_STATE_ENCODING;
> + inst->enc_state = VENUS_ENC_STATE_ENCODING;
> + } else if (inst->enc_state == VENUS_ENC_STATE_RESET &&
> + V4L2_TYPE_IS_CAPTURE(q->type)) {
> + ret = venus_helper_vb2_start_streaming(inst);
> + if (ret)
> + goto bufs_done;
>
> - mutex_unlock(&inst->lock);
> + venus_helper_process_initial_out_bufs(inst);
> + venus_helper_process_initial_cap_bufs(inst);
>
> - return 0;
> + inst->enc_state = VENUS_ENC_STATE_ENCODING;
> + } else if (inst->enc_state == VENUS_ENC_STATE_STOPPED &&
> + V4L2_TYPE_IS_OUTPUT(q->type)) {
> + inst->enc_state = VENUS_ENC_STATE_ENCODING;
> + }
> +
> +unlock:
> + mutex_unlock(&inst->lock);
> + return ret;
>
> deinit_sess:
> 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;
> @@ -943,33 +1019,97 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
> return ret;
> }
>
> -static void venc_vb2_buf_queue(struct vb2_buffer *vb)
> +static int venc_stop_capture(struct venus_inst *inst)
> +{
> + int ret;
> +
> + switch (inst->enc_state) {
> + case VENUS_ENC_STATE_ENCODING:
> + case VENUS_ENC_STATE_DRAIN:
> + case VENUS_ENC_STATE_STOPPED:
> + inst->enc_state = VENUS_ENC_STATE_RESET;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + ret = hfi_session_stop(inst);
> + ret |= hfi_session_unload_res(inst);
> + ret |= venus_helper_unregister_bufs(inst);
> + ret |= venus_helper_intbufs_free(inst);
> +
> + return ret;
> +}
> +
> +static int venc_stop_output(struct venus_inst *inst)
> +{
> + switch (inst->enc_state) {
> + case VENUS_ENC_STATE_ENCODING:
> + inst->enc_state = VENUS_ENC_STATE_STOPPED;
> + break;
> + case VENUS_ENC_STATE_DRAIN:
> + inst->enc_state = VENUS_ENC_STATE_STOPPED;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static void venc_stop_streaming(struct vb2_queue *q)
> +{
> + struct venus_inst *inst = vb2_get_drv_priv(q);
> + int ret = -EINVAL;
> +
> + mutex_lock(&inst->lock);
> +
> + if (V4L2_TYPE_IS_OUTPUT(q->type))
> + ret = venc_stop_output(inst);
> + else
> + ret = venc_stop_capture(inst);
> +
> + venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
> +
> + if (ret)
> + goto unlock;
> +
> + if (V4L2_TYPE_IS_OUTPUT(q->type))
> + inst->streamon_out = 0;
> + else
> + inst->streamon_cap = 0;
> +
> +unlock:
> + 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);
> + struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
>
> 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;
> - }
> + v4l2_m2m_buf_queue(m2m_ctx, vbuf);
> +
> + if (!(inst->streamon_out && inst->streamon_cap))
> + goto unlock;
> +
> + venus_helper_process_buf(vb);
>
> - venus_helper_vb2_buf_queue(vb);
> +unlock:
> mutex_unlock(&inst->lock);
> }
>
> 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,
> @@ -1025,8 +1165,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,
> };
>
> @@ -1098,11 +1242,9 @@ static int venc_open(struct file *file)
> inst->core = core;
> inst->session_type = VIDC_SESSION_TYPE_ENC;
> inst->clk_data.core_id = VIDC_CORE_ID_DEFAULT;
> + inst->enc_state = VENUS_ENC_STATE_INIT;
> 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);
> @@ -1167,7 +1309,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);
> --
> 2.17.1
>
Hi Stan,
On 2020-11-03 06:46, Fritz Koenig wrote:
> On Fri, Oct 23, 2020 at 5:57 AM Stanimir Varbanov
> <[email protected]> wrote:
>>
>> Redesign the encoder driver to be compliant with stateful encoder
>> spec - specifically adds handling of Reset state.
>>
>> Signed-off-by: Stanimir Varbanov <[email protected]>
>> ---
>> drivers/media/platform/qcom/venus/core.h | 10 +-
>> drivers/media/platform/qcom/venus/venc.c | 242
>> ++++++++++++++++++-----
>> 2 files changed, 197 insertions(+), 55 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/core.h
>> b/drivers/media/platform/qcom/venus/core.h
>> index 5c4693678e3f..294d5467a6a1 100644
>> --- a/drivers/media/platform/qcom/venus/core.h
>> +++ b/drivers/media/platform/qcom/venus/core.h
>> @@ -277,11 +277,11 @@ enum venus_dec_state {
>> };
>>
>> 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,
>> + VENUS_ENC_STATE_INIT = 0,
>> + VENUS_ENC_STATE_ENCODING = 1,
>> + VENUS_ENC_STATE_STOPPED = 2,
>> + VENUS_ENC_STATE_DRAIN = 3,
>> + VENUS_ENC_STATE_RESET = 4,
>> };
>>
>> struct venus_ts_metadata {
>> diff --git a/drivers/media/platform/qcom/venus/venc.c
>> b/drivers/media/platform/qcom/venus/venc.c
>> index c6143b07914c..aa9255ddb0a5 100644
>> --- a/drivers/media/platform/qcom/venus/venc.c
>> +++ b/drivers/media/platform/qcom/venus/venc.c
>> @@ -565,6 +565,7 @@ static const struct v4l2_ioctl_ops venc_ioctl_ops
>> = {
>> .vidioc_enum_frameintervals = venc_enum_frameintervals,
>> .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
>> .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
>> + .vidioc_try_encoder_cmd = v4l2_m2m_ioctl_try_encoder_cmd,
>> .vidioc_encoder_cmd = venc_encoder_cmd,
>> };
>>
>> @@ -850,6 +851,69 @@ static int venc_queue_setup(struct vb2_queue *q,
>> return ret;
>> }
>>
>> +static void venc_release_session(struct venus_inst *inst)
>> +{
>> + struct venus_core *core = inst->core;
>> + int ret, abort = 0;
>> +
>> + mutex_lock(&inst->lock);
>> +
>> + if (inst->enc_state != VENUS_ENC_STATE_RESET)
>> + dev_dbg(core->dev_enc, VDBGH "wrong state!\n");
>> +
>> + ret = hfi_session_stop(inst);
>> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
>> + ret = hfi_session_unload_res(inst);
>> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
>> + ret = venus_helper_unregister_bufs(inst);
>> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
>> + ret = venus_helper_intbufs_free(inst);
>> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
>> + 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);
>
> venus_pm_load_scale depends on inst->clk_data.codec_freq_data being
> set up. This occurs in venc_init_session(). I am seeing scenarios
> where the encoder is getting setup up, but before it is finished,
> teardown occurs. If this teardown occurs before
> inst->clk_data.codec_freq_data is initalized, a crash occurs. (also
> "wrong state!" is printed out)
venus_pm_load_scale(inst) is called here to remove the votes for clock
and bus bandwidth. This is not needed for instances which are closed
just after moving them to INIT state. I have tried with below state
handling
and it works well i.e call for venus_pm_load_scale(inst) from state
other
than the INIT state.
- venus_pm_load_scale(inst);
+ if(inst->enc_state != VENUS_ENC_STATE_INIT)
+ venus_pm_load_scale(inst);
>
> [ 106.593198] Unable to handle kernel NULL pointer dereference at
> virtual address 0000000000000008
> [ 106.603916] Mem abort info:
> [ 106.608470] ESR = 0x96000006
> [ 106.613802] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 106.619426] SET = 0, FnV = 0
> [ 106.622619] EA = 0, S1PTW = 0
> [ 106.625862] Data abort info:
> [ 106.628835] ISV = 0, ISS = 0x00000006
> [ 106.632785] CM = 0, WnR = 0
> [ 106.635850] user pgtable: 4k pages, 39-bit VAs,
> pgdp=000000014839f000
> [ 106.642472] [0000000000000008] pgd=000000016ab1f003,
> pud=000000016ab1f003, pmd=0000000000000000
> [ 106.651410] Internal error: Oops: 96000006 [#1] PREEMPT SMP
> [ 106.657132] Modules linked in: rfcomm algif_hash algif_skcipher
> af_alg uinput venus_dec venus_enc videobuf2_dma_sg hci_uart btqca
> venus_core qcom_spmi_adc5 qcom_spmi_temp_alarm qcom_vadc_common
> snd_soc_rt5682_i2c v4l2_mem2mem snd_soc_sc7180 snd_soc_rt5682
> snd_soc_qcom_common snd_soc_rl6231 bluetooth ecdh_generic ecc
> snd_soc_lpass_sc7180 snd_soc_lpass_hdmi snd_soc_lpass_cpu
> snd_soc_lpass_platform snd_soc_max98357a xt_MASQUERADE fuse
> iio_trig_sysfs cros_ec_lid_angle cros_ec_sensors cros_ec_sensors_core
> industrialio_triggered_buffer cros_ec_sensors_ring rmtfs_mem kfifo_buf
> cros_ec_sensorhub ath10k_snoc lzo_rle ath10k_core lzo_compress ath
> zram mac80211 ipa qmi_helpers cfg80211 qcom_q6v5_mss qcom_pil_info
> qcom_q6v5 qcom_common cdc_ether usbnet r8152 mii uvcvideo
> videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_common
> joydev
> [ 106.732576] CPU: 7 PID: 3622 Comm: DrmThread Not tainted 5.4.72 #40
> [ 106.739004] Hardware name: Google Lazor (rev1+) (DT)
> [ 106.744103] pstate: 60400009 (nZCv daif +PAN -UAO)
> [ 106.749027] pc : load_scale_v4+0x160/0x3a4 [venus_core]
> [ 106.754396] lr : load_scale_v4+0x154/0x3a4 [venus_core]
> [ 106.759766] sp : ffffffc0120ab9e0
> [ 106.763171] x29: ffffffc0120ab9e0 x28: 0000000000000005
> [ 106.768629] x27: 0000000000000000 x26: 0000000000000000
> [ 106.774080] x25: 0000000000000000 x24: 000000000000001e
> [ 106.779530] x23: 0000000000000000 x22: ffffff80b41a8000
> [ 106.784980] x21: ffffffd344e97e98 x20: ffffff80cb5b8080
> [ 106.790431] x19: ffffff80fa3b1410 x18: 00000000ffff0a10
> [ 106.795881] x17: 000000000000003c x16: ffffffd398ec2e88
> [ 106.801329] x15: 0000000000000006 x14: ffff001000000600
> [ 106.806779] x13: 000000000002cca2 x12: 0000000000000000
> [ 106.812229] x11: 0000000000000000 x10: 0000000000000000
> [ 106.817679] x9 : 472cbd12793c4600 x8 : 0000000000000000
> [ 106.823137] x7 : 0000000000000000 x6 : ffffffd399dbcc6c
> [ 106.828585] x5 : 0000000000000000 x4 : 0000000000000000
> [ 106.834035] x3 : 0000000000000000 x2 : ffffff80ff6ae5c0
> [ 106.839487] x1 : ffffff80ff69e018 x0 : ffffffd344e990ac
> [ 106.844937] Call trace:
> [ 106.847460] load_scale_v4+0x160/0x3a4 [venus_core]
> [ 106.852473] venc_buf_cleanup+0x198/0x1f0 [venus_enc]
> [ 106.857656] __vb2_queue_free+0x90/0x1f4 [videobuf2_common]
> [ 106.863374] vb2_core_queue_release+0x3c/0x50 [videobuf2_common]
> [ 106.869541] vb2_queue_release+0x1c/0x28 [videobuf2_v4l2]
> [ 106.875082] v4l2_m2m_ctx_release+0x24/0x40 [v4l2_mem2mem]
> [ 106.880710] venc_close+0x24/0x78 [venus_enc]
> [ 106.885188] v4l2_release+0x8c/0xdc
> [ 106.888779] __fput+0xe0/0x214
> [ 106.891916] ____fput+0x1c/0x28
> [ 106.895148] task_work_run+0x94/0xc4
> [ 106.898828] do_exit+0x244/0x7c8
> [ 106.902147] do_group_exit+0x88/0x98
> [ 106.905825] get_signal+0x1cc/0x674
> [ 106.909415] do_notify_resume+0x134/0x1410
> [ 106.913619] work_pending+0x8/0x10
> [ 106.917119] Code: 97ffd58d f94032c8 90000020 9102b000 (f9400501)
> [ 106.923377] ---[ end trace a9caaf72c228e386 ]---
> [ 106.928767] Kernel panic - not syncing: Fatal exception
> [ 106.934131] SMP: stopping secondary CPUs
> [ 106.938347] Kernel Offset: 0x1388a00000 from 0xffffffc010000000
> [ 106.944426] PHYS_OFFSET: 0xffffffd900000000
> [ 106.948722] CPU features: 0x08102e,2a80aa18
> [ 106.953015] Memory Limit: none
>
>
> This is the debug log before the crash:
> [Nov 2 15:33] qcom-venus aa00000.video-codec: VenusLow : venus hw
> version 4.44.20a
> [ +0.000065] videodev: v4l2_open: video2: open (0)
> [ +0.000019] video2: VIDIOC_ENUM_FMT: index=0, type=vid-cap-mplane,
> flags=0x1, pixelformat=H264, description='H.264'
> [ +0.000017] video2: VIDIOC_ENUM_FMT: index=1, type=vid-cap-mplane,
> flags=0x1, pixelformat=VP80, description='VP8'
> [ +0.000042] video2: VIDIOC_ENUM_FMT: index=2, type=vid-cap-mplane,
> flags=0x1, pixelformat=HEVC, description='HEVC'
> [ +0.000030] video2: VIDIOC_ENUM_FMT: error -22: index=3,
> type=vid-cap-mplane, flags=0x0, pixelformat=\x00\x00\x00\x00,
> description=''
> [ +0.000068] videodev: v4l2_release: video2: release
> [ +0.002752] qcom-venus aa00000.video-codec: VenusLow : venus hw
> version 4.44.20a
> [ +0.000062] videodev: v4l2_open: video2: open (0)
> [ +0.000071] video2: VIDIOC_ENUM_FRAMESIZES: index=0,
> pixelformat=H264, type=3, min=96x96, max=4096x4096, step=16x16
> [ +0.000012] video2: VIDIOC_TRY_ENCODER_CMD: cmd=1, flags=0x0
> [ +0.000005] video2: VIDIOC_QUERYCAP: driver=qcom-venus,
> card=Qualcomm Venus video encoder, bus=platform:qcom-venus,
> version=0x00050448, capabilities=0x84204000, device_caps=0x04204000
> [ +0.001055] video2: VIDIOC_REQBUFS: count=0, type=vid-out-mplane,
> memory=mmap
> [ +0.000382] video2: VIDIOC_REQBUFS: count=0, type=vid-cap-mplane,
> memory=mmap
> [ +0.000227] video2: VIDIOC_S_FMT: type=vid-cap-mplane, width=96,
> height=96, format=H264, field=none, colorspace=0, num_planes=1,
> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> [ +0.000027] plane 0: bytesperline=0 sizeimage=2097152
> [ +0.000527] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> [ +0.000024] plane 0: bytesperline=384 sizeimage=122880
> [ +0.000032] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> [ +0.000017] plane 0: bytesperline=384 sizeimage=122880
> [ +0.000024] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> [ +0.000017] plane 0: bytesperline=384 sizeimage=122880
> [ +0.000075] video2: VIDIOC_S_SELECTION: type=vid-out, target=0,
> flags=0x0, wxh=320x192, x,y=0,0
> [ +0.028300] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> [ +0.000036] plane 0: bytesperline=384 sizeimage=122880
> [ +0.000033] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> [ +0.000014] plane 0: bytesperline=384 sizeimage=122880
> [ +0.000020] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> [ +0.000120] plane 0: bytesperline=384 sizeimage=122880
> [ +0.000157] video2: VIDIOC_S_SELECTION: type=vid-out, target=0,
> flags=0x0, wxh=320x192, x,y=0,0
> [ +0.000250] video2: VIDIOC_S_EXT_CTRLS: which=0x990000, count=1,
> error_idx=0, request_fd=0, id/val=0x9909d7/0x1
> [ +0.000120] video2: VIDIOC_QUERYCTRL: error -22: id=0x990b84,
> type=0, name=, min/max=0/0, step=0, default=0, flags=0x00000000
> [ +0.000368] v4l2-ctrls: try_set_ext_ctrls: video2: video2:
> try_set_ext_ctrls_common failed (-22)
> [ +0.000087] video2: VIDIOC_S_EXT_CTRLS: error -22: which=0x990000,
> count=5, error_idx=5, request_fd=0, id/val=0x9909ca/0x0,
> id/val=0x990a62/0x33, id/val=0x990a6b/0x2, id/val=0x990a67/0xb,
> id/val=0x9909d8/0x1
> [ +0.000290] v4l2-ctrls: prepare_ext_ctrls: video2: cannot find
> control id 0x9909da
> [ +0.000010] v4l2-ctrls: try_set_ext_ctrls: video2: video2:
> try_set_ext_ctrls_common failed (-22)
> [ +0.000014] video2: VIDIOC_S_EXT_CTRLS: error -22: which=0x990000,
> count=2, error_idx=2, request_fd=0, id/val=0x9909da/0x1,
> id/val=0x9909cb/0x0
> [ +0.000225] video2: VIDIOC_G_FMT: type=vid-cap-mplane, width=320,
> height=192, format=H264, field=none, colorspace=0, num_planes=1,
> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> [ +0.000028] plane 0: bytesperline=0 sizeimage=49152
> [ +0.002272] video2: VIDIOC_REQBUFS: count=4, type=vid-cap-mplane,
> memory=mmap
> [ +0.001661] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=0,
> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
> sequence=0, memory=mmap
> [ +0.000034] plane 0: bytesused=0, data_offset=0x00000000,
> offset/userptr=0xee18ad4840000000, length=2097152
> [ +0.000009] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
> userbits=0x00000000
> [ +0.000142] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=1,
> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
> sequence=0, memory=mmap
> [ +0.000023] plane 0: bytesused=0, data_offset=0x00000000,
> offset/userptr=0xee18ad4840200000, length=2097152
> [ +0.000008] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
> userbits=0x00000000
> [ +0.000409] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=2,
> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
> sequence=0, memory=mmap
> [ +0.000027] plane 0: bytesused=0, data_offset=0x00000000,
> offset/userptr=0xee18ad4840400000, length=2097152
> [ +0.000007] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
> userbits=0x00000000
> [ +0.000233] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=3,
> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
> sequence=0, memory=mmap
> [ +0.000023] plane 0: bytesused=0, data_offset=0x00000000,
> offset/userptr=0xee18ad4840600000, length=2097152
> [ +0.000015] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
> userbits=0x00000000
> [ +0.000288] video2: VIDIOC_S_EXT_CTRLS: which=0x990000, count=1,
> error_idx=0, request_fd=0, id/val=0x9909cf/0x30d40
> [ +0.000184] video2: VIDIOC_S_PARM: type=vid-out-mplane,
> capability=0x1000, outputmode=0x0, timeperframe=1/30, extendedmode=0,
> writebuffers=0
> [ +0.310832] qcom-venus-encoder aa00000.video-codec:video-encoder:
> VenusHigh: wrong state!
>
>> + INIT_LIST_HEAD(&inst->registeredbufs);
>> +
>> + inst->enc_state = VENUS_ENC_STATE_INIT;
>> +
>> + 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;
>> @@ -881,61 +945,73 @@ 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);
>> - int ret;
>> + int ret = 0;
>>
>> mutex_lock(&inst->lock);
>>
>> - if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
>> + 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))
>> + goto unlock;
>>
>> - venus_helper_init_instance(inst);
>> + if (inst->enc_state == VENUS_ENC_STATE_INIT) {
>> + 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_set_properties(inst);
>> - if (ret)
>> - goto deinit_sess;
>> + ret = venc_verify_conf(inst);
>> + if (ret)
>> + goto deinit_sess;
>>
>> - ret = venc_verify_conf(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 = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
>> - inst->num_output_bufs, 0);
>> - if (ret)
>> - goto deinit_sess;
>> + ret = venus_helper_vb2_start_streaming(inst);
>> + if (ret)
>> + goto deinit_sess;
>>
>> - 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);
>>
>> - inst->enc_state = VENUS_ENC_STATE_ENCODING;
>> + inst->enc_state = VENUS_ENC_STATE_ENCODING;
>> + } else if (inst->enc_state == VENUS_ENC_STATE_RESET &&
>> + V4L2_TYPE_IS_CAPTURE(q->type)) {
>> + ret = venus_helper_vb2_start_streaming(inst);
>> + if (ret)
>> + goto bufs_done;
>>
>> - mutex_unlock(&inst->lock);
>> + venus_helper_process_initial_out_bufs(inst);
>> + venus_helper_process_initial_cap_bufs(inst);
>>
>> - return 0;
>> + inst->enc_state = VENUS_ENC_STATE_ENCODING;
>> + } else if (inst->enc_state == VENUS_ENC_STATE_STOPPED &&
>> + V4L2_TYPE_IS_OUTPUT(q->type)) {
>> + inst->enc_state = VENUS_ENC_STATE_ENCODING;
>> + }
>> +
>> +unlock:
>> + mutex_unlock(&inst->lock);
>> + return ret;
>>
>> deinit_sess:
>> 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;
>> @@ -943,33 +1019,97 @@ static int venc_start_streaming(struct
>> vb2_queue *q, unsigned int count)
>> return ret;
>> }
>>
>> -static void venc_vb2_buf_queue(struct vb2_buffer *vb)
>> +static int venc_stop_capture(struct venus_inst *inst)
>> +{
>> + int ret;
>> +
>> + switch (inst->enc_state) {
>> + case VENUS_ENC_STATE_ENCODING:
>> + case VENUS_ENC_STATE_DRAIN:
>> + case VENUS_ENC_STATE_STOPPED:
>> + inst->enc_state = VENUS_ENC_STATE_RESET;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + ret = hfi_session_stop(inst);
>> + ret |= hfi_session_unload_res(inst);
>> + ret |= venus_helper_unregister_bufs(inst);
>> + ret |= venus_helper_intbufs_free(inst);
>> +
>> + return ret;
>> +}
>> +
>> +static int venc_stop_output(struct venus_inst *inst)
>> +{
>> + switch (inst->enc_state) {
>> + case VENUS_ENC_STATE_ENCODING:
>> + inst->enc_state = VENUS_ENC_STATE_STOPPED;
>> + break;
>> + case VENUS_ENC_STATE_DRAIN:
>> + inst->enc_state = VENUS_ENC_STATE_STOPPED;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void venc_stop_streaming(struct vb2_queue *q)
>> +{
>> + struct venus_inst *inst = vb2_get_drv_priv(q);
>> + int ret = -EINVAL;
>> +
>> + mutex_lock(&inst->lock);
>> +
>> + if (V4L2_TYPE_IS_OUTPUT(q->type))
>> + ret = venc_stop_output(inst);
>> + else
>> + ret = venc_stop_capture(inst);
>> +
>> + venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
>> +
>> + if (ret)
>> + goto unlock;
>> +
>> + if (V4L2_TYPE_IS_OUTPUT(q->type))
>> + inst->streamon_out = 0;
>> + else
>> + inst->streamon_cap = 0;
>> +
>> +unlock:
>> + 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);
>> + struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
>>
>> 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;
>> - }
>> + v4l2_m2m_buf_queue(m2m_ctx, vbuf);
>> +
>> + if (!(inst->streamon_out && inst->streamon_cap))
>> + goto unlock;
>> +
>> + venus_helper_process_buf(vb);
>>
>> - venus_helper_vb2_buf_queue(vb);
>> +unlock:
>> mutex_unlock(&inst->lock);
>> }
>>
>> 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,
>> @@ -1025,8 +1165,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,
>> };
>>
>> @@ -1098,11 +1242,9 @@ static int venc_open(struct file *file)
>> inst->core = core;
>> inst->session_type = VIDC_SESSION_TYPE_ENC;
>> inst->clk_data.core_id = VIDC_CORE_ID_DEFAULT;
>> + inst->enc_state = VENUS_ENC_STATE_INIT;
>> 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);
>> @@ -1167,7 +1309,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);
>> --
>> 2.17.1
>>
Thanks,
Vikash
On 11/4/20 12:44 PM, [email protected] wrote:
> Hi Stan,
>
> On 2020-11-03 06:46, Fritz Koenig wrote:
>> On Fri, Oct 23, 2020 at 5:57 AM Stanimir Varbanov
>> <[email protected]> wrote:
>>>
>>> Redesign the encoder driver to be compliant with stateful encoder
>>> spec - specifically adds handling of Reset state.
>>>
>>> Signed-off-by: Stanimir Varbanov <[email protected]>
>>> ---
>>> Â drivers/media/platform/qcom/venus/core.h |Â 10 +-
>>> Â drivers/media/platform/qcom/venus/venc.c | 242 ++++++++++++++++++-----
>>> Â 2 files changed, 197 insertions(+), 55 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/core.h
>>> b/drivers/media/platform/qcom/venus/core.h
>>> index 5c4693678e3f..294d5467a6a1 100644
>>> --- a/drivers/media/platform/qcom/venus/core.h
>>> +++ b/drivers/media/platform/qcom/venus/core.h
>>> @@ -277,11 +277,11 @@ enum venus_dec_state {
>>> Â };
>>>
>>> Â 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,
>>> +Â Â Â Â Â Â VENUS_ENC_STATE_INITÂ Â Â Â Â Â Â Â Â Â Â = 0,
>>> +Â Â Â Â Â Â VENUS_ENC_STATE_ENCODINGÂ Â Â Â Â Â Â = 1,
>>> +Â Â Â Â Â Â VENUS_ENC_STATE_STOPPEDÂ Â Â Â Â Â Â Â = 2,
>>> +Â Â Â Â Â Â VENUS_ENC_STATE_DRAINÂ Â Â Â Â Â Â Â Â Â = 3,
>>> +Â Â Â Â Â Â VENUS_ENC_STATE_RESETÂ Â Â Â Â Â Â Â Â Â = 4,
>>> Â };
>>>
>>> Â struct venus_ts_metadata {
>>> diff --git a/drivers/media/platform/qcom/venus/venc.c
>>> b/drivers/media/platform/qcom/venus/venc.c
>>> index c6143b07914c..aa9255ddb0a5 100644
>>> --- a/drivers/media/platform/qcom/venus/venc.c
>>> +++ b/drivers/media/platform/qcom/venus/venc.c
>>> @@ -565,6 +565,7 @@ static const struct v4l2_ioctl_ops venc_ioctl_ops
>>> = {
>>> Â Â Â Â Â Â Â .vidioc_enum_frameintervals = venc_enum_frameintervals,
>>> Â Â Â Â Â Â Â .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
>>> Â Â Â Â Â Â Â .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
>>> +Â Â Â Â Â Â .vidioc_try_encoder_cmd = v4l2_m2m_ioctl_try_encoder_cmd,
>>> Â Â Â Â Â Â Â .vidioc_encoder_cmd = venc_encoder_cmd,
>>> Â };
>>>
>>> @@ -850,6 +851,69 @@ static int venc_queue_setup(struct vb2_queue *q,
>>> Â Â Â Â Â Â Â return ret;
>>> Â }
>>>
>>> +static void venc_release_session(struct venus_inst *inst)
>>> +{
>>> +Â Â Â Â Â Â struct venus_core *core = inst->core;
>>> +Â Â Â Â Â Â int ret, abort = 0;
>>> +
>>> +Â Â Â Â Â Â mutex_lock(&inst->lock);
>>> +
>>> +Â Â Â Â Â Â if (inst->enc_state != VENUS_ENC_STATE_RESET)
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â dev_dbg(core->dev_enc, VDBGH "wrong state!\n");
>>> +
>>> +Â Â Â Â Â Â ret = hfi_session_stop(inst);
>>> +Â Â Â Â Â Â abort |= (ret && ret != -EINVAL) ? 1 : 0;
>>> +Â Â Â Â Â Â ret = hfi_session_unload_res(inst);
>>> +Â Â Â Â Â Â abort |= (ret && ret != -EINVAL) ? 1 : 0;
>>> +Â Â Â Â Â Â ret = venus_helper_unregister_bufs(inst);
>>> +Â Â Â Â Â Â abort |= (ret && ret != -EINVAL) ? 1 : 0;
>>> +Â Â Â Â Â Â ret = venus_helper_intbufs_free(inst);
>>> +Â Â Â Â Â Â abort |= (ret && ret != -EINVAL) ? 1 : 0;
>>> +Â Â Â Â Â Â 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);
>>
>> venus_pm_load_scale depends on inst->clk_data.codec_freq_data being
>> set up. This occurs in venc_init_session(). I am seeing scenarios
>> where the encoder is getting setup up, but before it is finished,
>> teardown occurs. If this teardown occurs before
>> inst->clk_data.codec_freq_data is initalized, a crash occurs. (also
>> "wrong state!" is printed out)
>
> venus_pm_load_scale(inst) is called here to remove the votes for clock
> and bus bandwidth. This is not needed for instances which are closed
> just after moving them to INIT state. I have tried with below state
> handling
> and it works well i.e call for venus_pm_load_scale(inst) from state other
> than the INIT state.
IMO we have to check the INST state in venus_pm_load_scale(). We made
similar check in min_loaded_core() here [1].
[1] https://www.spinics.net/lists/kernel/msg3498215.html
>
> -Â Â Â Â Â Â venus_pm_load_scale(inst);
> +Â Â Â Â Â Â if(inst->enc_state != VENUS_ENC_STATE_INIT)
> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â venus_pm_load_scale(inst);
>
>>
>> [Â 106.593198] Unable to handle kernel NULL pointer dereference at
>> virtual address 0000000000000008
>> [Â 106.603916] Mem abort info:
>> [Â 106.608470]Â Â ESR = 0x96000006
>> [Â 106.613802]Â Â EC = 0x25: DABT (current EL), IL = 32 bits
>> [Â 106.619426]Â Â SET = 0, FnV = 0
>> [Â 106.622619]Â Â EA = 0, S1PTW = 0
>> [Â 106.625862] Data abort info:
>> [Â 106.628835]Â Â ISV = 0, ISS = 0x00000006
>> [Â 106.632785]Â Â CM = 0, WnR = 0
>> [Â 106.635850] user pgtable: 4k pages, 39-bit VAs, pgdp=000000014839f000
>> [Â 106.642472] [0000000000000008] pgd=000000016ab1f003,
>> pud=000000016ab1f003, pmd=0000000000000000
>> [Â 106.651410] Internal error: Oops: 96000006 [#1] PREEMPT SMP
>> [Â 106.657132] Modules linked in: rfcomm algif_hash algif_skcipher
>> af_alg uinput venus_dec venus_enc videobuf2_dma_sg hci_uart btqca
>> venus_core qcom_spmi_adc5 qcom_spmi_temp_alarm qcom_vadc_common
>> snd_soc_rt5682_i2c v4l2_mem2mem snd_soc_sc7180 snd_soc_rt5682
>> snd_soc_qcom_common snd_soc_rl6231 bluetooth ecdh_generic ecc
>> snd_soc_lpass_sc7180 snd_soc_lpass_hdmi snd_soc_lpass_cpu
>> snd_soc_lpass_platform snd_soc_max98357a xt_MASQUERADE fuse
>> iio_trig_sysfs cros_ec_lid_angle cros_ec_sensors cros_ec_sensors_core
>> industrialio_triggered_buffer cros_ec_sensors_ring rmtfs_mem kfifo_buf
>> cros_ec_sensorhub ath10k_snoc lzo_rle ath10k_core lzo_compress ath
>> zram mac80211 ipa qmi_helpers cfg80211 qcom_q6v5_mss qcom_pil_info
>> qcom_q6v5 qcom_common cdc_ether usbnet r8152 mii uvcvideo
>> videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_common
>> joydev
>> [Â 106.732576] CPU: 7 PID: 3622 Comm: DrmThread Not tainted 5.4.72 #40
>> [Â 106.739004] Hardware name: Google Lazor (rev1+) (DT)
>> [Â 106.744103] pstate: 60400009 (nZCv daif +PAN -UAO)
>> [Â 106.749027] pc : load_scale_v4+0x160/0x3a4 [venus_core]
>> [Â 106.754396] lr : load_scale_v4+0x154/0x3a4 [venus_core]
>> [Â 106.759766] sp : ffffffc0120ab9e0
>> [Â 106.763171] x29: ffffffc0120ab9e0 x28: 0000000000000005
>> [Â 106.768629] x27: 0000000000000000 x26: 0000000000000000
>> [Â 106.774080] x25: 0000000000000000 x24: 000000000000001e
>> [Â 106.779530] x23: 0000000000000000 x22: ffffff80b41a8000
>> [Â 106.784980] x21: ffffffd344e97e98 x20: ffffff80cb5b8080
>> [Â 106.790431] x19: ffffff80fa3b1410 x18: 00000000ffff0a10
>> [Â 106.795881] x17: 000000000000003c x16: ffffffd398ec2e88
>> [Â 106.801329] x15: 0000000000000006 x14: ffff001000000600
>> [Â 106.806779] x13: 000000000002cca2 x12: 0000000000000000
>> [Â 106.812229] x11: 0000000000000000 x10: 0000000000000000
>> [Â 106.817679] x9 : 472cbd12793c4600 x8 : 0000000000000000
>> [Â 106.823137] x7 : 0000000000000000 x6 : ffffffd399dbcc6c
>> [Â 106.828585] x5 : 0000000000000000 x4 : 0000000000000000
>> [Â 106.834035] x3 : 0000000000000000 x2 : ffffff80ff6ae5c0
>> [Â 106.839487] x1 : ffffff80ff69e018 x0 : ffffffd344e990ac
>> [Â 106.844937] Call trace:
>> [Â 106.847460]Â load_scale_v4+0x160/0x3a4 [venus_core]
>> [Â 106.852473]Â venc_buf_cleanup+0x198/0x1f0 [venus_enc]
>> [Â 106.857656]Â __vb2_queue_free+0x90/0x1f4 [videobuf2_common]
>> [Â 106.863374]Â vb2_core_queue_release+0x3c/0x50 [videobuf2_common]
>> [Â 106.869541]Â vb2_queue_release+0x1c/0x28 [videobuf2_v4l2]
>> [Â 106.875082]Â v4l2_m2m_ctx_release+0x24/0x40 [v4l2_mem2mem]
>> [Â 106.880710]Â venc_close+0x24/0x78 [venus_enc]
>> [Â 106.885188]Â v4l2_release+0x8c/0xdc
>> [Â 106.888779]Â __fput+0xe0/0x214
>> [Â 106.891916]Â ____fput+0x1c/0x28
>> [Â 106.895148]Â task_work_run+0x94/0xc4
>> [Â 106.898828]Â do_exit+0x244/0x7c8
>> [Â 106.902147]Â do_group_exit+0x88/0x98
>> [Â 106.905825]Â get_signal+0x1cc/0x674
>> [Â 106.909415]Â do_notify_resume+0x134/0x1410
>> [Â 106.913619]Â work_pending+0x8/0x10
>> [Â 106.917119] Code: 97ffd58d f94032c8 90000020 9102b000 (f9400501)
>> [Â 106.923377] ---[ end trace a9caaf72c228e386 ]---
>> [Â 106.928767] Kernel panic - not syncing: Fatal exception
>> [Â 106.934131] SMP: stopping secondary CPUs
>> [Â 106.938347] Kernel Offset: 0x1388a00000 from 0xffffffc010000000
>> [Â 106.944426] PHYS_OFFSET: 0xffffffd900000000
>> [Â 106.948722] CPU features: 0x08102e,2a80aa18
>> [Â 106.953015] Memory Limit: none
>>
>>
>> This is the debug log before the crash:
>> [Nov 2 15:33] qcom-venus aa00000.video-codec: VenusLow : venus hw
>> version 4.44.20a
>> [Â +0.000065] videodev: v4l2_open: video2: open (0)
>> [Â +0.000019] video2: VIDIOC_ENUM_FMT: index=0, type=vid-cap-mplane,
>> flags=0x1, pixelformat=H264, description='H.264'
>> [Â +0.000017] video2: VIDIOC_ENUM_FMT: index=1, type=vid-cap-mplane,
>> flags=0x1, pixelformat=VP80, description='VP8'
>> [Â +0.000042] video2: VIDIOC_ENUM_FMT: index=2, type=vid-cap-mplane,
>> flags=0x1, pixelformat=HEVC, description='HEVC'
>> [Â +0.000030] video2: VIDIOC_ENUM_FMT: error -22: index=3,
>> type=vid-cap-mplane, flags=0x0, pixelformat=\x00\x00\x00\x00,
>> description=''
>> [Â +0.000068] videodev: v4l2_release: video2: release
>> [Â +0.002752] qcom-venus aa00000.video-codec: VenusLow : venus hw
>> version 4.44.20a
>> [Â +0.000062] videodev: v4l2_open: video2: open (0)
>> [Â +0.000071] video2: VIDIOC_ENUM_FRAMESIZES: index=0,
>> pixelformat=H264, type=3, min=96x96, max=4096x4096, step=16x16
>> [Â +0.000012] video2: VIDIOC_TRY_ENCODER_CMD: cmd=1, flags=0x0
>> [Â +0.000005] video2: VIDIOC_QUERYCAP: driver=qcom-venus,
>> card=Qualcomm Venus video encoder, bus=platform:qcom-venus,
>> version=0x00050448, capabilities=0x84204000, device_caps=0x04204000
>> [Â +0.001055] video2: VIDIOC_REQBUFS: count=0, type=vid-out-mplane,
>> memory=mmap
>> [Â +0.000382] video2: VIDIOC_REQBUFS: count=0, type=vid-cap-mplane,
>> memory=mmap
>> [Â +0.000227] video2: VIDIOC_S_FMT: type=vid-cap-mplane, width=96,
>> height=96, format=H264, field=none, colorspace=0, num_planes=1,
>> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
>> [Â +0.000027] plane 0: bytesperline=0 sizeimage=2097152
>> [Â +0.000527] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
>> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
>> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
>> [Â +0.000024] plane 0: bytesperline=384 sizeimage=122880
>> [Â +0.000032] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
>> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
>> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
>> [Â +0.000017] plane 0: bytesperline=384 sizeimage=122880
>> [Â +0.000024] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
>> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
>> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
>> [Â +0.000017] plane 0: bytesperline=384 sizeimage=122880
>> [Â +0.000075] video2: VIDIOC_S_SELECTION: type=vid-out, target=0,
>> flags=0x0, wxh=320x192, x,y=0,0
>> [Â +0.028300] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
>> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
>> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
>> [Â +0.000036] plane 0: bytesperline=384 sizeimage=122880
>> [Â +0.000033] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
>> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
>> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
>> [Â +0.000014] plane 0: bytesperline=384 sizeimage=122880
>> [Â +0.000020] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
>> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
>> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
>> [Â +0.000120] plane 0: bytesperline=384 sizeimage=122880
>> [Â +0.000157] video2: VIDIOC_S_SELECTION: type=vid-out, target=0,
>> flags=0x0, wxh=320x192, x,y=0,0
>> [Â +0.000250] video2: VIDIOC_S_EXT_CTRLS: which=0x990000, count=1,
>> error_idx=0, request_fd=0, id/val=0x9909d7/0x1
>> [Â +0.000120] video2: VIDIOC_QUERYCTRL: error -22: id=0x990b84,
>> type=0, name=, min/max=0/0, step=0, default=0, flags=0x00000000
>> [Â +0.000368] v4l2-ctrls: try_set_ext_ctrls: video2: video2:
>> try_set_ext_ctrls_common failed (-22)
>> [Â +0.000087] video2: VIDIOC_S_EXT_CTRLS: error -22: which=0x990000,
>> count=5, error_idx=5, request_fd=0, id/val=0x9909ca/0x0,
>> id/val=0x990a62/0x33, id/val=0x990a6b/0x2, id/val=0x990a67/0xb,
>> id/val=0x9909d8/0x1
>> [Â +0.000290] v4l2-ctrls: prepare_ext_ctrls: video2: cannot find
>> control id 0x9909da
>> [Â +0.000010] v4l2-ctrls: try_set_ext_ctrls: video2: video2:
>> try_set_ext_ctrls_common failed (-22)
>> [Â +0.000014] video2: VIDIOC_S_EXT_CTRLS: error -22: which=0x990000,
>> count=2, error_idx=2, request_fd=0, id/val=0x9909da/0x1,
>> id/val=0x9909cb/0x0
>> [Â +0.000225] video2: VIDIOC_G_FMT: type=vid-cap-mplane, width=320,
>> height=192, format=H264, field=none, colorspace=0, num_planes=1,
>> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
>> [Â +0.000028] plane 0: bytesperline=0 sizeimage=49152
>> [Â +0.002272] video2: VIDIOC_REQBUFS: count=4, type=vid-cap-mplane,
>> memory=mmap
>> [Â +0.001661] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=0,
>> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
>> sequence=0, memory=mmap
>> [Â +0.000034] plane 0: bytesused=0, data_offset=0x00000000,
>> offset/userptr=0xee18ad4840000000, length=2097152
>> [Â +0.000009] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
>> userbits=0x00000000
>> [Â +0.000142] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=1,
>> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
>> sequence=0, memory=mmap
>> [Â +0.000023] plane 0: bytesused=0, data_offset=0x00000000,
>> offset/userptr=0xee18ad4840200000, length=2097152
>> [Â +0.000008] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
>> userbits=0x00000000
>> [Â +0.000409] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=2,
>> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
>> sequence=0, memory=mmap
>> [Â +0.000027] plane 0: bytesused=0, data_offset=0x00000000,
>> offset/userptr=0xee18ad4840400000, length=2097152
>> [Â +0.000007] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
>> userbits=0x00000000
>> [Â +0.000233] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=3,
>> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
>> sequence=0, memory=mmap
>> [Â +0.000023] plane 0: bytesused=0, data_offset=0x00000000,
>> offset/userptr=0xee18ad4840600000, length=2097152
>> [Â +0.000015] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
>> userbits=0x00000000
>> [Â +0.000288] video2: VIDIOC_S_EXT_CTRLS: which=0x990000, count=1,
>> error_idx=0, request_fd=0, id/val=0x9909cf/0x30d40
>> [Â +0.000184] video2: VIDIOC_S_PARM: type=vid-out-mplane,
>> capability=0x1000, outputmode=0x0, timeperframe=1/30, extendedmode=0,
>> writebuffers=0
>> [Â +0.310832] qcom-venus-encoder aa00000.video-codec:video-encoder:
>> VenusHigh: wrong state!
>>
>>> +Â Â Â Â Â Â INIT_LIST_HEAD(&inst->registeredbufs);
>>> +
>>> +Â Â Â Â Â Â inst->enc_state = VENUS_ENC_STATE_INIT;
>>> +
>>> +Â Â Â Â Â Â 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;
>>> @@ -881,61 +945,73 @@ 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);
>>> -Â Â Â Â Â Â int ret;
>>> +Â Â Â Â Â Â int ret = 0;
>>>
>>> Â Â Â Â Â Â Â mutex_lock(&inst->lock);
>>>
>>> -Â Â Â Â Â Â if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
>>> +Â Â Â Â Â Â 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))
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto unlock;
>>>
>>> -Â Â Â Â Â Â venus_helper_init_instance(inst);
>>> +Â Â Â Â Â Â if (inst->enc_state == VENUS_ENC_STATE_INIT) {
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â 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_set_properties(inst);
>>> -Â Â Â Â Â Â if (ret)
>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto deinit_sess;
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â ret = venc_verify_conf(inst);
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (ret)
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto deinit_sess;
>>>
>>> -Â Â Â Â Â Â ret = venc_verify_conf(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 = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â inst->num_output_bufs, 0);
>>> -Â Â Â Â Â Â if (ret)
>>> -Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto deinit_sess;
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â ret = venus_helper_vb2_start_streaming(inst);
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (ret)
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto deinit_sess;
>>>
>>> -Â Â Â Â Â Â 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);
>>>
>>> -Â Â Â Â Â Â inst->enc_state = VENUS_ENC_STATE_ENCODING;
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â inst->enc_state = VENUS_ENC_STATE_ENCODING;
>>> +Â Â Â Â Â Â } else if (inst->enc_state == VENUS_ENC_STATE_RESET &&
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â V4L2_TYPE_IS_CAPTURE(q->type)) {
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â ret = venus_helper_vb2_start_streaming(inst);
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â if (ret)
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto bufs_done;
>>>
>>> -Â Â Â Â Â Â mutex_unlock(&inst->lock);
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â venus_helper_process_initial_out_bufs(inst);
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â venus_helper_process_initial_cap_bufs(inst);
>>>
>>> -Â Â Â Â Â Â return 0;
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â inst->enc_state = VENUS_ENC_STATE_ENCODING;
>>> +Â Â Â Â Â Â } else if (inst->enc_state == VENUS_ENC_STATE_STOPPED &&
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â V4L2_TYPE_IS_OUTPUT(q->type)) {
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â inst->enc_state = VENUS_ENC_STATE_ENCODING;
>>> +Â Â Â Â Â Â }
>>> +
>>> +unlock:
>>> +Â Â Â Â Â Â mutex_unlock(&inst->lock);
>>> +Â Â Â Â Â Â return ret;
>>>
>>> Â deinit_sess:
>>> Â Â Â Â Â Â Â 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;
>>> @@ -943,33 +1019,97 @@ static int venc_start_streaming(struct
>>> vb2_queue *q, unsigned int count)
>>> Â Â Â Â Â Â Â return ret;
>>> Â }
>>>
>>> -static void venc_vb2_buf_queue(struct vb2_buffer *vb)
>>> +static int venc_stop_capture(struct venus_inst *inst)
>>> +{
>>> +Â Â Â Â Â Â int ret;
>>> +
>>> +Â Â Â Â Â Â switch (inst->enc_state) {
>>> +Â Â Â Â Â Â case VENUS_ENC_STATE_ENCODING:
>>> +Â Â Â Â Â Â case VENUS_ENC_STATE_DRAIN:
>>> +Â Â Â Â Â Â case VENUS_ENC_STATE_STOPPED:
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â inst->enc_state = VENUS_ENC_STATE_RESET;
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
>>> +Â Â Â Â Â Â default:
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â return -EINVAL;
>>> +Â Â Â Â Â Â }
>>> +
>>> +Â Â Â Â Â Â ret = hfi_session_stop(inst);
>>> +Â Â Â Â Â Â ret |= hfi_session_unload_res(inst);
>>> +Â Â Â Â Â Â ret |= venus_helper_unregister_bufs(inst);
>>> +Â Â Â Â Â Â ret |= venus_helper_intbufs_free(inst);
>>> +
>>> +Â Â Â Â Â Â return ret;
>>> +}
>>> +
>>> +static int venc_stop_output(struct venus_inst *inst)
>>> +{
>>> +Â Â Â Â Â Â switch (inst->enc_state) {
>>> +Â Â Â Â Â Â case VENUS_ENC_STATE_ENCODING:
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â inst->enc_state = VENUS_ENC_STATE_STOPPED;
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
>>> +Â Â Â Â Â Â case VENUS_ENC_STATE_DRAIN:
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â inst->enc_state = VENUS_ENC_STATE_STOPPED;
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â break;
>>> +Â Â Â Â Â Â default:
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â return -EINVAL;
>>> +Â Â Â Â Â Â }
>>> +
>>> +Â Â Â Â Â Â return 0;
>>> +}
>>> +
>>> +static void venc_stop_streaming(struct vb2_queue *q)
>>> +{
>>> +Â Â Â Â Â Â struct venus_inst *inst = vb2_get_drv_priv(q);
>>> +Â Â Â Â Â Â int ret = -EINVAL;
>>> +
>>> +Â Â Â Â Â Â mutex_lock(&inst->lock);
>>> +
>>> +Â Â Â Â Â Â if (V4L2_TYPE_IS_OUTPUT(q->type))
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â ret = venc_stop_output(inst);
>>> +Â Â Â Â Â Â else
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â ret = venc_stop_capture(inst);
>>> +
>>> +Â Â Â Â Â Â venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
>>> +
>>> +Â Â Â Â Â Â if (ret)
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto unlock;
>>> +
>>> +Â Â Â Â Â Â if (V4L2_TYPE_IS_OUTPUT(q->type))
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â inst->streamon_out = 0;
>>> +Â Â Â Â Â Â else
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â inst->streamon_cap = 0;
>>> +
>>> +unlock:
>>> +Â Â Â Â Â Â 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);
>>> +Â Â Â Â Â Â struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
>>>
>>> Â Â Â Â Â Â Â 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;
>>> -Â Â Â Â Â Â }
>>> +Â Â Â Â Â Â v4l2_m2m_buf_queue(m2m_ctx, vbuf);
>>> +
>>> +Â Â Â Â Â Â if (!(inst->streamon_out && inst->streamon_cap))
>>> +Â Â Â Â Â Â Â Â Â Â Â Â Â Â goto unlock;
>>> +
>>> +Â Â Â Â Â Â venus_helper_process_buf(vb);
>>>
>>> -Â Â Â Â Â Â venus_helper_vb2_buf_queue(vb);
>>> +unlock:
>>> Â Â Â Â Â Â Â mutex_unlock(&inst->lock);
>>> Â }
>>>
>>> Â 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,
>>> @@ -1025,8 +1165,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,
>>> Â };
>>>
>>> @@ -1098,11 +1242,9 @@ static int venc_open(struct file *file)
>>> Â Â Â Â Â Â Â inst->core = core;
>>> Â Â Â Â Â Â Â inst->session_type = VIDC_SESSION_TYPE_ENC;
>>> Â Â Â Â Â Â Â inst->clk_data.core_id = VIDC_CORE_ID_DEFAULT;
>>> +Â Â Â Â Â Â inst->enc_state = VENUS_ENC_STATE_INIT;
>>> Â Â Â Â Â Â Â 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);
>>> @@ -1167,7 +1309,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);
>>> --
>>> 2.17.1
>>>
>
> Thanks,
> Vikash
--
regards,
Stan
On Thu, Nov 5, 2020 at 3:51 AM Stanimir Varbanov
<[email protected]> wrote:
>
>
>
> On 11/4/20 12:44 PM, [email protected] wrote:
> > Hi Stan,
> >
> > On 2020-11-03 06:46, Fritz Koenig wrote:
> >> On Fri, Oct 23, 2020 at 5:57 AM Stanimir Varbanov
> >> <[email protected]> wrote:
> >>>
> >>> Redesign the encoder driver to be compliant with stateful encoder
> >>> spec - specifically adds handling of Reset state.
> >>>
> >>> Signed-off-by: Stanimir Varbanov <[email protected]>
> >>> ---
> >>> drivers/media/platform/qcom/venus/core.h | 10 +-
> >>> drivers/media/platform/qcom/venus/venc.c | 242 ++++++++++++++++++-----
> >>> 2 files changed, 197 insertions(+), 55 deletions(-)
> >>>
> >>> diff --git a/drivers/media/platform/qcom/venus/core.h
> >>> b/drivers/media/platform/qcom/venus/core.h
> >>> index 5c4693678e3f..294d5467a6a1 100644
> >>> --- a/drivers/media/platform/qcom/venus/core.h
> >>> +++ b/drivers/media/platform/qcom/venus/core.h
> >>> @@ -277,11 +277,11 @@ enum venus_dec_state {
> >>> };
> >>>
> >>> 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,
> >>> + VENUS_ENC_STATE_INIT = 0,
> >>> + VENUS_ENC_STATE_ENCODING = 1,
> >>> + VENUS_ENC_STATE_STOPPED = 2,
> >>> + VENUS_ENC_STATE_DRAIN = 3,
> >>> + VENUS_ENC_STATE_RESET = 4,
> >>> };
> >>>
> >>> struct venus_ts_metadata {
> >>> diff --git a/drivers/media/platform/qcom/venus/venc.c
> >>> b/drivers/media/platform/qcom/venus/venc.c
> >>> index c6143b07914c..aa9255ddb0a5 100644
> >>> --- a/drivers/media/platform/qcom/venus/venc.c
> >>> +++ b/drivers/media/platform/qcom/venus/venc.c
> >>> @@ -565,6 +565,7 @@ static const struct v4l2_ioctl_ops venc_ioctl_ops
> >>> = {
> >>> .vidioc_enum_frameintervals = venc_enum_frameintervals,
> >>> .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
> >>> .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> >>> + .vidioc_try_encoder_cmd = v4l2_m2m_ioctl_try_encoder_cmd,
> >>> .vidioc_encoder_cmd = venc_encoder_cmd,
> >>> };
> >>>
> >>> @@ -850,6 +851,69 @@ static int venc_queue_setup(struct vb2_queue *q,
> >>> return ret;
> >>> }
> >>>
> >>> +static void venc_release_session(struct venus_inst *inst)
> >>> +{
> >>> + struct venus_core *core = inst->core;
> >>> + int ret, abort = 0;
> >>> +
> >>> + mutex_lock(&inst->lock);
> >>> +
> >>> + if (inst->enc_state != VENUS_ENC_STATE_RESET)
> >>> + dev_dbg(core->dev_enc, VDBGH "wrong state!\n");
> >>> +
> >>> + ret = hfi_session_stop(inst);
> >>> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
> >>> + ret = hfi_session_unload_res(inst);
> >>> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
> >>> + ret = venus_helper_unregister_bufs(inst);
> >>> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
> >>> + ret = venus_helper_intbufs_free(inst);
> >>> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
> >>> + 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);
> >>
> >> venus_pm_load_scale depends on inst->clk_data.codec_freq_data being
> >> set up. This occurs in venc_init_session(). I am seeing scenarios
> >> where the encoder is getting setup up, but before it is finished,
> >> teardown occurs. If this teardown occurs before
> >> inst->clk_data.codec_freq_data is initalized, a crash occurs. (also
> >> "wrong state!" is printed out)
> >
> > venus_pm_load_scale(inst) is called here to remove the votes for clock
> > and bus bandwidth. This is not needed for instances which are closed
> > just after moving them to INIT state. I have tried with below state
> > handling
> > and it works well i.e call for venus_pm_load_scale(inst) from state other
> > than the INIT state.
>
> IMO we have to check the INST state in venus_pm_load_scale(). We made
> similar check in min_loaded_core() here [1].
>
> [1] https://www.spinics.net/lists/kernel/msg3498215.html
>
I took a look at putting this check in venus_pm_load_scale. The
problem I see with that is venus_pm_load_scale is shared between
encoder and decoder. So checking for VENUS_ENC_STATE_INIT during
decode doesn't make sense. I tried to match up with the instance
states in hfi.h, but it doesn't appear to align well. Is there a hfi
state that this would work for?
-Fritz
> >
> > - venus_pm_load_scale(inst);
> > + if(inst->enc_state != VENUS_ENC_STATE_INIT)
> > + venus_pm_load_scale(inst);
> >
> >>
> >> [ 106.593198] Unable to handle kernel NULL pointer dereference at
> >> virtual address 0000000000000008
> >> [ 106.603916] Mem abort info:
> >> [ 106.608470] ESR = 0x96000006
> >> [ 106.613802] EC = 0x25: DABT (current EL), IL = 32 bits
> >> [ 106.619426] SET = 0, FnV = 0
> >> [ 106.622619] EA = 0, S1PTW = 0
> >> [ 106.625862] Data abort info:
> >> [ 106.628835] ISV = 0, ISS = 0x00000006
> >> [ 106.632785] CM = 0, WnR = 0
> >> [ 106.635850] user pgtable: 4k pages, 39-bit VAs, pgdp=000000014839f000
> >> [ 106.642472] [0000000000000008] pgd=000000016ab1f003,
> >> pud=000000016ab1f003, pmd=0000000000000000
> >> [ 106.651410] Internal error: Oops: 96000006 [#1] PREEMPT SMP
> >> [ 106.657132] Modules linked in: rfcomm algif_hash algif_skcipher
> >> af_alg uinput venus_dec venus_enc videobuf2_dma_sg hci_uart btqca
> >> venus_core qcom_spmi_adc5 qcom_spmi_temp_alarm qcom_vadc_common
> >> snd_soc_rt5682_i2c v4l2_mem2mem snd_soc_sc7180 snd_soc_rt5682
> >> snd_soc_qcom_common snd_soc_rl6231 bluetooth ecdh_generic ecc
> >> snd_soc_lpass_sc7180 snd_soc_lpass_hdmi snd_soc_lpass_cpu
> >> snd_soc_lpass_platform snd_soc_max98357a xt_MASQUERADE fuse
> >> iio_trig_sysfs cros_ec_lid_angle cros_ec_sensors cros_ec_sensors_core
> >> industrialio_triggered_buffer cros_ec_sensors_ring rmtfs_mem kfifo_buf
> >> cros_ec_sensorhub ath10k_snoc lzo_rle ath10k_core lzo_compress ath
> >> zram mac80211 ipa qmi_helpers cfg80211 qcom_q6v5_mss qcom_pil_info
> >> qcom_q6v5 qcom_common cdc_ether usbnet r8152 mii uvcvideo
> >> videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_common
> >> joydev
> >> [ 106.732576] CPU: 7 PID: 3622 Comm: DrmThread Not tainted 5.4.72 #40
> >> [ 106.739004] Hardware name: Google Lazor (rev1+) (DT)
> >> [ 106.744103] pstate: 60400009 (nZCv daif +PAN -UAO)
> >> [ 106.749027] pc : load_scale_v4+0x160/0x3a4 [venus_core]
> >> [ 106.754396] lr : load_scale_v4+0x154/0x3a4 [venus_core]
> >> [ 106.759766] sp : ffffffc0120ab9e0
> >> [ 106.763171] x29: ffffffc0120ab9e0 x28: 0000000000000005
> >> [ 106.768629] x27: 0000000000000000 x26: 0000000000000000
> >> [ 106.774080] x25: 0000000000000000 x24: 000000000000001e
> >> [ 106.779530] x23: 0000000000000000 x22: ffffff80b41a8000
> >> [ 106.784980] x21: ffffffd344e97e98 x20: ffffff80cb5b8080
> >> [ 106.790431] x19: ffffff80fa3b1410 x18: 00000000ffff0a10
> >> [ 106.795881] x17: 000000000000003c x16: ffffffd398ec2e88
> >> [ 106.801329] x15: 0000000000000006 x14: ffff001000000600
> >> [ 106.806779] x13: 000000000002cca2 x12: 0000000000000000
> >> [ 106.812229] x11: 0000000000000000 x10: 0000000000000000
> >> [ 106.817679] x9 : 472cbd12793c4600 x8 : 0000000000000000
> >> [ 106.823137] x7 : 0000000000000000 x6 : ffffffd399dbcc6c
> >> [ 106.828585] x5 : 0000000000000000 x4 : 0000000000000000
> >> [ 106.834035] x3 : 0000000000000000 x2 : ffffff80ff6ae5c0
> >> [ 106.839487] x1 : ffffff80ff69e018 x0 : ffffffd344e990ac
> >> [ 106.844937] Call trace:
> >> [ 106.847460] load_scale_v4+0x160/0x3a4 [venus_core]
> >> [ 106.852473] venc_buf_cleanup+0x198/0x1f0 [venus_enc]
> >> [ 106.857656] __vb2_queue_free+0x90/0x1f4 [videobuf2_common]
> >> [ 106.863374] vb2_core_queue_release+0x3c/0x50 [videobuf2_common]
> >> [ 106.869541] vb2_queue_release+0x1c/0x28 [videobuf2_v4l2]
> >> [ 106.875082] v4l2_m2m_ctx_release+0x24/0x40 [v4l2_mem2mem]
> >> [ 106.880710] venc_close+0x24/0x78 [venus_enc]
> >> [ 106.885188] v4l2_release+0x8c/0xdc
> >> [ 106.888779] __fput+0xe0/0x214
> >> [ 106.891916] ____fput+0x1c/0x28
> >> [ 106.895148] task_work_run+0x94/0xc4
> >> [ 106.898828] do_exit+0x244/0x7c8
> >> [ 106.902147] do_group_exit+0x88/0x98
> >> [ 106.905825] get_signal+0x1cc/0x674
> >> [ 106.909415] do_notify_resume+0x134/0x1410
> >> [ 106.913619] work_pending+0x8/0x10
> >> [ 106.917119] Code: 97ffd58d f94032c8 90000020 9102b000 (f9400501)
> >> [ 106.923377] ---[ end trace a9caaf72c228e386 ]---
> >> [ 106.928767] Kernel panic - not syncing: Fatal exception
> >> [ 106.934131] SMP: stopping secondary CPUs
> >> [ 106.938347] Kernel Offset: 0x1388a00000 from 0xffffffc010000000
> >> [ 106.944426] PHYS_OFFSET: 0xffffffd900000000
> >> [ 106.948722] CPU features: 0x08102e,2a80aa18
> >> [ 106.953015] Memory Limit: none
> >>
> >>
> >> This is the debug log before the crash:
> >> [Nov 2 15:33] qcom-venus aa00000.video-codec: VenusLow : venus hw
> >> version 4.44.20a
> >> [ +0.000065] videodev: v4l2_open: video2: open (0)
> >> [ +0.000019] video2: VIDIOC_ENUM_FMT: index=0, type=vid-cap-mplane,
> >> flags=0x1, pixelformat=H264, description='H.264'
> >> [ +0.000017] video2: VIDIOC_ENUM_FMT: index=1, type=vid-cap-mplane,
> >> flags=0x1, pixelformat=VP80, description='VP8'
> >> [ +0.000042] video2: VIDIOC_ENUM_FMT: index=2, type=vid-cap-mplane,
> >> flags=0x1, pixelformat=HEVC, description='HEVC'
> >> [ +0.000030] video2: VIDIOC_ENUM_FMT: error -22: index=3,
> >> type=vid-cap-mplane, flags=0x0, pixelformat=\x00\x00\x00\x00,
> >> description=''
> >> [ +0.000068] videodev: v4l2_release: video2: release
> >> [ +0.002752] qcom-venus aa00000.video-codec: VenusLow : venus hw
> >> version 4.44.20a
> >> [ +0.000062] videodev: v4l2_open: video2: open (0)
> >> [ +0.000071] video2: VIDIOC_ENUM_FRAMESIZES: index=0,
> >> pixelformat=H264, type=3, min=96x96, max=4096x4096, step=16x16
> >> [ +0.000012] video2: VIDIOC_TRY_ENCODER_CMD: cmd=1, flags=0x0
> >> [ +0.000005] video2: VIDIOC_QUERYCAP: driver=qcom-venus,
> >> card=Qualcomm Venus video encoder, bus=platform:qcom-venus,
> >> version=0x00050448, capabilities=0x84204000, device_caps=0x04204000
> >> [ +0.001055] video2: VIDIOC_REQBUFS: count=0, type=vid-out-mplane,
> >> memory=mmap
> >> [ +0.000382] video2: VIDIOC_REQBUFS: count=0, type=vid-cap-mplane,
> >> memory=mmap
> >> [ +0.000227] video2: VIDIOC_S_FMT: type=vid-cap-mplane, width=96,
> >> height=96, format=H264, field=none, colorspace=0, num_planes=1,
> >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> >> [ +0.000027] plane 0: bytesperline=0 sizeimage=2097152
> >> [ +0.000527] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> >> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> >> [ +0.000024] plane 0: bytesperline=384 sizeimage=122880
> >> [ +0.000032] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> >> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> >> [ +0.000017] plane 0: bytesperline=384 sizeimage=122880
> >> [ +0.000024] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> >> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> >> [ +0.000017] plane 0: bytesperline=384 sizeimage=122880
> >> [ +0.000075] video2: VIDIOC_S_SELECTION: type=vid-out, target=0,
> >> flags=0x0, wxh=320x192, x,y=0,0
> >> [ +0.028300] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> >> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> >> [ +0.000036] plane 0: bytesperline=384 sizeimage=122880
> >> [ +0.000033] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> >> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> >> [ +0.000014] plane 0: bytesperline=384 sizeimage=122880
> >> [ +0.000020] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> >> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> >> [ +0.000120] plane 0: bytesperline=384 sizeimage=122880
> >> [ +0.000157] video2: VIDIOC_S_SELECTION: type=vid-out, target=0,
> >> flags=0x0, wxh=320x192, x,y=0,0
> >> [ +0.000250] video2: VIDIOC_S_EXT_CTRLS: which=0x990000, count=1,
> >> error_idx=0, request_fd=0, id/val=0x9909d7/0x1
> >> [ +0.000120] video2: VIDIOC_QUERYCTRL: error -22: id=0x990b84,
> >> type=0, name=, min/max=0/0, step=0, default=0, flags=0x00000000
> >> [ +0.000368] v4l2-ctrls: try_set_ext_ctrls: video2: video2:
> >> try_set_ext_ctrls_common failed (-22)
> >> [ +0.000087] video2: VIDIOC_S_EXT_CTRLS: error -22: which=0x990000,
> >> count=5, error_idx=5, request_fd=0, id/val=0x9909ca/0x0,
> >> id/val=0x990a62/0x33, id/val=0x990a6b/0x2, id/val=0x990a67/0xb,
> >> id/val=0x9909d8/0x1
> >> [ +0.000290] v4l2-ctrls: prepare_ext_ctrls: video2: cannot find
> >> control id 0x9909da
> >> [ +0.000010] v4l2-ctrls: try_set_ext_ctrls: video2: video2:
> >> try_set_ext_ctrls_common failed (-22)
> >> [ +0.000014] video2: VIDIOC_S_EXT_CTRLS: error -22: which=0x990000,
> >> count=2, error_idx=2, request_fd=0, id/val=0x9909da/0x1,
> >> id/val=0x9909cb/0x0
> >> [ +0.000225] video2: VIDIOC_G_FMT: type=vid-cap-mplane, width=320,
> >> height=192, format=H264, field=none, colorspace=0, num_planes=1,
> >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> >> [ +0.000028] plane 0: bytesperline=0 sizeimage=49152
> >> [ +0.002272] video2: VIDIOC_REQBUFS: count=4, type=vid-cap-mplane,
> >> memory=mmap
> >> [ +0.001661] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=0,
> >> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
> >> sequence=0, memory=mmap
> >> [ +0.000034] plane 0: bytesused=0, data_offset=0x00000000,
> >> offset/userptr=0xee18ad4840000000, length=2097152
> >> [ +0.000009] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
> >> userbits=0x00000000
> >> [ +0.000142] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=1,
> >> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
> >> sequence=0, memory=mmap
> >> [ +0.000023] plane 0: bytesused=0, data_offset=0x00000000,
> >> offset/userptr=0xee18ad4840200000, length=2097152
> >> [ +0.000008] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
> >> userbits=0x00000000
> >> [ +0.000409] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=2,
> >> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
> >> sequence=0, memory=mmap
> >> [ +0.000027] plane 0: bytesused=0, data_offset=0x00000000,
> >> offset/userptr=0xee18ad4840400000, length=2097152
> >> [ +0.000007] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
> >> userbits=0x00000000
> >> [ +0.000233] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=3,
> >> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
> >> sequence=0, memory=mmap
> >> [ +0.000023] plane 0: bytesused=0, data_offset=0x00000000,
> >> offset/userptr=0xee18ad4840600000, length=2097152
> >> [ +0.000015] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
> >> userbits=0x00000000
> >> [ +0.000288] video2: VIDIOC_S_EXT_CTRLS: which=0x990000, count=1,
> >> error_idx=0, request_fd=0, id/val=0x9909cf/0x30d40
> >> [ +0.000184] video2: VIDIOC_S_PARM: type=vid-out-mplane,
> >> capability=0x1000, outputmode=0x0, timeperframe=1/30, extendedmode=0,
> >> writebuffers=0
> >> [ +0.310832] qcom-venus-encoder aa00000.video-codec:video-encoder:
> >> VenusHigh: wrong state!
> >>
> >>> + INIT_LIST_HEAD(&inst->registeredbufs);
> >>> +
> >>> + inst->enc_state = VENUS_ENC_STATE_INIT;
> >>> +
> >>> + 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;
> >>> @@ -881,61 +945,73 @@ 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);
> >>> - int ret;
> >>> + int ret = 0;
> >>>
> >>> mutex_lock(&inst->lock);
> >>>
> >>> - if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> >>> + 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))
> >>> + goto unlock;
> >>>
> >>> - venus_helper_init_instance(inst);
> >>> + if (inst->enc_state == VENUS_ENC_STATE_INIT) {
> >>> + 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_set_properties(inst);
> >>> - if (ret)
> >>> - goto deinit_sess;
> >>> + ret = venc_verify_conf(inst);
> >>> + if (ret)
> >>> + goto deinit_sess;
> >>>
> >>> - ret = venc_verify_conf(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 = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
> >>> - inst->num_output_bufs, 0);
> >>> - if (ret)
> >>> - goto deinit_sess;
> >>> + ret = venus_helper_vb2_start_streaming(inst);
> >>> + if (ret)
> >>> + goto deinit_sess;
> >>>
> >>> - 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);
> >>>
> >>> - inst->enc_state = VENUS_ENC_STATE_ENCODING;
> >>> + inst->enc_state = VENUS_ENC_STATE_ENCODING;
> >>> + } else if (inst->enc_state == VENUS_ENC_STATE_RESET &&
> >>> + V4L2_TYPE_IS_CAPTURE(q->type)) {
> >>> + ret = venus_helper_vb2_start_streaming(inst);
> >>> + if (ret)
> >>> + goto bufs_done;
> >>>
> >>> - mutex_unlock(&inst->lock);
> >>> + venus_helper_process_initial_out_bufs(inst);
> >>> + venus_helper_process_initial_cap_bufs(inst);
> >>>
> >>> - return 0;
> >>> + inst->enc_state = VENUS_ENC_STATE_ENCODING;
> >>> + } else if (inst->enc_state == VENUS_ENC_STATE_STOPPED &&
> >>> + V4L2_TYPE_IS_OUTPUT(q->type)) {
> >>> + inst->enc_state = VENUS_ENC_STATE_ENCODING;
> >>> + }
> >>> +
> >>> +unlock:
> >>> + mutex_unlock(&inst->lock);
> >>> + return ret;
> >>>
> >>> deinit_sess:
> >>> 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;
> >>> @@ -943,33 +1019,97 @@ static int venc_start_streaming(struct
> >>> vb2_queue *q, unsigned int count)
> >>> return ret;
> >>> }
> >>>
> >>> -static void venc_vb2_buf_queue(struct vb2_buffer *vb)
> >>> +static int venc_stop_capture(struct venus_inst *inst)
> >>> +{
> >>> + int ret;
> >>> +
> >>> + switch (inst->enc_state) {
> >>> + case VENUS_ENC_STATE_ENCODING:
> >>> + case VENUS_ENC_STATE_DRAIN:
> >>> + case VENUS_ENC_STATE_STOPPED:
> >>> + inst->enc_state = VENUS_ENC_STATE_RESET;
> >>> + break;
> >>> + default:
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + ret = hfi_session_stop(inst);
> >>> + ret |= hfi_session_unload_res(inst);
> >>> + ret |= venus_helper_unregister_bufs(inst);
> >>> + ret |= venus_helper_intbufs_free(inst);
> >>> +
> >>> + return ret;
> >>> +}
> >>> +
> >>> +static int venc_stop_output(struct venus_inst *inst)
> >>> +{
> >>> + switch (inst->enc_state) {
> >>> + case VENUS_ENC_STATE_ENCODING:
> >>> + inst->enc_state = VENUS_ENC_STATE_STOPPED;
> >>> + break;
> >>> + case VENUS_ENC_STATE_DRAIN:
> >>> + inst->enc_state = VENUS_ENC_STATE_STOPPED;
> >>> + break;
> >>> + default:
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static void venc_stop_streaming(struct vb2_queue *q)
> >>> +{
> >>> + struct venus_inst *inst = vb2_get_drv_priv(q);
> >>> + int ret = -EINVAL;
> >>> +
> >>> + mutex_lock(&inst->lock);
> >>> +
> >>> + if (V4L2_TYPE_IS_OUTPUT(q->type))
> >>> + ret = venc_stop_output(inst);
> >>> + else
> >>> + ret = venc_stop_capture(inst);
> >>> +
> >>> + venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
> >>> +
> >>> + if (ret)
> >>> + goto unlock;
> >>> +
> >>> + if (V4L2_TYPE_IS_OUTPUT(q->type))
> >>> + inst->streamon_out = 0;
> >>> + else
> >>> + inst->streamon_cap = 0;
> >>> +
> >>> +unlock:
> >>> + 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);
> >>> + struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
> >>>
> >>> 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;
> >>> - }
> >>> + v4l2_m2m_buf_queue(m2m_ctx, vbuf);
> >>> +
> >>> + if (!(inst->streamon_out && inst->streamon_cap))
> >>> + goto unlock;
> >>> +
> >>> + venus_helper_process_buf(vb);
> >>>
> >>> - venus_helper_vb2_buf_queue(vb);
> >>> +unlock:
> >>> mutex_unlock(&inst->lock);
> >>> }
> >>>
> >>> 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,
> >>> @@ -1025,8 +1165,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,
> >>> };
> >>>
> >>> @@ -1098,11 +1242,9 @@ static int venc_open(struct file *file)
> >>> inst->core = core;
> >>> inst->session_type = VIDC_SESSION_TYPE_ENC;
> >>> inst->clk_data.core_id = VIDC_CORE_ID_DEFAULT;
> >>> + inst->enc_state = VENUS_ENC_STATE_INIT;
> >>> 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);
> >>> @@ -1167,7 +1309,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);
> >>> --
> >>> 2.17.1
> >>>
> >
> > Thanks,
> > Vikash
>
> --
> regards,
> Stan
On Mon, Nov 9, 2020 at 9:48 PM Fritz Koenig <[email protected]> wrote:
>
> On Thu, Nov 5, 2020 at 3:51 AM Stanimir Varbanov
> <[email protected]> wrote:
> >
> >
> >
> > On 11/4/20 12:44 PM, [email protected] wrote:
> > > Hi Stan,
> > >
> > > On 2020-11-03 06:46, Fritz Koenig wrote:
> > >> On Fri, Oct 23, 2020 at 5:57 AM Stanimir Varbanov
> > >> <[email protected]> wrote:
> > >>>
> > >>> Redesign the encoder driver to be compliant with stateful encoder
> > >>> spec - specifically adds handling of Reset state.
> > >>>
> > >>> Signed-off-by: Stanimir Varbanov <[email protected]>
> > >>> ---
> > >>> drivers/media/platform/qcom/venus/core.h | 10 +-
> > >>> drivers/media/platform/qcom/venus/venc.c | 242 ++++++++++++++++++-----
> > >>> 2 files changed, 197 insertions(+), 55 deletions(-)
> > >>>
> > >>> diff --git a/drivers/media/platform/qcom/venus/core.h
> > >>> b/drivers/media/platform/qcom/venus/core.h
> > >>> index 5c4693678e3f..294d5467a6a1 100644
> > >>> --- a/drivers/media/platform/qcom/venus/core.h
> > >>> +++ b/drivers/media/platform/qcom/venus/core.h
> > >>> @@ -277,11 +277,11 @@ enum venus_dec_state {
> > >>> };
> > >>>
> > >>> 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,
> > >>> + VENUS_ENC_STATE_INIT = 0,
> > >>> + VENUS_ENC_STATE_ENCODING = 1,
> > >>> + VENUS_ENC_STATE_STOPPED = 2,
> > >>> + VENUS_ENC_STATE_DRAIN = 3,
> > >>> + VENUS_ENC_STATE_RESET = 4,
> > >>> };
> > >>>
> > >>> struct venus_ts_metadata {
> > >>> diff --git a/drivers/media/platform/qcom/venus/venc.c
> > >>> b/drivers/media/platform/qcom/venus/venc.c
> > >>> index c6143b07914c..aa9255ddb0a5 100644
> > >>> --- a/drivers/media/platform/qcom/venus/venc.c
> > >>> +++ b/drivers/media/platform/qcom/venus/venc.c
> > >>> @@ -565,6 +565,7 @@ static const struct v4l2_ioctl_ops venc_ioctl_ops
> > >>> = {
> > >>> .vidioc_enum_frameintervals = venc_enum_frameintervals,
> > >>> .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
> > >>> .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> > >>> + .vidioc_try_encoder_cmd = v4l2_m2m_ioctl_try_encoder_cmd,
> > >>> .vidioc_encoder_cmd = venc_encoder_cmd,
> > >>> };
> > >>>
> > >>> @@ -850,6 +851,69 @@ static int venc_queue_setup(struct vb2_queue *q,
> > >>> return ret;
> > >>> }
> > >>>
> > >>> +static void venc_release_session(struct venus_inst *inst)
> > >>> +{
> > >>> + struct venus_core *core = inst->core;
> > >>> + int ret, abort = 0;
> > >>> +
> > >>> + mutex_lock(&inst->lock);
> > >>> +
> > >>> + if (inst->enc_state != VENUS_ENC_STATE_RESET)
> > >>> + dev_dbg(core->dev_enc, VDBGH "wrong state!\n");
> > >>> +
> > >>> + ret = hfi_session_stop(inst);
> > >>> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
> > >>> + ret = hfi_session_unload_res(inst);
> > >>> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
> > >>> + ret = venus_helper_unregister_bufs(inst);
> > >>> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
> > >>> + ret = venus_helper_intbufs_free(inst);
> > >>> + abort |= (ret && ret != -EINVAL) ? 1 : 0;
> > >>> + 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);
> > >>
> > >> venus_pm_load_scale depends on inst->clk_data.codec_freq_data being
> > >> set up. This occurs in venc_init_session(). I am seeing scenarios
> > >> where the encoder is getting setup up, but before it is finished,
> > >> teardown occurs. If this teardown occurs before
> > >> inst->clk_data.codec_freq_data is initalized, a crash occurs. (also
> > >> "wrong state!" is printed out)
> > >
> > > venus_pm_load_scale(inst) is called here to remove the votes for clock
> > > and bus bandwidth. This is not needed for instances which are closed
> > > just after moving them to INIT state. I have tried with below state
> > > handling
> > > and it works well i.e call for venus_pm_load_scale(inst) from state other
> > > than the INIT state.
> >
> > IMO we have to check the INST state in venus_pm_load_scale(). We made
> > similar check in min_loaded_core() here [1].
> >
> > [1] https://www.spinics.net/lists/kernel/msg3498215.html
> >
>
> I took a look at putting this check in venus_pm_load_scale. The
> problem I see with that is venus_pm_load_scale is shared between
> encoder and decoder. So checking for VENUS_ENC_STATE_INIT during
> decode doesn't make sense. I tried to match up with the instance
> states in hfi.h, but it doesn't appear to align well. Is there a hfi
> state that this would work for?
>
> -Fritz
>
I think I found a reasonable approach:
https://www.spinics.net/lists/linux-media/msg180866.html
-Fritz
> > >
> > > - venus_pm_load_scale(inst);
> > > + if(inst->enc_state != VENUS_ENC_STATE_INIT)
> > > + venus_pm_load_scale(inst);
> > >
> > >>
> > >> [ 106.593198] Unable to handle kernel NULL pointer dereference at
> > >> virtual address 0000000000000008
> > >> [ 106.603916] Mem abort info:
> > >> [ 106.608470] ESR = 0x96000006
> > >> [ 106.613802] EC = 0x25: DABT (current EL), IL = 32 bits
> > >> [ 106.619426] SET = 0, FnV = 0
> > >> [ 106.622619] EA = 0, S1PTW = 0
> > >> [ 106.625862] Data abort info:
> > >> [ 106.628835] ISV = 0, ISS = 0x00000006
> > >> [ 106.632785] CM = 0, WnR = 0
> > >> [ 106.635850] user pgtable: 4k pages, 39-bit VAs, pgdp=000000014839f000
> > >> [ 106.642472] [0000000000000008] pgd=000000016ab1f003,
> > >> pud=000000016ab1f003, pmd=0000000000000000
> > >> [ 106.651410] Internal error: Oops: 96000006 [#1] PREEMPT SMP
> > >> [ 106.657132] Modules linked in: rfcomm algif_hash algif_skcipher
> > >> af_alg uinput venus_dec venus_enc videobuf2_dma_sg hci_uart btqca
> > >> venus_core qcom_spmi_adc5 qcom_spmi_temp_alarm qcom_vadc_common
> > >> snd_soc_rt5682_i2c v4l2_mem2mem snd_soc_sc7180 snd_soc_rt5682
> > >> snd_soc_qcom_common snd_soc_rl6231 bluetooth ecdh_generic ecc
> > >> snd_soc_lpass_sc7180 snd_soc_lpass_hdmi snd_soc_lpass_cpu
> > >> snd_soc_lpass_platform snd_soc_max98357a xt_MASQUERADE fuse
> > >> iio_trig_sysfs cros_ec_lid_angle cros_ec_sensors cros_ec_sensors_core
> > >> industrialio_triggered_buffer cros_ec_sensors_ring rmtfs_mem kfifo_buf
> > >> cros_ec_sensorhub ath10k_snoc lzo_rle ath10k_core lzo_compress ath
> > >> zram mac80211 ipa qmi_helpers cfg80211 qcom_q6v5_mss qcom_pil_info
> > >> qcom_q6v5 qcom_common cdc_ether usbnet r8152 mii uvcvideo
> > >> videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_common
> > >> joydev
> > >> [ 106.732576] CPU: 7 PID: 3622 Comm: DrmThread Not tainted 5.4.72 #40
> > >> [ 106.739004] Hardware name: Google Lazor (rev1+) (DT)
> > >> [ 106.744103] pstate: 60400009 (nZCv daif +PAN -UAO)
> > >> [ 106.749027] pc : load_scale_v4+0x160/0x3a4 [venus_core]
> > >> [ 106.754396] lr : load_scale_v4+0x154/0x3a4 [venus_core]
> > >> [ 106.759766] sp : ffffffc0120ab9e0
> > >> [ 106.763171] x29: ffffffc0120ab9e0 x28: 0000000000000005
> > >> [ 106.768629] x27: 0000000000000000 x26: 0000000000000000
> > >> [ 106.774080] x25: 0000000000000000 x24: 000000000000001e
> > >> [ 106.779530] x23: 0000000000000000 x22: ffffff80b41a8000
> > >> [ 106.784980] x21: ffffffd344e97e98 x20: ffffff80cb5b8080
> > >> [ 106.790431] x19: ffffff80fa3b1410 x18: 00000000ffff0a10
> > >> [ 106.795881] x17: 000000000000003c x16: ffffffd398ec2e88
> > >> [ 106.801329] x15: 0000000000000006 x14: ffff001000000600
> > >> [ 106.806779] x13: 000000000002cca2 x12: 0000000000000000
> > >> [ 106.812229] x11: 0000000000000000 x10: 0000000000000000
> > >> [ 106.817679] x9 : 472cbd12793c4600 x8 : 0000000000000000
> > >> [ 106.823137] x7 : 0000000000000000 x6 : ffffffd399dbcc6c
> > >> [ 106.828585] x5 : 0000000000000000 x4 : 0000000000000000
> > >> [ 106.834035] x3 : 0000000000000000 x2 : ffffff80ff6ae5c0
> > >> [ 106.839487] x1 : ffffff80ff69e018 x0 : ffffffd344e990ac
> > >> [ 106.844937] Call trace:
> > >> [ 106.847460] load_scale_v4+0x160/0x3a4 [venus_core]
> > >> [ 106.852473] venc_buf_cleanup+0x198/0x1f0 [venus_enc]
> > >> [ 106.857656] __vb2_queue_free+0x90/0x1f4 [videobuf2_common]
> > >> [ 106.863374] vb2_core_queue_release+0x3c/0x50 [videobuf2_common]
> > >> [ 106.869541] vb2_queue_release+0x1c/0x28 [videobuf2_v4l2]
> > >> [ 106.875082] v4l2_m2m_ctx_release+0x24/0x40 [v4l2_mem2mem]
> > >> [ 106.880710] venc_close+0x24/0x78 [venus_enc]
> > >> [ 106.885188] v4l2_release+0x8c/0xdc
> > >> [ 106.888779] __fput+0xe0/0x214
> > >> [ 106.891916] ____fput+0x1c/0x28
> > >> [ 106.895148] task_work_run+0x94/0xc4
> > >> [ 106.898828] do_exit+0x244/0x7c8
> > >> [ 106.902147] do_group_exit+0x88/0x98
> > >> [ 106.905825] get_signal+0x1cc/0x674
> > >> [ 106.909415] do_notify_resume+0x134/0x1410
> > >> [ 106.913619] work_pending+0x8/0x10
> > >> [ 106.917119] Code: 97ffd58d f94032c8 90000020 9102b000 (f9400501)
> > >> [ 106.923377] ---[ end trace a9caaf72c228e386 ]---
> > >> [ 106.928767] Kernel panic - not syncing: Fatal exception
> > >> [ 106.934131] SMP: stopping secondary CPUs
> > >> [ 106.938347] Kernel Offset: 0x1388a00000 from 0xffffffc010000000
> > >> [ 106.944426] PHYS_OFFSET: 0xffffffd900000000
> > >> [ 106.948722] CPU features: 0x08102e,2a80aa18
> > >> [ 106.953015] Memory Limit: none
> > >>
> > >>
> > >> This is the debug log before the crash:
> > >> [Nov 2 15:33] qcom-venus aa00000.video-codec: VenusLow : venus hw
> > >> version 4.44.20a
> > >> [ +0.000065] videodev: v4l2_open: video2: open (0)
> > >> [ +0.000019] video2: VIDIOC_ENUM_FMT: index=0, type=vid-cap-mplane,
> > >> flags=0x1, pixelformat=H264, description='H.264'
> > >> [ +0.000017] video2: VIDIOC_ENUM_FMT: index=1, type=vid-cap-mplane,
> > >> flags=0x1, pixelformat=VP80, description='VP8'
> > >> [ +0.000042] video2: VIDIOC_ENUM_FMT: index=2, type=vid-cap-mplane,
> > >> flags=0x1, pixelformat=HEVC, description='HEVC'
> > >> [ +0.000030] video2: VIDIOC_ENUM_FMT: error -22: index=3,
> > >> type=vid-cap-mplane, flags=0x0, pixelformat=\x00\x00\x00\x00,
> > >> description=''
> > >> [ +0.000068] videodev: v4l2_release: video2: release
> > >> [ +0.002752] qcom-venus aa00000.video-codec: VenusLow : venus hw
> > >> version 4.44.20a
> > >> [ +0.000062] videodev: v4l2_open: video2: open (0)
> > >> [ +0.000071] video2: VIDIOC_ENUM_FRAMESIZES: index=0,
> > >> pixelformat=H264, type=3, min=96x96, max=4096x4096, step=16x16
> > >> [ +0.000012] video2: VIDIOC_TRY_ENCODER_CMD: cmd=1, flags=0x0
> > >> [ +0.000005] video2: VIDIOC_QUERYCAP: driver=qcom-venus,
> > >> card=Qualcomm Venus video encoder, bus=platform:qcom-venus,
> > >> version=0x00050448, capabilities=0x84204000, device_caps=0x04204000
> > >> [ +0.001055] video2: VIDIOC_REQBUFS: count=0, type=vid-out-mplane,
> > >> memory=mmap
> > >> [ +0.000382] video2: VIDIOC_REQBUFS: count=0, type=vid-cap-mplane,
> > >> memory=mmap
> > >> [ +0.000227] video2: VIDIOC_S_FMT: type=vid-cap-mplane, width=96,
> > >> height=96, format=H264, field=none, colorspace=0, num_planes=1,
> > >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> > >> [ +0.000027] plane 0: bytesperline=0 sizeimage=2097152
> > >> [ +0.000527] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> > >> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> > >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> > >> [ +0.000024] plane 0: bytesperline=384 sizeimage=122880
> > >> [ +0.000032] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> > >> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> > >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> > >> [ +0.000017] plane 0: bytesperline=384 sizeimage=122880
> > >> [ +0.000024] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> > >> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> > >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> > >> [ +0.000017] plane 0: bytesperline=384 sizeimage=122880
> > >> [ +0.000075] video2: VIDIOC_S_SELECTION: type=vid-out, target=0,
> > >> flags=0x0, wxh=320x192, x,y=0,0
> > >> [ +0.028300] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> > >> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> > >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> > >> [ +0.000036] plane 0: bytesperline=384 sizeimage=122880
> > >> [ +0.000033] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> > >> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> > >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> > >> [ +0.000014] plane 0: bytesperline=384 sizeimage=122880
> > >> [ +0.000020] video2: VIDIOC_S_FMT: type=vid-out-mplane, width=384,
> > >> height=192, format=NV12, field=none, colorspace=0, num_planes=1,
> > >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> > >> [ +0.000120] plane 0: bytesperline=384 sizeimage=122880
> > >> [ +0.000157] video2: VIDIOC_S_SELECTION: type=vid-out, target=0,
> > >> flags=0x0, wxh=320x192, x,y=0,0
> > >> [ +0.000250] video2: VIDIOC_S_EXT_CTRLS: which=0x990000, count=1,
> > >> error_idx=0, request_fd=0, id/val=0x9909d7/0x1
> > >> [ +0.000120] video2: VIDIOC_QUERYCTRL: error -22: id=0x990b84,
> > >> type=0, name=, min/max=0/0, step=0, default=0, flags=0x00000000
> > >> [ +0.000368] v4l2-ctrls: try_set_ext_ctrls: video2: video2:
> > >> try_set_ext_ctrls_common failed (-22)
> > >> [ +0.000087] video2: VIDIOC_S_EXT_CTRLS: error -22: which=0x990000,
> > >> count=5, error_idx=5, request_fd=0, id/val=0x9909ca/0x0,
> > >> id/val=0x990a62/0x33, id/val=0x990a6b/0x2, id/val=0x990a67/0xb,
> > >> id/val=0x9909d8/0x1
> > >> [ +0.000290] v4l2-ctrls: prepare_ext_ctrls: video2: cannot find
> > >> control id 0x9909da
> > >> [ +0.000010] v4l2-ctrls: try_set_ext_ctrls: video2: video2:
> > >> try_set_ext_ctrls_common failed (-22)
> > >> [ +0.000014] video2: VIDIOC_S_EXT_CTRLS: error -22: which=0x990000,
> > >> count=2, error_idx=2, request_fd=0, id/val=0x9909da/0x1,
> > >> id/val=0x9909cb/0x0
> > >> [ +0.000225] video2: VIDIOC_G_FMT: type=vid-cap-mplane, width=320,
> > >> height=192, format=H264, field=none, colorspace=0, num_planes=1,
> > >> flags=0x0, ycbcr_enc=0, quantization=0, xfer_func=0
> > >> [ +0.000028] plane 0: bytesperline=0 sizeimage=49152
> > >> [ +0.002272] video2: VIDIOC_REQBUFS: count=4, type=vid-cap-mplane,
> > >> memory=mmap
> > >> [ +0.001661] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=0,
> > >> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
> > >> sequence=0, memory=mmap
> > >> [ +0.000034] plane 0: bytesused=0, data_offset=0x00000000,
> > >> offset/userptr=0xee18ad4840000000, length=2097152
> > >> [ +0.000009] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
> > >> userbits=0x00000000
> > >> [ +0.000142] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=1,
> > >> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
> > >> sequence=0, memory=mmap
> > >> [ +0.000023] plane 0: bytesused=0, data_offset=0x00000000,
> > >> offset/userptr=0xee18ad4840200000, length=2097152
> > >> [ +0.000008] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
> > >> userbits=0x00000000
> > >> [ +0.000409] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=2,
> > >> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
> > >> sequence=0, memory=mmap
> > >> [ +0.000027] plane 0: bytesused=0, data_offset=0x00000000,
> > >> offset/userptr=0xee18ad4840400000, length=2097152
> > >> [ +0.000007] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
> > >> userbits=0x00000000
> > >> [ +0.000233] video2: VIDIOC_QUERYBUF: 00:00:00.00000000 index=3,
> > >> type=vid-cap-mplane, request_fd=0, flags=0x00004000, field=any,
> > >> sequence=0, memory=mmap
> > >> [ +0.000023] plane 0: bytesused=0, data_offset=0x00000000,
> > >> offset/userptr=0xee18ad4840600000, length=2097152
> > >> [ +0.000015] timecode=00:00:00 type=0, flags=0x00000000, frames=0,
> > >> userbits=0x00000000
> > >> [ +0.000288] video2: VIDIOC_S_EXT_CTRLS: which=0x990000, count=1,
> > >> error_idx=0, request_fd=0, id/val=0x9909cf/0x30d40
> > >> [ +0.000184] video2: VIDIOC_S_PARM: type=vid-out-mplane,
> > >> capability=0x1000, outputmode=0x0, timeperframe=1/30, extendedmode=0,
> > >> writebuffers=0
> > >> [ +0.310832] qcom-venus-encoder aa00000.video-codec:video-encoder:
> > >> VenusHigh: wrong state!
> > >>
> > >>> + INIT_LIST_HEAD(&inst->registeredbufs);
> > >>> +
> > >>> + inst->enc_state = VENUS_ENC_STATE_INIT;
> > >>> +
> > >>> + 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;
> > >>> @@ -881,61 +945,73 @@ 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);
> > >>> - int ret;
> > >>> + int ret = 0;
> > >>>
> > >>> mutex_lock(&inst->lock);
> > >>>
> > >>> - if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> > >>> + 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))
> > >>> + goto unlock;
> > >>>
> > >>> - venus_helper_init_instance(inst);
> > >>> + if (inst->enc_state == VENUS_ENC_STATE_INIT) {
> > >>> + 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_set_properties(inst);
> > >>> - if (ret)
> > >>> - goto deinit_sess;
> > >>> + ret = venc_verify_conf(inst);
> > >>> + if (ret)
> > >>> + goto deinit_sess;
> > >>>
> > >>> - ret = venc_verify_conf(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 = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
> > >>> - inst->num_output_bufs, 0);
> > >>> - if (ret)
> > >>> - goto deinit_sess;
> > >>> + ret = venus_helper_vb2_start_streaming(inst);
> > >>> + if (ret)
> > >>> + goto deinit_sess;
> > >>>
> > >>> - 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);
> > >>>
> > >>> - inst->enc_state = VENUS_ENC_STATE_ENCODING;
> > >>> + inst->enc_state = VENUS_ENC_STATE_ENCODING;
> > >>> + } else if (inst->enc_state == VENUS_ENC_STATE_RESET &&
> > >>> + V4L2_TYPE_IS_CAPTURE(q->type)) {
> > >>> + ret = venus_helper_vb2_start_streaming(inst);
> > >>> + if (ret)
> > >>> + goto bufs_done;
> > >>>
> > >>> - mutex_unlock(&inst->lock);
> > >>> + venus_helper_process_initial_out_bufs(inst);
> > >>> + venus_helper_process_initial_cap_bufs(inst);
> > >>>
> > >>> - return 0;
> > >>> + inst->enc_state = VENUS_ENC_STATE_ENCODING;
> > >>> + } else if (inst->enc_state == VENUS_ENC_STATE_STOPPED &&
> > >>> + V4L2_TYPE_IS_OUTPUT(q->type)) {
> > >>> + inst->enc_state = VENUS_ENC_STATE_ENCODING;
> > >>> + }
> > >>> +
> > >>> +unlock:
> > >>> + mutex_unlock(&inst->lock);
> > >>> + return ret;
> > >>>
> > >>> deinit_sess:
> > >>> 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;
> > >>> @@ -943,33 +1019,97 @@ static int venc_start_streaming(struct
> > >>> vb2_queue *q, unsigned int count)
> > >>> return ret;
> > >>> }
> > >>>
> > >>> -static void venc_vb2_buf_queue(struct vb2_buffer *vb)
> > >>> +static int venc_stop_capture(struct venus_inst *inst)
> > >>> +{
> > >>> + int ret;
> > >>> +
> > >>> + switch (inst->enc_state) {
> > >>> + case VENUS_ENC_STATE_ENCODING:
> > >>> + case VENUS_ENC_STATE_DRAIN:
> > >>> + case VENUS_ENC_STATE_STOPPED:
> > >>> + inst->enc_state = VENUS_ENC_STATE_RESET;
> > >>> + break;
> > >>> + default:
> > >>> + return -EINVAL;
> > >>> + }
> > >>> +
> > >>> + ret = hfi_session_stop(inst);
> > >>> + ret |= hfi_session_unload_res(inst);
> > >>> + ret |= venus_helper_unregister_bufs(inst);
> > >>> + ret |= venus_helper_intbufs_free(inst);
> > >>> +
> > >>> + return ret;
> > >>> +}
> > >>> +
> > >>> +static int venc_stop_output(struct venus_inst *inst)
> > >>> +{
> > >>> + switch (inst->enc_state) {
> > >>> + case VENUS_ENC_STATE_ENCODING:
> > >>> + inst->enc_state = VENUS_ENC_STATE_STOPPED;
> > >>> + break;
> > >>> + case VENUS_ENC_STATE_DRAIN:
> > >>> + inst->enc_state = VENUS_ENC_STATE_STOPPED;
> > >>> + break;
> > >>> + default:
> > >>> + return -EINVAL;
> > >>> + }
> > >>> +
> > >>> + return 0;
> > >>> +}
> > >>> +
> > >>> +static void venc_stop_streaming(struct vb2_queue *q)
> > >>> +{
> > >>> + struct venus_inst *inst = vb2_get_drv_priv(q);
> > >>> + int ret = -EINVAL;
> > >>> +
> > >>> + mutex_lock(&inst->lock);
> > >>> +
> > >>> + if (V4L2_TYPE_IS_OUTPUT(q->type))
> > >>> + ret = venc_stop_output(inst);
> > >>> + else
> > >>> + ret = venc_stop_capture(inst);
> > >>> +
> > >>> + venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
> > >>> +
> > >>> + if (ret)
> > >>> + goto unlock;
> > >>> +
> > >>> + if (V4L2_TYPE_IS_OUTPUT(q->type))
> > >>> + inst->streamon_out = 0;
> > >>> + else
> > >>> + inst->streamon_cap = 0;
> > >>> +
> > >>> +unlock:
> > >>> + 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);
> > >>> + struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
> > >>>
> > >>> 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;
> > >>> - }
> > >>> + v4l2_m2m_buf_queue(m2m_ctx, vbuf);
> > >>> +
> > >>> + if (!(inst->streamon_out && inst->streamon_cap))
> > >>> + goto unlock;
> > >>> +
> > >>> + venus_helper_process_buf(vb);
> > >>>
> > >>> - venus_helper_vb2_buf_queue(vb);
> > >>> +unlock:
> > >>> mutex_unlock(&inst->lock);
> > >>> }
> > >>>
> > >>> 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,
> > >>> @@ -1025,8 +1165,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,
> > >>> };
> > >>>
> > >>> @@ -1098,11 +1242,9 @@ static int venc_open(struct file *file)
> > >>> inst->core = core;
> > >>> inst->session_type = VIDC_SESSION_TYPE_ENC;
> > >>> inst->clk_data.core_id = VIDC_CORE_ID_DEFAULT;
> > >>> + inst->enc_state = VENUS_ENC_STATE_INIT;
> > >>> 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);
> > >>> @@ -1167,7 +1309,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);
> > >>> --
> > >>> 2.17.1
> > >>>
> > >
> > > Thanks,
> > > Vikash
> >
> > --
> > regards,
> > Stan