[PATCH v4 0/3] soc: qcom: qcom_stats: Add DDR stats

[PATCH v4 0/3] soc: qcom: qcom_stats: Add DDR stats

[Maulik Shah]

This series adds support to read various DDR low power mode and frequency
stats. This was added in past with series [1] but reverted with [4] due
to some SoCs boot up failures. This series is more aligned to downstream
implementation and fixes the issues mentioned in [4].

The series [1] tried to add three feature support

A. Reading DDR Frequency and low power stats from MSG RAM
   (targets where DDR stats are readily available in MSG RAM to read)

B. Trigger QMP to ask AOP to populate DDR Frequency and low power stats
   (targets where DDR stats are available but duration field syncing
   requires QMP message to be sent to AOP)

C. Trigger QMP to ask AOP to populate DDR vote table information
   (To read different DRV / Direct Resource Voter, CPUSS, DSPs's votes
   for DDR frequency)

Current series do not include reading the DDR vote table information (C)
part from [1] which is to be separately sent potentially including reading
other resources votes like Cx Rail level vote information. These vote
tables details are not strictly related to DDR Frequency and low power
stats (A) and (B) this series is adding.

This series updates respective SoC devicetree with QMP handle (where DDR
stats syncing is required) and it is backward compatible with older
devicetree as without the QMP handle present, ddr stats can be still be
read (duration field will be read as 0).

Note that [1] was only partially reverted and hence device binding update
for QMP handle [2] is already present along with the fix to have
dependency on AOSS QMP driver in Kconfig [3].

[1] https://lore.kernel.org/all/20231130-topic-ddr_sleep_stats-v1-0-5981c2e764b6@linaro.org/
[2] https://lore.kernel.org/all/20231130-topic-ddr_sleep_stats-v1-2-5981c2e764b6@linaro.org/
[3] https://lore.kernel.org/lkml/20231205-qcom_stats-aoss_qmp-dependency-v1-1-8dabe1b5c32a@quicinc.com/T/
[4] https://lore.kernel.org/all/20231214-topic-undo_ddr_stats-v1-1-1fe32c258e56@linaro.org/

Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
Changes in v4:
- Handle clean up for qmp_get() probe defer case
- Link to v3: https://lore.kernel.org/r/20250525-ddr_stats_-v3-0-49a3c1e42db7@oss.qualcomm.com

Changes in v3:
- Use correct format specifiers
- Handle qmp_get() failure cases
- Link to v2: https://lore.kernel.org/r/20250521-ddr_stats_-v2-0-2c54ea4fc071@oss.qualcomm.com

Changes in v2:
- Mention count in decimal instead of hex
- Update read failure cases to return error code instead of success
- Fix typo in comment
- Link to v1: https://lore.kernel.org/r/20250429-ddr_stats_-v1-0-4fc818aab7bb@oss.qualcomm.com

---
Maulik Shah (3):
      soc: qcom: qcom_stats: Add support to read DDR statistic
      soc: qcom: qcom_stats: Add QMP support for syncing ddr stats
      arm64: dts: qcom: Add QMP handle for qcom_stats

 arch/arm64/boot/dts/qcom/sm8450.dtsi |   1 +
 arch/arm64/boot/dts/qcom/sm8550.dtsi |   1 +
 arch/arm64/boot/dts/qcom/sm8650.dtsi |   1 +
 arch/arm64/boot/dts/qcom/sm8750.dtsi |   1 +
 drivers/soc/qcom/qcom_stats.c        | 131 +++++++++++++++++++++++++++++++++++
 5 files changed, 135 insertions(+)
---
base-commit: 393d0c54cae31317deaa9043320c5fd9454deabc
change-id: 20250426-ddr_stats_-391505b3da6a

Best regards,
-- 
Maulik Shah <maulik.shah@oss.qualcomm.com>

[PATCH v4 1/3] soc: qcom: qcom_stats: Add support to read DDR statistic

[Maulik Shah]

DDR statistic provide different DDR LPM and DDR frequency statistic.
Add support to read from MSGRAM and display via debugfs.

Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
 drivers/soc/qcom/qcom_stats.c | 99 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
index 5de99cf59b9fbe32c0580e371c3cc362dfabb895..33fd2a1574464768bd07289e743fbb79ba415e84 100644
--- a/drivers/soc/qcom/qcom_stats.c
+++ b/drivers/soc/qcom/qcom_stats.c
@@ -1,8 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (c) 2011-2021, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2022-2025, Qualcomm Innovation Center, Inc. All rights reserved.
  */
 
+#include <linux/bitfield.h>
 #include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/io.h>
@@ -24,6 +26,17 @@
 #define ACCUMULATED_OFFSET	0x18
 #define CLIENT_VOTES_OFFSET	0x20
 
