Skip to contentSkip to conversation

media: venus: add debugfs support

PATCH In review Malathi Gottam 2 participants 2 messages Jan 7, 2019

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 media: venus: add debugfs support

Enable logs in venus through debugfs to print
debug information.

---
 drivers/media/platform/qcom/venus/core.c       |  62 ++++++++-
 drivers/media/platform/qcom/venus/core.h       |  32 +++++
 drivers/media/platform/qcom/venus/firmware.c   |   6 +-
 drivers/media/platform/qcom/venus/helpers.c    |  51 +++++--
 drivers/media/platform/qcom/venus/hfi.c        |  93 ++++++++++---
 drivers/media/platform/qcom/venus/hfi_cmds.c   |  33 +++--
 drivers/media/platform/qcom/venus/hfi_msgs.c   |   5 +
 drivers/media/platform/qcom/venus/vdec.c       | 185 +++++++++++++++++++------
 drivers/media/platform/qcom/venus/vdec_ctrls.c |   7 +-
 drivers/media/platform/qcom/venus/venc.c       | 139 +++++++++++++++----
 drivers/media/platform/qcom/venus/venc_ctrls.c |   9 +-
 11 files changed, 512 insertions(+), 110 deletions(-)
drivers/media/platform/qcom/venus/core.c +59 -3
index cb411eb..abfc6c9 100644
@@ -13,6 +13,7 @@
  *
  */
 #include <linux/clk.h>
+#include <linux/debugfs.h>
 #include <linux/init.h>
 #include <linux/ioctl.h>
 #include <linux/list.h>
@@ -31,6 +32,50 @@
 #include "venc.h"
 #include "firmware.h"
 
+struct dentry *debugfs_root;
+int venus_debug = ERR;
+EXPORT_SYMBOL_GPL(venus_debug);
+
+static struct dentry *venus_debugfs_init_drv(void)
+{
+	bool ok = false;
+	struct dentry *dir = NULL;
+
+	dir = debugfs_create_dir("venus", NULL);
+	if (IS_ERR_OR_NULL(dir)) {
+		dir = NULL;
+		pr_err("failed to create debug dir");
+		goto failed_create_dir;
+	}
+
+#define __debugfs_create(__type, __fname, __value) ({                          \
+	struct dentry *f = debugfs_create_##__type(__fname, 0644,	\
+		dir, __value);                                                \
+	if (IS_ERR_OR_NULL(f)) {                                              \
+		dprintk(ERR, "Failed creating debugfs file '%pd/%s'\n",  \
+			dir, __fname);                                         \
+		f = NULL;                                                     \
+	}                                                                     \
+	f;                                                                    \
+})
+
+	ok =
+	__debugfs_create(x32, "debug_level", &venus_debug);
+
+#undef __debugfs_create
+
+	if (!ok)
+		goto failed_create_dir;
+
+	return dir;
+
+failed_create_dir:
+	if (dir)
+		debugfs_remove_recursive(debugfs_root);
+
+	return NULL;
+}
+
 static void venus_event_notify(struct venus_core *core, u32 event)
 {
 	struct venus_inst *inst;
@@ -137,6 +182,7 @@ static int venus_clks_enable(struct venus_core *core)
 
 	return 0;
 err:
+	dprintk(ERR, "Failed to enable clk:%d\n", i);
 	while (i--)
 		clk_disable_unprepare(core->clks[i]);
 
@@ -236,6 +282,8 @@ static int venus_probe(struct platform_device *pdev)
 	struct resource *r;
 	int ret;
 
+	debugfs_root = venus_debugfs_init_drv();
+
 	core = devm_kzalloc(dev, sizeof(*core), GFP_KERNEL);
 	if (!core)
 		return -ENOMEM;
@@ -245,8 +293,10 @@ static int venus_probe(struct platform_device *pdev)
 
 	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	core->base = devm_ioremap_resource(dev, r);
-	if (IS_ERR(core->base))
+	if (IS_ERR(core->base)) {
+		dprintk(ERR, "Failed to ioremap platform resources");
 		return PTR_ERR(core->base);
+		}
 
 	core->irq = platform_get_irq(pdev, 0);
 	if (core->irq < 0)
@@ -297,8 +347,10 @@ static int venus_probe(struct platform_device *pdev)
 		goto err_runtime_disable;
 
 	ret = venus_firmware_init(core);
-	if (ret)
+	if (ret) {
+		dprintk(ERR, "Failed to init video firmware\n");
 		goto err_runtime_disable;
+		}
 
 	ret = venus_boot(core);
 	if (ret)
@@ -321,8 +373,10 @@ static int venus_probe(struct platform_device *pdev)
 		goto err_venus_shutdown;
 
 	ret = v4l2_device_register(dev, &core->v4l2_dev);
-	if (ret)
+	if (ret) {
+		dprintk(ERR, "Failed to register v4l2 device\n");
 		goto err_core_deinit;
+		}
 
 	ret = pm_runtime_put_sync(dev);
 	if (ret)
@@ -366,6 +420,8 @@ static int venus_remove(struct platform_device *pdev)
 
 	v4l2_device_unregister(&core->v4l2_dev);
 
+	debugfs_remove_recursive(debugfs_root);
+
 	return ret;
 }
 
