[PATCH v5,0/8] media: mediatek: vcodec: Add debugfs file for decode and encode

[PATCH v5,0/8] media: mediatek: vcodec: Add debugfs file for decode and encode

[Yunfei Dong]

Need to change kernel driver to open decode and encode debug log at current period,
it's very unreasonable. Adding debugfs common interface to support decode and encode,
using echo command to control debug log level and getting useful information for each
instance.

patch 1 add dbgfs common interface.
patch 2~5 support decode.
patch 6~7 support encode
patch 8 add help function
---
changed with v4:
- rebase to the top of media stage header.

changed with v3:
- add help function for patch 8
- remove append '\0' and enlarge buffer size for patch 4

changed with v2:
- using pr_debug and dev_dbg instead of pr_info for patch 2.
- fix word fail: informatiaoin -> information for patch 3.
- used to print each instance format information for patch 5.

changed with v1:
- add new patch 4 and 5.
- using cmd 'cat vdec' to show debug information instead of pr_info directly.
---
Yunfei Dong (8):
  media: mediatek: vcodec: Add debugfs interface to get debug
    information
  media: mediatek: vcodec: Add debug params to control different log
    level
  media: mediatek: vcodec: Add a debugfs file to get different useful
    information
  media: mediatek: vcodec: Get each context resolution information
  media: mediatek: vcodec: Get each instance format type
  media: mediatek: vcodec: Change dbgfs interface to support encode
  media: mediatek: vcodec: Add encode to support dbgfs
  media: mediatek: vcodec: Add dbgfs help function

 .../media/platform/mediatek/vcodec/Makefile   |   6 +
 .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 216 ++++++++++++++++++
 .../mediatek/vcodec/mtk_vcodec_dbgfs.h        |  72 ++++++
 .../mediatek/vcodec/mtk_vcodec_dec_drv.c      |   4 +
 .../platform/mediatek/vcodec/mtk_vcodec_drv.h |   4 +
 .../mediatek/vcodec/mtk_vcodec_enc_drv.c      |   2 +
 .../mediatek/vcodec/mtk_vcodec_util.c         |   8 +
 .../mediatek/vcodec/mtk_vcodec_util.h         |  26 ++-
 8 files changed, 335 insertions(+), 3 deletions(-)
 create mode 100644 drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
 create mode 100644 drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h

-- 
2.25.1

[PATCH v5,1/8] media: mediatek: vcodec: Add debugfs interface to get debug information

[Yunfei Dong]

This will be useful when debugging specific issues related to kernel
in running status.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../media/platform/mediatek/vcodec/Makefile   |  6 ++++
 .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 33 +++++++++++++++++++
 .../mediatek/vcodec/mtk_vcodec_dbgfs.h        | 32 ++++++++++++++++++
 .../mediatek/vcodec/mtk_vcodec_dec_drv.c      |  2 ++
 .../platform/mediatek/vcodec/mtk_vcodec_drv.h |  4 +++
 5 files changed, 77 insertions(+)
 create mode 100644 drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
 create mode 100644 drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h

diff --git a/drivers/media/platform/mediatek/vcodec/Makefile b/drivers/media/platform/mediatek/vcodec/Makefile
index d24b452d0fb3..d9979d0259c1 100644
--- a/drivers/media/platform/mediatek/vcodec/Makefile
+++ b/drivers/media/platform/mediatek/vcodec/Makefile
@@ -45,3 +45,9 @@ endif
 ifneq ($(CONFIG_VIDEO_MEDIATEK_VCODEC_SCP),)
 mtk-vcodec-common-y += mtk_vcodec_fw_scp.o
 endif
+
+ifneq ($(CONFIG_DEBUG_FS),)
+obj-$(CONFIG_VIDEO_MEDIATEK_VCODEC) += mtk-vcodec-dbgfs.o
+
+mtk-vcodec-dbgfs-y := mtk_vcodec_dbgfs.o
+endif
\ No newline at end of file
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
new file mode 100644
index 000000000000..fb9edd379af5
--- /dev/null
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2023 MediaTek Inc.
+ * Author: Yunfei Dong <yunfei.dong@mediatek.com>
+ */
+
+#include <linux/debugfs.h>
+
+#include "mtk_vcodec_dbgfs.h"
+#include "mtk_vcodec_drv.h"
+#include "mtk_vcodec_util.h"
+
+void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev)
+{
+	struct dentry *vcodec_root;
+
+	vcodec_dev->dbgfs.vcodec_root = debugfs_create_dir("vcodec-dec", NULL);
+	if (IS_ERR(vcodec_dev->dbgfs.vcodec_root))
+		dev_err(&vcodec_dev->plat_dev->dev, "create vcodec dir err:%d\n",
+			IS_ERR(vcodec_dev->dbgfs.vcodec_root));
+
+	vcodec_root = vcodec_dev->dbgfs.vcodec_root;
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_dbgfs_init);
+
+void mtk_vcodec_dbgfs_deinit(struct mtk_vcodec_dev *vcodec_dev)
+{
+	debugfs_remove_recursive(vcodec_dev->dbgfs.vcodec_root);
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_dbgfs_deinit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Mediatek video codec driver");
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
new file mode 100644
index 000000000000..5eec2211cbbe
--- /dev/null
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2023 MediaTek Inc.
+ * Author: Yunfei Dong <yunfei.dong@mediatek.com>
+ */
+
+#ifndef __MTK_VCODEC_DBGFS_H__
+#define __MTK_VCODEC_DBGFS_H__
+
+struct mtk_vcodec_dev;
+
+/**
+ * struct mtk_vcodec_dbgfs  - dbgfs information
+ * @vcodec_root: vcodec dbgfs entry
+ */
+struct mtk_vcodec_dbgfs {
+	struct dentry *vcodec_root;
+};
+
+#if defined(CONFIG_DEBUG_FS)
+void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev);
+void mtk_vcodec_dbgfs_deinit(struct mtk_vcodec_dev *vcodec_dev);
+#else
+static inline void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev)
+{
+}
+
+static inline void mtk_vcodec_dbgfs_deinit(struct mtk_vcodec_dev *vcodec_dev)
+{
+}
+#endif
+#endif
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
index 7bd300341cf0..51a9af193f92 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
@@ -422,6 +422,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
 		mtk_v4l2_debug(0, "media registered as /dev/media%d", vfd_dec->minor);
 	}
 
+	mtk_vcodec_dbgfs_init(dev);
 	mtk_v4l2_debug(0, "decoder registered as /dev/video%d", vfd_dec->minor);
 
 	return 0;
