2020-11-10 01:23:19

by Rishabh Bhatnagar

[permalink] [raw]
Subject: [PATCH 0/2] Add trace events to q6v5_pas and mdt_loader driver

Create and insert trace points in mdt_loader and qcom_q6v5_pas
drivers. These tracepoints will be used to analyze the time taken
at each step during bootup/shutdown of the remoteproc. Also
provide information about location and size of firmware segments
being loaded.

Rishabh Bhatnagar (2):
soc: qcom: Add tracepoints to mdt loader
remoteproc: qcom: Add trace events for q6v5_pas driver

drivers/remoteproc/qcom_q6v5_pas.c | 11 ++++++++
drivers/soc/qcom/mdt_loader.c | 8 ++++++
include/trace/events/mdt_loader.h | 57 ++++++++++++++++++++++++++++++++++++++
include/trace/events/q6v5_pas.h | 34 +++++++++++++++++++++++
4 files changed, 110 insertions(+)
create mode 100644 include/trace/events/mdt_loader.h
create mode 100644 include/trace/events/q6v5_pas.h

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2020-11-10 01:24:05

by Rishabh Bhatnagar

[permalink] [raw]
Subject: [PATCH 2/2] remoteproc: qcom: Add trace events for q6v5_pas driver

Add tracepoints for q6v5_pas driver. These will help in
analyzing the time taken by each step in remoteproc
bootup/shutdown process and also serve as standard
checkpoints in code.

Signed-off-by: Rishabh Bhatnagar <[email protected]>
---
drivers/remoteproc/qcom_q6v5_pas.c | 11 +++++++++++
include/trace/events/q6v5_pas.h | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 45 insertions(+)
create mode 100644 include/trace/events/q6v5_pas.h

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 3837f23..b3c0a6a 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -29,6 +29,9 @@
#include "qcom_q6v5.h"
#include "remoteproc_internal.h"

+#define CREATE_TRACE_POINTS
+#include <trace/events/q6v5_pas.h>
+
struct adsp_data {
int crash_reason_smem;
const char *firmware_name;
@@ -121,12 +124,14 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
int ret;

+ trace_q6v5_pas("setting up memory and loading segments", rproc->name);
ret = qcom_mdt_load(adsp->dev, fw, rproc->firmware, adsp->pas_id,
adsp->mem_region, adsp->mem_phys, adsp->mem_size,
&adsp->mem_reloc);
if (ret)
return ret;

+ trace_q6v5_pas("done loading segments", rproc->name);
qcom_pil_info_store(adsp->info_name, adsp->mem_phys, adsp->mem_size);

return 0;
@@ -137,6 +142,7 @@ static int adsp_start(struct rproc *rproc)
struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
int ret;

+ trace_q6v5_pas("Voting for resources", rproc->name);
qcom_q6v5_prepare(&adsp->q6v5);

ret = adsp_pds_enable(adsp, adsp->active_pds, adsp->active_pd_count);
@@ -163,12 +169,14 @@ static int adsp_start(struct rproc *rproc)
if (ret)
goto disable_cx_supply;

+ trace_q6v5_pas("Before authenticate and reset", rproc->name);
ret = qcom_scm_pas_auth_and_reset(adsp->pas_id);
if (ret) {
dev_err(adsp->dev,
"failed to authenticate image and release reset\n");
goto disable_px_supply;
}
+ trace_q6v5_pas("After authenticate and reset", rproc->name);

ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000));
if (ret == -ETIMEDOUT) {
@@ -177,6 +185,7 @@ static int adsp_start(struct rproc *rproc)
goto disable_px_supply;
}

+ trace_q6v5_pas("Remoteproc is up", rproc->name);
return 0;

disable_px_supply:
@@ -214,6 +223,7 @@ static int adsp_stop(struct rproc *rproc)
int handover;
int ret;

+ trace_q6v5_pas("Request stop", rproc->name);
ret = qcom_q6v5_request_stop(&adsp->q6v5);
if (ret == -ETIMEDOUT)
dev_err(adsp->dev, "timed out on wait\n");
@@ -227,6 +237,7 @@ static int adsp_stop(struct rproc *rproc)
if (handover)
qcom_pas_handover(&adsp->q6v5);

+ trace_q6v5_pas("Remoteproc is down", rproc->name);
return ret;
}

diff --git a/include/trace/events/q6v5_pas.h b/include/trace/events/q6v5_pas.h
new file mode 100644
index 0000000..38ee5e2
--- /dev/null
+++ b/include/trace/events/q6v5_pas.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2020, The Linux Foundation. All rights reserved.
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM q6v5_pas
+
+#if !defined(_TRACE_Q6V5_PAS_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_Q6V5_PAS_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(q6v5_pas,
+
+ TP_PROTO(const char *event, const char *rproc_name),
+
+ TP_ARGS(event, rproc_name),
+
+ TP_STRUCT__entry(
+ __string(event, event)
+ __string(rproc_name, rproc_name)
+ ),
+
+ TP_fast_assign(
+ __assign_str(event, event);
+ __assign_str(rproc_name, rproc_name);
+ ),
+
+ TP_printk("event=%s remoteproc:%s", __get_str(event), __get_str(rproc_name))
+);
+
+#endif
+#include <trace/define_trace.h>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2020-11-10 03:31:41

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH 2/2] remoteproc: qcom: Add trace events for q6v5_pas driver

