Re: [RFC V3 3/4] soc: qcom: Introduce SCMI based Memlat (Memory Latency) governor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 7/3/24 14:14, Shivnandan Kumar wrote:


On 7/3/2024 12:44 AM, Sibi Sankar wrote:
Introduce a client driver that uses the memlat algorithm string hosted
on ARM SCMI QCOM Vendor Protocol to detect memory latency workloads and
control frequency/level of the various memory buses (DDR/LLCC/DDR_QOS).


Hey Shiv,

Thanks for taking time to review the series :)

Co-developed-by: Shivnandan Kumar <quic_kshivnan@xxxxxxxxxxx>
Signed-off-by: Shivnandan Kumar <quic_kshivnan@xxxxxxxxxxx>
Co-developed-by: Ramakrishna Gottimukkula <quic_rgottimu@xxxxxxxxxxx>
Signed-off-by: Ramakrishna Gottimukkula <quic_rgottimu@xxxxxxxxxxx>
Co-developed-by: Amir Vajid <avajid@xxxxxxxxxxx>
Signed-off-by: Amir Vajid <avajid@xxxxxxxxxxx>
Signed-off-by: Sibi Sankar <quic_sibis@xxxxxxxxxxx>
---

V2:
* Make driver changes to the accommodate bindings changes. [Rob]
* Replace explicit of_node_put with _free.

  drivers/soc/qcom/Kconfig            |  12 +
  drivers/soc/qcom/Makefile           |   1 +
  drivers/soc/qcom/qcom_scmi_client.c | 583 ++++++++++++++++++++++++++++
  3 files changed, 596 insertions(+)
  create mode 100644 drivers/soc/qcom/qcom_scmi_client.c

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 432c85bd8ad4..b253504bd386 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -294,4 +294,16 @@ config QCOM_PBS
        This module provides the APIs to the client drivers that wants to send the
        PBS trigger event to the PBS RAM.
+config QCOM_SCMI_CLIENT
+    tristate "Qualcomm Technologies Inc. SCMI client driver"
+    depends on ARM_SCMI_PROTOCOL_VENDOR_QCOM || COMPILE_TEST
+    default n
+    help
+      This driver uses the memlat algorithm string hosted on QCOM SCMI
+      Vendor Protocol to detect memory latency workloads and control
+      frequency/level of the various memory buses (DDR/LLCC/DDR_QOS).
+
+      This driver defines/documents the parameter IDs used while configuring
+      the memory buses.
+
  endmenu
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index d3560f861085..8a2e832d1d5d 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_QCOM_APR) += apr.o
  obj-$(CONFIG_QCOM_LLCC) += llcc-qcom.o
  obj-$(CONFIG_QCOM_KRYO_L2_ACCESSORS) +=    kryo-l2-accessors.o
  obj-$(CONFIG_QCOM_ICC_BWMON)    += icc-bwmon.o
+obj-$(CONFIG_QCOM_SCMI_CLIENT)    += qcom_scmi_client.o
  qcom_ice-objs            += ice.o
  obj-$(CONFIG_QCOM_INLINE_CRYPTO_ENGINE)    += qcom_ice.o
  obj-$(CONFIG_QCOM_PBS) +=    qcom-pbs.o
