All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/2] add debug capabilities to v4l2 encoder for STMicroelectronics SOC
@ 2016-09-12 16:01 Jean-Christophe Trotin
  2016-09-12 16:01 ` [PATCH v1 1/2] st-hva: encoding summary at instance release Jean-Christophe Trotin
  2016-09-12 16:01 ` [PATCH v1 2/2] st-hva: add debug file system Jean-Christophe Trotin
  0 siblings, 2 replies; 6+ messages in thread
From: Jean-Christophe Trotin @ 2016-09-12 16:01 UTC (permalink / raw)
  To: linux-media, Hans Verkuil
  Cc: kernel, Benjamin Gaignard, Yannick Fertre, Hugues Fruchet,
	Jean-Christophe Trotin

version 1:
- Initial submission

With the first patch, a short summary about the encoding operation is
unconditionnaly printed at each instance closing:
- information about the stream (format, profile, level, resolution)
- performance information (number of encoded frames, maximum framerate)
- potential (system, encoding...) errors

With the second patch, 4 static debugfs entries are created to dump:
- the device-related information ("st-hva/device")
- the list of registered encoders ("st-hva/encoders")
- the current values of the hva registers ("st-hva/regs")
- the information about the last closed instance ("st-hva/last")

Moreover, a debugfs entry is dynamically created for each opened instance,
("st-hva/<instance identifier>") to dump:
- the information about the stream (profile, level, resolution,
  alignment...)
- the control parameters (bitrate mode, framerate, GOP size...)
- the potential (system, encoding...) errors
- the performance information about the encoding (HW processing
  duration, average bitrate, average framerate...)
Each time a running instance is closed, its context (including the debug
information) is saved to feed, on demand, the last closed instance debugfs
entry.

These debug capabilities are mainly implemented in the hva-debug.c file.

Jean-Christophe Trotin (2):
  st-hva: encoding summary at instance release
  st-hva: add debug file system

 drivers/media/platform/sti/hva/Makefile    |   2 +-
 drivers/media/platform/sti/hva/hva-debug.c | 487 +++++++++++++++++++++++++++++
 drivers/media/platform/sti/hva/hva-h264.c  |   6 +
 drivers/media/platform/sti/hva/hva-hw.c    |  44 +++
 drivers/media/platform/sti/hva/hva-hw.h    |   1 +
 drivers/media/platform/sti/hva/hva-mem.c   |   5 +-
 drivers/media/platform/sti/hva/hva-v4l2.c  |  41 ++-
 drivers/media/platform/sti/hva/hva.h       |  85 ++++-
 8 files changed, 656 insertions(+), 15 deletions(-)
 create mode 100644 drivers/media/platform/sti/hva/hva-debug.c

-- 
1.9.1


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

* [PATCH v1 1/2] st-hva: encoding summary at instance release
  2016-09-12 16:01 [PATCH v1 0/2] add debug capabilities to v4l2 encoder for STMicroelectronics SOC Jean-Christophe Trotin
@ 2016-09-12 16:01 ` Jean-Christophe Trotin
  2016-09-15 10:24   ` Hans Verkuil
  2016-09-12 16:01 ` [PATCH v1 2/2] st-hva: add debug file system Jean-Christophe Trotin
  1 sibling, 1 reply; 6+ messages in thread
From: Jean-Christophe Trotin @ 2016-09-12 16:01 UTC (permalink / raw)
  To: linux-media, Hans Verkuil
  Cc: kernel, Benjamin Gaignard, Yannick Fertre, Hugues Fruchet,
	Jean-Christophe Trotin

This patch prints unconditionnaly a short summary about the encoding
operation at each instance closing, for debug purpose:
- information about the stream (format, profile, level, resolution)
- performance information (number of encoded frames, maximum framerate)
- potential (system, encoding...) errors

Signed-off-by: Yannick Fertre <yannick.fertre@st.com>
Signed-off-by: Jean-Christophe Trotin <jean-christophe.trotin@st.com>
---
 drivers/media/platform/sti/hva/Makefile    |   2 +-
 drivers/media/platform/sti/hva/hva-debug.c | 125 +++++++++++++++++++++++++++++
 drivers/media/platform/sti/hva/hva-h264.c  |   6 ++
 drivers/media/platform/sti/hva/hva-hw.c    |   5 ++
 drivers/media/platform/sti/hva/hva-mem.c   |   5 +-
 drivers/media/platform/sti/hva/hva-v4l2.c  |  30 ++++---
 drivers/media/platform/sti/hva/hva.h       |  27 +++++++
 7 files changed, 188 insertions(+), 12 deletions(-)
 create mode 100644 drivers/media/platform/sti/hva/hva-debug.c

diff --git a/drivers/media/platform/sti/hva/Makefile b/drivers/media/platform/sti/hva/Makefile
index ffb69ce..0895d2d 100644
--- a/drivers/media/platform/sti/hva/Makefile
+++ b/drivers/media/platform/sti/hva/Makefile
@@ -1,2 +1,2 @@
 obj-$(CONFIG_VIDEO_STI_HVA) := st-hva.o
-st-hva-y := hva-v4l2.o hva-hw.o hva-mem.o hva-h264.o
+st-hva-y := hva-v4l2.o hva-hw.o hva-mem.o hva-h264.o hva-debug.o
diff --git a/drivers/media/platform/sti/hva/hva-debug.c b/drivers/media/platform/sti/hva/hva-debug.c
new file mode 100644
index 0000000..71bbf32
--- /dev/null
+++ b/drivers/media/platform/sti/hva/hva-debug.c
@@ -0,0 +1,125 @@
+/*
+ * Copyright (C) STMicroelectronics SA 2015
+ * Authors: Yannick Fertre <yannick.fertre@st.com>
+ *          Hugues Fruchet <hugues.fruchet@st.com>
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include "hva.h"
+
+/*
+ * encoding summary
+ */
+
+char *hva_dbg_summary(struct hva_ctx *ctx)
+{
+	struct hva_streaminfo *stream = &ctx->streaminfo;
+	struct hva_frameinfo *frame = &ctx->frameinfo;
+	struct hva_ctx_dbg *dbg = &ctx->dbg;
+	static char str[200] = "";
+	char *cur = str;
+	size_t left = sizeof(str);
+	int cnt = 0;
+	int ret = 0;
+	u32 errors;
+
+	/* frame info */
+	cur += cnt;
+	left -= cnt;
+	ret = snprintf(cur, left, "%4.4s %dx%d > ",
+		       (char *)&frame->pixelformat,
+		       frame->aligned_width, frame->aligned_height);
+	cnt = (left > ret ? ret : left);
+
+	/* stream info */
+	cur += cnt;
+	left -= cnt;
+	ret = snprintf(cur, left, "%4.4s %dx%d %s %s: ",
+		       (char *)&stream->streamformat,
+		       stream->width, stream->height,
+		       stream->profile, stream->level);
+	cnt = (left > ret ? ret : left);
+
+	/* performance info */
+	cur += cnt;
+	left -= cnt;
+	ret = snprintf(cur, left, "%d frames encoded", dbg->cnt_duration);
+	cnt = (left > ret ? ret : left);
+
+	if (dbg->cnt_duration && dbg->total_duration) {
+		u64 div;
+		u32 fps;
+
+		div = (u64)dbg->cnt_duration * 100000;
+		do_div(div, dbg->total_duration);
+		fps = (u32)div;
+		cur += cnt;
+		left -= cnt;
+		ret = snprintf(cur, left, ", max fps (0.1Hz)=%d", fps);
+		cnt = (left > ret ? ret : left);
+	}
+
+	/* error info */
+	errors = dbg->sys_errors + dbg->encode_errors + dbg->frame_errors;
+	if (errors) {
+		cur += cnt;
+		left -= cnt;
+		ret = snprintf(cur, left, ", %d errors", errors);
+		cnt = (left > ret ? ret : left);
+	}
+
+	return str;
+}
+
+/*
+ * performance debug info
+ */
+
+void hva_dbg_perf_begin(struct hva_ctx *ctx)
+{
+	struct hva_ctx_dbg *dbg = &ctx->dbg;
+
+	dbg->begin = ktime_get();
+
+	/*
+	 * filter sequences valid for performance:
+	 * - begin/begin (no stream available) is an invalid sequence
+	 * - begin/end is a valid sequence
+	 */
+	dbg->is_valid_period = false;
+}
+
+void hva_dbg_perf_end(struct hva_ctx *ctx, struct hva_stream *stream)
+{
+	struct device *dev = ctx_to_dev(ctx);
+	u64 div;
+	u32 duration;
+	u32 bytesused;
+	u32 timestamp;
+	struct hva_ctx_dbg *dbg = &ctx->dbg;
+	ktime_t end = ktime_get();
+
+	/* stream bytesused and timestamp in us */
+	bytesused = vb2_get_plane_payload(&stream->vbuf.vb2_buf, 0);
+	div = stream->vbuf.vb2_buf.timestamp;
+	do_div(div, 1000);
+	timestamp = (u32)div;
+
+	/* encoding duration */
+	div = (u64)ktime_us_delta(end, dbg->begin);
+
+	dev_dbg(dev,
+		"%s perf stream[%d] dts=%d encoded using %d bytes in %d us",
+		ctx->name,
+		stream->vbuf.sequence,
+		timestamp,
+		bytesused, (u32)div);
+
+	do_div(div, 100);
+	duration = (u32)div;
+
+	dbg->total_duration += duration;
+	dbg->cnt_duration++;
+
+	dbg->is_valid_period = true;
+}
diff --git a/drivers/media/platform/sti/hva/hva-h264.c b/drivers/media/platform/sti/hva/hva-h264.c
index 8cc8467..b1e2b60 100644
--- a/drivers/media/platform/sti/hva/hva-h264.c
+++ b/drivers/media/platform/sti/hva/hva-h264.c
@@ -607,6 +607,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
 			"%s   width(%d) or height(%d) exceeds limits (%dx%d)\n",
 			pctx->name, frame_width, frame_height,
 			H264_MAX_SIZE_W, H264_MAX_SIZE_H);
+		pctx->dbg.frame_errors++;
 		return -EINVAL;
 	}
 
@@ -717,6 +718,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
 	default:
 		dev_err(dev, "%s   invalid source pixel format\n",
 			pctx->name);
+		pctx->dbg.frame_errors++;
 		return -EINVAL;
 	}
 
@@ -741,6 +743,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
 
 	if (td->framerate_den == 0) {
 		dev_err(dev, "%s   invalid framerate\n", pctx->name);
+		pctx->dbg.frame_errors++;
 		return -EINVAL;
 	}
 
@@ -831,6 +834,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
 	    (payload > MAX_SPS_PPS_SIZE)) {
 		dev_err(dev, "%s   invalid sps/pps size %d\n", pctx->name,
 			payload);
+		pctx->dbg.frame_errors++;
 		return -EINVAL;
 	}
 
@@ -842,6 +846,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
 						   (u8 *)stream->vaddr,
 						   &payload)) {
 		dev_err(dev, "%s   fail to get SEI nal\n", pctx->name);
+		pctx->dbg.frame_errors++;
 		return -EINVAL;
 	}
 
@@ -963,6 +968,7 @@ err_seq_info:
 err_ctx:
 	devm_kfree(dev, ctx);
 err:
+	pctx->dbg.sys_errors++;
 	return ret;
 }
 
diff --git a/drivers/media/platform/sti/hva/hva-hw.c b/drivers/media/platform/sti/hva/hva-hw.c
index 8d9ad1c..c84b860 100644
--- a/drivers/media/platform/sti/hva/hva-hw.c
+++ b/drivers/media/platform/sti/hva/hva-hw.c
@@ -470,6 +470,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
 
 	if (pm_runtime_get_sync(dev) < 0) {
 		dev_err(dev, "%s     failed to get pm_runtime\n", ctx->name);
+		ctx->dbg.sys_errors++;
 		ret = -EFAULT;
 		goto out;
 	}
@@ -481,6 +482,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
 		break;
 	default:
 		dev_dbg(dev, "%s     unknown command 0x%x\n", ctx->name, cmd);
+		ctx->dbg.encode_errors++;
 		ret = -EFAULT;
 		goto out;
 	}
@@ -511,6 +513,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
 					 msecs_to_jiffies(2000))) {
 		dev_err(dev, "%s     %s: time out on completion\n", ctx->name,
 			__func__);
+		ctx->dbg.encode_errors++;
 		ret = -EFAULT;
 		goto out;
 	}
@@ -518,6 +521,8 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
 	/* get encoding status */
 	ret = ctx->hw_err ? -EFAULT : 0;
 
+	ctx->dbg.encode_errors += ctx->hw_err ? 1 : 0;
+
 out:
 	disable_irq(hva->irq_its);
 	disable_irq(hva->irq_err);
diff --git a/drivers/media/platform/sti/hva/hva-mem.c b/drivers/media/platform/sti/hva/hva-mem.c
index 649f660..2b8ed81 100644
--- a/drivers/media/platform/sti/hva/hva-mem.c
+++ b/drivers/media/platform/sti/hva/hva-mem.c
@@ -17,14 +17,17 @@ int hva_mem_alloc(struct hva_ctx *ctx, u32 size, const char *name,
 	void *base;
 
 	b = devm_kzalloc(dev, sizeof(*b), GFP_KERNEL);
-	if (!b)
+	if (!b) {
+		ctx->dbg.sys_errors++;
 		return -ENOMEM;
+	}
 
 	base = dma_alloc_attrs(dev, size, &paddr, GFP_KERNEL | GFP_DMA,
 			       DMA_ATTR_WRITE_COMBINE);
 	if (!base) {
 		dev_err(dev, "%s %s : dma_alloc_attrs failed for %s (size=%d)\n",
 			ctx->name, __func__, name, size);
+		ctx->dbg.sys_errors++;
 		devm_kfree(dev, b);
 		return -ENOMEM;
 	}
diff --git a/drivers/media/platform/sti/hva/hva-v4l2.c b/drivers/media/platform/sti/hva/hva-v4l2.c
index 1696e02..fb65816 100644
--- a/drivers/media/platform/sti/hva/hva-v4l2.c
+++ b/drivers/media/platform/sti/hva/hva-v4l2.c
@@ -614,19 +614,17 @@ static int hva_s_ctrl(struct v4l2_ctrl *ctrl)
 		break;
 	case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
 		ctx->ctrls.profile = ctrl->val;
-		if (ctx->flags & HVA_FLAG_STREAMINFO)
-			snprintf(ctx->streaminfo.profile,
-				 sizeof(ctx->streaminfo.profile),
-				 "%s profile",
-				 v4l2_ctrl_get_menu(ctrl->id)[ctrl->val]);
+		snprintf(ctx->streaminfo.profile,
+			 sizeof(ctx->streaminfo.profile),
+			 "%s profile",
+			 v4l2_ctrl_get_menu(ctrl->id)[ctrl->val]);
 		break;
 	case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
 		ctx->ctrls.level = ctrl->val;
-		if (ctx->flags & HVA_FLAG_STREAMINFO)
-			snprintf(ctx->streaminfo.level,
-				 sizeof(ctx->streaminfo.level),
-				 "level %s",
-				 v4l2_ctrl_get_menu(ctrl->id)[ctrl->val]);
+		snprintf(ctx->streaminfo.level,
+			 sizeof(ctx->streaminfo.level),
+			 "level %s",
+			 v4l2_ctrl_get_menu(ctrl->id)[ctrl->val]);
 		break;
 	case V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE:
 		ctx->ctrls.entropy_mode = ctrl->val;
@@ -793,6 +791,8 @@ static void hva_run_work(struct work_struct *work)
 	/* protect instance against reentrancy */
 	mutex_lock(&ctx->lock);
 
+	hva_dbg_perf_begin(ctx);
+
 	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
 	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 
@@ -812,6 +812,8 @@ static void hva_run_work(struct work_struct *work)
 		dst_buf->field = V4L2_FIELD_NONE;
 		dst_buf->sequence = ctx->stream_num - 1;
 
+		hva_dbg_perf_end(ctx, stream);
+
 		v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_DONE);
 		v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_DONE);
 	}
@@ -1026,6 +1028,8 @@ err:
 			v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_QUEUED);
 	}
 
+	ctx->dbg.sys_errors++;
+
 	return ret;
 }
 
@@ -1150,6 +1154,7 @@ static int hva_open(struct file *file)
 	if (ret) {
 		dev_err(dev, "%s [x:x] failed to setup controls\n",
 			HVA_PREFIX);
+		ctx->dbg.sys_errors++;
 		goto err_fh;
 	}
 	ctx->fh.ctrl_handler = &ctx->ctrl_handler;
@@ -1162,6 +1167,7 @@ static int hva_open(struct file *file)
 		ret = PTR_ERR(ctx->fh.m2m_ctx);
 		dev_err(dev, "%s failed to initialize m2m context (%d)\n",
 			HVA_PREFIX, ret);
+		ctx->dbg.sys_errors++;
 		goto err_ctrls;
 	}
 
@@ -1206,6 +1212,10 @@ static int hva_release(struct file *file)
 		hva->nb_of_instances--;
 	}
 
+	/* trace a summary of instance before closing (debug purpose) */
+	if (ctx->flags & HVA_FLAG_STREAMINFO)
+		dev_info(dev, "%s %s\n", ctx->name, hva_dbg_summary(ctx));
+
 	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
 
 	v4l2_ctrl_handler_free(&ctx->ctrl_handler);
diff --git a/drivers/media/platform/sti/hva/hva.h b/drivers/media/platform/sti/hva/hva.h
index caa5808..badc5f4 100644
--- a/drivers/media/platform/sti/hva/hva.h
+++ b/drivers/media/platform/sti/hva/hva.h
@@ -153,6 +153,27 @@ struct hva_stream {
 #define to_hva_stream(vb) \
 	container_of(vb, struct hva_stream, vbuf)
 
+/**
+ * struct hva_ctx_dbg - instance context debug info
+ *
+ * @is_valid_period: true if the sequence is valid for performance
+ * @begin:           start time of last HW task
+ * @total_duration:  total HW processing durations in 0.1ms
+ * @cnt_duration:    number of HW processings
+ * @sys_errors:      number of system errors (memory, resource, pm..)
+ * @encode_errors:   number of encoding errors (hw/driver errors)
+ * @frame_errors:    number of frame errors (format, size, header...)
+ */
+struct hva_ctx_dbg {
+	bool	is_valid_period;
+	ktime_t	begin;
+	u32	total_duration;
+	u32	cnt_duration;
+	u32	sys_errors;
+	u32	encode_errors;
+	u32	frame_errors;
+};
+
 struct hva_dev;
 struct hva_enc;
 
@@ -182,6 +203,7 @@ struct hva_enc;
  * @priv:            private codec data for this instance, allocated
  *                   by encoder @open time
  * @hw_err:          true if hardware error detected
+ * @dbg:             context debug info
  */
 struct hva_ctx {
 	struct hva_dev		        *hva_dev;
@@ -207,6 +229,7 @@ struct hva_ctx {
 	struct hva_enc			*enc;
 	void				*priv;
 	bool				hw_err;
+	struct hva_ctx_dbg		dbg;
 };
 
 #define HVA_FLAG_STREAMINFO	0x0001
@@ -312,4 +335,8 @@ struct hva_enc {
 				  struct hva_stream *stream);
 };
 
+char *hva_dbg_summary(struct hva_ctx *ctx);
+void hva_dbg_perf_begin(struct hva_ctx *ctx);
+void hva_dbg_perf_end(struct hva_ctx *ctx, struct hva_stream *stream);
+
 #endif /* HVA_H */
-- 
1.9.1


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

* [PATCH v1 2/2] st-hva: add debug file system
  2016-09-12 16:01 [PATCH v1 0/2] add debug capabilities to v4l2 encoder for STMicroelectronics SOC Jean-Christophe Trotin
  2016-09-12 16:01 ` [PATCH v1 1/2] st-hva: encoding summary at instance release Jean-Christophe Trotin