On Mon 09 Nov 19:20 CST 2020, Rishabh Bhatnagar wrote:

> Add tracepoints for q6v5_pas driver. These will help in
> analyzing the time taken by each step in remoteproc
> bootup/shutdown process and also serve as standard
> checkpoints in code.
>

These tracepoints seems quite generic and useful to drivers other than
the Qualcomm PAS driver. Please move them into the framework instead.

> Signed-off-by: Rishabh Bhatnagar <[email protected]>
> ---
> drivers/remoteproc/qcom_q6v5_pas.c | 11 +++++++++++
> include/trace/events/q6v5_pas.h | 34 ++++++++++++++++++++++++++++++++++
> 2 files changed, 45 insertions(+)
> create mode 100644 include/trace/events/q6v5_pas.h
>
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 3837f23..b3c0a6a 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -29,6 +29,9 @@
> #include "qcom_q6v5.h"
> #include "remoteproc_internal.h"
>
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/q6v5_pas.h>
> +
> struct adsp_data {
> int crash_reason_smem;
> const char *firmware_name;
> @@ -121,12 +124,14 @@ static int adsp_load(struct rproc *rproc, const struct firmware *fw)
> struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
> int ret;
>
> + trace_q6v5_pas("setting up memory and loading segments", rproc->name);
> ret = qcom_mdt_load(adsp->dev, fw, rproc->firmware, adsp->pas_id,
> adsp->mem_region, adsp->mem_phys, adsp->mem_size,
> &adsp->mem_reloc);
> if (ret)
> return ret;
>
> + trace_q6v5_pas("done loading segments", rproc->name);
> qcom_pil_info_store(adsp->info_name, adsp->mem_phys, adsp->mem_size);
>
> return 0;
> @@ -137,6 +142,7 @@ static int adsp_start(struct rproc *rproc)
> struct qcom_adsp *adsp = (struct qcom_adsp *)rproc->priv;
> int ret;
>
> + trace_q6v5_pas("Voting for resources", rproc->name);
> qcom_q6v5_prepare(&adsp->q6v5);
>
> ret = adsp_pds_enable(adsp, adsp->active_pds, adsp->active_pd_count);
> @@ -163,12 +169,14 @@ static int adsp_start(struct rproc *rproc)
> if (ret)
> goto disable_cx_supply;
>
> + trace_q6v5_pas("Before authenticate and reset", rproc->name);
> ret = qcom_scm_pas_auth_and_reset(adsp->pas_id);
> if (ret) {
> dev_err(adsp->dev,
> "failed to authenticate image and release reset\n");
> goto disable_px_supply;
> }
> + trace_q6v5_pas("After authenticate and reset", rproc->name);
>
> ret = qcom_q6v5_wait_for_start(&adsp->q6v5, msecs_to_jiffies(5000));
> if (ret == -ETIMEDOUT) {
> @@ -177,6 +185,7 @@ static int adsp_start(struct rproc *rproc)
> goto disable_px_supply;
> }
>
> + trace_q6v5_pas("Remoteproc is up", rproc->name);
> return 0;
>
> disable_px_supply:
> @@ -214,6 +223,7 @@ static int adsp_stop(struct rproc *rproc)
> int handover;
> int ret;
>
> + trace_q6v5_pas("Request stop", rproc->name);
> ret = qcom_q6v5_request_stop(&adsp->q6v5);
> if (ret == -ETIMEDOUT)
> dev_err(adsp->dev, "timed out on wait\n");
> @@ -227,6 +237,7 @@ static int adsp_stop(struct rproc *rproc)
> if (handover)
> qcom_pas_handover(&adsp->q6v5);
>
> + trace_q6v5_pas("Remoteproc is down", rproc->name);
> return ret;
> }
>
> diff --git a/include/trace/events/q6v5_pas.h b/include/trace/events/q6v5_pas.h
> new file mode 100644
> index 0000000..38ee5e2
> --- /dev/null
> +++ b/include/trace/events/q6v5_pas.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + */
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM q6v5_pas
> +
> +#if !defined(_TRACE_Q6V5_PAS_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_Q6V5_PAS_H
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(q6v5_pas,
> +
> + TP_PROTO(const char *event, const char *rproc_name),

Rather than distinguishing the trace events by the textual first
parameter, split it into individual trace events for each event.

Regards,
Bjorn

> +
> + TP_ARGS(event, rproc_name),
> +
> + TP_STRUCT__entry(
> + __string(event, event)
> + __string(rproc_name, rproc_name)
> + ),
> +
> + TP_fast_assign(
> + __assign_str(event, event);
> + __assign_str(rproc_name, rproc_name);
> + ),
> +
> + TP_printk("event=%s remoteproc:%s", __get_str(event), __get_str(rproc_name))
> +);
> +
> +#endif
> +#include <trace/define_trace.h>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>