* [PATCH 0/2] Add trace events to q6v5_pas and mdt_loader driver @ 2020-11-10 1:20 Rishabh Bhatnagar 2020-11-10 1:20 ` [PATCH 1/2] soc: qcom: Add tracepoints to mdt loader Rishabh Bhatnagar 2020-11-10 1:20 ` [PATCH 2/2] remoteproc: qcom: Add trace events for q6v5_pas driver Rishabh Bhatnagar 0 siblings, 2 replies; 5+ messages in thread From: Rishabh Bhatnagar @ 2020-11-10 1:20 UTC (permalink / raw) To: linux-remoteproc, linux-kernel Cc: bjorn.andersson, tsoni, psodagud, sidgup, Rishabh Bhatnagar 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] soc: qcom: Add tracepoints to mdt loader 2020-11-10 1:20 [PATCH 0/2] Add trace events to q6v5_pas and mdt_loader driver Rishabh Bhatnagar @ 2020-11-10 1:20 ` Rishabh Bhatnagar 2020-11-10 3:35 ` Bjorn Andersson 2020-11-10 1:20 ` [PATCH 2/2] remoteproc: qcom: Add trace events for q6v5_pas driver Rishabh Bhatnagar 1 sibling, 1 reply; 5+ messages in thread From: Rishabh Bhatnagar @ 2020-11-10 1:20 UTC (permalink / raw) To: linux-remoteproc, linux-kernel Cc: bjorn.andersson, tsoni, psodagud, sidgup, Rishabh Bhatnagar Add trace events to the mdt loader driver. These events can help us trace the region where we are loading the segments and the time it takes to initialize the image and setup the memory region. Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org> --- drivers/soc/qcom/mdt_loader.c | 8 ++++++ include/trace/events/mdt_loader.h | 57 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 include/trace/events/mdt_loader.h diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c index 24cd193..df69e23 100644 --- a/drivers/soc/qcom/mdt_loader.c +++ b/drivers/soc/qcom/mdt_loader.c @@ -17,6 +17,9 @@ #include <linux/slab.h> #include <linux/soc/qcom/mdt_loader.h> +#define CREATE_TRACE_POINTS +#include <trace/events/mdt_loader.h> + static bool mdt_phdr_valid(const struct elf32_phdr *phdr) { if (phdr->p_type != PT_LOAD) @@ -169,6 +172,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, goto out; } + trace_memory_setup("pas_init_image", fw_name); ret = qcom_scm_pas_init_image(pas_id, metadata, metadata_len); kfree(metadata); @@ -196,8 +200,10 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, if (relocate) { if (pas_init) { + trace_memory_setup("pas_mem_setup", fw_name); ret = qcom_scm_pas_mem_setup(pas_id, mem_phys, max_addr - min_addr); + if (ret) { dev_err(dev, "unable to setup relocation\n"); goto out; @@ -232,6 +238,8 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, ptr = mem_region + offset; + trace_regions(ptr, phdr->p_filesz, i); + if (phdr->p_filesz && phdr->p_offset < fw->size) { /* Firmware is large enough to be non-split */ if (phdr->p_offset + phdr->p_filesz > fw->size) { diff --git a/include/trace/events/mdt_loader.h b/include/trace/events/mdt_loader.h new file mode 100644 index 0000000..6299f65 --- /dev/null +++ b/include/trace/events/mdt_loader.h @@ -0,0 +1,57 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2020, The Linux Foundation. All rights reserved. + */ + +#undef TRACE_SYSTEM +#define TRACE_SYSTEM mdt_loader + +#if !defined(_TRACE_MDT_LOADER_H) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_MDT_LOADER_H + +#include <linux/types.h> +#include <linux/tracepoint.h> + +TRACE_EVENT(memory_setup, + + TP_PROTO(const char *event, char *fw_name), + + TP_ARGS(event, fw_name), + + TP_STRUCT__entry( + __string(event, event) + __string(fw_name, fw_name) + ), + + TP_fast_assign( + __assign_str(event, event); + __assign_str(fw_name, fw_name); + ), + + TP_printk("doing %s for %s", __get_str(event), __get_str(fw_name)) +); + +TRACE_EVENT(regions, + + TP_PROTO(void *region_start, size_t region_size, int i), + + TP_ARGS(region_start, region_size, i), + + TP_STRUCT__entry( + __field(void *, region_start) + __field(size_t, region_size) + __field(int, index) + ), + + TP_fast_assign( + __entry->region_start = region_start; + __entry->region_size = region_size; + __entry->index = i; + ), + + TP_printk("segment %d: region start=%pK size=%zx", __entry->index, + __entry->region_start, __entry->region_size) +); + +#endif +#include <trace/define_trace.h> -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] soc: qcom: Add tracepoints to mdt loader 2020-11-10 1:20 ` [PATCH 1/2] soc: qcom: Add tracepoints to mdt loader Rishabh Bhatnagar @ 2020-11-10 3:35 ` Bjorn Andersson 0 siblings, 0 replies; 5+ messages in thread From: Bjorn Andersson @ 2020-11-10 3:35 UTC (permalink / raw) To: Rishabh Bhatnagar; +Cc: linux-remoteproc, linux-kernel, tsoni, psodagud, sidgup On Mon 09 Nov 19:20 CST 2020, Rishabh Bhatnagar wrote: > Add trace events to the mdt loader driver. These events > can help us trace the region where we are loading the > segments and the time it takes to initialize the image > and setup the memory region. > > Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org> > --- > drivers/soc/qcom/mdt_loader.c | 8 ++++++ > include/trace/events/mdt_loader.h | 57 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 65 insertions(+) > create mode 100644 include/trace/events/mdt_loader.h > > diff --git a/drivers/soc/qcom/mdt_loader.c b/drivers/soc/qcom/mdt_loader.c > index 24cd193..df69e23 100644 > --- a/drivers/soc/qcom/mdt_loader.c > +++ b/drivers/soc/qcom/mdt_loader.c > @@ -17,6 +17,9 @@ > #include <linux/slab.h> > #include <linux/soc/qcom/mdt_loader.h> > > +#define CREATE_TRACE_POINTS > +#include <trace/events/mdt_loader.h> > + > static bool mdt_phdr_valid(const struct elf32_phdr *phdr) > { > if (phdr->p_type != PT_LOAD) > @@ -169,6 +172,7 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, > goto out; > } > > + trace_memory_setup("pas_init_image", fw_name); I think it would be favourable if you pushed this into the PAS functions in the scm driver instead. > ret = qcom_scm_pas_init_image(pas_id, metadata, metadata_len); > > kfree(metadata); > @@ -196,8 +200,10 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, > > if (relocate) { > if (pas_init) { > + trace_memory_setup("pas_mem_setup", fw_name); > ret = qcom_scm_pas_mem_setup(pas_id, mem_phys, > max_addr - min_addr); > + > if (ret) { > dev_err(dev, "unable to setup relocation\n"); > goto out; > @@ -232,6 +238,8 @@ static int __qcom_mdt_load(struct device *dev, const struct firmware *fw, > > ptr = mem_region + offset; > > + trace_regions(ptr, phdr->p_filesz, i); "regions" is a very generic name for a trace event, perhaps trace_qcom_mdt_load_segment() ? I think it would be quite useful with a trace event indicating which firmware mdt file (and what .bXX files) we're trying to load. PS. ptr is a virtual address, there's no point in tracing this - we're interested in "mem_reloc + offset". Regards, Bjorn > + > if (phdr->p_filesz && phdr->p_offset < fw->size) { > /* Firmware is large enough to be non-split */ > if (phdr->p_offset + phdr->p_filesz > fw->size) { > diff --git a/include/trace/events/mdt_loader.h b/include/trace/events/mdt_loader.h > new file mode 100644 > index 0000000..6299f65 > --- /dev/null > +++ b/include/trace/events/mdt_loader.h > @@ -0,0 +1,57 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2020, The Linux Foundation. All rights reserved. > + */ > + > +#undef TRACE_SYSTEM > +#define TRACE_SYSTEM mdt_loader > + > +#if !defined(_TRACE_MDT_LOADER_H) || defined(TRACE_HEADER_MULTI_READ) > +#define _TRACE_MDT_LOADER_H > + > +#include <linux/types.h> > +#include <linux/tracepoint.h> > + > +TRACE_EVENT(memory_setup, > + > + TP_PROTO(const char *event, char *fw_name), > + > + TP_ARGS(event, fw_name), > + > + TP_STRUCT__entry( > + __string(event, event) > + __string(fw_name, fw_name) > + ), > + > + TP_fast_assign( > + __assign_str(event, event); > + __assign_str(fw_name, fw_name); > + ), > + > + TP_printk("doing %s for %s", __get_str(event), __get_str(fw_name)) > +); > + > +TRACE_EVENT(regions, > + > + TP_PROTO(void *region_start, size_t region_size, int i), > + > + TP_ARGS(region_start, region_size, i), > + > + TP_STRUCT__entry( > + __field(void *, region_start) > + __field(size_t, region_size) > + __field(int, index) > + ), > + > + TP_fast_assign( > + __entry->region_start = region_start; > + __entry->region_size = region_size; > + __entry->index = i; > + ), > + > + TP_printk("segment %d: region start=%pK size=%zx", __entry->index, > + __entry->region_start, __entry->region_size) > +); > + > +#endif > +#include <trace/define_trace.h> > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] remoteproc: qcom: Add trace events for q6v5_pas driver 2020-11-10 1:20 [PATCH 0/2] Add trace events to q6v5_pas and mdt_loader driver Rishabh Bhatnagar 2020-11-10 1:20 ` [PATCH 1/2] soc: qcom: Add tracepoints to mdt loader Rishabh Bhatnagar @ 2020-11-10 1:20 ` Rishabh Bhatnagar 2020-11-10 3:29 ` Bjorn Andersson 1 sibling, 1 reply; 5+ messages in thread From: Rishabh Bhatnagar @ 2020-11-10 1:20 UTC (permalink / raw) To: linux-remoteproc, linux-kernel Cc: bjorn.andersson, tsoni, psodagud, sidgup, Rishabh Bhatnagar 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 <rishabhb@codeaurora.org> --- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] remoteproc: qcom: Add trace events for q6v5_pas driver 2020-11-10 1:20 ` [PATCH 2/2] remoteproc: qcom: Add trace events for q6v5_pas driver Rishabh Bhatnagar @ 2020-11-10 3:29 ` Bjorn Andersson 0 siblings, 0 replies; 5+ messages in thread From: Bjorn Andersson @ 2020-11-10 3:29 UTC (permalink / raw) To: Rishabh Bhatnagar; +Cc: linux-remoteproc, linux-kernel, tsoni, psodagud, sidgup 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 <rishabhb@codeaurora.org> > --- > 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-10 3:35 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-10 1:20 [PATCH 0/2] Add trace events to q6v5_pas and mdt_loader driver Rishabh Bhatnagar 2020-11-10 1:20 ` [PATCH 1/2] soc: qcom: Add tracepoints to mdt loader Rishabh Bhatnagar 2020-11-10 3:35 ` Bjorn Andersson 2020-11-10 1:20 ` [PATCH 2/2] remoteproc: qcom: Add trace events for q6v5_pas driver Rishabh Bhatnagar 2020-11-10 3:29 ` Bjorn Andersson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).