@ 2016-09-12 16:01 ` Jean-Christophe Trotin
  2016-09-15 10:30   ` Hans Verkuil
  1 sibling, 1 reply; 6+ messages in thread
From: Jean-Christophe Trotin @ 2016-09-12 16:01 UTC (permalink / raw)
  To: linux-media, Hans Verkuil
  Cc: kernel, Benjamin Gaignard, Yannick Fertre, Hugues Fruchet,
	Jean-Christophe Trotin

This patch creates 4 static debugfs entries to dump:
- the device-related information ("st-hva/device")
- the list of registered encoders ("st-hva/encoders")
- the current values of the hva registers ("st-hva/regs")
- the information about the last closed instance ("st-hva/last")

It also creates dynamically a debugfs entry for each opened instance,
("st-hva/<instance identifier>") to dump:
- the information about the stream (profile, level, resolution,
  alignment...)
- the control parameters (bitrate mode, framerate, GOP size...)
- the potential (system, encoding...) errors
- the performance information about the encoding (HW processing
  duration, average bitrate, average framerate...)
Each time a running instance is closed, its context (including the
debug information) is saved to feed, on demand, the last closed
instance debugfs entry.

Signed-off-by: Yannick Fertre <yannick.fertre@st.com>
Signed-off-by: Jean-Christophe Trotin <jean-christophe.trotin@st.com>
---
 drivers/media/platform/sti/hva/hva-debug.c | 362 +++++++++++++++++++++++++++++
 drivers/media/platform/sti/hva/hva-hw.c    |  39 ++++
 drivers/media/platform/sti/hva/hva-hw.h    |   1 +
 drivers/media/platform/sti/hva/hva-v4l2.c  |  11 +-
 drivers/media/platform/sti/hva/hva.h       |  86 +++++--
 5 files changed, 482 insertions(+), 17 deletions(-)

diff --git a/drivers/media/platform/sti/hva/hva-debug.c b/drivers/media/platform/sti/hva/hva-debug.c
index 71bbf32..433b1d4 100644
--- a/drivers/media/platform/sti/hva/hva-debug.c
+++ b/drivers/media/platform/sti/hva/hva-debug.c
@@ -5,7 +5,113 @@
  * License terms:  GNU General Public License (GPL), version 2
  */
 
+#include <linux/debugfs.h>
+
 #include "hva.h"
+#include "hva-hw.h"
+
+static void format_ctx(struct seq_file *s, struct hva_ctx *ctx)
+{
+	struct hva_streaminfo *stream = &ctx->streaminfo;
+	struct hva_frameinfo *frame = &ctx->frameinfo;
+	struct hva_controls *ctrls = &ctx->ctrls;
+	struct hva_ctx_dbg *dbg = &ctx->dbg;
+	u32 bitrate_mode, aspect, entropy, vui_sar, sei_fp;
+
+	seq_printf(s, "|-%s\n  |\n", ctx->name);
+
+	seq_printf(s, "  |-[%sframe info]\n",
+		   ctx->flags & HVA_FLAG_FRAMEINFO ? "" : "default ");
+	seq_printf(s, "  | |- pixel format=%4.4s\n"
+		      "  | |- wxh=%dx%d\n"
+		      "  | |- wxh (w/ encoder alignment constraint)=%dx%d\n"
+		      "  |\n",
+		      (char *)&frame->pixelformat,
+		      frame->width, frame->height,
+		      frame->aligned_width, frame->aligned_height);
+
+	seq_printf(s, "  |-[%sstream info]\n",
+		   ctx->flags & HVA_FLAG_STREAMINFO ? "" : "default ");
+	seq_printf(s, "  | |- stream format=%4.4s\n"
+		      "  | |- wxh=%dx%d\n"
+		      "  | |- %s\n"
+		      "  | |- %s\n"
+		      "  |\n",
+		      (char *)&stream->streamformat,
+		      stream->width, stream->height,
+		      stream->profile, stream->level);
+
+	bitrate_mode = V4L2_CID_MPEG_VIDEO_BITRATE_MODE;
+	aspect = V4L2_CID_MPEG_VIDEO_ASPECT;
+	seq_puts(s, "  |-[parameters]\n");
+	seq_printf(s, "  | |- %s\n"
+		      "  | |- bitrate=%d bps\n"
+		      "  | |- GOP size=%d\n"
+		      "  | |- video aspect=%s\n"
+		      "  | |- framerate=%d/%d\n",
+		      v4l2_ctrl_get_menu(bitrate_mode)[ctrls->bitrate_mode],
+		      ctrls->bitrate,
+		      ctrls->gop_size,
+		      v4l2_ctrl_get_menu(aspect)[ctrls->aspect],
+		      ctrls->time_per_frame.denominator,
+		      ctrls->time_per_frame.numerator);
+
+	entropy = V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE;
+	vui_sar = V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_IDC;
+	sei_fp =  V4L2_CID_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE;
+	if (stream->streamformat == V4L2_PIX_FMT_H264) {
+		seq_printf(s, "  | |- %s entropy mode\n"
+			      "  | |- CPB size=%d kB\n"
+			      "  | |- DCT8x8 enable=%s\n"
+			      "  | |- qpmin=%d\n"
+			      "  | |- qpmax=%d\n"
+			      "  | |- PAR enable=%s\n"
+			      "  | |- PAR id=%s\n"
+			      "  | |- SEI frame packing enable=%s\n"
+			      "  | |- SEI frame packing type=%s\n",
+			      v4l2_ctrl_get_menu(entropy)[ctrls->entropy_mode],
+			      ctrls->cpb_size,
+			      ctrls->dct8x8 ? "true" : "false",
+			      ctrls->qpmin,
+			      ctrls->qpmax,
+			      ctrls->vui_sar ? "true" : "false",
+			      v4l2_ctrl_get_menu(vui_sar)[ctrls->vui_sar_idc],
+			      ctrls->sei_fp ? "true" : "false",
+			      v4l2_ctrl_get_menu(sei_fp)[ctrls->sei_fp_type]);
+	}
+
+	if (dbg->sys_errors || dbg->encode_errors || dbg->frame_errors) {
+		seq_puts(s, "  |\n  |-[errors]\n");
+		seq_printf(s, "  | |- system=%d\n"
+			      "  | |- encoding=%d\n"
+			      "  | |- frame=%d\n",
+			      dbg->sys_errors,
+			      dbg->encode_errors,
+			      dbg->frame_errors);
+	}
+
+	seq_puts(s, "  |\n  |-[performances]\n");
+	seq_printf(s, "  | |- frames encoded=%d\n"
+		      "  | |- avg HW processing duration (0.1ms)=%d [min=%d, max=%d]\n"
+		      "  | |- avg encoding period (0.1ms)=%d [min=%d, max=%d]\n"
+		      "  | |- avg fps (0.1Hz)=%d\n"
+		      "  | |- max reachable fps (0.1Hz)=%d\n"
+		      "  | |- avg bitrate (kbps)=%d [min=%d, max=%d]\n"
+		      "  | |- last bitrate (kbps)=%d\n",
+		      dbg->cnt_duration,
+		      dbg->avg_duration,
+		      dbg->min_duration,
+		      dbg->max_duration,
+		      dbg->avg_period,
+		      dbg->min_period,
+		      dbg->max_period,
+		      dbg->avg_fps,
+		      dbg->max_fps,
+		      dbg->avg_bitrate,
+		      dbg->min_bitrate,
+		      dbg->max_bitrate,
+		      dbg->last_bitrate);
+}
 
 /*
  * encoding summary
@@ -77,10 +183,52 @@ char *hva_dbg_summary(struct hva_ctx *ctx)
 
 void hva_dbg_perf_begin(struct hva_ctx *ctx)
 {
+	u64 div;
+	u32 period;
+	u32 bitrate;
 	struct hva_ctx_dbg *dbg = &ctx->dbg;
+	ktime_t prev = dbg->begin;
 
 	dbg->begin = ktime_get();
 
+	if (dbg->is_valid_period) {
+		/* encoding period */
+		div = (u64)ktime_us_delta(dbg->begin, prev);
+		do_div(div, 100);
+		period = (u32)div;
+		dbg->min_period = min(period, dbg->min_period);
+		dbg->max_period = max(period, dbg->max_period);
+		dbg->total_period += period;
+		dbg->cnt_period++;
+
+		/*
+		 * minimum and maximum bitrates are based on the
+		 * encoding period values upon a window of 32 samples
+		 */
+		dbg->window_duration += period;
+		dbg->cnt_window++;
+		if (dbg->cnt_window >= 32) {
+			/*
+			 * bitrate in kbps = (size * 8 / 1000) /
+			 *                   (duration / 10000)
+			 *                 = size * 80 / duration
+			 */
+			if (dbg->window_duration > 0) {
+				div = (u64)dbg->window_stream_size * 80;
+				do_div(div, dbg->window_duration);
+				bitrate = (u32)div;
+				dbg->last_bitrate = bitrate;
+				dbg->min_bitrate = min(bitrate,
+						       dbg->min_bitrate);
+				dbg->max_bitrate = max(bitrate,
+						       dbg->max_bitrate);
+			}
+			dbg->window_stream_size = 0;
+			dbg->window_duration = 0;
+			dbg->cnt_window = 0;
+		}
+	}
+
 	/*
 	 * filter sequences valid for performance:
 	 * - begin/begin (no stream available) is an invalid sequence
@@ -118,8 +266,222 @@ void hva_dbg_perf_end(struct hva_ctx *ctx, struct hva_stream *stream)
 	do_div(div, 100);
 	duration = (u32)div;
 
+	dbg->min_duration = min(duration, dbg->min_duration);
+	dbg->max_duration = max(duration, dbg->max_duration);
 	dbg->total_duration += duration;
 	dbg->cnt_duration++;
 
+	/*
+	 * the average bitrate is based on the total stream size
+	 * and the total encoding periods
+	 */
+	dbg->total_stream_size += bytesused;
+	dbg->window_stream_size += bytesused;
+
 	dbg->is_valid_period = true;
 }