+#define DDR_STATS_MAGIC_KEY		0xA1157A75
+#define DDR_STATS_MAX_NUM_MODES		20
+#define DDR_STATS_MAGIC_KEY_ADDR	0x0
+#define DDR_STATS_NUM_MODES_ADDR	0x4
+#define DDR_STATS_ENTRY_START_ADDR	0x8
+
+#define DDR_STATS_CP_IDX(data)		FIELD_GET(GENMASK(4, 0), data)
+#define DDR_STATS_LPM_NAME(data)	FIELD_GET(GENMASK(7, 0), data)
+#define DDR_STATS_TYPE(data)		FIELD_GET(GENMASK(15, 8), data)
+#define DDR_STATS_FREQ(data)		FIELD_GET(GENMASK(31, 16), data)
+
 struct subsystem_data {
 	const char *name;
 	u32 smem_item;
@@ -48,12 +61,19 @@ static const struct subsystem_data subsystems[] = {
 
 struct stats_config {
 	size_t stats_offset;
+	size_t ddr_stats_offset;
 	size_t num_records;
 	bool appended_stats_avail;
 	bool dynamic_offset;
 	bool subsystem_stats_in_smem;
 };
 
+struct ddr_stats_entry {
+	u32 name;
+	u32 count;
+	u64 duration;
+};
+
 struct stats_data {
 	bool appended_stats_avail;
 	void __iomem *base;
@@ -122,8 +142,85 @@ static int qcom_soc_sleep_stats_show(struct seq_file *s, void *unused)
 	return 0;
 }
 
+static void qcom_ddr_stats_print(struct seq_file *s, struct ddr_stats_entry *data)
+{
+	u32 cp_idx;
+
+	/*
+	 * DDR statistic have two different types of details encoded.
+	 * (1) DDR LPM Stats
+	 * (2) DDR Frequency Stats
+	 *
+	 * The name field have details like which type of DDR stat (bits 8:15)
+	 * along with other details as explained below
+	 *
+	 * In case of DDR LPM stat, name field will be encoded as,
+	 * Bits	 -  Meaning
+	 * 0:7	 -  DDR LPM name, can be of 0xd4, 0xd3, 0x11 and 0xd0.
+	 * 8:15	 -  0x0 (indicates its a LPM stat)
+	 * 16:31 -  Unused
+	 *
+	 * In case of DDR FREQ stats, name field will be encoded as,
+	 * Bits  -  Meaning
+	 * 0:4   -  DDR Clock plan index (CP IDX)
+	 * 5:7   -  Unused
+	 * 8:15  -  0x1 (indicates its Freq stat)
+	 * 16:31 -  Frequency value in Mhz
+	 */
+	switch (DDR_STATS_TYPE(data->name)) {
+	case 0:
+		seq_printf(s, "DDR LPM Stat Name:0x%lx\tcount:%u\tDuration (ticks):%llu\n",
+			   DDR_STATS_LPM_NAME(data->name), data->count, data->duration);
+		break;
+	case 1:
+		if (!data->count || !DDR_STATS_FREQ(data->name))
+			return;
+
+		cp_idx = DDR_STATS_CP_IDX(data->name);
+		seq_printf(s, "DDR Freq %luMhz:\tCP IDX:%u\tcount:%u\tDuration (ticks):%llu\n",
+			   DDR_STATS_FREQ(data->name), cp_idx, data->count, data->duration);
+		break;
+	}
+}
+
+static int qcom_ddr_stats_show(struct seq_file *s, void *d)
+{
+	struct ddr_stats_entry data[DDR_STATS_MAX_NUM_MODES];
+	void __iomem *reg = (void __iomem *)s->private;
+	u32 entry_count;
+	int i;
+
+	entry_count = readl_relaxed(reg + DDR_STATS_NUM_MODES_ADDR);
+	if (entry_count > DDR_STATS_MAX_NUM_MODES)
+		return -EINVAL;
+
+	reg += DDR_STATS_ENTRY_START_ADDR;
+	memcpy_fromio(data, reg, sizeof(struct ddr_stats_entry) * entry_count);
+
+	for (i = 0; i < entry_count; i++)
+		qcom_ddr_stats_print(s, &data[i]);
+
+	return 0;
+}
+
 DEFINE_SHOW_ATTRIBUTE(qcom_soc_sleep_stats);
 DEFINE_SHOW_ATTRIBUTE(qcom_subsystem_sleep_stats);
+DEFINE_SHOW_ATTRIBUTE(qcom_ddr_stats);
+
+static void qcom_create_ddr_stat_files(struct dentry *root, void __iomem *reg,
+				       const struct stats_config *config)
+{
+	u32 key;
+
+	if (!config->ddr_stats_offset)
+		return;
+
+	key = readl_relaxed(reg + config->ddr_stats_offset + DDR_STATS_MAGIC_KEY_ADDR);
+	if (key == DDR_STATS_MAGIC_KEY)
+		debugfs_create_file("ddr_stats", 0400, root,
+				    (__force void *)reg + config->ddr_stats_offset,
+				    &qcom_ddr_stats_fops);
+}
 
 static void qcom_create_soc_sleep_stat_files(struct dentry *root, void __iomem *reg,
 					     struct stats_data *d,
@@ -212,6 +309,7 @@ static int qcom_stats_probe(struct platform_device *pdev)
 
 	qcom_create_subsystem_stat_files(root, config);
 	qcom_create_soc_sleep_stat_files(root, reg, d, config);
+	qcom_create_ddr_stat_files(root, reg, config);
 
 	platform_set_drvdata(pdev, root);
 
@@ -254,6 +352,7 @@ static const struct stats_config rpmh_data_sdm845 = {
 
 static const struct stats_config rpmh_data = {
 	.stats_offset = 0x48,
+	.ddr_stats_offset = 0xb8,
 	.num_records = 3,
 	.appended_stats_avail = false,
 	.dynamic_offset = false,

-- 
2.34.1

[PATCH v4 2/3] soc: qcom: qcom_stats: Add QMP support for syncing ddr stats

[Maulik Shah]

Recent SoCs (SM8450 onwards) require QMP command to be sent before reading
ddr stats. The duration field of ddr stats will get populated only if QMP
command is sent.

Add support to send ddr stats freqsync QMP command.

Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
 drivers/soc/qcom/qcom_stats.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
index 33fd2a1574464768bd07289e743fbb79ba415e84..0545c8cbefb8f18758d4eb51638e4ecb94e05422 100644
--- a/drivers/soc/qcom/qcom_stats.c
+++ b/drivers/soc/qcom/qcom_stats.c
@@ -13,6 +13,7 @@
 #include <linux/platform_device.h>
 #include <linux/seq_file.h>
 
+#include <linux/soc/qcom/qcom_aoss.h>
 #include <linux/soc/qcom/smem.h>
 #include <clocksource/arm_arch_timer.h>
 
@@ -37,6 +38,8 @@
 #define DDR_STATS_TYPE(data)		FIELD_GET(GENMASK(15, 8), data)
 #define DDR_STATS_FREQ(data)		FIELD_GET(GENMASK(31, 16), data)
 
+static struct qmp *qcom_stats_qmp;
+
 struct subsystem_data {
 	const char *name;
 	u32 smem_item;
@@ -188,12 +191,28 @@ static int qcom_ddr_stats_show(struct seq_file *s, void *d)
 	struct ddr_stats_entry data[DDR_STATS_MAX_NUM_MODES];
 	void __iomem *reg = (void __iomem *)s->private;
 	u32 entry_count;
-	int i;
+	int i, ret;
 
 	entry_count = readl_relaxed(reg + DDR_STATS_NUM_MODES_ADDR);
 	if (entry_count > DDR_STATS_MAX_NUM_MODES)
 		return -EINVAL;
 
+	if (qcom_stats_qmp) {
+		/*
+		 * Recent SoCs (SM8450 onwards) do not have duration field
+		 * populated from boot up onwards for both DDR LPM Stats
+		 * and DDR Frequency Stats.
+		 *
+		 * Send QMP message to Always on processor which will
+		 * populate duration field into MSG RAM area.
+		 *
+		 * Sent every time to read latest data.
+		 */
+		ret = qmp_send(qcom_stats_qmp, "{class: ddr, action: freqsync}");
+		if (ret)
+			return ret;
+	}
+
 	reg += DDR_STATS_ENTRY_START_ADDR;
 	memcpy_fromio(data, reg, sizeof(struct ddr_stats_entry) * entry_count);
 
@@ -304,6 +323,19 @@ static int qcom_stats_probe(struct platform_device *pdev)
 
 	for (i = 0; i < config->num_records; i++)
 		d[i].appended_stats_avail = config->appended_stats_avail;
+	/*
+	 * QMP is used for DDR stats syncing to MSG RAM for recent SoCs (SM8450 onwards).
+	 * The prior SoCs do not need QMP handle as the required stats are already present
+	 * in MSG RAM, provided the DDR_STATS_MAGIC_KEY matches.
+	 */
+	qcom_stats_qmp = qmp_get(&pdev->dev);
+	if (IS_ERR(qcom_stats_qmp)) {
+		if (PTR_ERR(qcom_stats_qmp) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
+		/* We assume any other error means it's not defined/needed */
+		qcom_stats_qmp = NULL;
+	}
 
 	root = debugfs_create_dir("qcom_stats", NULL);
 

-- 
2.34.1

[PATCH v4 3/3] arm64: dts: qcom: Add QMP handle for qcom_stats

[Maulik Shah]

Add QMP handle which is used to send QMP command to always on processor
to populate DDR stats. Add QMP handle for SM8450/SM8550/SM8650/SM8750.

Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/sm8450.dtsi | 1 +
 arch/arm64/boot/dts/qcom/sm8550.dtsi | 1 +
 arch/arm64/boot/dts/qcom/sm8650.dtsi | 1 +
 arch/arm64/boot/dts/qcom/sm8750.dtsi | 1 +
 4 files changed, 4 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 54c6d0fdb2afa51084c510eddc341d6087189611..33574ad706b915136546c7f92c7cd0b8a0d62b7e 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -3739,6 +3739,7 @@ aoss_qmp: power-management@c300000 {
 		sram@c3f0000 {
 			compatible = "qcom,rpmh-stats";
 			reg = <0 0x0c3f0000 0 0x400>;
+			qcom,qmp = <&aoss_qmp>;
 		};
 
 		spmi_bus: spmi@c400000 {
diff --git a/arch/arm64/boot/dts/qcom/sm8550.dtsi b/arch/arm64/boot/dts/qcom/sm8550.dtsi
index 82cabf777cd2c1dc87457aeede913873e7322ec2..e8371a90b9b98fbc12a429def8f6246c6418540a 100644
--- a/arch/arm64/boot/dts/qcom/sm8550.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8550.dtsi
@@ -3943,6 +3943,7 @@ aoss_qmp: power-management@c300000 {
 		sram@c3f0000 {
 			compatible = "qcom,rpmh-stats";
 			reg = <0 0x0c3f0000 0 0x400>;
+			qcom,qmp = <&aoss_qmp>;
 		};
 
 		spmi_bus: spmi@c400000 {
diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi
index c2937f7217943c4ca91a91eadc8259b2d6a01372..875b5a89d2555f258665c881ee3d96965b6d7a6a 100644
--- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
@@ -5725,6 +5725,7 @@ aoss_qmp: power-management@c300000 {
 		sram@c3f0000 {
 			compatible = "qcom,rpmh-stats";
 			reg = <0 0x0c3f0000 0 0x400>;
+			qcom,qmp = <&aoss_qmp>;
 		};
 
 		spmi_bus: spmi@c400000 {
diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
index 149d2ed17641a085d510f3a8eab5a96304787f0c..4c54ed84e2d1ec836438448e2a02b6fe028f4c24 100644
--- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
@@ -2490,6 +2490,7 @@ aoss_qmp: power-management@c300000 {
 		sram@c3f0000 {
 			compatible = "qcom,rpmh-stats";
 			reg = <0x0 0x0c3f0000 0x0 0x400>;
+			qcom,qmp = <&aoss_qmp>;
 		};
 
 		spmi_bus: spmi@c400000 {

-- 
2.34.1

Re: [PATCH v4 2/3] soc: qcom: qcom_stats: Add QMP support for syncing ddr stats

[Dmitry Baryshkov]

On Wed, May 28, 2025 at 02:51:32PM +0530, Maulik Shah wrote:
> Recent SoCs (SM8450 onwards) require QMP command to be sent before reading
> ddr stats. The duration field of ddr stats will get populated only if QMP
> command is sent.
> 
> Add support to send ddr stats freqsync QMP command.
> 
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/qcom_stats.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
> index 33fd2a1574464768bd07289e743fbb79ba415e84..0545c8cbefb8f18758d4eb51638e4ecb94e05422 100644
> --- a/drivers/soc/qcom/qcom_stats.c
> +++ b/drivers/soc/qcom/qcom_stats.c
> @@ -13,6 +13,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/seq_file.h>
>  
> +#include <linux/soc/qcom/qcom_aoss.h>
>  #include <linux/soc/qcom/smem.h>
>  #include <clocksource/arm_arch_timer.h>
>  
> @@ -37,6 +38,8 @@
>  #define DDR_STATS_TYPE(data)		FIELD_GET(GENMASK(15, 8), data)
>  #define DDR_STATS_FREQ(data)		FIELD_GET(GENMASK(31, 16), data)
>  
> +static struct qmp *qcom_stats_qmp;
> +
>  struct subsystem_data {
>  	const char *name;
>  	u32 smem_item;
> @@ -188,12 +191,28 @@ static int qcom_ddr_stats_show(struct seq_file *s, void *d)
>  	struct ddr_stats_entry data[DDR_STATS_MAX_NUM_MODES];
>  	void __iomem *reg = (void __iomem *)s->private;
>  	u32 entry_count;
> -	int i;
> +	int i, ret;
>  
>  	entry_count = readl_relaxed(reg + DDR_STATS_NUM_MODES_ADDR);
>  	if (entry_count > DDR_STATS_MAX_NUM_MODES)
>  		return -EINVAL;
>  
> +	if (qcom_stats_qmp) {
> +		/*
> +		 * Recent SoCs (SM8450 onwards) do not have duration field
> +		 * populated from boot up onwards for both DDR LPM Stats
> +		 * and DDR Frequency Stats.
> +		 *
> +		 * Send QMP message to Always on processor which will
> +		 * populate duration field into MSG RAM area.
> +		 *
> +		 * Sent every time to read latest data.
> +		 */
> +		ret = qmp_send(qcom_stats_qmp, "{class: ddr, action: freqsync}");
> +		if (ret)
> +			return ret;
> +	}
> +
>  	reg += DDR_STATS_ENTRY_START_ADDR;
>  	memcpy_fromio(data, reg, sizeof(struct ddr_stats_entry) * entry_count);
>  
> @@ -304,6 +323,19 @@ static int qcom_stats_probe(struct platform_device *pdev)
>  
>  	for (i = 0; i < config->num_records; i++)
>  		d[i].appended_stats_avail = config->appended_stats_avail;
> +	/*
> +	 * QMP is used for DDR stats syncing to MSG RAM for recent SoCs (SM8450 onwards).
> +	 * The prior SoCs do not need QMP handle as the required stats are already present
> +	 * in MSG RAM, provided the DDR_STATS_MAGIC_KEY matches.
> +	 */
> +	qcom_stats_qmp = qmp_get(&pdev->dev);
> +	if (IS_ERR(qcom_stats_qmp)) {
> +		if (PTR_ERR(qcom_stats_qmp) == -EPROBE_DEFER)
> +			return -EPROBE_DEFER;
> +
> +		/* We assume any other error means it's not defined/needed */
> +		qcom_stats_qmp = NULL;

I still think that we shouldn't be ignoring actual errors here. I'd say,
check for of_property_present(dev->of_node, "qcom,qmp") before.

> +	}
>  
>  	root = debugfs_create_dir("qcom_stats", NULL);
>  
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v4 1/3] soc: qcom: qcom_stats: Add support to read DDR statistic

[Dmitry Baryshkov]

On Wed, May 28, 2025 at 02:51:31PM +0530, Maulik Shah wrote:
> DDR statistic provide different DDR LPM and DDR frequency statistic.
> Add support to read from MSGRAM and display via debugfs.
> 
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
>  drivers/soc/qcom/qcom_stats.c | 99 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 99 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

Re: [PATCH v4 3/3] arm64: dts: qcom: Add QMP handle for qcom_stats

[Dmitry Baryshkov]

On Wed, May 28, 2025 at 02:51:33PM +0530, Maulik Shah wrote:
> Add QMP handle which is used to send QMP command to always on processor
> to populate DDR stats. Add QMP handle for SM8450/SM8550/SM8650/SM8750.
> 
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/sm8450.dtsi | 1 +
>  arch/arm64/boot/dts/qcom/sm8550.dtsi | 1 +
>  arch/arm64/boot/dts/qcom/sm8650.dtsi | 1 +
>  arch/arm64/boot/dts/qcom/sm8750.dtsi | 1 +
>  4 files changed, 4 insertions(+)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

-- 
With best wishes
Dmitry

Re: [PATCH v4 2/3] soc: qcom: qcom_stats: Add QMP support for syncing ddr stats

[Konrad Dybcio]

On 5/28/25 1:02 PM, Dmitry Baryshkov wrote:
> On Wed, May 28, 2025 at 02:51:32PM +0530, Maulik Shah wrote:
>> Recent SoCs (SM8450 onwards) require QMP command to be sent before reading
>> ddr stats. The duration field of ddr stats will get populated only if QMP
>> command is sent.
>>
>> Add support to send ddr stats freqsync QMP command.
>>
>> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
>> ---
>>  drivers/soc/qcom/qcom_stats.c | 34 +++++++++++++++++++++++++++++++++-
>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
>> index 33fd2a1574464768bd07289e743fbb79ba415e84..0545c8cbefb8f18758d4eb51638e4ecb94e05422 100644
>> --- a/drivers/soc/qcom/qcom_stats.c
>> +++ b/drivers/soc/qcom/qcom_stats.c
>> @@ -13,6 +13,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/seq_file.h>
>>  
>> +#include <linux/soc/qcom/qcom_aoss.h>
>>  #include <linux/soc/qcom/smem.h>
>>  #include <clocksource/arm_arch_timer.h>
>>  
>> @@ -37,6 +38,8 @@
>>  #define DDR_STATS_TYPE(data)		FIELD_GET(GENMASK(15, 8), data)
>>  #define DDR_STATS_FREQ(data)		FIELD_GET(GENMASK(31, 16), data)
>>  
>> +static struct qmp *qcom_stats_qmp;
>> +
>>  struct subsystem_data {
>>  	const char *name;
>>  	u32 smem_item;
>> @@ -188,12 +191,28 @@ static int qcom_ddr_stats_show(struct seq_file *s, void *d)
>>  	struct ddr_stats_entry data[DDR_STATS_MAX_NUM_MODES];
>>  	void __iomem *reg = (void __iomem *)s->private;
>>  	u32 entry_count;
>> -	int i;
>> +	int i, ret;
>>  
>>  	entry_count = readl_relaxed(reg + DDR_STATS_NUM_MODES_ADDR);
>>  	if (entry_count > DDR_STATS_MAX_NUM_MODES)
>>  		return -EINVAL;
>>  
>> +	if (qcom_stats_qmp) {
>> +		/*
>> +		 * Recent SoCs (SM8450 onwards) do not have duration field
>> +		 * populated from boot up onwards for both DDR LPM Stats
>> +		 * and DDR Frequency Stats.
>> +		 *
>> +		 * Send QMP message to Always on processor which will
>> +		 * populate duration field into MSG RAM area.
>> +		 *
>> +		 * Sent every time to read latest data.
>> +		 */
>> +		ret = qmp_send(qcom_stats_qmp, "{class: ddr, action: freqsync}");
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>>  	reg += DDR_STATS_ENTRY_START_ADDR;
>>  	memcpy_fromio(data, reg, sizeof(struct ddr_stats_entry) * entry_count);
>>  
>> @@ -304,6 +323,19 @@ static int qcom_stats_probe(struct platform_device *pdev)
>>  
>>  	for (i = 0; i < config->num_records; i++)
>>  		d[i].appended_stats_avail = config->appended_stats_avail;
>> +	/*
>> +	 * QMP is used for DDR stats syncing to MSG RAM for recent SoCs (SM8450 onwards).
>> +	 * The prior SoCs do not need QMP handle as the required stats are already present
>> +	 * in MSG RAM, provided the DDR_STATS_MAGIC_KEY matches.
>> +	 */
>> +	qcom_stats_qmp = qmp_get(&pdev->dev);
>> +	if (IS_ERR(qcom_stats_qmp)) {
>> +		if (PTR_ERR(qcom_stats_qmp) == -EPROBE_DEFER)
>> +			return -EPROBE_DEFER;
>> +
>> +		/* We assume any other error means it's not defined/needed */
>> +		qcom_stats_qmp = NULL;
> 
> I still think that we shouldn't be ignoring actual errors here. I'd say,
> check for of_property_present(dev->of_node, "qcom,qmp") before.

/**
 * qmp_get() - get a qmp handle from a device
 * @dev: client device pointer
 *
 * Return: handle to qmp device on success, ERR_PTR() on failure
 */
struct qmp *qmp_get(struct device *dev)
{
        struct platform_device *pdev;
        struct device_node *np;
        struct qmp *qmp;

        if (!dev || !dev->of_node)
                return ERR_PTR(-EINVAL);

        np = of_parse_phandle(dev->of_node, "qcom,qmp", 0);
        if (!np)
                return ERR_PTR(-ENODEV);

        pdev = of_find_device_by_node(np);
        of_node_put(np);
        if (!pdev)
                return ERR_PTR(-EINVAL);

        qmp = platform_get_drvdata(pdev);

        if (!qmp) {
                put_device(&pdev->dev);
                return ERR_PTR(-EPROBE_DEFER);
        }
        return qmp;
}
EXPORT_SYMBOL_GPL(qmp_get);


Konrad

Re: [PATCH v4 1/3] soc: qcom: qcom_stats: Add support to read DDR statistic

[Konrad Dybcio]

On 5/28/25 11:21 AM, Maulik Shah wrote:
> DDR statistic provide different DDR LPM and DDR frequency statistic.
> Add support to read from MSGRAM and display via debugfs.
> 
> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> ---

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

Re: [PATCH v4 2/3] soc: qcom: qcom_stats: Add QMP support for syncing ddr stats

[Dmitry Baryshkov]

On Thu, May 29, 2025 at 09:07:49PM +0200, Konrad Dybcio wrote:
> On 5/28/25 1:02 PM, Dmitry Baryshkov wrote:
> > On Wed, May 28, 2025 at 02:51:32PM +0530, Maulik Shah wrote:
> >> Recent SoCs (SM8450 onwards) require QMP command to be sent before reading
> >> ddr stats. The duration field of ddr stats will get populated only if QMP
> >> command is sent.
> >>
> >> Add support to send ddr stats freqsync QMP command.
> >>
> >> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
> >> ---
> >>  drivers/soc/qcom/qcom_stats.c | 34 +++++++++++++++++++++++++++++++++-
> >>  1 file changed, 33 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
> >> index 33fd2a1574464768bd07289e743fbb79ba415e84..0545c8cbefb8f18758d4eb51638e4ecb94e05422 100644
> >> --- a/drivers/soc/qcom/qcom_stats.c
> >> +++ b/drivers/soc/qcom/qcom_stats.c
> >> @@ -13,6 +13,7 @@
> >>  #include <linux/platform_device.h>
> >>  #include <linux/seq_file.h>
> >>  
> >> +#include <linux/soc/qcom/qcom_aoss.h>
> >>  #include <linux/soc/qcom/smem.h>
> >>  #include <clocksource/arm_arch_timer.h>
> >>  
> >> @@ -37,6 +38,8 @@
> >>  #define DDR_STATS_TYPE(data)		FIELD_GET(GENMASK(15, 8), data)
> >>  #define DDR_STATS_FREQ(data)		FIELD_GET(GENMASK(31, 16), data)
> >>  
> >> +static struct qmp *qcom_stats_qmp;
> >> +
> >>  struct subsystem_data {
> >>  	const char *name;
> >>  	u32 smem_item;
> >> @@ -188,12 +191,28 @@ static int qcom_ddr_stats_show(struct seq_file *s, void *d)
> >>  	struct ddr_stats_entry data[DDR_STATS_MAX_NUM_MODES];
> >>  	void __iomem *reg = (void __iomem *)s->private;
> >>  	u32 entry_count;
> >> -	int i;
> >> +	int i, ret;
> >>  
> >>  	entry_count = readl_relaxed(reg + DDR_STATS_NUM_MODES_ADDR);
> >>  	if (entry_count > DDR_STATS_MAX_NUM_MODES)
> >>  		return -EINVAL;
> >>  
> >> +	if (qcom_stats_qmp) {
> >> +		/*
> >> +		 * Recent SoCs (SM8450 onwards) do not have duration field
> >> +		 * populated from boot up onwards for both DDR LPM Stats
> >> +		 * and DDR Frequency Stats.
> >> +		 *
> >> +		 * Send QMP message to Always on processor which will
> >> +		 * populate duration field into MSG RAM area.
> >> +		 *
> >> +		 * Sent every time to read latest data.
> >> +		 */
> >> +		ret = qmp_send(qcom_stats_qmp, "{class: ddr, action: freqsync}");
> >> +		if (ret)
> >> +			return ret;
> >> +	}
> >> +
> >>  	reg += DDR_STATS_ENTRY_START_ADDR;
> >>  	memcpy_fromio(data, reg, sizeof(struct ddr_stats_entry) * entry_count);
> >>  
> >> @@ -304,6 +323,19 @@ static int qcom_stats_probe(struct platform_device *pdev)
> >>  
> >>  	for (i = 0; i < config->num_records; i++)
> >>  		d[i].appended_stats_avail = config->appended_stats_avail;
> >> +	/*
> >> +	 * QMP is used for DDR stats syncing to MSG RAM for recent SoCs (SM8450 onwards).
> >> +	 * The prior SoCs do not need QMP handle as the required stats are already present
> >> +	 * in MSG RAM, provided the DDR_STATS_MAGIC_KEY matches.
> >> +	 */
> >> +	qcom_stats_qmp = qmp_get(&pdev->dev);
> >> +	if (IS_ERR(qcom_stats_qmp)) {
> >> +		if (PTR_ERR(qcom_stats_qmp) == -EPROBE_DEFER)
> >> +			return -EPROBE_DEFER;
> >> +
> >> +		/* We assume any other error means it's not defined/needed */
> >> +		qcom_stats_qmp = NULL;
> > 
> > I still think that we shouldn't be ignoring actual errors here. I'd say,
> > check for of_property_present(dev->of_node, "qcom,qmp") before.
> 
> /**
>  * qmp_get() - get a qmp handle from a device
>  * @dev: client device pointer
>  *
>  * Return: handle to qmp device on success, ERR_PTR() on failure
>  */
> struct qmp *qmp_get(struct device *dev)
> {
>         struct platform_device *pdev;
>         struct device_node *np;
>         struct qmp *qmp;
> 
>         if (!dev || !dev->of_node)
>                 return ERR_PTR(-EINVAL);
> 
>         np = of_parse_phandle(dev->of_node, "qcom,qmp", 0);
>         if (!np)
>                 return ERR_PTR(-ENODEV);

So, I'd say, we need to identify whether it is this caluse or another
error. It should be enough to ignore ENODEV and return an error in all
other cases.

> 
>         pdev = of_find_device_by_node(np);
>         of_node_put(np);
>         if (!pdev)
>                 return ERR_PTR(-EINVAL);
> 
>         qmp = platform_get_drvdata(pdev);
> 
>         if (!qmp) {
>                 put_device(&pdev->dev);
>                 return ERR_PTR(-EPROBE_DEFER);
>         }
>         return qmp;
> }
> EXPORT_SYMBOL_GPL(qmp_get);
> 
> 
> Konrad

-- 
With best wishes
Dmitry

Re: [PATCH v4 2/3] soc: qcom: qcom_stats: Add QMP support for syncing ddr stats

[Maulik Shah (mkshah)]

On 5/31/2025 2:19 AM, Dmitry Baryshkov wrote:
> On Thu, May 29, 2025 at 09:07:49PM +0200, Konrad Dybcio wrote:
>> On 5/28/25 1:02 PM, Dmitry Baryshkov wrote:
>>> On Wed, May 28, 2025 at 02:51:32PM +0530, Maulik Shah wrote:
>>>> Recent SoCs (SM8450 onwards) require QMP command to be sent before reading
>>>> ddr stats. The duration field of ddr stats will get populated only if QMP
>>>> command is sent.
>>>>
>>>> Add support to send ddr stats freqsync QMP command.
>>>>
>>>> Signed-off-by: Maulik Shah <maulik.shah@oss.qualcomm.com>
>>>> ---
>>>>  drivers/soc/qcom/qcom_stats.c | 34 +++++++++++++++++++++++++++++++++-
>>>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/soc/qcom/qcom_stats.c b/drivers/soc/qcom/qcom_stats.c
>>>> index 33fd2a1574464768bd07289e743fbb79ba415e84..0545c8cbefb8f18758d4eb51638e4ecb94e05422 100644
>>>> --- a/drivers/soc/qcom/qcom_stats.c
>>>> +++ b/drivers/soc/qcom/qcom_stats.c
>>>> @@ -13,6 +13,7 @@
>>>>  #include <linux/platform_device.h>
>>>>  #include <linux/seq_file.h>
>>>>  
>>>> +#include <linux/soc/qcom/qcom_aoss.h>
>>>>  #include <linux/soc/qcom/smem.h>
>>>>  #include <clocksource/arm_arch_timer.h>
>>>>  
>>>> @@ -37,6 +38,8 @@
>>>>  #define DDR_STATS_TYPE(data)		FIELD_GET(GENMASK(15, 8), data)
>>>>  #define DDR_STATS_FREQ(data)		FIELD_GET(GENMASK(31, 16), data)
>>>>  
>>>> +static struct qmp *qcom_stats_qmp;
>>>> +
>>>>  struct subsystem_data {
>>>>  	const char *name;
>>>>  	u32 smem_item;
>>>> @@ -188,12 +191,28 @@ static int qcom_ddr_stats_show(struct seq_file *s, void *d)
>>>>  	struct ddr_stats_entry data[DDR_STATS_MAX_NUM_MODES];
>>>>  	void __iomem *reg = (void __iomem *)s->private;
>>>>  	u32 entry_count;
>>>> -	int i;
>>>> +	int i, ret;
>>>>  
>>>>  	entry_count = readl_relaxed(reg + DDR_STATS_NUM_MODES_ADDR);
>>>>  	if (entry_count > DDR_STATS_MAX_NUM_MODES)
>>>>  		return -EINVAL;
>>>>  
>>>> +	if (qcom_stats_qmp) {
>>>> +		/*
>>>> +		 * Recent SoCs (SM8450 onwards) do not have duration field
>>>> +		 * populated from boot up onwards for both DDR LPM Stats
>>>> +		 * and DDR Frequency Stats.
>>>> +		 *
>>>> +		 * Send QMP message to Always on processor which will
>>>> +		 * populate duration field into MSG RAM area.
>>>> +		 *
>>>> +		 * Sent every time to read latest data.
>>>> +		 */
>>>> +		ret = qmp_send(qcom_stats_qmp, "{class: ddr, action: freqsync}");
>>>> +		if (ret)
>>>> +			return ret;
>>>> +	}
>>>> +
>>>>  	reg += DDR_STATS_ENTRY_START_ADDR;
>>>>  	memcpy_fromio(data, reg, sizeof(struct ddr_stats_entry) * entry_count);
>>>>  
>>>> @@ -304,6 +323,19 @@ static int qcom_stats_probe(struct platform_device *pdev)
>>>>  
>>>>  	for (i = 0; i < config->num_records; i++)
>>>>  		d[i].appended_stats_avail = config->appended_stats_avail;
>>>> +	/*
>>>> +	 * QMP is used for DDR stats syncing to MSG RAM for recent SoCs (SM8450 onwards).
>>>> +	 * The prior SoCs do not need QMP handle as the required stats are already present
>>>> +	 * in MSG RAM, provided the DDR_STATS_MAGIC_KEY matches.
>>>> +	 */
>>>> +	qcom_stats_qmp = qmp_get(&pdev->dev);
>>>> +	if (IS_ERR(qcom_stats_qmp)) {
>>>> +		if (PTR_ERR(qcom_stats_qmp) == -EPROBE_DEFER)
>>>> +			return -EPROBE_DEFER;
>>>> +
>>>> +		/* We assume any other error means it's not defined/needed */
>>>> +		qcom_stats_qmp = NULL;
>>>
>>> I still think that we shouldn't be ignoring actual errors here. I'd say,
>>> check for of_property_present(dev->of_node, "qcom,qmp") before.
>>
>> /**
>>  * qmp_get() - get a qmp handle from a device
>>  * @dev: client device pointer
>>  *
>>  * Return: handle to qmp device on success, ERR_PTR() on failure
>>  */
>> struct qmp *qmp_get(struct device *dev)
>> {
>>         struct platform_device *pdev;
>>         struct device_node *np;
>>         struct qmp *qmp;
>>
>>         if (!dev || !dev->of_node)
>>                 return ERR_PTR(-EINVAL);
>>
>>         np = of_parse_phandle(dev->of_node, "qcom,qmp", 0);
>>         if (!np)
>>                 return ERR_PTR(-ENODEV);
> 
> So, I'd say, we need to identify whether it is this caluse or another
> error. It should be enough to ignore ENODEV and return an error in all
> other cases.
> 

I will update in v5 to check of_property_present(dev->of_node, "qcom,qmp") and then
return error accordingly.

Thanks,
Maulik

>>
>>         pdev = of_find_device_by_node(np);
>>         of_node_put(np);
>>         if (!pdev)
>>                 return ERR_PTR(-EINVAL);
>>
>>         qmp = platform_get_drvdata(pdev);
>>
>>         if (!qmp) {
>>                 put_device(&pdev->dev);
>>                 return ERR_PTR(-EPROBE_DEFER);
>>         }
>>         return qmp;
>> }
>> EXPORT_SYMBOL_GPL(qmp_get);
>>
>>
>> Konrad
>