@@ -497,6 +498,7 @@ static void mtk_vcodec_dec_remove(struct platform_device *pdev)
 	if (dev->vfd_dec)
 		video_unregister_device(dev->vfd_dec);
 
+	mtk_vcodec_dbgfs_deinit(dev);
 	v4l2_device_unregister(&dev->v4l2_dev);
 	if (!dev->vdec_pdata->is_subdev_supported)
 		pm_runtime_disable(dev->pm.dev);
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
index 1f4c5774ec47..3ec027ec8192 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
@@ -16,6 +16,7 @@
 #include <media/v4l2-mem2mem.h>
 #include <media/videobuf2-core.h>
 
+#include "mtk_vcodec_dbgfs.h"
 #include "mtk_vcodec_util.h"
 #include "vdec_msg_queue.h"
 
@@ -470,6 +471,7 @@ struct mtk_vcodec_enc_pdata {
  * @dec_active_cnt: used to mark whether need to record register value
  * @vdec_racing_info: record register value
  * @dec_racing_info_mutex: mutex lock used for inner racing mode
+ * @dbgfs: debug log related information
  */
 struct mtk_vcodec_dev {
 	struct v4l2_device v4l2_dev;
@@ -519,6 +521,8 @@ struct mtk_vcodec_dev {
 	u32 vdec_racing_info[132];
 	/* Protects access to vdec_racing_info data */
 	struct mutex dec_racing_info_mutex;
+
+	struct mtk_vcodec_dbgfs dbgfs;
 };
 
 static inline struct mtk_vcodec_ctx *fh_to_ctx(struct v4l2_fh *fh)
-- 
2.25.1

[PATCH v5,2/8] media: mediatek: vcodec: Add debug params to control different log level

[Yunfei Dong]

Add parameter mtk_vcodec_dbg to open each codec log.
Add parameter mtk_v4l2_dbg_level to open each instance log according to
the parameter value.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../mediatek/vcodec/mtk_vcodec_dbgfs.c        |  2 ++
 .../mediatek/vcodec/mtk_vcodec_util.c         |  8 ++++++
 .../mediatek/vcodec/mtk_vcodec_util.h         | 26 ++++++++++++++++---
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
index fb9edd379af5..b5093e4e4aa2 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
@@ -20,6 +20,8 @@ void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev)
 			IS_ERR(vcodec_dev->dbgfs.vcodec_root));
 
 	vcodec_root = vcodec_dev->dbgfs.vcodec_root;
+	debugfs_create_x32("mtk_v4l2_dbg_level", 0644, vcodec_root, &mtk_v4l2_dbg_level);
+	debugfs_create_x32("mtk_vcodec_dbg", 0644, vcodec_root, &mtk_vcodec_dbg);
 }
 EXPORT_SYMBOL_GPL(mtk_vcodec_dbgfs_init);
 
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.c
index ace78c4b5b9e..f214e6f67005 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.c
@@ -13,6 +13,14 @@
 #include "mtk_vcodec_drv.h"
 #include "mtk_vcodec_util.h"
 