diff --git a/drivers/soc/qcom/qcom_scmi_client.c b/drivers/soc/qcom/qcom_scmi_client.c
new file mode 100644
index 000000000000..8369b415c0ab
--- /dev/null
+++ b/drivers/soc/qcom/qcom_scmi_client.c
@@ -0,0 +1,583 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include <linux/cpu.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/qcom_scmi_vendor.h>
+#include <linux/scmi_protocol.h>
+#include <linux/units.h>
+#include <dt-bindings/soc/qcom,scmi-vendor.h>
+
+#define MEMLAT_ALGO_STR                0x4D454D4C4154 /* MEMLAT */
+#define INVALID_IDX                0xff
+#define MAX_MEMORY_TYPES            3
+#define MAX_MONITOR_CNT                4
+#define MAX_NAME_LEN                20
+#define MAX_MAP_ENTRIES                7
+#define CPUCP_DEFAULT_SAMPLING_PERIOD_MS    4
+#define CPUCP_DEFAULT_FREQ_METHOD        1
+
+/**
+ * scmi_memlat_protocol_cmd - parameter_ids supported by the "MEMLAT" algo_str hosted + *                            by the Qualcomm SCMI Vendor Protocol on the SCMI controller.
+ *
+ * MEMLAT (Memory Latency) monitors the counters to detect memory latency bound workloads
+ * and scales the frequency/levels of the memory buses accordingly.
+ *
+ * @MEMLAT_SET_MEM_GROUP: initializes the frequency/level scaling functions for the memory bus. + * @MEMLAT_SET_MONITOR: configures the monitor to work on a specific memory bus. + * @MEMLAT_SET_COMMON_EV_MAP: set up common counters used to monitor the cpu frequency. + * @MEMLAT_SET_GRP_EV_MAP: set up any specific counters used to monitor the memory bus. + * @MEMLAT_IPM_CEIL: set the IPM (Instruction Per Misses) ceiling per monitor.
+ * @MEMLAT_SAMPLE_MS: set the sampling period for all the monitors.
+ * MEMLAT_MON_FREQ_MAP: setup the cpufreq to memfreq map.
+ * MEMLAT_SET_MIN_FREQ: set the max frequency of the memory bus.
+ * MEMLAT_SET_MAX_FREQ: set the min frequency of the memory bus.
+ * MEMLAT_START_TIMER: start all the monitors with the requested sampling period.
+ * MEMLAT_START_TIMER: stop all the running monitors.

Typo above, it should be MEMLAT_STOP_TIMER

ack, thanks for catching this.


+ * MEMLAT_SET_EFFECTIVE_FREQ_METHOD: set the method used to determine cpu frequency.
+ */
+enum scmi_memlat_protocol_cmd {
+    MEMLAT_SET_MEM_GROUP = 16,
+    MEMLAT_SET_MONITOR,
+    MEMLAT_SET_COMMON_EV_MAP,
+    MEMLAT_SET_GRP_EV_MAP,
+    MEMLAT_IPM_CEIL = 23,
+    MEMLAT_SAMPLE_MS = 31,
+    MEMLAT_MON_FREQ_MAP,
+    MEMLAT_SET_MIN_FREQ,
+    MEMLAT_SET_MAX_FREQ,
+    MEMLAT_START_TIMER = 36,
+    MEMLAT_STOP_TIMER,
+    MEMLAT_SET_EFFECTIVE_FREQ_METHOD = 39,
+};
+
+struct map_table {

[...]

+
+static int populate_physical_mask(struct device_node *np, u32 *mask, u32 *cluster_info)
+{
+    struct device_node *dev_phandle;
+    int cpu, i = 0, physical_id;
+
+    do {
+        dev_phandle = of_parse_phandle(np, "cpus", i++);
+        cpu = of_cpu_node_to_id(dev_phandle);
+        if (cpu != -ENODEV) {
+            physical_id = topology_core_id(cpu);
+            for (int j = 0; j < topology_cluster_id(cpu); j++)
+                physical_id += *(cluster_info + j);
+            *mask |= BIT(physical_id);
+        }
+    } while (dev_phandle);
+
+    return 0;
+}
+
+static struct cpufreq_memfreq_map *init_cpufreq_memfreq_map(struct device *dev,
+                                struct scmi_memory_info *memory,
+                                struct device_node *of_node,
+                                u32 *cnt)
+{
+    struct device_node *tbl_np, *opp_np;
+    struct cpufreq_memfreq_map *tbl;
+    int ret, i = 0;
+    u32 level, len;
+    u64 rate;
+
+    tbl_np = of_parse_phandle(of_node, "operating-points-v2", 0);
+    if (!tbl_np)
+        return ERR_PTR(-ENODEV);
+
+    len = min(of_get_available_child_count(tbl_np), MAX_MAP_ENTRIES);
+    if (len == 0)
+        return ERR_PTR(-ENODEV);
+
+    tbl = devm_kzalloc(dev, (len + 1) * sizeof(struct cpufreq_memfreq_map),
+               GFP_KERNEL);
+    if (!tbl)
+        return ERR_PTR(-ENOMEM);
+
+    for_each_available_child_of_node(tbl_np, opp_np) {
+        ret = of_property_read_u64_index(opp_np, "opp-hz", 0, &rate);
+        if (ret < 0)
+            return ERR_PTR(ret);
+
+        tbl[i].cpufreq_mhz = rate / HZ_PER_MHZ;
+
+        if (memory->hw_type != QCOM_MEM_TYPE_DDR_QOS) {
+            ret = of_property_read_u64_index(opp_np, "opp-hz", 1, &rate);
+            if (ret < 0)
+                return ERR_PTR(ret);
+
+            tbl[i].memfreq_khz = rate / HZ_PER_KHZ;
+        } else {
+            ret = of_property_read_u32(opp_np, "opp-level", &level);
+            if (ret < 0)
+                return ERR_PTR(ret);
+
+            tbl[i].memfreq_khz = level;
+        }
+
+        dev_dbg(dev, "Entry%d CPU:%u, Mem:%u\n", i, tbl[i].cpufreq_mhz, tbl[i].memfreq_khz);
+        i++;
+    }
+    *cnt = len;
+    tbl[i].cpufreq_mhz = 0;

This is not needed as for allocating memory, devm_kzalloc is used.

ack


+
+    return tbl;
+}
+
+static int process_scmi_memlat_of_node(struct scmi_device *sdev, struct scmi_memlat_info *info)
+{
+    struct scmi_monitor_info *monitor;
+    struct scmi_memory_info *memory;
+    char name[MAX_NAME_LEN];
+    u64 memfreq[2];
+    int ret;
+
+    ret = populate_cluster_info(info->cluster_info);
+    if (ret < 0) {
+        dev_err_probe(&sdev->dev, ret, "failed to populate cluster info\n");
+        goto err;
+    }
+
+    of_node_get(sdev->handle->dev->of_node);
+    do {
+        snprintf(name, sizeof(name), "memory-%d", info->memory_cnt);
+        struct device_node *memory_np __free(device_node) =
+            of_find_node_by_name(sdev->handle->dev->of_node, name);
+
+        if (!memory_np)
+            break;
+
+        memory = devm_kzalloc(&sdev->dev, sizeof(*memory), GFP_KERNEL);
+        if (!memory) {
+            ret = -ENOMEM;
+            goto err;
+        }
+
+        ret = of_property_read_u32(memory_np, "qcom,memory-type", &memory->hw_type);
+        if (ret) {
+            dev_err_probe(&sdev->dev, ret, "failed to read memory type\n");
+            goto err;
+        }
+
+        ret = of_property_read_u64_array(memory_np, "freq-table-hz", memfreq, 2);
+        if (ret && (ret != -EINVAL)) {
+            dev_err_probe(&sdev->dev, ret, "failed to read min/max freq\n");
+            goto err;
+        }
+
+        if (memory->hw_type != QCOM_MEM_TYPE_DDR_QOS) {
+            memory->min_freq = memfreq[0] / HZ_PER_KHZ;
+            memory->max_freq = memfreq[1] / HZ_PER_KHZ;
+        } else {
+            memory->min_freq = memfreq[0];
+            memory->max_freq = memfreq[1];
+        }
+        info->memory[info->memory_cnt++] = memory;
+
+        do {
+            snprintf(name, sizeof(name), "monitor-%d", memory->monitor_cnt);
+            struct device_node *monitor_np __free(device_node) =
+                of_get_child_by_name(memory_np, name);
+
+            if (!monitor_np)
+                break;
+
+            monitor = devm_kzalloc(&sdev->dev, sizeof(*monitor), GFP_KERNEL);
+            if (!monitor) {
+                ret = -ENOMEM;
+                goto err;
+            }
+
+            monitor->mon_type = of_property_read_bool(monitor_np, "qcom,compute-type");
+            if (!monitor->mon_type) {
+                ret = of_property_read_u32(monitor_np, "qcom,ipm-ceil",
+                               &monitor->ipm_ceil);
+                if (ret) {
+                    dev_err_probe(&sdev->dev, ret,
+                              "failed to read IPM ceiling\n");
+                    goto err;
+                }
+            }
+
+            /*
+             * Variants of the SoC having reduced number of cpus operate
+             * with the same number of logical cpus but the physical
+             * cpu disabled will differ between parts. Calculate the
+             * physical cpu number using cluster information instead.
+             */
+            ret = populate_physical_mask(monitor_np, &monitor->mask,
+                             info->cluster_info);
+            if (ret < 0) {
+                dev_err_probe(&sdev->dev, ret, "failed to populate cpu mask\n");
+                goto err;
+            }
+

Above error check is not needed as populate_physical_mask always return 0.

ack, will fix this in the next re-spin.



+            monitor->freq_map = init_cpufreq_memfreq_map(&sdev->dev, memory, monitor_np,
+                                     &monitor->freq_map_len);
+            if (IS_ERR(monitor->freq_map)) {
+                dev_err_probe(&sdev->dev, PTR_ERR(monitor->freq_map),
+                          "failed to populate cpufreq-memfreq map\n");
+                goto err;
+            }
+
+            strscpy(monitor->mon_name, name, sizeof(monitor->mon_name));
+            monitor->mon_idx = memory->monitor_cnt;
+
+            memory->monitor[memory->monitor_cnt++] = monitor;

If from dt, more than 4 monitor are passed then this may lead to overflow, adding a check and giving warning in case number of monitor exceeds MAX_MONITOR_CNT will be better idea.

I'll make sure to account for ^^ by either switching to dynamic
allocation or by using MAX_MONITOR_CNT.



+        } while (1);
+

[...]

+MODULE_DESCRIPTION("QTI SCMI client driver");
+MODULE_LICENSE("GPL");




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux