media: venus: queue initial buffers

PATCH In review Malathi Gottam 3 participants 5 messages Oct 9, 2018 – Nov 1, 2018

Subsystems (4)

Media (V4L/DVB) Arm Qualcomm mailing list Qualcomm media platform Qualcomm venus video accelerator driver

lore

Share & tools

lore

Malathi Gottam

author

2018-10-09 07:50 UTC #0 lore raw

PATCH 1/1 media: venus: queue initial buffers

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(+)

drivers/media/platform/qcom/venus/helpers.c +22 -0

index e436385..2679adb 100644
@@ -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;

drivers/media/platform/qcom/venus/helpers.h +1 -0

index 2475f284..f4d76ab 100644
@@ -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);

drivers/media/platform/qcom/venus/venc.c +5 -0

index ce85962..ef11495 100644
@@ -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;

Signature · 1 line

1.9.1

Stanimir Varbanov

2018-10-09 15:17 UTC #1 lore raw

Hi Malathi,

Quoted 3 lines · Malathi Gottam

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?

Quoted 70 lines

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(+)

drivers/media/platform/qcom/venus/helpers.c +22 -0

index e436385..2679adb 100644
@@ -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;

drivers/media/platform/qcom/venus/helpers.h +1 -0

index 2475f284..f4d76ab 100644
@@ -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);

drivers/media/platform/qcom/venus/venc.c +5 -0

index ce85962..ef11495 100644
@@ -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;

Signature · 2 lines

regards,
Stan

[email protected]

author

2018-10-20 07:50 UTC #2 lore raw

Quoted 85 lines · Stanimir Varbanov

Hi Malathi,

Quoted 3 lines · Malathi Gottam

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?

Quoted 72 lines

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(+)

drivers/media/platform/qcom/venus/helpers.c +22 -0

index e436385..2679adb 100644
@@ -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)
 {

drivers/media/platform/qcom/venus/helpers.c +0 -0

 	struct venus_core *core = inst->core;

drivers/media/platform/qcom/venus/helpers.h +1 -0

index 2475f284..f4d76ab 100644
@@ -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);

drivers/media/platform/qcom/venus/venc.c +5 -0

index ce85962..ef11495 100644
@@ -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);

Quoted 2 lines

Quoted 2 lines
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.

Stanimir Varbanov

2018-11-01 12:26 UTC #3 lore raw

Hi,

Quoted 92 lines · On 10/20/18 10:50 AM, [email protected] wrote:

Quoted 87 lines · Stanimir Varbanov
Hi Malathi,
Quoted 3 lines · Malathi Gottam
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?
Quoted 74 lines
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(+)
drivers/media/platform/qcom/venus/helpers.c +22 -0
index e436385..2679adb 100644
@@ -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;
drivers/media/platform/qcom/venus/helpers.h +1 -0
index 2475f284..f4d76ab 100644
@@ -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);
drivers/media/platform/qcom/venus/venc.c +5 -0
index ce85962..ef11495 100644
@@ -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);
 
Quoted 2 lines
Quoted 2 lines
return 0;
Hi Stan,

Quoted 6 lines

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.

Signature · 2 lines

regards,
Stan

kbuild test robot

build bot

2018-10-09 18:26 UTC #4 lore raw

In reply to Malathi Gottam

Thank you for the patch! Yet something to improve:

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