linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [V4] venus: vdec: decoded picture buffer handling during reconfig sequence
@ 2021-10-14  4:14 Mansur Alisha Shaik
  2021-10-14  9:32 ` kernel test robot
  2021-10-14 13:10 ` kernel test robot
  0 siblings, 2 replies; 3+ messages in thread
From: Mansur Alisha Shaik @ 2021-10-14  4:14 UTC (permalink / raw)
  To: linux-media, stanimir.varbanov
  Cc: linux-kernel, linux-arm-msm, vgarodia, dikshita, Mansur Alisha Shaik

In existing implementation, driver is freeing and un-mapping all the
decoded picture buffers(DPB) as part of dynamic resolution change(DRC)
handling. As a result, when firmware try to access the DPB buffer, from
previous sequence, SMMU context fault is seen due to the buffer being
already unmapped.

With this change, driver defines ownership of each DPB buffer. If a buffer
is owned by firmware, driver would skip from un-mapping the same.

changes in V4:
- As per comments moved static global variable to venus_inst structure
- Addressed other review comments
Changes in V3:
- Migrated id allocation using kernel API ida_alloc_min()

Signed-off-by: Mansur Alisha Shaik <mansur@codeaurora.org>
---
 drivers/media/platform/qcom/venus/core.h    |  1 +
 drivers/media/platform/qcom/venus/helpers.c | 51 ++++++++++++++++++++-
 drivers/media/platform/qcom/venus/helpers.h |  3 ++
 drivers/media/platform/qcom/venus/vdec.c    |  8 +++-
 4 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index a3f077f64be0..34cbd3ef0cd9 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -454,6 +454,7 @@ struct venus_inst {
 	bool next_buf_last;
 	bool drain_active;
 	enum venus_inst_modes flags;
+	struct ida dpb_ids;
 };
 
 #define IS_V1(core)	((core)->res->hfi_version == HFI_VERSION_1XX)
diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 7f2f5b91caaa..464bc7693987 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -3,6 +3,7 @@
  * Copyright (c) 2012-2016, The Linux Foundation. All rights reserved.
  * Copyright (C) 2017 Linaro Ltd.
  */
+#include <linux/idr.h>
 #include <linux/list.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
@@ -21,6 +22,11 @@
 #define NUM_MBS_720P	(((ALIGN(1280, 16)) >> 4) * ((ALIGN(736, 16)) >> 4))
 #define NUM_MBS_4K	(((ALIGN(4096, 16)) >> 4) * ((ALIGN(2304, 16)) >> 4))
 
+enum dpb_buf_owner {
+	DRIVER,
+	FIRMWARE,
+};
+
 struct intbuf {
 	struct list_head list;
 	u32 type;
@@ -28,6 +34,8 @@ struct intbuf {
 	void *va;
 	dma_addr_t da;
 	unsigned long attrs;
+	enum dpb_buf_owner owned_by;
+	u32 dpb_out_tag;
 };
 
 bool venus_helper_check_codec(struct venus_inst *inst, u32 v4l2_pixfmt)
@@ -95,9 +103,16 @@ int venus_helper_queue_dpb_bufs(struct venus_inst *inst)
 		fdata.device_addr = buf->da;
 		fdata.buffer_type = buf->type;
 
+		if (buf->owned_by == FIRMWARE)
+			continue;
+
+		fdata.clnt_data = buf->dpb_out_tag;
+
 		ret = hfi_session_process_buf(inst, &fdata);
 		if (ret)
 			goto fail;
+
+		buf->owned_by = FIRMWARE;
 	}
 
 fail:
@@ -110,13 +125,19 @@ int venus_helper_free_dpb_bufs(struct venus_inst *inst)
 	struct intbuf *buf, *n;
 
 	list_for_each_entry_safe(buf, n, &inst->dpbbufs, list) {
+		if (buf->owned_by == FIRMWARE)
+			continue;
+
+		ida_free(&inst->dpb_ids, buf->dpb_out_tag);
+
 		list_del_init(&buf->list);
 		dma_free_attrs(inst->core->dev, buf->size, buf->va, buf->da,
 			       buf->attrs);
 		kfree(buf);
 	}
 
-	INIT_LIST_HEAD(&inst->dpbbufs);
+	if (list_empty(&inst->dpbbufs))
+		INIT_LIST_HEAD(&inst->dpbbufs);
 
 	return 0;
 }
@@ -134,6 +155,7 @@ int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
 	unsigned int i;
 	u32 count;
 	int ret;
+	int id;
 
 	/* no need to allocate dpb buffers */
 	if (!inst->dpb_fmt)