drivers/media/platform/qcom/venus/core.h +32 -0
index 6382cea..c31d9e0 100644
@@ -25,6 +25,38 @@
 
 #define VIDC_CLKS_NUM_MAX	4
 
+extern int venus_debug;
+enum venus_msg_prio {
+	ERR  = 0x0001,
+	WARN = 0x0002,
+	INFO = 0x0004,
+	DBG  = 0x0008,
+};
+
+static inline char *get_debug_level_str(int level)
+{
+	switch (level) {
+	case ERR:
+		return "err";
+	case WARN:
+		return "warn";
+	case INFO:
+		return "info";
+	case DBG:
+		return "dbg";
+	default:
+		return "???";
+	}
+}
+
+#define dprintk(dbg_lvl, fmt, arg...)				\
+	do {							\
+		if (venus_debug & dbg_lvl)				\
+			pr_info("venus:" fmt, \
+					get_debug_level_str(dbg_lvl),	\
+					## arg);	\
+	} while (0)
+
 struct freq_tbl {
 	unsigned int load;
 	unsigned long freq;
drivers/media/platform/qcom/venus/firmware.c +4 -2
index c29acfd..eaf5951 100644
@@ -142,7 +142,8 @@ static int venus_boot_no_tz(struct venus_core *core, phys_addr_t mem_phys,
 		dev_err(dev, "could not map video firmware region\n");
 		return ret;
 	}
-
+	dprintk(DBG, "%s: Successfully mapped and performed test translation\n",
+		dev_name(dev));
 	venus_reset_cpu(core);
 
 	return 0;
@@ -258,7 +259,8 @@ int venus_firmware_init(struct venus_core *core)
 		dev_err(core->fw.dev, "could not attach device\n");
 		goto err_iommu_free;
 	}
-
+	dprintk(DBG, "Attached and created mapping for %s\n",
+		dev_name(core->fw.dev));
 	core->fw.iommu_domain = iommu_dom;
 
 	of_node_put(np);
drivers/media/platform/qcom/venus/helpers.c +42 -9

143 lines not on this page · raw message

drivers/media/platform/qcom/venus/hfi.c +76 -17

214 lines not on this page · raw message

drivers/media/platform/qcom/venus/hfi_cmds.c +25 -8
index 87a4414..3c451a7 100644
@@ -16,6 +16,7 @@
 #include <linux/hash.h>
 
 #include "hfi_cmds.h"
+#include "core.h"
 
 static enum hfi_version hfi_ver;
 
@@ -161,8 +162,10 @@ void pkt_sys_image_version(struct hfi_sys_get_property_pkt *pkt)
 int pkt_session_init(struct hfi_session_init_pkt *pkt, void *cookie,
 		     u32 session_type, u32 codec)
 {
-	if (!pkt || !cookie || !codec)
+	if (!pkt || !cookie || !codec) {
+		dprintk(ERR, "%s invalid parameters\n", __func__);
 		return -EINVAL;
+		}
 
 	pkt->shdr.hdr.size = sizeof(*pkt);
 	pkt->shdr.hdr.pkt_type = HFI_CMD_SYS_SESSION_INIT;
@@ -185,8 +188,10 @@ int pkt_session_set_buffers(struct hfi_session_set_buffers_pkt *pkt,
 {
 	unsigned int i;
 
-	if (!cookie || !pkt || !bd)
+	if (!cookie || !pkt || !bd) {
+		dprintk(ERR, "%s - invalid params\n", __func__);
 		return -EINVAL;
+		}
 
 	pkt->shdr.hdr.pkt_type = HFI_CMD_SESSION_SET_BUFFERS;
 	pkt->shdr.session_id = hash32_ptr(cookie);
@@ -215,7 +220,8 @@ int pkt_session_set_buffers(struct hfi_session_set_buffers_pkt *pkt,
 	}
 
 	pkt->buffer_type = bd->buffer_type;
-
+	dprintk(DBG, "buftype:%d buffer addr: %x\n",
+		pkt->buffer_type, bd->device_addr);
 	return 0;
 }
 
@@ -263,9 +269,11 @@ int pkt_session_unset_buffers(struct hfi_session_release_buffer_pkt *pkt,
 int pkt_session_etb_decoder(struct hfi_session_empty_buffer_compressed_pkt *pkt,
 			    void *cookie, struct hfi_frame_data *in_frame)
 {
-	if (!cookie || !in_frame->device_addr)
+	if (!cookie || !in_frame->device_addr) {
+		dprintk(ERR, "%s: invalid params addr: %#x\n",
+			__func__, in_frame->device_addr);
 		return -EINVAL;
-
+	}
 	pkt->shdr.hdr.size = sizeof(*pkt);
 	pkt->shdr.hdr.pkt_type = HFI_CMD_SESSION_EMPTY_BUFFER;
 	pkt->shdr.session_id = hash32_ptr(cookie);
@@ -287,8 +295,11 @@ int pkt_session_etb_encoder(
 		struct hfi_session_empty_buffer_uncompressed_plane0_pkt *pkt,
 		void *cookie, struct hfi_frame_data *in_frame)
 {
-	if (!cookie || !in_frame->device_addr)
+	if (!cookie || !in_frame->device_addr) {
+		dprintk(ERR, "%s: invalid params addr: %#x\n",
+			__func__, in_frame->device_addr);
 		return -EINVAL;
+	}
 
 	pkt->shdr.hdr.size = sizeof(*pkt);
 	pkt->shdr.hdr.pkt_type = HFI_CMD_SESSION_EMPTY_BUFFER;
@@ -312,8 +323,11 @@ int pkt_session_etb_encoder(
 int pkt_session_ftb(struct hfi_session_fill_buffer_pkt *pkt, void *cookie,
 		    struct hfi_frame_data *out_frame)
 {
-	if (!cookie || !out_frame || !out_frame->device_addr)
+	if (!cookie || !out_frame || !out_frame->device_addr) {
+		dprintk(ERR, "%s: invalid params addr: %#x\n",
+			__func__, out_frame->device_addr);
 		return -EINVAL;
+	}
 
 	pkt->shdr.hdr.size = sizeof(*pkt);
 	pkt->shdr.hdr.pkt_type = HFI_CMD_SESSION_FILL_BUFFER;
@@ -339,8 +353,11 @@ int pkt_session_parse_seq_header(
 		struct hfi_session_parse_sequence_header_pkt *pkt,
 		void *cookie, u32 seq_hdr, u32 seq_hdr_len)
 {
-	if (!cookie || !seq_hdr || !seq_hdr_len)
+	if (!cookie || !seq_hdr || !seq_hdr_len) {
+		dprintk(ERR, "%s: invalid params hdr: %d, hdr_len:%d\n",
+			__func__, seq_hdr, seq_hdr_len);
 		return -EINVAL;
+	}
 
 	pkt->shdr.hdr.size = sizeof(*pkt);
 	pkt->shdr.hdr.pkt_type = HFI_CMD_SESSION_PARSE_SEQUENCE_HEADER;
drivers/media/platform/qcom/venus/hfi_msgs.c +5 -0
index 0ecdaa1..b6ea2f9 100644
@@ -238,9 +238,14 @@ static void hfi_sys_init_done(struct venus_core *core, struct venus_inst *inst,
 	}
 
 	error = hfi_parser(core, inst, pkt->data, rem_bytes);
+	dprintk(DBG,
+		"supported_codecs[%d]: enc = %#x, dec = %#x\n",
+		core->codecs_count, core->enc_codecs,
+		core->dec_codecs);
 
 done:
 	core->error = error;
+	dprintk(INFO, "sys init ret: %d", core->error);
 	complete(&core->done);
 }
 
drivers/media/platform/qcom/venus/vdec.c +145 -40

406 lines not on this page · raw message

drivers/media/platform/qcom/venus/vdec_ctrls.c +6 -1
index f4604b0..bfc6904 100644
@@ -91,8 +91,11 @@ int vdec_ctrl_init(struct venus_inst *inst)
 	int ret;
 
 	ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 7);
-	if (ret)
+	if (ret) {
+		dprintk(ERR, "CTRL ERR: Control handler init failed, %d\n",
+			inst->ctrl_handler.error);
 		return ret;
+	}
 
 	ctrl = v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &vdec_ctrl_ops,
 		V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE,
@@ -147,6 +150,8 @@ int vdec_ctrl_init(struct venus_inst *inst)
 
 	ret = inst->ctrl_handler.error;
 	if (ret) {
+		dprintk(ERR, "Error adding ctrl to ctrl handle, %d\n",
+			inst->ctrl_handler.error);
 		v4l2_ctrl_handler_free(&inst->ctrl_handler);
 		return ret;
 	}
drivers/media/platform/qcom/venus/venc.c +111 -28

300 lines not on this page · raw message

drivers/media/platform/qcom/venus/venc_ctrls.c +7 -2
index ac1e1d2..dbbec61 100644
@@ -203,6 +203,7 @@ static int venc_op_s_ctrl(struct v4l2_ctrl *ctrl)
 		mutex_unlock(&inst->lock);
 		break;
 	default:
+		dprintk(ERR, "Unsupported ctrl: %x\n", ctrl->id);
 		return -EINVAL;
 	}
 
@@ -218,9 +219,11 @@ int venc_ctrl_init(struct venus_inst *inst)
 	int ret;
 
 	ret = v4l2_ctrl_handler_init(&inst->ctrl_handler, 28);
-	if (ret)
+	if (ret) {
+		dprintk(ERR, "CTRL ERR: Control handler init failed, %d\n",
+			inst->ctrl_handler.error);
 		return ret;
-
+	}
 	v4l2_ctrl_new_std_menu(&inst->ctrl_handler, &venc_ctrl_ops,
 		V4L2_CID_MPEG_VIDEO_BITRATE_MODE,
 		V4L2_MPEG_VIDEO_BITRATE_MODE_CBR,
@@ -351,6 +354,8 @@ int venc_ctrl_init(struct venus_inst *inst)
 
 	return 0;
 err:
+	dprintk(ERR, "Error adding ctrl to ctrl handle, %d\n",
+		inst->ctrl_handler.error);
 	v4l2_ctrl_handler_free(&inst->ctrl_handler);
 	return ret;
 }
Signature · 1 line
1.9.1
#1 lore raw

drivers/media/platform/qcom/venus/core.c line 49

Quoted 11 lines · Malathi Gottam
drivers/media/platform/qcom/venus/core.c +11 -0
+static struct dentry *venus_debugfs_init_drv(void)
+{
+	bool ok = false;
+	struct dentry *dir = NULL;
+
+	dir = debugfs_create_dir("venus", NULL);
+	if (IS_ERR_OR_NULL(dir)) {
+		dir = NULL;
+		pr_err("failed to create debug dir");
+		goto failed_create_dir;
+	}

Huh? When does debugfs_create_dir() return ERR_PTR()?

Any interface that mixes returning NULL for error-reporting with returning ERR_PTR(...) for the same needs to be put out of its misery. As it happens, debugfs_create_dir() returns NULL on all errors. And if you've meant the above as "future-proofing"... don't do that, please. That only breeds confusion down the road, as the code gets cut'n'pasted around.

drivers/media/platform/qcom/venus/helpers.c int venus_helper_set_num_bufs(struct venus_inst *inst, unsigned int input_bufs, · line 762

Quoted 10 lines
drivers/media/platform/qcom/venus/helpers.c +4 -1
-	if (ret)
+	if (ret) {
+		dprintk(ERR,
+			"Failed to set actual buffer count %d for buffer type %d\n",
+			buf_count.count_actual, buf_count.type);
 		return ret;
 
 	buf_count.type = HFI_BUFFER_OUTPUT;
 	buf_count.count_actual = output_bufs;

*blink*
Does that even compile?