+#if defined(CONFIG_DEBUG_FS)
+int mtk_vcodec_dbg;
+EXPORT_SYMBOL(mtk_vcodec_dbg);
+
+int mtk_v4l2_dbg_level;
+EXPORT_SYMBOL(mtk_v4l2_dbg_level);
+#endif
+
 void __iomem *mtk_vcodec_get_reg_addr(struct mtk_vcodec_ctx *data,
 					unsigned int reg_idx)
 {
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
index 71956627a0e2..587aa817e7f4 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
@@ -35,15 +35,35 @@ struct mtk_vcodec_dev;
 	pr_err("[MTK_VCODEC][ERROR][%d]: " fmt "\n",		\
 	       ((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
 
+#if defined(CONFIG_DEBUG_FS)
+extern int mtk_v4l2_dbg_level;
+extern int mtk_vcodec_dbg;
 
-#define mtk_v4l2_debug(level, fmt, args...) pr_debug(fmt, ##args)
+#define mtk_v4l2_debug(level, fmt, args...)				\
+	do {								\
+		if (mtk_v4l2_dbg_level >= level)			\
+			pr_debug("[MTK_V4L2] %s, %d: " fmt "\n",        \
+				 __func__, __LINE__, ##args);	        \
+	} while (0)
 
-#define mtk_v4l2_debug_enter()  mtk_v4l2_debug(3, "+")
-#define mtk_v4l2_debug_leave()  mtk_v4l2_debug(3, "-")
+#define mtk_vcodec_debug(h, fmt, args...)				                      \
+	do {								                      \
+		if (mtk_vcodec_dbg)					                      \
+			dev_dbg(&(((struct mtk_vcodec_ctx *)(h)->ctx)->dev->plat_dev->dev),   \
+				"[MTK_VCODEC][%d]: %s, %d " fmt "\n",                         \
+				((struct mtk_vcodec_ctx *)(h)->ctx)->id,                      \
+				__func__, __LINE__, ##args);                                  \
+	} while (0)
+#else
+#define mtk_v4l2_debug(level, fmt, args...) pr_debug(fmt, ##args)
 
 #define mtk_vcodec_debug(h, fmt, args...)			\
 	pr_debug("[MTK_VCODEC][%d]: " fmt "\n",			\
 		((struct mtk_vcodec_ctx *)(h)->ctx)->id, ##args)
+#endif
+
+#define mtk_v4l2_debug_enter()  mtk_v4l2_debug(3, "+")
+#define mtk_v4l2_debug_leave()  mtk_v4l2_debug(3, "-")
 
 #define mtk_vcodec_debug_enter(h)  mtk_vcodec_debug(h, "+")
 #define mtk_vcodec_debug_leave(h)  mtk_vcodec_debug(h, "-")
-- 
2.25.1

[PATCH v5,3/8] media: mediatek: vcodec: Add a debugfs file to get different useful information

[Yunfei Dong]

In oder to get each instance information according to test command, adding
one file node "vdec".

Can use echo command to set different string value as 'echo -picinfo > vdec'.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 64 +++++++++++++++++++
 .../mediatek/vcodec/mtk_vcodec_dbgfs.h        | 31 +++++++++
 .../mediatek/vcodec/mtk_vcodec_dec_drv.c      |  2 +
 3 files changed, 97 insertions(+)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
index b5093e4e4aa2..4aade064af96 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
@@ -10,6 +10,64 @@
 #include "mtk_vcodec_drv.h"
 #include "mtk_vcodec_util.h"
 
+static ssize_t mtk_vdec_dbgfs_write(struct file *filp, const char __user *ubuf,
+				    size_t count, loff_t *ppos)
+{
+	struct mtk_vcodec_dev *vcodec_dev = filp->private_data;
+	struct mtk_vcodec_dbgfs *dbgfs = &vcodec_dev->dbgfs;
+	int len;
+
+	mutex_lock(&dbgfs->dbgfs_lock);
+	len = simple_write_to_buffer(dbgfs->dbgfs_buf, sizeof(dbgfs->dbgfs_buf),
+				     ppos, ubuf, count);
+	mutex_unlock(&dbgfs->dbgfs_lock);
+	if (len > 0)
+		return count;
+
+	return len;
+}
+
+static const struct file_operations vdec_fops = {
+	.open = simple_open,
+	.write = mtk_vdec_dbgfs_write,
+};
+
+void mtk_vcodec_dbgfs_create(struct mtk_vcodec_ctx *ctx)
+{
+	struct mtk_vcodec_dbgfs_inst *dbgfs_inst;
+	struct mtk_vcodec_dev *vcodec_dev = ctx->dev;
+
+	dbgfs_inst = kzalloc(sizeof(*dbgfs_inst), GFP_KERNEL);
+	if (!dbgfs_inst)
+		return;
+
+	list_add_tail(&dbgfs_inst->node, &vcodec_dev->dbgfs.dbgfs_head);
+
+	vcodec_dev->dbgfs.inst_count++;
+
+	dbgfs_inst->inst_id = ctx->id;
+	dbgfs_inst->vcodec_ctx = ctx;
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_dbgfs_create);
+
+void mtk_vcodec_dbgfs_remove(struct mtk_vcodec_dev *vcodec_dev, int ctx_id)
+{
+	struct mtk_vcodec_dbgfs_inst *dbgfs_inst;
+
+	list_for_each_entry(dbgfs_inst, &vcodec_dev->dbgfs.dbgfs_head, node) {
+		if (dbgfs_inst && dbgfs_inst->inst_id == ctx_id) {
+			vcodec_dev->dbgfs.inst_count--;
+			break;
+		}
+	}
+
+	if (dbgfs_inst) {
+		list_del(&dbgfs_inst->node);
+		kfree(dbgfs_inst);
+	}
+}
+EXPORT_SYMBOL_GPL(mtk_vcodec_dbgfs_remove);
+
 void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev)
 {
 	struct dentry *vcodec_root;
@@ -22,6 +80,12 @@ void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev)
 	vcodec_root = vcodec_dev->dbgfs.vcodec_root;
 	debugfs_create_x32("mtk_v4l2_dbg_level", 0644, vcodec_root, &mtk_v4l2_dbg_level);
 	debugfs_create_x32("mtk_vcodec_dbg", 0644, vcodec_root, &mtk_vcodec_dbg);
+
+	vcodec_dev->dbgfs.inst_count = 0;
+
+	INIT_LIST_HEAD(&vcodec_dev->dbgfs.dbgfs_head);
+	debugfs_create_file("vdec", 0200, vcodec_root, vcodec_dev, &vdec_fops);
+	mutex_init(&vcodec_dev->dbgfs.dbgfs_lock);
 }
 EXPORT_SYMBOL_GPL(mtk_vcodec_dbgfs_init);
 
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
index 5eec2211cbbe..9df760475684 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
@@ -8,19 +8,50 @@
 #define __MTK_VCODEC_DBGFS_H__
 
 struct mtk_vcodec_dev;
+struct mtk_vcodec_ctx;
+
+/**
+ * struct mtk_vcodec_dbgfs_inst  - debugfs information for each inst
+ * @node:       list node for each inst
+ * @vcodec_ctx: struct mtk_vcodec_ctx
+ * @inst_id:    index of the context that the same with ctx->id
+ */
+struct mtk_vcodec_dbgfs_inst {
+	struct list_head node;
+	struct mtk_vcodec_ctx *vcodec_ctx;
+	int inst_id;
+};
 
 /**
  * struct mtk_vcodec_dbgfs  - dbgfs information
+ * @dbgfs_head:  list head used to link each instance
  * @vcodec_root: vcodec dbgfs entry
+ * @dbgfs_lock:  dbgfs lock used to protect dbgfs_buf
+ * @dbgfs_buf:   dbgfs buf used to store dbgfs cmd
+ * @inst_count:  the count of total instance
  */
 struct mtk_vcodec_dbgfs {
+	struct list_head dbgfs_head;
 	struct dentry *vcodec_root;
+	struct mutex dbgfs_lock;
+	char dbgfs_buf[1024];
+	int inst_count;
 };
 
 #if defined(CONFIG_DEBUG_FS)
+void mtk_vcodec_dbgfs_create(struct mtk_vcodec_ctx *ctx);
+void mtk_vcodec_dbgfs_remove(struct mtk_vcodec_dev *vcodec_dev, int ctx_id);
 void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev);
 void mtk_vcodec_dbgfs_deinit(struct mtk_vcodec_dev *vcodec_dev);
 #else
+static inline void mtk_vcodec_dbgfs_create(struct mtk_vcodec_ctx *ctx)
+{
+}
+
+static inline void mtk_vcodec_dbgfs_remove(struct mtk_vcodec_dev *vcodec_dev, int ctx_id)
+{
+}
+
 static inline void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev)
 {
 }
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
index 51a9af193f92..b25943ac3909 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
@@ -215,6 +215,7 @@ static int fops_vcodec_open(struct file *file)
 	ctx->dev->vdec_pdata->init_vdec_params(ctx);
 
 	list_add(&ctx->list, &dev->ctx_list);
+	mtk_vcodec_dbgfs_create(ctx);
 
 	mutex_unlock(&dev->dev_mutex);
 	mtk_v4l2_debug(0, "%s decoder [%d]", dev_name(&dev->plat_dev->dev),
@@ -256,6 +257,7 @@ static int fops_vcodec_release(struct file *file)
 	v4l2_fh_exit(&ctx->fh);
 	v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
 
+	mtk_vcodec_dbgfs_remove(dev, ctx->id);
 	list_del_init(&ctx->list);
 	kfree(ctx);
 	mutex_unlock(&dev->dev_mutex);
-- 
2.25.1

Re: [PATCH v5,3/8] media: mediatek: vcodec: Add a debugfs file to get different useful information

[AngeloGioacchino Del Regno]

Il 25/05/23 04:12, Yunfei Dong ha scritto:
> In oder to get each instance information according to test command, adding
> one file node "vdec".
> 
> Can use echo command to set different string value as 'echo -picinfo > vdec'.
> 
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH v5,4/8] media: mediatek: vcodec: Get each context resolution information

[Yunfei Dong]

Will store the string to temp buffer like "echo '-picinfo' > vdec" when
user want to get needed information.

Then getting debug information using command 'cat vdec' calling mtk_vdec_dbgfs_read
to analysis the temp buffer.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 42 +++++++++++++++++++
 .../mediatek/vcodec/mtk_vcodec_dbgfs.h        |  8 ++++
 2 files changed, 50 insertions(+)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
index 4aade064af96..a47005e0bc16 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
@@ -27,9 +27,51 @@ static ssize_t mtk_vdec_dbgfs_write(struct file *filp, const char __user *ubuf,
 	return len;
 }
 
+static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf,
+				   size_t count, loff_t *ppos)
+{
+	struct mtk_vcodec_dev *vcodec_dev = filp->private_data;
+	struct mtk_vcodec_dbgfs *dbgfs = &vcodec_dev->dbgfs;
+	struct mtk_vcodec_dbgfs_inst *dbgfs_inst;
+	struct mtk_vcodec_ctx *ctx;
+	int total_len = 200 * (dbgfs->inst_count == 0 ? 1 : dbgfs->inst_count);
+	int used_len = 0, curr_len, ret;
+	bool dbgfs_index[MTK_VDEC_DBGFS_MAX] = {0};
+	char *buf = kmalloc(total_len, GFP_KERNEL);
+
+	if (!buf)
+		return -ENOMEM;
+
+	if (strstr(dbgfs->dbgfs_buf, "-picinfo"))
+		dbgfs_index[MTK_VDEC_DBGFS_PICINFO] = true;
+
+	mutex_lock(&dbgfs->dbgfs_lock);
+	list_for_each_entry(dbgfs_inst, &dbgfs->dbgfs_head, node) {
+		ctx = dbgfs_inst->vcodec_ctx;
+
+		curr_len = snprintf(buf + used_len, total_len - used_len,
+				    "inst[%d]:\n ", ctx->id);
+		used_len += curr_len;
+
+		if (dbgfs_index[MTK_VDEC_DBGFS_PICINFO]) {
+			curr_len = snprintf(buf + used_len, total_len - used_len,
+					    "\treal(%dx%d)=>align(%dx%d)\n",
+					    ctx->picinfo.pic_w, ctx->picinfo.pic_h,
+					    ctx->picinfo.buf_w, ctx->picinfo.buf_h);
+			used_len += curr_len;
+		}
+	}
+	mutex_unlock(&dbgfs->dbgfs_lock);
+
+	ret = simple_read_from_buffer(ubuf, count, ppos, buf, used_len);
+	kfree(buf);
+	return ret;
+}
+
 static const struct file_operations vdec_fops = {
 	.open = simple_open,
 	.write = mtk_vdec_dbgfs_write,
+	.read = mtk_vdec_dbgfs_read,
 };
 
 void mtk_vcodec_dbgfs_create(struct mtk_vcodec_ctx *ctx)
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
index 9df760475684..da61b2dffe29 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
@@ -10,6 +10,14 @@
 struct mtk_vcodec_dev;
 struct mtk_vcodec_ctx;
 
+/*
+ * enum mtk_vdec_dbgfs_log_index  - used to get different debug information
+ */
+enum mtk_vdec_dbgfs_log_index {
+	MTK_VDEC_DBGFS_PICINFO,
+	MTK_VDEC_DBGFS_MAX,
+};
+
 /**
  * struct mtk_vcodec_dbgfs_inst  - debugfs information for each inst
  * @node:       list node for each inst
-- 
2.25.1

Re: [PATCH v5,4/8] media: mediatek: vcodec: Get each context resolution information

[AngeloGioacchino Del Regno]

Il 25/05/23 04:12, Yunfei Dong ha scritto:
> Will store the string to temp buffer like "echo '-picinfo' > vdec" when
> user want to get needed information.
> 
> Then getting debug information using command 'cat vdec' calling mtk_vdec_dbgfs_read
> to analysis the temp buffer.
> 
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH v5,5/8] media: mediatek: vcodec: Get each instance format type

[Yunfei Dong]

Adding echo command to get capture and output queue format
type of each instance:"echo '-format' > vdec", not current
hardware supported.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 48 +++++++++++++++++++
 .../mediatek/vcodec/mtk_vcodec_dbgfs.h        |  1 +
 2 files changed, 49 insertions(+)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
index a47005e0bc16..b2b69c3400d4 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
@@ -10,6 +10,48 @@
 #include "mtk_vcodec_drv.h"
 #include "mtk_vcodec_util.h"
 
+static void mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_ctx *ctx, char *buf,
+					   int *used, int total)
+{
+	int curr_len;
+
+	switch (ctx->current_codec) {
+	case V4L2_PIX_FMT_H264_SLICE:
+		curr_len = snprintf(buf + *used, total - *used,
+				    "\toutput format: h264 slice\n");
+		break;
+	case V4L2_PIX_FMT_VP8_FRAME:
+		curr_len = snprintf(buf + *used, total - *used,
+				    "\toutput format: vp8 slice\n");
+		break;
+	case V4L2_PIX_FMT_VP9_FRAME:
+		curr_len = snprintf(buf + *used, total - *used,
+				    "\toutput format: vp9 slice\n");
+		break;
+	default:
+		curr_len = snprintf(buf + *used, total - *used,
+				    "\tunsupported output format: 0x%x\n",
+				    ctx->current_codec);
+	}
+	*used += curr_len;
+
+	switch (ctx->capture_fourcc) {
+	case V4L2_PIX_FMT_MM21:
+		curr_len = snprintf(buf + *used, total - *used,
+				    "\tcapture format: MM21\n");
+		break;
+	case V4L2_PIX_FMT_MT21C:
+		curr_len = snprintf(buf + *used, total - *used,
+				    "\tcapture format: MT21C\n");
+		break;
+	default:
+		curr_len = snprintf(buf + *used, total - *used,
+				    "\tunsupported capture format: 0x%x\n",
+				    ctx->capture_fourcc);
+	}
+	*used += curr_len;
+}
+
 static ssize_t mtk_vdec_dbgfs_write(struct file *filp, const char __user *ubuf,
 				    size_t count, loff_t *ppos)
 {
@@ -45,6 +87,9 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf,
 	if (strstr(dbgfs->dbgfs_buf, "-picinfo"))
 		dbgfs_index[MTK_VDEC_DBGFS_PICINFO] = true;
 
+	if (strstr(dbgfs->dbgfs_buf, "-format"))
+		dbgfs_index[MTK_VDEC_DBGFS_FORMAT] = true;
+
 	mutex_lock(&dbgfs->dbgfs_lock);
 	list_for_each_entry(dbgfs_inst, &dbgfs->dbgfs_head, node) {
 		ctx = dbgfs_inst->vcodec_ctx;
@@ -60,6 +105,9 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf,
 					    ctx->picinfo.buf_w, ctx->picinfo.buf_h);
 			used_len += curr_len;
 		}
+
+		if (dbgfs_index[MTK_VDEC_DBGFS_FORMAT])
+			mtk_vdec_dbgfs_get_format_type(ctx, buf, &used_len, total_len);
 	}
 	mutex_unlock(&dbgfs->dbgfs_lock);
 
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
index da61b2dffe29..b0bdb84a46df 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
@@ -15,6 +15,7 @@ struct mtk_vcodec_ctx;
  */
 enum mtk_vdec_dbgfs_log_index {
 	MTK_VDEC_DBGFS_PICINFO,
+	MTK_VDEC_DBGFS_FORMAT,
 	MTK_VDEC_DBGFS_MAX,
 };
 
-- 
2.25.1

Re: [PATCH v5,5/8] media: mediatek: vcodec: Get each instance format type

[AngeloGioacchino Del Regno]

Il 25/05/23 04:12, Yunfei Dong ha scritto:
> Adding echo command to get capture and output queue format
> type of each instance:"echo '-format' > vdec", not current
> hardware supported.
> 
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH v5,6/8] media: mediatek: vcodec: Change dbgfs interface to support encode

[Yunfei Dong]

Extend dbgfs init interface to support encode and create encode
dbgfs file.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 .../media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c    | 9 +++++++--
 .../media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h    | 4 ++--
 .../media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c  | 2 +-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
index b2b69c3400d4..237d0dc8a1fc 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
@@ -158,11 +158,14 @@ void mtk_vcodec_dbgfs_remove(struct mtk_vcodec_dev *vcodec_dev, int ctx_id)
 }
 EXPORT_SYMBOL_GPL(mtk_vcodec_dbgfs_remove);
 
