From 63342afea65e9efa06889775c27001745e157770 Mon Sep 17 00:00:00 2001 From: Stanimir Varbanov Date: Thu, 30 Jan 2020 16:44:24 +0100 Subject: media: venus: vdec: Use pmruntime autosuspend Implement pmruntime autosuspend in video decoder. This will allow to save power while the userspace is inactive for some reasonable period of time. Here we power-off venus core clocks and power domain and don't touch vcodec because it is under hardware control. The later decision is made to simplify the code and avoid a mess in the power management code. Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/qcom/venus/core.c | 3 + drivers/media/platform/qcom/venus/core.h | 2 + drivers/media/platform/qcom/venus/vdec.c | 134 ++++++++++++++++++++++++++----- 3 files changed, 119 insertions(+), 20 deletions(-) (limited to 'drivers/media/platform/qcom') diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index 194b10b98767..12688f04d32c 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -210,6 +210,8 @@ static int venus_probe(struct platform_device *pdev) if (!core->res) return -ENODEV; + mutex_init(&core->pm_lock); + core->pm_ops = venus_pm_get(core->res->hfi_version); if (!core->pm_ops) return -ENODEV; @@ -336,6 +338,7 @@ static int venus_remove(struct platform_device *pdev) icc_put(core->cpucfg_path); v4l2_device_unregister(&core->v4l2_dev); + mutex_destroy(&core->pm_lock); return ret; } diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h index bd3ac6a4501f..3ab644a39786 100644 --- a/drivers/media/platform/qcom/venus/core.h +++ b/drivers/media/platform/qcom/venus/core.h @@ -128,6 +128,7 @@ struct venus_caps { * @error: an error returned during last HFI sync operations * @sys_error: an error flag that signal system error event * @core_ops: the core operations + * @pm_lock: a lock for PM operations * @enc_codecs: encoders supported by this core * @dec_codecs: decoders supported by this core * @max_sessions_supported: holds the maximum number of sessions @@ -168,6 +169,7 @@ struct venus_core { bool sys_error; const struct hfi_core_ops *core_ops; const struct venus_pm_ops *pm_ops; + struct mutex pm_lock; unsigned long enc_codecs; unsigned long dec_codecs; unsigned int max_sessions_supported; diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c index 4ed2628585a1..e8e1ecf7cf4a 100644 --- a/drivers/media/platform/qcom/venus/vdec.c +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -545,6 +545,64 @@ static const struct v4l2_ioctl_ops vdec_ioctl_ops = { .vidioc_decoder_cmd = vdec_decoder_cmd, }; +static int vdec_pm_get(struct venus_inst *inst) +{ + struct venus_core *core = inst->core; + struct device *dev = core->dev_dec; + int ret; + + mutex_lock(&core->pm_lock); + ret = pm_runtime_get_sync(dev); + mutex_unlock(&core->pm_lock); + + return ret < 0 ? ret : 0; +} + +static int vdec_pm_put(struct venus_inst *inst, bool autosuspend) +{ + struct venus_core *core = inst->core; + struct device *dev = core->dev_dec; + int ret; + + mutex_lock(&core->pm_lock); + + if (autosuspend) + ret = pm_runtime_put_autosuspend(dev); + else + ret = pm_runtime_put_sync(dev); + + mutex_unlock(&core->pm_lock); + + return ret < 0 ? ret : 0; +} + +static int vdec_pm_get_put(struct venus_inst *inst) +{ + struct venus_core *core = inst->core; + struct device *dev = core->dev_dec; + int ret = 0; + + mutex_lock(&core->pm_lock); + + if (pm_runtime_suspended(dev)) { + ret = pm_runtime_get_sync(dev); + if (ret < 0) + goto error; + + ret = pm_runtime_put_autosuspend(dev); + } + +error: + mutex_unlock(&core->pm_lock); + + return ret < 0 ? ret : 0; +} + +static void vdec_pm_touch(struct venus_inst *inst) +{ + pm_runtime_mark_last_busy(inst->core->dev_dec); +} + static int vdec_set_properties(struct venus_inst *inst) { struct vdec_controls *ctr = &inst->controls.dec; @@ -746,11 +804,19 @@ static int vdec_queue_setup(struct vb2_queue *q, return 0; } - ret = vdec_session_init(inst); + ret = vdec_pm_get(inst); if (ret) return ret; + ret = vdec_session_init(inst); + if (ret) + goto put_power; + ret = vdec_num_buffers(inst, &in_num, &out_num); + if (ret) + goto put_power; + + ret = vdec_pm_put(inst, false); if (ret) return ret; @@ -786,6 +852,10 @@ static int vdec_queue_setup(struct vb2_queue *q, } return ret; + +put_power: + vdec_pm_put(inst, false); + return ret; } static int vdec_verify_conf(struct venus_inst *inst) @@ -947,14 +1017,23 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count) mutex_lock(&inst->lock); - ret = venus_pm_acquire_core(inst); - if (ret) - goto error; - - if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) + if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { ret = vdec_start_capture(inst); - else + } else { + ret = vdec_pm_get(inst); + if (ret) + goto error; + + ret = venus_pm_acquire_core(inst); + if (ret) + goto put_power; + + ret = vdec_pm_put(inst, true); + if (ret) + goto error; + ret = vdec_start_output(inst); + } if (ret) goto error; @@ -962,6 +1041,8 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count) mutex_unlock(&inst->lock); return 0; +put_power: + vdec_pm_put(inst, false); error: venus_helper_buffers_done(inst, VB2_BUF_STATE_QUEUED); mutex_unlock(&inst->lock); @@ -1055,8 +1136,9 @@ static void vdec_session_release(struct venus_inst *inst) struct venus_core *core = inst->core; int ret, abort = 0; - mutex_lock(&inst->lock); + vdec_pm_get(inst); + mutex_lock(&inst->lock); inst->codec_state = VENUS_DEC_STATE_DEINIT; ret = hfi_session_stop(inst); @@ -1078,10 +1160,11 @@ static void vdec_session_release(struct venus_inst *inst) venus_helper_free_dpb_bufs(inst); venus_pm_load_scale(inst); - venus_pm_release_core(inst); INIT_LIST_HEAD(&inst->registeredbufs); - mutex_unlock(&inst->lock); + + venus_pm_release_core(inst); + vdec_pm_put(inst, false); } static int vdec_buf_init(struct vb2_buffer *vb) @@ -1102,6 +1185,15 @@ static void vdec_buf_cleanup(struct vb2_buffer *vb) vdec_session_release(inst); } +static void vdec_vb2_buf_queue(struct vb2_buffer *vb) +{ + struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue); + + vdec_pm_get_put(inst); + + venus_helper_vb2_buf_queue(vb); +} + static const struct vb2_ops vdec_vb2_ops = { .queue_setup = vdec_queue_setup, .buf_init = vdec_buf_init, @@ -1109,7 +1201,7 @@ static const struct vb2_ops vdec_vb2_ops = { .buf_prepare = venus_helper_vb2_buf_prepare, .start_streaming = vdec_start_streaming, .stop_streaming = vdec_stop_streaming, - .buf_queue = venus_helper_vb2_buf_queue, + .buf_queue = vdec_vb2_buf_queue, }; static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type, @@ -1121,6 +1213,8 @@ static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type, struct vb2_buffer *vb; unsigned int type; + vdec_pm_touch(inst); + if (buf_type == HFI_BUFFER_INPUT) type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE; else @@ -1227,6 +1321,8 @@ static void vdec_event_notify(struct venus_inst *inst, u32 event, struct venus_core *core = inst->core; struct device *dev = core->dev_dec; + vdec_pm_touch(inst); + switch (event) { case EVT_SESSION_ERROR: inst->session_error = true; @@ -1347,13 +1443,9 @@ static int vdec_open(struct file *file) init_waitqueue_head(&inst->reconf_wait); venus_helper_init_instance(inst); - ret = pm_runtime_get_sync(core->dev_dec); - if (ret < 0) - goto err_free_inst; - ret = vdec_ctrl_init(inst); if (ret) - goto err_put_sync; + goto err_free; ret = hfi_session_create(inst, &vdec_hfi_ops); if (ret) @@ -1392,9 +1484,7 @@ err_session_destroy: hfi_session_destroy(inst); err_ctrl_deinit: vdec_ctrl_deinit(inst); -err_put_sync: - pm_runtime_put_sync(core->dev_dec); -err_free_inst: +err_free: kfree(inst); return ret; } @@ -1403,6 +1493,8 @@ static int vdec_close(struct file *file) { struct venus_inst *inst = to_inst(file); + vdec_pm_get(inst); + v4l2_m2m_ctx_release(inst->m2m_ctx); v4l2_m2m_release(inst->m2m_dev); vdec_ctrl_deinit(inst); @@ -1411,7 +1503,7 @@ static int vdec_close(struct file *file) v4l2_fh_del(&inst->fh); v4l2_fh_exit(&inst->fh); - pm_runtime_put_sync(inst->core->dev_dec); + vdec_pm_put(inst, false); kfree(inst); return 0; @@ -1468,6 +1560,8 @@ static int vdec_probe(struct platform_device *pdev) core->dev_dec = dev; video_set_drvdata(vdev, core); + pm_runtime_set_autosuspend_delay(dev, 2000); + pm_runtime_use_autosuspend(dev); pm_runtime_enable(dev); return 0; -- cgit v1.2.3 From 380f3bbd9562dc93be2e3cadc329b15284fbedae Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Thu, 19 Mar 2020 23:21:05 +0100 Subject: media: venus: hfi_cmds.h: Replace zero-length array with flexible-array member The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99: struct foo { int stuff; struct boo array[]; }; By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being inadvertently introduced[3] to the codebase from now on. Also, notice that, dynamic memory allocations won't be affected by this change: "Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1] This issue was found with the help of Coccinelle. [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html [2] https://github.com/KSPP/linux/issues/21 [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour") Signed-off-by: Gustavo A. R. Silva Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/qcom/venus/hfi_cmds.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/media/platform/qcom') diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.h b/drivers/media/platform/qcom/venus/hfi_cmds.h index cae9d5d61c0c..83705e237f1c 100644 --- a/drivers/media/platform/qcom/venus/hfi_cmds.h +++ b/drivers/media/platform/qcom/venus/hfi_cmds.h @@ -107,7 +107,7 @@ struct hfi_session_abort_pkt { struct hfi_session_set_property_pkt { struct hfi_session_hdr_pkt shdr; u32 num_properties; - u32 data[0]; + u32 data[]; }; struct hfi_session_set_buffers_pkt { -- cgit v1.2.3 From 0f61e171e4bbac4595175070c75707f1b12f4e37 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Thu, 19 Mar 2020 23:22:29 +0100 Subject: media: venus: hfi_msgs.h: Replace zero-length array with flexible-array member The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member[1][2], introduced in C99: struct foo { int stuff; struct boo array[]; }; By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being inadvertently introduced[3] to the codebase from now on. Also, notice that, dynamic memory allocations won't be affected by this change: "Flexible array members have incomplete type, and so the sizeof operator may not be applied. As a quirk of the original implementation of zero-length arrays, sizeof evaluates to zero."[1] This issue was found with the help of Coccinelle. [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html [2] https://github.com/KSPP/linux/issues/21 [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour") Signed-off-by: Gustavo A. R. Silva Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/qcom/venus/hfi_msgs.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/media/platform/qcom') diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.h b/drivers/media/platform/qcom/venus/hfi_msgs.h index 7694b1d25d9d..526d9f5b487b 100644 --- a/drivers/media/platform/qcom/venus/hfi_msgs.h +++ b/drivers/media/platform/qcom/venus/hfi_msgs.h @@ -155,7 +155,7 @@ struct hfi_msg_session_empty_buffer_done_pkt { u32 input_tag; u32 packet_buffer; u32 extradata_buffer; - u32 data[0]; + u32 data[]; }; struct hfi_msg_session_fbd_compressed_pkt { @@ -175,7 +175,7 @@ struct hfi_msg_session_fbd_compressed_pkt { u32 picture_type; u32 packet_buffer; u32 extradata_buffer; - u32 data[0]; + u32 data[]; }; struct hfi_msg_session_fbd_uncompressed_plane0_pkt { @@ -202,7 +202,7 @@ struct hfi_msg_session_fbd_uncompressed_plane0_pkt { u32 picture_type; u32 packet_buffer; u32 extradata_buffer; - u32 data[0]; + u32 data[]; }; struct hfi_msg_session_fbd_uncompressed_plane1_pkt { @@ -211,7 +211,7 @@ struct hfi_msg_session_fbd_uncompressed_plane1_pkt { u32 filled_len; u32 offset; u32 packet_buffer2; - u32 data[0]; + u32 data[]; }; struct hfi_msg_session_fbd_uncompressed_plane2_pkt { @@ -220,7 +220,7 @@ struct hfi_msg_session_fbd_uncompressed_plane2_pkt { u32 filled_len; u32 offset; u32 packet_buffer3; - u32 data[0]; + u32 data[]; }; struct hfi_msg_session_parse_sequence_header_done_pkt { -- cgit v1.2.3 From 07f8f22a33a9e3e9955e24a84e2f856dcc8c31c4 Mon Sep 17 00:00:00 2001 From: Mansur Alisha Shaik Date: Fri, 10 Apr 2020 09:17:25 +0200 Subject: media: venus: core: remove CNOC voting while device suspend The Venus driver is voting Configuration NoC during .probe but not clear voting in .suspend. Because of this NoC is up during shutdown also. As a consequence the whole device could leak energy while in .suspend. So correct this by moving voting in .resume and unvoting in .suspend Signed-off-by: Mansur Alisha Shaik Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/qcom/venus/core.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'drivers/media/platform/qcom') diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index 12688f04d32c..4395cb96fb04 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -244,10 +244,6 @@ static int venus_probe(struct platform_device *pdev) if (ret) return ret; - ret = icc_set_bw(core->cpucfg_path, 0, kbps_to_icc(1000)); - if (ret) - return ret; - ret = hfi_create(core, &venus_core_ops); if (ret) return ret; @@ -353,6 +349,10 @@ static __maybe_unused int venus_runtime_suspend(struct device *dev) if (ret) return ret; + ret = icc_set_bw(core->cpucfg_path, 0, 0); + if (ret) + return ret; + if (pm_ops->core_power) ret = pm_ops->core_power(dev, POWER_OFF); @@ -371,6 +371,10 @@ static __maybe_unused int venus_runtime_resume(struct device *dev) return ret; } + ret = icc_set_bw(core->cpucfg_path, 0, kbps_to_icc(1000)); + if (ret) + return ret; + return hfi_core_resume(core, false); } -- cgit v1.2.3 From cb1c05c89b1f0b5ee24b65fbad3811127f3205d0 Mon Sep 17 00:00:00 2001 From: Stanimir Varbanov Date: Tue, 31 Mar 2020 16:54:28 +0200 Subject: media: venus: core: Add missing mutex destroy This adds missing mutex_destroy in remove method of venus core driver. Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/qcom/venus/core.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/media/platform/qcom') diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index 4395cb96fb04..f8b9a732bc65 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -335,6 +335,7 @@ static int venus_remove(struct platform_device *pdev) v4l2_device_unregister(&core->v4l2_dev); mutex_destroy(&core->pm_lock); + mutex_destroy(&core->lock); return ret; } -- cgit v1.2.3 From 82223aa54bb7075e9bf24b64720c18361ed190a1 Mon Sep 17 00:00:00 2001 From: Stanimir Varbanov Date: Tue, 31 Mar 2020 17:37:02 +0200 Subject: media: venus: core: Fix mutex destroy in remove The hfi_destroy function is called too early in remove method. It destroys a mutex which is used later in the .remove from pmruntime. Solve the issue by moving hfi_destroy after last usage of the mutex. Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/qcom/venus/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/media/platform/qcom') diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index f8b9a732bc65..afd76bcd9978 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -318,7 +318,6 @@ static int venus_remove(struct platform_device *pdev) ret = hfi_core_deinit(core, true); WARN_ON(ret); - hfi_destroy(core); venus_shutdown(core); of_platform_depopulate(dev); @@ -330,6 +329,8 @@ static int venus_remove(struct platform_device *pdev) if (pm_ops->core_put) pm_ops->core_put(dev); + hfi_destroy(core); + icc_put(core->video_path); icc_put(core->cpucfg_path); -- cgit v1.2.3 From 18cf8ba1d3e6faf8db8c6e49f4014b657ac96888 Mon Sep 17 00:00:00 2001 From: Stanimir Varbanov Date: Tue, 31 Mar 2020 17:42:52 +0200 Subject: media: venus: core: Constify codec frequency data array The array is not changed in the code, so make it const. Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/qcom/venus/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/media/platform/qcom') diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index afd76bcd9978..203c6538044f 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -456,7 +456,7 @@ static const struct freq_tbl sdm845_freq_table[] = { { 244800, 100000000 }, /* 1920x1080@30 */ }; -static struct codec_freq_data sdm845_codec_freq_data[] = { +static const struct codec_freq_data sdm845_codec_freq_data[] = { { V4L2_PIX_FMT_H264, VIDC_SESSION_TYPE_ENC, 675, 10 }, { V4L2_PIX_FMT_HEVC, VIDC_SESSION_TYPE_ENC, 675, 10 }, { V4L2_PIX_FMT_VP8, VIDC_SESSION_TYPE_ENC, 675, 10 }, -- cgit v1.2.3 From 0febf9236970b5282588147961b068b889a77563 Mon Sep 17 00:00:00 2001 From: Stanimir Varbanov Date: Tue, 31 Mar 2020 17:47:38 +0200 Subject: media: venus: helpers: Done buffers per queue type Currently calling venus_helper_buffers_done() will return buffers to user for both capture and output queues in the same call. This is wrong because both queues are really separate and calling stop_streaming on one queue shouldn't return buffers for the other. Solve this by add a new queue type argument and fix the clients of the helper function. Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/qcom/venus/helpers.c | 18 ++++++++++++------ drivers/media/platform/qcom/venus/helpers.h | 2 +- drivers/media/platform/qcom/venus/vdec.c | 5 ++--- drivers/media/platform/qcom/venus/venc.c | 2 +- 4 files changed, 16 insertions(+), 11 deletions(-) (limited to 'drivers/media/platform/qcom') diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c index bcc603804041..0143af7822b2 100644 --- a/drivers/media/platform/qcom/venus/helpers.c +++ b/drivers/media/platform/qcom/venus/helpers.c @@ -1129,15 +1129,18 @@ unlock: } EXPORT_SYMBOL_GPL(venus_helper_vb2_buf_queue); -void venus_helper_buffers_done(struct venus_inst *inst, +void venus_helper_buffers_done(struct venus_inst *inst, unsigned int type, enum vb2_buffer_state state) { struct vb2_v4l2_buffer *buf; - while ((buf = v4l2_m2m_src_buf_remove(inst->m2m_ctx))) - v4l2_m2m_buf_done(buf, state); - while ((buf = v4l2_m2m_dst_buf_remove(inst->m2m_ctx))) - v4l2_m2m_buf_done(buf, state); + if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) { + while ((buf = v4l2_m2m_src_buf_remove(inst->m2m_ctx))) + v4l2_m2m_buf_done(buf, state); + } else if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) { + while ((buf = v4l2_m2m_dst_buf_remove(inst->m2m_ctx))) + v4l2_m2m_buf_done(buf, state); + } } EXPORT_SYMBOL_GPL(venus_helper_buffers_done); @@ -1168,7 +1171,10 @@ void venus_helper_vb2_stop_streaming(struct vb2_queue *q) INIT_LIST_HEAD(&inst->registeredbufs); } - venus_helper_buffers_done(inst, VB2_BUF_STATE_ERROR); + venus_helper_buffers_done(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, + VB2_BUF_STATE_ERROR); + venus_helper_buffers_done(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, + VB2_BUF_STATE_ERROR); if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) inst->streamon_out = 0; diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h index b64875564064..8fbbda12a4fe 100644 --- a/drivers/media/platform/qcom/venus/helpers.h +++ b/drivers/media/platform/qcom/venus/helpers.h @@ -14,7 +14,7 @@ struct venus_core; bool venus_helper_check_codec(struct venus_inst *inst, u32 v4l2_pixfmt); struct vb2_v4l2_buffer *venus_helper_find_buf(struct venus_inst *inst, unsigned int type, u32 idx); -void venus_helper_buffers_done(struct venus_inst *inst, +void venus_helper_buffers_done(struct venus_inst *inst, unsigned int type, enum vb2_buffer_state state); int venus_helper_vb2_buf_init(struct vb2_buffer *vb); int venus_helper_vb2_buf_prepare(struct vb2_buffer *vb); diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c index e8e1ecf7cf4a..7d093accbd59 100644 --- a/drivers/media/platform/qcom/venus/vdec.c +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -1044,7 +1044,7 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count) put_power: vdec_pm_put(inst, false); error: - venus_helper_buffers_done(inst, VB2_BUF_STATE_QUEUED); + venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_QUEUED); mutex_unlock(&inst->lock); return ret; } @@ -1071,7 +1071,6 @@ static int vdec_stop_capture(struct venus_inst *inst) break; case VENUS_DEC_STATE_DRC: ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT); - vdec_cancel_dst_buffers(inst); inst->codec_state = VENUS_DEC_STATE_CAPTURE_SETUP; INIT_LIST_HEAD(&inst->registeredbufs); venus_helper_free_dpb_bufs(inst); @@ -1117,7 +1116,7 @@ static void vdec_stop_streaming(struct vb2_queue *q) else ret = vdec_stop_output(inst); - venus_helper_buffers_done(inst, VB2_BUF_STATE_ERROR); + venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR); if (ret) goto unlock; diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c index 9981a2a27c90..3d8431dc14c4 100644 --- a/drivers/media/platform/qcom/venus/venc.c +++ b/drivers/media/platform/qcom/venus/venc.c @@ -1018,7 +1018,7 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count) deinit_sess: hfi_session_deinit(inst); bufs_done: - venus_helper_buffers_done(inst, VB2_BUF_STATE_QUEUED); + venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_QUEUED); if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) inst->streamon_out = 0; else -- cgit v1.2.3 From 51df3c81ba10b00f4da81225310a3503b0583062 Mon Sep 17 00:00:00 2001 From: Stanimir Varbanov Date: Tue, 31 Mar 2020 18:19:51 +0200 Subject: media: venus: vdec: Mark flushed buffers with error state Once the hfi_session_flush is issued by the vdec all queued buffers to firmware should be returned to the v4l driver. Some of those buffers are not processed at the time of flush command, those buffers has filled len zero (no data). Catch that in buffer_done callback and mark not filled capture buffers with error state so that client can discard them. Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/qcom/venus/vdec.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/media/platform/qcom') diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c index 7d093accbd59..5823537b3131 100644 --- a/drivers/media/platform/qcom/venus/vdec.c +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -1241,6 +1241,9 @@ static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type, if (inst->codec_state == VENUS_DEC_STATE_DRAIN) inst->codec_state = VENUS_DEC_STATE_STOPPED; } + + if (!bytesused) + state = VB2_BUF_STATE_ERROR; } else { vbuf->sequence = inst->sequence_out++; } -- cgit v1.2.3 From bc3d870e414b42d72cd386aa20a4fc3612e4feb7 Mon Sep 17 00:00:00 2001 From: Stanimir Varbanov Date: Fri, 3 Apr 2020 15:10:13 +0200 Subject: media: venus: vdec: Init registered list unconditionally Presently the list initialization is done only in dynamic-resolution-change state, which leads to list corruptions and use-after-free. Init list_head unconditionally in vdec_stop_capture called by vb2 stop_streaming without takeing into account current codec state. Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/qcom/venus/vdec.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/media/platform/qcom') diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c index 5823537b3131..f23cbd812ef4 100644 --- a/drivers/media/platform/qcom/venus/vdec.c +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -1072,13 +1072,14 @@ static int vdec_stop_capture(struct venus_inst *inst) case VENUS_DEC_STATE_DRC: ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT); inst->codec_state = VENUS_DEC_STATE_CAPTURE_SETUP; - INIT_LIST_HEAD(&inst->registeredbufs); venus_helper_free_dpb_bufs(inst); break; default: - return 0; + break; } + INIT_LIST_HEAD(&inst->registeredbufs); + return ret; } -- cgit v1.2.3 From 85872f861d4cc535b0dbfd0a9062bdbdcbed20c3 Mon Sep 17 00:00:00 2001 From: Stanimir Varbanov Date: Fri, 3 Apr 2020 17:56:29 +0200 Subject: media: venus: Mark last capture buffer According to stateful Codec API the decoder will process all remaining buffers from before the source change event in dynamic-resolution-change state and mark the last buffer with V4L2_BUF_FLAG_LAST. In Venus case the firmware doesn't mark that last buffer and some mechanism have to be created in v4l decoder driver. Fortunately the firmware interface (HFI) claims that the decoder output buffers will be returned to v4l decoder driver before it send the insufficient event. In order to do that we save last queued in the driver capture buffer in the event_notify and issue flush on output firmware buffers queue. Once the saved buffer is returned (as a result of flush command) we mark it as LAST. For all that possible we extend HFI flush command with one more argument and one more flush_done HFI driver callback. Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/qcom/venus/core.h | 5 +++- drivers/media/platform/qcom/venus/hfi.c | 10 ++++--- drivers/media/platform/qcom/venus/hfi.h | 3 +- drivers/media/platform/qcom/venus/hfi_msgs.c | 2 ++ drivers/media/platform/qcom/venus/vdec.c | 45 ++++++++++++++++++++++++---- 5 files changed, 54 insertions(+), 11 deletions(-) (limited to 'drivers/media/platform/qcom') diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h index 3ab644a39786..7118612673c9 100644 --- a/drivers/media/platform/qcom/venus/core.h +++ b/drivers/media/platform/qcom/venus/core.h @@ -261,7 +261,8 @@ enum venus_dec_state { VENUS_DEC_STATE_SEEK = 4, VENUS_DEC_STATE_DRAIN = 5, VENUS_DEC_STATE_DECODING = 6, - VENUS_DEC_STATE_DRC = 7 + VENUS_DEC_STATE_DRC = 7, + VENUS_DEC_STATE_DRC_FLUSH_DONE = 8, }; struct venus_ts_metadata { @@ -326,6 +327,7 @@ struct venus_ts_metadata { * @priv: a private for HFI operations callbacks * @session_type: the type of the session (decoder or encoder) * @hprop: a union used as a holder by get property + * @last_buf: last capture buffer for dynamic-resoluton-change */ struct venus_inst { struct list_head list; @@ -387,6 +389,7 @@ struct venus_inst { union hfi_get_property hprop; unsigned int core_acquired: 1; unsigned int bit_depth; + struct vb2_buffer *last_buf; }; #define IS_V1(core) ((core)->res->hfi_version == HFI_VERSION_1XX) diff --git a/drivers/media/platform/qcom/venus/hfi.c b/drivers/media/platform/qcom/venus/hfi.c index 3d8b1284d1f3..a211eb93e0f9 100644 --- a/drivers/media/platform/qcom/venus/hfi.c +++ b/drivers/media/platform/qcom/venus/hfi.c @@ -382,7 +382,7 @@ int hfi_session_unload_res(struct venus_inst *inst) } EXPORT_SYMBOL_GPL(hfi_session_unload_res); -int hfi_session_flush(struct venus_inst *inst, u32 type) +int hfi_session_flush(struct venus_inst *inst, u32 type, bool block) { const struct hfi_ops *ops = inst->core->ops; int ret; @@ -393,9 +393,11 @@ int hfi_session_flush(struct venus_inst *inst, u32 type) if (ret) return ret; - ret = wait_session_msg(inst); - if (ret) - return ret; + if (block) { + ret = wait_session_msg(inst); + if (ret) + return ret; + } return 0; } diff --git a/drivers/media/platform/qcom/venus/hfi.h b/drivers/media/platform/qcom/venus/hfi.h index 855822c9f39b..62c315291484 100644 --- a/drivers/media/platform/qcom/venus/hfi.h +++ b/drivers/media/platform/qcom/venus/hfi.h @@ -102,6 +102,7 @@ struct hfi_inst_ops { u32 hfi_flags, u64 timestamp_us); void (*event_notify)(struct venus_inst *inst, u32 event, struct hfi_event_data *data); + void (*flush_done)(struct venus_inst *inst); }; struct hfi_ops { @@ -161,7 +162,7 @@ int hfi_session_continue(struct venus_inst *inst); int hfi_session_abort(struct venus_inst *inst); int hfi_session_load_res(struct venus_inst *inst); int hfi_session_unload_res(struct venus_inst *inst); -int hfi_session_flush(struct venus_inst *inst, u32 type); +int hfi_session_flush(struct venus_inst *inst, u32 type, bool block); int hfi_session_set_buffers(struct venus_inst *inst, struct hfi_buffer_desc *bd); int hfi_session_unset_buffers(struct venus_inst *inst, diff --git a/drivers/media/platform/qcom/venus/hfi_msgs.c b/drivers/media/platform/qcom/venus/hfi_msgs.c index 04ef2286efc6..279a9d6fe737 100644 --- a/drivers/media/platform/qcom/venus/hfi_msgs.c +++ b/drivers/media/platform/qcom/venus/hfi_msgs.c @@ -439,6 +439,8 @@ static void hfi_session_flush_done(struct venus_core *core, inst->error = pkt->error_type; complete(&inst->done); + if (inst->ops->flush_done) + inst->ops->flush_done(inst); } static void hfi_session_etb_done(struct venus_core *core, diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c index f23cbd812ef4..527944c822b5 100644 --- a/drivers/media/platform/qcom/venus/vdec.c +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -906,7 +906,7 @@ static int vdec_start_capture(struct venus_inst *inst) return 0; reconfigure: - ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT); + ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT, true); if (ret) return ret; @@ -1063,14 +1063,16 @@ static int vdec_stop_capture(struct venus_inst *inst) switch (inst->codec_state) { case VENUS_DEC_STATE_DECODING: - ret = hfi_session_flush(inst, HFI_FLUSH_ALL); + ret = hfi_session_flush(inst, HFI_FLUSH_ALL, true); /* fallthrough */ case VENUS_DEC_STATE_DRAIN: vdec_cancel_dst_buffers(inst); inst->codec_state = VENUS_DEC_STATE_STOPPED; break; case VENUS_DEC_STATE_DRC: - ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT); + WARN_ON(1); + fallthrough; + case VENUS_DEC_STATE_DRC_FLUSH_DONE: inst->codec_state = VENUS_DEC_STATE_CAPTURE_SETUP; venus_helper_free_dpb_bufs(inst); break; @@ -1091,12 +1093,12 @@ static int vdec_stop_output(struct venus_inst *inst) case VENUS_DEC_STATE_DECODING: case VENUS_DEC_STATE_DRAIN: case VENUS_DEC_STATE_STOPPED: - ret = hfi_session_flush(inst, HFI_FLUSH_ALL); + ret = hfi_session_flush(inst, HFI_FLUSH_ALL, true); inst->codec_state = VENUS_DEC_STATE_SEEK; break; case VENUS_DEC_STATE_INIT: case VENUS_DEC_STATE_CAPTURE_SETUP: - ret = hfi_session_flush(inst, HFI_FLUSH_INPUT); + ret = hfi_session_flush(inst, HFI_FLUSH_INPUT, true); break; default: break; @@ -1234,6 +1236,13 @@ static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type, vb->timestamp = timestamp_us * NSEC_PER_USEC; vbuf->sequence = inst->sequence_cap++; + if (inst->last_buf == vb) { + inst->last_buf = NULL; + vbuf->flags |= V4L2_BUF_FLAG_LAST; + vb2_set_plane_payload(vb, 0, 0); + vb->timestamp = 0; + } + if (vbuf->flags & V4L2_BUF_FLAG_LAST) { const struct v4l2_event ev = { .type = V4L2_EVENT_EOS }; @@ -1311,6 +1320,25 @@ static void vdec_event_change(struct venus_inst *inst, } } + /* + * The assumption is that the firmware have to return the last buffer + * before this event is received in the v4l2 driver. Also the firmware + * itself doesn't mark the last decoder output buffer with HFI EOS flag. + */ + + if (!sufficient && inst->codec_state == VENUS_DEC_STATE_DRC) { + struct vb2_v4l2_buffer *last; + int ret; + + last = v4l2_m2m_last_dst_buf(inst->m2m_ctx); + if (last) + inst->last_buf = &last->vb2_buf; + + ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT, false); + if (ret) + dev_dbg(dev, "flush output error %d\n", ret); + } + inst->reconfig = true; v4l2_event_queue_fh(&inst->fh, &ev); wake_up(&inst->reconf_wait); @@ -1351,9 +1379,16 @@ static void vdec_event_notify(struct venus_inst *inst, u32 event, } } +static void vdec_flush_done(struct venus_inst *inst) +{ + if (inst->codec_state == VENUS_DEC_STATE_DRC) + inst->codec_state = VENUS_DEC_STATE_DRC_FLUSH_DONE; +} + static const struct hfi_inst_ops vdec_hfi_ops = { .buf_done = vdec_buf_done, .event_notify = vdec_event_notify, + .flush_done = vdec_flush_done, }; static void vdec_inst_init(struct venus_inst *inst) -- cgit v1.2.3 From 4470ff693833ad2cf06d4d07067b2e6a2458c6c1 Mon Sep 17 00:00:00 2001 From: Stanimir Varbanov Date: Sat, 18 Apr 2020 11:06:22 +0200 Subject: media: venus: venc,vdec: Return EBUSY on S_FMT while streaming According to the v4l spec s_fmt must return EBUSY while the particular queue is streaming. Add such check in encoder and decoder s_fmt methods. Signed-off-by: Stanimir Varbanov Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/qcom/venus/vdec.c | 8 ++++++++ drivers/media/platform/qcom/venus/venc.c | 8 ++++++++ 2 files changed, 16 insertions(+) (limited to 'drivers/media/platform/qcom') diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c index 527944c822b5..7c4c483d5438 100644 --- a/drivers/media/platform/qcom/venus/vdec.c +++ b/drivers/media/platform/qcom/venus/vdec.c @@ -276,6 +276,14 @@ static int vdec_s_fmt(struct file *file, void *fh, struct v4l2_format *f) const struct venus_format *fmt; struct v4l2_format format; u32 pixfmt_out = 0, pixfmt_cap = 0; + struct vb2_queue *q; + + q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type); + if (!q) + return -EINVAL; + + if (vb2_is_busy(q)) + return -EBUSY; orig_pixmp = *pixmp; diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c index 3d8431dc14c4..feed648550d1 100644 --- a/drivers/media/platform/qcom/venus/venc.c +++ b/drivers/media/platform/qcom/venus/venc.c @@ -357,6 +357,14 @@ static int venc_s_fmt(struct file *file, void *fh, struct v4l2_format *f) const struct venus_format *fmt; struct v4l2_format format; u32 pixfmt_out = 0, pixfmt_cap = 0; + struct vb2_queue *q; + + q = v4l2_m2m_get_vq(inst->m2m_ctx, f->type); + if (!q) + return -EINVAL; + + if (vb2_is_busy(q)) + return -EBUSY; orig_pixmp = *pixmp; -- cgit v1.2.3