Buffers can be queued to driver before the planes are
set to start streaming. Queue those buffers to firmware
once start streaming is called on both the planes.
Signed-off-by: Malathi Gottam <[email protected]>
---
drivers/media/platform/qcom/venus/helpers.c | 22 ++++++++++++++++++++++
drivers/media/platform/qcom/venus/helpers.h | 1 +
drivers/media/platform/qcom/venus/venc.c | 5 +++++
3 files changed, 28 insertions(+)
diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index e436385..2679adb 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -1041,6 +1041,28 @@ void venus_helper_vb2_stop_streaming(struct vb2_queue *q)
}
EXPORT_SYMBOL_GPL(venus_helper_vb2_stop_streaming);
+int venus_helper_queue_initial_bufs(struct venus_inst *inst)
+{
+ struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
+ struct v4l2_m2m_buffer *buf, *n;
+ int ret;
+
+ v4l2_m2m_for_each_dst_buf_safe(m2m_ctx, buf, n) {
+ ret = session_process_buf(inst, &buf->vb);
+ if (ret)
+ return_buf_error(inst, &buf->vb);
+ }
+
+ v4l2_m2m_for_each_src_buf_safe(m2m_ctx, buf, n) {
+ ret = session_process_buf(inst, &buf->vb);
+ if (ret)
+ return_buf_error(inst, &buf->vb);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(venus_helper_queue_initial_bufs);
+
int venus_helper_vb2_start_streaming(struct venus_inst *inst)
{
struct venus_core *core = inst->core;
diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
index 2475f284..f4d76ab 100644
--- a/drivers/media/platform/qcom/venus/helpers.h
+++ b/drivers/media/platform/qcom/venus/helpers.h
@@ -31,6 +31,7 @@ void venus_helper_buffers_done(struct venus_inst *inst,
int venus_helper_vb2_start_streaming(struct venus_inst *inst);
void venus_helper_m2m_device_run(void *priv);
void venus_helper_m2m_job_abort(void *priv);
+int venus_helper_queue_initial_bufs(struct venus_inst *inst);
int venus_helper_get_bufreq(struct venus_inst *inst, u32 type,
struct hfi_buffer_requirements *req);
u32 venus_helper_get_framesz_raw(u32 hfi_fmt, u32 width, u32 height);
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index ce85962..ef11495 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -989,6 +989,11 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
if (ret)
goto deinit_sess;
+ ret = venus_helper_queue_initial_bufs(inst);
+ if (ret)
+ goto deinit_sess;
+ }
+
mutex_unlock(&inst->lock);
return 0;
--
1.9.1
Hi Malathi,
On 10/09/2018 10:50 AM, Malathi Gottam wrote:
> Buffers can be queued to driver before the planes are
> set to start streaming. Queue those buffers to firmware
> once start streaming is called on both the planes.
yes and this is done in venus_helper_m2m_device_run mem2mem operation
when streamon on both queues is called, thus below function just
duplicates .device_run.
Do you fix an issue with that patch?
>
> Signed-off-by: Malathi Gottam <[email protected]>
> ---
> drivers/media/platform/qcom/venus/helpers.c | 22 ++++++++++++++++++++++
> drivers/media/platform/qcom/venus/helpers.h | 1 +
> drivers/media/platform/qcom/venus/venc.c | 5 +++++
> 3 files changed, 28 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index e436385..2679adb 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -1041,6 +1041,28 @@ void venus_helper_vb2_stop_streaming(struct vb2_queue *q)
> }
> EXPORT_SYMBOL_GPL(venus_helper_vb2_stop_streaming);
>
> +int venus_helper_queue_initial_bufs(struct venus_inst *inst)
> +{
> + struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
> + struct v4l2_m2m_buffer *buf, *n;
> + int ret;
> +
> + v4l2_m2m_for_each_dst_buf_safe(m2m_ctx, buf, n) {
> + ret = session_process_buf(inst, &buf->vb);
> + if (ret)
> + return_buf_error(inst, &buf->vb);
> + }
> +
> + v4l2_m2m_for_each_src_buf_safe(m2m_ctx, buf, n) {
> + ret = session_process_buf(inst, &buf->vb);
> + if (ret)
> + return_buf_error(inst, &buf->vb);
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(venus_helper_queue_initial_bufs);
> +
> int venus_helper_vb2_start_streaming(struct venus_inst *inst)
> {
> struct venus_core *core = inst->core;
> diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
> index 2475f284..f4d76ab 100644
> --- a/drivers/media/platform/qcom/venus/helpers.h
> +++ b/drivers/media/platform/qcom/venus/helpers.h
> @@ -31,6 +31,7 @@ void venus_helper_buffers_done(struct venus_inst *inst,
> int venus_helper_vb2_start_streaming(struct venus_inst *inst);
> void venus_helper_m2m_device_run(void *priv);
> void venus_helper_m2m_job_abort(void *priv);
> +int venus_helper_queue_initial_bufs(struct venus_inst *inst);
> int venus_helper_get_bufreq(struct venus_inst *inst, u32 type,
> struct hfi_buffer_requirements *req);
> u32 venus_helper_get_framesz_raw(u32 hfi_fmt, u32 width, u32 height);
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index ce85962..ef11495 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -989,6 +989,11 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
> if (ret)
> goto deinit_sess;
>
> + ret = venus_helper_queue_initial_bufs(inst);
> + if (ret)
> + goto deinit_sess;
> + }
> +
> mutex_unlock(&inst->lock);
>
> return 0;
>
--
regards,
Stan
Hi Malathi,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.19-rc7 next-20181009]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Malathi-Gottam/media-venus-queue-initial-buffers/20181009-221017
base: git://linuxtv.org/media_tree.git master
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=m68k
All error/warnings (new ones prefixed by >>):
drivers/media//platform/qcom/venus/venc.c: In function 'venc_start_streaming':
>> drivers/media//platform/qcom/venus/venc.c:994:3: error: label 'deinit_sess' used but not defined
goto deinit_sess;
^~~~
>> drivers/media//platform/qcom/venus/venc.c:973:3: error: label 'bufs_done' used but not defined
goto bufs_done;
^~~~
drivers/media//platform/qcom/venus/venc.c: At top level:
>> drivers/media//platform/qcom/venus/venc.c:997:15: error: expected declaration specifiers or '...' before '&' token
mutex_unlock(&inst->lock);
^
>> drivers/media//platform/qcom/venus/venc.c:999:2: error: expected identifier or '(' before 'return'
return 0;
^~~~~~
>> drivers/media//platform/qcom/venus/venc.c:1001:12: error: expected '=', ',', ';', 'asm' or '__attribute__' before ':' token
deinit_sess:
^
drivers/media//platform/qcom/venus/venc.c:1003:10: error: expected '=', ',', ';', 'asm' or '__attribute__' before ':' token
bufs_done:
^
>> drivers/media//platform/qcom/venus/venc.c:1005:2: error: expected identifier or '(' before 'if'
if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
^~
>> drivers/media//platform/qcom/venus/venc.c:1007:2: error: expected identifier or '(' before 'else'
else
^~~~
drivers/media//platform/qcom/venus/venc.c:1009:15: error: expected declaration specifiers or '...' before '&' token
mutex_unlock(&inst->lock);
^
drivers/media//platform/qcom/venus/venc.c:1010:2: error: expected identifier or '(' before 'return'
return ret;
^~~~~~
>> drivers/media//platform/qcom/venus/venc.c:1011:1: error: expected identifier or '(' before '}' token
}
^
drivers/media//platform/qcom/venus/venc.c: In function 'venc_start_streaming':
>> drivers/media//platform/qcom/venus/venc.c:995:2: warning: control reaches end of non-void function [-Wreturn-type]
}
^
vim +/deinit_sess +994 drivers/media//platform/qcom/venus/venc.c
948
949 static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
950 {
951 struct venus_inst *inst = vb2_get_drv_priv(q);
952 int ret;
953
954 mutex_lock(&inst->lock);
955
956 if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
957 inst->streamon_out = 1;
958 else
959 inst->streamon_cap = 1;
960
961 if (!(inst->streamon_out & inst->streamon_cap)) {
962 mutex_unlock(&inst->lock);
963 return 0;
964 }
965
966 venus_helper_init_instance(inst);
967
968 inst->sequence_cap = 0;
969 inst->sequence_out = 0;
970
971 ret = venc_init_session(inst);
972 if (ret)
> 973 goto bufs_done;
974
975 ret = venc_set_properties(inst);
976 if (ret)
977 goto deinit_sess;
978
979 ret = venc_verify_conf(inst);
980 if (ret)
981 goto deinit_sess;
982
983 ret = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
984 inst->num_output_bufs, 0);
985 if (ret)
986 goto deinit_sess;
987
988 ret = venus_helper_vb2_start_streaming(inst);
989 if (ret)
990 goto deinit_sess;
991
992 ret = venus_helper_queue_initial_bufs(inst);
993 if (ret)
> 994 goto deinit_sess;
> 995 }
996
> 997 mutex_unlock(&inst->lock);
998
> 999 return 0;
1000
> 1001 deinit_sess:
1002 hfi_session_deinit(inst);
1003 bufs_done:
1004 venus_helper_buffers_done(inst, VB2_BUF_STATE_QUEUED);
> 1005 if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
1006 inst->streamon_out = 0;
> 1007 else
1008 inst->streamon_cap = 0;
1009 mutex_unlock(&inst->lock);
> 1010 return ret;
> 1011 }
1012
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 2018-10-09 20:47, Stanimir Varbanov wrote:
> Hi Malathi,
>
> On 10/09/2018 10:50 AM, Malathi Gottam wrote:
>> Buffers can be queued to driver before the planes are
>> set to start streaming. Queue those buffers to firmware
>> once start streaming is called on both the planes.
>
> yes and this is done in venus_helper_m2m_device_run mem2mem operation
> when streamon on both queues is called, thus below function just
> duplicates .device_run.
>
> Do you fix an issue with that patch?
>
>>
>> Signed-off-by: Malathi Gottam <[email protected]>
>> ---
>> drivers/media/platform/qcom/venus/helpers.c | 22
>> ++++++++++++++++++++++
>> drivers/media/platform/qcom/venus/helpers.h | 1 +
>> drivers/media/platform/qcom/venus/venc.c | 5 +++++
>> 3 files changed, 28 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/venus/helpers.c
>> b/drivers/media/platform/qcom/venus/helpers.c
>> index e436385..2679adb 100644
>> --- a/drivers/media/platform/qcom/venus/helpers.c
>> +++ b/drivers/media/platform/qcom/venus/helpers.c
>> @@ -1041,6 +1041,28 @@ void venus_helper_vb2_stop_streaming(struct
>> vb2_queue *q)
>> }
>> EXPORT_SYMBOL_GPL(venus_helper_vb2_stop_streaming);
>>
>> +int venus_helper_queue_initial_bufs(struct venus_inst *inst)
>> +{
>> + struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
>> + struct v4l2_m2m_buffer *buf, *n;
>> + int ret;
>> +
>> + v4l2_m2m_for_each_dst_buf_safe(m2m_ctx, buf, n) {
>> + ret = session_process_buf(inst, &buf->vb);
>> + if (ret)
>> + return_buf_error(inst, &buf->vb);
>> + }
>> +
>> + v4l2_m2m_for_each_src_buf_safe(m2m_ctx, buf, n) {
>> + ret = session_process_buf(inst, &buf->vb);
>> + if (ret)
>> + return_buf_error(inst, &buf->vb);
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(venus_helper_queue_initial_bufs);
>> +
>> int venus_helper_vb2_start_streaming(struct venus_inst *inst)
>> {
>> struct venus_core *core = inst->core;
>> diff --git a/drivers/media/platform/qcom/venus/helpers.h
>> b/drivers/media/platform/qcom/venus/helpers.h
>> index 2475f284..f4d76ab 100644
>> --- a/drivers/media/platform/qcom/venus/helpers.h
>> +++ b/drivers/media/platform/qcom/venus/helpers.h
>> @@ -31,6 +31,7 @@ void venus_helper_buffers_done(struct venus_inst
>> *inst,
>> int venus_helper_vb2_start_streaming(struct venus_inst *inst);
>> void venus_helper_m2m_device_run(void *priv);
>> void venus_helper_m2m_job_abort(void *priv);
>> +int venus_helper_queue_initial_bufs(struct venus_inst *inst);
>> int venus_helper_get_bufreq(struct venus_inst *inst, u32 type,
>> struct hfi_buffer_requirements *req);
>> u32 venus_helper_get_framesz_raw(u32 hfi_fmt, u32 width, u32 height);
>> diff --git a/drivers/media/platform/qcom/venus/venc.c
>> b/drivers/media/platform/qcom/venus/venc.c
>> index ce85962..ef11495 100644
>> --- a/drivers/media/platform/qcom/venus/venc.c
>> +++ b/drivers/media/platform/qcom/venus/venc.c
>> @@ -989,6 +989,11 @@ static int venc_start_streaming(struct vb2_queue
>> *q, unsigned int count)
>> if (ret)
>> goto deinit_sess;
>>
>> + ret = venus_helper_queue_initial_bufs(inst);
>> + if (ret)
>> + goto deinit_sess;
>> + }
>> +
>> mutex_unlock(&inst->lock);
>>
>> return 0;
>>
Hi Stan,
As I considered playback sequence as well, this function
"venus_helper_m2m_device_run" was muted as a part of it. So I had to
explicitly implement this function for encoder.
For now, we can omit this patch. Once the playback sequence gets merged
into upstream, we can re-look into this.
Hi,
On 10/20/18 10:50 AM, [email protected] wrote:
> On 2018-10-09 20:47, Stanimir Varbanov wrote:
>> Hi Malathi,
>>
>> On 10/09/2018 10:50 AM, Malathi Gottam wrote:
>>> Buffers can be queued to driver before the planes are
>>> set to start streaming. Queue those buffers to firmware
>>> once start streaming is called on both the planes.
>>
>> yes and this is done in venus_helper_m2m_device_run mem2mem operation
>> when streamon on both queues is called, thus below function just
>> duplicates .device_run.
>>
>> Do you fix an issue with that patch?
>>
>>>
>>> Signed-off-by: Malathi Gottam <[email protected]>
>>> ---
>>> drivers/media/platform/qcom/venus/helpers.c | 22 ++++++++++++++++++++++
>>> drivers/media/platform/qcom/venus/helpers.h | 1 +
>>> drivers/media/platform/qcom/venus/venc.c | 5 +++++
>>> 3 files changed, 28 insertions(+)
>>>
>>> diff --git a/drivers/media/platform/qcom/venus/helpers.c
>>> b/drivers/media/platform/qcom/venus/helpers.c
>>> index e436385..2679adb 100644
>>> --- a/drivers/media/platform/qcom/venus/helpers.c
>>> +++ b/drivers/media/platform/qcom/venus/helpers.c
>>> @@ -1041,6 +1041,28 @@ void venus_helper_vb2_stop_streaming(struct
>>> vb2_queue *q)
>>> }
>>> EXPORT_SYMBOL_GPL(venus_helper_vb2_stop_streaming);
>>>
>>> +int venus_helper_queue_initial_bufs(struct venus_inst *inst)
>>> +{
>>> + struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
>>> + struct v4l2_m2m_buffer *buf, *n;
>>> + int ret;
>>> +
>>> + v4l2_m2m_for_each_dst_buf_safe(m2m_ctx, buf, n) {
>>> + ret = session_process_buf(inst, &buf->vb);
>>> + if (ret)
>>> + return_buf_error(inst, &buf->vb);
>>> + }
>>> +
>>> + v4l2_m2m_for_each_src_buf_safe(m2m_ctx, buf, n) {
>>> + ret = session_process_buf(inst, &buf->vb);
>>> + if (ret)
>>> + return_buf_error(inst, &buf->vb);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL(venus_helper_queue_initial_bufs);
>>> +
>>> int venus_helper_vb2_start_streaming(struct venus_inst *inst)
>>> {
>>> struct venus_core *core = inst->core;
>>> diff --git a/drivers/media/platform/qcom/venus/helpers.h
>>> b/drivers/media/platform/qcom/venus/helpers.h
>>> index 2475f284..f4d76ab 100644
>>> --- a/drivers/media/platform/qcom/venus/helpers.h
>>> +++ b/drivers/media/platform/qcom/venus/helpers.h
>>> @@ -31,6 +31,7 @@ void venus_helper_buffers_done(struct venus_inst
>>> *inst,
>>> int venus_helper_vb2_start_streaming(struct venus_inst *inst);
>>> void venus_helper_m2m_device_run(void *priv);
>>> void venus_helper_m2m_job_abort(void *priv);
>>> +int venus_helper_queue_initial_bufs(struct venus_inst *inst);
>>> int venus_helper_get_bufreq(struct venus_inst *inst, u32 type,
>>> struct hfi_buffer_requirements *req);
>>> u32 venus_helper_get_framesz_raw(u32 hfi_fmt, u32 width, u32 height);
>>> diff --git a/drivers/media/platform/qcom/venus/venc.c
>>> b/drivers/media/platform/qcom/venus/venc.c
>>> index ce85962..ef11495 100644
>>> --- a/drivers/media/platform/qcom/venus/venc.c
>>> +++ b/drivers/media/platform/qcom/venus/venc.c
>>> @@ -989,6 +989,11 @@ static int venc_start_streaming(struct vb2_queue
>>> *q, unsigned int count)
>>> if (ret)
>>> goto deinit_sess;
>>>
>>> + ret = venus_helper_queue_initial_bufs(inst);
>>> + if (ret)
>>> + goto deinit_sess;
>>> + }
>>> +
>>> mutex_unlock(&inst->lock);
>>>
>>> return 0;
>>>
> Hi Stan,
>
> As I considered playback sequence as well, this function
> "venus_helper_m2m_device_run" was muted as a part of it. So I had to
> explicitly implement this function for encoder.
>
> For now, we can omit this patch. Once the playback sequence gets merged
> into upstream, we can re-look into this.
Sorry, I didn't look deeply into playback sequence patches yet, but
looks like we have to fix them first. So please drop this patch.
--
regards,
Stan