-void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev)
+void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev, bool is_encode)
 {
 	struct dentry *vcodec_root;
 
-	vcodec_dev->dbgfs.vcodec_root = debugfs_create_dir("vcodec-dec", NULL);
+	if (is_encode)
+		vcodec_dev->dbgfs.vcodec_root = debugfs_create_dir("vcodec-enc", NULL);
+	else
+		vcodec_dev->dbgfs.vcodec_root = debugfs_create_dir("vcodec-dec", NULL);
 	if (IS_ERR(vcodec_dev->dbgfs.vcodec_root))
 		dev_err(&vcodec_dev->plat_dev->dev, "create vcodec dir err:%d\n",
 			IS_ERR(vcodec_dev->dbgfs.vcodec_root));
@@ -172,6 +175,8 @@ void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev)
 	debugfs_create_x32("mtk_vcodec_dbg", 0644, vcodec_root, &mtk_vcodec_dbg);
 
 	vcodec_dev->dbgfs.inst_count = 0;
+	if (is_encode)
+		return;
 
 	INIT_LIST_HEAD(&vcodec_dev->dbgfs.dbgfs_head);
 	debugfs_create_file("vdec", 0200, vcodec_root, vcodec_dev, &vdec_fops);
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
index b0bdb84a46df..ba43518d0fcb 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
@@ -50,7 +50,7 @@ struct mtk_vcodec_dbgfs {
 #if defined(CONFIG_DEBUG_FS)
 void mtk_vcodec_dbgfs_create(struct mtk_vcodec_ctx *ctx);
 void mtk_vcodec_dbgfs_remove(struct mtk_vcodec_dev *vcodec_dev, int ctx_id);
-void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev);
+void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev, bool is_encode);
 void mtk_vcodec_dbgfs_deinit(struct mtk_vcodec_dev *vcodec_dev);
 #else
 static inline void mtk_vcodec_dbgfs_create(struct mtk_vcodec_ctx *ctx)
