linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Maulik Shah <mkshah@codeaurora.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: swboyd@chromium.org, mka@chromium.org, evgreen@chromium.org,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
	agross@kernel.org, dianders@chromium.org, linux@roeck-us.net,
	rnayak@codeaurora.org, lsrao@codeaurora.org,
	Mahesh Sivasubramanian <msivasub@codeaurora.org>,
	Lina Iyer <ilina@codeaurora.org>
Subject: Re: [PATCH v10 2/5] soc: qcom: Add SoC sleep stats driver
Date: Wed, 6 Oct 2021 20:48:42 +0530	[thread overview]
Message-ID: <d63173d7-4885-4e9d-6b44-77a88d7a71b7@codeaurora.org> (raw)
In-Reply-To: <YVx4bFN05OPLll6u@ripper>

Hi,

On 10/5/2021 9:38 PM, Bjorn Andersson wrote:
> On Tue 05 Oct 02:11 PDT 2021, Maulik Shah wrote:
> 
>> From: Mahesh Sivasubramanian <msivasub@codeaurora.org>
>>
>> Let's add a driver to read the stats from remote processor and
>> export to debugfs.
>>
>> The driver creates "qcom_sleep_stats" directory in debugfs and
>> adds files for various low power mode available. Below is sample
>> output with command
>>
>> cat /sys/kernel/debug/qcom_sleep_stats/ddr
>> count = 0
>> Last Entered At = 0
>> Last Exited At = 0
>> Accumulated Duration = 0
>>
> 
> What's the minimal effort needed to get any of these values to become
> non-zero, on any of the upstream boards?

on SC7180/SC7280 ADSP stats should show up non-zero values after boot 
up. The SoC level stats will become non-zero when device is enters into
deepeest low power mode (cxsd) which also can be checked on sc7280.

For the stats read from AOP sram, The low power mode name like 'ddr', 
'cxsd' or 'aosd' is also read from stats area so in a way its correctly
map to respective addresses.

> 
>> Signed-off-by: Mahesh Sivasubramanian <msivasub@codeaurora.org>
>> Signed-off-by: Lina Iyer <ilina@codeaurora.org>
>> [mkshah: add subsystem sleep stats, create one file for each stat]
>> Signed-off-by: Maulik Shah <mkshah@codeaurora.org>
>> ---
>>   drivers/soc/qcom/Kconfig            |  10 ++
>>   drivers/soc/qcom/Makefile           |   1 +
>>   drivers/soc/qcom/qcom_sleep_stats.c | 240 ++++++++++++++++++++++++++++++++++++
>>   3 files changed, 251 insertions(+)
>>   create mode 100644 drivers/soc/qcom/qcom_sleep_stats.c
>>
>> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
>> index abfef20..ad45f59 100644
>> --- a/drivers/soc/qcom/Kconfig
>> +++ b/drivers/soc/qcom/Kconfig
>> @@ -182,6 +182,16 @@ config QCOM_SMSM
>>   	  Say yes here to support the Qualcomm Shared Memory State Machine.
>>   	  The state machine is represented by bits in shared memory.
>>   
>> +config QCOM_SLEEP_STATS
>> +	tristate "Qualcomm Technologies, Inc. (QTI) Sleep stats driver"
>> +	depends on ARCH_QCOM && DEBUG_FS || COMPILE_TEST
> 
> Is this equivalent to (QCOM && DBGFS) || COMPILE_TEST or
> QCOM && (DBGFS || COMPILE_TEST) ?
> 
> The prior seems like what we want.
> 
> 
> I think the rest looks good now.
> 
> Thanks,
> Bjorn

Updated in v11 to mention (QCOM && DBGFS) || COMPILE_TEST.

Thanks,
Maulik

