Thread (46 messages) 46 messages, 10 authors, 2020-06-12

Re: [PATCH v3 6/7] venus: Make debug infrastructure more flexible

From: Stanimir Varbanov <hidden>
Date: 2020-06-10 13:29:44
Also in: linux-acpi, linux-arm-msm, linux-btrfs, linux-media, lkml, netdev


On 6/9/20 2:14 PM, Greg Kroah-Hartman wrote:
On Tue, Jun 09, 2020 at 01:46:03PM +0300, Stanimir Varbanov wrote:
quoted
Here we introduce few debug macros with levels (low, medium and
high) and debug macro for firmware. Enabling the particular level
will be done by dynamic debug with levels.

For example to enable debug messages with low level:
echo 'module venus_dec level 0x01 +p' > debugfs/dynamic_debug/control

If you want to enable all levels:
echo 'module venus_dec level 0x07 +p' > debugfs/dynamic_debug/control

All the features which dynamic debugging provide are preserved.

And finaly all dev_dbg are translated to VDBGX with appropriate
debug levels.

Signed-off-by: Stanimir Varbanov <redacted>
---
 drivers/media/platform/qcom/venus/core.h      |  5 ++
 drivers/media/platform/qcom/venus/helpers.c   |  2 +-
 drivers/media/platform/qcom/venus/hfi_msgs.c  | 30 ++++-----
 drivers/media/platform/qcom/venus/hfi_venus.c | 20 ++++--
 .../media/platform/qcom/venus/pm_helpers.c    |  3 +-
 drivers/media/platform/qcom/venus/vdec.c      | 63 +++++++++++++++++--
 drivers/media/platform/qcom/venus/venc.c      |  4 ++
 7 files changed, 96 insertions(+), 31 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index b48782f9aa95..63eabf5ff96d 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -15,6 +15,11 @@
 #include "dbgfs.h"
 #include "hfi.h"
 
+#define VDBGL(fmt, args...)	pr_debug_level(0x01, fmt, ##args)
+#define VDBGM(fmt, args...)	pr_debug_level(0x02, fmt, ##args)
+#define VDBGH(fmt, args...)	pr_debug_level(0x04, fmt, ##args)
+#define VDBGFW(fmt, args...)	pr_debug_level(0x08, fmt, ##args)
+
 #define VIDC_CLKS_NUM_MAX		4
 #define VIDC_VCODEC_CLKS_NUM_MAX	2
 #define VIDC_PMDOMAINS_NUM_MAX		3
diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 0143af7822b2..115a9a2af1d6 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -396,7 +396,7 @@ put_ts_metadata(struct venus_inst *inst, struct vb2_v4l2_buffer *vbuf)
 	}
 
 	if (slot == -1) {
-		dev_dbg(inst->core->dev, "%s: no free slot\n", __func__);
+		VDBGH("no free slot for timestamp\n");
So you just lost the information that dev_dbg() gave you with regards to
the device/driver/instance creating that message?
No, I don't lose anything.  When I do debug I know that all debug
messages comes from my driver.  dev_dbg will give me few device
identifiers which I don't care so much. IMO, the device information
makes more sense to dev_err/warn/err variants.  On the other side we
will have dev_dbg_level(group) if still someone needs the device
information.
Ick, no, don't do that.

And why is this driver somehow "special" compared to all the rest of
Of course it is special ... to me ;-)
the kernel?  Why is the current dev_dbg() control not sufficient that
you need to change the core for just this tiny thing?

thanks,

greg k-h
-- 
regards,
Stan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help