@@ -61,7 +61,7 @@ static inline void mtk_vcodec_dbgfs_remove(struct mtk_vcodec_dev *vcodec_dev, in
 {
 }
 
-static inline void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev)
+static inline void mtk_vcodec_dbgfs_init(struct mtk_vcodec_dev *vcodec_dev, bool is_encode)
 {
 }
 
diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
index b25943ac3909..d41f2121b94f 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dec_drv.c
@@ -424,7 +424,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
 		mtk_v4l2_debug(0, "media registered as /dev/media%d", vfd_dec->minor);
 	}
 
-	mtk_vcodec_dbgfs_init(dev);
+	mtk_vcodec_dbgfs_init(dev, false);
 	mtk_v4l2_debug(0, "decoder registered as /dev/video%d", vfd_dec->minor);
 
 	return 0;
-- 
2.25.1

[PATCH v5,7/8] media: mediatek: vcodec: Add encode to support dbgfs

[Yunfei Dong]

Add encode to support dbgfs.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c
index 168004a08888..5df0a22ff3b5 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c
@@ -358,6 +358,7 @@ static int mtk_vcodec_probe(struct platform_device *pdev)
 		goto err_enc_reg;
 	}
 
+	mtk_vcodec_dbgfs_init(dev, true);
 	mtk_v4l2_debug(0, "encoder %d registered as /dev/video%d",
 		       dev->venc_pdata->core_id, vfd_enc->num);
 
@@ -468,6 +469,7 @@ static void mtk_vcodec_enc_remove(struct platform_device *pdev)
 	if (dev->vfd_enc)
 		video_unregister_device(dev->vfd_enc);
 
+	mtk_vcodec_dbgfs_deinit(dev);
 	v4l2_device_unregister(&dev->v4l2_dev);
 	pm_runtime_disable(dev->pm.dev);
 	mtk_vcodec_fw_release(dev->fw_handler);
-- 
2.25.1

[PATCH v5,8/8] media: mediatek: vcodec: Add dbgfs help function

[Yunfei Dong]

Getting dbgfs help information with command "echo -help > vdec".

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 24 ++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
index 237d0dc8a1fc..2372fc449b45 100644
--- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
+++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
@@ -52,6 +52,23 @@ static void mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_ctx *ctx, char *buf
 	*used += curr_len;
 }
 
