Add Interconnect framework support to dynamically set the DRAM
bandwidth from different clients. Both the MC and EMC drivers are
added as ICC providers. The path for any request is:
MC-Client[1-n] -> MC -> EMC -> EMEM/DRAM
MC client's request for bandwidth will go to the MC driver which
passes the client request info like BPMP Client ID, Client type
and the Bandwidth to the BPMP-FW. The final DRAM freq to achieve
the requested bandwidth is set by the BPMP-FW based on the passed
parameters.
Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
---
drivers/memory/tegra/mc.c | 5 +
drivers/memory/tegra/tegra186-emc.c | 125 ++++++++++++++++++++++++
drivers/memory/tegra/tegra186.c | 3 +
drivers/memory/tegra/tegra234.c | 143 +++++++++++++++++++++++++++-
include/linux/tegra-icc.h | 65 +++++++++++++
include/soc/tegra/mc.h | 7 ++
6 files changed, 347 insertions(+), 1 deletion(-)
create mode 100644 include/linux/tegra-icc.h
diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index 9082b6c3763d..983455b1f98d 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -15,6 +15,7 @@
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/sort.h>
+#include <linux/tegra-icc.h>
#include <soc/tegra/fuse.h>
@@ -792,6 +793,8 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
mc->provider.data = &mc->provider;
mc->provider.set = mc->soc->icc_ops->set;
mc->provider.aggregate = mc->soc->icc_ops->aggregate;
+ mc->provider.get_bw = mc->soc->icc_ops->get_bw;
+ mc->provider.xlate = mc->soc->icc_ops->xlate;
mc->provider.xlate_extended = mc->soc->icc_ops->xlate_extended;
icc_provider_init(&mc->provider);
@@ -824,6 +827,8 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
err = icc_link_create(node, TEGRA_ICC_MC);
if (err)
goto remove_nodes;
+
+ node->data = (struct tegra_mc_client *)&(mc->soc->clients[i]);
}
err = icc_provider_register(&mc->provider);
diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
index e935ad4e95b6..1eefcf2ac0c7 100644
--- a/drivers/memory/tegra/tegra186-emc.c
+++ b/drivers/memory/tegra/tegra186-emc.c
@@ -7,9 +7,11 @@
#include <linux/debugfs.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
+#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <soc/tegra/bpmp.h>
+#include "mc.h"
struct tegra186_emc_dvfs {
unsigned long latency;
@@ -29,8 +31,15 @@ struct tegra186_emc {
unsigned long min_rate;
unsigned long max_rate;
} debugfs;
+
+ struct icc_provider provider;
};
+static inline struct tegra186_emc *to_tegra186_emc(struct icc_provider *provider)
+{
+ return container_of(provider, struct tegra186_emc, provider);
+}
+
/*
* debugfs interface
*
@@ -146,11 +155,104 @@ DEFINE_DEBUGFS_ATTRIBUTE(tegra186_emc_debug_max_rate_fops,
tegra186_emc_debug_max_rate_get,
tegra186_emc_debug_max_rate_set, "%llu\n");
+/*
+ * tegra_emc_icc_set_bw() - Set BW api for EMC provider
+ * @src: ICC node for External Memory Controller (EMC)
+ * @dst: ICC node for External Memory (DRAM)
+ *
+ * Do nothing here as info to BPMP-FW is now passed in the BW set function
+ * of the MC driver. BPMP-FW sets the final Freq based on the passed values.
+ */
+static int tegra_emc_icc_set_bw(struct icc_node *src, struct icc_node *dst)
+{
+ return 0;
+}
+
+static struct icc_node *
+tegra_emc_of_icc_xlate(struct of_phandle_args *spec, void *data)
+{
+ struct icc_provider *provider = data;
+ struct icc_node *node;
+
+ /* External Memory is the only possible ICC route */
+ list_for_each_entry(node, &provider->nodes, node_list) {
+ if (node->id != TEGRA_ICC_EMEM)
+ continue;
+
+ return node;
+ }
+
+ return ERR_PTR(-EPROBE_DEFER);
+}
+
+static int tegra_emc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peak)
+{
+ *avg = 0;
+ *peak = 0;
+
+ return 0;
+}
+
+static int tegra_emc_interconnect_init(struct tegra186_emc *emc)
+{
+ struct tegra_mc *mc = dev_get_drvdata(emc->dev->parent);
+ const struct tegra_mc_soc *soc = mc->soc;
+ struct icc_node *node;
+ int err;
+
+ emc->provider.dev = emc->dev;
+ emc->provider.set = tegra_emc_icc_set_bw;
+ emc->provider.data = &emc->provider;
+ emc->provider.aggregate = soc->icc_ops->aggregate;
+ emc->provider.xlate = tegra_emc_of_icc_xlate;
+ emc->provider.get_bw = tegra_emc_icc_get_init_bw;
+
+ icc_provider_init(&emc->provider);
+
+ /* create External Memory Controller node */
+ node = icc_node_create(TEGRA_ICC_EMC);
+ if (IS_ERR(node)) {
+ err = PTR_ERR(node);
+ goto err_msg;
+ }
+
+ node->name = "External Memory Controller";
+ icc_node_add(node, &emc->provider);
+
+ /* link External Memory Controller to External Memory (DRAM) */
+ err = icc_link_create(node, TEGRA_ICC_EMEM);
+ if (err)
+ goto remove_nodes;
+
+ /* create External Memory node */
+ node = icc_node_create(TEGRA_ICC_EMEM);
+ if (IS_ERR(node)) {
+ err = PTR_ERR(node);
+ goto remove_nodes;
+ }
+
+ node->name = "External Memory (DRAM)";
+ icc_node_add(node, &emc->provider);
+
+ err = icc_provider_register(&emc->provider);
+ if (err)
+ goto remove_nodes;
+
+ return 0;
+remove_nodes:
+ icc_nodes_remove(&emc->provider);
+err_msg:
+ dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
+
+ return err;
+}
+
static int tegra186_emc_probe(struct platform_device *pdev)
{
struct mrq_emc_dvfs_latency_response response;
struct tegra_bpmp_message msg;
struct tegra186_emc *emc;
+ struct tegra_mc *mc;
unsigned int i;
int err;
@@ -158,6 +260,9 @@ static int tegra186_emc_probe(struct platform_device *pdev)
if (!emc)
return -ENOMEM;
+ platform_set_drvdata(pdev, emc);
+ emc->dev = &pdev->dev;
+
emc->bpmp = tegra_bpmp_get(&pdev->dev);
if (IS_ERR(emc->bpmp))
return dev_err_probe(&pdev->dev, PTR_ERR(emc->bpmp), "failed to get BPMP\n");
@@ -236,6 +341,25 @@ static int tegra186_emc_probe(struct platform_device *pdev)
debugfs_create_file("max_rate", S_IRUGO | S_IWUSR, emc->debugfs.root,
emc, &tegra186_emc_debug_max_rate_fops);
+ mc = dev_get_drvdata(emc->dev->parent);
+ if (mc && mc->soc->icc_ops) {
+ /*
+ * Initialize the ICC even if BPMP-FW doesn't support 'MRQ_BWMGR_INT'.
+ * Use the flag 'mc->bwmgr_mrq_supported' within MC driver and return
+ * EINVAL instead of passing the request to BPMP-FW later when the BW
+ * request is made by client with 'icc_set_bw()' call.
+ */
+ err = tegra_emc_interconnect_init(emc);
+ if (err)
+ goto put_bpmp;
+
+ if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_BWMGR_INT))
+ mc->bwmgr_mrq_supported = true;
+ else
+
+ dev_info(&pdev->dev, "MRQ_BWMGR_INT not present\n");
+ }
+
return 0;
put_bpmp:
@@ -272,6 +396,7 @@ static struct platform_driver tegra186_emc_driver = {
.name = "tegra186-emc",
.of_match_table = tegra186_emc_of_match,
.suppress_bind_attrs = true,
+ .sync_state = icc_sync_state,
},
.probe = tegra186_emc_probe,
.remove = tegra186_emc_remove,
diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
index 7bb73f06fad3..386e029e41bb 100644
--- a/drivers/memory/tegra/tegra186.c
+++ b/drivers/memory/tegra/tegra186.c
@@ -10,6 +10,7 @@
#include <linux/of_device.h>
#include <linux/platform_device.h>
+#include <soc/tegra/bpmp.h>
#include <soc/tegra/mc.h>
#if defined(CONFIG_ARCH_TEGRA_186_SOC)
@@ -65,6 +66,8 @@ static int tegra186_mc_probe(struct tegra_mc *mc)
static void tegra186_mc_remove(struct tegra_mc *mc)
{
of_platform_depopulate(mc->dev);
+
+ tegra_bpmp_put(mc->bpmp);
}
#if IS_ENABLED(CONFIG_IOMMU_API)
diff --git a/drivers/memory/tegra/tegra234.c b/drivers/memory/tegra/tegra234.c
index 02dcc5748bba..4f34247c9bda 100644
--- a/drivers/memory/tegra/tegra234.c
+++ b/drivers/memory/tegra/tegra234.c
@@ -1,18 +1,24 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Copyright (C) 2021-2022, NVIDIA CORPORATION. All rights reserved.
+ * Copyright (C) 20212-2023, NVIDIA CORPORATION. All rights reserved.
*/
#include <soc/tegra/mc.h>
#include <dt-bindings/memory/tegra234-mc.h>
+#include <linux/interconnect.h>
+#include <linux/of_device.h>
+#include <linux/tegra-icc.h>
+#include <soc/tegra/bpmp.h>
#include "mc.h"
static const struct tegra_mc_client tegra234_mc_clients[] = {
{
.id = TEGRA234_MEMORY_CLIENT_MGBEARD,
.name = "mgbeard",
+ .bpmp_id = TEGRA_ICC_BPMP_EQOS,
+ .type = TEGRA_ICC_NISO,
.sid = TEGRA234_SID_MGBE,
.regs = {
.sid = {
@@ -23,6 +29,8 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
}, {
.id = TEGRA234_MEMORY_CLIENT_MGBEBRD,
.name = "mgbebrd",
+ .bpmp_id = TEGRA_ICC_BPMP_EQOS,
+ .type = TEGRA_ICC_NISO,
.sid = TEGRA234_SID_MGBE_VF1,
.regs = {
.sid = {
@@ -33,6 +41,8 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
}, {
.id = TEGRA234_MEMORY_CLIENT_MGBECRD,
.name = "mgbecrd",
+ .bpmp_id = TEGRA_ICC_BPMP_EQOS,
+ .type = TEGRA_ICC_NISO,
.sid = TEGRA234_SID_MGBE_VF2,
.regs = {
.sid = {
@@ -43,6 +53,8 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
}, {
.id = TEGRA234_MEMORY_CLIENT_MGBEDRD,
.name = "mgbedrd",
+ .bpmp_id = TEGRA_ICC_BPMP_EQOS,
+ .type = TEGRA_ICC_NISO,
.sid = TEGRA234_SID_MGBE_VF3,
.regs = {
.sid = {
@@ -52,6 +64,8 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
},
}, {
.id = TEGRA234_MEMORY_CLIENT_MGBEAWR,
+ .bpmp_id = TEGRA_ICC_BPMP_EQOS,
+ .type = TEGRA_ICC_NISO,
.name = "mgbeawr",
.sid = TEGRA234_SID_MGBE,
.regs = {
@@ -63,6 +77,8 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
}, {
.id = TEGRA234_MEMORY_CLIENT_MGBEBWR,
.name = "mgbebwr",
+ .bpmp_id = TEGRA_ICC_BPMP_EQOS,
+ .type = TEGRA_ICC_NISO,
.sid = TEGRA234_SID_MGBE_VF1,
.regs = {
.sid = {
@@ -73,6 +89,8 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
}, {
.id = TEGRA234_MEMORY_CLIENT_MGBECWR,
.name = "mgbecwr",
+ .bpmp_id = TEGRA_ICC_BPMP_EQOS,
+ .type = TEGRA_ICC_NISO,
.sid = TEGRA234_SID_MGBE_VF2,
.regs = {
.sid = {
@@ -83,6 +101,8 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
}, {
.id = TEGRA234_MEMORY_CLIENT_SDMMCRAB,
.name = "sdmmcrab",
+ .bpmp_id = TEGRA_ICC_BPMP_SDMMC_4,
+ .type = TEGRA_ICC_NISO,
.sid = TEGRA234_SID_SDMMC4,
.regs = {
.sid = {
@@ -93,6 +113,8 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
}, {
.id = TEGRA234_MEMORY_CLIENT_MGBEDWR,
.name = "mgbedwr",
+ .bpmp_id = TEGRA_ICC_BPMP_EQOS,
+ .type = TEGRA_ICC_NISO,
.sid = TEGRA234_SID_MGBE_VF3,
.regs = {
.sid = {
@@ -103,6 +125,8 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
}, {
.id = TEGRA234_MEMORY_CLIENT_SDMMCWAB,
.name = "sdmmcwab",
+ .bpmp_id = TEGRA_ICC_BPMP_SDMMC_4,
+ .type = TEGRA_ICC_NISO,
.sid = TEGRA234_SID_SDMMC4,
.regs = {
.sid = {
@@ -153,6 +177,8 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
}, {
.id = TEGRA234_MEMORY_CLIENT_APEDMAR,
.name = "apedmar",
+ .bpmp_id = TEGRA_ICC_BPMP_APEDMA,
+ .type = TEGRA_ICC_ISO_AUDIO,
.sid = TEGRA234_SID_APE,
.regs = {
.sid = {
@@ -163,6 +189,8 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
}, {
.id = TEGRA234_MEMORY_CLIENT_APEDMAW,
.name = "apedmaw",
+ .bpmp_id = TEGRA_ICC_BPMP_APEDMA,
+ .type = TEGRA_ICC_ISO_AUDIO,
.sid = TEGRA234_SID_APE,
.regs = {
.sid = {
@@ -333,6 +361,118 @@ static const struct tegra_mc_client tegra234_mc_clients[] = {
},
};
+/*
+ * tegra234_mc_icc_set() - Pass MC client info to the BPMP-FW
+ * @src: ICC node for Memory Controller's (MC) Client
+ * @dst: ICC node for Memory Controller (MC)
+ *
+ * Passing the current request info from the MC to the BPMP-FW where
+ * LA and PTSA registers are accessed and the final EMC freq is set
+ * based on client_id, type, latency and bandwidth.
+ * icc_set_bw() makes set_bw calls for both MC and EMC providers in
+ * sequence. Both the calls are protected by 'mutex_lock(&icc_lock)'.
+ * So, the data passed won't be updated by concurrent set calls from
+ * other clients.
+ */
+static int tegra234_mc_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+ struct tegra_mc *mc = icc_provider_to_tegra_mc(dst->provider);
+ struct mrq_bwmgr_int_request bwmgr_req = { 0 };
+ struct mrq_bwmgr_int_response bwmgr_resp = { 0 };
+ const struct tegra_mc_client *pclient = src->data;
+ struct tegra_bpmp_message msg;
+ int ret;
+
+ /*
+ * Same Src and Dst node will happen during boot from icc_node_add().
+ * This can be used to pre-initialize and set bandwidth for all clients
+ * before their drivers are loaded. We are skipping this case as for us,
+ * the pre-initialization already happened in Bootloader(MB2) and BPMP-FW.
+ */
+ if (src->id == dst->id)
+ return 0;
+
+ if (!mc->bwmgr_mrq_supported)
+ return -EINVAL;
+
+ if (!mc->bpmp) {
+ mc->bpmp = tegra_bpmp_get(mc->dev);
+ if (IS_ERR(mc->bpmp)) {
+ ret = PTR_ERR(mc->bpmp);
+ dev_err(mc->dev, "failed to get BPMP: %d\n", ret);
+ return ret;
+ }
+ }
+
+ if (pclient->type == TEGRA_ICC_NISO)
+ bwmgr_req.bwmgr_calc_set_req.niso_bw = src->avg_bw;
+ else
+ bwmgr_req.bwmgr_calc_set_req.iso_bw = src->avg_bw;
+
+ bwmgr_req.bwmgr_calc_set_req.client_id = pclient->bpmp_id;
+
+ bwmgr_req.cmd = CMD_BWMGR_INT_CALC_AND_SET;
+ bwmgr_req.bwmgr_calc_set_req.mc_floor = src->peak_bw;
+ bwmgr_req.bwmgr_calc_set_req.floor_unit = BWMGR_INT_UNIT_KBPS;
+
+ memset(&msg, 0, sizeof(msg));
+ msg.mrq = MRQ_BWMGR_INT;
+ msg.tx.data = &bwmgr_req;
+ msg.tx.size = sizeof(bwmgr_req);
+ msg.rx.data = &bwmgr_resp;
+ msg.rx.size = sizeof(bwmgr_resp);
+
+ ret = tegra_bpmp_transfer(mc->bpmp, &msg);
+ if (ret < 0) {
+ dev_err(mc->dev, "BPMP transfer failed: %d\n", ret);
+ goto error;
+ }
+ if (msg.rx.ret < 0) {
+ pr_err("failed to set bandwidth for %u: %d\n",
+ bwmgr_req.bwmgr_calc_set_req.client_id, msg.rx.ret);
+ ret = -EINVAL;
+ }
+
+error:
+ return ret;
+}
+
+static struct icc_node*
+tegra234_mc_of_icc_xlate(struct of_phandle_args *spec, void *data)
+{
+ struct tegra_mc *mc = icc_provider_to_tegra_mc(data);
+ unsigned int cl_id = spec->args[0];
+ struct icc_node *node;
+
+ list_for_each_entry(node, &mc->provider.nodes, node_list) {
+ if (node->id != cl_id)
+ continue;
+
+ return node;
+ }
+
+ /*
+ * If a client driver calls devm_of_icc_get() before the MC driver
+ * is probed, then return EPROBE_DEFER to the client driver.
+ */
+ return ERR_PTR(-EPROBE_DEFER);
+}
+
+static int tegra234_mc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peak)
+{
+ *avg = 0;
+ *peak = 0;
+
+ return 0;
+}
+
+static const struct tegra_mc_icc_ops tegra234_mc_icc_ops = {
+ .xlate = tegra234_mc_of_icc_xlate,
+ .aggregate = icc_std_aggregate,
+ .get_bw = tegra234_mc_icc_get_init_bw,
+ .set = tegra234_mc_icc_set,
+};
+
const struct tegra_mc_soc tegra234_mc_soc = {
.num_clients = ARRAY_SIZE(tegra234_mc_clients),
.clients = tegra234_mc_clients,
@@ -345,6 +485,7 @@ const struct tegra_mc_soc tegra234_mc_soc = {
MC_INT_SECURITY_VIOLATION | MC_INT_DECERR_EMEM,
.has_addr_hi_reg = true,
.ops = &tegra186_mc_ops,
+ .icc_ops = &tegra234_mc_icc_ops,
.ch_intmask = 0x0000ff00,
.global_intstatus_channel_shift = 8,
/*
diff --git a/include/linux/tegra-icc.h b/include/linux/tegra-icc.h
new file mode 100644
index 000000000000..4b4d4bee290c
--- /dev/null
+++ b/include/linux/tegra-icc.h
@@ -0,0 +1,65 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2022-2023 NVIDIA CORPORATION. All rights reserved.
+ */
+
+#ifndef LINUX_TEGRA_ICC_H
+#define LINUX_TEGRA_ICC_H
+
+enum tegra_icc_client_type {
+ TEGRA_ICC_NONE,
+ TEGRA_ICC_NISO,
+ TEGRA_ICC_ISO_DISPLAY,
+ TEGRA_ICC_ISO_VI,
+ TEGRA_ICC_ISO_AUDIO,
+ TEGRA_ICC_ISO_VIFAL,
+};
+
+/* ICC ID's for MC client's used in BPMP */
+#define TEGRA_ICC_BPMP_DEBUG 1
+#define TEGRA_ICC_BPMP_CPU_CLUSTER0 2
+#define TEGRA_ICC_BPMP_CPU_CLUSTER1 3
+#define TEGRA_ICC_BPMP_CPU_CLUSTER2 4
+#define TEGRA_ICC_BPMP_GPU 5
+#define TEGRA_ICC_BPMP_CACTMON 6
+#define TEGRA_ICC_BPMP_DISPLAY 7
+#define TEGRA_ICC_BPMP_VI 8
+#define TEGRA_ICC_BPMP_EQOS 9
+#define TEGRA_ICC_BPMP_PCIE_0 10
+#define TEGRA_ICC_BPMP_PCIE_1 11
+#define TEGRA_ICC_BPMP_PCIE_2 12
+#define TEGRA_ICC_BPMP_PCIE_3 13
+#define TEGRA_ICC_BPMP_PCIE_4 14
+#define TEGRA_ICC_BPMP_PCIE_5 15
+#define TEGRA_ICC_BPMP_PCIE_6 16
+#define TEGRA_ICC_BPMP_PCIE_7 17
+#define TEGRA_ICC_BPMP_PCIE_8 18
+#define TEGRA_ICC_BPMP_PCIE_9 19
+#define TEGRA_ICC_BPMP_PCIE_10 20
+#define TEGRA_ICC_BPMP_DLA_0 21
+#define TEGRA_ICC_BPMP_DLA_1 22
+#define TEGRA_ICC_BPMP_SDMMC_1 23
+#define TEGRA_ICC_BPMP_SDMMC_2 24
+#define TEGRA_ICC_BPMP_SDMMC_3 25
+#define TEGRA_ICC_BPMP_SDMMC_4 26
+#define TEGRA_ICC_BPMP_NVDEC 27
+#define TEGRA_ICC_BPMP_NVENC 28
+#define TEGRA_ICC_BPMP_NVJPG_0 29
+#define TEGRA_ICC_BPMP_NVJPG_1 30
+#define TEGRA_ICC_BPMP_OFAA 31
+#define TEGRA_ICC_BPMP_XUSB_HOST 32
+#define TEGRA_ICC_BPMP_XUSB_DEV 33
+#define TEGRA_ICC_BPMP_TSEC 34
+#define TEGRA_ICC_BPMP_VIC 35
+#define TEGRA_ICC_BPMP_APE 36
+#define TEGRA_ICC_BPMP_APEDMA 37
+#define TEGRA_ICC_BPMP_SE 38
+#define TEGRA_ICC_BPMP_ISP 39
+#define TEGRA_ICC_BPMP_HDA 40
+#define TEGRA_ICC_BPMP_VIFAL 41
+#define TEGRA_ICC_BPMP_VI2FAL 42
+#define TEGRA_ICC_BPMP_VI2 43
+#define TEGRA_ICC_BPMP_RCE 44
+#define TEGRA_ICC_BPMP_PVA 45
+
+#endif /* LINUX_TEGRA_ICC_H */
diff --git a/include/soc/tegra/mc.h b/include/soc/tegra/mc.h
index 51a2263e1bc5..900d88b26fae 100644
--- a/include/soc/tegra/mc.h
+++ b/include/soc/tegra/mc.h
@@ -13,6 +13,7 @@
#include <linux/irq.h>
#include <linux/reset-controller.h>
#include <linux/types.h>
+#include <linux/tegra-icc.h>
struct clk;
struct device;
@@ -26,6 +27,8 @@ struct tegra_mc_timing {
struct tegra_mc_client {
unsigned int id;
+ unsigned int bpmp_id;
+ enum tegra_icc_client_type type;
const char *name;
/*
* For Tegra210 and earlier, this is the SWGROUP ID used for IOVA translations in the
@@ -166,8 +169,10 @@ struct tegra_mc_icc_ops {
int (*set)(struct icc_node *src, struct icc_node *dst);
int (*aggregate)(struct icc_node *node, u32 tag, u32 avg_bw,
u32 peak_bw, u32 *agg_avg, u32 *agg_peak);
+ struct icc_node* (*xlate)(struct of_phandle_args *spec, void *data);
struct icc_node_data *(*xlate_extended)(struct of_phandle_args *spec,
void *data);
+ int (*get_bw)(struct icc_node *node, u32 *avg, u32 *peak);
};
struct tegra_mc_ops {
@@ -214,6 +219,7 @@ struct tegra_mc_soc {
};
struct tegra_mc {
+ struct tegra_bpmp *bpmp;
struct device *dev;
struct tegra_smmu *smmu;
struct gart_device *gart;
@@ -229,6 +235,7 @@ struct tegra_mc {
struct tegra_mc_timing *timings;
unsigned int num_timings;
+ bool bwmgr_mrq_supported;
struct reset_controller_dev reset;
struct icc_provider provider;
--
2.17.1On 27/03/2023 18:14, Sumit Gupta wrote:
> Add Interconnect framework support to dynamically set the DRAM
> bandwidth from different clients. Both the MC and EMC drivers are
> added as ICC providers. The path for any request is:
> MC-Client[1-n] -> MC -> EMC -> EMEM/DRAM
>
> MC client's request for bandwidth will go to the MC driver which
> passes the client request info like BPMP Client ID, Client type
> and the Bandwidth to the BPMP-FW. The final DRAM freq to achieve
> the requested bandwidth is set by the BPMP-FW based on the passed
> parameters.
>
> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
> ---
> drivers/memory/tegra/mc.c | 5 +
> drivers/memory/tegra/tegra186-emc.c | 125 ++++++++++++++++++++++++
> drivers/memory/tegra/tegra186.c | 3 +
> drivers/memory/tegra/tegra234.c | 143 +++++++++++++++++++++++++++-
> include/linux/tegra-icc.h | 65 +++++++++++++
> include/soc/tegra/mc.h | 7 ++
> 6 files changed, 347 insertions(+), 1 deletion(-)
> create mode 100644 include/linux/tegra-icc.h
>
> diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
> index 9082b6c3763d..983455b1f98d 100644
> --- a/drivers/memory/tegra/mc.c
> +++ b/drivers/memory/tegra/mc.c
> @@ -15,6 +15,7 @@
> #include <linux/platform_device.h>
> #include <linux/slab.h>
> #include <linux/sort.h>
> +#include <linux/tegra-icc.h>
>
> #include <soc/tegra/fuse.h>
>
> @@ -792,6 +793,8 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
> mc->provider.data = &mc->provider;
> mc->provider.set = mc->soc->icc_ops->set;
> mc->provider.aggregate = mc->soc->icc_ops->aggregate;
> + mc->provider.get_bw = mc->soc->icc_ops->get_bw;
> + mc->provider.xlate = mc->soc->icc_ops->xlate;
> mc->provider.xlate_extended = mc->soc->icc_ops->xlate_extended;
>
> icc_provider_init(&mc->provider);
> @@ -824,6 +827,8 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
> err = icc_link_create(node, TEGRA_ICC_MC);
> if (err)
> goto remove_nodes;
> +
> + node->data = (struct tegra_mc_client *)&(mc->soc->clients[i]);
> }
>
> err = icc_provider_register(&mc->provider);
> diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
> index e935ad4e95b6..1eefcf2ac0c7 100644
> --- a/drivers/memory/tegra/tegra186-emc.c
> +++ b/drivers/memory/tegra/tegra186-emc.c
> @@ -7,9 +7,11 @@
> #include <linux/debugfs.h>
> #include <linux/module.h>
> #include <linux/mod_devicetable.h>
> +#include <linux/of_platform.h>
> #include <linux/platform_device.h>
>
> #include <soc/tegra/bpmp.h>
> +#include "mc.h"
>
> struct tegra186_emc_dvfs {
> unsigned long latency;
> @@ -29,8 +31,15 @@ struct tegra186_emc {
> unsigned long min_rate;
> unsigned long max_rate;
> } debugfs;
> +
> + struct icc_provider provider;
> };
>
> +static inline struct tegra186_emc *to_tegra186_emc(struct icc_provider *provider)
> +{
> + return container_of(provider, struct tegra186_emc, provider);
> +}
> +
> /*
> * debugfs interface
> *
> @@ -146,11 +155,104 @@ DEFINE_DEBUGFS_ATTRIBUTE(tegra186_emc_debug_max_rate_fops,
> tegra186_emc_debug_max_rate_get,
> tegra186_emc_debug_max_rate_set, "%llu\n");
>
> +/*
> + * tegra_emc_icc_set_bw() - Set BW api for EMC provider
> + * @src: ICC node for External Memory Controller (EMC)
> + * @dst: ICC node for External Memory (DRAM)
> + *
> + * Do nothing here as info to BPMP-FW is now passed in the BW set function
> + * of the MC driver. BPMP-FW sets the final Freq based on the passed values.
> + */
> +static int tegra_emc_icc_set_bw(struct icc_node *src, struct icc_node *dst)
> +{
> + return 0;
> +}
> +
> +static struct icc_node *
> +tegra_emc_of_icc_xlate(struct of_phandle_args *spec, void *data)
> +{
> + struct icc_provider *provider = data;
> + struct icc_node *node;
> +
> + /* External Memory is the only possible ICC route */
> + list_for_each_entry(node, &provider->nodes, node_list) {
> + if (node->id != TEGRA_ICC_EMEM)
> + continue;
> +
> + return node;
> + }
> +
> + return ERR_PTR(-EPROBE_DEFER);
> +}
> +
> +static int tegra_emc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peak)
> +{
> + *avg = 0;
> + *peak = 0;
> +
> + return 0;
> +}
> +
> +static int tegra_emc_interconnect_init(struct tegra186_emc *emc)
> +{
> + struct tegra_mc *mc = dev_get_drvdata(emc->dev->parent);
> + const struct tegra_mc_soc *soc = mc->soc;
> + struct icc_node *node;
> + int err;
> +
> + emc->provider.dev = emc->dev;
> + emc->provider.set = tegra_emc_icc_set_bw;
> + emc->provider.data = &emc->provider;
> + emc->provider.aggregate = soc->icc_ops->aggregate;
> + emc->provider.xlate = tegra_emc_of_icc_xlate;
> + emc->provider.get_bw = tegra_emc_icc_get_init_bw;
> +
> + icc_provider_init(&emc->provider);
> +
> + /* create External Memory Controller node */
> + node = icc_node_create(TEGRA_ICC_EMC);
> + if (IS_ERR(node)) {
> + err = PTR_ERR(node);
> + goto err_msg;
> + }
> +
> + node->name = "External Memory Controller";
> + icc_node_add(node, &emc->provider);
> +
> + /* link External Memory Controller to External Memory (DRAM) */
> + err = icc_link_create(node, TEGRA_ICC_EMEM);
> + if (err)
> + goto remove_nodes;
> +
> + /* create External Memory node */
> + node = icc_node_create(TEGRA_ICC_EMEM);
> + if (IS_ERR(node)) {
> + err = PTR_ERR(node);
> + goto remove_nodes;
> + }
> +
> + node->name = "External Memory (DRAM)";
> + icc_node_add(node, &emc->provider);
> +
> + err = icc_provider_register(&emc->provider);
> + if (err)
> + goto remove_nodes;
> +
> + return 0;
Blank line
> +remove_nodes:
> + icc_nodes_remove(&emc->provider);
> +err_msg:
> + dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
> +
> + return err;
> +}
> +
> static int tegra186_emc_probe(struct platform_device *pdev)
> {
> struct mrq_emc_dvfs_latency_response response;
> struct tegra_bpmp_message msg;
> struct tegra186_emc *emc;
> + struct tegra_mc *mc;
> unsigned int i;
> int err;
>
> @@ -158,6 +260,9 @@ static int tegra186_emc_probe(struct platform_device *pdev)
> if (!emc)
> return -ENOMEM;
>
> + platform_set_drvdata(pdev, emc);
> + emc->dev = &pdev->dev;
This patch looks like stiched from two or more patches... emc->dev does
not look like new member of emc, thus why do you set in exisitng
function in this patch? Why it wasn't needed before?
Same about line before.
> +
> emc->bpmp = tegra_bpmp_get(&pdev->dev);
> if (IS_ERR(emc->bpmp))
> return dev_err_probe(&pdev->dev, PTR_ERR(emc->bpmp), "failed to get BPMP\n");
> @@ -236,6 +341,25 @@ static int tegra186_emc_probe(struct platform_device *pdev)
> debugfs_create_file("max_rate", S_IRUGO | S_IWUSR, emc->debugfs.root,
> emc, &tegra186_emc_debug_max_rate_fops);
>
> + mc = dev_get_drvdata(emc->dev->parent);
> + if (mc && mc->soc->icc_ops) {
> + /*
> + * Initialize the ICC even if BPMP-FW doesn't support 'MRQ_BWMGR_INT'.
> + * Use the flag 'mc->bwmgr_mrq_supported' within MC driver and return
> + * EINVAL instead of passing the request to BPMP-FW later when the BW
> + * request is made by client with 'icc_set_bw()' call.
> + */
> + err = tegra_emc_interconnect_init(emc);
> + if (err)
> + goto put_bpmp;
> +
> + if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_BWMGR_INT))
> + mc->bwmgr_mrq_supported = true;
> + else
> +
Drop blank line.
> + dev_info(&pdev->dev, "MRQ_BWMGR_INT not present\n");
And what user is supposed to do with this? Either make it descriptive or
drop.
> + }
> +
> return 0;
>
> put_bpmp:
> @@ -272,6 +396,7 @@ static struct platform_driver tegra186_emc_driver = {
> .name = "tegra186-emc",
> .of_match_table = tegra186_emc_of_match,
> .suppress_bind_attrs = true,
> + .sync_state = icc_sync_state,
> },
> .probe = tegra186_emc_probe,
> .remove = tegra186_emc_remove,
> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
> index 7bb73f06fad3..386e029e41bb 100644
> --- a/drivers/memory/tegra/tegra186.c
> +++ b/drivers/memory/tegra/tegra186.c
> @@ -10,6 +10,7 @@
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
>
> +#include <soc/tegra/bpmp.h>
> #include <soc/tegra/mc.h>
>
> #if defined(CONFIG_ARCH_TEGRA_186_SOC)
> @@ -65,6 +66,8 @@ static int tegra186_mc_probe(struct tegra_mc *mc)
> static void tegra186_mc_remove(struct tegra_mc *mc)
> {
> of_platform_depopulate(mc->dev);
> +
> + tegra_bpmp_put(mc->bpmp);
> }
>
> #if IS_ENABLED(CONFIG_IOMMU_API)
> diff --git a/drivers/memory/tegra/tegra234.c b/drivers/memory/tegra/tegra234.c
> index 02dcc5748bba..4f34247c9bda 100644
> --- a/drivers/memory/tegra/tegra234.c
> +++ b/drivers/memory/tegra/tegra234.c
> @@ -1,18 +1,24 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /*
> - * Copyright (C) 2021-2022, NVIDIA CORPORATION. All rights reserved.
> + * Copyright (C) 20212-2023, NVIDIA CORPORATION. All rights reserved.
Typo, 2021.
> */
>
> #include <soc/tegra/mc.h>
>
> #include <dt-bindings/memory/tegra234-mc.h>
> +#include <linux/interconnect.h>
> +#include <linux/of_device.h>
One more suprising change...
> +#include <linux/tegra-icc.h>
>
> +#include <soc/tegra/bpmp.h>
> #include "mc.h"
>
> static const struct tegra_mc_client tegra234_mc_clients[] = {
> {
> .id = TEGRA234_MEMORY_CLIENT_MGBEARD,
> .name = "mgbeard",
> + .bpmp_id = TEGRA_ICC_BPMP_EQOS,
> + .type = TEGRA_ICC_NISO,
> .sid = TEGRA234_SID_MGBE,
> .regs = {
> .sid = {
Best regards,
Krzysztof
On 28/03/23 13:01, Krzysztof Kozlowski wrote:
> External email: Use caution opening links or attachments
>
>
> On 27/03/2023 18:14, Sumit Gupta wrote:
>> Add Interconnect framework support to dynamically set the DRAM
>> bandwidth from different clients. Both the MC and EMC drivers are
>> added as ICC providers. The path for any request is:
>> MC-Client[1-n] -> MC -> EMC -> EMEM/DRAM
>>
>> MC client's request for bandwidth will go to the MC driver which
>> passes the client request info like BPMP Client ID, Client type
>> and the Bandwidth to the BPMP-FW. The final DRAM freq to achieve
>> the requested bandwidth is set by the BPMP-FW based on the passed
>> parameters.
>>
>> Signed-off-by: Sumit Gupta <sumitg@nvidia.com>
>> ---
>> drivers/memory/tegra/mc.c | 5 +
>> drivers/memory/tegra/tegra186-emc.c | 125 ++++++++++++++++++++++++
>> drivers/memory/tegra/tegra186.c | 3 +
>> drivers/memory/tegra/tegra234.c | 143 +++++++++++++++++++++++++++-
>> include/linux/tegra-icc.h | 65 +++++++++++++
>> include/soc/tegra/mc.h | 7 ++
>> 6 files changed, 347 insertions(+), 1 deletion(-)
>> create mode 100644 include/linux/tegra-icc.h
>>
>> diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
>> index 9082b6c3763d..983455b1f98d 100644
>> --- a/drivers/memory/tegra/mc.c
>> +++ b/drivers/memory/tegra/mc.c
>> @@ -15,6 +15,7 @@
>> #include <linux/platform_device.h>
>> #include <linux/slab.h>
>> #include <linux/sort.h>
>> +#include <linux/tegra-icc.h>
>>
>> #include <soc/tegra/fuse.h>
>>
>> @@ -792,6 +793,8 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
>> mc->provider.data = &mc->provider;
>> mc->provider.set = mc->soc->icc_ops->set;
>> mc->provider.aggregate = mc->soc->icc_ops->aggregate;
>> + mc->provider.get_bw = mc->soc->icc_ops->get_bw;
>> + mc->provider.xlate = mc->soc->icc_ops->xlate;
>> mc->provider.xlate_extended = mc->soc->icc_ops->xlate_extended;
>>
>> icc_provider_init(&mc->provider);
>> @@ -824,6 +827,8 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
>> err = icc_link_create(node, TEGRA_ICC_MC);
>> if (err)
>> goto remove_nodes;
>> +
>> + node->data = (struct tegra_mc_client *)&(mc->soc->clients[i]);
>> }
>>
>> err = icc_provider_register(&mc->provider);
>> diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
>> index e935ad4e95b6..1eefcf2ac0c7 100644
>> --- a/drivers/memory/tegra/tegra186-emc.c
>> +++ b/drivers/memory/tegra/tegra186-emc.c
>> @@ -7,9 +7,11 @@
>> #include <linux/debugfs.h>
>> #include <linux/module.h>
>> #include <linux/mod_devicetable.h>
>> +#include <linux/of_platform.h>
>> #include <linux/platform_device.h>
>>
>> #include <soc/tegra/bpmp.h>
>> +#include "mc.h"
>>
>> struct tegra186_emc_dvfs {
>> unsigned long latency;
>> @@ -29,8 +31,15 @@ struct tegra186_emc {
>> unsigned long min_rate;
>> unsigned long max_rate;
>> } debugfs;
>> +
>> + struct icc_provider provider;
>> };
>>
>> +static inline struct tegra186_emc *to_tegra186_emc(struct icc_provider *provider)
>> +{
>> + return container_of(provider, struct tegra186_emc, provider);
>> +}
>> +
>> /*
>> * debugfs interface
>> *
>> @@ -146,11 +155,104 @@ DEFINE_DEBUGFS_ATTRIBUTE(tegra186_emc_debug_max_rate_fops,
>> tegra186_emc_debug_max_rate_get,
>> tegra186_emc_debug_max_rate_set, "%llu\n");
>>
>> +/*
>> + * tegra_emc_icc_set_bw() - Set BW api for EMC provider
>> + * @src: ICC node for External Memory Controller (EMC)
>> + * @dst: ICC node for External Memory (DRAM)
>> + *
>> + * Do nothing here as info to BPMP-FW is now passed in the BW set function
>> + * of the MC driver. BPMP-FW sets the final Freq based on the passed values.
>> + */
>> +static int tegra_emc_icc_set_bw(struct icc_node *src, struct icc_node *dst)
>> +{
>> + return 0;
>> +}
>> +
>> +static struct icc_node *
>> +tegra_emc_of_icc_xlate(struct of_phandle_args *spec, void *data)
>> +{
>> + struct icc_provider *provider = data;
>> + struct icc_node *node;
>> +
>> + /* External Memory is the only possible ICC route */
>> + list_for_each_entry(node, &provider->nodes, node_list) {
>> + if (node->id != TEGRA_ICC_EMEM)
>> + continue;
>> +
>> + return node;
>> + }
>> +
>> + return ERR_PTR(-EPROBE_DEFER);
>> +}
>> +
>> +static int tegra_emc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peak)
>> +{
>> + *avg = 0;
>> + *peak = 0;
>> +
>> + return 0;
>> +}
>> +
>> +static int tegra_emc_interconnect_init(struct tegra186_emc *emc)
>> +{
>> + struct tegra_mc *mc = dev_get_drvdata(emc->dev->parent);
>> + const struct tegra_mc_soc *soc = mc->soc;
>> + struct icc_node *node;
>> + int err;
>> +
>> + emc->provider.dev = emc->dev;
>> + emc->provider.set = tegra_emc_icc_set_bw;
>> + emc->provider.data = &emc->provider;
>> + emc->provider.aggregate = soc->icc_ops->aggregate;
>> + emc->provider.xlate = tegra_emc_of_icc_xlate;
>> + emc->provider.get_bw = tegra_emc_icc_get_init_bw;
>> +
>> + icc_provider_init(&emc->provider);
>> +
>> + /* create External Memory Controller node */
>> + node = icc_node_create(TEGRA_ICC_EMC);
>> + if (IS_ERR(node)) {
>> + err = PTR_ERR(node);
>> + goto err_msg;
>> + }
>> +
>> + node->name = "External Memory Controller";
>> + icc_node_add(node, &emc->provider);
>> +
>> + /* link External Memory Controller to External Memory (DRAM) */
>> + err = icc_link_create(node, TEGRA_ICC_EMEM);
>> + if (err)
>> + goto remove_nodes;
>> +
>> + /* create External Memory node */
>> + node = icc_node_create(TEGRA_ICC_EMEM);
>> + if (IS_ERR(node)) {
>> + err = PTR_ERR(node);
>> + goto remove_nodes;
>> + }
>> +
>> + node->name = "External Memory (DRAM)";
>> + icc_node_add(node, &emc->provider);
>> +
>> + err = icc_provider_register(&emc->provider);
>> + if (err)
>> + goto remove_nodes;
>> +
>> + return 0;
>
> Blank line
>
>> +remove_nodes:
>> + icc_nodes_remove(&emc->provider);
>> +err_msg:
>> + dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
>> +
>> + return err;
>> +}
>> +
>> static int tegra186_emc_probe(struct platform_device *pdev)
>> {
>> struct mrq_emc_dvfs_latency_response response;
>> struct tegra_bpmp_message msg;
>> struct tegra186_emc *emc;
>> + struct tegra_mc *mc;
>> unsigned int i;
>> int err;
>>
>> @@ -158,6 +260,9 @@ static int tegra186_emc_probe(struct platform_device *pdev)
>> if (!emc)
>> return -ENOMEM;
>>
>> + platform_set_drvdata(pdev, emc);
>> + emc->dev = &pdev->dev;
>
> This patch looks like stiched from two or more patches... emc->dev does
> not look like new member of emc, thus why do you set in exisitng
> function in this patch? Why it wasn't needed before?
>
> Same about line before.
>
Replied in other mail. will fix this.
>> +
>> emc->bpmp = tegra_bpmp_get(&pdev->dev);
>> if (IS_ERR(emc->bpmp))
>> return dev_err_probe(&pdev->dev, PTR_ERR(emc->bpmp), "failed to get BPMP\n");
>> @@ -236,6 +341,25 @@ static int tegra186_emc_probe(struct platform_device *pdev)
>> debugfs_create_file("max_rate", S_IRUGO | S_IWUSR, emc->debugfs.root,
>> emc, &tegra186_emc_debug_max_rate_fops);
>>
>> + mc = dev_get_drvdata(emc->dev->parent);
>> + if (mc && mc->soc->icc_ops) {
>> + /*
>> + * Initialize the ICC even if BPMP-FW doesn't support 'MRQ_BWMGR_INT'.
>> + * Use the flag 'mc->bwmgr_mrq_supported' within MC driver and return
>> + * EINVAL instead of passing the request to BPMP-FW later when the BW
>> + * request is made by client with 'icc_set_bw()' call.
>> + */
>> + err = tegra_emc_interconnect_init(emc);
>> + if (err)
>> + goto put_bpmp;
>> +
>> + if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_BWMGR_INT))
>> + mc->bwmgr_mrq_supported = true;
>> + else
>> +
>
> Drop blank line.
>
>> + dev_info(&pdev->dev, "MRQ_BWMGR_INT not present\n");
>
> And what user is supposed to do with this? Either make it descriptive or
> drop.
>
Replied in other mail. will fix this.
>> + }
>> +
>> return 0;
>>
>> put_bpmp:
>> @@ -272,6 +396,7 @@ static struct platform_driver tegra186_emc_driver = {
>> .name = "tegra186-emc",
>> .of_match_table = tegra186_emc_of_match,
>> .suppress_bind_attrs = true,
>> + .sync_state = icc_sync_state,
>> },
>> .probe = tegra186_emc_probe,
>> .remove = tegra186_emc_remove,
>> diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
>> index 7bb73f06fad3..386e029e41bb 100644
>> --- a/drivers/memory/tegra/tegra186.c
>> +++ b/drivers/memory/tegra/tegra186.c
>> @@ -10,6 +10,7 @@
>> #include <linux/of_device.h>
>> #include <linux/platform_device.h>
>>
>> +#include <soc/tegra/bpmp.h>
>> #include <soc/tegra/mc.h>
>>
>> #if defined(CONFIG_ARCH_TEGRA_186_SOC)
>> @@ -65,6 +66,8 @@ static int tegra186_mc_probe(struct tegra_mc *mc)
>> static void tegra186_mc_remove(struct tegra_mc *mc)
>> {
>> of_platform_depopulate(mc->dev);
>> +
>> + tegra_bpmp_put(mc->bpmp);
>> }
>>
>> #if IS_ENABLED(CONFIG_IOMMU_API)
>> diff --git a/drivers/memory/tegra/tegra234.c b/drivers/memory/tegra/tegra234.c
>> index 02dcc5748bba..4f34247c9bda 100644
>> --- a/drivers/memory/tegra/tegra234.c
>> +++ b/drivers/memory/tegra/tegra234.c
>> @@ -1,18 +1,24 @@
>> // SPDX-License-Identifier: GPL-2.0-only
>> /*
>> - * Copyright (C) 2021-2022, NVIDIA CORPORATION. All rights reserved.
>> + * Copyright (C) 20212-2023, NVIDIA CORPORATION. All rights reserved.
>
> Typo, 2021.
>
Will fix.
>> */
>>
>> #include <soc/tegra/mc.h>
>>
>> #include <dt-bindings/memory/tegra234-mc.h>
>> +#include <linux/interconnect.h>
>> +#include <linux/of_device.h>
>
> One more suprising change...
>
Will remove the header file "of_device.h".
>> +#include <linux/tegra-icc.h>
>>
>> +#include <soc/tegra/bpmp.h>
>> #include "mc.h"
>>
>> static const struct tegra_mc_client tegra234_mc_clients[] = {
>> {
>> .id = TEGRA234_MEMORY_CLIENT_MGBEARD,
>> .name = "mgbeard",
>> + .bpmp_id = TEGRA_ICC_BPMP_EQOS,
>> + .type = TEGRA_ICC_NISO,
>> .sid = TEGRA234_SID_MGBE,
>> .regs = {
>> .sid = {
>
>
> Best regards,
> Krzysztof
>
On Tue, Mar 28, 2023 at 09:31:58AM +0200, Krzysztof Kozlowski wrote:
> On 27/03/2023 18:14, Sumit Gupta wrote:
[...]
> > diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
[...]
> > @@ -158,6 +260,9 @@ static int tegra186_emc_probe(struct platform_device *pdev)
> > if (!emc)
> > return -ENOMEM;
> >
> > + platform_set_drvdata(pdev, emc);
> > + emc->dev = &pdev->dev;
>
> This patch looks like stiched from two or more patches... emc->dev does
> not look like new member of emc, thus why do you set in exisitng
> function in this patch? Why it wasn't needed before?
This looks like it may be leftover from some development. These two
lines exist in this driver a few lines further down. Either one pair
should be removed. I don't see why this would need to be moved, so
probably the above additions can just be dropped.
Thierry
> > emc->bpmp = tegra_bpmp_get(&pdev->dev);
> > if (IS_ERR(emc->bpmp))
> > return dev_err_probe(&pdev->dev, PTR_ERR(emc->bpmp), "failed to get BPMP\n");
> > @@ -236,6 +341,25 @@ static int tegra186_emc_probe(struct platform_device *pdev)
> > debugfs_create_file("max_rate", S_IRUGO | S_IWUSR, emc->debugfs.root,
> > emc, &tegra186_emc_debug_max_rate_fops);
> >
> > + mc = dev_get_drvdata(emc->dev->parent);
> > + if (mc && mc->soc->icc_ops) {
> > + /*
> > + * Initialize the ICC even if BPMP-FW doesn't support 'MRQ_BWMGR_INT'.
> > + * Use the flag 'mc->bwmgr_mrq_supported' within MC driver and return
> > + * EINVAL instead of passing the request to BPMP-FW later when the BW
> > + * request is made by client with 'icc_set_bw()' call.
> > + */
> > + err = tegra_emc_interconnect_init(emc);
> > + if (err)
> > + goto put_bpmp;
> > +
> > + if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_BWMGR_INT))
> > + mc->bwmgr_mrq_supported = true;
> > + else
> > +
>
> Drop blank line.
>
> > + dev_info(&pdev->dev, "MRQ_BWMGR_INT not present\n");
>
> And what user is supposed to do with this? Either make it descriptive or
> drop.
Agreed. I think we can just drop this. If the intention was to provide a
quick way for people to detect whether BWMGR is available or not, using
something from sysfs/debugfs would be preferable.
Thierry
On 28/03/23 16:35, Thierry Reding wrote:
> On Tue, Mar 28, 2023 at 09:31:58AM +0200, Krzysztof Kozlowski wrote:
>> On 27/03/2023 18:14, Sumit Gupta wrote:
> [...]
>>> diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
> [...]
>>> @@ -158,6 +260,9 @@ static int tegra186_emc_probe(struct platform_device *pdev)
>>> if (!emc)
>>> return -ENOMEM;
>>>
>>> + platform_set_drvdata(pdev, emc);
>>> + emc->dev = &pdev->dev;
>>
>> This patch looks like stiched from two or more patches... emc->dev does
>> not look like new member of emc, thus why do you set in exisitng
>> function in this patch? Why it wasn't needed before?
>
> This looks like it may be leftover from some development. These two
> lines exist in this driver a few lines further down. Either one pair
> should be removed. I don't see why this would need to be moved, so
> probably the above additions can just be dropped.
>
> Thierry
>
Yes, sorry i was left over. Will remove this.
Thank you for catching.
>>> emc->bpmp = tegra_bpmp_get(&pdev->dev);
>>> if (IS_ERR(emc->bpmp))
>>> return dev_err_probe(&pdev->dev, PTR_ERR(emc->bpmp), "failed to get BPMP\n");
>>> @@ -236,6 +341,25 @@ static int tegra186_emc_probe(struct platform_device *pdev)
>>> debugfs_create_file("max_rate", S_IRUGO | S_IWUSR, emc->debugfs.root,
>>> emc, &tegra186_emc_debug_max_rate_fops);
>>>
>>> + mc = dev_get_drvdata(emc->dev->parent);
>>> + if (mc && mc->soc->icc_ops) {
>>> + /*
>>> + * Initialize the ICC even if BPMP-FW doesn't support 'MRQ_BWMGR_INT'.
>>> + * Use the flag 'mc->bwmgr_mrq_supported' within MC driver and return
>>> + * EINVAL instead of passing the request to BPMP-FW later when the BW
>>> + * request is made by client with 'icc_set_bw()' call.
>>> + */
>>> + err = tegra_emc_interconnect_init(emc);
>>> + if (err)
>>> + goto put_bpmp;
>>> +
>>> + if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_BWMGR_INT))
>>> + mc->bwmgr_mrq_supported = true;
>>> + else
>>> +
>>
>> Drop blank line.
>>
Ok.
>>> + dev_info(&pdev->dev, "MRQ_BWMGR_INT not present\n");
>>
>> And what user is supposed to do with this? Either make it descriptive or
>> drop.
>
> Agreed. I think we can just drop this. If the intention was to provide a
> quick way for people to detect whether BWMGR is available or not, using
> something from sysfs/debugfs would be preferable.
>
> Thierry
Sure, will drop this.
Thank you,
Sumit Gupta
© 2016 - 2026 Red Hat, Inc.