+
+static void hva_dbg_perf_compute(struct hva_ctx *ctx)
+{
+	u64 div;
+	struct hva_ctx_dbg *dbg = &ctx->dbg;
+
+	if (dbg->cnt_duration > 0) {
+		div = (u64)dbg->total_duration;
+		do_div(div, dbg->cnt_duration);
+		dbg->avg_duration = (u32)div;
+	} else {
+		dbg->avg_duration = 0;
+	}
+
+	if (dbg->total_duration > 0) {
+		div = (u64)dbg->cnt_duration * 100000;
+		do_div(div, dbg->total_duration);
+		dbg->max_fps = (u32)div;
+	} else {
+		dbg->max_fps = 0;
+	}
+
+	if (dbg->cnt_period > 0) {
+		div = (u64)dbg->total_period;
+		do_div(div, dbg->cnt_period);
+		dbg->avg_period = (u32)div;
+	} else {
+		dbg->avg_period = 0;
+	}
+
+	if (dbg->total_period > 0) {
+		div = (u64)dbg->cnt_period * 100000;
+		do_div(div, dbg->total_period);
+		dbg->avg_fps = (u32)div;
+	} else {
+		dbg->avg_fps = 0;
+	}
+
+	if (dbg->total_period > 0) {
+		/*
+		 * bitrate in kbps = (video size * 8 / 1000) /
+		 *                   (video duration / 10000)
+		 *                 = video size * 80 / video duration
+		 */
+		div = (u64)dbg->total_stream_size * 80;
+		do_div(div, dbg->total_period);
+		dbg->avg_bitrate = (u32)div;
+	} else {
+		dbg->avg_bitrate = 0;
+	}
+}
+
+/*
+ * device debug info
+ */
+
+static int hva_dbg_device(struct seq_file *s, void *data)
+{
+	struct hva_dev *hva = s->private;
+
+	seq_printf(s, "[%s]\n", hva->v4l2_dev.name);
+	seq_printf(s, "registered as /dev/video%d\n", hva->vdev->num);
+
+	return 0;
+}
+
+static int hva_dbg_encoders(struct seq_file *s, void *data)
+{
+	struct hva_dev *hva = s->private;
+	unsigned int i = 0;
+
+	seq_printf(s, "[encoders]\n|- %d registered encoders:\n",
+		   hva->nb_of_encoders);
+
+	while (hva->encoders[i]) {
+		seq_printf(s, "|- %s: %4.4s => %4.4s\n", hva->encoders[i]->name,
+			   (char *)&hva->encoders[i]->pixelformat,
+			   (char *)&hva->encoders[i]->streamformat);
+		i++;
+	}
+
+	return 0;
+}
+
+static int hva_dbg_last(struct seq_file *s, void *data)
+{
+	struct hva_dev *hva = s->private;
+	struct hva_ctx *last_ctx = &hva->dbg.last_ctx;
+
+	if (last_ctx->flags & HVA_FLAG_STREAMINFO) {
+		seq_puts(s, "[last encoding]\n");
+
+		hva_dbg_perf_compute(last_ctx);
+		format_ctx(s, last_ctx);
+	} else {
+		seq_puts(s, "[no information recorded about last encoding]\n");
+	}
+
+	return 0;
+}
+
+static int hva_dbg_regs(struct seq_file *s, void *data)
+{
+	struct hva_dev *hva = s->private;
+
+	hva_hw_dump_regs(hva, s);
+
+	return 0;
+}
+
+#define hva_dbg_declare(name)						  \
+	static int hva_dbg_##name##_open(struct inode *i, struct file *f) \
+	{								  \
+		return single_open(f, hva_dbg_##name, i->i_private);	  \
+	}								  \
+	static const struct file_operations hva_dbg_##name##_fops = {	  \
+		.open		= hva_dbg_##name##_open,		  \
+		.read		= seq_read,				  \
+		.llseek		= seq_lseek,				  \
+		.release	= single_release,			  \
+	}
+
+#define hva_dbg_create_entry(name)					 \
+	debugfs_create_file(#name, S_IRUGO, hva->dbg.debugfs_entry, hva, \
+			    &hva_dbg_##name##_fops)
+
+hva_dbg_declare(device);
+hva_dbg_declare(encoders);
+hva_dbg_declare(last);
+hva_dbg_declare(regs);
+
+void hva_debugfs_create(struct hva_dev *hva)
+{
+	hva->dbg.debugfs_entry = debugfs_create_dir(HVA_NAME, NULL);
+	if (!hva->dbg.debugfs_entry)
+		goto err;
+
+	if (!hva_dbg_create_entry(device))
+		goto err;
+
+	if (!hva_dbg_create_entry(encoders))
+		goto err;
+
+	if (!hva_dbg_create_entry(last))
+		goto err;
+
+	if (!hva_dbg_create_entry(regs))
+		goto err;
+
+	return;
+
+err:
+	hva_debugfs_remove(hva);
+}
+
+void hva_debugfs_remove(struct hva_dev *hva)
+{
+	debugfs_remove_recursive(hva->dbg.debugfs_entry);
+	hva->dbg.debugfs_entry = NULL;
+}
+
+/*
+ * context (instance) debug info
+ */
+
+static int hva_dbg_ctx(struct seq_file *s, void *data)
+{
+	struct hva_ctx *ctx = s->private;
+
+	seq_printf(s, "[running encoding %d]\n", ctx->id);
+
+	hva_dbg_perf_compute(ctx);
+	format_ctx(s, ctx);
+
+	return 0;
+}
+
+hva_dbg_declare(ctx);
+
+void hva_dbg_ctx_create(struct hva_ctx *ctx)
+{
+	struct hva_dev *hva = ctx->hva_dev;
+	char name[4] = "";
+
+	ctx->dbg.min_duration = UINT_MAX;
+	ctx->dbg.min_period = UINT_MAX;
+	ctx->dbg.min_bitrate = UINT_MAX;
+
+	snprintf(name, sizeof(name), "%d", hva->instance_id);
+
+	ctx->dbg.debugfs_entry = debugfs_create_file(name, S_IRUGO,
+						     hva->dbg.debugfs_entry,
+						     ctx, &hva_dbg_ctx_fops);
+}
+
+void hva_dbg_ctx_remove(struct hva_ctx *ctx)
+{
+	struct hva_dev *hva = ctx->hva_dev;
+
+	if (ctx->flags & HVA_FLAG_STREAMINFO)
+		/* save context before removing */
+		memcpy(&hva->dbg.last_ctx, ctx, sizeof(*ctx));
+
+	debugfs_remove(ctx->dbg.debugfs_entry);
+}
diff --git a/drivers/media/platform/sti/hva/hva-hw.c b/drivers/media/platform/sti/hva/hva-hw.c
index c84b860..d395204 100644
--- a/drivers/media/platform/sti/hva/hva-hw.c
+++ b/drivers/media/platform/sti/hva/hva-hw.c
@@ -9,6 +9,7 @@
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/seq_file.h>
 
 #include "hva.h"
 #include "hva-hw.h"
@@ -541,3 +542,41 @@ out:
 
 	return ret;
 }
+
+#define DUMP(reg) seq_printf(s, "%-30s: 0x%08X\n",\
+			     #reg, readl_relaxed(hva->regs + reg))
+
+void hva_hw_dump_regs(struct hva_dev *hva, struct seq_file *s)
+{
+	struct device *dev = hva_to_dev(hva);
+
+	mutex_lock(&hva->protect_mutex);
+
+	if (pm_runtime_get_sync(dev) < 0) {
+		seq_puts(s, "Cannot wake up IP\n");
+		mutex_unlock(&hva->protect_mutex);
+		return;
+	}
+
+	seq_printf(s, "Registers:\nReg @ = 0x%p\n", hva->regs);
+
+	DUMP(HVA_HIF_REG_RST);
+	DUMP(HVA_HIF_REG_RST_ACK);
+	DUMP(HVA_HIF_REG_MIF_CFG);
+	DUMP(HVA_HIF_REG_HEC_MIF_CFG);
+	DUMP(HVA_HIF_REG_CFL);
+	DUMP(HVA_HIF_REG_SFL);
+	DUMP(HVA_HIF_REG_LMI_ERR);
+	DUMP(HVA_HIF_REG_EMI_ERR);
+	DUMP(HVA_HIF_REG_HEC_MIF_ERR);
+	DUMP(HVA_HIF_REG_HEC_STS);
+	DUMP(HVA_HIF_REG_HVC_STS);
+	DUMP(HVA_HIF_REG_HJE_STS);
+	DUMP(HVA_HIF_REG_CNT);
+	DUMP(HVA_HIF_REG_HEC_CHKSYN_DIS);
+	DUMP(HVA_HIF_REG_CLK_GATING);
+	DUMP(HVA_HIF_REG_VERSION);
+
+	pm_runtime_put_autosuspend(dev);
+	mutex_unlock(&hva->protect_mutex);
+}
diff --git a/drivers/media/platform/sti/hva/hva-hw.h b/drivers/media/platform/sti/hva/hva-hw.h
index efb45b9..f8df405 100644
--- a/drivers/media/platform/sti/hva/hva-hw.h
+++ b/drivers/media/platform/sti/hva/hva-hw.h
@@ -38,5 +38,6 @@ int hva_hw_runtime_suspend(struct device *dev);
 int hva_hw_runtime_resume(struct device *dev);
 int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
 			struct hva_buffer *task);
+void hva_hw_dump_regs(struct hva_dev *hva, struct seq_file *s);
 
 #endif /* HVA_HW_H */
diff --git a/drivers/media/platform/sti/hva/hva-v4l2.c b/drivers/media/platform/sti/hva/hva-v4l2.c
index fb65816..71bb7b3 100644
--- a/drivers/media/platform/sti/hva/hva-v4l2.c
+++ b/drivers/media/platform/sti/hva/hva-v4l2.c
@@ -15,8 +15,6 @@
 #include "hva.h"
 #include "hva-hw.h"
 
-#define HVA_NAME "st-hva"
-
 #define MIN_FRAMES	1
 #define MIN_STREAMS	1
 
@@ -1181,6 +1179,8 @@ static int hva_open(struct file *file)
 	/* default parameters for frame and stream */
 	set_default_params(ctx);
 
+	hva_dbg_ctx_create(ctx);
+
 	dev_info(dev, "%s encoder instance created\n", ctx->name);
 
 	return 0;
@@ -1223,6 +1223,8 @@ static int hva_release(struct file *file)
 	v4l2_fh_del(&ctx->fh);
 	v4l2_fh_exit(&ctx->fh);
 
+	hva_dbg_ctx_remove(ctx);
+
 	dev_info(dev, "%s encoder instance released\n", ctx->name);
 
 	kfree(ctx);
@@ -1347,6 +1349,8 @@ static int hva_probe(struct platform_device *pdev)
 		goto err_hw;
 	}
 