+static void mtk_vdec_dbgfs_get_help(char *buf, int *used, int total)
+{
+	int curr_len;
+
+	curr_len = snprintf(buf + *used, total - *used,
+			    "help: (1: echo -'info' > vdec 2: cat vdec)\n");
+	*used += curr_len;
+
+	curr_len = snprintf(buf + *used, total - *used,
+			    "\t-picinfo: get resolution\n");
+	*used += curr_len;
+
+	curr_len = snprintf(buf + *used, total - *used,
+			    "\t-format: get output & capture queue format\n");
+	*used += curr_len;
+}
+
 static ssize_t mtk_vdec_dbgfs_write(struct file *filp, const char __user *ubuf,
 				    size_t count, loff_t *ppos)
 {
@@ -84,6 +101,11 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf,
 	if (!buf)
 		return -ENOMEM;
 
+	if (strstr(dbgfs->dbgfs_buf, "-help")) {
+		mtk_vdec_dbgfs_get_help(buf, &used_len, total_len);
+		goto read_buffer;
+	}
+
 	if (strstr(dbgfs->dbgfs_buf, "-picinfo"))
 		dbgfs_index[MTK_VDEC_DBGFS_PICINFO] = true;
 
@@ -110,7 +132,7 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf,
 			mtk_vdec_dbgfs_get_format_type(ctx, buf, &used_len, total_len);
 	}
 	mutex_unlock(&dbgfs->dbgfs_lock);
-
+read_buffer:
 	ret = simple_read_from_buffer(ubuf, count, ppos, buf, used_len);
 	kfree(buf);
 	return ret;
-- 
2.25.1

Re: [PATCH v5,8/8] media: mediatek: vcodec: Add dbgfs help function

[AngeloGioacchino Del Regno]