> 
>> +	depends on QCOM_SMEM
>> +	help
>> +	  Qualcomm Technologies, Inc. (QTI) Sleep stats driver to read
>> +	  the shared memory exported by the remote processor related to
>> +	  various SoC level low power modes statistics and export to debugfs
>> +	  interface.
>> +
>>   config QCOM_SOCINFO
>>   	tristate "Qualcomm socinfo driver"
>>   	depends on QCOM_SMEM
>> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
>> index 24514c7..ad84f7b 100644
>> --- a/drivers/soc/qcom/Makefile
>> +++ b/drivers/soc/qcom/Makefile
>> @@ -19,6 +19,7 @@ obj-$(CONFIG_QCOM_SMEM) +=	smem.o
>>   obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
>>   obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
>>   obj-$(CONFIG_QCOM_SMSM)	+= smsm.o
>> +obj-$(CONFIG_QCOM_SLEEP_STATS)	+= qcom_sleep_stats.o
>>   obj-$(CONFIG_QCOM_SOCINFO)	+= socinfo.o
>>   obj-$(CONFIG_QCOM_SPM)		+= spm.o
>>   obj-$(CONFIG_QCOM_WCNSS_CTRL) += wcnss_ctrl.o
>> diff --git a/drivers/soc/qcom/qcom_sleep_stats.c b/drivers/soc/qcom/qcom_sleep_stats.c
>> new file mode 100644
>> index 0000000..c85e19b
>> --- /dev/null
>> +++ b/drivers/soc/qcom/qcom_sleep_stats.c
>> @@ -0,0 +1,240 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2011-2021, The Linux Foundation. All rights reserved.
>> + */
>> +
>> +#include <linux/debugfs.h>
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/seq_file.h>
>> +
>> +#include <linux/soc/qcom/smem.h>
>> +#include <clocksource/arm_arch_timer.h>
>> +
>> +#define STAT_TYPE_OFFSET	0x0
>> +#define COUNT_OFFSET		0x4
>> +#define LAST_ENTERED_AT_OFFSET	0x8
>> +#define LAST_EXITED_AT_OFFSET	0x10
>> +#define ACCUMULATED_OFFSET	0x18
>> +#define CLIENT_VOTES_OFFSET	0x1c
>> +
>> +struct subsystem_data {
>> +	const char *name;
>> +	u32 smem_item;
>> +	u32 pid;
>> +};
>> +
>> +static const struct subsystem_data subsystems[] = {
>> +	{ "modem", 605, 1 },
>> +	{ "wpss", 605, 13 },
>> +	{ "adsp", 606, 2 },
>> +	{ "cdsp", 607, 5 },
>> +	{ "slpi", 608, 3 },
>> +	{ "gpu", 609, 0 },
>> +	{ "display", 610, 0 },
>> +	{ "adsp_island", 613, 2 },
>> +	{ "slpi_island", 613, 3 },
>> +};
>> +
>> +struct stats_config {
>> +	size_t stats_offset;
>> +	size_t num_records;
>> +	bool appended_stats_avail;
>> +};
>> +
>> +struct stats_data {
>> +	bool appended_stats_avail;
>> +	void __iomem *base;
>> +};
>> +
>> +struct sleep_stats {
>> +	u32 stat_type;
>> +	u32 count;
>> +	u64 last_entered_at;
>> +	u64 last_exited_at;
>> +	u64 accumulated;
>> +};
>> +
>> +struct appended_stats {
>> +	u32 client_votes;
>> +	u32 reserved[3];
>> +};
>> +
>> +static void qcom_print_stats(struct seq_file *s, const struct sleep_stats *stat)
>> +{
>> +	u64 accumulated = stat->accumulated;
>> +	/*
>> +	 * If a subsystem is in sleep when reading the sleep stats adjust
>> +	 * the accumulated sleep duration to show actual sleep time.
>> +	 */
>> +	if (stat->last_entered_at > stat->last_exited_at)
>> +		accumulated += arch_timer_read_counter() - stat->last_entered_at;
>> +
>> +	seq_printf(s, "Count: %u\n", stat->count);
>> +	seq_printf(s, "Last Entered At: %llu\n", stat->last_entered_at);
>> +	seq_printf(s, "Last Exited At: %llu\n", stat->last_exited_at);
>> +	seq_printf(s, "Accumulated Duration: %llu\n", accumulated);
>> +}
>> +
>> +static int qcom_subsystem_sleep_stats_show(struct seq_file *s, void *unused)
>> +{
>> +	struct subsystem_data *subsystem = s->private;
>> +	struct sleep_stats *stat;
>> +
>> +	/* Items are allocated lazily, so lookup pointer each time */
>> +	stat = qcom_smem_get(subsystem->pid, subsystem->smem_item, NULL);
>> +	if (IS_ERR(stat))
>> +		return -EIO;
>> +
>> +	qcom_print_stats(s, stat);
>> +
>> +	return 0;
>> +}
>> +
>> +static int qcom_soc_sleep_stats_show(struct seq_file *s, void *unused)
>> +{
>> +	struct stats_data *d = s->private;
>> +	void __iomem *reg = d->base;
>> +	struct sleep_stats stat;
>> +
>> +	memcpy_fromio(&stat, reg, sizeof(stat));
>> +	qcom_print_stats(s, &stat);
>> +
>> +	if (d->appended_stats_avail) {
>> +		struct appended_stats votes;
>> +
>> +		memcpy_fromio(&votes, reg + CLIENT_VOTES_OFFSET, sizeof(votes));
>> +		seq_printf(s, "Client Votes: %#x\n", votes.client_votes);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +DEFINE_SHOW_ATTRIBUTE(qcom_soc_sleep_stats);
>> +DEFINE_SHOW_ATTRIBUTE(qcom_subsystem_sleep_stats);
>> +
>> +static void qcom_create_soc_sleep_stat_files(struct dentry *root, void __iomem *reg,
>> +					     struct stats_data *d,
>> +					     const struct stats_config *config)
>> +{
>> +	char stat_type[sizeof(u32) + 1] = {0};
>> +	u32 offset = 0, type;
>> +	int i, j;
>> +
>> +	for (i = 0; i < config->num_records; i++) {
>> +		d[i].base = reg + offset + config->stats_offset;
>> +
>> +		/*
>> +		 * Read the low power mode name and create debugfs file for it.
>> +		 * The names read could be of below,
>> +		 * (may change depending on low power mode supported).
>> +		 * For rpmh-sleep-stats: "aosd", "cxsd" and "ddr".
>> +		 * For rpm-sleep-stats: "vmin" and "vlow".
>> +		 */
>> +		type = readl(d[i].base);
>> +		for (j = 0; j < sizeof(u32); j++) {
>> +			stat_type[j] = type & 0xff;
>> +			type = type >> 8;
>> +		}
>> +		strim(stat_type);
>> +		debugfs_create_file(stat_type, 0400, root, &d[i],
>> +				    &qcom_soc_sleep_stats_fops);
>> +
>> +		offset += sizeof(struct sleep_stats);
>> +		if (d[i].appended_stats_avail)
>> +			offset += sizeof(struct appended_stats);
>> +	}
>> +}
>> +
>> +static void qcom_create_subsystem_stat_files(struct dentry *root)
>> +{
>> +	const struct sleep_stats *stat;
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(subsystems); i++) {
>> +		stat = qcom_smem_get(subsystems[i].pid, subsystems[i].smem_item, NULL);
>> +		if (IS_ERR(stat))
>> +			continue;
>> +
>> +		debugfs_create_file(subsystems[i].name, 0400, root, (void *)&subsystems[i],
>> +				    &qcom_subsystem_sleep_stats_fops);
>> +	}
>> +}
>> +
>> +static int qcom_sleep_stats_probe(struct platform_device *pdev)
>> +{
>> +	void __iomem *reg;
>> +	struct dentry *root;
>> +	const struct stats_config *config;
>> +	struct stats_data *d;
>> +	int i;
>> +
>> +	config = device_get_match_data(&pdev->dev);
>> +	if (!config)
>> +		return -ENODEV;
>> +
>> +	reg = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
>> +	if (IS_ERR(reg))
>> +		return -ENOMEM;
>> +
>> +	d = devm_kcalloc(&pdev->dev, config->num_records,
>> +			 sizeof(*d), GFP_KERNEL);
>> +	if (!d)
>> +		return -ENOMEM;
>> +
>> +	for (i = 0; i < config->num_records; i++)
>> +		d[i].appended_stats_avail = config->appended_stats_avail;
>> +
>> +	root = debugfs_create_dir("qcom_sleep_stats", NULL);
>> +
>> +	qcom_create_subsystem_stat_files(root);
>> +	qcom_create_soc_sleep_stat_files(root, reg, d, config);
>> +
>> +	platform_set_drvdata(pdev, root);
>> +
>> +	return 0;
>> +}
>> +
>> +static int qcom_sleep_stats_remove(struct platform_device *pdev)
>> +{
>> +	struct dentry *root = platform_get_drvdata(pdev);
>> +
>> +	debugfs_remove_recursive(root);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct stats_config rpm_data = {
>> +	.stats_offset = 0x2e8,
>> +	.num_records = 2,
>> +	.appended_stats_avail = true,
>> +};
>> +
>> +static const struct stats_config rpmh_data = {
>> +	.stats_offset = 0x48,
>> +	.num_records = 3,
>> +	.appended_stats_avail = false,
>> +};
>> +
>> +static const struct of_device_id qcom_sleep_stats_table[] = {
>> +	{ .compatible = "qcom,rpm-sleep-stats", .data = &rpm_data },
>> +	{ .compatible = "qcom,rpmh-sleep-stats", .data = &rpmh_data },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, qcom_sleep_stats_table);
>> +
>> +static struct platform_driver qcom_sleep_stats = {
>> +	.probe = qcom_sleep_stats_probe,
>> +	.remove = qcom_sleep_stats_remove,
>> +	.driver = {
>> +		.name = "qcom_sleep_stats",
>> +		.of_match_table = qcom_sleep_stats_table,
>> +	},
>> +};
>> +module_platform_driver(qcom_sleep_stats);
>> +
>> +MODULE_DESCRIPTION("Qualcomm Technologies, Inc. (QTI) Sleep Stats driver");
>> +MODULE_LICENSE("GPL v2");
>> -- 
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
>> of Code Aurora Forum, hosted by The Linux Foundation
>>

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member of Code Aurora Forum, hosted by The Linux Foundation

  reply	other threads:[~2021-10-06 15:19 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-05  9:11 [PATCH v10 0/5] Introduce SoC sleep stats driver Maulik Shah
2021-10-05  9:11 ` [PATCH v10 1/5] dt-bindings: Introduce SoC sleep stats bindings Maulik Shah
2021-10-05 12:26   ` Rob Herring
2021-10-05 16:01   ` Bjorn Andersson
2021-10-06 15:14     ` Maulik Shah
2021-10-05  9:11 ` [PATCH v10 2/5] soc: qcom: Add SoC sleep stats driver Maulik Shah
2021-10-05 16:08   ` Bjorn Andersson
2021-10-06 15:18     ` Maulik Shah [this message]
2021-10-05  9:11 ` [PATCH v10 3/5] arm64: dts: qcom: sc7180: Enable SoC sleep stats Maulik Shah
2021-10-05  9:11 ` [PATCH v10 4/5] arm64: defconfig: Enable SoC sleep stats driver Maulik Shah
2021-10-05  9:11 ` [PATCH v10 5/5] arm64: dts: qcom: sc7280: Enable SoC sleep stats Maulik Shah
2021-10-05 16:09   ` Bjorn Andersson
2021-10-06 15:20     ` Maulik Shah

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d63173d7-4885-4e9d-6b44-77a88d7a71b7@codeaurora.org \
    --to=mkshah@codeaurora.org \
    --cc=agross@kernel.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=dianders@chromium.org \
    --cc=evgreen@chromium.org \
    --cc=ilina@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=lsrao@codeaurora.org \
    --cc=mka@chromium.org \
    --cc=msivasub@codeaurora.org \
    --cc=rnayak@codeaurora.org \
    --cc=swboyd@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).