@@ -171,6 +193,15 @@ int venus_helper_alloc_dpb_bufs(struct venus_inst *inst)
 			ret = -ENOMEM;
 			goto fail;
 		}
+		buf->owned_by = DRIVER;
+
+		id = ida_alloc_min(&inst->dpb_ids, VB2_MAX_FRAME, GFP_KERNEL);
+		if (id < 0) {
+			ret = id;
+			goto fail;
+		}
+
+		buf->dpb_out_tag = id;
 
 		list_add_tail(&buf->list, &inst->dpbbufs);
 	}
@@ -1371,6 +1402,24 @@ venus_helper_find_buf(struct venus_inst *inst, unsigned int type, u32 idx)
 }
 EXPORT_SYMBOL_GPL(venus_helper_find_buf);
 
+void venus_helper_change_dpb_owner(struct venus_inst *inst,
+				   struct vb2_v4l2_buffer *vbuf, unsigned int type,
+				   unsigned int buf_type, u32 tag)
+{
+	struct intbuf *dpb_buf;
+
+	if (!V4L2_TYPE_IS_CAPTURE(type) ||
+	    buf_type != inst->dpb_buftype)
+		return;
+
+	list_for_each_entry(dpb_buf, &inst->dpbbufs, list)
+		if (dpb_buf->dpb_out_tag == tag) {
+			dpb_buf->owned_by = DRIVER;
+			break;
+		}
+}
+EXPORT_SYMBOL_GPL(venus_helper_change_dpb_owner);
+
 int venus_helper_vb2_buf_init(struct vb2_buffer *vb)
 {
 	struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
index e6269b4be3af..ff8889795b43 100644
--- a/drivers/media/platform/qcom/venus/helpers.h
+++ b/drivers/media/platform/qcom/venus/helpers.h
@@ -14,6 +14,9 @@ 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_change_dpb_owner(struct venus_inst *inst,
+				   struct vb2_v4l2_buffer *vbuf, unsigned int type,
+				   unsigned int buf_type, u32 idx);
 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);
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 41c5a353fcae..c2f24e454b28 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -1317,6 +1317,7 @@ static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type,
 	struct vb2_v4l2_buffer *vbuf;
 	struct vb2_buffer *vb;
 	unsigned int type;
+	struct intbuf *dpb_buf;
 
 	vdec_pm_touch(inst);
 
@@ -1326,8 +1327,10 @@ static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type,
 		type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
 
 	vbuf = venus_helper_find_buf(inst, type, tag);
-	if (!vbuf)
+	if (!vbuf) {
+		venus_helper_change_dpb_buf_owner(inst, vbuf, type, buf_type, tag);
 		return;
+	}
 
 	vbuf->flags = flags;
 	vbuf->field = V4L2_FIELD_NONE;
@@ -1606,6 +1609,8 @@ static int vdec_open(struct file *file)
 
 	vdec_inst_init(inst);
 