Il 25/05/23 04:12, Yunfei Dong ha scritto:
> Getting dbgfs help information with command "echo -help > vdec".
> 
> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> ---
>   .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 24 ++++++++++++++++++-
>   1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
> index 237d0dc8a1fc..2372fc449b45 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
> @@ -52,6 +52,23 @@ static void mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_ctx *ctx, char *buf
>   	*used += curr_len;
>   }
>   
> +static void mtk_vdec_dbgfs_get_help(char *buf, int *used, int total)
> +{
> +	int curr_len;
> +
> +	curr_len = snprintf(buf + *used, total - *used,
> +			    "help: (1: echo -'info' > vdec 2: cat vdec)\n");
> +	*used += curr_len;
> +
> +	curr_len = snprintf(buf + *used, total - *used,
> +			    "\t-picinfo: get resolution\n");
> +	*used += curr_len;
> +
> +	curr_len = snprintf(buf + *used, total - *used,
> +			    "\t-format: get output & capture queue format\n");
> +	*used += curr_len;
> +}
> +
>   static ssize_t mtk_vdec_dbgfs_write(struct file *filp, const char __user *ubuf,
>   				    size_t count, loff_t *ppos)
>   {
> @@ -84,6 +101,11 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf,
>   	if (!buf)
>   		return -ENOMEM;
>   
> +	if (strstr(dbgfs->dbgfs_buf, "-help")) {

I would print the help strings in two conditions:
1. -help
2. (nothing)

...so that if you don't echo anything to vdec (no params), you get the help text.
Otherwise, you would have to know that "-help" is a parameter that gives you help
text in the first place.

As for this commit "as is", it works as intended and it is useful to retrieve
the help text; you can either send a followup commit that extends the help to
the corner case that I've explained, or send a v6 adding that to this same commit.

I would prefer to see a v6 -- BUT -- since this series was sent a long time ago,
you will get my R-b and I will leave the final choice to Hans.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Re: [PATCH v5,8/8] media: mediatek: vcodec: Add dbgfs help function

[Chen-Yu Tsai]

On Tue, May 30, 2023 at 6:06 PM AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 25/05/23 04:12, Yunfei Dong ha scritto:
> > Getting dbgfs help information with command "echo -help > vdec".
> >
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > ---
> >   .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 24 ++++++++++++++++++-
> >   1 file changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
> > index 237d0dc8a1fc..2372fc449b45 100644
> > --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
> > +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
> > @@ -52,6 +52,23 @@ static void mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_ctx *ctx, char *buf
> >       *used += curr_len;
> >   }
> >
> > +static void mtk_vdec_dbgfs_get_help(char *buf, int *used, int total)
> > +{
> > +     int curr_len;
> > +
> > +     curr_len = snprintf(buf + *used, total - *used,
> > +                         "help: (1: echo -'info' > vdec 2: cat vdec)\n");
> > +     *used += curr_len;
> > +
> > +     curr_len = snprintf(buf + *used, total - *used,
> > +                         "\t-picinfo: get resolution\n");
> > +     *used += curr_len;
> > +
> > +     curr_len = snprintf(buf + *used, total - *used,
> > +                         "\t-format: get output & capture queue format\n");
> > +     *used += curr_len;
> > +}
> > +
> >   static ssize_t mtk_vdec_dbgfs_write(struct file *filp, const char __user *ubuf,
> >                                   size_t count, loff_t *ppos)
> >   {
> > @@ -84,6 +101,11 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf,
> >       if (!buf)
> >               return -ENOMEM;
> >
> > +     if (strstr(dbgfs->dbgfs_buf, "-help")) {
>
> I would print the help strings in two conditions:
> 1. -help
> 2. (nothing)
>
> ...so that if you don't echo anything to vdec (no params), you get the help text.
> Otherwise, you would have to know that "-help" is a parameter that gives you help
> text in the first place.
>
> As for this commit "as is", it works as intended and it is useful to retrieve
> the help text; you can either send a followup commit that extends the help to
> the corner case that I've explained, or send a v6 adding that to this same commit.
>
> I would prefer to see a v6 -- BUT -- since this series was sent a long time ago,
> you will get my R-b and I will leave the final choice to Hans.
>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

The help file could just exist as a separate file. You can then use cleaner
interfaces for it.

ChenYu

Re: [PATCH v5,8/8] media: mediatek: vcodec: Add dbgfs help function

[Hans Verkuil]

On 5/30/23 12:06, AngeloGioacchino Del Regno wrote:
> Il 25/05/23 04:12, Yunfei Dong ha scritto:
>> Getting dbgfs help information with command "echo -help > vdec".
>>
>> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>> ---
>>   .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 24 ++++++++++++++++++-
>>   1 file changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
>> index 237d0dc8a1fc..2372fc449b45 100644
>> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
>> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
>> @@ -52,6 +52,23 @@ static void mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_ctx *ctx, char *buf
>>   	*used += curr_len;
>>   }
>>   
>> +static void mtk_vdec_dbgfs_get_help(char *buf, int *used, int total)
>> +{
>> +	int curr_len;
>> +
>> +	curr_len = snprintf(buf + *used, total - *used,
>> +			    "help: (1: echo -'info' > vdec 2: cat vdec)\n");
>> +	*used += curr_len;
>> +
>> +	curr_len = snprintf(buf + *used, total - *used,
>> +			    "\t-picinfo: get resolution\n");
>> +	*used += curr_len;
>> +
>> +	curr_len = snprintf(buf + *used, total - *used,
>> +			    "\t-format: get output & capture queue format\n");
>> +	*used += curr_len;
>> +}
>> +
>>   static ssize_t mtk_vdec_dbgfs_write(struct file *filp, const char __user *ubuf,
>>   				    size_t count, loff_t *ppos)
>>   {
>> @@ -84,6 +101,11 @@ static ssize_t mtk_vdec_dbgfs_read(struct file *filp, char __user *ubuf,
>>   	if (!buf)
>>   		return -ENOMEM;
>>   
>> +	if (strstr(dbgfs->dbgfs_buf, "-help")) {
> 
> I would print the help strings in two conditions:
> 1. -help
> 2. (nothing)
> 
> ...so that if you don't echo anything to vdec (no params), you get the help text.
> Otherwise, you would have to know that "-help" is a parameter that gives you help
> text in the first place.
> 
> As for this commit "as is", it works as intended and it is useful to retrieve
> the help text; you can either send a followup commit that extends the help to
> the corner case that I've explained, or send a v6 adding that to this same commit.
> 
> I would prefer to see a v6 -- BUT -- since this series was sent a long time ago,
> you will get my R-b and I will leave the final choice to Hans.
> 
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> 
> 

I prefer a v6, rebased on top of the media_stage tree.

Regards,

	Hans

Re: [PATCH v5,8/8] media: mediatek: vcodec: Add dbgfs help function

[Yunfei Dong (董云飞)]

Hi Hans,

On Tue, 2023-05-30 at 12:33 +0200, Hans Verkuil wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 5/30/23 12:06, AngeloGioacchino Del Regno wrote:
> > Il 25/05/23 04:12, Yunfei Dong ha scritto:
> >> Getting dbgfs help information with command "echo -help > vdec".
> >>
> >> Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> >> ---
> >>   .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 24
> ++++++++++++++++++-
> >>   1 file changed, 23 insertions(+), 1 deletion(-)
> >>
> >> diff --git
> a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
> b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
> >> index 237d0dc8a1fc..2372fc449b45 100644
> >> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
> >> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
> >> @@ -52,6 +52,23 @@ static void
> mtk_vdec_dbgfs_get_format_type(struct mtk_vcodec_ctx *ctx, char *buf
> >>   *used += curr_len;
> >>   }
> >>   
> >> +static void mtk_vdec_dbgfs_get_help(char *buf, int *used, int
> total)
> >> +{
> >> +int curr_len;
> >> +
> >> +curr_len = snprintf(buf + *used, total - *used,
> >> +    "help: (1: echo -'info' > vdec 2: cat vdec)\n");
> >> +*used += curr_len;
> >> +
> >> +curr_len = snprintf(buf + *used, total - *used,
> >> +    "\t-picinfo: get resolution\n");
> >> +*used += curr_len;
> >> +
> >> +curr_len = snprintf(buf + *used, total - *used,
> >> +    "\t-format: get output & capture queue format\n");
> >> +*used += curr_len;
> >> +}
> >> +
> >>   static ssize_t mtk_vdec_dbgfs_write(struct file *filp, const
> char __user *ubuf,
> >>       size_t count, loff_t *ppos)
> >>   {
> >> @@ -84,6 +101,11 @@ static ssize_t mtk_vdec_dbgfs_read(struct file
> *filp, char __user *ubuf,
> >>   if (!buf)
> >>   return -ENOMEM;
> >>   
> >> +if (strstr(dbgfs->dbgfs_buf, "-help")) {
> > 
> > I would print the help strings in two conditions:
> > 1. -help
> > 2. (nothing)
> > 
> > ...so that if you don't echo anything to vdec (no params), you get
> the help text.
> > Otherwise, you would have to know that "-help" is a parameter that
> gives you help
> > text in the first place.
> > 
> > As for this commit "as is", it works as intended and it is useful
> to retrieve
> > the help text; you can either send a followup commit that extends
> the help to
> > the corner case that I've explained, or send a v6 adding that to
> this same commit.
> > 
> > I would prefer to see a v6 -- BUT -- since this series was sent a
> long time ago,
> > you will get my R-b and I will leave the final choice to Hans.
> > 
> > Reviewed-by: AngeloGioacchino Del Regno <
> angelogioacchino.delregno@collabora.com>
> > 
> > 
> 
> I prefer a v6, rebased on top of the media_stage tree.
> 
> Regards,
> 
> Hans

I already have sent patch v6 to review.

Best Regards,
Yunfei Dong

Re: [PATCH v5,0/8] media: mediatek: vcodec: Add debugfs file for decode and encode

[Yunfei Dong (董云飞)]

Hi Hans,

Sorry to disturb you.

Could you please help to review and apply this patch series if it is ok
for you? Or whose review is expected before you can apply?

Best Regards,
Yunfei Dong

On Thu, 2023-05-25 at 10:12 +0800, Yunfei Dong wrote:
> Need to change kernel driver to open decode and encode debug log at
> current period,
> it's very unreasonable. Adding debugfs common interface to support
> decode and encode,
> using echo command to control debug log level and getting useful
> information for each
> instance.
> 
> patch 1 add dbgfs common interface.
> patch 2~5 support decode.
> patch 6~7 support encode
> patch 8 add help function
> ---
> changed with v4:
> - rebase to the top of media stage header.
> 
> changed with v3:
> - add help function for patch 8
> - remove append '\0' and enlarge buffer size for patch 4
> 
> changed with v2:
> - using pr_debug and dev_dbg instead of pr_info for patch 2.
> - fix word fail: informatiaoin -> information for patch 3.
> - used to print each instance format information for patch 5.
> 
> changed with v1:
> - add new patch 4 and 5.
> - using cmd 'cat vdec' to show debug information instead of pr_info
> directly.
> ---
> Yunfei Dong (8):
>   media: mediatek: vcodec: Add debugfs interface to get debug
>     information
>   media: mediatek: vcodec: Add debug params to control different log
>     level
>   media: mediatek: vcodec: Add a debugfs file to get different useful
>     information
>   media: mediatek: vcodec: Get each context resolution information
>   media: mediatek: vcodec: Get each instance format type
>   media: mediatek: vcodec: Change dbgfs interface to support encode
>   media: mediatek: vcodec: Add encode to support dbgfs
>   media: mediatek: vcodec: Add dbgfs help function
> 
>  .../media/platform/mediatek/vcodec/Makefile   |   6 +
>  .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 216
> ++++++++++++++++++
>  .../mediatek/vcodec/mtk_vcodec_dbgfs.h        |  72 ++++++
>  .../mediatek/vcodec/mtk_vcodec_dec_drv.c      |   4 +
>  .../platform/mediatek/vcodec/mtk_vcodec_drv.h |   4 +
>  .../mediatek/vcodec/mtk_vcodec_enc_drv.c      |   2 +
>  .../mediatek/vcodec/mtk_vcodec_util.c         |   8 +
>  .../mediatek/vcodec/mtk_vcodec_util.h         |  26 ++-
>  8 files changed, 335 insertions(+), 3 deletions(-)
>  create mode 100644
> drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
>  create mode 100644
> drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
> 

Re: [PATCH v5,0/8] media: mediatek: vcodec: Add debugfs file for decode and encode

[Hans Verkuil]

On 5/30/23 10:27, Yunfei Dong (董云飞) wrote:
> Hi Hans,
> 
> Sorry to disturb you.
> 
> Could you please help to review and apply this patch series if it is ok
> for you? Or whose review is expected before you can apply?

AngeloGioacchino Del Regno reviewed this series before, so I'd like to
have his OK for this series.

Regards,

	Hans

> 
> Best Regards,
> Yunfei Dong
> 
> On Thu, 2023-05-25 at 10:12 +0800, Yunfei Dong wrote:
>> Need to change kernel driver to open decode and encode debug log at
>> current period,
>> it's very unreasonable. Adding debugfs common interface to support
>> decode and encode,
>> using echo command to control debug log level and getting useful
>> information for each
>> instance.
>>
>> patch 1 add dbgfs common interface.
>> patch 2~5 support decode.
>> patch 6~7 support encode
>> patch 8 add help function
>> ---
>> changed with v4:
>> - rebase to the top of media stage header.
>>
>> changed with v3:
>> - add help function for patch 8
>> - remove append '\0' and enlarge buffer size for patch 4
>>
>> changed with v2:
>> - using pr_debug and dev_dbg instead of pr_info for patch 2.
>> - fix word fail: informatiaoin -> information for patch 3.
>> - used to print each instance format information for patch 5.
>>
>> changed with v1:
>> - add new patch 4 and 5.
>> - using cmd 'cat vdec' to show debug information instead of pr_info
>> directly.
>> ---
>> Yunfei Dong (8):
>>   media: mediatek: vcodec: Add debugfs interface to get debug
>>     information
>>   media: mediatek: vcodec: Add debug params to control different log
>>     level
>>   media: mediatek: vcodec: Add a debugfs file to get different useful
>>     information
>>   media: mediatek: vcodec: Get each context resolution information
>>   media: mediatek: vcodec: Get each instance format type
>>   media: mediatek: vcodec: Change dbgfs interface to support encode
>>   media: mediatek: vcodec: Add encode to support dbgfs
>>   media: mediatek: vcodec: Add dbgfs help function
>>
>>  .../media/platform/mediatek/vcodec/Makefile   |   6 +
>>  .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 216
>> ++++++++++++++++++
>>  .../mediatek/vcodec/mtk_vcodec_dbgfs.h        |  72 ++++++
>>  .../mediatek/vcodec/mtk_vcodec_dec_drv.c      |   4 +
>>  .../platform/mediatek/vcodec/mtk_vcodec_drv.h |   4 +
>>  .../mediatek/vcodec/mtk_vcodec_enc_drv.c      |   2 +
>>  .../mediatek/vcodec/mtk_vcodec_util.c         |   8 +
>>  .../mediatek/vcodec/mtk_vcodec_util.h         |  26 ++-
>>  8 files changed, 335 insertions(+), 3 deletions(-)
>>  create mode 100644
>> drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
>>  create mode 100644
>> drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
>>

Re: [PATCH v5,0/8] media: mediatek: vcodec: Add debugfs file for decode and encode

[Chen-Yu Tsai]

On Thu, May 25, 2023 at 10:12 AM Yunfei Dong <yunfei.dong@mediatek.com> wrote:
>
> Need to change kernel driver to open decode and encode debug log at current period,
> it's very unreasonable. Adding debugfs common interface to support decode and encode,
> using echo command to control debug log level and getting useful information for each
> instance.
>
> patch 1 add dbgfs common interface.
> patch 2~5 support decode.
> patch 6~7 support encode
> patch 8 add help function

I find the interface kind of weird. A lot of debugfs usage in other places
just dumps out the current state. Here you are writing to it to ask it to
do a snapshot, and then you read it later. This ends up requiring manual
management of a buffer, instead of using seq_file, which is commonly used
for virtual files.

ChenYu

> ---
> changed with v4:
> - rebase to the top of media stage header.
>
> changed with v3:
> - add help function for patch 8
> - remove append '\0' and enlarge buffer size for patch 4
>
> changed with v2:
> - using pr_debug and dev_dbg instead of pr_info for patch 2.
> - fix word fail: informatiaoin -> information for patch 3.
> - used to print each instance format information for patch 5.
>
> changed with v1:
> - add new patch 4 and 5.
> - using cmd 'cat vdec' to show debug information instead of pr_info directly.
> ---
> Yunfei Dong (8):
>   media: mediatek: vcodec: Add debugfs interface to get debug
>     information
>   media: mediatek: vcodec: Add debug params to control different log
>     level
>   media: mediatek: vcodec: Add a debugfs file to get different useful
>     information
>   media: mediatek: vcodec: Get each context resolution information
>   media: mediatek: vcodec: Get each instance format type
>   media: mediatek: vcodec: Change dbgfs interface to support encode
>   media: mediatek: vcodec: Add encode to support dbgfs
>   media: mediatek: vcodec: Add dbgfs help function
>
>  .../media/platform/mediatek/vcodec/Makefile   |   6 +
>  .../mediatek/vcodec/mtk_vcodec_dbgfs.c        | 216 ++++++++++++++++++
>  .../mediatek/vcodec/mtk_vcodec_dbgfs.h        |  72 ++++++
>  .../mediatek/vcodec/mtk_vcodec_dec_drv.c      |   4 +
>  .../platform/mediatek/vcodec/mtk_vcodec_drv.h |   4 +
>  .../mediatek/vcodec/mtk_vcodec_enc_drv.c      |   2 +
>  .../mediatek/vcodec/mtk_vcodec_util.c         |   8 +
>  .../mediatek/vcodec/mtk_vcodec_util.h         |  26 ++-
>  8 files changed, 335 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.c
>  create mode 100644 drivers/media/platform/mediatek/vcodec/mtk_vcodec_dbgfs.h
>
> --
> 2.25.1
>