| Message ID | 20200609104604.1594-6-stanimir.varbanov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Venus dynamic debug | expand |
On Tue, Jun 09, 2020 at 01:46:02PM +0300, Stanimir Varbanov wrote: > +int venus_dbgfs_init(struct venus_core *core) > +{ > + core->root = debugfs_create_dir("venus", NULL); > + if (IS_ERR(core->root)) > + return IS_ERR(core->root); You really do not care, and obviously did not test this on a system with CONFIG_DEBUG_FS disabled :) Just make the call to debugfs, and move on, feed it into other debugfs calls, all is good. This function should just return 'void', no need to care about this at all. > + ret = venus_sys_set_debug(hdev, venus_fw_debug); > + if (ret) > + dev_warn(dev, "setting fw debug msg ON failed (%d)\n", ret); Why do you care about this "error"? thanks, greg k-h
Hi Greg, Thanks for the comments! On 6/9/20 2:12 PM, Greg Kroah-Hartman wrote: > On Tue, Jun 09, 2020 at 01:46:02PM +0300, Stanimir Varbanov wrote: >> +int venus_dbgfs_init(struct venus_core *core) >> +{ >> + core->root = debugfs_create_dir("venus", NULL); >> + if (IS_ERR(core->root)) >> + return IS_ERR(core->root); > > You really do not care, and obviously did not test this on a system with > CONFIG_DEBUG_FS disabled :) Probably not :( > > Just make the call to debugfs, and move on, feed it into other debugfs > calls, all is good. > > This function should just return 'void', no need to care about this at > all. > >> + ret = venus_sys_set_debug(hdev, venus_fw_debug); >> + if (ret) >> + dev_warn(dev, "setting fw debug msg ON failed (%d)\n", ret); > > Why do you care about this "error"? I don't care so much, that's why it is dev_warn. But if I enable debug messages from the firmware and I don't see them this warn will give me an idea why.
diff --git a/drivers/media/platform/qcom/venus/Makefile b/drivers/media/platform/qcom/venus/Makefile index 64af0bc1edae..dfc636865709 100644 --- a/drivers/media/platform/qcom/venus/Makefile +++ b/drivers/media/platform/qcom/venus/Makefile @@ -3,7 +3,7 @@ venus-core-objs += core.o helpers.o firmware.o \ hfi_venus.o hfi_msgs.o hfi_cmds.o hfi.o \ - hfi_parser.o pm_helpers.o + hfi_parser.o pm_helpers.o dbgfs.o venus-dec-objs += vdec.o vdec_ctrls.o venus-enc-objs += venc.o venc_ctrls.o diff --git a/drivers/media/platform/qcom/venus/core.c b/drivers/media/platform/qcom/venus/core.c index 203c6538044f..bbb394ca4175 100644 --- a/drivers/media/platform/qcom/venus/core.c +++ b/drivers/media/platform/qcom/venus/core.c @@ -290,6 +290,10 @@ static int venus_probe(struct platform_device *pdev) if (ret) goto err_dev_unregister; + ret = venus_dbgfs_init(core); + if (ret) + goto err_dev_unregister; + return 0; err_dev_unregister: @@ -337,6 +341,7 @@ static int venus_remove(struct platform_device *pdev) v4l2_device_unregister(&core->v4l2_dev); mutex_destroy(&core->pm_lock); mutex_destroy(&core->lock); + venus_dbgfs_deinit(core); return ret; } diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h index 7118612673c9..b48782f9aa95 100644 --- a/drivers/media/platform/qcom/venus/core.h +++ b/drivers/media/platform/qcom/venus/core.h @@ -12,6 +12,7 @@ #include <media/v4l2-ctrls.h> #include <media/v4l2-device.h> +#include "dbgfs.h" #include "hfi.h" #define VIDC_CLKS_NUM_MAX 4 @@ -136,6 +137,7 @@ struct venus_caps { * @priv: a private filed for HFI operations * @ops: the core HFI operations * @work: a delayed work for handling system fatal error + * @root: debugfs root directory */ struct venus_core { void __iomem *base; @@ -185,6 +187,7 @@ struct venus_core { unsigned int codecs_count; unsigned int core0_usage_count; unsigned int core1_usage_count; + struct dentry *root; }; struct vdec_controls { diff --git a/drivers/media/platform/qcom/venus/dbgfs.c b/drivers/media/platform/qcom/venus/dbgfs.c new file mode 100644 index 000000000000..a2465fe8e20b --- /dev/null +++ b/drivers/media/platform/qcom/venus/dbgfs.c @@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2020 Linaro Ltd. + */ + +#include <linux/debugfs.h> + +#include "core.h" + +extern int venus_fw_debug; + +int venus_dbgfs_init(struct venus_core *core) +{ + core->root = debugfs_create_dir("venus", NULL); + if (IS_ERR(core->root)) + return IS_ERR(core->root); + + debugfs_create_x32("fw_level", 0644, core->root, &venus_fw_debug); + + return 0; +} + +void venus_dbgfs_deinit(struct venus_core *core) +{ + debugfs_remove_recursive(core->root); +} diff --git a/drivers/media/platform/qcom/venus/dbgfs.h b/drivers/media/platform/qcom/venus/dbgfs.h new file mode 100644 index 000000000000..4e35bd7db15f --- /dev/null +++ b/drivers/media/platform/qcom/venus/dbgfs.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* Copyright (C) 2020 Linaro Ltd. */ + +#ifndef __VENUS_DBGFS_H__ +#define __VENUS_DBGFS_H__ + +struct venus_core; + +int venus_dbgfs_init(struct venus_core *core); +void venus_dbgfs_deinit(struct venus_core *core); + +#endif diff --git a/drivers/media/platform/qcom/venus/hfi_venus.c b/drivers/media/platform/qcom/venus/hfi_venus.c index 0d8855014ab3..3a04b08ab85a 100644 --- a/drivers/media/platform/qcom/venus/hfi_venus.c +++ b/drivers/media/platform/qcom/venus/hfi_venus.c @@ -130,7 +130,7 @@ struct venus_hfi_device { }; static bool venus_pkt_debug; -static int venus_fw_debug = HFI_DEBUG_MSG_ERROR | HFI_DEBUG_MSG_FATAL; +int venus_fw_debug = HFI_DEBUG_MSG_ERROR | HFI_DEBUG_MSG_FATAL; static bool venus_sys_idle_indicator; static bool venus_fw_low_power_mode = true; static int venus_hw_rsp_timeout = 1000; @@ -1130,9 +1130,14 @@ static int venus_session_init(struct venus_inst *inst, u32 session_type, u32 codec) { struct venus_hfi_device *hdev = to_hfi_priv(inst->core); + struct device *dev = hdev->core->dev; struct hfi_session_init_pkt pkt; int ret; + ret = venus_sys_set_debug(hdev, venus_fw_debug); + if (ret) + dev_warn(dev, "setting fw debug msg ON failed (%d)\n", ret); + ret = pkt_session_init(&pkt, inst, session_type, codec); if (ret) goto err;
This will be useful when debugging specific issues related to firmware HFI interface. Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org> --- drivers/media/platform/qcom/venus/Makefile | 2 +- drivers/media/platform/qcom/venus/core.c | 5 ++++ drivers/media/platform/qcom/venus/core.h | 3 +++ drivers/media/platform/qcom/venus/dbgfs.c | 26 +++++++++++++++++++ drivers/media/platform/qcom/venus/dbgfs.h | 12 +++++++++ drivers/media/platform/qcom/venus/hfi_venus.c | 7 ++++- 6 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 drivers/media/platform/qcom/venus/dbgfs.c create mode 100644 drivers/media/platform/qcom/venus/dbgfs.h -- 2.17.1