+	ida_init(&inst->dpb_ids);
+
 	/*
 	 * create m2m device for every instance, the m2m context scheduling
 	 * is made by firmware side so we do not need to care about.
@@ -1651,6 +1656,7 @@ static int vdec_close(struct file *file)
 	v4l2_m2m_ctx_release(inst->m2m_ctx);
 	v4l2_m2m_release(inst->m2m_dev);
 	vdec_ctrl_deinit(inst);
+	ida_destroy(&inst->dpb_ids);
 	hfi_session_destroy(inst);
 	mutex_destroy(&inst->lock);
 	v4l2_fh_del(&inst->fh);
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member 
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [V4] venus: vdec: decoded picture buffer handling during reconfig sequence
  2021-10-14  4:14 [V4] venus: vdec: decoded picture buffer handling during reconfig sequence Mansur Alisha Shaik
@ 2021-10-14  9:32 ` kernel test robot
  2021-10-14 13:10 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2021-10-14  9:32 UTC (permalink / raw)
  To: Mansur Alisha Shaik, linux-media, stanimir.varbanov
  Cc: kbuild-all, linux-kernel, linux-arm-msm, vgarodia, dikshita,
	Mansur Alisha Shaik

[-- Attachment #1: Type: text/plain, Size: 4626 bytes --]

Hi Mansur,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20211013]
[also build test ERROR on v5.15-rc5]
[cannot apply to media-tree/master v5.15-rc5 v5.15-rc4 v5.15-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mansur-Alisha-Shaik/venus-vdec-decoded-picture-buffer-handling-during-reconfig-sequence/20211014-121505
base:    8006b911c90a4ec09958447d24c8a4c3538f5723
config: arm64-randconfig-r006-20211014 (attached as .config)
compiler: aarch64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/168172cb6c2fae8b603221ca11142de23a40f980
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mansur-Alisha-Shaik/venus-vdec-decoded-picture-buffer-handling-during-reconfig-sequence/20211014-121505
        git checkout 168172cb6c2fae8b603221ca11142de23a40f980
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=arm64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/media/platform/qcom/venus/vdec.c: In function 'vdec_buf_done':
>> drivers/media/platform/qcom/venus/vdec.c:1331:17: error: implicit declaration of function 'venus_helper_change_dpb_buf_owner'; did you mean 'venus_helper_change_dpb_owner'? [-Werror=implicit-function-declaration]
    1331 |                 venus_helper_change_dpb_buf_owner(inst, vbuf, type, buf_type, tag);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                 venus_helper_change_dpb_owner
   drivers/media/platform/qcom/venus/vdec.c:1320:24: warning: unused variable 'dpb_buf' [-Wunused-variable]
    1320 |         struct intbuf *dpb_buf;
         |                        ^~~~~~~
   cc1: some warnings being treated as errors


vim +1331 drivers/media/platform/qcom/venus/vdec.c

  1311	
  1312	static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type,
  1313				  u32 tag, u32 bytesused, u32 data_offset, u32 flags,
  1314				  u32 hfi_flags, u64 timestamp_us)
  1315	{
  1316		enum vb2_buffer_state state = VB2_BUF_STATE_DONE;
  1317		struct vb2_v4l2_buffer *vbuf;
  1318		struct vb2_buffer *vb;
  1319		unsigned int type;
  1320		struct intbuf *dpb_buf;
  1321	
  1322		vdec_pm_touch(inst);
  1323	
  1324		if (buf_type == HFI_BUFFER_INPUT)
  1325			type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
  1326		else
  1327			type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
  1328	
  1329		vbuf = venus_helper_find_buf(inst, type, tag);
  1330		if (!vbuf) {
> 1331			venus_helper_change_dpb_buf_owner(inst, vbuf, type, buf_type, tag);
  1332			return;
  1333		}
  1334	
  1335		vbuf->flags = flags;
  1336		vbuf->field = V4L2_FIELD_NONE;
  1337		vb = &vbuf->vb2_buf;
  1338	
  1339		if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
  1340			vb2_set_plane_payload(vb, 0, bytesused);
  1341			vb->planes[0].data_offset = data_offset;
  1342			vb->timestamp = timestamp_us * NSEC_PER_USEC;
  1343			vbuf->sequence = inst->sequence_cap++;
  1344	
  1345			if (vbuf->flags & V4L2_BUF_FLAG_LAST) {
  1346				const struct v4l2_event ev = { .type = V4L2_EVENT_EOS };
  1347	
  1348				v4l2_event_queue_fh(&inst->fh, &ev);
  1349	
  1350				if (inst->codec_state == VENUS_DEC_STATE_DRAIN) {
  1351					inst->drain_active = false;
  1352					inst->codec_state = VENUS_DEC_STATE_STOPPED;
  1353				}
  1354			}
  1355	
  1356			if (!bytesused)
  1357				state = VB2_BUF_STATE_ERROR;
  1358		} else {
  1359			vbuf->sequence = inst->sequence_out++;
  1360		}
  1361	
  1362		venus_helper_get_ts_metadata(inst, timestamp_us, vbuf);
  1363	
  1364		if (hfi_flags & HFI_BUFFERFLAG_READONLY)
  1365			venus_helper_acquire_buf_ref(vbuf);
  1366	
  1367		if (hfi_flags & HFI_BUFFERFLAG_DATACORRUPT)
  1368			state = VB2_BUF_STATE_ERROR;
  1369	
  1370		if (hfi_flags & HFI_BUFFERFLAG_DROP_FRAME) {
  1371			state = VB2_BUF_STATE_ERROR;
  1372			vb2_set_plane_payload(vb, 0, 0);
  1373			vb->timestamp = 0;
  1374		}
  1375	
  1376		v4l2_m2m_buf_done(vbuf, state);
  1377	}
  1378	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45175 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [V4] venus: vdec: decoded picture buffer handling during reconfig sequence
  2021-10-14  4:14 [V4] venus: vdec: decoded picture buffer handling during reconfig sequence Mansur Alisha Shaik
  2021-10-14  9:32 ` kernel test robot
@ 2021-10-14 13:10 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2021-10-14 13:10 UTC (permalink / raw)
  To: Mansur Alisha Shaik, linux-media, stanimir.varbanov
  Cc: kbuild-all, linux-kernel, linux-arm-msm, vgarodia, dikshita,
	Mansur Alisha Shaik

[-- Attachment #1: Type: text/plain, Size: 4625 bytes --]

Hi Mansur,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on next-20211013]
[also build test ERROR on v5.15-rc5]
[cannot apply to media-tree/master v5.15-rc5 v5.15-rc4 v5.15-rc3]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mansur-Alisha-Shaik/venus-vdec-decoded-picture-buffer-handling-during-reconfig-sequence/20211014-121505
base:    8006b911c90a4ec09958447d24c8a4c3538f5723
config: nios2-allyesconfig (attached as .config)
compiler: nios2-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/168172cb6c2fae8b603221ca11142de23a40f980
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mansur-Alisha-Shaik/venus-vdec-decoded-picture-buffer-handling-during-reconfig-sequence/20211014-121505
        git checkout 168172cb6c2fae8b603221ca11142de23a40f980
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=nios2 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/media/platform/qcom/venus/vdec.c: In function 'vdec_buf_done':
   drivers/media/platform/qcom/venus/vdec.c:1331:17: error: implicit declaration of function 'venus_helper_change_dpb_buf_owner'; did you mean 'venus_helper_change_dpb_owner'? [-Werror=implicit-function-declaration]
    1331 |                 venus_helper_change_dpb_buf_owner(inst, vbuf, type, buf_type, tag);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                 venus_helper_change_dpb_owner
>> drivers/media/platform/qcom/venus/vdec.c:1320:24: error: unused variable 'dpb_buf' [-Werror=unused-variable]
    1320 |         struct intbuf *dpb_buf;
         |                        ^~~~~~~
   cc1: all warnings being treated as errors


vim +/dpb_buf +1320 drivers/media/platform/qcom/venus/vdec.c

  1311	
  1312	static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type,
  1313				  u32 tag, u32 bytesused, u32 data_offset, u32 flags,
  1314				  u32 hfi_flags, u64 timestamp_us)
  1315	{
  1316		enum vb2_buffer_state state = VB2_BUF_STATE_DONE;
  1317		struct vb2_v4l2_buffer *vbuf;
  1318		struct vb2_buffer *vb;
  1319		unsigned int type;
> 1320		struct intbuf *dpb_buf;
  1321	
  1322		vdec_pm_touch(inst);
  1323	
  1324		if (buf_type == HFI_BUFFER_INPUT)
  1325			type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
  1326		else
  1327			type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
  1328	
  1329		vbuf = venus_helper_find_buf(inst, type, tag);
  1330		if (!vbuf) {
  1331			venus_helper_change_dpb_buf_owner(inst, vbuf, type, buf_type, tag);
  1332			return;
  1333		}
  1334	
  1335		vbuf->flags = flags;
  1336		vbuf->field = V4L2_FIELD_NONE;
  1337		vb = &vbuf->vb2_buf;
  1338	
  1339		if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
  1340			vb2_set_plane_payload(vb, 0, bytesused);
  1341			vb->planes[0].data_offset = data_offset;
  1342			vb->timestamp = timestamp_us * NSEC_PER_USEC;
  1343			vbuf->sequence = inst->sequence_cap++;
  1344	
  1345			if (vbuf->flags & V4L2_BUF_FLAG_LAST) {
  1346				const struct v4l2_event ev = { .type = V4L2_EVENT_EOS };
  1347	
  1348				v4l2_event_queue_fh(&inst->fh, &ev);
  1349	
  1350				if (inst->codec_state == VENUS_DEC_STATE_DRAIN) {
  1351					inst->drain_active = false;
  1352					inst->codec_state = VENUS_DEC_STATE_STOPPED;
  1353				}
  1354			}
  1355	
  1356			if (!bytesused)
  1357				state = VB2_BUF_STATE_ERROR;
  1358		} else {
  1359			vbuf->sequence = inst->sequence_out++;
  1360		}
  1361	
  1362		venus_helper_get_ts_metadata(inst, timestamp_us, vbuf);
  1363	
  1364		if (hfi_flags & HFI_BUFFERFLAG_READONLY)
  1365			venus_helper_acquire_buf_ref(vbuf);
  1366	
  1367		if (hfi_flags & HFI_BUFFERFLAG_DATACORRUPT)
  1368			state = VB2_BUF_STATE_ERROR;
  1369	
  1370		if (hfi_flags & HFI_BUFFERFLAG_DROP_FRAME) {
  1371			state = VB2_BUF_STATE_ERROR;
  1372			vb2_set_plane_payload(vb, 0, 0);
  1373			vb->timestamp = 0;
  1374		}
  1375	
  1376		v4l2_m2m_buf_done(vbuf, state);
  1377	}
  1378	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61369 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-10-14 13:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-14  4:14 [V4] venus: vdec: decoded picture buffer handling during reconfig sequence Mansur Alisha Shaik
2021-10-14  9:32 ` kernel test robot
2021-10-14 13:10 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).