+	hva_debugfs_create(hva);
+
 	hva->work_queue = create_workqueue(HVA_NAME);
 	if (!hva->work_queue) {
 		dev_err(dev, "%s %s failed to allocate work queue\n",
@@ -1368,6 +1372,7 @@ static int hva_probe(struct platform_device *pdev)
 err_work_queue:
 	destroy_workqueue(hva->work_queue);
 err_v4l2:
+	hva_debugfs_remove(hva);
 	v4l2_device_unregister(&hva->v4l2_dev);
 err_hw:
 	hva_hw_remove(hva);
@@ -1386,6 +1391,8 @@ static int hva_remove(struct platform_device *pdev)
 
 	hva_hw_remove(hva);
 
+	hva_debugfs_remove(hva);
+
 	v4l2_device_unregister(&hva->v4l2_dev);
 
 	dev_info(dev, "%s %s removed\n", HVA_PREFIX, pdev->name);
diff --git a/drivers/media/platform/sti/hva/hva.h b/drivers/media/platform/sti/hva/hva.h
index badc5f4..eef9769 100644
--- a/drivers/media/platform/sti/hva/hva.h
+++ b/drivers/media/platform/sti/hva/hva.h
@@ -21,7 +21,8 @@
 
 #define ctx_to_hdev(c)  (c->hva_dev)
 
-#define HVA_PREFIX "[---:----]"
+#define HVA_NAME	"st-hva"
+#define HVA_PREFIX	"[---:----]"
 
 extern const struct hva_enc nv12h264enc;
 extern const struct hva_enc nv21h264enc;
@@ -156,22 +157,60 @@ struct hva_stream {
 /**
  * struct hva_ctx_dbg - instance context debug info
  *
- * @is_valid_period: true if the sequence is valid for performance
- * @begin:           start time of last HW task
- * @total_duration:  total HW processing durations in 0.1ms
- * @cnt_duration:    number of HW processings
- * @sys_errors:      number of system errors (memory, resource, pm..)
- * @encode_errors:   number of encoding errors (hw/driver errors)
- * @frame_errors:    number of frame errors (format, size, header...)
+ * @debugfs_entry:      debugfs entry
+ * @is_valid_period:    true if the sequence is valid for performance
+ * @begin:              start time of last HW task
+ * @total_duration:     total HW processing durations in 0.1ms
+ * @cnt_duration:       number of HW processings
+ * @min_duration:       minimum HW processing duration in 0.1ms
+ * @max_duration:       maximum HW processing duration in 0.1ms
+ * @avg_duration:       average HW processing duration in 0.1ms
+ * @max_fps:            maximum frames encoded per second (in 0.1Hz)
+ * @total_period:       total encoding periods in 0.1ms
+ * @cnt_period:         number of periods
+ * @min_period:         minimum encoding period in 0.1ms
+ * @max_period:         maximum encoding period in 0.1ms
+ * @avg_period:         average encoding period in 0.1ms
+ * @total_stream_size:  total number of encoded bytes
+ * @avg_fps:            average frames encoded per second (in 0.1Hz)
+ * @window_duration:    duration of the sampling window in 0.1ms
+ * @cnt_window:         number of samples in the window
+ * @window_stream_size: number of encoded bytes upon the sampling window
+ * @last_bitrate:       bitrate upon the last sampling window
+ * @min_bitrate:        minimum bitrate in kbps
+ * @max_bitrate:        maximum bitrate in kbps
+ * @avg_bitrate:        average bitrate in kbps
+ * @sys_errors:         number of system errors (memory, resource, pm..)
+ * @encode_errors:      number of encoding errors (hw/driver errors)
+ * @frame_errors:       number of frame errors (format, size, header...)
  */
 struct hva_ctx_dbg {
-	bool	is_valid_period;
-	ktime_t	begin;
-	u32	total_duration;
-	u32	cnt_duration;
-	u32	sys_errors;
-	u32	encode_errors;
-	u32	frame_errors;
+	struct dentry	*debugfs_entry;
+	bool		is_valid_period;
+	ktime_t		begin;
+	u32		total_duration;
+	u32		cnt_duration;
+	u32		min_duration;
+	u32		max_duration;
+	u32		avg_duration;
+	u32		max_fps;
+	u32		total_period;
+	u32		cnt_period;
+	u32		min_period;
+	u32		max_period;
+	u32		avg_period;
+	u32		total_stream_size;
+	u32		avg_fps;
+	u32		window_duration;
+	u32		cnt_window;
+	u32		window_stream_size;
+	u32		last_bitrate;
+	u32		min_bitrate;
+	u32		max_bitrate;
+	u32		avg_bitrate;
+	u32		sys_errors;
+	u32		encode_errors;
+	u32		frame_errors;
 };
 
 struct hva_dev;
@@ -235,6 +274,17 @@ struct hva_ctx {
 #define HVA_FLAG_STREAMINFO	0x0001
 #define HVA_FLAG_FRAMEINFO	0x0002
 
+/**
+ * struct hva_dev_dbg - device debug info
+ *
+ * @debugfs_entry: debugfs entry
+ * @last_ctx:      debug information about last running instance context
+ */
+struct hva_dev_dbg {
+	struct dentry	*debugfs_entry;
+	struct hva_ctx	last_ctx;
+};
+
 #define HVA_MAX_INSTANCES	16
 #define HVA_MAX_ENCODERS	10
 #define HVA_MAX_FORMATS		HVA_MAX_ENCODERS
@@ -273,6 +323,7 @@ struct hva_ctx {
  * @lmi_err_reg:         local memory interface error register value
  * @emi_err_reg:         external memory interface error register value
  * @hec_mif_err_reg:     HEC memory interface error register value
+ * @dbg:             device debug info
  */
 struct hva_dev {
 	struct v4l2_device	v4l2_dev;
@@ -307,6 +358,7 @@ struct hva_dev {
 	u32			lmi_err_reg;
 	u32			emi_err_reg;
 	u32			hec_mif_err_reg;
+	struct hva_dev_dbg	dbg;
 };
 
 /**
@@ -335,6 +387,10 @@ struct hva_enc {
 				  struct hva_stream *stream);
 };
 
+void hva_debugfs_create(struct hva_dev *hva);
+void hva_debugfs_remove(struct hva_dev *hva);
+void hva_dbg_ctx_create(struct hva_ctx *ctx);
+void hva_dbg_ctx_remove(struct hva_ctx *ctx);
 char *hva_dbg_summary(struct hva_ctx *ctx);
 void hva_dbg_perf_begin(struct hva_ctx *ctx);
 void hva_dbg_perf_end(struct hva_ctx *ctx, struct hva_stream *stream);
-- 
1.9.1


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

* Re: [PATCH v1 1/2] st-hva: encoding summary at instance release
  2016-09-12 16:01 ` [PATCH v1 1/2] st-hva: encoding summary at instance release Jean-Christophe Trotin
@ 2016-09-15 10:24   ` Hans Verkuil
  0 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2016-09-15 10:24 UTC (permalink / raw)
  To: Jean-Christophe Trotin, linux-media
  Cc: kernel, Benjamin Gaignard, Yannick Fertre, Hugues Fruchet

Hi Jean-Christophe,

I think it would be better to add a Kconfig option to explicitly enable this encoding/performance
summary.

Wouldn't debugfs be more appropriate for this? Especially given the next patch.

Regards,

	Hans

On 09/12/2016 06:01 PM, Jean-Christophe Trotin wrote:
> This patch prints unconditionnaly a short summary about the encoding
> operation at each instance closing, for debug purpose:
> - information about the stream (format, profile, level, resolution)
> - performance information (number of encoded frames, maximum framerate)
> - potential (system, encoding...) errors
> 
> Signed-off-by: Yannick Fertre <yannick.fertre@st.com>
> Signed-off-by: Jean-Christophe Trotin <jean-christophe.trotin@st.com>
> ---
>  drivers/media/platform/sti/hva/Makefile    |   2 +-
>  drivers/media/platform/sti/hva/hva-debug.c | 125 +++++++++++++++++++++++++++++
>  drivers/media/platform/sti/hva/hva-h264.c  |   6 ++
>  drivers/media/platform/sti/hva/hva-hw.c    |   5 ++
>  drivers/media/platform/sti/hva/hva-mem.c   |   5 +-
>  drivers/media/platform/sti/hva/hva-v4l2.c  |  30 ++++---
>  drivers/media/platform/sti/hva/hva.h       |  27 +++++++
>  7 files changed, 188 insertions(+), 12 deletions(-)
>  create mode 100644 drivers/media/platform/sti/hva/hva-debug.c
> 
> diff --git a/drivers/media/platform/sti/hva/Makefile b/drivers/media/platform/sti/hva/Makefile
> index ffb69ce..0895d2d 100644
> --- a/drivers/media/platform/sti/hva/Makefile
> +++ b/drivers/media/platform/sti/hva/Makefile
> @@ -1,2 +1,2 @@
>  obj-$(CONFIG_VIDEO_STI_HVA) := st-hva.o
> -st-hva-y := hva-v4l2.o hva-hw.o hva-mem.o hva-h264.o
> +st-hva-y := hva-v4l2.o hva-hw.o hva-mem.o hva-h264.o hva-debug.o
> diff --git a/drivers/media/platform/sti/hva/hva-debug.c b/drivers/media/platform/sti/hva/hva-debug.c
> new file mode 100644
> index 0000000..71bbf32
> --- /dev/null
> +++ b/drivers/media/platform/sti/hva/hva-debug.c
> @@ -0,0 +1,125 @@
> +/*
> + * Copyright (C) STMicroelectronics SA 2015
> + * Authors: Yannick Fertre <yannick.fertre@st.com>
> + *          Hugues Fruchet <hugues.fruchet@st.com>
> + * License terms:  GNU General Public License (GPL), version 2
> + */
> +
> +#include "hva.h"
> +
> +/*
> + * encoding summary
> + */
> +
> +char *hva_dbg_summary(struct hva_ctx *ctx)
> +{
> +	struct hva_streaminfo *stream = &ctx->streaminfo;
> +	struct hva_frameinfo *frame = &ctx->frameinfo;
> +	struct hva_ctx_dbg *dbg = &ctx->dbg;
> +	static char str[200] = "";
> +	char *cur = str;
> +	size_t left = sizeof(str);
> +	int cnt = 0;
> +	int ret = 0;
> +	u32 errors;
> +
> +	/* frame info */
> +	cur += cnt;
> +	left -= cnt;
> +	ret = snprintf(cur, left, "%4.4s %dx%d > ",
> +		       (char *)&frame->pixelformat,
> +		       frame->aligned_width, frame->aligned_height);
> +	cnt = (left > ret ? ret : left);
> +
> +	/* stream info */
> +	cur += cnt;
> +	left -= cnt;
> +	ret = snprintf(cur, left, "%4.4s %dx%d %s %s: ",
> +		       (char *)&stream->streamformat,
> +		       stream->width, stream->height,
> +		       stream->profile, stream->level);
> +	cnt = (left > ret ? ret : left);
> +
> +	/* performance info */
> +	cur += cnt;
> +	left -= cnt;
> +	ret = snprintf(cur, left, "%d frames encoded", dbg->cnt_duration);
> +	cnt = (left > ret ? ret : left);
> +
> +	if (dbg->cnt_duration && dbg->total_duration) {
> +		u64 div;
> +		u32 fps;
> +
> +		div = (u64)dbg->cnt_duration * 100000;
> +		do_div(div, dbg->total_duration);
> +		fps = (u32)div;
> +		cur += cnt;
> +		left -= cnt;
> +		ret = snprintf(cur, left, ", max fps (0.1Hz)=%d", fps);
> +		cnt = (left > ret ? ret : left);
> +	}
> +
> +	/* error info */
> +	errors = dbg->sys_errors + dbg->encode_errors + dbg->frame_errors;
> +	if (errors) {
> +		cur += cnt;
> +		left -= cnt;
> +		ret = snprintf(cur, left, ", %d errors", errors);
> +		cnt = (left > ret ? ret : left);
> +	}
> +
> +	return str;
> +}
> +
> +/*
> + * performance debug info
> + */
> +
> +void hva_dbg_perf_begin(struct hva_ctx *ctx)
> +{
> +	struct hva_ctx_dbg *dbg = &ctx->dbg;
> +
> +	dbg->begin = ktime_get();
> +
> +	/*
> +	 * filter sequences valid for performance:
> +	 * - begin/begin (no stream available) is an invalid sequence
> +	 * - begin/end is a valid sequence
> +	 */
> +	dbg->is_valid_period = false;
> +}
> +
> +void hva_dbg_perf_end(struct hva_ctx *ctx, struct hva_stream *stream)
> +{
> +	struct device *dev = ctx_to_dev(ctx);
> +	u64 div;
> +	u32 duration;
> +	u32 bytesused;
> +	u32 timestamp;
> +	struct hva_ctx_dbg *dbg = &ctx->dbg;
> +	ktime_t end = ktime_get();
> +
> +	/* stream bytesused and timestamp in us */
> +	bytesused = vb2_get_plane_payload(&stream->vbuf.vb2_buf, 0);
> +	div = stream->vbuf.vb2_buf.timestamp;
> +	do_div(div, 1000);
> +	timestamp = (u32)div;
> +
> +	/* encoding duration */
> +	div = (u64)ktime_us_delta(end, dbg->begin);
> +
> +	dev_dbg(dev,
> +		"%s perf stream[%d] dts=%d encoded using %d bytes in %d us",
> +		ctx->name,
> +		stream->vbuf.sequence,
> +		timestamp,
> +		bytesused, (u32)div);
> +
> +	do_div(div, 100);
> +	duration = (u32)div;
> +
> +	dbg->total_duration += duration;
> +	dbg->cnt_duration++;
> +
> +	dbg->is_valid_period = true;
> +}
> diff --git a/drivers/media/platform/sti/hva/hva-h264.c b/drivers/media/platform/sti/hva/hva-h264.c
> index 8cc8467..b1e2b60 100644
> --- a/drivers/media/platform/sti/hva/hva-h264.c
> +++ b/drivers/media/platform/sti/hva/hva-h264.c
> @@ -607,6 +607,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>  			"%s   width(%d) or height(%d) exceeds limits (%dx%d)\n",
>  			pctx->name, frame_width, frame_height,
>  			H264_MAX_SIZE_W, H264_MAX_SIZE_H);
> +		pctx->dbg.frame_errors++;
>  		return -EINVAL;
>  	}
>  
> @@ -717,6 +718,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>  	default:
>  		dev_err(dev, "%s   invalid source pixel format\n",
>  			pctx->name);
> +		pctx->dbg.frame_errors++;
>  		return -EINVAL;
>  	}
>  
> @@ -741,6 +743,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>  
>  	if (td->framerate_den == 0) {
>  		dev_err(dev, "%s   invalid framerate\n", pctx->name);
> +		pctx->dbg.frame_errors++;
>  		return -EINVAL;
>  	}
>  
> @@ -831,6 +834,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>  	    (payload > MAX_SPS_PPS_SIZE)) {
>  		dev_err(dev, "%s   invalid sps/pps size %d\n", pctx->name,
>  			payload);
> +		pctx->dbg.frame_errors++;
>  		return -EINVAL;
>  	}
>  
> @@ -842,6 +846,7 @@ static int hva_h264_prepare_task(struct hva_ctx *pctx,
>  						   (u8 *)stream->vaddr,
>  						   &payload)) {
>  		dev_err(dev, "%s   fail to get SEI nal\n", pctx->name);
> +		pctx->dbg.frame_errors++;
>  		return -EINVAL;
>  	}
>  
> @@ -963,6 +968,7 @@ err_seq_info:
>  err_ctx:
>  	devm_kfree(dev, ctx);
>  err:
> +	pctx->dbg.sys_errors++;
>  	return ret;
>  }
>  
> diff --git a/drivers/media/platform/sti/hva/hva-hw.c b/drivers/media/platform/sti/hva/hva-hw.c
> index 8d9ad1c..c84b860 100644
> --- a/drivers/media/platform/sti/hva/hva-hw.c
> +++ b/drivers/media/platform/sti/hva/hva-hw.c
> @@ -470,6 +470,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
>  
>  	if (pm_runtime_get_sync(dev) < 0) {
>  		dev_err(dev, "%s     failed to get pm_runtime\n", ctx->name);
> +		ctx->dbg.sys_errors++;
>  		ret = -EFAULT;
>  		goto out;
>  	}
> @@ -481,6 +482,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
>  		break;
>  	default:
>  		dev_dbg(dev, "%s     unknown command 0x%x\n", ctx->name, cmd);
> +		ctx->dbg.encode_errors++;
>  		ret = -EFAULT;
>  		goto out;
>  	}
> @@ -511,6 +513,7 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
>  					 msecs_to_jiffies(2000))) {
>  		dev_err(dev, "%s     %s: time out on completion\n", ctx->name,
>  			__func__);
> +		ctx->dbg.encode_errors++;
>  		ret = -EFAULT;
>  		goto out;
>  	}
> @@ -518,6 +521,8 @@ int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
>  	/* get encoding status */
>  	ret = ctx->hw_err ? -EFAULT : 0;
>  
> +	ctx->dbg.encode_errors += ctx->hw_err ? 1 : 0;
> +
>  out:
>  	disable_irq(hva->irq_its);
>  	disable_irq(hva->irq_err);
> diff --git a/drivers/media/platform/sti/hva/hva-mem.c b/drivers/media/platform/sti/hva/hva-mem.c
> index 649f660..2b8ed81 100644
> --- a/drivers/media/platform/sti/hva/hva-mem.c
> +++ b/drivers/media/platform/sti/hva/hva-mem.c
> @@ -17,14 +17,17 @@ int hva_mem_alloc(struct hva_ctx *ctx, u32 size, const char *name,
>  	void *base;
>  
>  	b = devm_kzalloc(dev, sizeof(*b), GFP_KERNEL);
> -	if (!b)
> +	if (!b) {
> +		ctx->dbg.sys_errors++;
>  		return -ENOMEM;
> +	}
>  
>  	base = dma_alloc_attrs(dev, size, &paddr, GFP_KERNEL | GFP_DMA,
>  			       DMA_ATTR_WRITE_COMBINE);
>  	if (!base) {
>  		dev_err(dev, "%s %s : dma_alloc_attrs failed for %s (size=%d)\n",
>  			ctx->name, __func__, name, size);
> +		ctx->dbg.sys_errors++;
>  		devm_kfree(dev, b);
>  		return -ENOMEM;
>  	}
> diff --git a/drivers/media/platform/sti/hva/hva-v4l2.c b/drivers/media/platform/sti/hva/hva-v4l2.c
> index 1696e02..fb65816 100644
> --- a/drivers/media/platform/sti/hva/hva-v4l2.c
> +++ b/drivers/media/platform/sti/hva/hva-v4l2.c
> @@ -614,19 +614,17 @@ static int hva_s_ctrl(struct v4l2_ctrl *ctrl)
>  		break;
>  	case V4L2_CID_MPEG_VIDEO_H264_PROFILE:
>  		ctx->ctrls.profile = ctrl->val;
> -		if (ctx->flags & HVA_FLAG_STREAMINFO)
> -			snprintf(ctx->streaminfo.profile,
> -				 sizeof(ctx->streaminfo.profile),
> -				 "%s profile",
> -				 v4l2_ctrl_get_menu(ctrl->id)[ctrl->val]);
> +		snprintf(ctx->streaminfo.profile,
> +			 sizeof(ctx->streaminfo.profile),
> +			 "%s profile",
> +			 v4l2_ctrl_get_menu(ctrl->id)[ctrl->val]);
>  		break;
>  	case V4L2_CID_MPEG_VIDEO_H264_LEVEL:
>  		ctx->ctrls.level = ctrl->val;
> -		if (ctx->flags & HVA_FLAG_STREAMINFO)
> -			snprintf(ctx->streaminfo.level,
> -				 sizeof(ctx->streaminfo.level),
> -				 "level %s",
> -				 v4l2_ctrl_get_menu(ctrl->id)[ctrl->val]);
> +		snprintf(ctx->streaminfo.level,
> +			 sizeof(ctx->streaminfo.level),
> +			 "level %s",
> +			 v4l2_ctrl_get_menu(ctrl->id)[ctrl->val]);
>  		break;
>  	case V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE:
>  		ctx->ctrls.entropy_mode = ctrl->val;
> @@ -793,6 +791,8 @@ static void hva_run_work(struct work_struct *work)
>  	/* protect instance against reentrancy */
>  	mutex_lock(&ctx->lock);
>  
> +	hva_dbg_perf_begin(ctx);
> +
>  	src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
>  	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
>  
> @@ -812,6 +812,8 @@ static void hva_run_work(struct work_struct *work)
>  		dst_buf->field = V4L2_FIELD_NONE;
>  		dst_buf->sequence = ctx->stream_num - 1;
>  
> +		hva_dbg_perf_end(ctx, stream);
> +
>  		v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_DONE);
>  		v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_DONE);
>  	}
> @@ -1026,6 +1028,8 @@ err:
>  			v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_QUEUED);
>  	}
>  
> +	ctx->dbg.sys_errors++;
> +
>  	return ret;
>  }
>  
> @@ -1150,6 +1154,7 @@ static int hva_open(struct file *file)
>  	if (ret) {
>  		dev_err(dev, "%s [x:x] failed to setup controls\n",
>  			HVA_PREFIX);
> +		ctx->dbg.sys_errors++;
>  		goto err_fh;
>  	}
>  	ctx->fh.ctrl_handler = &ctx->ctrl_handler;
> @@ -1162,6 +1167,7 @@ static int hva_open(struct file *file)
>  		ret = PTR_ERR(ctx->fh.m2m_ctx);
>  		dev_err(dev, "%s failed to initialize m2m context (%d)\n",
>  			HVA_PREFIX, ret);
> +		ctx->dbg.sys_errors++;
>  		goto err_ctrls;
>  	}
>  
> @@ -1206,6 +1212,10 @@ static int hva_release(struct file *file)
>  		hva->nb_of_instances--;
>  	}
>  
> +	/* trace a summary of instance before closing (debug purpose) */
> +	if (ctx->flags & HVA_FLAG_STREAMINFO)
> +		dev_info(dev, "%s %s\n", ctx->name, hva_dbg_summary(ctx));
> +
>  	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
>  
>  	v4l2_ctrl_handler_free(&ctx->ctrl_handler);
> diff --git a/drivers/media/platform/sti/hva/hva.h b/drivers/media/platform/sti/hva/hva.h
> index caa5808..badc5f4 100644
> --- a/drivers/media/platform/sti/hva/hva.h
> +++ b/drivers/media/platform/sti/hva/hva.h
> @@ -153,6 +153,27 @@ struct hva_stream {
>  #define to_hva_stream(vb) \
>  	container_of(vb, struct hva_stream, vbuf)
>  
> +/**
> + * struct hva_ctx_dbg - instance context debug info
> + *
> + * @is_valid_period: true if the sequence is valid for performance
> + * @begin:           start time of last HW task
> + * @total_duration:  total HW processing durations in 0.1ms
> + * @cnt_duration:    number of HW processings
> + * @sys_errors:      number of system errors (memory, resource, pm..)
> + * @encode_errors:   number of encoding errors (hw/driver errors)
> + * @frame_errors:    number of frame errors (format, size, header...)
> + */
> +struct hva_ctx_dbg {
> +	bool	is_valid_period;
> +	ktime_t	begin;
> +	u32	total_duration;
> +	u32	cnt_duration;
> +	u32	sys_errors;
> +	u32	encode_errors;
> +	u32	frame_errors;
> +};
> +
>  struct hva_dev;
>  struct hva_enc;
>  
> @@ -182,6 +203,7 @@ struct hva_enc;
>   * @priv:            private codec data for this instance, allocated
>   *                   by encoder @open time
>   * @hw_err:          true if hardware error detected
> + * @dbg:             context debug info
>   */
>  struct hva_ctx {
>  	struct hva_dev		        *hva_dev;
> @@ -207,6 +229,7 @@ struct hva_ctx {
>  	struct hva_enc			*enc;
>  	void				*priv;
>  	bool				hw_err;
> +	struct hva_ctx_dbg		dbg;
>  };
>  
>  #define HVA_FLAG_STREAMINFO	0x0001
> @@ -312,4 +335,8 @@ struct hva_enc {
>  				  struct hva_stream *stream);
>  };
>  
> +char *hva_dbg_summary(struct hva_ctx *ctx);
> +void hva_dbg_perf_begin(struct hva_ctx *ctx);
> +void hva_dbg_perf_end(struct hva_ctx *ctx, struct hva_stream *stream);
> +
>  #endif /* HVA_H */
> 

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

* Re: [PATCH v1 2/2] st-hva: add debug file system
  2016-09-12 16:01 ` [PATCH v1 2/2] st-hva: add debug file system Jean-Christophe Trotin
@ 2016-09-15 10:30   ` Hans Verkuil
  2016-09-20  9:47     ` Jean Christophe TROTIN
  0 siblings, 1 reply; 6+ messages in thread
From: Hans Verkuil @ 2016-09-15 10:30 UTC (permalink / raw)
  To: Jean-Christophe Trotin, linux-media
  Cc: kernel, Benjamin Gaignard, Yannick Fertre, Hugues Fruchet

Same thing here: this should be enabled in a Kconfig option. That new Kconfig
option should depend on DEBUG_FS as well.

BTW (independent of these two patches): I think it would be easier to maintain
if you make a Kconfig file in the sti directory, rather than adding them to the
Kconfig in platform.

Regards,

	Hans

On 09/12/2016 06:01 PM, Jean-Christophe Trotin wrote:
> This patch creates 4 static debugfs entries to dump:
> - the device-related information ("st-hva/device")
> - the list of registered encoders ("st-hva/encoders")
> - the current values of the hva registers ("st-hva/regs")
> - the information about the last closed instance ("st-hva/last")
> 
> It also creates dynamically a debugfs entry for each opened instance,
> ("st-hva/<instance identifier>") to dump:
> - the information about the stream (profile, level, resolution,
>   alignment...)
> - the control parameters (bitrate mode, framerate, GOP size...)
> - the potential (system, encoding...) errors
> - the performance information about the encoding (HW processing
>   duration, average bitrate, average framerate...)
> Each time a running instance is closed, its context (including the
> debug information) is saved to feed, on demand, the last closed
> instance debugfs entry.
> 
> Signed-off-by: Yannick Fertre <yannick.fertre@st.com>
> Signed-off-by: Jean-Christophe Trotin <jean-christophe.trotin@st.com>
> ---
>  drivers/media/platform/sti/hva/hva-debug.c | 362 +++++++++++++++++++++++++++++
>  drivers/media/platform/sti/hva/hva-hw.c    |  39 ++++
>  drivers/media/platform/sti/hva/hva-hw.h    |   1 +
>  drivers/media/platform/sti/hva/hva-v4l2.c  |  11 +-
>  drivers/media/platform/sti/hva/hva.h       |  86 +++++--
>  5 files changed, 482 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/platform/sti/hva/hva-debug.c b/drivers/media/platform/sti/hva/hva-debug.c
> index 71bbf32..433b1d4 100644
> --- a/drivers/media/platform/sti/hva/hva-debug.c
> +++ b/drivers/media/platform/sti/hva/hva-debug.c
> @@ -5,7 +5,113 @@
>   * License terms:  GNU General Public License (GPL), version 2
>   */
>  
> +#include <linux/debugfs.h>
> +
>  #include "hva.h"
> +#include "hva-hw.h"
> +
> +static void format_ctx(struct seq_file *s, struct hva_ctx *ctx)
> +{
> +	struct hva_streaminfo *stream = &ctx->streaminfo;
> +	struct hva_frameinfo *frame = &ctx->frameinfo;
> +	struct hva_controls *ctrls = &ctx->ctrls;
> +	struct hva_ctx_dbg *dbg = &ctx->dbg;
> +	u32 bitrate_mode, aspect, entropy, vui_sar, sei_fp;
> +
> +	seq_printf(s, "|-%s\n  |\n", ctx->name);
> +
> +	seq_printf(s, "  |-[%sframe info]\n",
> +		   ctx->flags & HVA_FLAG_FRAMEINFO ? "" : "default ");
> +	seq_printf(s, "  | |- pixel format=%4.4s\n"
> +		      "  | |- wxh=%dx%d\n"
> +		      "  | |- wxh (w/ encoder alignment constraint)=%dx%d\n"
> +		      "  |\n",
> +		      (char *)&frame->pixelformat,
> +		      frame->width, frame->height,
> +		      frame->aligned_width, frame->aligned_height);
> +
> +	seq_printf(s, "  |-[%sstream info]\n",
> +		   ctx->flags & HVA_FLAG_STREAMINFO ? "" : "default ");
> +	seq_printf(s, "  | |- stream format=%4.4s\n"
> +		      "  | |- wxh=%dx%d\n"
> +		      "  | |- %s\n"
> +		      "  | |- %s\n"
> +		      "  |\n",
> +		      (char *)&stream->streamformat,
> +		      stream->width, stream->height,
> +		      stream->profile, stream->level);
> +
> +	bitrate_mode = V4L2_CID_MPEG_VIDEO_BITRATE_MODE;
> +	aspect = V4L2_CID_MPEG_VIDEO_ASPECT;
> +	seq_puts(s, "  |-[parameters]\n");
> +	seq_printf(s, "  | |- %s\n"
> +		      "  | |- bitrate=%d bps\n"
> +		      "  | |- GOP size=%d\n"
> +		      "  | |- video aspect=%s\n"
> +		      "  | |- framerate=%d/%d\n",
> +		      v4l2_ctrl_get_menu(bitrate_mode)[ctrls->bitrate_mode],
> +		      ctrls->bitrate,
> +		      ctrls->gop_size,
> +		      v4l2_ctrl_get_menu(aspect)[ctrls->aspect],
> +		      ctrls->time_per_frame.denominator,
> +		      ctrls->time_per_frame.numerator);
> +
> +	entropy = V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE;
> +	vui_sar = V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_IDC;
> +	sei_fp =  V4L2_CID_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE;
> +	if (stream->streamformat == V4L2_PIX_FMT_H264) {
> +		seq_printf(s, "  | |- %s entropy mode\n"
> +			      "  | |- CPB size=%d kB\n"
> +			      "  | |- DCT8x8 enable=%s\n"
> +			      "  | |- qpmin=%d\n"
> +			      "  | |- qpmax=%d\n"
> +			      "  | |- PAR enable=%s\n"
> +			      "  | |- PAR id=%s\n"
> +			      "  | |- SEI frame packing enable=%s\n"
> +			      "  | |- SEI frame packing type=%s\n",
> +			      v4l2_ctrl_get_menu(entropy)[ctrls->entropy_mode],
> +			      ctrls->cpb_size,
> +			      ctrls->dct8x8 ? "true" : "false",
> +			      ctrls->qpmin,
> +			      ctrls->qpmax,
> +			      ctrls->vui_sar ? "true" : "false",
> +			      v4l2_ctrl_get_menu(vui_sar)[ctrls->vui_sar_idc],
> +			      ctrls->sei_fp ? "true" : "false",
> +			      v4l2_ctrl_get_menu(sei_fp)[ctrls->sei_fp_type]);
> +	}
> +
> +	if (dbg->sys_errors || dbg->encode_errors || dbg->frame_errors) {
> +		seq_puts(s, "  |\n  |-[errors]\n");
> +		seq_printf(s, "  | |- system=%d\n"
> +			      "  | |- encoding=%d\n"
> +			      "  | |- frame=%d\n",
> +			      dbg->sys_errors,
> +			      dbg->encode_errors,
> +			      dbg->frame_errors);
> +	}
> +
> +	seq_puts(s, "  |\n  |-[performances]\n");
> +	seq_printf(s, "  | |- frames encoded=%d\n"
> +		      "  | |- avg HW processing duration (0.1ms)=%d [min=%d, max=%d]\n"
> +		      "  | |- avg encoding period (0.1ms)=%d [min=%d, max=%d]\n"
> +		      "  | |- avg fps (0.1Hz)=%d\n"
> +		      "  | |- max reachable fps (0.1Hz)=%d\n"
> +		      "  | |- avg bitrate (kbps)=%d [min=%d, max=%d]\n"
> +		      "  | |- last bitrate (kbps)=%d\n",
> +		      dbg->cnt_duration,
> +		      dbg->avg_duration,
> +		      dbg->min_duration,
> +		      dbg->max_duration,
> +		      dbg->avg_period,
> +		      dbg->min_period,
> +		      dbg->max_period,
> +		      dbg->avg_fps,
> +		      dbg->max_fps,
> +		      dbg->avg_bitrate,
> +		      dbg->min_bitrate,
> +		      dbg->max_bitrate,
> +		      dbg->last_bitrate);
> +}
>  
>  /*
>   * encoding summary
> @@ -77,10 +183,52 @@ char *hva_dbg_summary(struct hva_ctx *ctx)
>  
>  void hva_dbg_perf_begin(struct hva_ctx *ctx)
>  {
> +	u64 div;
> +	u32 period;
> +	u32 bitrate;
>  	struct hva_ctx_dbg *dbg = &ctx->dbg;
> +	ktime_t prev = dbg->begin;
>  
>  	dbg->begin = ktime_get();
>  
> +	if (dbg->is_valid_period) {
> +		/* encoding period */
> +		div = (u64)ktime_us_delta(dbg->begin, prev);
> +		do_div(div, 100);
> +		period = (u32)div;
> +		dbg->min_period = min(period, dbg->min_period);
> +		dbg->max_period = max(period, dbg->max_period);
> +		dbg->total_period += period;
> +		dbg->cnt_period++;
> +
> +		/*
> +		 * minimum and maximum bitrates are based on the
> +		 * encoding period values upon a window of 32 samples
> +		 */
> +		dbg->window_duration += period;
> +		dbg->cnt_window++;
> +		if (dbg->cnt_window >= 32) {
> +			/*
> +			 * bitrate in kbps = (size * 8 / 1000) /
> +			 *                   (duration / 10000)
> +			 *                 = size * 80 / duration
> +			 */
> +			if (dbg->window_duration > 0) {
> +				div = (u64)dbg->window_stream_size * 80;
> +				do_div(div, dbg->window_duration);
> +				bitrate = (u32)div;
> +				dbg->last_bitrate = bitrate;
> +				dbg->min_bitrate = min(bitrate,
> +						       dbg->min_bitrate);
> +				dbg->max_bitrate = max(bitrate,
> +						       dbg->max_bitrate);
> +			}
> +			dbg->window_stream_size = 0;
> +			dbg->window_duration = 0;
> +			dbg->cnt_window = 0;
> +		}
> +	}
> +
>  	/*
>  	 * filter sequences valid for performance:
>  	 * - begin/begin (no stream available) is an invalid sequence
> @@ -118,8 +266,222 @@ void hva_dbg_perf_end(struct hva_ctx *ctx, struct hva_stream *stream)
>  	do_div(div, 100);
>  	duration = (u32)div;
>  
> +	dbg->min_duration = min(duration, dbg->min_duration);
> +	dbg->max_duration = max(duration, dbg->max_duration);
>  	dbg->total_duration += duration;
>  	dbg->cnt_duration++;
>  
> +	/*
> +	 * the average bitrate is based on the total stream size
> +	 * and the total encoding periods
> +	 */
> +	dbg->total_stream_size += bytesused;
> +	dbg->window_stream_size += bytesused;
> +
>  	dbg->is_valid_period = true;
>  }
> +
> +static void hva_dbg_perf_compute(struct hva_ctx *ctx)
> +{
> +	u64 div;
> +	struct hva_ctx_dbg *dbg = &ctx->dbg;
> +
> +	if (dbg->cnt_duration > 0) {
> +		div = (u64)dbg->total_duration;
> +		do_div(div, dbg->cnt_duration);
> +		dbg->avg_duration = (u32)div;
> +	} else {
> +		dbg->avg_duration = 0;
> +	}
> +
> +	if (dbg->total_duration > 0) {
> +		div = (u64)dbg->cnt_duration * 100000;
> +		do_div(div, dbg->total_duration);
> +		dbg->max_fps = (u32)div;
> +	} else {
> +		dbg->max_fps = 0;
> +	}
> +
> +	if (dbg->cnt_period > 0) {
> +		div = (u64)dbg->total_period;
> +		do_div(div, dbg->cnt_period);
> +		dbg->avg_period = (u32)div;
> +	} else {
> +		dbg->avg_period = 0;
> +	}
> +
> +	if (dbg->total_period > 0) {
> +		div = (u64)dbg->cnt_period * 100000;
> +		do_div(div, dbg->total_period);
> +		dbg->avg_fps = (u32)div;
> +	} else {
> +		dbg->avg_fps = 0;
> +	}
> +
> +	if (dbg->total_period > 0) {
> +		/*
> +		 * bitrate in kbps = (video size * 8 / 1000) /
> +		 *                   (video duration / 10000)
> +		 *                 = video size * 80 / video duration
> +		 */
> +		div = (u64)dbg->total_stream_size * 80;
> +		do_div(div, dbg->total_period);
> +		dbg->avg_bitrate = (u32)div;
> +	} else {
> +		dbg->avg_bitrate = 0;
> +	}
> +}
> +
> +/*
> + * device debug info
> + */
> +
> +static int hva_dbg_device(struct seq_file *s, void *data)
> +{
> +	struct hva_dev *hva = s->private;
> +
> +	seq_printf(s, "[%s]\n", hva->v4l2_dev.name);
> +	seq_printf(s, "registered as /dev/video%d\n", hva->vdev->num);
> +
> +	return 0;
> +}
> +
> +static int hva_dbg_encoders(struct seq_file *s, void *data)
> +{
> +	struct hva_dev *hva = s->private;
> +	unsigned int i = 0;
> +
> +	seq_printf(s, "[encoders]\n|- %d registered encoders:\n",
> +		   hva->nb_of_encoders);
> +
> +	while (hva->encoders[i]) {
> +		seq_printf(s, "|- %s: %4.4s => %4.4s\n", hva->encoders[i]->name,
> +			   (char *)&hva->encoders[i]->pixelformat,
> +			   (char *)&hva->encoders[i]->streamformat);
> +		i++;
> +	}
> +
> +	return 0;
> +}
> +
> +static int hva_dbg_last(struct seq_file *s, void *data)
> +{
> +	struct hva_dev *hva = s->private;
> +	struct hva_ctx *last_ctx = &hva->dbg.last_ctx;
> +
> +	if (last_ctx->flags & HVA_FLAG_STREAMINFO) {
> +		seq_puts(s, "[last encoding]\n");
> +
> +		hva_dbg_perf_compute(last_ctx);
> +		format_ctx(s, last_ctx);
> +	} else {
> +		seq_puts(s, "[no information recorded about last encoding]\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static int hva_dbg_regs(struct seq_file *s, void *data)
> +{
> +	struct hva_dev *hva = s->private;
> +
> +	hva_hw_dump_regs(hva, s);
> +
> +	return 0;
> +}
> +
> +#define hva_dbg_declare(name)						  \
> +	static int hva_dbg_##name##_open(struct inode *i, struct file *f) \
> +	{								  \
> +		return single_open(f, hva_dbg_##name, i->i_private);	  \
> +	}								  \
> +	static const struct file_operations hva_dbg_##name##_fops = {	  \
> +		.open		= hva_dbg_##name##_open,		  \
> +		.read		= seq_read,				  \
> +		.llseek		= seq_lseek,				  \
> +		.release	= single_release,			  \
> +	}
> +
> +#define hva_dbg_create_entry(name)					 \
> +	debugfs_create_file(#name, S_IRUGO, hva->dbg.debugfs_entry, hva, \
> +			    &hva_dbg_##name##_fops)
> +
> +hva_dbg_declare(device);
> +hva_dbg_declare(encoders);
> +hva_dbg_declare(last);
> +hva_dbg_declare(regs);
> +
> +void hva_debugfs_create(struct hva_dev *hva)
> +{
> +	hva->dbg.debugfs_entry = debugfs_create_dir(HVA_NAME, NULL);
> +	if (!hva->dbg.debugfs_entry)
> +		goto err;
> +
> +	if (!hva_dbg_create_entry(device))
> +		goto err;
> +
> +	if (!hva_dbg_create_entry(encoders))
> +		goto err;
> +
> +	if (!hva_dbg_create_entry(last))
> +		goto err;
> +
> +	if (!hva_dbg_create_entry(regs))
> +		goto err;
> +
> +	return;
> +
> +err:
> +	hva_debugfs_remove(hva);
> +}
> +
> +void hva_debugfs_remove(struct hva_dev *hva)
> +{
> +	debugfs_remove_recursive(hva->dbg.debugfs_entry);
> +	hva->dbg.debugfs_entry = NULL;
> +}
> +
> +/*
> + * context (instance) debug info
> + */
> +
> +static int hva_dbg_ctx(struct seq_file *s, void *data)
> +{
> +	struct hva_ctx *ctx = s->private;
> +
> +	seq_printf(s, "[running encoding %d]\n", ctx->id);
> +
> +	hva_dbg_perf_compute(ctx);
> +	format_ctx(s, ctx);
> +
> +	return 0;
> +}
> +
> +hva_dbg_declare(ctx);
> +
> +void hva_dbg_ctx_create(struct hva_ctx *ctx)
> +{
> +	struct hva_dev *hva = ctx->hva_dev;
> +	char name[4] = "";
> +
> +	ctx->dbg.min_duration = UINT_MAX;
> +	ctx->dbg.min_period = UINT_MAX;
> +	ctx->dbg.min_bitrate = UINT_MAX;
> +
> +	snprintf(name, sizeof(name), "%d", hva->instance_id);
> +
> +	ctx->dbg.debugfs_entry = debugfs_create_file(name, S_IRUGO,
> +						     hva->dbg.debugfs_entry,
> +						     ctx, &hva_dbg_ctx_fops);
> +}
> +
> +void hva_dbg_ctx_remove(struct hva_ctx *ctx)
> +{
> +	struct hva_dev *hva = ctx->hva_dev;
> +
> +	if (ctx->flags & HVA_FLAG_STREAMINFO)
> +		/* save context before removing */
> +		memcpy(&hva->dbg.last_ctx, ctx, sizeof(*ctx));
> +
> +	debugfs_remove(ctx->dbg.debugfs_entry);
> +}
> diff --git a/drivers/media/platform/sti/hva/hva-hw.c b/drivers/media/platform/sti/hva/hva-hw.c
> index c84b860..d395204 100644
> --- a/drivers/media/platform/sti/hva/hva-hw.c
> +++ b/drivers/media/platform/sti/hva/hva-hw.c
> @@ -9,6 +9,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/seq_file.h>
>  
>  #include "hva.h"
>  #include "hva-hw.h"
> @@ -541,3 +542,41 @@ out:
>  
>  	return ret;
>  }
> +
> +#define DUMP(reg) seq_printf(s, "%-30s: 0x%08X\n",\
> +			     #reg, readl_relaxed(hva->regs + reg))
> +
> +void hva_hw_dump_regs(struct hva_dev *hva, struct seq_file *s)
> +{
> +	struct device *dev = hva_to_dev(hva);
> +
> +	mutex_lock(&hva->protect_mutex);
> +
> +	if (pm_runtime_get_sync(dev) < 0) {
> +		seq_puts(s, "Cannot wake up IP\n");
> +		mutex_unlock(&hva->protect_mutex);
> +		return;
> +	}
> +
> +	seq_printf(s, "Registers:\nReg @ = 0x%p\n", hva->regs);
> +
> +	DUMP(HVA_HIF_REG_RST);
> +	DUMP(HVA_HIF_REG_RST_ACK);
> +	DUMP(HVA_HIF_REG_MIF_CFG);
> +	DUMP(HVA_HIF_REG_HEC_MIF_CFG);
> +	DUMP(HVA_HIF_REG_CFL);
> +	DUMP(HVA_HIF_REG_SFL);
> +	DUMP(HVA_HIF_REG_LMI_ERR);
> +	DUMP(HVA_HIF_REG_EMI_ERR);
> +	DUMP(HVA_HIF_REG_HEC_MIF_ERR);
> +	DUMP(HVA_HIF_REG_HEC_STS);
> +	DUMP(HVA_HIF_REG_HVC_STS);
> +	DUMP(HVA_HIF_REG_HJE_STS);
> +	DUMP(HVA_HIF_REG_CNT);
> +	DUMP(HVA_HIF_REG_HEC_CHKSYN_DIS);
> +	DUMP(HVA_HIF_REG_CLK_GATING);
> +	DUMP(HVA_HIF_REG_VERSION);
> +
> +	pm_runtime_put_autosuspend(dev);
> +	mutex_unlock(&hva->protect_mutex);
> +}
> diff --git a/drivers/media/platform/sti/hva/hva-hw.h b/drivers/media/platform/sti/hva/hva-hw.h
> index efb45b9..f8df405 100644
> --- a/drivers/media/platform/sti/hva/hva-hw.h
> +++ b/drivers/media/platform/sti/hva/hva-hw.h
> @@ -38,5 +38,6 @@ int hva_hw_runtime_suspend(struct device *dev);
>  int hva_hw_runtime_resume(struct device *dev);
>  int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
>  			struct hva_buffer *task);
> +void hva_hw_dump_regs(struct hva_dev *hva, struct seq_file *s);
>  
>  #endif /* HVA_HW_H */
> diff --git a/drivers/media/platform/sti/hva/hva-v4l2.c b/drivers/media/platform/sti/hva/hva-v4l2.c
> index fb65816..71bb7b3 100644
> --- a/drivers/media/platform/sti/hva/hva-v4l2.c
> +++ b/drivers/media/platform/sti/hva/hva-v4l2.c
> @@ -15,8 +15,6 @@
>  #include "hva.h"
>  #include "hva-hw.h"
>  
> -#define HVA_NAME "st-hva"
> -
>  #define MIN_FRAMES	1
>  #define MIN_STREAMS	1
>  
> @@ -1181,6 +1179,8 @@ static int hva_open(struct file *file)
>  	/* default parameters for frame and stream */
>  	set_default_params(ctx);
>  
> +	hva_dbg_ctx_create(ctx);
> +
>  	dev_info(dev, "%s encoder instance created\n", ctx->name);
>  
>  	return 0;
> @@ -1223,6 +1223,8 @@ static int hva_release(struct file *file)
>  	v4l2_fh_del(&ctx->fh);
>  	v4l2_fh_exit(&ctx->fh);
>  
> +	hva_dbg_ctx_remove(ctx);
> +
>  	dev_info(dev, "%s encoder instance released\n", ctx->name);
>  
>  	kfree(ctx);
> @@ -1347,6 +1349,8 @@ static int hva_probe(struct platform_device *pdev)
>  		goto err_hw;
>  	}
>  
> +	hva_debugfs_create(hva);
> +
>  	hva->work_queue = create_workqueue(HVA_NAME);
>  	if (!hva->work_queue) {
>  		dev_err(dev, "%s %s failed to allocate work queue\n",
> @@ -1368,6 +1372,7 @@ static int hva_probe(struct platform_device *pdev)
>  err_work_queue:
>  	destroy_workqueue(hva->work_queue);
>  err_v4l2:
> +	hva_debugfs_remove(hva);
>  	v4l2_device_unregister(&hva->v4l2_dev);
>  err_hw:
>  	hva_hw_remove(hva);
> @@ -1386,6 +1391,8 @@ static int hva_remove(struct platform_device *pdev)
>  
>  	hva_hw_remove(hva);
>  
> +	hva_debugfs_remove(hva);
> +
>  	v4l2_device_unregister(&hva->v4l2_dev);
>  
>  	dev_info(dev, "%s %s removed\n", HVA_PREFIX, pdev->name);
> diff --git a/drivers/media/platform/sti/hva/hva.h b/drivers/media/platform/sti/hva/hva.h
> index badc5f4..eef9769 100644
> --- a/drivers/media/platform/sti/hva/hva.h
> +++ b/drivers/media/platform/sti/hva/hva.h
> @@ -21,7 +21,8 @@
>  
>  #define ctx_to_hdev(c)  (c->hva_dev)
>  
> -#define HVA_PREFIX "[---:----]"
> +#define HVA_NAME	"st-hva"
> +#define HVA_PREFIX	"[---:----]"
>  
>  extern const struct hva_enc nv12h264enc;
>  extern const struct hva_enc nv21h264enc;
> @@ -156,22 +157,60 @@ struct hva_stream {
>  /**
>   * struct hva_ctx_dbg - instance context debug info
>   *
> - * @is_valid_period: true if the sequence is valid for performance
> - * @begin:           start time of last HW task
> - * @total_duration:  total HW processing durations in 0.1ms
> - * @cnt_duration:    number of HW processings
> - * @sys_errors:      number of system errors (memory, resource, pm..)
> - * @encode_errors:   number of encoding errors (hw/driver errors)
> - * @frame_errors:    number of frame errors (format, size, header...)
> + * @debugfs_entry:      debugfs entry
> + * @is_valid_period:    true if the sequence is valid for performance
> + * @begin:              start time of last HW task
> + * @total_duration:     total HW processing durations in 0.1ms
> + * @cnt_duration:       number of HW processings
> + * @min_duration:       minimum HW processing duration in 0.1ms
> + * @max_duration:       maximum HW processing duration in 0.1ms
> + * @avg_duration:       average HW processing duration in 0.1ms
> + * @max_fps:            maximum frames encoded per second (in 0.1Hz)
> + * @total_period:       total encoding periods in 0.1ms
> + * @cnt_period:         number of periods
> + * @min_period:         minimum encoding period in 0.1ms
> + * @max_period:         maximum encoding period in 0.1ms
> + * @avg_period:         average encoding period in 0.1ms
> + * @total_stream_size:  total number of encoded bytes
> + * @avg_fps:            average frames encoded per second (in 0.1Hz)
> + * @window_duration:    duration of the sampling window in 0.1ms
> + * @cnt_window:         number of samples in the window
> + * @window_stream_size: number of encoded bytes upon the sampling window
> + * @last_bitrate:       bitrate upon the last sampling window
> + * @min_bitrate:        minimum bitrate in kbps
> + * @max_bitrate:        maximum bitrate in kbps
> + * @avg_bitrate:        average bitrate in kbps
> + * @sys_errors:         number of system errors (memory, resource, pm..)
> + * @encode_errors:      number of encoding errors (hw/driver errors)
> + * @frame_errors:       number of frame errors (format, size, header...)
>   */
>  struct hva_ctx_dbg {
> -	bool	is_valid_period;
> -	ktime_t	begin;
> -	u32	total_duration;
> -	u32	cnt_duration;
> -	u32	sys_errors;
> -	u32	encode_errors;
> -	u32	frame_errors;
> +	struct dentry	*debugfs_entry;
> +	bool		is_valid_period;
> +	ktime_t		begin;
> +	u32		total_duration;
> +	u32		cnt_duration;
> +	u32		min_duration;
> +	u32		max_duration;
> +	u32		avg_duration;
> +	u32		max_fps;
> +	u32		total_period;
> +	u32		cnt_period;
> +	u32		min_period;
> +	u32		max_period;
> +	u32		avg_period;
> +	u32		total_stream_size;
> +	u32		avg_fps;
> +	u32		window_duration;
> +	u32		cnt_window;
> +	u32		window_stream_size;
> +	u32		last_bitrate;
> +	u32		min_bitrate;
> +	u32		max_bitrate;
> +	u32		avg_bitrate;
> +	u32		sys_errors;
> +	u32		encode_errors;
> +	u32		frame_errors;
>  };
>  
>  struct hva_dev;
> @@ -235,6 +274,17 @@ struct hva_ctx {
>  #define HVA_FLAG_STREAMINFO	0x0001
>  #define HVA_FLAG_FRAMEINFO	0x0002
>  
> +/**
> + * struct hva_dev_dbg - device debug info
> + *
> + * @debugfs_entry: debugfs entry
> + * @last_ctx:      debug information about last running instance context
> + */
> +struct hva_dev_dbg {
> +	struct dentry	*debugfs_entry;
> +	struct hva_ctx	last_ctx;
> +};
> +
>  #define HVA_MAX_INSTANCES	16
>  #define HVA_MAX_ENCODERS	10
>  #define HVA_MAX_FORMATS		HVA_MAX_ENCODERS
> @@ -273,6 +323,7 @@ struct hva_ctx {
>   * @lmi_err_reg:         local memory interface error register value
>   * @emi_err_reg:         external memory interface error register value
>   * @hec_mif_err_reg:     HEC memory interface error register value
> + * @dbg:             device debug info
>   */
>  struct hva_dev {
>  	struct v4l2_device	v4l2_dev;
> @@ -307,6 +358,7 @@ struct hva_dev {
>  	u32			lmi_err_reg;
>  	u32			emi_err_reg;
>  	u32			hec_mif_err_reg;
> +	struct hva_dev_dbg	dbg;
>  };
>  
>  /**
> @@ -335,6 +387,10 @@ struct hva_enc {
>  				  struct hva_stream *stream);
>  };
>  
> +void hva_debugfs_create(struct hva_dev *hva);
> +void hva_debugfs_remove(struct hva_dev *hva);
> +void hva_dbg_ctx_create(struct hva_ctx *ctx);
> +void hva_dbg_ctx_remove(struct hva_ctx *ctx);
>  char *hva_dbg_summary(struct hva_ctx *ctx);
>  void hva_dbg_perf_begin(struct hva_ctx *ctx);
>  void hva_dbg_perf_end(struct hva_ctx *ctx, struct hva_stream *stream);
> 

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

* Re: [PATCH v1 2/2] st-hva: add debug file system
  2016-09-15 10:30   ` Hans Verkuil
@ 2016-09-20  9:47     ` Jean Christophe TROTIN
  0 siblings, 0 replies; 6+ messages in thread
From: Jean Christophe TROTIN @ 2016-09-20  9:47 UTC (permalink / raw)
  To: Hans Verkuil, linux-media
  Cc: kernel, Benjamin Gaignard, Yannick FERTRE, Hugues FRUCHET



On 09/15/2016 12:30 PM, Hans Verkuil wrote:
> Same thing here: this should be enabled in a Kconfig option. That new Kconfig
> option should depend on DEBUG_FS as well.
>
> BTW (independent of these two patches): I think it would be easier to maintain
> if you make a Kconfig file in the sti directory, rather than adding them to the
> Kconfig in platform.
>
> Regards,
>
>         Hans

Hi Hans,

Thank you for your remarks about the 2 debug patches for HVA.
I've just sent a version 2 with the following propositions:

- the encoding summary (first patch) doesn't include any longer information
about the encoding performance. There's no more computation in this patch
(hva_dbg_perf_begin and hva_dbg_perf_end removed). Thus, there's no potential
penalty on the encoding performance. So, I propose that the short summary (very
useful to easily get a status about the encoding operation) is always enabled
(dev_info).

- for the second patch, I propose that the computation of the performances
(hva_dbg_perf_begin and hva_dbg_perf_end) is conditioned by CONFIG_DEBUG_FS. It
avoids the definition of a new Kconfig option (mkt-vpu also uses
CONFIG_DEBUG_FS). As to the functions that manage the debugfs entries
(hva_*_create/remove), I propose that they are called even if DEBUG_FS is
disabled: in this case, thanks to the debugfs API, the entries will simply not
created.

Finally, I think that your proposition of having a Kconfig in the sti directory
makes sense. It might be part of a different patch series dealing with the "sti
coherency" (we discussed about that on IRC with Benjamin Gaignard few weeks ago).

Regards,
Jean-Christophe.

>
> On 09/12/2016 06:01 PM, Jean-Christophe Trotin wrote:
>> This patch creates 4 static debugfs entries to dump:
>> - the device-related information ("st-hva/device")
>> - the list of registered encoders ("st-hva/encoders")
>> - the current values of the hva registers ("st-hva/regs")
>> - the information about the last closed instance ("st-hva/last")
>>
>> It also creates dynamically a debugfs entry for each opened instance,
>> ("st-hva/<instance identifier>") to dump:
>> - the information about the stream (profile, level, resolution,
>>   alignment...)
>> - the control parameters (bitrate mode, framerate, GOP size...)
>> - the potential (system, encoding...) errors
>> - the performance information about the encoding (HW processing
>>   duration, average bitrate, average framerate...)
>> Each time a running instance is closed, its context (including the
>> debug information) is saved to feed, on demand, the last closed
>> instance debugfs entry.
>>
>> Signed-off-by: Yannick Fertre <yannick.fertre@st.com>
>> Signed-off-by: Jean-Christophe Trotin <jean-christophe.trotin@st.com>
>> ---
>>  drivers/media/platform/sti/hva/hva-debug.c | 362 +++++++++++++++++++++++++++++
>>  drivers/media/platform/sti/hva/hva-hw.c    |  39 ++++
>>  drivers/media/platform/sti/hva/hva-hw.h    |   1 +
>>  drivers/media/platform/sti/hva/hva-v4l2.c  |  11 +-
>>  drivers/media/platform/sti/hva/hva.h       |  86 +++++--
>>  5 files changed, 482 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/media/platform/sti/hva/hva-debug.c b/drivers/media/platform/sti/hva/hva-debug.c
>> index 71bbf32..433b1d4 100644
>> --- a/drivers/media/platform/sti/hva/hva-debug.c
>> +++ b/drivers/media/platform/sti/hva/hva-debug.c
>> @@ -5,7 +5,113 @@
>>   * License terms:  GNU General Public License (GPL), version 2
>>   */
>>
>> +#include <linux/debugfs.h>
>> +
>>  #include "hva.h"
>> +#include "hva-hw.h"
>> +
>> +static void format_ctx(struct seq_file *s, struct hva_ctx *ctx)
>> +{
>> +     struct hva_streaminfo *stream = &ctx->streaminfo;
>> +     struct hva_frameinfo *frame = &ctx->frameinfo;
>> +     struct hva_controls *ctrls = &ctx->ctrls;
>> +     struct hva_ctx_dbg *dbg = &ctx->dbg;
>> +     u32 bitrate_mode, aspect, entropy, vui_sar, sei_fp;
>> +
>> +     seq_printf(s, "|-%s\n  |\n", ctx->name);
>> +
>> +     seq_printf(s, "  |-[%sframe info]\n",
>> +                ctx->flags & HVA_FLAG_FRAMEINFO ? "" : "default ");
>> +     seq_printf(s, "  | |- pixel format=%4.4s\n"
>> +                   "  | |- wxh=%dx%d\n"
>> +                   "  | |- wxh (w/ encoder alignment constraint)=%dx%d\n"
>> +                   "  |\n",
>> +                   (char *)&frame->pixelformat,
>> +                   frame->width, frame->height,
>> +                   frame->aligned_width, frame->aligned_height);
>> +
>> +     seq_printf(s, "  |-[%sstream info]\n",
>> +                ctx->flags & HVA_FLAG_STREAMINFO ? "" : "default ");
>> +     seq_printf(s, "  | |- stream format=%4.4s\n"
>> +                   "  | |- wxh=%dx%d\n"
>> +                   "  | |- %s\n"
>> +                   "  | |- %s\n"
>> +                   "  |\n",
>> +                   (char *)&stream->streamformat,
>> +                   stream->width, stream->height,
>> +                   stream->profile, stream->level);
>> +
>> +     bitrate_mode = V4L2_CID_MPEG_VIDEO_BITRATE_MODE;
>> +     aspect = V4L2_CID_MPEG_VIDEO_ASPECT;
>> +     seq_puts(s, "  |-[parameters]\n");
>> +     seq_printf(s, "  | |- %s\n"
>> +                   "  | |- bitrate=%d bps\n"
>> +                   "  | |- GOP size=%d\n"
>> +                   "  | |- video aspect=%s\n"
>> +                   "  | |- framerate=%d/%d\n",
>> +                   v4l2_ctrl_get_menu(bitrate_mode)[ctrls->bitrate_mode],
>> +                   ctrls->bitrate,
>> +                   ctrls->gop_size,
>> +                   v4l2_ctrl_get_menu(aspect)[ctrls->aspect],
>> +                   ctrls->time_per_frame.denominator,
>> +                   ctrls->time_per_frame.numerator);
>> +
>> +     entropy = V4L2_CID_MPEG_VIDEO_H264_ENTROPY_MODE;
>> +     vui_sar = V4L2_CID_MPEG_VIDEO_H264_VUI_SAR_IDC;
>> +     sei_fp =  V4L2_CID_MPEG_VIDEO_H264_SEI_FP_ARRANGEMENT_TYPE;
>> +     if (stream->streamformat == V4L2_PIX_FMT_H264) {
>> +             seq_printf(s, "  | |- %s entropy mode\n"
>> +                           "  | |- CPB size=%d kB\n"
>> +                           "  | |- DCT8x8 enable=%s\n"
>> +                           "  | |- qpmin=%d\n"
>> +                           "  | |- qpmax=%d\n"
>> +                           "  | |- PAR enable=%s\n"
>> +                           "  | |- PAR id=%s\n"
>> +                           "  | |- SEI frame packing enable=%s\n"
>> +                           "  | |- SEI frame packing type=%s\n",
>> +                           v4l2_ctrl_get_menu(entropy)[ctrls->entropy_mode],
>> +                           ctrls->cpb_size,
>> +                           ctrls->dct8x8 ? "true" : "false",
>> +                           ctrls->qpmin,
>> +                           ctrls->qpmax,
>> +                           ctrls->vui_sar ? "true" : "false",
>> +                           v4l2_ctrl_get_menu(vui_sar)[ctrls->vui_sar_idc],
>> +                           ctrls->sei_fp ? "true" : "false",
>> +                           v4l2_ctrl_get_menu(sei_fp)[ctrls->sei_fp_type]);
>> +     }
>> +
>> +     if (dbg->sys_errors || dbg->encode_errors || dbg->frame_errors) {
>> +             seq_puts(s, "  |\n  |-[errors]\n");
>> +             seq_printf(s, "  | |- system=%d\n"
>> +                           "  | |- encoding=%d\n"
>> +                           "  | |- frame=%d\n",
>> +                           dbg->sys_errors,
>> +                           dbg->encode_errors,
>> +                           dbg->frame_errors);
>> +     }
>> +
>> +     seq_puts(s, "  |\n  |-[performances]\n");
>> +     seq_printf(s, "  | |- frames encoded=%d\n"
>> +                   "  | |- avg HW processing duration (0.1ms)=%d [min=%d, max=%d]\n"
>> +                   "  | |- avg encoding period (0.1ms)=%d [min=%d, max=%d]\n"
>> +                   "  | |- avg fps (0.1Hz)=%d\n"
>> +                   "  | |- max reachable fps (0.1Hz)=%d\n"
>> +                   "  | |- avg bitrate (kbps)=%d [min=%d, max=%d]\n"
>> +                   "  | |- last bitrate (kbps)=%d\n",
>> +                   dbg->cnt_duration,
>> +                   dbg->avg_duration,
>> +                   dbg->min_duration,
>> +                   dbg->max_duration,
>> +                   dbg->avg_period,
>> +                   dbg->min_period,
>> +                   dbg->max_period,
>> +                   dbg->avg_fps,
>> +                   dbg->max_fps,
>> +                   dbg->avg_bitrate,
>> +                   dbg->min_bitrate,
>> +                   dbg->max_bitrate,
>> +                   dbg->last_bitrate);
>> +}
>>
>>  /*
>>   * encoding summary
>> @@ -77,10 +183,52 @@ char *hva_dbg_summary(struct hva_ctx *ctx)
>>
>>  void hva_dbg_perf_begin(struct hva_ctx *ctx)
>>  {
>> +     u64 div;
>> +     u32 period;
>> +     u32 bitrate;
>>       struct hva_ctx_dbg *dbg = &ctx->dbg;
>> +     ktime_t prev = dbg->begin;
>>
>>       dbg->begin = ktime_get();
>>
>> +     if (dbg->is_valid_period) {
>> +             /* encoding period */
>> +             div = (u64)ktime_us_delta(dbg->begin, prev);
>> +             do_div(div, 100);
>> +             period = (u32)div;
>> +             dbg->min_period = min(period, dbg->min_period);
>> +             dbg->max_period = max(period, dbg->max_period);
>> +             dbg->total_period += period;
>> +             dbg->cnt_period++;
>> +
>> +             /*
>> +              * minimum and maximum bitrates are based on the
>> +              * encoding period values upon a window of 32 samples
>> +              */
>> +             dbg->window_duration += period;
>> +             dbg->cnt_window++;
>> +             if (dbg->cnt_window >= 32) {
>> +                     /*
>> +                      * bitrate in kbps = (size * 8 / 1000) /
>> +                      *                   (duration / 10000)
>> +                      *                 = size * 80 / duration
>> +                      */
>> +                     if (dbg->window_duration > 0) {
>> +                             div = (u64)dbg->window_stream_size * 80;
>> +                             do_div(div, dbg->window_duration);
>> +                             bitrate = (u32)div;
>> +                             dbg->last_bitrate = bitrate;
>> +                             dbg->min_bitrate = min(bitrate,
>> +                                                    dbg->min_bitrate);
>> +                             dbg->max_bitrate = max(bitrate,
>> +                                                    dbg->max_bitrate);
>> +                     }
>> +                     dbg->window_stream_size = 0;
>> +                     dbg->window_duration = 0;
>> +                     dbg->cnt_window = 0;
>> +             }
>> +     }
>> +
>>       /*
>>        * filter sequences valid for performance:
>>        * - begin/begin (no stream available) is an invalid sequence
>> @@ -118,8 +266,222 @@ void hva_dbg_perf_end(struct hva_ctx *ctx, struct hva_stream *stream)
>>       do_div(div, 100);
>>       duration = (u32)div;
>>
>> +     dbg->min_duration = min(duration, dbg->min_duration);
>> +     dbg->max_duration = max(duration, dbg->max_duration);
>>       dbg->total_duration += duration;
>>       dbg->cnt_duration++;
>>
>> +     /*
>> +      * the average bitrate is based on the total stream size
>> +      * and the total encoding periods
>> +      */
>> +     dbg->total_stream_size += bytesused;
>> +     dbg->window_stream_size += bytesused;
>> +
>>       dbg->is_valid_period = true;
>>  }
>> +
>> +static void hva_dbg_perf_compute(struct hva_ctx *ctx)
>> +{
>> +     u64 div;
>> +     struct hva_ctx_dbg *dbg = &ctx->dbg;
>> +
>> +     if (dbg->cnt_duration > 0) {
>> +             div = (u64)dbg->total_duration;
>> +             do_div(div, dbg->cnt_duration);
>> +             dbg->avg_duration = (u32)div;
>> +     } else {
>> +             dbg->avg_duration = 0;
>> +     }
>> +
>> +     if (dbg->total_duration > 0) {
>> +             div = (u64)dbg->cnt_duration * 100000;
>> +             do_div(div, dbg->total_duration);
>> +             dbg->max_fps = (u32)div;
>> +     } else {
>> +             dbg->max_fps = 0;
>> +     }
>> +
>> +     if (dbg->cnt_period > 0) {
>> +             div = (u64)dbg->total_period;
>> +             do_div(div, dbg->cnt_period);
>> +             dbg->avg_period = (u32)div;
>> +     } else {
>> +             dbg->avg_period = 0;
>> +     }
>> +
>> +     if (dbg->total_period > 0) {
>> +             div = (u64)dbg->cnt_period * 100000;
>> +             do_div(div, dbg->total_period);
>> +             dbg->avg_fps = (u32)div;
>> +     } else {
>> +             dbg->avg_fps = 0;
>> +     }
>> +
>> +     if (dbg->total_period > 0) {
>> +             /*
>> +              * bitrate in kbps = (video size * 8 / 1000) /
>> +              *                   (video duration / 10000)
>> +              *                 = video size * 80 / video duration
>> +              */
>> +             div = (u64)dbg->total_stream_size * 80;
>> +             do_div(div, dbg->total_period);
>> +             dbg->avg_bitrate = (u32)div;
>> +     } else {
>> +             dbg->avg_bitrate = 0;
>> +     }
>> +}
>> +
>> +/*
>> + * device debug info
>> + */
>> +
>> +static int hva_dbg_device(struct seq_file *s, void *data)
>> +{
>> +     struct hva_dev *hva = s->private;
>> +
>> +     seq_printf(s, "[%s]\n", hva->v4l2_dev.name);
>> +     seq_printf(s, "registered as /dev/video%d\n", hva->vdev->num);
>> +
>> +     return 0;
>> +}
>> +
>> +static int hva_dbg_encoders(struct seq_file *s, void *data)
>> +{
>> +     struct hva_dev *hva = s->private;
>> +     unsigned int i = 0;
>> +
>> +     seq_printf(s, "[encoders]\n|- %d registered encoders:\n",
>> +                hva->nb_of_encoders);
>> +
>> +     while (hva->encoders[i]) {
>> +             seq_printf(s, "|- %s: %4.4s => %4.4s\n", hva->encoders[i]->name,
>> +                        (char *)&hva->encoders[i]->pixelformat,
>> +                        (char *)&hva->encoders[i]->streamformat);
>> +             i++;
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static int hva_dbg_last(struct seq_file *s, void *data)
>> +{
>> +     struct hva_dev *hva = s->private;
>> +     struct hva_ctx *last_ctx = &hva->dbg.last_ctx;
>> +
>> +     if (last_ctx->flags & HVA_FLAG_STREAMINFO) {
>> +             seq_puts(s, "[last encoding]\n");
>> +
>> +             hva_dbg_perf_compute(last_ctx);
>> +             format_ctx(s, last_ctx);
>> +     } else {
>> +             seq_puts(s, "[no information recorded about last encoding]\n");
>> +     }
>> +
>> +     return 0;
>> +}
>> +
>> +static int hva_dbg_regs(struct seq_file *s, void *data)
>> +{
>> +     struct hva_dev *hva = s->private;
>> +
>> +     hva_hw_dump_regs(hva, s);
>> +
>> +     return 0;
>> +}
>> +
>> +#define hva_dbg_declare(name)                                                  \
>> +     static int hva_dbg_##name##_open(struct inode *i, struct file *f) \
>> +     {                                                                 \
>> +             return single_open(f, hva_dbg_##name, i->i_private);      \
>> +     }                                                                 \
>> +     static const struct file_operations hva_dbg_##name##_fops = {     \
>> +             .open           = hva_dbg_##name##_open,                  \
>> +             .read           = seq_read,                               \
>> +             .llseek         = seq_lseek,                              \
>> +             .release        = single_release,                         \
>> +     }
>> +
>> +#define hva_dbg_create_entry(name)                                    \
>> +     debugfs_create_file(#name, S_IRUGO, hva->dbg.debugfs_entry, hva, \
>> +                         &hva_dbg_##name##_fops)
>> +
>> +hva_dbg_declare(device);
>> +hva_dbg_declare(encoders);
>> +hva_dbg_declare(last);
>> +hva_dbg_declare(regs);
>> +
>> +void hva_debugfs_create(struct hva_dev *hva)
>> +{
>> +     hva->dbg.debugfs_entry = debugfs_create_dir(HVA_NAME, NULL);
>> +     if (!hva->dbg.debugfs_entry)
>> +             goto err;
>> +
>> +     if (!hva_dbg_create_entry(device))
>> +             goto err;
>> +
>> +     if (!hva_dbg_create_entry(encoders))
>> +             goto err;
>> +
>> +     if (!hva_dbg_create_entry(last))
>> +             goto err;
>> +
>> +     if (!hva_dbg_create_entry(regs))
>> +             goto err;
>> +
>> +     return;
>> +
>> +err:
>> +     hva_debugfs_remove(hva);
>> +}
>> +
>> +void hva_debugfs_remove(struct hva_dev *hva)
>> +{
>> +     debugfs_remove_recursive(hva->dbg.debugfs_entry);
>> +     hva->dbg.debugfs_entry = NULL;
>> +}
>> +
>> +/*
>> + * context (instance) debug info
>> + */
>> +
>> +static int hva_dbg_ctx(struct seq_file *s, void *data)
>> +{
>> +     struct hva_ctx *ctx = s->private;
>> +
>> +     seq_printf(s, "[running encoding %d]\n", ctx->id);
>> +
>> +     hva_dbg_perf_compute(ctx);
>> +     format_ctx(s, ctx);
>> +
>> +     return 0;
>> +}
>> +
>> +hva_dbg_declare(ctx);
>> +
>> +void hva_dbg_ctx_create(struct hva_ctx *ctx)
>> +{
>> +     struct hva_dev *hva = ctx->hva_dev;
>> +     char name[4] = "";
>> +
>> +     ctx->dbg.min_duration = UINT_MAX;
>> +     ctx->dbg.min_period = UINT_MAX;
>> +     ctx->dbg.min_bitrate = UINT_MAX;
>> +
>> +     snprintf(name, sizeof(name), "%d", hva->instance_id);
>> +
>> +     ctx->dbg.debugfs_entry = debugfs_create_file(name, S_IRUGO,
>> +                                                  hva->dbg.debugfs_entry,
>> +                                                  ctx, &hva_dbg_ctx_fops);
>> +}
>> +
>> +void hva_dbg_ctx_remove(struct hva_ctx *ctx)
>> +{
>> +     struct hva_dev *hva = ctx->hva_dev;
>> +
>> +     if (ctx->flags & HVA_FLAG_STREAMINFO)
>> +             /* save context before removing */
>> +             memcpy(&hva->dbg.last_ctx, ctx, sizeof(*ctx));
>> +
>> +     debugfs_remove(ctx->dbg.debugfs_entry);
>> +}
>> diff --git a/drivers/media/platform/sti/hva/hva-hw.c b/drivers/media/platform/sti/hva/hva-hw.c
>> index c84b860..d395204 100644
>> --- a/drivers/media/platform/sti/hva/hva-hw.c
>> +++ b/drivers/media/platform/sti/hva/hva-hw.c
>> @@ -9,6 +9,7 @@
>>  #include <linux/interrupt.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_runtime.h>
>> +#include <linux/seq_file.h>
>>
>>  #include "hva.h"
>>  #include "hva-hw.h"
>> @@ -541,3 +542,41 @@ out:
>>
>>       return ret;
>>  }
>> +
>> +#define DUMP(reg) seq_printf(s, "%-30s: 0x%08X\n",\
>> +                          #reg, readl_relaxed(hva->regs + reg))
>> +
>> +void hva_hw_dump_regs(struct hva_dev *hva, struct seq_file *s)
>> +{
>> +     struct device *dev = hva_to_dev(hva);
>> +
>> +     mutex_lock(&hva->protect_mutex);
>> +
>> +     if (pm_runtime_get_sync(dev) < 0) {
>> +             seq_puts(s, "Cannot wake up IP\n");
>> +             mutex_unlock(&hva->protect_mutex);
>> +             return;
>> +     }
>> +
>> +     seq_printf(s, "Registers:\nReg @ = 0x%p\n", hva->regs);
>> +
>> +     DUMP(HVA_HIF_REG_RST);
>> +     DUMP(HVA_HIF_REG_RST_ACK);
>> +     DUMP(HVA_HIF_REG_MIF_CFG);
>> +     DUMP(HVA_HIF_REG_HEC_MIF_CFG);
>> +     DUMP(HVA_HIF_REG_CFL);
>> +     DUMP(HVA_HIF_REG_SFL);
>> +     DUMP(HVA_HIF_REG_LMI_ERR);
>> +     DUMP(HVA_HIF_REG_EMI_ERR);
>> +     DUMP(HVA_HIF_REG_HEC_MIF_ERR);
>> +     DUMP(HVA_HIF_REG_HEC_STS);
>> +     DUMP(HVA_HIF_REG_HVC_STS);
>> +     DUMP(HVA_HIF_REG_HJE_STS);
>> +     DUMP(HVA_HIF_REG_CNT);
>> +     DUMP(HVA_HIF_REG_HEC_CHKSYN_DIS);
>> +     DUMP(HVA_HIF_REG_CLK_GATING);
>> +     DUMP(HVA_HIF_REG_VERSION);
>> +
>> +     pm_runtime_put_autosuspend(dev);
>> +     mutex_unlock(&hva->protect_mutex);
>> +}
>> diff --git a/drivers/media/platform/sti/hva/hva-hw.h b/drivers/media/platform/sti/hva/hva-hw.h
>> index efb45b9..f8df405 100644
>> --- a/drivers/media/platform/sti/hva/hva-hw.h
>> +++ b/drivers/media/platform/sti/hva/hva-hw.h
>> @@ -38,5 +38,6 @@ int hva_hw_runtime_suspend(struct device *dev);
>>  int hva_hw_runtime_resume(struct device *dev);
>>  int hva_hw_execute_task(struct hva_ctx *ctx, enum hva_hw_cmd_type cmd,
>>                       struct hva_buffer *task);
>> +void hva_hw_dump_regs(struct hva_dev *hva, struct seq_file *s);
>>
>>  #endif /* HVA_HW_H */
>> diff --git a/drivers/media/platform/sti/hva/hva-v4l2.c b/drivers/media/platform/sti/hva/hva-v4l2.c
>> index fb65816..71bb7b3 100644
>> --- a/drivers/media/platform/sti/hva/hva-v4l2.c
>> +++ b/drivers/media/platform/sti/hva/hva-v4l2.c
>> @@ -15,8 +15,6 @@
>>  #include "hva.h"
>>  #include "hva-hw.h"
>>
>> -#define HVA_NAME "st-hva"
>> -
>>  #define MIN_FRAMES   1
>>  #define MIN_STREAMS  1
>>
>> @@ -1181,6 +1179,8 @@ static int hva_open(struct file *file)
>>       /* default parameters for frame and stream */
>>       set_default_params(ctx);
>>
>> +     hva_dbg_ctx_create(ctx);
>> +
>>       dev_info(dev, "%s encoder instance created\n", ctx->name);
>>
>>       return 0;
>> @@ -1223,6 +1223,8 @@ static int hva_release(struct file *file)
>>       v4l2_fh_del(&ctx->fh);
>>       v4l2_fh_exit(&ctx->fh);
>>
>> +     hva_dbg_ctx_remove(ctx);
>> +
>>       dev_info(dev, "%s encoder instance released\n", ctx->name);
>>
>>       kfree(ctx);
>> @@ -1347,6 +1349,8 @@ static int hva_probe(struct platform_device *pdev)
>>               goto err_hw;
>>       }
>>
>> +     hva_debugfs_create(hva);
>> +
>>       hva->work_queue = create_workqueue(HVA_NAME);
>>       if (!hva->work_queue) {
>>               dev_err(dev, "%s %s failed to allocate work queue\n",
>> @@ -1368,6 +1372,7 @@ static int hva_probe(struct platform_device *pdev)
>>  err_work_queue:
>>       destroy_workqueue(hva->work_queue);
>>  err_v4l2:
>> +     hva_debugfs_remove(hva);
>>       v4l2_device_unregister(&hva->v4l2_dev);
>>  err_hw:
>>       hva_hw_remove(hva);
>> @@ -1386,6 +1391,8 @@ static int hva_remove(struct platform_device *pdev)
>>
>>       hva_hw_remove(hva);
>>
>> +     hva_debugfs_remove(hva);
>> +
>>       v4l2_device_unregister(&hva->v4l2_dev);
>>
>>       dev_info(dev, "%s %s removed\n", HVA_PREFIX, pdev->name);
>> diff --git a/drivers/media/platform/sti/hva/hva.h b/drivers/media/platform/sti/hva/hva.h
>> index badc5f4..eef9769 100644
>> --- a/drivers/media/platform/sti/hva/hva.h
>> +++ b/drivers/media/platform/sti/hva/hva.h
>> @@ -21,7 +21,8 @@
>>
>>  #define ctx_to_hdev(c)  (c->hva_dev)
>>
>> -#define HVA_PREFIX "[---:----]"
>> +#define HVA_NAME     "st-hva"
>> +#define HVA_PREFIX   "[---:----]"
>>
>>  extern const struct hva_enc nv12h264enc;
>>  extern const struct hva_enc nv21h264enc;
>> @@ -156,22 +157,60 @@ struct hva_stream {
>>  /**
>>   * struct hva_ctx_dbg - instance context debug info
>>   *
>> - * @is_valid_period: true if the sequence is valid for performance
>> - * @begin:           start time of last HW task
>> - * @total_duration:  total HW processing durations in 0.1ms
>> - * @cnt_duration:    number of HW processings
>> - * @sys_errors:      number of system errors (memory, resource, pm..)
>> - * @encode_errors:   number of encoding errors (hw/driver errors)
>> - * @frame_errors:    number of frame errors (format, size, header...)
>> + * @debugfs_entry:      debugfs entry
>> + * @is_valid_period:    true if the sequence is valid for performance
>> + * @begin:              start time of last HW task
>> + * @total_duration:     total HW processing durations in 0.1ms
>> + * @cnt_duration:       number of HW processings
>> + * @min_duration:       minimum HW processing duration in 0.1ms
>> + * @max_duration:       maximum HW processing duration in 0.1ms
>> + * @avg_duration:       average HW processing duration in 0.1ms
>> + * @max_fps:            maximum frames encoded per second (in 0.1Hz)
>> + * @total_period:       total encoding periods in 0.1ms
>> + * @cnt_period:         number of periods
>> + * @min_period:         minimum encoding period in 0.1ms
>> + * @max_period:         maximum encoding period in 0.1ms
>> + * @avg_period:         average encoding period in 0.1ms
>> + * @total_stream_size:  total number of encoded bytes
>> + * @avg_fps:            average frames encoded per second (in 0.1Hz)
>> + * @window_duration:    duration of the sampling window in 0.1ms
>> + * @cnt_window:         number of samples in the window
>> + * @window_stream_size: number of encoded bytes upon the sampling window
>> + * @last_bitrate:       bitrate upon the last sampling window
>> + * @min_bitrate:        minimum bitrate in kbps
>> + * @max_bitrate:        maximum bitrate in kbps
>> + * @avg_bitrate:        average bitrate in kbps
>> + * @sys_errors:         number of system errors (memory, resource, pm..)
>> + * @encode_errors:      number of encoding errors (hw/driver errors)
>> + * @frame_errors:       number of frame errors (format, size, header...)
>>   */
>>  struct hva_ctx_dbg {
>> -     bool    is_valid_period;
>> -     ktime_t begin;
>> -     u32     total_duration;
>> -     u32     cnt_duration;
>> -     u32     sys_errors;
>> -     u32     encode_errors;
>> -     u32     frame_errors;
>> +     struct dentry   *debugfs_entry;
>> +     bool            is_valid_period;
>> +     ktime_t         begin;
>> +     u32             total_duration;
>> +     u32             cnt_duration;
>> +     u32             min_duration;
>> +     u32             max_duration;
>> +     u32             avg_duration;
>> +     u32             max_fps;
>> +     u32             total_period;
>> +     u32             cnt_period;
>> +     u32             min_period;
>> +     u32             max_period;
>> +     u32             avg_period;
>> +     u32             total_stream_size;
>> +     u32             avg_fps;
>> +     u32             window_duration;
>> +     u32             cnt_window;
>> +     u32             window_stream_size;
>> +     u32             last_bitrate;
>> +     u32             min_bitrate;
>> +     u32             max_bitrate;
>> +     u32             avg_bitrate;
>> +     u32             sys_errors;
>> +     u32             encode_errors;
>> +     u32             frame_errors;
>>  };
>>
>>  struct hva_dev;
>> @@ -235,6 +274,17 @@ struct hva_ctx {
>>  #define HVA_FLAG_STREAMINFO  0x0001
>>  #define HVA_FLAG_FRAMEINFO   0x0002
>>
>> +/**
>> + * struct hva_dev_dbg - device debug info
>> + *
>> + * @debugfs_entry: debugfs entry
>> + * @last_ctx:      debug information about last running instance context
>> + */
>> +struct hva_dev_dbg {
>> +     struct dentry   *debugfs_entry;
>> +     struct hva_ctx  last_ctx;
>> +};
>> +
>>  #define HVA_MAX_INSTANCES    16
>>  #define HVA_MAX_ENCODERS     10
>>  #define HVA_MAX_FORMATS              HVA_MAX_ENCODERS
>> @@ -273,6 +323,7 @@ struct hva_ctx {
>>   * @lmi_err_reg:         local memory interface error register value
>>   * @emi_err_reg:         external memory interface error register value
>>   * @hec_mif_err_reg:     HEC memory interface error register value
>> + * @dbg:             device debug info
>>   */
>>  struct hva_dev {
>>       struct v4l2_device      v4l2_dev;
>> @@ -307,6 +358,7 @@ struct hva_dev {
>>       u32                     lmi_err_reg;
>>       u32                     emi_err_reg;
>>       u32                     hec_mif_err_reg;
>> +     struct hva_dev_dbg      dbg;
>>  };
>>
>>  /**
>> @@ -335,6 +387,10 @@ struct hva_enc {
>>                                 struct hva_stream *stream);
>>  };
>>
>> +void hva_debugfs_create(struct hva_dev *hva);
>> +void hva_debugfs_remove(struct hva_dev *hva);
>> +void hva_dbg_ctx_create(struct hva_ctx *ctx);
>> +void hva_dbg_ctx_remove(struct hva_ctx *ctx);
>>  char *hva_dbg_summary(struct hva_ctx *ctx);
>>  void hva_dbg_perf_begin(struct hva_ctx *ctx);
>>  void hva_dbg_perf_end(struct hva_ctx *ctx, struct hva_stream *stream);
>>

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

end of thread, other threads:[~2016-09-20  9:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-12 16:01 [PATCH v1 0/2] add debug capabilities to v4l2 encoder for STMicroelectronics SOC Jean-Christophe Trotin
2016-09-12 16:01 ` [PATCH v1 1/2] st-hva: encoding summary at instance release Jean-Christophe Trotin
2016-09-15 10:24   ` Hans Verkuil
2016-09-12 16:01 ` [PATCH v1 2/2] st-hva: add debug file system Jean-Christophe Trotin
2016-09-15 10:30   ` Hans Verkuil
2016-09-20  9:47     ` Jean Christophe TROTIN

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.