This series adds QoS support for QNOC type device which can be found on
SC7280 platform. It adds support for programming priority,
priority forward disable and urgency forwarding. This helps in
priortizing the traffic originating from different interconnect masters
at NOC(Network On Chip).
Changes in v2:
- Updated regmap_update to make use GENMASK and FIELD_PREP.
- Removed the regmap structure from qcom_icc_node.
- Made qcom_icc_rpmh_configure_qos() static
- Removed qcom_icc_rpmh_map() API, inlined the code in probe
function.
- Updated declarations to reverse christmas tree fashion.
Odelu Kukatla (4):
interconnect: qcom: icc-rpmh: Add QoS configuration support
interconnect: qcom: sc7280: enable QoS configuration
dt-bindings: interconnect: add clock property to configure QoS on
SC7280
arm64: dts: qcom: sc7280: Add clocks for QoS configuration
.../interconnect/qcom,sc7280-rpmh.yaml | 49 +++
arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +
drivers/interconnect/qcom/icc-rpmh.c | 161 ++++++++-
drivers/interconnect/qcom/icc-rpmh.h | 32 ++
drivers/interconnect/qcom/sc7280.c | 332 ++++++++++++++++++
5 files changed, 573 insertions(+), 4 deletions(-)
--
2.17.1
It adds QoS support for QNOC device and includes support for
configuring priority, priority forward disable, urgency forwarding.
This helps in priortizing the traffic originating from different
interconnect masters at NoC(Network On Chip).
Signed-off-by: Odelu Kukatla <[email protected]>
---
drivers/interconnect/qcom/icc-rpmh.c | 161 ++++++++++++++++++++++++++-
drivers/interconnect/qcom/icc-rpmh.h | 32 ++++++
2 files changed, 189 insertions(+), 4 deletions(-)
diff --git a/drivers/interconnect/qcom/icc-rpmh.c b/drivers/interconnect/qcom/icc-rpmh.c
index c1aa265c1f4e..67e600f1191d 100644
--- a/drivers/interconnect/qcom/icc-rpmh.c
+++ b/drivers/interconnect/qcom/icc-rpmh.c
@@ -1,28 +1,69 @@
// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (c) 2020, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
*/
+#include <linux/clk.h>
#include <linux/interconnect.h>
#include <linux/interconnect-provider.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/of_platform.h>
#include <linux/slab.h>
+#include <linux/bits.h>
#include "bcm-voter.h"
#include "icc-common.h"
#include "icc-rpmh.h"
+/* QNOC QoS */
+#define QOSGEN_MAINCTL_LO(p, qp) (0x8 + (p->port_offsets[qp]))
+#define QOS_SLV_URG_MSG_EN_MASK BIT_MASK(3)
+#define QOS_DFLT_PRIO_MASK GENMASK(6, 4)
+#define QOS_DISABLE_MASK BIT_MASK(24)
+
+/**
+ * qcom_icc_set_qos - initialize static QoS configurations
+ * @qp: qcom icc provider to which @node belongs
+ * @node: qcom icc node to operate on
+ */
+static void qcom_icc_set_qos(struct qcom_icc_provider *qp,
+ struct qcom_icc_node *node)
+{
+ const struct qcom_icc_qosbox *qos = node->qosbox;
+ int port;
+
+ if (!qp->regmap)
+ return;
+
+ if (!qos)
+ return;
+
+ for (port = 0; port < qos->num_ports; port++) {
+ regmap_update_bits(qp->regmap, QOSGEN_MAINCTL_LO(qos, port),
+ QOS_DISABLE_MASK,
+ FIELD_PREP(QOS_DISABLE_MASK, qos->prio_fwd_disable));
+
+ regmap_update_bits(qp->regmap, QOSGEN_MAINCTL_LO(qos, port),
+ QOS_DFLT_PRIO_MASK,
+ FIELD_PREP(QOS_DFLT_PRIO_MASK, qos->prio));
+
+ regmap_update_bits(qp->regmap, QOSGEN_MAINCTL_LO(qos, port),
+ QOS_SLV_URG_MSG_EN_MASK,
+ FIELD_PREP(QOS_SLV_URG_MSG_EN_MASK, qos->urg_fwd));
+ }
+}
+
/**
* qcom_icc_pre_aggregate - cleans up stale values from prior icc_set
* @node: icc node to operate on
*/
void qcom_icc_pre_aggregate(struct icc_node *node)
{
- size_t i;
- struct qcom_icc_node *qn;
struct qcom_icc_provider *qp;
+ struct qcom_icc_node *qn;
+ size_t i;
qn = node->data;
qp = to_qcom_provider(node->provider);
@@ -49,8 +90,8 @@ EXPORT_SYMBOL_GPL(qcom_icc_pre_aggregate);
int qcom_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
{
- size_t i;
struct qcom_icc_node *qn;
+ size_t i;
qn = node->data;
@@ -159,13 +200,96 @@ int qcom_icc_bcm_init(struct qcom_icc_bcm *bcm, struct device *dev)
}
EXPORT_SYMBOL_GPL(qcom_icc_bcm_init);
+static bool bcm_needs_qos_proxy(struct qcom_icc_bcm *bcm)
+{
+ int i;
+
+ for (i = 0; i < bcm->num_nodes; i++)
+ if (bcm->nodes[i]->qosbox)
+ return true;
+
+ return false;
+}
+
+static int enable_qos_deps(struct qcom_icc_provider *qp)
+{
+ struct qcom_icc_bcm *bcm;
+ bool keepalive;
+ int ret, i;
+
+ for (i = 0; i < qp->num_bcms; i++) {
+ bcm = qp->bcms[i];
+ if (bcm_needs_qos_proxy(bcm)) {
+ keepalive = bcm->keepalive;
+ bcm->keepalive = true;
+
+ qcom_icc_bcm_voter_add(qp->voter, bcm);
+ ret = qcom_icc_bcm_voter_commit(qp->voter);
+
+ bcm->keepalive = keepalive;
+
+ if (ret) {
+ dev_err(qp->dev, "failed to vote BW to %s for QoS\n",
+ bcm->name);
+ return ret;
+ }
+ }
+ }
+
+ ret = clk_bulk_prepare_enable(qp->num_clks, qp->clks);
+ if (ret)
+ dev_err(qp->dev, "failed to enable clocks for QoS\n");
+
+ return ret;
+}
+
+static void disable_qos_deps(struct qcom_icc_provider *qp)
+{
+ struct qcom_icc_bcm *bcm;
+ int i;
+
+ clk_bulk_disable_unprepare(qp->num_clks, qp->clks);
+
+ for (i = 0; i < qp->num_bcms; i++) {
+ bcm = qp->bcms[i];
+ if (bcm_needs_qos_proxy(bcm)) {
+ qcom_icc_bcm_voter_add(qp->voter, bcm);
+ qcom_icc_bcm_voter_commit(qp->voter);
+ }
+ }
+}
+
+static int qcom_icc_rpmh_configure_qos(struct qcom_icc_provider *qp)
+{
+ struct qcom_icc_node *qnode;
+ size_t i;
+ int ret;
+
+ ret = enable_qos_deps(qp);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < qp->num_nodes; i++) {
+ qnode = qp->nodes[i];
+ if (!qnode)
+ continue;
+
+ if (qnode->qosbox)
+ qcom_icc_set_qos(qp, qnode);
+ }
+
+ disable_qos_deps(qp);
+
+ return ret;
+}
+
int qcom_icc_rpmh_probe(struct platform_device *pdev)
{
+ struct qcom_icc_node * const *qnodes, *qn;
const struct qcom_icc_desc *desc;
struct device *dev = &pdev->dev;
struct icc_onecell_data *data;
struct icc_provider *provider;
- struct qcom_icc_node * const *qnodes, *qn;
struct qcom_icc_provider *qp;
struct icc_node *node;
size_t num_nodes, i, j;
@@ -199,12 +323,35 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev)
qp->dev = dev;
qp->bcms = desc->bcms;
+ qp->nodes = desc->nodes;
qp->num_bcms = desc->num_bcms;
+ qp->num_nodes = desc->num_nodes;
qp->voter = of_bcm_voter_get(qp->dev, NULL);
if (IS_ERR(qp->voter))
return PTR_ERR(qp->voter);
+ if (desc->config) {
+ struct resource *res;
+ void __iomem *base;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
+
+ base = devm_ioremap(dev, res->start, resource_size(res));
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ qp->regmap = devm_regmap_init_mmio(dev, base, desc->config);
+ if (IS_ERR(qp->regmap))
+ return PTR_ERR(qp->regmap);
+ }
+
+ qp->num_clks = devm_clk_bulk_get_all(qp->dev, &qp->clks);
+ if (qp->num_clks < 0)
+ return qp->num_clks;
+
for (i = 0; i < qp->num_bcms; i++)
qcom_icc_bcm_init(qp->bcms[i], dev);
@@ -229,6 +376,10 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev)
data->nodes[i] = node;
}
+ ret = qcom_icc_rpmh_configure_qos(qp);
+ if (ret)
+ goto err_remove_nodes;
+
ret = icc_provider_register(provider);
if (ret)
goto err_remove_nodes;
@@ -247,6 +398,7 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev)
err_deregister_provider:
icc_provider_deregister(provider);
err_remove_nodes:
+ clk_bulk_put_all(qp->num_clks, qp->clks);
icc_nodes_remove(provider);
return ret;
@@ -258,6 +410,7 @@ void qcom_icc_rpmh_remove(struct platform_device *pdev)
struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
icc_provider_deregister(&qp->provider);
+ clk_bulk_put_all(qp->num_clks, qp->clks);
icc_nodes_remove(&qp->provider);
}
EXPORT_SYMBOL_GPL(qcom_icc_rpmh_remove);
diff --git a/drivers/interconnect/qcom/icc-rpmh.h b/drivers/interconnect/qcom/icc-rpmh.h
index 2de29460e808..65cef156f212 100644
--- a/drivers/interconnect/qcom/icc-rpmh.h
+++ b/drivers/interconnect/qcom/icc-rpmh.h
@@ -1,12 +1,14 @@
/* SPDX-License-Identifier: GPL-2.0 */
/*
* Copyright (c) 2020, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
*/
#ifndef __DRIVERS_INTERCONNECT_QCOM_ICC_RPMH_H__
#define __DRIVERS_INTERCONNECT_QCOM_ICC_RPMH_H__
#include <dt-bindings/interconnect/qcom,icc.h>
+#include <linux/regmap.h>
#define to_qcom_provider(_provider) \
container_of(_provider, struct qcom_icc_provider, provider)
@@ -18,6 +20,11 @@
* @bcms: list of bcms that maps to the provider
* @num_bcms: number of @bcms
* @voter: bcm voter targeted by this provider
+ * @nodes: list of icc nodes that maps to the provider
+ * @num_nodes: number of @nodes
+ * @regmap: used for QoS, register access
+ * @clks : clks required for register access
+ * @num_clks: number of @clks
*/
struct qcom_icc_provider {
struct icc_provider provider;
@@ -25,6 +32,11 @@ struct qcom_icc_provider {
struct qcom_icc_bcm * const *bcms;
size_t num_bcms;
struct bcm_voter *voter;
+ struct qcom_icc_node * const *nodes;
+ size_t num_nodes;
+ struct regmap *regmap;
+ struct clk_bulk_data *clks;
+ int num_clks;
};
/**
@@ -41,6 +53,23 @@ struct bcm_db {
u8 reserved;
};
+/**
+ * struct qcom_icc_qosbox - Qualcomm specific QoS config
+ * @prio: priority value assigned to requests on the node
+ * @urg_fwd: if set, master priority is used for requests.
+ * @prio_fwd_disable: if set, master priority is ignored and NoC's default priority is used.
+ * @num_ports: number of @ports
+ * @port_offsets: qos register offsets
+ */
+
+struct qcom_icc_qosbox {
+ const u32 prio;
+ const bool urg_fwd;
+ const bool prio_fwd_disable;
+ const u32 num_ports;
+ const u32 port_offsets[] __counted_by(num_ports);
+};
+
#define MAX_LINKS 128
#define MAX_BCMS 64
#define MAX_BCM_PER_NODE 3
@@ -58,6 +87,7 @@ struct bcm_db {
* @max_peak: current max aggregate value of all peak bw requests
* @bcms: list of bcms associated with this logical node
* @num_bcms: num of @bcms
+ * @qosbox: qos config data associated with node
*/
struct qcom_icc_node {
const char *name;
@@ -70,6 +100,7 @@ struct qcom_icc_node {
u64 max_peak[QCOM_ICC_NUM_BUCKETS];
struct qcom_icc_bcm *bcms[MAX_BCM_PER_NODE];
size_t num_bcms;
+ const struct qcom_icc_qosbox *qosbox;
};
/**
@@ -114,6 +145,7 @@ struct qcom_icc_fabric {
};
struct qcom_icc_desc {
+ const struct regmap_config *config;
struct qcom_icc_node * const *nodes;
size_t num_nodes;
struct qcom_icc_bcm * const *bcms;
--
2.17.1
Added clock property to enable clocks required for accessing
qos registers.
Signed-off-by: Odelu Kukatla <[email protected]>
---
.../interconnect/qcom,sc7280-rpmh.yaml | 49 +++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml b/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml
index b135597d9489..758a6e924037 100644
--- a/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml
+++ b/Documentation/devicetree/bindings/interconnect/qcom,sc7280-rpmh.yaml
@@ -53,10 +53,50 @@ allOf:
required:
- reg
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,sc7280-aggre1-noc
+ then:
+ properties:
+ clocks:
+ items:
+ - description: aggre UFS PHY AXI clock
+ - description: aggre USB3 PRIM AXI clock
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,sc7280-aggre2-noc
+ then:
+ properties:
+ clocks:
+ items:
+ - description: RPMH CC IPA clock
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,sc7280-aggre1-noc
+ - qcom,sc7280-aggre2-noc
+ then:
+ required:
+ - clocks
+ else:
+ properties:
+ clocks: false
+
unevaluatedProperties: false
examples:
- |
+ #include <dt-bindings/clock/qcom,gcc-sc7280.h>
interconnect {
compatible = "qcom,sc7280-clk-virt";
#interconnect-cells = <2>;
@@ -69,3 +109,12 @@ examples:
#interconnect-cells = <2>;
qcom,bcm-voters = <&apps_bcm_voter>;
};
+
+ interconnect@16e0000 {
+ reg = <0x016e0000 0x1c080>;
+ compatible = "qcom,sc7280-aggre1-noc";
+ #interconnect-cells = <2>;
+ qcom,bcm-voters = <&apps_bcm_voter>;
+ clocks = <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
+ <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>;
+ };
--
2.17.1
Enable QoS configuration for the master ports with predefined values
for priority and urgency.
Signed-off-by: Odelu Kukatla <[email protected]>
---
drivers/interconnect/qcom/sc7280.c | 332 +++++++++++++++++++++++++++++
1 file changed, 332 insertions(+)
diff --git a/drivers/interconnect/qcom/sc7280.c b/drivers/interconnect/qcom/sc7280.c
index 7d33694368e8..438f927935e5 100644
--- a/drivers/interconnect/qcom/sc7280.c
+++ b/drivers/interconnect/qcom/sc7280.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
/*
* Copyright (c) 2021, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
*
*/
@@ -16,29 +17,53 @@
#include "icc-rpmh.h"
#include "sc7280.h"
+static struct qcom_icc_qosbox qhm_qspi_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x7000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node qhm_qspi = {
.name = "qhm_qspi",
.id = SC7280_MASTER_QSPI_0,
.channels = 1,
.buswidth = 4,
+ .qosbox = &qhm_qspi_qos,
.num_links = 1,
.links = { SC7280_SLAVE_A1NOC_SNOC },
};
+static struct qcom_icc_qosbox qhm_qup0_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x11000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node qhm_qup0 = {
.name = "qhm_qup0",
.id = SC7280_MASTER_QUP_0,
.channels = 1,
.buswidth = 4,
+ .qosbox = &qhm_qup0_qos,
.num_links = 1,
.links = { SC7280_SLAVE_A1NOC_SNOC },
};
+static struct qcom_icc_qosbox qhm_qup1_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x8000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node qhm_qup1 = {
.name = "qhm_qup1",
.id = SC7280_MASTER_QUP_1,
.channels = 1,
.buswidth = 4,
+ .qosbox = &qhm_qup1_qos,
.num_links = 1,
.links = { SC7280_SLAVE_A1NOC_SNOC },
};
@@ -52,38 +77,70 @@ static struct qcom_icc_node qnm_a1noc_cfg = {
.links = { SC7280_SLAVE_SERVICE_A1NOC },
};
+static struct qcom_icc_qosbox xm_sdc1_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0xc000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node xm_sdc1 = {
.name = "xm_sdc1",
.id = SC7280_MASTER_SDCC_1,
.channels = 1,
.buswidth = 8,
+ .qosbox = &xm_sdc1_qos,
.num_links = 1,
.links = { SC7280_SLAVE_A1NOC_SNOC },
};
+static struct qcom_icc_qosbox xm_sdc2_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0xe000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node xm_sdc2 = {
.name = "xm_sdc2",
.id = SC7280_MASTER_SDCC_2,
.channels = 1,
.buswidth = 8,
+ .qosbox = &xm_sdc2_qos,
.num_links = 1,
.links = { SC7280_SLAVE_A1NOC_SNOC },
};
+static struct qcom_icc_qosbox xm_sdc4_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x9000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node xm_sdc4 = {
.name = "xm_sdc4",
.id = SC7280_MASTER_SDCC_4,
.channels = 1,
.buswidth = 8,
+ .qosbox = &xm_sdc4_qos,
.num_links = 1,
.links = { SC7280_SLAVE_A1NOC_SNOC },
};
+static struct qcom_icc_qosbox xm_ufs_mem_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0xa000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node xm_ufs_mem = {
.name = "xm_ufs_mem",
.id = SC7280_MASTER_UFS_MEM,
.channels = 1,
.buswidth = 8,
+ .qosbox = &xm_ufs_mem_qos,
.num_links = 1,
.links = { SC7280_SLAVE_A1NOC_SNOC },
};
@@ -97,20 +154,36 @@ static struct qcom_icc_node xm_usb2 = {
.links = { SC7280_SLAVE_A1NOC_SNOC },
};
+static struct qcom_icc_qosbox xm_usb3_0_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0xb000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node xm_usb3_0 = {
.name = "xm_usb3_0",
.id = SC7280_MASTER_USB3_0,
.channels = 1,
.buswidth = 8,
+ .qosbox = &xm_usb3_0_qos,
.num_links = 1,
.links = { SC7280_SLAVE_A1NOC_SNOC },
};
+static struct qcom_icc_qosbox qhm_qdss_bam_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x18000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node qhm_qdss_bam = {
.name = "qhm_qdss_bam",
.id = SC7280_MASTER_QDSS_BAM,
.channels = 1,
.buswidth = 4,
+ .qosbox = &qhm_qdss_bam_qos,
.num_links = 1,
.links = { SC7280_SLAVE_A2NOC_SNOC },
};
@@ -124,29 +197,53 @@ static struct qcom_icc_node qnm_a2noc_cfg = {
.links = { SC7280_SLAVE_SERVICE_A2NOC },
};
+static struct qcom_icc_qosbox qnm_cnoc_datapath_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x1c000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node qnm_cnoc_datapath = {
.name = "qnm_cnoc_datapath",
.id = SC7280_MASTER_CNOC_A2NOC,
.channels = 1,
.buswidth = 8,
+ .qosbox = &qnm_cnoc_datapath_qos,
.num_links = 1,
.links = { SC7280_SLAVE_A2NOC_SNOC },
};
+static struct qcom_icc_qosbox qxm_crypto_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x1d000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node qxm_crypto = {
.name = "qxm_crypto",
.id = SC7280_MASTER_CRYPTO,
.channels = 1,
.buswidth = 8,
+ .qosbox = &qxm_crypto_qos,
.num_links = 1,
.links = { SC7280_SLAVE_A2NOC_SNOC },
};
+static struct qcom_icc_qosbox qxm_ipa_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x10000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node qxm_ipa = {
.name = "qxm_ipa",
.id = SC7280_MASTER_IPA,
.channels = 1,
.buswidth = 8,
+ .qosbox = &qxm_ipa_qos,
.num_links = 1,
.links = { SC7280_SLAVE_A2NOC_SNOC },
};
@@ -168,11 +265,19 @@ static struct qcom_icc_node xm_pcie3_1 = {
.links = { SC7280_SLAVE_ANOC_PCIE_GEM_NOC },
};
+static struct qcom_icc_qosbox xm_qdss_etr_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x15000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node xm_qdss_etr = {
.name = "xm_qdss_etr",
.id = SC7280_MASTER_QDSS_ETR,
.channels = 1,
.buswidth = 8,
+ .qosbox = &xm_qdss_etr_qos,
.num_links = 1,
.links = { SC7280_SLAVE_A2NOC_SNOC },
};
@@ -300,20 +405,36 @@ static struct qcom_icc_node qnm_cnoc_dc_noc = {
.links = { SC7280_SLAVE_LLCC_CFG, SC7280_SLAVE_GEM_NOC_CFG },
};
+static struct qcom_icc_qosbox alm_gpu_tcu_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0xd7000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node alm_gpu_tcu = {
.name = "alm_gpu_tcu",
.id = SC7280_MASTER_GPU_TCU,
.channels = 1,
.buswidth = 8,
+ .qosbox = &alm_gpu_tcu_qos,
.num_links = 2,
.links = { SC7280_SLAVE_GEM_NOC_CNOC, SC7280_SLAVE_LLCC },
};
+static struct qcom_icc_qosbox alm_sys_tcu_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0xd6000 },
+ .prio = 6,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node alm_sys_tcu = {
.name = "alm_sys_tcu",
.id = SC7280_MASTER_SYS_TCU,
.channels = 1,
.buswidth = 8,
+ .qosbox = &alm_sys_tcu_qos,
.num_links = 2,
.links = { SC7280_SLAVE_GEM_NOC_CNOC, SC7280_SLAVE_LLCC },
};
@@ -328,11 +449,19 @@ static struct qcom_icc_node chm_apps = {
SC7280_SLAVE_MEM_NOC_PCIE_SNOC },
};
+static struct qcom_icc_qosbox qnm_cmpnoc_qos = {
+ .num_ports = 2,
+ .port_offsets = { 0x21000, 0x61000 },
+ .prio = 0,
+ .urg_fwd = 1,
+};
+
static struct qcom_icc_node qnm_cmpnoc = {
.name = "qnm_cmpnoc",
.id = SC7280_MASTER_COMPUTE_NOC,
.channels = 2,
.buswidth = 32,
+ .qosbox = &qnm_cmpnoc_qos,
.num_links = 2,
.links = { SC7280_SLAVE_GEM_NOC_CNOC, SC7280_SLAVE_LLCC },
};
@@ -348,29 +477,53 @@ static struct qcom_icc_node qnm_gemnoc_cfg = {
SC7280_SLAVE_SERVICE_GEM_NOC },
};
+static struct qcom_icc_qosbox qnm_gpu_qos = {
+ .num_ports = 2,
+ .port_offsets = { 0x22000, 0x62000 },
+ .prio = 0,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node qnm_gpu = {
.name = "qnm_gpu",
.id = SC7280_MASTER_GFX3D,
.channels = 2,
.buswidth = 32,
+ .qosbox = &qnm_gpu_qos,
.num_links = 2,
.links = { SC7280_SLAVE_GEM_NOC_CNOC, SC7280_SLAVE_LLCC },
};
+static struct qcom_icc_qosbox qnm_mnoc_hf_qos = {
+ .num_ports = 2,
+ .port_offsets = { 0x23000, 0x63000 },
+ .prio = 0,
+ .urg_fwd = 1,
+};
+
static struct qcom_icc_node qnm_mnoc_hf = {
.name = "qnm_mnoc_hf",
.id = SC7280_MASTER_MNOC_HF_MEM_NOC,
.channels = 2,
.buswidth = 32,
+ .qosbox = &qnm_mnoc_hf_qos,
.num_links = 1,
.links = { SC7280_SLAVE_LLCC },
};
+static struct qcom_icc_qosbox qnm_mnoc_sf_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0xcf000 },
+ .prio = 0,
+ .urg_fwd = 1,
+};
+
static struct qcom_icc_node qnm_mnoc_sf = {
.name = "qnm_mnoc_sf",
.id = SC7280_MASTER_MNOC_SF_MEM_NOC,
.channels = 1,
.buswidth = 32,
+ .qosbox = &qnm_mnoc_sf_qos,
.num_links = 2,
.links = { SC7280_SLAVE_GEM_NOC_CNOC, SC7280_SLAVE_LLCC },
};
@@ -384,20 +537,36 @@ static struct qcom_icc_node qnm_pcie = {
.links = { SC7280_SLAVE_GEM_NOC_CNOC, SC7280_SLAVE_LLCC },
};
+static struct qcom_icc_qosbox qnm_snoc_gc_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0xd3000 },
+ .prio = 0,
+ .urg_fwd = 1,
+};
+
static struct qcom_icc_node qnm_snoc_gc = {
.name = "qnm_snoc_gc",
.id = SC7280_MASTER_SNOC_GC_MEM_NOC,
.channels = 1,
.buswidth = 8,
+ .qosbox = &qnm_snoc_gc_qos,
.num_links = 1,
.links = { SC7280_SLAVE_LLCC },
};
+static struct qcom_icc_qosbox qnm_snoc_sf_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0xd4000 },
+ .prio = 0,
+ .urg_fwd = 1,
+};
+
static struct qcom_icc_node qnm_snoc_sf = {
.name = "qnm_snoc_sf",
.id = SC7280_MASTER_SNOC_SF_MEM_NOC,
.channels = 1,
.buswidth = 16,
+ .qosbox = &qnm_snoc_sf_qos,
.num_links = 3,
.links = { SC7280_SLAVE_GEM_NOC_CNOC, SC7280_SLAVE_LLCC,
SC7280_SLAVE_MEM_NOC_PCIE_SNOC },
@@ -432,56 +601,104 @@ static struct qcom_icc_node qnm_mnoc_cfg = {
.links = { SC7280_SLAVE_SERVICE_MNOC },
};
+static struct qcom_icc_qosbox qnm_video0_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x14000 },
+ .prio = 0,
+ .urg_fwd = 1,
+};
+
static struct qcom_icc_node qnm_video0 = {
.name = "qnm_video0",
.id = SC7280_MASTER_VIDEO_P0,
.channels = 1,
.buswidth = 32,
+ .qosbox = &qnm_video0_qos,
.num_links = 1,
.links = { SC7280_SLAVE_MNOC_SF_MEM_NOC },
};
+static struct qcom_icc_qosbox qnm_video_cpu_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x15000 },
+ .prio = 0,
+ .urg_fwd = 1,
+};
+
static struct qcom_icc_node qnm_video_cpu = {
.name = "qnm_video_cpu",
.id = SC7280_MASTER_VIDEO_PROC,
.channels = 1,
.buswidth = 8,
+ .qosbox = &qnm_video_cpu_qos,
.num_links = 1,
.links = { SC7280_SLAVE_MNOC_SF_MEM_NOC },
};
+static struct qcom_icc_qosbox qxm_camnoc_hf_qos = {
+ .num_ports = 2,
+ .port_offsets = { 0x10000, 0x10180 },
+ .prio = 0,
+ .urg_fwd = 1,
+};
+
static struct qcom_icc_node qxm_camnoc_hf = {
.name = "qxm_camnoc_hf",
.id = SC7280_MASTER_CAMNOC_HF,
.channels = 2,
.buswidth = 32,
+ .qosbox = &qxm_camnoc_hf_qos,
.num_links = 1,
.links = { SC7280_SLAVE_MNOC_HF_MEM_NOC },
};
+static struct qcom_icc_qosbox qxm_camnoc_icp_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x11000 },
+ .prio = 0,
+ .urg_fwd = 1,
+};
+
static struct qcom_icc_node qxm_camnoc_icp = {
.name = "qxm_camnoc_icp",
.id = SC7280_MASTER_CAMNOC_ICP,
.channels = 1,
.buswidth = 8,
+ .qosbox = &qxm_camnoc_icp_qos,
.num_links = 1,
.links = { SC7280_SLAVE_MNOC_SF_MEM_NOC },
};
+static struct qcom_icc_qosbox qxm_camnoc_sf_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x12000 },
+ .prio = 0,
+ .urg_fwd = 1,
+};
+
static struct qcom_icc_node qxm_camnoc_sf = {
.name = "qxm_camnoc_sf",
.id = SC7280_MASTER_CAMNOC_SF,
.channels = 1,
.buswidth = 32,
+ .qosbox = &qxm_camnoc_sf_qos,
.num_links = 1,
.links = { SC7280_SLAVE_MNOC_SF_MEM_NOC },
};
+static struct qcom_icc_qosbox qxm_mdp0_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x16000 },
+ .prio = 0,
+ .urg_fwd = 1,
+};
+
static struct qcom_icc_node qxm_mdp0 = {
.name = "qxm_mdp0",
.id = SC7280_MASTER_MDP0,
.channels = 1,
.buswidth = 32,
+ .qosbox = &qxm_mdp0_qos,
.num_links = 1,
.links = { SC7280_SLAVE_MNOC_HF_MEM_NOC },
};
@@ -531,20 +748,36 @@ static struct qcom_icc_node qnm_snoc_cfg = {
.links = { SC7280_SLAVE_SERVICE_SNOC },
};
+static struct qcom_icc_qosbox qxm_pimem_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0x8000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node qxm_pimem = {
.name = "qxm_pimem",
.id = SC7280_MASTER_PIMEM,
.channels = 1,
.buswidth = 8,
+ .qosbox = &qxm_pimem_qos,
.num_links = 1,
.links = { SC7280_SLAVE_SNOC_GEM_NOC_GC },
};
+static struct qcom_icc_qosbox xm_gic_qos = {
+ .num_ports = 1,
+ .port_offsets = { 0xa000 },
+ .prio = 2,
+ .urg_fwd = 0,
+};
+
static struct qcom_icc_node xm_gic = {
.name = "xm_gic",
.id = SC7280_MASTER_GIC,
.channels = 1,
.buswidth = 8,
+ .qosbox = &xm_gic_qos,
.num_links = 1,
.links = { SC7280_SLAVE_SNOC_GEM_NOC_GC },
};
@@ -1502,7 +1735,16 @@ static struct qcom_icc_node * const aggre1_noc_nodes[] = {
[SLAVE_SERVICE_A1NOC] = &srvc_aggre1_noc,
};
+static const struct regmap_config sc7280_aggre1_noc_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x1c080,
+ .fast_io = true,
+};
+
static const struct qcom_icc_desc sc7280_aggre1_noc = {
+ .config = &sc7280_aggre1_noc_regmap_config,
.nodes = aggre1_noc_nodes,
.num_nodes = ARRAY_SIZE(aggre1_noc_nodes),
.bcms = aggre1_noc_bcms,
@@ -1513,6 +1755,14 @@ static struct qcom_icc_bcm * const aggre2_noc_bcms[] = {
&bcm_ce0,
};
+static const struct regmap_config sc7280_aggre2_noc_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x2b080,
+ .fast_io = true,
+};
+
static struct qcom_icc_node * const aggre2_noc_nodes[] = {
[MASTER_QDSS_BAM] = &qhm_qdss_bam,
[MASTER_A2NOC_CFG] = &qnm_a2noc_cfg,
@@ -1525,6 +1775,7 @@ static struct qcom_icc_node * const aggre2_noc_nodes[] = {
};
static const struct qcom_icc_desc sc7280_aggre2_noc = {
+ .config = &sc7280_aggre2_noc_regmap_config,
.nodes = aggre2_noc_nodes,
.num_nodes = ARRAY_SIZE(aggre2_noc_nodes),
.bcms = aggre2_noc_bcms,
@@ -1605,7 +1856,16 @@ static struct qcom_icc_node * const cnoc2_nodes[] = {
[SLAVE_SNOC_CFG] = &qns_snoc_cfg,
};
+static const struct regmap_config sc7280_cnoc2_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x1000,
+ .fast_io = true,
+};
+
static const struct qcom_icc_desc sc7280_cnoc2 = {
+ .config = &sc7280_cnoc2_regmap_config,
.nodes = cnoc2_nodes,
.num_nodes = ARRAY_SIZE(cnoc2_nodes),
.bcms = cnoc2_bcms,
@@ -1637,7 +1897,16 @@ static struct qcom_icc_node * const cnoc3_nodes[] = {
[SLAVE_TCU] = &xs_sys_tcu_cfg,
};
+static const struct regmap_config sc7280_cnoc3_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x1000,
+ .fast_io = true,
+};
+
static const struct qcom_icc_desc sc7280_cnoc3 = {
+ .config = &sc7280_cnoc3_regmap_config,
.nodes = cnoc3_nodes,
.num_nodes = ARRAY_SIZE(cnoc3_nodes),
.bcms = cnoc3_bcms,
@@ -1653,7 +1922,16 @@ static struct qcom_icc_node * const dc_noc_nodes[] = {
[SLAVE_GEM_NOC_CFG] = &qns_gemnoc,
};
+static const struct regmap_config sc7280_dc_noc_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x5080,
+ .fast_io = true,
+};
+
static const struct qcom_icc_desc sc7280_dc_noc = {
+ .config = &sc7280_dc_noc_regmap_config,
.nodes = dc_noc_nodes,
.num_nodes = ARRAY_SIZE(dc_noc_nodes),
.bcms = dc_noc_bcms,
@@ -1689,7 +1967,16 @@ static struct qcom_icc_node * const gem_noc_nodes[] = {
[SLAVE_SERVICE_GEM_NOC] = &srvc_sys_gemnoc,
};
+static const struct regmap_config sc7280_gem_noc_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0xe2200,
+ .fast_io = true,
+};
+
static const struct qcom_icc_desc sc7280_gem_noc = {
+ .config = &sc7280_gem_noc_regmap_config,
.nodes = gem_noc_nodes,
.num_nodes = ARRAY_SIZE(gem_noc_nodes),
.bcms = gem_noc_bcms,
@@ -1709,7 +1996,16 @@ static struct qcom_icc_node * const lpass_ag_noc_nodes[] = {
[SLAVE_SERVICE_LPASS_AG_NOC] = &srvc_niu_lpass_agnoc,
};
+static const struct regmap_config sc7280_lpass_ag_noc_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0xf080,
+ .fast_io = true,
+};
+
static const struct qcom_icc_desc sc7280_lpass_ag_noc = {
+ .config = &sc7280_lpass_ag_noc_regmap_config,
.nodes = lpass_ag_noc_nodes,
.num_nodes = ARRAY_SIZE(lpass_ag_noc_nodes),
.bcms = lpass_ag_noc_bcms,
@@ -1726,7 +2022,16 @@ static struct qcom_icc_node * const mc_virt_nodes[] = {
[SLAVE_EBI1] = &ebi,
};
+static const struct regmap_config sc7280_mc_virt_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x4,
+ .fast_io = true,
+};
+
static const struct qcom_icc_desc sc7280_mc_virt = {
+ .config = &sc7280_mc_virt_regmap_config,
.nodes = mc_virt_nodes,
.num_nodes = ARRAY_SIZE(mc_virt_nodes),
.bcms = mc_virt_bcms,
@@ -1753,7 +2058,16 @@ static struct qcom_icc_node * const mmss_noc_nodes[] = {
[SLAVE_SERVICE_MNOC] = &srvc_mnoc,
};
+static const struct regmap_config sc7280_mmss_noc_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x1e080,
+ .fast_io = true,
+};
+
static const struct qcom_icc_desc sc7280_mmss_noc = {
+ .config = &sc7280_mmss_noc_regmap_config,
.nodes = mmss_noc_nodes,
.num_nodes = ARRAY_SIZE(mmss_noc_nodes),
.bcms = mmss_noc_bcms,
@@ -1772,7 +2086,16 @@ static struct qcom_icc_node * const nsp_noc_nodes[] = {
[SLAVE_SERVICE_NSP_NOC] = &service_nsp_noc,
};
+static const struct regmap_config sc7280_nsp_noc_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x10000,
+ .fast_io = true,
+};
+
static const struct qcom_icc_desc sc7280_nsp_noc = {
+ .config = &sc7280_nsp_noc_regmap_config,
.nodes = nsp_noc_nodes,
.num_nodes = ARRAY_SIZE(nsp_noc_nodes),
.bcms = nsp_noc_bcms,
@@ -1797,7 +2120,16 @@ static struct qcom_icc_node * const system_noc_nodes[] = {
[SLAVE_SERVICE_SNOC] = &srvc_snoc,
};
+static const struct regmap_config sc7280_system_noc_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x15480,
+ .fast_io = true,
+};
+
static const struct qcom_icc_desc sc7280_system_noc = {
+ .config = &sc7280_system_noc_regmap_config,
.nodes = system_noc_nodes,
.num_nodes = ARRAY_SIZE(system_noc_nodes),
.bcms = system_noc_bcms,
--
2.17.1
On 05/02/2024 15:56, Odelu Kukatla wrote:
> Added clock property to enable clocks required for accessing
> qos registers.
I have no idea how you came up with that CC list. It makes no sense.
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.
Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline), work on fork of kernel
(don't, instead use mainline) or you ignore some maintainers (really
don't). Just use b4 and everything should be fine, although remember
about `b4 prep --auto-to-cc` if you added new patches to the patchset.
You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time, thus I will skip this patch entirely till you follow
the process allowing the patch to be tested.
Please kindly resend and include all necessary To/Cc entries. Tomorrow.
Best regards,
Krzysztof
Add handles for the required clocks to be enabled for configuring
QoS on sc7280.
Signed-off-by: Odelu Kukatla <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index c3a94c4c6490..6b50f37af508 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -2099,6 +2099,8 @@
reg = <0 0x016e0000 0 0x1c080>;
#interconnect-cells = <2>;
qcom,bcm-voters = <&apps_bcm_voter>;
+ clocks = <&gcc GCC_AGGRE_UFS_PHY_AXI_CLK>,
+ <&gcc GCC_AGGRE_USB3_PRIM_AXI_CLK>;
};
aggre2_noc: interconnect@1700000 {
@@ -2106,6 +2108,7 @@
compatible = "qcom,sc7280-aggre2-noc";
#interconnect-cells = <2>;
qcom,bcm-voters = <&apps_bcm_voter>;
+ clocks = <&rpmhcc RPMH_IPA_CLK>;
};
mmss_noc: interconnect@1740000 {
--
2.17.1
On Mon, 5 Feb 2024 at 16:57, Odelu Kukatla <[email protected]> wrote:
>
> It adds QoS support for QNOC device and includes support for
> configuring priority, priority forward disable, urgency forwarding.
> This helps in priortizing the traffic originating from different
> interconnect masters at NoC(Network On Chip).
>
> Signed-off-by: Odelu Kukatla <[email protected]>
> ---
> drivers/interconnect/qcom/icc-rpmh.c | 161 ++++++++++++++++++++++++++-
> drivers/interconnect/qcom/icc-rpmh.h | 32 ++++++
> 2 files changed, 189 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/interconnect/qcom/icc-rpmh.c b/drivers/interconnect/qcom/icc-rpmh.c
> index c1aa265c1f4e..67e600f1191d 100644
> --- a/drivers/interconnect/qcom/icc-rpmh.c
> +++ b/drivers/interconnect/qcom/icc-rpmh.c
> @@ -1,28 +1,69 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> */
>
> +#include <linux/clk.h>
> #include <linux/interconnect.h>
> #include <linux/interconnect-provider.h>
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_platform.h>
> #include <linux/slab.h>
> +#include <linux/bits.h>
>
> #include "bcm-voter.h"
> #include "icc-common.h"
> #include "icc-rpmh.h"
>
> +/* QNOC QoS */
> +#define QOSGEN_MAINCTL_LO(p, qp) (0x8 + (p->port_offsets[qp]))
> +#define QOS_SLV_URG_MSG_EN_MASK BIT_MASK(3)
> +#define QOS_DFLT_PRIO_MASK GENMASK(6, 4)
> +#define QOS_DISABLE_MASK BIT_MASK(24)
> +
> +/**
> + * qcom_icc_set_qos - initialize static QoS configurations
> + * @qp: qcom icc provider to which @node belongs
> + * @node: qcom icc node to operate on
> + */
> +static void qcom_icc_set_qos(struct qcom_icc_provider *qp,
> + struct qcom_icc_node *node)
> +{
> + const struct qcom_icc_qosbox *qos = node->qosbox;
> + int port;
> +
> + if (!qp->regmap)
> + return;
> +
> + if (!qos)
> + return;
> +
> + for (port = 0; port < qos->num_ports; port++) {
> + regmap_update_bits(qp->regmap, QOSGEN_MAINCTL_LO(qos, port),
> + QOS_DISABLE_MASK,
> + FIELD_PREP(QOS_DISABLE_MASK, qos->prio_fwd_disable));
> +
> + regmap_update_bits(qp->regmap, QOSGEN_MAINCTL_LO(qos, port),
> + QOS_DFLT_PRIO_MASK,
> + FIELD_PREP(QOS_DFLT_PRIO_MASK, qos->prio));
> +
> + regmap_update_bits(qp->regmap, QOSGEN_MAINCTL_LO(qos, port),
> + QOS_SLV_URG_MSG_EN_MASK,
> + FIELD_PREP(QOS_SLV_URG_MSG_EN_MASK, qos->urg_fwd));
> + }
> +}
> +
> /**
> * qcom_icc_pre_aggregate - cleans up stale values from prior icc_set
> * @node: icc node to operate on
> */
> void qcom_icc_pre_aggregate(struct icc_node *node)
> {
> - size_t i;
> - struct qcom_icc_node *qn;
> struct qcom_icc_provider *qp;
> + struct qcom_icc_node *qn;
> + size_t i;
>
> qn = node->data;
> qp = to_qcom_provider(node->provider);
> @@ -49,8 +90,8 @@ EXPORT_SYMBOL_GPL(qcom_icc_pre_aggregate);
> int qcom_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
> u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
> {
> - size_t i;
> struct qcom_icc_node *qn;
> + size_t i;
Please abstain from making the cleanup changes in the same patch as
the functional changes. These changes either should be dropped or
moved to a separate patch.
>
> qn = node->data;
>
> @@ -159,13 +200,96 @@ int qcom_icc_bcm_init(struct qcom_icc_bcm *bcm, struct device *dev)
> }
> EXPORT_SYMBOL_GPL(qcom_icc_bcm_init);
>
> +static bool bcm_needs_qos_proxy(struct qcom_icc_bcm *bcm)
Although this part is not exported to the modules, I think it deserves
some documentation. Either in the commit message or in the source
file.
> +{
> + int i;
> +
> + for (i = 0; i < bcm->num_nodes; i++)
> + if (bcm->nodes[i]->qosbox)
> + return true;
> +
> + return false;
> +}
> +
> +static int enable_qos_deps(struct qcom_icc_provider *qp)
> +{
> + struct qcom_icc_bcm *bcm;
> + bool keepalive;
> + int ret, i;
> +
> + for (i = 0; i < qp->num_bcms; i++) {
> + bcm = qp->bcms[i];
> + if (bcm_needs_qos_proxy(bcm)) {
> + keepalive = bcm->keepalive;
> + bcm->keepalive = true;
> +
> + qcom_icc_bcm_voter_add(qp->voter, bcm);
> + ret = qcom_icc_bcm_voter_commit(qp->voter);
> +
> + bcm->keepalive = keepalive;
> +
> + if (ret) {
> + dev_err(qp->dev, "failed to vote BW to %s for QoS\n",
> + bcm->name);
> + return ret;
> + }
> + }
> + }
> +
> + ret = clk_bulk_prepare_enable(qp->num_clks, qp->clks);
> + if (ret)
> + dev_err(qp->dev, "failed to enable clocks for QoS\n");
> +
> + return ret;
> +}
> +
> +static void disable_qos_deps(struct qcom_icc_provider *qp)
> +{
> + struct qcom_icc_bcm *bcm;
> + int i;
> +
> + clk_bulk_disable_unprepare(qp->num_clks, qp->clks);
> +
> + for (i = 0; i < qp->num_bcms; i++) {
> + bcm = qp->bcms[i];
> + if (bcm_needs_qos_proxy(bcm)) {
> + qcom_icc_bcm_voter_add(qp->voter, bcm);
> + qcom_icc_bcm_voter_commit(qp->voter);
> + }
> + }
> +}
> +
> +static int qcom_icc_rpmh_configure_qos(struct qcom_icc_provider *qp)
> +{
> + struct qcom_icc_node *qnode;
> + size_t i;
> + int ret;
> +
> + ret = enable_qos_deps(qp);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < qp->num_nodes; i++) {
> + qnode = qp->nodes[i];
> + if (!qnode)
> + continue;
> +
> + if (qnode->qosbox)
> + qcom_icc_set_qos(qp, qnode);
> + }
> +
> + disable_qos_deps(qp);
> +
> + return ret;
> +}
> +
> int qcom_icc_rpmh_probe(struct platform_device *pdev)
> {
> + struct qcom_icc_node * const *qnodes, *qn;
> const struct qcom_icc_desc *desc;
> struct device *dev = &pdev->dev;
> struct icc_onecell_data *data;
> struct icc_provider *provider;
> - struct qcom_icc_node * const *qnodes, *qn;
> struct qcom_icc_provider *qp;
> struct icc_node *node;
> size_t num_nodes, i, j;
> @@ -199,12 +323,35 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev)
>
> qp->dev = dev;
> qp->bcms = desc->bcms;
> + qp->nodes = desc->nodes;
> qp->num_bcms = desc->num_bcms;
> + qp->num_nodes = desc->num_nodes;
>
> qp->voter = of_bcm_voter_get(qp->dev, NULL);
> if (IS_ERR(qp->voter))
> return PTR_ERR(qp->voter);
>
> + if (desc->config) {
> + struct resource *res;
> + void __iomem *base;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -ENODEV;
Having a working ICC even without QoS changes is still important. I'd
suggest to warn the user here and continue probe, skipping QoS
settings.
> +
> + base = devm_ioremap(dev, res->start, resource_size(res));
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + qp->regmap = devm_regmap_init_mmio(dev, base, desc->config);
> + if (IS_ERR(qp->regmap))
> + return PTR_ERR(qp->regmap);
> + }
> +
> + qp->num_clks = devm_clk_bulk_get_all(qp->dev, &qp->clks);
> + if (qp->num_clks < 0)
> + return qp->num_clks;
> +
> for (i = 0; i < qp->num_bcms; i++)
> qcom_icc_bcm_init(qp->bcms[i], dev);
>
> @@ -229,6 +376,10 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev)
> data->nodes[i] = node;
> }
>
> + ret = qcom_icc_rpmh_configure_qos(qp);
> + if (ret)
> + goto err_remove_nodes;
> +
> ret = icc_provider_register(provider);
> if (ret)
> goto err_remove_nodes;
> @@ -247,6 +398,7 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev)
> err_deregister_provider:
> icc_provider_deregister(provider);
> err_remove_nodes:
> + clk_bulk_put_all(qp->num_clks, qp->clks);
No!!! You should never manually free the resources that are handled by
the devm_ functions. In rare cases it is really necessary for some
reason, there exists a counterpart devm_free_something funciton.
> icc_nodes_remove(provider);
>
> return ret;
> @@ -258,6 +410,7 @@ void qcom_icc_rpmh_remove(struct platform_device *pdev)
> struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
>
> icc_provider_deregister(&qp->provider);
> + clk_bulk_put_all(qp->num_clks, qp->clks);
And here again.
> icc_nodes_remove(&qp->provider);
> }
> EXPORT_SYMBOL_GPL(qcom_icc_rpmh_remove);
> diff --git a/drivers/interconnect/qcom/icc-rpmh.h b/drivers/interconnect/qcom/icc-rpmh.h
> index 2de29460e808..65cef156f212 100644
> --- a/drivers/interconnect/qcom/icc-rpmh.h
> +++ b/drivers/interconnect/qcom/icc-rpmh.h
> @@ -1,12 +1,14 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> /*
> * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> */
>
> #ifndef __DRIVERS_INTERCONNECT_QCOM_ICC_RPMH_H__
> #define __DRIVERS_INTERCONNECT_QCOM_ICC_RPMH_H__
>
> #include <dt-bindings/interconnect/qcom,icc.h>
> +#include <linux/regmap.h>
>
> #define to_qcom_provider(_provider) \
> container_of(_provider, struct qcom_icc_provider, provider)
> @@ -18,6 +20,11 @@
> * @bcms: list of bcms that maps to the provider
> * @num_bcms: number of @bcms
> * @voter: bcm voter targeted by this provider
> + * @nodes: list of icc nodes that maps to the provider
> + * @num_nodes: number of @nodes
> + * @regmap: used for QoS, register access
> + * @clks : clks required for register access
> + * @num_clks: number of @clks
> */
> struct qcom_icc_provider {
> struct icc_provider provider;
> @@ -25,6 +32,11 @@ struct qcom_icc_provider {
> struct qcom_icc_bcm * const *bcms;
> size_t num_bcms;
> struct bcm_voter *voter;
> + struct qcom_icc_node * const *nodes;
> + size_t num_nodes;
> + struct regmap *regmap;
> + struct clk_bulk_data *clks;
> + int num_clks;
Is it really necessary to store this in the global data? If I
understand correctly, QoS is only programmed during the boot and is
not to be reprogrammed later. Is my understanding correct?
> };
>
> /**
> @@ -41,6 +53,23 @@ struct bcm_db {
> u8 reserved;
> };
>
> +/**
> + * struct qcom_icc_qosbox - Qualcomm specific QoS config
Why is it qosbox?
> + * @prio: priority value assigned to requests on the node
> + * @urg_fwd: if set, master priority is used for requests.
> + * @prio_fwd_disable: if set, master priority is ignored and NoC's default priority is used.
> + * @num_ports: number of @ports
> + * @port_offsets: qos register offsets
> + */
> +
> +struct qcom_icc_qosbox {
> + const u32 prio;
> + const bool urg_fwd;
> + const bool prio_fwd_disable;
> + const u32 num_ports;
> + const u32 port_offsets[] __counted_by(num_ports);
> +};
> +
> #define MAX_LINKS 128
> #define MAX_BCMS 64
> #define MAX_BCM_PER_NODE 3
> @@ -58,6 +87,7 @@ struct bcm_db {
> * @max_peak: current max aggregate value of all peak bw requests
> * @bcms: list of bcms associated with this logical node
> * @num_bcms: num of @bcms
> + * @qosbox: qos config data associated with node
> */
> struct qcom_icc_node {
> const char *name;
> @@ -70,6 +100,7 @@ struct qcom_icc_node {
> u64 max_peak[QCOM_ICC_NUM_BUCKETS];
> struct qcom_icc_bcm *bcms[MAX_BCM_PER_NODE];
> size_t num_bcms;
> + const struct qcom_icc_qosbox *qosbox;
> };
>
> /**
> @@ -114,6 +145,7 @@ struct qcom_icc_fabric {
> };
>
> struct qcom_icc_desc {
> + const struct regmap_config *config;
> struct qcom_icc_node * const *nodes;
> size_t num_nodes;
> struct qcom_icc_bcm * const *bcms;
> --
> 2.17.1
>
>
--
With best wishes
Dmitry
On 05/02/2024 15:56, Odelu Kukatla wrote:
> Enable QoS configuration for the master ports with predefined values
> for priority and urgency.
>
> Signed-off-by: Odelu Kukatla <[email protected]>
> ---
> drivers/interconnect/qcom/sc7280.c | 332 +++++++++++++++++++++++++++++
> 1 file changed, 332 insertions(+)
>
> diff --git a/drivers/interconnect/qcom/sc7280.c b/drivers/interconnect/qcom/sc7280.c
> index 7d33694368e8..438f927935e5 100644
> --- a/drivers/interconnect/qcom/sc7280.c
> +++ b/drivers/interconnect/qcom/sc7280.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0
> /*
> * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
> *
> */
>
> @@ -16,29 +17,53 @@
> #include "icc-rpmh.h"
> #include "sc7280.h"
>
> +static struct qcom_icc_qosbox qhm_qspi_qos = {
Why this cannot be const?
> + .num_ports = 1,
> + .port_offsets = { 0x7000 },
> + .prio = 2,
> + .urg_fwd = 0,
Best regards,
Krzysztof
Hi Odelu,
kernel test robot noticed the following build errors:
[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.8-rc3 next-20240205]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Odelu-Kukatla/interconnect-qcom-icc-rpmh-Add-QoS-configuration-support/20240205-230208
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20240205145606.16936-2-quic_okukatla%40quicinc.com
patch subject: [PATCH v2 1/4] interconnect: qcom: icc-rpmh: Add QoS configuration support
config: arm-allmodconfig (https://download.01.org/0day-ci/archive/20240206/[email protected]/config)
compiler: arm-linux-gnueabi-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240206/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
drivers/interconnect/qcom/icc-rpmh.c: In function 'qcom_icc_set_qos':
>> drivers/interconnect/qcom/icc-rpmh.c:46:36: error: implicit declaration of function 'FIELD_PREP' [-Werror=implicit-function-declaration]
46 | FIELD_PREP(QOS_DISABLE_MASK, qos->prio_fwd_disable));
| ^~~~~~~~~~
cc1: some warnings being treated as errors
vim +/FIELD_PREP +46 drivers/interconnect/qcom/icc-rpmh.c
25
26 /**
27 * qcom_icc_set_qos - initialize static QoS configurations
28 * @qp: qcom icc provider to which @node belongs
29 * @node: qcom icc node to operate on
30 */
31 static void qcom_icc_set_qos(struct qcom_icc_provider *qp,
32 struct qcom_icc_node *node)
33 {
34 const struct qcom_icc_qosbox *qos = node->qosbox;
35 int port;
36
37 if (!qp->regmap)
38 return;
39
40 if (!qos)
41 return;
42
43 for (port = 0; port < qos->num_ports; port++) {
44 regmap_update_bits(qp->regmap, QOSGEN_MAINCTL_LO(qos, port),
45 QOS_DISABLE_MASK,
> 46 FIELD_PREP(QOS_DISABLE_MASK, qos->prio_fwd_disable));
47
48 regmap_update_bits(qp->regmap, QOSGEN_MAINCTL_LO(qos, port),
49 QOS_DFLT_PRIO_MASK,
50 FIELD_PREP(QOS_DFLT_PRIO_MASK, qos->prio));
51
52 regmap_update_bits(qp->regmap, QOSGEN_MAINCTL_LO(qos, port),
53 QOS_SLV_URG_MSG_EN_MASK,
54 FIELD_PREP(QOS_SLV_URG_MSG_EN_MASK, qos->urg_fwd));
55 }
56 }
57
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On 2/5/24 15:56, Odelu Kukatla wrote:
> This series adds QoS support for QNOC type device which can be found on
> SC7280 platform. It adds support for programming priority,
> priority forward disable and urgency forwarding. This helps in
> priortizing the traffic originating from different interconnect masters
> at NOC(Network On Chip).
>
> Changes in v2:
> - Updated regmap_update to make use GENMASK and FIELD_PREP.
> - Removed the regmap structure from qcom_icc_node.
> - Made qcom_icc_rpmh_configure_qos() static
> - Removed qcom_icc_rpmh_map() API, inlined the code in probe
> function.
> - Updated declarations to reverse christmas tree fashion.
You ignored some of my previous review comments without a response.
Konrad
On 2/5/2024 8:38 PM, Dmitry Baryshkov wrote:
> On Mon, 5 Feb 2024 at 16:57, Odelu Kukatla <[email protected]> wrote:
>>
>> It adds QoS support for QNOC device and includes support for
>> configuring priority, priority forward disable, urgency forwarding.
>> This helps in priortizing the traffic originating from different
>> interconnect masters at NoC(Network On Chip).
>>
>> Signed-off-by: Odelu Kukatla <[email protected]>
>> ---
>> drivers/interconnect/qcom/icc-rpmh.c | 161 ++++++++++++++++++++++++++-
>> drivers/interconnect/qcom/icc-rpmh.h | 32 ++++++
>> 2 files changed, 189 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/interconnect/qcom/icc-rpmh.c b/drivers/interconnect/qcom/icc-rpmh.c
>> index c1aa265c1f4e..67e600f1191d 100644
>> --- a/drivers/interconnect/qcom/icc-rpmh.c
>> +++ b/drivers/interconnect/qcom/icc-rpmh.c
>> @@ -1,28 +1,69 @@
>> // SPDX-License-Identifier: GPL-2.0
>> /*
>> * Copyright (c) 2020, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> */
>>
>> +#include <linux/clk.h>
>> #include <linux/interconnect.h>
>> #include <linux/interconnect-provider.h>
>> #include <linux/module.h>
>> #include <linux/of.h>
>> #include <linux/of_platform.h>
>> #include <linux/slab.h>
>> +#include <linux/bits.h>
>>
>> #include "bcm-voter.h"
>> #include "icc-common.h"
>> #include "icc-rpmh.h"
>>
>> +/* QNOC QoS */
>> +#define QOSGEN_MAINCTL_LO(p, qp) (0x8 + (p->port_offsets[qp]))
>> +#define QOS_SLV_URG_MSG_EN_MASK BIT_MASK(3)
>> +#define QOS_DFLT_PRIO_MASK GENMASK(6, 4)
>> +#define QOS_DISABLE_MASK BIT_MASK(24)
>> +
>> +/**
>> + * qcom_icc_set_qos - initialize static QoS configurations
>> + * @qp: qcom icc provider to which @node belongs
>> + * @node: qcom icc node to operate on
>> + */
>> +static void qcom_icc_set_qos(struct qcom_icc_provider *qp,
>> + struct qcom_icc_node *node)
>> +{
>> + const struct qcom_icc_qosbox *qos = node->qosbox;
>> + int port;
>> +
>> + if (!qp->regmap)
>> + return;
>> +
>> + if (!qos)
>> + return;
>> +
>> + for (port = 0; port < qos->num_ports; port++) {
>> + regmap_update_bits(qp->regmap, QOSGEN_MAINCTL_LO(qos, port),
>> + QOS_DISABLE_MASK,
>> + FIELD_PREP(QOS_DISABLE_MASK, qos->prio_fwd_disable));
>> +
>> + regmap_update_bits(qp->regmap, QOSGEN_MAINCTL_LO(qos, port),
>> + QOS_DFLT_PRIO_MASK,
>> + FIELD_PREP(QOS_DFLT_PRIO_MASK, qos->prio));
>> +
>> + regmap_update_bits(qp->regmap, QOSGEN_MAINCTL_LO(qos, port),
>> + QOS_SLV_URG_MSG_EN_MASK,
>> + FIELD_PREP(QOS_SLV_URG_MSG_EN_MASK, qos->urg_fwd));
>> + }
>> +}
>> +
>> /**
>> * qcom_icc_pre_aggregate - cleans up stale values from prior icc_set
>> * @node: icc node to operate on
>> */
>> void qcom_icc_pre_aggregate(struct icc_node *node)
>> {
>> - size_t i;
>> - struct qcom_icc_node *qn;
>> struct qcom_icc_provider *qp;
>> + struct qcom_icc_node *qn;
>> + size_t i;
>>
>> qn = node->data;
>> qp = to_qcom_provider(node->provider);
>> @@ -49,8 +90,8 @@ EXPORT_SYMBOL_GPL(qcom_icc_pre_aggregate);
>> int qcom_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
>> u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
>> {
>> - size_t i;
>> struct qcom_icc_node *qn;
>> + size_t i;
>
> Please abstain from making the cleanup changes in the same patch as
> the functional changes. These changes either should be dropped or
> moved to a separate patch.
>
Sure, i will take care of it in next revision.
>>
>> qn = node->data;
>>
>> @@ -159,13 +200,96 @@ int qcom_icc_bcm_init(struct qcom_icc_bcm *bcm, struct device *dev)
>> }
>> EXPORT_SYMBOL_GPL(qcom_icc_bcm_init);
>>
>> +static bool bcm_needs_qos_proxy(struct qcom_icc_bcm *bcm)
>
> Although this part is not exported to the modules, I think it deserves
> some documentation. Either in the commit message or in the source
> file.
>
I am going to address this in V3.
>> +{
>> + int i;
>> +
>> + for (i = 0; i < bcm->num_nodes; i++)
>> + if (bcm->nodes[i]->qosbox)
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +static int enable_qos_deps(struct qcom_icc_provider *qp)
>> +{
>> + struct qcom_icc_bcm *bcm;
>> + bool keepalive;
>> + int ret, i;
>> +
>> + for (i = 0; i < qp->num_bcms; i++) {
>> + bcm = qp->bcms[i];
>> + if (bcm_needs_qos_proxy(bcm)) {
>> + keepalive = bcm->keepalive;
>> + bcm->keepalive = true;
>> +
>> + qcom_icc_bcm_voter_add(qp->voter, bcm);
>> + ret = qcom_icc_bcm_voter_commit(qp->voter);
>> +
>> + bcm->keepalive = keepalive;
>> +
>> + if (ret) {
>> + dev_err(qp->dev, "failed to vote BW to %s for QoS\n",
>> + bcm->name);
>> + return ret;
>> + }
>> + }
>> + }
>> +
>> + ret = clk_bulk_prepare_enable(qp->num_clks, qp->clks);
>> + if (ret)
>> + dev_err(qp->dev, "failed to enable clocks for QoS\n");
>> +
>> + return ret;
>> +}
>> +
>> +static void disable_qos_deps(struct qcom_icc_provider *qp)
>> +{
>> + struct qcom_icc_bcm *bcm;
>> + int i;
>> +
>> + clk_bulk_disable_unprepare(qp->num_clks, qp->clks);
>> +
>> + for (i = 0; i < qp->num_bcms; i++) {
>> + bcm = qp->bcms[i];
>> + if (bcm_needs_qos_proxy(bcm)) {
>> + qcom_icc_bcm_voter_add(qp->voter, bcm);
>> + qcom_icc_bcm_voter_commit(qp->voter);
>> + }
>> + }
>> +}
>> +
>> +static int qcom_icc_rpmh_configure_qos(struct qcom_icc_provider *qp)
>> +{
>> + struct qcom_icc_node *qnode;
>> + size_t i;
>> + int ret;
>> +
>> + ret = enable_qos_deps(qp);
>> + if (ret)
>> + return ret;
>> +
>> + for (i = 0; i < qp->num_nodes; i++) {
>> + qnode = qp->nodes[i];
>> + if (!qnode)
>> + continue;
>> +
>> + if (qnode->qosbox)
>> + qcom_icc_set_qos(qp, qnode);
>> + }
>> +
>> + disable_qos_deps(qp);
>> +
>> + return ret;
>> +}
>> +
>> int qcom_icc_rpmh_probe(struct platform_device *pdev)
>> {
>> + struct qcom_icc_node * const *qnodes, *qn;
>> const struct qcom_icc_desc *desc;
>> struct device *dev = &pdev->dev;
>> struct icc_onecell_data *data;
>> struct icc_provider *provider;
>> - struct qcom_icc_node * const *qnodes, *qn;
>> struct qcom_icc_provider *qp;
>> struct icc_node *node;
>> size_t num_nodes, i, j;
>> @@ -199,12 +323,35 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev)
>>
>> qp->dev = dev;
>> qp->bcms = desc->bcms;
>> + qp->nodes = desc->nodes;
>> qp->num_bcms = desc->num_bcms;
>> + qp->num_nodes = desc->num_nodes;
>>
>> qp->voter = of_bcm_voter_get(qp->dev, NULL);
>> if (IS_ERR(qp->voter))
>> return PTR_ERR(qp->voter);
>>
>> + if (desc->config) {
>> + struct resource *res;
>> + void __iomem *base;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res)
>> + return -ENODEV;
>
> Having a working ICC even without QoS changes is still important. I'd
> suggest to warn the user here and continue probe, skipping QoS
> settings.
>
I am going to address this in V3.
>> +
>> + base = devm_ioremap(dev, res->start, resource_size(res));
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> +
>> + qp->regmap = devm_regmap_init_mmio(dev, base, desc->config);
>> + if (IS_ERR(qp->regmap))
>> + return PTR_ERR(qp->regmap);
>> + }
>> +
>> + qp->num_clks = devm_clk_bulk_get_all(qp->dev, &qp->clks);
>> + if (qp->num_clks < 0)
>> + return qp->num_clks;
>> +
>> for (i = 0; i < qp->num_bcms; i++)
>> qcom_icc_bcm_init(qp->bcms[i], dev);
>>
>> @@ -229,6 +376,10 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev)
>> data->nodes[i] = node;
>> }
>>
>> + ret = qcom_icc_rpmh_configure_qos(qp);
>> + if (ret)
>> + goto err_remove_nodes;
>> +
>> ret = icc_provider_register(provider);
>> if (ret)
>> goto err_remove_nodes;
>> @@ -247,6 +398,7 @@ int qcom_icc_rpmh_probe(struct platform_device *pdev)
>> err_deregister_provider:
>> icc_provider_deregister(provider);
>> err_remove_nodes:
>> + clk_bulk_put_all(qp->num_clks, qp->clks);
>
> No!!! You should never manually free the resources that are handled by
> the devm_ functions. In rare cases it is really necessary for some
> reason, there exists a counterpart devm_free_something funciton.
>
Yeah, i will remove it in next revision.
>> icc_nodes_remove(provider);
>>
>> return ret;
>> @@ -258,6 +410,7 @@ void qcom_icc_rpmh_remove(struct platform_device *pdev)
>> struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
>>
>> icc_provider_deregister(&qp->provider);
>> + clk_bulk_put_all(qp->num_clks, qp->clks);
>
> And here again.
>
Yeah, i will remove it in next revision.
>> icc_nodes_remove(&qp->provider);
>> }
>> EXPORT_SYMBOL_GPL(qcom_icc_rpmh_remove);
>> diff --git a/drivers/interconnect/qcom/icc-rpmh.h b/drivers/interconnect/qcom/icc-rpmh.h
>> index 2de29460e808..65cef156f212 100644
>> --- a/drivers/interconnect/qcom/icc-rpmh.h
>> +++ b/drivers/interconnect/qcom/icc-rpmh.h
>> @@ -1,12 +1,14 @@
>> /* SPDX-License-Identifier: GPL-2.0 */
>> /*
>> * Copyright (c) 2020, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> */
>>
>> #ifndef __DRIVERS_INTERCONNECT_QCOM_ICC_RPMH_H__
>> #define __DRIVERS_INTERCONNECT_QCOM_ICC_RPMH_H__
>>
>> #include <dt-bindings/interconnect/qcom,icc.h>
>> +#include <linux/regmap.h>
>>
>> #define to_qcom_provider(_provider) \
>> container_of(_provider, struct qcom_icc_provider, provider)
>> @@ -18,6 +20,11 @@
>> * @bcms: list of bcms that maps to the provider
>> * @num_bcms: number of @bcms
>> * @voter: bcm voter targeted by this provider
>> + * @nodes: list of icc nodes that maps to the provider
>> + * @num_nodes: number of @nodes
>> + * @regmap: used for QoS, register access
>> + * @clks : clks required for register access
>> + * @num_clks: number of @clks
>> */
>> struct qcom_icc_provider {
>> struct icc_provider provider;
>> @@ -25,6 +32,11 @@ struct qcom_icc_provider {
>> struct qcom_icc_bcm * const *bcms;
>> size_t num_bcms;
>> struct bcm_voter *voter;
>> + struct qcom_icc_node * const *nodes;
>> + size_t num_nodes;
>> + struct regmap *regmap;
>> + struct clk_bulk_data *clks;
>> + int num_clks;
>
> Is it really necessary to store this in the global data? If I
> understand correctly, QoS is only programmed during the boot and is
> not to be reprogrammed later. Is my understanding correct?
>
qcom_icc_rpmh_configure_qos() can be invoked from outside of qcom_icc_rpmh_probe() in QuickBoot mode where *_probe does not happen.
so it will be easier to access this data if we keep it in global data when invoked outside of *_probe().
>> };
>>
>> /**
>> @@ -41,6 +53,23 @@ struct bcm_db {
>> u8 reserved;
>> };
>>
>> +/**
>> + * struct qcom_icc_qosbox - Qualcomm specific QoS config
>
> Why is it qosbox?
>
QOS box is the name used to describe the QOS policy override part of NIU HW in NOC.
>> + * @prio: priority value assigned to requests on the node
>> + * @urg_fwd: if set, master priority is used for requests.
>> + * @prio_fwd_disable: if set, master priority is ignored and NoC's default priority is used.
>> + * @num_ports: number of @ports
>> + * @port_offsets: qos register offsets
>> + */
>> +
>> +struct qcom_icc_qosbox {
>> + const u32 prio;
>> + const bool urg_fwd;
>> + const bool prio_fwd_disable;
>> + const u32 num_ports;
>> + const u32 port_offsets[] __counted_by(num_ports);
>> +};
>> +
>> #define MAX_LINKS 128
>> #define MAX_BCMS 64
>> #define MAX_BCM_PER_NODE 3
>> @@ -58,6 +87,7 @@ struct bcm_db {
>> * @max_peak: current max aggregate value of all peak bw requests
>> * @bcms: list of bcms associated with this logical node
>> * @num_bcms: num of @bcms
>> + * @qosbox: qos config data associated with node
>> */
>> struct qcom_icc_node {
>> const char *name;
>> @@ -70,6 +100,7 @@ struct qcom_icc_node {
>> u64 max_peak[QCOM_ICC_NUM_BUCKETS];
>> struct qcom_icc_bcm *bcms[MAX_BCM_PER_NODE];
>> size_t num_bcms;
>> + const struct qcom_icc_qosbox *qosbox;
>> };
>>
>> /**
>> @@ -114,6 +145,7 @@ struct qcom_icc_fabric {
>> };
>>
>> struct qcom_icc_desc {
>> + const struct regmap_config *config;
>> struct qcom_icc_node * const *nodes;
>> size_t num_nodes;
>> struct qcom_icc_bcm * const *bcms;
>> --
>> 2.17.1
>>
>>
>
>
Thanks,
Odelu
On 2/5/2024 8:42 PM, Krzysztof Kozlowski wrote:
> On 05/02/2024 15:56, Odelu Kukatla wrote:
>> Enable QoS configuration for the master ports with predefined values
>> for priority and urgency.
>>
>> Signed-off-by: Odelu Kukatla <[email protected]>
>> ---
>> drivers/interconnect/qcom/sc7280.c | 332 +++++++++++++++++++++++++++++
>> 1 file changed, 332 insertions(+)
>>
>> diff --git a/drivers/interconnect/qcom/sc7280.c b/drivers/interconnect/qcom/sc7280.c
>> index 7d33694368e8..438f927935e5 100644
>> --- a/drivers/interconnect/qcom/sc7280.c
>> +++ b/drivers/interconnect/qcom/sc7280.c
>> @@ -1,6 +1,7 @@
>> // SPDX-License-Identifier: GPL-2.0
>> /*
>> * Copyright (c) 2021, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2024 Qualcomm Innovation Center, Inc. All rights reserved.
>> *
>> */
>>
>> @@ -16,29 +17,53 @@
>> #include "icc-rpmh.h"
>> #include "sc7280.h"
>>
>> +static struct qcom_icc_qosbox qhm_qspi_qos = {
>
> Why this cannot be const?
>
I am addressing this in V3.
>> + .num_ports = 1,
>> + .port_offsets = { 0x7000 },
>> + .prio = 2,
>> + .urg_fwd = 0,
>
>
> Best regards,
> Krzysztof
>
Thanks,
Odelu
On 2/5/2024 8:40 PM, Krzysztof Kozlowski wrote:
> On 05/02/2024 15:56, Odelu Kukatla wrote:
>> Added clock property to enable clocks required for accessing
>> qos registers.
>
> I have no idea how you came up with that CC list. It makes no sense.
>
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.
>
> Tools like b4 or scripts/get_maintainer.pl provide you proper list of
> people, so fix your workflow. Tools might also fail if you work on some
> ancient tree (don't, instead use mainline), work on fork of kernel
> (don't, instead use mainline) or you ignore some maintainers (really
> don't). Just use b4 and everything should be fine, although remember
> about `b4 prep --auto-to-cc` if you added new patches to the patchset.
>
> You missed at least devicetree list (maybe more), so this won't be
> tested by automated tooling. Performing review on untested code might be
> a waste of time, thus I will skip this patch entirely till you follow
> the process allowing the patch to be tested.
>
> Please kindly resend and include all necessary To/Cc entries. Tomorrow.
>
> Best regards,
> Krzysztof
>
Thanks for your comments. I used latest tree only, but looks like i made a mistake while sending it.
I will use scripts/get_maintainer.pl script to get the list for next revision with other's comments addressed.
Thanks,
Odelu
On 2/6/2024 2:37 PM, Konrad Dybcio wrote:
>
>
> On 2/5/24 15:56, Odelu Kukatla wrote:
>> This series adds QoS support for QNOC type device which can be found on
>> SC7280 platform. It adds support for programming priority,
>> priority forward disable and urgency forwarding. This helps in
>> priortizing the traffic originating from different interconnect masters
>> at NOC(Network On Chip).
>>
>> Changes in v2:
>> - Updated regmap_update to make use GENMASK and FIELD_PREP.
>> - Removed the regmap structure from qcom_icc_node.
>> - Made qcom_icc_rpmh_configure_qos() static
>> - Removed qcom_icc_rpmh_map() API, inlined the code in probe
>> function.
>> - Updated declarations to reverse christmas tree fashion.
>
> You ignored some of my previous review comments without a response.
>
> Konrad
Thanks Konrad! i replied to all of your comments.
I will send V3 with all the comments addressed.
Thanks,
Odelu