Tegra234 Memory interconnect support
Subsystems (4)
cover Tegra234 Memory interconnect support
This patch series adds memory interconnect support for Tegra234 SoC. It is used to dynamically scale DRAM Frequency as per the bandwidth requests from different Memory Controller (MC) clients.
MC Clients use ICC Framework's icc_set_bw() api to dynamically request for the DRAM bandwidth (BW). As per path, the request will be routed from MC to the EMC driver. EMC driver will then send the Client ID, type, and frequency request info to the BPMP-FW which will set the final DRAM freq considering all exisiting requests.
MC and EMC are the ICC providers. Nodes in path for a request will be:
Client[1-n] -> MC -> EMC -> EMEM/DRAM
The patch series also adds interconnect support in the CPUFREQ driver for scaling bandwidth with CPU frequency. For that, added per cluster OPP table in the CPUFREQ driver and using that to scale DRAM freq by requesting the minimum BW respective to the given CPU frequency in OPP table for that cluster.
Sumit Gupta (10):
memory: tegra: add interconnect support for DRAM scaling in Tegra234
memory: tegra: adding iso mc clients for Tegra234
memory: tegra: add pcie mc clients for Tegra234
memory: tegra: add support for software mc clients in Tegra234
dt-bindings: tegra: add icc ids for dummy MC clients
arm64: tegra: Add cpu OPP tables and interconnects property
cpufreq: Add Tegra234 to cpufreq-dt-platdev blocklist
cpufreq: tegra194: add OPP support and set bandwidth
memory: tegra: get number of enabled mc channels
memory: tegra: make cluster bw request a multiple of mc_channels
arch/arm64/boot/dts/nvidia/tegra234.dtsi | 276 +++++++++++ drivers/cpufreq/cpufreq-dt-platdev.c | 1 + drivers/cpufreq/tegra194-cpufreq.c | 152 +++++- drivers/memory/tegra/mc.c | 80 +++- drivers/memory/tegra/mc.h | 1 + drivers/memory/tegra/tegra186-emc.c | 166 +++++++ drivers/memory/tegra/tegra234.c | 565 ++++++++++++++++++++++- include/dt-bindings/memory/tegra234-mc.h | 5 + include/soc/tegra/mc.h | 11 + include/soc/tegra/tegra-icc.h | 79 ++++ 10 files changed, 1312 insertions(+), 24 deletions(-) create mode 100644 include/soc/tegra/tegra-icc.h
Signature · 1 line
2.17.1
PATCH 1/10 memory: tegra: add interconnect support for DRAM scaling in Tegra234
Adding 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 will be:
MC-Client[1-n] -> MC -> EMC -> EMEM/DRAM
MC clients will request for bandwidth to the MC driver which will pass the tegra icc node having current request info to the EMC driver. The EMC driver will send the BPMP Client ID, Client type and bandwidth request info to the BPMP-FW where the final DRAM freq for achieving the requested bandwidth is set based on the passed parameters.
--- drivers/memory/tegra/mc.c | 18 ++- drivers/memory/tegra/tegra186-emc.c | 166 ++++++++++++++++++++++++++++ drivers/memory/tegra/tegra234.c | 101 ++++++++++++++++- include/soc/tegra/mc.h | 7 ++ include/soc/tegra/tegra-icc.h | 72 ++++++++++++ 5 files changed, 362 insertions(+), 2 deletions(-) create mode 100644 include/soc/tegra/tegra-icc.h
drivers/memory/tegra/mc.c +17 -1
@@ -17,6 +17,7 @@#include <linux/sort.h> #include <soc/tegra/fuse.h> +#include <soc/tegra/tegra-icc.h> #include "mc.h"
@@ -779,6 +780,7 @@ const char *const tegra_mc_error_names[8] = {*/ static int tegra_mc_interconnect_setup (struct tegra_mc *mc) { +struct tegra_icc_node *tnode;struct icc_node *node;unsigned int i;int err;
@@ -792,7 +794,11 @@ 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.xlate_extended = mc->soc->icc_ops->xlate_extended; + mc->provider.get_bw = mc->soc->icc_ops->get_bw; +if (mc->soc->icc_ops->xlate) + mc->provider.xlate = mc->soc->icc_ops->xlate; +if (mc->soc->icc_ops->xlate_extended) + mc->provider.xlate_extended = mc->soc->icc_ops->xlate_extended; err =icc_provider_add (&mc->provider);if (err)
@@ -814,6 +820,10 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc) goto remove_nodes;for (i =0 ; i < mc->soc->num_clients; i++) { + tnode =kzalloc (sizeof (*tnode),GFP_KERNEL ); +if (!tnode) +return -ENOMEM ; +/* create MC client node */ node =icc_node_create (mc->soc->clients[i].id);if (IS_ERR (node)) {
@@ -828,6 +838,12 @@ 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 = tnode; + tnode->node = node; + tnode->type = mc->soc->clients[i].type; + tnode->bpmp_id = mc->soc->clients[i].bpmp_id; + tnode->mc = mc; }return 0 ;
drivers/memory/tegra/tegra186-emc.c +166 -0
211 lines not on this page · raw message
drivers/memory/tegra/tegra234.c +100 -1
195 lines not on this page · raw message
include/soc/tegra/mc.h +7 -0
@@ -13,6 +13,7 @@#include <linux/irq.h> #include <linux/reset-controller.h> #include <linux/types.h> +#include <soc/tegra/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 {
@@ -238,6 +243,8 @@ struct tegra_mc {struct {struct dentry *root; } debugfs; + +struct tegra_icc_node *curr_tnode; };int tegra_mc_write_emem_configuration (struct tegra_mc *mc,unsigned long rate);
include/soc/tegra/tegra-icc.h new file +72 -0
72 lines not on this page · raw message
Signature · 1 line
2.17.1
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 9 lines
drivers/memory/tegra/mc.c +1
-0@@ -779,6 +780,7 @@ const char *const tegra_mc_error_names[8] = {*/ static int tegra_mc_interconnect_setup (struct tegra_mc *mc) { +struct tegra_icc_node *tnode;struct icc_node *node;unsigned int i;int err;
drivers/memory/tegra/mc.c static int tegra_mc_interconnect_setup(struct tegra_mc *mc) · line 801
Quoted 9 lines
drivers/memory/tegra/mc.c +5
-1@@ -792,7 +794,11 @@ 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.xlate_extended = mc->soc->icc_ops->xlate_extended; + mc->provider.get_bw = mc->soc->icc_ops->get_bw; +if (mc->soc->icc_ops->xlate) + mc->provider.xlate = mc->soc->icc_ops->xlate; +if (mc->soc->icc_ops->xlate_extended) + mc->provider.xlate_extended = mc->soc->icc_ops->xlate_extended;
These IFs look pointless
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
Quoted 16 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 12 lines
drivers/memory/tegra/mc.c +1
-0@@ -779,6 +780,7 @@ const char *const tegra_mc_error_names[8] = {*/ static int tegra_mc_interconnect_setup (struct tegra_mc *mc) { +struct tegra_icc_node *tnode;struct icc_node *node;unsigned int i;int err;@@ -792,7 +794,11 @@ 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;
Quoted 8 lines
drivers/memory/tegra/mc.c static int tegra_mc_interconnect_setup(struct tegra_mc *mc) · line 801
Quoted 6 lines
drivers/memory/tegra/mc.c +5
-1- mc->provider.xlate_extended = mc->soc->icc_ops->xlate_extended; + mc->provider.get_bw = mc->soc->icc_ops->get_bw; +if (mc->soc->icc_ops->xlate) + mc->provider.xlate = mc->soc->icc_ops->xlate; +if (mc->soc->icc_ops->xlate_extended) + mc->provider.xlate_extended = mc->soc->icc_ops->xlate_extended;These IFs look pointless
Ok. Will remove.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
In reply to Sumit Gupta20.12.2022 19:02, Sumit Gupta пишет:
drivers/memory/tegra/tegra186-emc.c static int tegra186_emc_probe(struct platform_device *pdev) · line 396
Quoted 4 lines
drivers/memory/tegra/tegra186-emc.c +4
-0+if (tegra_bpmp_mrq_is_supported (emc->bpmp,MRQ_BWMGR_INT )) { + err =tegra_emc_interconnect_init (emc); +if (!err) +return err;
"return err" doesn't sound good
drivers/memory/tegra/tegra186-emc.c static int tegra186_emc_probe(struct platform_device *pdev) · line 398
Quoted 2 lines
drivers/memory/tegra/tegra186-emc.c +2
-0+dev_err (&pdev->dev,"tegra_emc_interconnect_init failed:%d\n" , err); + goto put_bpmp;
Move error message to tegra_emc_interconnect_init()
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
Quoted 9 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
drivers/memory/tegra/tegra186-emc.c static int tegra186_emc_probe(struct platform_device *pdev) · line 396
Quoted 4 lines
drivers/memory/tegra/tegra186-emc.c +4
-0+if (tegra_bpmp_mrq_is_supported (emc->bpmp,MRQ_BWMGR_INT )) { + err =tegra_emc_interconnect_init (emc); +if (!err) +return err;
Quoted 6 lines
"return err" doesn't sound good
drivers/memory/tegra/tegra186-emc.c static int tegra186_emc_probe(struct platform_device *pdev) · line 398
Quoted 2 lines
drivers/memory/tegra/tegra186-emc.c +2
-0+dev_err (&pdev->dev,"tegra_emc_interconnect_init failed:%d\n" , err); + goto put_bpmp;Move error message to
tegra_emc_interconnect_init()
Ok, will change.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
In reply to Sumit Gupta20.12.2022 19:02, Sumit Gupta пишет:
include/soc/tegra/tegra-icc.h line 16
Quoted 11 lines
include/soc/tegra/tegra-icc.h new file +11
-0+#ifndef MEMORY_TEGRA_ICC_H +#define MEMORY_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 , +};
You using only TEGRA_ICC_NISO and !TEGRA_ICC_NISO in the code.
include/soc/tegra/mc.h defines TAG_DEFAULT/ISO, please drop all these duplicated and unused "types" unless there is a good reason to keep them
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
Quoted 18 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
include/soc/tegra/tegra-icc.h line 16
Quoted 11 lines
include/soc/tegra/tegra-icc.h new file +11
-0+#ifndef MEMORY_TEGRA_ICC_H +#define MEMORY_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 , +};You using only
TEGRA_ICC_NISOand !TEGRA_ICC_NISO in the code.
Quoted 3 lines
include/soc/tegra/mc.hdefines TAG_DEFAULT/ISO, please drop all these duplicated and unused "types" unless there is a good reason to keep them
These type are used while defining clients in "tegra234_mc_clients[]" and its passed to BPMP-FW which has handling for each client type.
kernel$ grep -B2 ".type = TEGRA_ICC_ISO" drivers/memory/tegra/tegra234.c
.name = "hdar",
.bpmp_id = TEGRA_ICC_BPMP_HDA,
.type = TEGRA_ICC_ISO_AUDIO,
--
.name = "hdaw",
.bpmp_id = TEGRA_ICC_BPMP_HDA,
.type = TEGRA_ICC_ISO_AUDIO,
--
.name = "vi2w",
.bpmp_id = TEGRA_ICC_BPMP_VI2,
.type = TEGRA_ICC_ISO_VI, --
.name = "vi2falr",
.bpmp_id = TEGRA_ICC_BPMP_VI2FAL,
.type = TEGRA_ICC_ISO_VIFAL,
--
.name = "vi2falw",
.bpmp_id = TEGRA_ICC_BPMP_VI2FAL,
.type = TEGRA_ICC_ISO_VIFAL,
--
.name = "aper",
.bpmp_id = TEGRA_ICC_BPMP_APE,
.type = TEGRA_ICC_ISO_AUDIO,
--
.name = "apew",
.bpmp_id = TEGRA_ICC_BPMP_APE,
.type = TEGRA_ICC_ISO_AUDIO,
--
.name = "nvdisplayr",
.bpmp_id = TEGRA_ICC_BPMP_DISPLAY,
.type = TEGRA_ICC_ISO_DISPLAY,
--
.name = "nvdisplayr1",
.bpmp_id = TEGRA_ICC_BPMP_DISPLAY,
.type = TEGRA_ICC_ISO_DISPLAY,
--
.name = "apedmar",
.bpmp_id = TEGRA_ICC_BPMP_APEDMA,
.type = TEGRA_ICC_ISO_AUDIO,
--
.name = "apedmaw",
.bpmp_id = TEGRA_ICC_BPMP_APEDMA,
.type = TEGRA_ICC_ISO_AUDIO,
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
21.12.2022 11:05, Sumit Gupta пишет:
Quoted 19 lines · Dmitry Osipenko
Quoted 18 lines
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
include/soc/tegra/tegra-icc.h line 16
Quoted 11 lines
include/soc/tegra/tegra-icc.h new file +11
-0+#ifndef MEMORY_TEGRA_ICC_H +#define MEMORY_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, +};You using only
TEGRA_ICC_NISOand !TEGRA_ICC_NISO in the code.
Quoted 6 lines
Quoted 3 lines
include/soc/tegra/mc.hdefines TAG_DEFAULT/ISO, please drop all these duplicated and unused "types" unless there is a good reason to keep themThese type are used while defining clients in "tegra234_mc_clients[]" and its passed to BPMP-FW which has handling for each client type.
The type should be based on the ICC tag, IMO. AFAICS, type isn't fixed in FW and you can set both ISO and NISO BW, hence it's up to a device driver to select the appropriate tag.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
Quoted 27 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
21.12.2022 11:05, Sumit Gupta пишет:
Quoted 23 lines · Dmitry Osipenko
Quoted 21 lines
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
include/soc/tegra/tegra-icc.h line 16
Quoted 11 lines
include/soc/tegra/tegra-icc.h new file +11
-0+#ifndef MEMORY_TEGRA_ICC_H +#define MEMORY_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 , +};You using only
TEGRA_ICC_NISOand !TEGRA_ICC_NISO in the code.
include/soc/tegra/mc.hdefines TAG_DEFAULT/ISO, please drop all these duplicated and unused "types" unless there is a good reason to keep them
Quoted 7 lines
Quoted 2 lines
These type are used while defining clients in "tegra234_mc_clients[]" and its passed to BPMP-FW which has handling for each client type.
The type should be based on the ICC tag, IMO. AFAICS, type isn't fixed in FW and you can set both ISO and NISO BW, hence it's up to a device driver to select the appropriate tag.
Type for a MC client is fixed. So, adding the tag and giving option to client driver won't have impact. Also, we need to pass the type to BPMP from bw set api. But the tag info is available to aggregate api and not set.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
In reply to Sumit Gupta20.12.2022 19:02, Sumit Gupta пишет:
drivers/memory/tegra/tegra186-emc.c DEFINE_DEBUGFS_ATTRIBUTE(tegra186_emc_debug_max_rate_fops, · line 243
Quoted 7 lines
drivers/memory/tegra/tegra186-emc.c +7
-0+static int tegra_emc_icc_get_init_bw (struct icc_node *node,u32 *avg,u32 *peak) +{ + *avg =0 ; + *peak =0 ; + +return 0 ; +}
Looks wrong, you should add ICC support to all the drivers first and only then enable ICC. I think you added this init_bw() to work around the fact that ICC isn't supported by T234 drivers.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
Quoted 4 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 11 lines
drivers/memory/tegra/tegra186-emc.c DEFINE_DEBUGFS_ATTRIBUTE(tegra186_emc_debug_max_rate_fops, · line 243
Quoted 7 lines
drivers/memory/tegra/tegra186-emc.c +7
-0+static int tegra_emc_icc_get_init_bw (struct icc_node *node,u32 *avg,u32 *peak) +{ + *avg =0 ; + *peak =0 ; + +return 0 ; +}Looks wrong, you should add ICC support to all the drivers first and only then enable ICC. I think you added this
init_bw()to work around the fact that ICC isn't supported by T234 drivers.
If get_bw hook is not added then max freq is set due to 'INT_MAX' below.
void icc_node_add(struct icc_node *node, struct icc_provider *provider) {
....
/* get the initial bandwidth values and sync them with hardware */ if (provider->get_bw) {
provider->get_bw(node, &node->init_avg, &node->init_peak);
} else {
node->init_avg = INT_MAX;
node->init_peak = INT_MAX;
}
So, will have to add the empty functions at least.
static int tegra_emc_icc_get_init_bw(struct icc_node *node, u32 *avg,
u32 *peak)
{
- *avg = 0;
- *peak = 0;
-
return 0;
}
Support to all the client drivers can't be added at once as there are many drivers all with different requirements and handling. This patch series is the beginning to add the basic interconnect support in new Tegra SoC's. Support for more clients will be added later one by one or in batch.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
21.12.2022 12:35, Sumit Gupta пишет:
Quoted 44 lines · Dmitry Osipenko
Quoted 4 lines
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 12 lines
drivers/memory/tegra/tegra186-emc.c DEFINE_DEBUGFS_ATTRIBUTE(tegra186_emc_debug_max_rate_fops, · line 243
Quoted 8 lines
+static int
tegra_emc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peak)drivers/memory/tegra/tegra186-emc.c +6
-0+{ + *avg = 0; + *peak = 0; + + return 0; +}Looks wrong, you should add ICC support to all the drivers first and only then enable ICC. I think you added this
init_bw()to work around the fact that ICC isn't supported by T234 drivers.If
get_bwhook is not added then max freq is set due to 'INT_MAX' below.void
icc_node_add(struct icc_node *node, struct icc_provider *provider){
....
/* get the initial bandwidth values and sync them with hardware */ if (provider->get_bw) {
provider->get_bw(node, &node->init_avg, &node->init_peak); } else {
node->init_avg=INT_MAX;
node->init_peak=INT_MAX;
}So, will have to add the empty functions at least.
static int
tegra_emc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peak)
{
- *avg = 0;
- *peak = 0;
-
return 0;
}
Quoted 5 lines
Support to all the client drivers can't be added at once as there are many drivers all with different requirements and handling. This patch series is the beginning to add the basic interconnect support in new Tegra SoC's. Support for more clients will be added later one by one or in batch.
This means that bandwidth management isn't working properly. You should leave the freq to INT_MAX and fix the missing integer overflows in the code if any, or read out the BW from FW.
Once you'll enable ICC for all drivers, it will start working.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
Quoted 54 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
21.12.2022 12:35, Sumit Gupta пишет:
Quoted 44 lines · Dmitry Osipenko
Quoted 4 lines
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 12 lines
drivers/memory/tegra/tegra186-emc.c DEFINE_DEBUGFS_ATTRIBUTE(tegra186_emc_debug_max_rate_fops, · line 243
Quoted 8 lines
+static int
tegra_emc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peak)drivers/memory/tegra/tegra186-emc.c +6
-0+{ + *avg =0 ; + *peak =0 ; + +return 0 ; +}Looks wrong, you should add ICC support to all the drivers first and only then enable ICC. I think you added this
init_bw()to work around the fact that ICC isn't supported by T234 drivers.If
get_bwhook is not added then max freq is set due to 'INT_MAX' below.void icc_node_add(struct icc_node *node, struct icc_provider *provider) { .... /* get the initial bandwidth values and sync them with hardware */ if (provider->get_bw) { provider->get_bw(node, &node->init_avg, &node->init_peak); } else { node->init_avg = INT_MAX; node->init_peak = INT_MAX; }So, will have to add the empty functions at least.
static int tegra_emc_icc_get_init_bw(struct
icc_node*node, u32 *avg,
u32 *peak)
{(quoted diff) +0
-3- *avg = 0; - *peak = 0; - return 0; }Quoted 5 lines
Support to all the client drivers can't be added at once as there are many drivers all with different requirements and handling. This patch series is the beginning to add the basic interconnect support in new Tegra SoC's. Support for more clients will be added later one by one or in batch.
Quoted 6 lines
This means that bandwidth management isn't working properly. You should leave the freq to
INT_MAXand fix the missing integer overflows in the code if any, or read out the BW from FW.Once you'll enable ICC for all drivers, it will start working.
Referred the below patches and now understand what you mean.
https://patchwork.kernel.org/project/linux-arm-msm/cover/[email protected]/
https://lore.kernel.org/all/[email protected]/
There is no MRQ currently in BPMP-FW to get the bandwidth for a client.
So, can't add that in get_bw(). As explained earlier, we are in process adding of adding support for all clients but that may take some time. We can remove the get_bw() calls initializing "*bw = 0" once we have the support added in all clients. Noticed that below drivers also do same.
$ grep -rB3 "*peak = 0" drivers/interconnect/*
drivers/interconnect/imx/imx.c-static int imx_icc_get_bw(struct
icc_node *node, u32 *avg, u32 *peak)
drivers/interconnect/imx/imx.c-{
drivers/interconnect/imx/imx.c- *avg = 0;
drivers/interconnect/imx/imx.c: *peak = 0;
--
drivers/interconnect/qcom/msm8974.c-static int msm8974_get_bw(struct
icc_node *node, u32 *avg, u32 *peak)
drivers/interconnect/qcom/msm8974.c-{
drivers/interconnect/qcom/msm8974.c- *avg = 0;
drivers/interconnect/qcom/msm8974.c: *peak = 0;Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
In reply to Sumit Gupta20.12.2022 19:02, Sumit Gupta пишет:
Quoted 8 lines
drivers/memory/tegra/tegra234.c +8
-0+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 tegra_icc_node *tnode = src->data; + +/* +* 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
drivers/memory/tegra/tegra234.c static const struct tegra_mc_client tegra234_mc_clients[] = { · line 393
Quoted 10 lines
drivers/memory/tegra/tegra234.c +10
-0+* 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 (tnode->node) + mc->curr_tnode = tnode; +else +pr_err ("%s, tegra_icc_node is null\n" , __func__);
The tnode->node can't be NULL.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
Quoted 16 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 12 lines
drivers/memory/tegra/tegra234.c +12
-0+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 tegra_icc_node *tnode = src->data; + +/* +* 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)
Quoted 8 lines
drivers/memory/tegra/tegra234.c static const struct tegra_mc_client tegra234_mc_clients[] = { · line 393
Quoted 6 lines
drivers/memory/tegra/tegra234.c +6
-0+return 0 ; + +if (tnode->node) + mc->curr_tnode = tnode; +else +pr_err ("%s, tegra_icc_node is null\n" , __func__);The
tnode->nodecan't be NULL.
Ok, will remove the check.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
In reply to Sumit Gupta20.12.2022 19:02, Sumit Gupta пишет:
Quoted 24 lines
static int
tegra186_emc_probe(struct platform_device *pdev){drivers/memory/tegra/tegra186-emc.c +5
-0struct 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 +307,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 +388,19 @@ 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);
drivers/memory/tegra/tegra186-emc.c static int tegra186_emc_probe(struct platform_device *pdev) · line 401
Quoted 10 lines
drivers/memory/tegra/tegra186-emc.c +10
-0+if (mc && mc->soc->icc_ops) { +if (tegra_bpmp_mrq_is_supported (emc->bpmp,MRQ_BWMGR_INT )) { + err =tegra_emc_interconnect_init (emc); +if (!err) +return err; +dev_err (&pdev->dev,"tegra_emc_interconnect_init failed:%d\n" , err); + goto put_bpmp; + }else { +dev_info (&pdev->dev,"MRQ_BWMGR_INT not present\n" ); + }
If there is no MRQ_BWMGR_INT, then device drivers using ICC won't probe. This is either a error condition, or ICC should inited and then ICC changes should be skipped.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
Quoted 27 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 23 lines
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;drivers/memory/tegra/tegra186-emc.c +4
-0+struct tegra_mc *mc;unsigned int i;int err;@@ -158,6 +307,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 +388,19 @@ 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);
Quoted 16 lines
drivers/memory/tegra/tegra186-emc.c static int tegra186_emc_probe(struct platform_device *pdev) · line 401
Quoted 11 lines
drivers/memory/tegra/tegra186-emc.c +11
-0+ mc =dev_get_drvdata (emc->dev->parent); +if (mc && mc->soc->icc_ops) { +if (tegra_bpmp_mrq_is_supported (emc->bpmp,MRQ_BWMGR_INT )) { + err =tegra_emc_interconnect_init (emc); +if (!err) +return err; +dev_err (&pdev->dev,"tegra_emc_interconnect_init failed:%d\n" , err); + goto put_bpmp; + }else { +dev_info (&pdev->dev,"MRQ_BWMGR_INT not present\n" ); + }If there is no
MRQ_BWMGR_INT, then device drivers using ICC won't probe. This is either a error condition, or ICC should inited and then ICC changes should be skipped.
If the MRQ_BW_MGR is not supported by a BPMP-FW binary, then the MC & EMC drivers will still probe successfully and scaling will be disabled.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
In reply to Sumit Gupta20.12.2022 19:02, Sumit Gupta пишет:
drivers/memory/tegra/tegra186-emc.c DEFINE_DEBUGFS_ATTRIBUTE(tegra186_emc_debug_max_rate_fops, · line 176
Quoted 9 lines
drivers/memory/tegra/tegra186-emc.c +9
-0+static int tegra_emc_icc_set_bw (struct icc_node *src,struct icc_node *dst) +{ +struct tegra186_emc *emc =to_tegra186_emc (dst->provider); +struct tegra_mc *mc =dev_get_drvdata (emc->dev->parent); +struct mrq_bwmgr_int_request bwmgr_req = {0 }; +struct mrq_bwmgr_int_response bwmgr_resp = {0 }; +struct tegra_icc_node *tnode = mc->curr_tnode; +struct tegra_bpmp_message msg; +int ret =0 ;
Nit: unnecessarily initialized var, the same for the rest of the code
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
In reply to Sumit Gupta20.12.2022 19:02, Sumit Gupta пишет:
Quoted 10 lines
drivers/memory/tegra/tegra186-emc.c +10
-0+static int tegra_emc_icc_set_bw (struct icc_node *src,struct icc_node *dst) +{ +struct tegra186_emc *emc =to_tegra186_emc (dst->provider); +struct tegra_mc *mc =dev_get_drvdata (emc->dev->parent); +struct mrq_bwmgr_int_request bwmgr_req = {0 }; +struct mrq_bwmgr_int_response bwmgr_resp = {0 }; +struct tegra_icc_node *tnode = mc->curr_tnode; +struct tegra_bpmp_message msg; +int ret =0 ; +
drivers/memory/tegra/tegra186-emc.c DEFINE_DEBUGFS_ATTRIBUTE(tegra186_emc_debug_max_rate_fops, · line 187
Quoted 10 lines
drivers/memory/tegra/tegra186-emc.c +10
-0+/* +* 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->curr_tnode->type ==TEGRA_ICC_NISO )
The mc->curr_tnode usage looks suspicious, why you can't use src node?
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
21.12.2022 22:20, Dmitry Osipenko пишет:
Quoted 15 lines
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 14 lines
drivers/memory/tegra/tegra186-emc.c +14
-0+static int tegra_emc_icc_set_bw (struct icc_node *src,struct icc_node *dst) +{ +struct tegra186_emc *emc =to_tegra186_emc (dst->provider); +struct tegra_mc *mc =dev_get_drvdata (emc->dev->parent); +struct mrq_bwmgr_int_request bwmgr_req = {0 }; +struct mrq_bwmgr_int_response bwmgr_resp = {0 }; +struct tegra_icc_node *tnode = mc->curr_tnode; +struct tegra_bpmp_message msg; +int ret =0 ; + +/* +* 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,
Quoted 9 lines
drivers/memory/tegra/tegra186-emc.c DEFINE_DEBUGFS_ATTRIBUTE(tegra186_emc_debug_max_rate_fops, · line 187
Quoted 6 lines
drivers/memory/tegra/tegra186-emc.c +6
-0+* the pre-initialization already happened in Bootloader(MB2) and BPMP-FW. +*/ +if (src->id == dst->id) +return 0 ; + +if (mc->curr_tnode->type ==TEGRA_ICC_NISO )The
mc->curr_tnodeusage looks suspicious, why you can't use src node?
This function sets memory BW for a memory client and not for EMC. Apparently, you should move the BW setting to tegra234_mc_icc_set() and then tegra_emc_icc_set_bw() will be a noop.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
Quoted 29 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
21.12.2022 22:20, Dmitry Osipenko пишет:
Quoted 15 lines
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 14 lines
drivers/memory/tegra/tegra186-emc.c +14
-0+static int tegra_emc_icc_set_bw (struct icc_node *src,struct icc_node *dst) +{ +struct tegra186_emc *emc =to_tegra186_emc (dst->provider); +struct tegra_mc *mc =dev_get_drvdata (emc->dev->parent); +struct mrq_bwmgr_int_request bwmgr_req = {0 }; +struct mrq_bwmgr_int_response bwmgr_resp = {0 }; +struct tegra_icc_node *tnode = mc->curr_tnode; +struct tegra_bpmp_message msg; +int ret =0 ; + +/* +* 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, Quoted 9 lines
drivers/memory/tegra/tegra186-emc.c DEFINE_DEBUGFS_ATTRIBUTE(tegra186_emc_debug_max_rate_fops, · line 187
Quoted 6 lines
drivers/memory/tegra/tegra186-emc.c +6
-0+* the pre-initialization already happened in Bootloader(MB2) and BPMP-FW. +*/ +if (src->id == dst->id) +return 0 ; + +if (mc->curr_tnode->type ==TEGRA_ICC_NISO )The
mc->curr_tnodeusage looks suspicious, why you can't use src node?
Quoted 4 lines
This function sets memory BW for a memory client and not for EMC. Apparently, you should move the BW setting to
tegra234_mc_icc_set()and thentegra_emc_icc_set_bw()will be a noop.
Yes, will move this code to set_bw() in MC driver. The set_bw call in EMC will be a dummy function after change.
This change will also help to remove "struct tegra_icc_node *curr_tnode;" from "struct tegra_mc" which is used for passing data between MC and EMC drivers to transfer to BPMP.
static int tegra_emc_icc_set_bw(struct icc_node *src, struct icc_node *dst)
{
return 0;
}Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
In reply to Dmitry OsipenkoQuoted 18 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 14 lines
drivers/memory/tegra/tegra186-emc.c +14
-0+static int tegra_emc_icc_set_bw (struct icc_node *src,struct icc_node *dst) +{ +struct tegra186_emc *emc =to_tegra186_emc (dst->provider); +struct tegra_mc *mc =dev_get_drvdata (emc->dev->parent); +struct mrq_bwmgr_int_request bwmgr_req = {0 }; +struct mrq_bwmgr_int_response bwmgr_resp = {0 }; +struct tegra_icc_node *tnode = mc->curr_tnode; +struct tegra_bpmp_message msg; +int ret =0 ; + +/* +* 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,
Quoted 9 lines
drivers/memory/tegra/tegra186-emc.c DEFINE_DEBUGFS_ATTRIBUTE(tegra186_emc_debug_max_rate_fops, · line 187
Quoted 6 lines
drivers/memory/tegra/tegra186-emc.c +6
-0+* the pre-initialization already happened in Bootloader(MB2) and BPMP-FW. +*/ +if (src->id == dst->id) +return 0 ; + +if (mc->curr_tnode->type ==TEGRA_ICC_NISO )The
mc->curr_tnodeusage looks suspicious, why you can't use src node?
Now, we can get rid of "curr_tnode" after moving the code from the EMC to MC driver for transferring MC client's request info to the BPMP.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
In reply to Sumit Gupta20.12.2022 19:02, Sumit Gupta пишет:
Quoted 20 lines
static int
tegra_mc_interconnect_setup(struct tegra_mc *mc){drivers/memory/tegra/mc.c +6
-1+struct tegra_icc_node *tnode;struct icc_node *node;unsigned int i;int err;@@ -792,7 +794,11 @@ 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.xlate_extended = mc->soc->icc_ops->xlate_extended; + mc->provider.get_bw = mc->soc->icc_ops->get_bw; +if (mc->soc->icc_ops->xlate) + mc->provider.xlate = mc->soc->icc_ops->xlate; +if (mc->soc->icc_ops->xlate_extended) + mc->provider.xlate_extended = mc->soc->icc_ops->xlate_extended; err =icc_provider_add (&mc->provider);if (err)
drivers/memory/tegra/mc.c static int tegra_mc_interconnect_setup(struct tegra_mc *mc) · line 823
Quoted 4 lines
drivers/memory/tegra/mc.c +1
-0@@ -814,6 +820,10 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc) goto remove_nodes;for (i =0 ; i < mc->soc->num_clients; i++) { + tnode =kzalloc (sizeof (*tnode),GFP_KERNEL );
devm_kzalloc
On the other hand, the tnode is unnecessary at all. Use struct tegra_mc_client for sw clients.
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
In reply to Sumit GuptaQuoted 66 lines · Sumit Gupta
Adding 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 will be:
MC-Client[1-n] -> MC -> EMC -> EMEM/DRAMMC clients will request for bandwidth to the MC driver which will pass the tegra icc node having current request info to the EMC driver. The EMC driver will send the BPMP Client ID, Client type and bandwidth request info to the BPMP-FW where the final DRAM freq for achieving the requested bandwidth is set based on the passed parameters.
Signed-off-by Sumit Gupta <[email protected]>--- drivers/memory/tegra/mc.c | 18 ++- drivers/memory/tegra/tegra186-emc.c | 166 ++++++++++++++++++++++++++++ drivers/memory/tegra/tegra234.c | 101 ++++++++++++++++- include/soc/tegra/mc.h | 7 ++ include/soc/tegra/tegra-icc.h | 72 ++++++++++++ 5 files changed, 362 insertions(+), 2 deletions(-) create mode 100644 include/soc/tegra/tegra-icc.hdrivers/memory/tegra/mc.c +11
-1@@ -17,6 +17,7 @@#include <linux/sort.h> #include <soc/tegra/fuse.h> +#include <soc/tegra/tegra-icc.h> #include "mc.h" @@ -779,6 +780,7 @@ const char *const tegra_mc_error_names[8] = {*/ static int tegra_mc_interconnect_setup (struct tegra_mc *mc) { +struct tegra_icc_node *tnode;struct icc_node *node;unsigned int i;int err;@@ -792,7 +794,11 @@ 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.xlate_extended = mc->soc->icc_ops->xlate_extended; + mc->provider.get_bw = mc->soc->icc_ops->get_bw; +if (mc->soc->icc_ops->xlate) + mc->provider.xlate = mc->soc->icc_ops->xlate; +if (mc->soc->icc_ops->xlate_extended) + mc->provider.xlate_extended = mc->soc->icc_ops->xlate_extended; err =icc_provider_add (&mc->provider);if (err)@@ -814,6 +820,10 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc) goto remove_nodes;for (i =0 ; i < mc->soc->num_clients; i++) { + tnode =kzalloc (sizeof (*tnode),GFP_KERNEL ); +if (!tnode) +return -ENOMEM ; +/* create MC client node */ node =icc_node_create (mc->soc->clients[i].id);if (IS_ERR (node)) {
drivers/memory/tegra/mc.c static int tegra_mc_interconnect_setup(struct tegra_mc *mc) · line 842
Quoted 5 lines
drivers/memory/tegra/mc.c +2
-0@@ -828,6 +838,12 @@ 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 = tnode;
Where is it freed?
(...)
include/soc/tegra/tegra-icc.h
Quoted 16 lines
drivers/memory/tegra/mc.c +2
-0struct tegra_mc_ops {@@ -238,6 +243,8 @@ struct tegra_mc {struct {struct dentry *root; } debugfs; + +struct tegra_icc_node *curr_tnode; };int tegra_mc_write_emem_configuration (struct tegra_mc *mc,unsigned long rate);include/soc/tegra/tegra-icc.h new file +0
-0
Why not in linux?
include/soc/tegra/tegra-icc.h line 6
Quoted 7 lines
include/soc/tegra/tegra-icc.h new file +6
-0@@ -0,0 +1,72 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* +* Copyright (C) 2022-2023 NVIDIA CORPORATION. All rights reserved. +*/ + +#ifndef MEMORY_TEGRA_ICC_H
This does not match the path/name.
Quoted 56 lines
include/soc/tegra/tegra-icc.h new file +56
-0+#define MEMORY_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 , +}; + +struct tegra_icc_node { +struct icc_node *node; +struct tegra_mc *mc; +u32 bpmp_id; +u32 type; +}; + +/* 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
include/soc/tegra/tegra-icc.h line 72
Quoted 10 lines
include/soc/tegra/tegra-icc.h new file +10
-0+#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 /* MEMORY_TEGRA_ICC_H */
Best regards,
Krzysztof
Re: [Patch v1 01/10] memory: tegra: add interconnect support for DRAM scaling in Tegra234
Quoted 79 lines · Krzysztof Kozlowski
External email: Use caution opening links or attachments
Quoted 66 lines · Sumit Gupta
Adding 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 will be:
MC-Client[1-n] -> MC -> EMC -> EMEM/DRAMMC clients will request for bandwidth to the MC driver which will pass the tegra icc node having current request info to the EMC driver. The EMC driver will send the BPMP Client ID, Client type and bandwidth request info to the BPMP-FW where the final DRAM freq for achieving the requested bandwidth is set based on the passed parameters.
Signed-off-by Sumit Gupta <[email protected]>--- drivers/memory/tegra/mc.c | 18 ++- drivers/memory/tegra/tegra186-emc.c | 166 ++++++++++++++++++++++++++++ drivers/memory/tegra/tegra234.c | 101 ++++++++++++++++- include/soc/tegra/mc.h | 7 ++ include/soc/tegra/tegra-icc.h | 72 ++++++++++++ 5 files changed, 362 insertions(+), 2 deletions(-)create mode 100644
include/soc/tegra/tegra-icc.hdrivers/memory/tegra/mc.c +11
-1@@ -17,6 +17,7 @@#include <linux/sort.h> #include <soc/tegra/fuse.h> +#include <soc/tegra/tegra-icc.h> #include "mc.h" @@ -779,6 +780,7 @@ const char *const tegra_mc_error_names[8] = {*/ static int tegra_mc_interconnect_setup (struct tegra_mc *mc) { +struct tegra_icc_node *tnode;struct icc_node *node;unsigned int i;int err;@@ -792,7 +794,11 @@ 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.xlate_extended = mc->soc->icc_ops->xlate_extended; + mc->provider.get_bw = mc->soc->icc_ops->get_bw; +if (mc->soc->icc_ops->xlate) + mc->provider.xlate = mc->soc->icc_ops->xlate; +if (mc->soc->icc_ops->xlate_extended) + mc->provider.xlate_extended = mc->soc->icc_ops->xlate_extended; err =icc_provider_add (&mc->provider);if (err)@@ -814,6 +820,10 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc) goto remove_nodes;for (i =0 ; i < mc->soc->num_clients; i++) { + tnode =kzalloc (sizeof (*tnode),GFP_KERNEL ); +if (!tnode) +return -ENOMEM ; +/* create MC client node */ node =icc_node_create (mc->soc->clients[i].id);if (IS_ERR (node)) {drivers/memory/tegra/mc.c static int tegra_mc_interconnect_setup(struct tegra_mc *mc) · line 842
Quoted 5 lines
drivers/memory/tegra/mc.c +2
-0@@ -828,6 +838,12 @@ 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 = tnode;Where is it freed?
Quoted 2 lines
(...)
Have removed 'struct tegra_icc_node' in v2. Instead 'node->data'
now points to the entry of MC client in the static table 'tegra234_mc_clients' as below. So, the old alloc of 'struct tegra_icc_node' is not required now.
+ node->data = (char *)&(mc->soc->clients[i]);
Quoted 10 lines
Quoted 10 lines
struct tegra_mc_ops {drivers/memory/tegra/mc.c +2
-0@@ -238,6 +243,8 @@ struct tegra_mc {struct {struct dentry *root; } debugfs; + +struct tegra_icc_node *curr_tnode; };
Quoted 9 lines
include/soc/tegra/tegra-icc.h
Quoted 6 lines
drivers/memory/tegra/mc.c +0
-0int tegra_mc_write_emem_configuration (struct tegra_mc *mc,unsigned long rate);include/soc/tegra/tegra-icc.h new file +0
-0Why not in linux?
Moved the file to 'include/linux/tegra-icc.h' in v2.
Quoted 10 lines
include/soc/tegra/tegra-icc.h line 6
Quoted 7 lines
include/soc/tegra/tegra-icc.h new file +6
-0@@ -0,0 +1,72 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* +* Copyright (C) 2022-2023 NVIDIA CORPORATION. All rights reserved. +*/ + +#ifndef MEMORY_TEGRA_ICC_H This does not match the path/name.
Have changed path name to below in v2.
+#ifndef LINUX_TEGRA_ICC_H
+#define LINUX_TEGRA_ICC_H
Quoted 70 lines
Quoted 56 lines
include/soc/tegra/tegra-icc.h new file +56
-0+#define MEMORY_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 , +}; + +struct tegra_icc_node { +struct icc_node *node; +struct tegra_mc *mc; +u32 bpmp_id; +u32 type; +}; + +/* 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 37include/soc/tegra/tegra-icc.h line 72
Quoted 10 lines
include/soc/tegra/tegra-icc.h new file +10
-0+#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 /* MEMORY_TEGRA_ICC_H */ Best regards,
Krzysztof
PATCH 2/10 memory: tegra: adding iso mc clients for Tegra234
In reply to Sumit GuptaAdding Isochronous (ISO) MC clients to the mc_clients table for Tegra234. ISO clients have guaranteed bandwidth requirement.
--- drivers/memory/tegra/tegra234.c | 108 ++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+)
drivers/memory/tegra/tegra234.c +108 -0
@@ -14,6 +14,30 @@static const struct tegra_mc_client tegra234_mc_clients[] = { { + .id =TEGRA234_MEMORY_CLIENT_HDAR , + .name ="hdar" , + .bpmp_id =TEGRA_ICC_BPMP_HDA , + .type =TEGRA_ICC_ISO_AUDIO , + .sid =TEGRA234_SID_HDA , + .regs = { + .sid = { + .override =0xa8 , + .security =0xac , + }, + }, + }, { + .id =TEGRA234_MEMORY_CLIENT_HDAW , + .name ="hdaw" , + .bpmp_id =TEGRA_ICC_BPMP_HDA , + .type =TEGRA_ICC_ISO_AUDIO , + .sid =TEGRA234_SID_HDA , + .regs = { + .sid = { + .override =0x1a8 , + .security =0x1ac , + }, + }, + }, { .id =TEGRA234_MEMORY_CLIENT_MGBEARD , .name ="mgbeard" , .bpmp_id =TEGRA_ICC_BPMP_EQOS ,
@@ -133,6 +157,90 @@ static const struct tegra_mc_client tegra234_mc_clients[] = { .security = 0x33c, }, }, + }, { + .id = TEGRA234_MEMORY_CLIENT_VI2W, + .name = "vi2w", + .bpmp_id = TEGRA_ICC_BPMP_VI2, + .type = TEGRA_ICC_ISO_VI, + .sid = TEGRA234_SID_ISO_VI2, + .regs = { + .sid = { + .override = 0x380, + .security = 0x384, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_VI2FALR, + .name = "vi2falr", + .bpmp_id = TEGRA_ICC_BPMP_VI2FAL, + .type = TEGRA_ICC_ISO_VIFAL, + .sid = TEGRA234_SID_ISO_VI2FALC, + .regs = { + .sid = { + .override = 0x388, + .security = 0x38c, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_VI2FALW, + .name = "vi2falw", + .bpmp_id = TEGRA_ICC_BPMP_VI2FAL, + .type = TEGRA_ICC_ISO_VIFAL, + .sid = TEGRA234_SID_ISO_VI2FALC, + .regs = { + .sid = { + .override = 0x3e0, + .security = 0x3e4, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_APER, + .name = "aper", + .bpmp_id = TEGRA_ICC_BPMP_APE, + .type = TEGRA_ICC_ISO_AUDIO, + .sid = TEGRA234_SID_APE, + .regs = { + .sid = { + .override = 0x3d0, + .security = 0x3d4, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_APEW, + .name = "apew", + .bpmp_id = TEGRA_ICC_BPMP_APE, + .type = TEGRA_ICC_ISO_AUDIO, + .sid = TEGRA234_SID_APE, + .regs = { + .sid = { + .override = 0x3d8, + .security = 0x3dc, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_NVDISPLAYR, + .name = "nvdisplayr", + .bpmp_id = TEGRA_ICC_BPMP_DISPLAY, + .type = TEGRA_ICC_ISO_DISPLAY, + .sid = TEGRA234_SID_ISO_NVDISPLAY, + .regs = { + .sid = { + .override = 0x490, + .security = 0x494, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_NVDISPLAYR1, + .name = "nvdisplayr1", + .bpmp_id = TEGRA_ICC_BPMP_DISPLAY, + .type = TEGRA_ICC_ISO_DISPLAY, + .sid = TEGRA234_SID_ISO_NVDISPLAY, + .regs = { + .sid = { + .override = 0x508, + .security = 0x50c, + }, + }, }, { .id = TEGRA234_MEMORY_CLIENT_BPMPR, .name = "bpmpr",
Signature · 1 line
2.17.1
PATCH 3/10 memory: tegra: add pcie mc clients for Tegra234
In reply to Sumit GuptaAdding PCIE clients representing each controller to the mc_clients table for Tegra234.
--- drivers/memory/tegra/tegra234.c | 314 +++++++++++++++++++++++++++++++- 1 file changed, 313 insertions(+), 1 deletion(-)
drivers/memory/tegra/tegra234.c +313 -1
@@ -465,7 +465,319 @@ static const struct tegra_mc_client tegra234_mc_clients[] = { .security = 0x37c, }, }, - }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE0R, + .name = "pcie0r", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_0, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE0, + .regs = { + .sid = { + .override = 0x6c0, + .security = 0x6c4, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE0W, + .name = "pcie0w", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_0, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE0, + .regs = { + .sid = { + .override = 0x6c8, + .security = 0x6cc, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE1R, + .name = "pcie1r", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_1, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE1, + .regs = { + .sid = { + .override = 0x6d0, + .security = 0x6d4, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE1W, + .name = "pcie1w", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_1, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE1, + .regs = { + .sid = { + .override = 0x6d8, + .security = 0x6dc, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE2AR, + .name = "pcie2ar", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_2, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE2, + .regs = { + .sid = { + .override = 0x6e0, + .security = 0x6e4, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE2AW, + .name = "pcie2aw", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_2, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE2, + .regs = { + .sid = { + .override = 0x6e8, + .security = 0x6ec, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE3R, + .name = "pcie3r", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_3, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE3, + .regs = { + .sid = { + .override = 0x6f0, + .security = 0x6f4, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE3W, + .name = "pcie3w", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_3, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE3, + .regs = { + .sid = { + .override = 0x6f8, + .security = 0x6fc, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE4R, + .name = "pcie4r", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_4, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE4, + .regs = { + .sid = { + .override = 0x700, + .security = 0x704, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE4W, + .name = "pcie4w", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_4, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE4, + .regs = { + .sid = { + .override = 0x708, + .security = 0x70c, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE5R, + .name = "pcie5r", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_5, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE5, + .regs = { + .sid = { + .override = 0x710, + .security = 0x714, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE5W, + .name = "pcie5w", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_5, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE5, + .regs = { + .sid = { + .override = 0x718, + .security = 0x71c, + }, + } + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE6AR, + .name = "pcie6ar", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_6, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE6, + .regs = { + .sid = { + .override = 0x140, + .security = 0x144, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE5R1, + .name = "pcie5r1", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_5, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE5, + .regs = { + .sid = { + .override = 0x778, + .security = 0x77c, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE6AW, + .name = "pcie6aw", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_6, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE6, + .regs = { + .sid = { + .override = 0x148, + .security = 0x14c, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE6AR1, + .name = "pcie6ar1", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_6, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE6, + .regs = { + .sid = { + .override = 0x1e8, + .security = 0x1ec, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE7AR, + .name = "pcie7ar", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_7, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE7, + .regs = { + .sid = { + .override = 0x150, + .security = 0x154, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE7AW, + .name = "pcie7aw", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_7, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE7, + .regs = { + .sid = { + .override = 0x180, + .security = 0x184, + }, + } + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE7AR1, + .name = "pcie7ar1", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_7, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE7, + .regs = { + .sid = { + .override = 0x248, + .security = 0x24c, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE8AR, + .name = "pcie8ar", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_8, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE8, + .regs = { + .sid = { + .override = 0x190, + .security = 0x194, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE8AW, + .name = "pcie8aw", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_8, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE8, + .regs = { + .sid = { + .override = 0x1d8, + .security = 0x1dc, + }, + } + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE9AR, + .name = "pcie9ar", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_9, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE9, + .regs = { + .sid = { + .override = 0x1e0, + .security = 0x1e4, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE9AW, + .name = "pcie9aw", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_9, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE9, + .regs = { + .sid = { + .override = 0x1f0, + .security = 0x1f4, + }, + } + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE10AR, + .name = "pcie10ar", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_10, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE10, + .regs = { + .sid = { + .override = 0x1f8, + .security = 0x1fc, + }, + }, + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE10AW, + .name = "pcie10aw", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_10, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE10, + .regs = { + .sid = { + .override = 0x200, + .security = 0x204, + }, + } + }, { + .id = TEGRA234_MEMORY_CLIENT_PCIE10AR1, + .name = "pcie10ar1", + .bpmp_id = TEGRA_ICC_BPMP_PCIE_10, + .type = TEGRA_ICC_NISO, + .sid = TEGRA234_SID_PCIE10, + .regs = { + .sid = { + .override = 0x240, + .security = 0x244, + }, + }, + } };/*
Signature · 1 line
2.17.1
Re: [Patch v1 03/10] memory: tegra: add pcie mc clients for Tegra234
Quoted 13 lines · Sumit Gupta
Adding PCIE clients representing each controller to the
mc_clientstable for Tegra234.Signed-off-by Sumit Gupta <[email protected]>--- drivers/memory/tegra/tegra234.c | 314 +++++++++++++++++++++++++++++++- 1 file changed, 313 insertions(+), 1 deletion(-)drivers/memory/tegra/tegra234.c +0
-0
drivers/memory/tegra/tegra234.c static const struct tegra_mc_client tegra234_mc_clients[] = { · line 468
Quoted 5 lines
drivers/memory/tegra/tegra234.c +1
-1@@ -465,7 +465,319 @@ static const struct tegra_mc_client tegra234_mc_clients[] = { .security = 0x37c, }, }, - }, + }, {
Didn't you change the same structure in previous patch?
Best regards,
Krzysztof
Re: [Patch v1 03/10] memory: tegra: add pcie mc clients for Tegra234
Quoted 25 lines · Krzysztof Kozlowski
External email: Use caution opening links or attachments
Quoted 13 lines · Sumit Gupta
Adding PCIE clients representing each controller to the
mc_clientstable for Tegra234.Signed-off-by Sumit Gupta <[email protected]>--- drivers/memory/tegra/tegra234.c | 314 +++++++++++++++++++++++++++++++- 1 file changed, 313 insertions(+), 1 deletion(-)drivers/memory/tegra/tegra234.c +0
-0drivers/memory/tegra/tegra234.c static const struct tegra_mc_client tegra234_mc_clients[] = { · line 468
Quoted 5 lines
drivers/memory/tegra/tegra234.c +1
-1@@ -465,7 +465,319 @@ static const struct tegra_mc_client tegra234_mc_clients[] = { .security = 0x37c, }, }, - }, + }, {Didn't you change the same structure in previous patch?
Quoted 3 lines
Best regards,
Krzysztof
Yes, kept the Iso-chronous (ISO) MC clients and PCIE (NISO) client entries in separate patches. Will merge them together in a single patch.
PATCH 4/10 memory: tegra: add support for software mc clients in Tegra234
In reply to Sumit GuptaAdding support for dummy memory controller clients for use by software.
--- drivers/memory/tegra/mc.c | 65 +++++++++++++++++++++++---------- drivers/memory/tegra/tegra234.c | 21 +++++++++++ include/soc/tegra/mc.h | 3 ++ include/soc/tegra/tegra-icc.h | 7 ++++ 4 files changed, 76 insertions(+), 20 deletions(-)
drivers/memory/tegra/mc.c +45 -20
@@ -755,6 +755,39 @@ const char *const tegra_mc_error_names[8] = { [6] = "SMMU translation error", }; +static int tegra_mc_add_icc_node (struct tegra_mc *mc,unsigned int id,const char *name, +unsigned int bpmp_id,unsigned int type) +{ +struct tegra_icc_node *tnode; +struct icc_node *node; +int err; + + tnode =kzalloc (sizeof (*tnode),GFP_KERNEL ); +if (!tnode) +return -ENOMEM ; + +/* create MC client node */ + node =icc_node_create (id); +if (IS_ERR (node)) +return -EINVAL ; + + node->name = name; +icc_node_add (node, &mc->provider); + +/* link Memory Client to Memory Controller */ + err =icc_link_create (node,TEGRA_ICC_MC ); +if (err) +return err; + + node->data = tnode; + tnode->node = node; + tnode->bpmp_id = bpmp_id; + tnode->type = type; + tnode->mc = mc; + +return 0 ; +} +/* * Memory Controller (MC) has few Memory Clients that are issuing memory * bandwidth allocation requests to the MC interconnect provider. The MC
@@ -780,7 +813,6 @@ const char *const tegra_mc_error_names[8] = {*/ static int tegra_mc_interconnect_setup (struct tegra_mc *mc) { -struct tegra_icc_node *tnode;struct icc_node *node;unsigned int i;int err;
@@ -820,30 +852,23 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc) goto remove_nodes;for (i =0 ; i < mc->soc->num_clients; i++) { - tnode =kzalloc (sizeof (*tnode),GFP_KERNEL ); -if (!tnode) -return -ENOMEM ; - -/* create MC client node */ - node =icc_node_create (mc->soc->clients[i].id); -if (IS_ERR (node)) { - err =PTR_ERR (node); + err =tegra_mc_add_icc_node (mc, mc->soc->clients[i].id, + mc->soc->clients[i].name, + mc->soc->clients[i].bpmp_id, + mc->soc->clients[i].type); +if (err) goto remove_nodes; - } - node->name = mc->soc->clients[i].name; -icc_node_add (node, &mc->provider); + } + +for (i =0 ; i < mc->soc->num_sw_clients; i++) { + err =tegra_mc_add_icc_node (mc, mc->soc->sw_clients[i].id, + mc->soc->sw_clients[i].name, + mc->soc->sw_clients[i].bpmp_id, + mc->soc->sw_clients[i].type); -/* link Memory Client to Memory Controller */ - err =icc_link_create (node,TEGRA_ICC_MC );if (err) goto remove_nodes; - - node->data = tnode; - tnode->node = node; - tnode->type = mc->soc->clients[i].type; - tnode->bpmp_id = mc->soc->clients[i].bpmp_id; - tnode->mc = mc; }return 0 ;
drivers/memory/tegra/tegra234.c +21 -0
@@ -780,6 +780,25 @@ static const struct tegra_mc_client tegra234_mc_clients[] = { } }; +static const struct tegra_mc_sw_client tegra234_mc_sw_clients[] = { + { + .id =TEGRA_ICC_MC_CPU_CLUSTER0 , + .name ="sw_cluster0" , + .bpmp_id =TEGRA_ICC_BPMP_CPU_CLUSTER0 , + .type =TEGRA_ICC_NISO , + }, { + .id =TEGRA_ICC_MC_CPU_CLUSTER1 , + .name ="sw_cluster1" , + .bpmp_id =TEGRA_ICC_BPMP_CPU_CLUSTER1 , + .type =TEGRA_ICC_NISO , + }, { + .id =TEGRA_ICC_MC_CPU_CLUSTER2 , + .name ="sw_cluster2" , + .bpmp_id =TEGRA_ICC_BPMP_CPU_CLUSTER2 , + .type =TEGRA_ICC_NISO , + }, +}; +/* * tegra234_mc_icc_set() - Pass MC client info to External Memory Controller (EMC) * @src: ICC node for Memory Controller's (MC) Client
@@ -854,6 +873,8 @@ static const struct tegra_mc_icc_ops tegra234_mc_icc_ops = {const struct tegra_mc_soc tegra234_mc_soc = { .num_clients =ARRAY_SIZE (tegra234_mc_clients), .clients = tegra234_mc_clients, + .num_sw_clients =ARRAY_SIZE (tegra234_mc_sw_clients), + .sw_clients = tegra234_mc_sw_clients, .num_address_bits =40 , .num_channels =16 , .client_id_mask =0x1ff ,
include/soc/tegra/mc.h +3 -0
@@ -192,6 +192,9 @@ struct tegra_mc_soc {const struct tegra_mc_client *clients;unsigned int num_clients; +const struct tegra_mc_sw_client *sw_clients; +unsigned int num_sw_clients; +const unsigned long *emem_regs;unsigned int num_emem_regs;
include/soc/tegra/tegra-icc.h +7 -0
@@ -22,6 +22,13 @@ struct tegra_icc_node {u32 type; }; +struct tegra_mc_sw_client { +unsigned int id; +unsigned int bpmp_id; +unsigned int type; +const char *name; +}; +/* ICC ID's for MC client's used in BPMP */ #define TEGRA_ICC_BPMP_DEBUG 1#define TEGRA_ICC_BPMP_CPU_CLUSTER0 2
Signature · 1 line
2.17.1
Re: [Patch v1 04/10] memory: tegra: add support for software mc clients in Tegra234
Quoted 2 lines · Sumit Gupta
Adding support for dummy memory controller clients for use by software.
Use imperative mode (applies to other commits as well)
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
Quoted 20 lines
--- drivers/memory/tegra/mc.c | 65 +++++++++++++++++++++++---------- drivers/memory/tegra/tegra234.c | 21 +++++++++++ include/soc/tegra/mc.h | 3 ++ include/soc/tegra/tegra-icc.h | 7 ++++ 4 files changed, 76 insertions(+), 20 deletions(-)drivers/memory/tegra/mc.c +5
-0@@ -755,6 +755,39 @@ const char *const tegra_mc_error_names[8] = { [6] = "SMMU translation error", }; +static int tegra_mc_add_icc_node (struct tegra_mc *mc,unsigned int id,const char *name, +unsigned int bpmp_id,unsigned int type) +{ +struct tegra_icc_node *tnode; +struct icc_node *node;
drivers/memory/tegra/mc.c const char *const tegra_mc_error_names[8] = { · line 772
Quoted 10 lines
drivers/memory/tegra/mc.c +10
-0+int err; + + tnode =kzalloc (sizeof (*tnode),GFP_KERNEL ); +if (!tnode) +return -ENOMEM ; + +/* create MC client node */ + node =icc_node_create (id); +if (IS_ERR (node)) +return -EINVAL ;
Why do you return other error? It does not look like you moved the code correctly, but with changes. I also do not see how this is related to commit msg...
Quoted 43 lines
drivers/memory/tegra/mc.c +20
-9+ + node->name = name; +icc_node_add (node, &mc->provider); + +/* link Memory Client to Memory Controller */ + err =icc_link_create (node,TEGRA_ICC_MC ); +if (err) +return err; + + node->data = tnode; + tnode->node = node; + tnode->bpmp_id = bpmp_id; + tnode->type = type; + tnode->mc = mc; + +return 0 ; +} +/* * Memory Controller (MC) has few Memory Clients that are issuing memory * bandwidth allocation requests to the MC interconnect provider. The MC @@ -780,7 +813,6 @@ const char *const tegra_mc_error_names[8] = {*/ static int tegra_mc_interconnect_setup (struct tegra_mc *mc) { -struct tegra_icc_node *tnode;struct icc_node *node;unsigned int i;int err;@@ -820,30 +852,23 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc) goto remove_nodes;for (i =0 ; i < mc->soc->num_clients; i++) { - tnode =kzalloc (sizeof (*tnode),GFP_KERNEL ); -if (!tnode) -return -ENOMEM ; - -/* create MC client node */ - node =icc_node_create (mc->soc->clients[i].id); -if (IS_ERR (node)) { - err =PTR_ERR (node); + err = tegra_mc_add_icc_node(mc, mc->soc->clients[i].id, + mc->soc->clients[i].name,
drivers/memory/tegra/mc.c static int tegra_mc_interconnect_setup(struct tegra_mc *mc) · line 863
Quoted 10 lines
drivers/memory/tegra/mc.c +5
-3+ mc->soc->clients[i].bpmp_id, + mc->soc->clients[i].type); +if (err) goto remove_nodes; - } - node->name = mc->soc->clients[i].name; -icc_node_add (node, &mc->provider); + } +
Best regards,
Krzysztof
Re: [Patch v1 04/10] memory: tegra: add support for software mc clients in Tegra234
Quoted 10 lines · Krzysztof Kozlowski
External email: Use caution opening links or attachments
Quoted 2 lines · Sumit Gupta
Adding support for dummy memory controller clients for use by software.
Use imperative mode (applies to other commits as well)
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
Thank you for suggesting.
I referred this and changed in v2.
Quoted 26 lines
Quoted 26 lines
--- drivers/memory/tegra/mc.c | 65 +++++++++++++++++++++++---------- drivers/memory/tegra/tegra234.c | 21 +++++++++++ include/soc/tegra/mc.h | 3 ++ include/soc/tegra/tegra-icc.h | 7 ++++ 4 files changed, 76 insertions(+), 20 deletions(-)drivers/memory/tegra/mc.c +11
-0@@ -755,6 +755,39 @@ const char *const tegra_mc_error_names[8] = { [6] = "SMMU translation error", }; +static int tegra_mc_add_icc_node (struct tegra_mc *mc,unsigned int id,const char *name, +unsigned int bpmp_id,unsigned int type) +{ +struct tegra_icc_node *tnode; +struct icc_node *node; +int err; + + tnode =kzalloc (sizeof (*tnode),GFP_KERNEL ); +if (!tnode) +return -ENOMEM ; +
Quoted 9 lines
drivers/memory/tegra/mc.c const char *const tegra_mc_error_names[8] = { · line 772
Quoted 4 lines
drivers/memory/tegra/mc.c +4
-0+/* create MC client node */ + node =icc_node_create (id); +if (IS_ERR (node)) +return -EINVAL ;Why do you return other error? It does not look like you moved the code correctly, but with changes. I also do not see how this is related to commit msg...
Corrected in v2.
Thanks,
Sumit
Quoted 57 lines
Quoted 43 lines
drivers/memory/tegra/mc.c +20
-9+ + node->name = name; +icc_node_add (node, &mc->provider); + +/* link Memory Client to Memory Controller */ + err =icc_link_create (node,TEGRA_ICC_MC ); +if (err) +return err; + + node->data = tnode; + tnode->node = node; + tnode->bpmp_id = bpmp_id; + tnode->type = type; + tnode->mc = mc; + +return 0 ; +} +/* * Memory Controller (MC) has few Memory Clients that are issuing memory * bandwidth allocation requests to the MC interconnect provider. The MC @@ -780,7 +813,6 @@ const char *const tegra_mc_error_names[8] = {*/ static int tegra_mc_interconnect_setup (struct tegra_mc *mc) { -struct tegra_icc_node *tnode;struct icc_node *node;unsigned int i;int err;@@ -820,30 +852,23 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc) goto remove_nodes;for (i =0 ; i < mc->soc->num_clients; i++) { - tnode =kzalloc (sizeof (*tnode),GFP_KERNEL ); -if (!tnode) -return -ENOMEM ; - -/* create MC client node */ - node =icc_node_create (mc->soc->clients[i].id); -if (IS_ERR (node)) { - err =PTR_ERR (node); + err = tegra_mc_add_icc_node(mc, mc->soc->clients[i].id, + mc->soc->clients[i].name,drivers/memory/tegra/mc.c static int tegra_mc_interconnect_setup(struct tegra_mc *mc) · line 863
Quoted 10 lines
drivers/memory/tegra/mc.c +5
-3+ mc->soc->clients[i].bpmp_id, + mc->soc->clients[i].type); +if (err) goto remove_nodes; - } - node->name = mc->soc->clients[i].name; -icc_node_add (node, &mc->provider); + } +Best regards,
Krzysztof
PATCH 5/10 dt-bindings: tegra: add icc ids for dummy MC clients
In reply to Sumit GuptaAdding ICC id's for dummy software clients representing CCPLEX clusters.
--- include/dt-bindings/memory/tegra234-mc.h | 5 +++++ 1 file changed, 5 insertions(+)
include/dt-bindings/memory/tegra234-mc.h +5 -0
@@ -536,4 +536,9 @@#define TEGRA234_MEMORY_CLIENT_NVJPG1SRD 0x123#define TEGRA234_MEMORY_CLIENT_NVJPG1SWR 0x124 +/* ICC ID's for dummy MC clients used to represent CPU Clusters */ +#define TEGRA_ICC_MC_CPU_CLUSTER0 1003 +#define TEGRA_ICC_MC_CPU_CLUSTER1 1004 +#define TEGRA_ICC_MC_CPU_CLUSTER2 1005 +#endif
Signature · 1 line
2.17.1
Re: [Patch v1 05/10] dt-bindings: tegra: add icc ids for dummy MC clients
Quoted 12 lines · Sumit Gupta
Adding ICC id's for dummy software clients representing CCPLEX clusters.
Signed-off-by Sumit Gupta <[email protected]>--- include/dt-bindings/memory/tegra234-mc.h | 5 +++++ 1 file changed, 5 insertions(+)include/dt-bindings/memory/tegra234-mc.h +0
-0
include/dt-bindings/memory/tegra234-mc.h line 540
Quoted 5 lines
include/dt-bindings/memory/tegra234-mc.h +2
-0@@ -536,4 +536,9 @@#define TEGRA234_MEMORY_CLIENT_NVJPG1SRD 0x123#define TEGRA234_MEMORY_CLIENT_NVJPG1SWR 0x124 +/* ICC ID's for dummy MC clients used to represent CPU Clusters */ +#define TEGRA_ICC_MC_CPU_CLUSTER0 1003
Why the IDs do not start from 0?
Best regards,
Krzysztof
Re: [Patch v1 05/10] dt-bindings: tegra: add icc ids for dummy MC clients
Quoted 19 lines · Krzysztof Kozlowski
External email: Use caution opening links or attachments
Quoted 15 lines · Sumit Gupta
Adding ICC id's for dummy software clients representing CCPLEX clusters.
Signed-off-by Sumit Gupta <[email protected]>--- include/dt-bindings/memory/tegra234-mc.h | 5 +++++ 1 file changed, 5 insertions(+)include/dt-bindings/memory/tegra234-mc.h +0
-0@@ -536,4 +536,9 @@#define TEGRA234_MEMORY_CLIENT_NVJPG1SRD 0x123#define TEGRA234_MEMORY_CLIENT_NVJPG1SWR 0x124
Quoted 8 lines
include/dt-bindings/memory/tegra234-mc.h line 540
Quoted 2 lines
include/dt-bindings/memory/tegra234-mc.h +2
-0+/* ICC ID's for dummy MC clients used to represent CPU Clusters */ +#define TEGRA_ICC_MC_CPU_CLUSTER0 1003Why the IDs do not start from 0?
Best regards,
Krzysztof
MC client ID's are starting from zero. These ID's are used as "icc_node->id" while creating the icc_node. So, can't use the duplicate numbers.
$ grep TEGRA234_MEMORY_ "include/dt-bindings/memory/tegra234- mc.h" #define TEGRA234_MEMORY_CLIENT_PTCR 0x00 #define TEGRA234_MEMORY_CLIENT_MIU7R 0x01 #define TEGRA234_MEMORY_CLIENT_MIU7W 0x02 #define TEGRA234_MEMORY_CLIENT_MIU8R 0x03 ....
Dummy ID's which are already being used are starting from '1000'. So, added to that number as the ID's used for creating "icc_node" for CPU clusters are also dummy and they are not part of HW MC client's list.
$ grep "100[0-9]" ./drivers/memory/tegra/mc.h #define TEGRA_ICC_MC 1000 #define TEGRA_ICC_EMC 1001 #define TEGRA_ICC_EMEM 1002
Re: [Patch v1 05/10] dt-bindings: tegra: add icc ids for dummy MC clients
In reply to Sumit GuptaQuoted 4 lines · Sumit Gupta
Adding ICC id's for dummy software clients representing CCPLEX clusters.
Signed-off-by Sumit Gupta <[email protected]>---
Best regards,
Krzysztof
PATCH 6/10 arm64: tegra: Add cpu OPP tables and interconnects property
In reply to Sumit GuptaAdd OPP table and interconnects property required to scale DDR frequency for better performance. The OPP table has CPU frequency to per MC channel bandwidth mapping in each operating point entry. One table is added for each cluster even though the table data is same because the bandwidth request is per cluster. OPP framework is creating a single icc path if the table is marked 'opp-shared' and shared among all clusters. For us the OPP table is same but the MC client ID argument to interconnects property is different for each cluster which makes different icc path for all.
--- arch/arm64/boot/dts/nvidia/tegra234.dtsi | 276 +++++++++++++++++++++++ 1 file changed, 276 insertions(+)
arch/arm64/boot/dts/nvidia/tegra234.dtsi +276 -0
@@ -2840,6 +2840,9 @@ enable-method = "psci"; + operating-points-v2 = <&cl0_opp_tbl>; + interconnects = <&mc TEGRA_ICC_MC_CPU_CLUSTER0 &emc>; + i-cache-size = <65536>; i-cache-line-size = <64>; i-cache-sets = <256>;
@@ -2856,6 +2859,9 @@ enable-method = "psci"; + operating-points-v2 = <&cl0_opp_tbl>; + interconnects = <&mc TEGRA_ICC_MC_CPU_CLUSTER0 &emc>; + i-cache-size = <65536>; i-cache-line-size = <64>; i-cache-sets = <256>;
@@ -2872,6 +2878,9 @@ enable-method = "psci"; + operating-points-v2 = <&cl0_opp_tbl>; + interconnects = <&mc TEGRA_ICC_MC_CPU_CLUSTER0 &emc>; + i-cache-size = <65536>; i-cache-line-size = <64>; i-cache-sets = <256>;
@@ -2888,6 +2897,9 @@ enable-method = "psci"; + operating-points-v2 = <&cl0_opp_tbl>; + interconnects = <&mc TEGRA_ICC_MC_CPU_CLUSTER0 &emc>; + i-cache-size = <65536>; i-cache-line-size = <64>; i-cache-sets = <256>;
@@ -2904,6 +2916,9 @@ enable-method = "psci"; + operating-points-v2 = <&cl1_opp_tbl>; + interconnects = <&mc TEGRA_ICC_MC_CPU_CLUSTER1 &emc>; + i-cache-size = <65536>; i-cache-line-size = <64>; i-cache-sets = <256>;
@@ -2920,6 +2935,9 @@ enable-method = "psci"; + operating-points-v2 = <&cl1_opp_tbl>; + interconnects = <&mc TEGRA_ICC_MC_CPU_CLUSTER1 &emc>; + i-cache-size = <65536>; i-cache-line-size = <64>; i-cache-sets = <256>;
@@ -2936,6 +2954,9 @@ enable-method = "psci"; + operating-points-v2 = <&cl1_opp_tbl>; + interconnects = <&mc TEGRA_ICC_MC_CPU_CLUSTER1 &emc>; + i-cache-size = <65536>; i-cache-line-size = <64>; i-cache-sets = <256>;
@@ -2952,6 +2973,9 @@ enable-method = "psci"; + operating-points-v2 = <&cl1_opp_tbl>; + interconnects = <&mc TEGRA_ICC_MC_CPU_CLUSTER1 &emc>; + i-cache-size = <65536>; i-cache-line-size = <64>; i-cache-sets = <256>;
@@ -2968,6 +2992,9 @@ enable-method = "psci"; + operating-points-v2 = <&cl2_opp_tbl>; + interconnects = <&mc TEGRA_ICC_MC_CPU_CLUSTER2 &emc>; + i-cache-size = <65536>; i-cache-line-size = <64>; i-cache-sets = <256>;
@@ -2984,6 +3011,9 @@ enable-method = "psci"; + operating-points-v2 = <&cl2_opp_tbl>; + interconnects = <&mc TEGRA_ICC_MC_CPU_CLUSTER2 &emc>; + i-cache-size = <65536>; i-cache-line-size = <64>; i-cache-sets = <256>;
@@ -3000,6 +3030,9 @@ enable-method = "psci"; + operating-points-v2 = <&cl2_opp_tbl>; + interconnects = <&mc TEGRA_ICC_MC_CPU_CLUSTER2 &emc>; + i-cache-size = <65536>; i-cache-line-size = <64>; i-cache-sets = <256>;
@@ -3016,6 +3049,9 @@ enable-method = "psci"; + operating-points-v2 = <&cl2_opp_tbl>; + interconnects = <&mc TEGRA_ICC_MC_CPU_CLUSTER2 &emc>; + i-cache-size = <65536>; i-cache-line-size = <64>; i-cache-sets = <256>;
@@ -3272,4 +3308,244 @@ interrupt-parent = <&gic>; always-on; }; + + cl0_opp_tbl: opp-table-cluster0 { + compatible = "operating-points-v2"; + opp-shared; + + cl0_ch1_opp1: opp-115200000 { + opp-hz = /bits/ 64 <115200000>; + opp-peak-kBps = <816000>; + }; + + cl0_ch1_opp2: opp-268800000 { + opp-hz = /bits/ 64 <268800000>; + opp-peak-kBps = <816000>; + }; + + cl0_ch1_opp3: opp-422400000 { + opp-hz = /bits/ 64 <422400000>; + opp-peak-kBps = <816000>; + }; + + cl0_ch1_opp4: opp-576000000 { + opp-hz = /bits/ 64 <576000000>; + opp-peak-kBps = <816000>; + }; + + cl0_ch1_opp5: opp-729600000 { + opp-hz = /bits/ 64 <729600000>; + opp-peak-kBps = <816000>; + }; + + cl0_ch1_opp6: opp-883200000 { + opp-hz = /bits/ 64 <883200000>; + opp-peak-kBps = <816000>; + }; + + cl0_ch1_opp7: opp-1036800000 { + opp-hz = /bits/ 64 <1036800000>; + opp-peak-kBps = <816000>; + }; + + cl0_ch1_opp8: opp-1190400000 { + opp-hz = /bits/ 64 <1190400000>; + opp-peak-kBps = <816000>; + }; + + cl0_ch1_opp9: opp-1344000000 { + opp-hz = /bits/ 64 <1344000000>; + opp-peak-kBps = <1632000>; + }; + + cl0_ch1_opp10: opp-1497600000 { + opp-hz = /bits/ 64 <1497600000>; + opp-peak-kBps = <1632000>; + }; + + cl0_ch1_opp11: opp-1651200000 { + opp-hz = /bits/ 64 <1651200000>; + opp-peak-kBps = <2660000>; + }; + + cl0_ch1_opp12: opp-1804800000 { + opp-hz = /bits/ 64 <1804800000>; + opp-peak-kBps = <2660000>; + }; + + cl0_ch1_opp13: opp-1958400000 { + opp-hz = /bits/ 64 <1958400000>; + opp-peak-kBps = <3200000>; + }; + + cl0_ch1_opp14: opp-2112000000 { + opp-hz = /bits/ 64 <2112000000>; + opp-peak-kBps = <6400000>; + }; + + cl0_ch1_opp15: opp-2201600000 { + opp-hz = /bits/ 64 <2201600000>; + opp-peak-kBps = <6400000>; + }; + }; + + cl1_opp_tbl: opp-table-cluster1 { + compatible = "operating-points-v2"; + opp-shared; + + cl1_ch1_opp1: opp-115200000 { + opp-hz = /bits/ 64 <115200000>; + opp-peak-kBps = <816000>; + }; + + cl1_ch1_opp2: opp-268800000 { + opp-hz = /bits/ 64 <268800000>; + opp-peak-kBps = <816000>; + }; + + cl1_ch1_opp3: opp-422400000 { + opp-hz = /bits/ 64 <422400000>; + opp-peak-kBps = <816000>; + }; + + cl1_ch1_opp4: opp-576000000 { + opp-hz = /bits/ 64 <576000000>; + opp-peak-kBps = <816000>; + }; + + cl1_ch1_opp5: opp-729600000 { + opp-hz = /bits/ 64 <729600000>; + opp-peak-kBps = <816000>; + }; + + cl1_ch1_opp6: opp-883200000 { + opp-hz = /bits/ 64 <883200000>; + opp-peak-kBps = <816000>; + }; + + cl1_ch1_opp7: opp-1036800000 { + opp-hz = /bits/ 64 <1036800000>; + opp-peak-kBps = <816000>; + }; + + cl1_ch1_opp8: opp-1190400000 { + opp-hz = /bits/ 64 <1190400000>; + opp-peak-kBps = <816000>; + }; + + cl1_ch1_opp9: opp-1344000000 { + opp-hz = /bits/ 64 <1344000000>; + opp-peak-kBps = <1632000>; + }; + + cl1_ch1_opp10: opp-1497600000 { + opp-hz = /bits/ 64 <1497600000>; + opp-peak-kBps = <1632000>; + }; + + cl1_ch1_opp11: opp-1651200000 { + opp-hz = /bits/ 64 <1651200000>; + opp-peak-kBps = <2660000>; + }; + + cl1_ch1_opp12: opp-1804800000 { + opp-hz = /bits/ 64 <1804800000>; + opp-peak-kBps = <2660000>; + }; + + cl1_ch1_opp13: opp-1958400000 { + opp-hz = /bits/ 64 <1958400000>; + opp-peak-kBps = <3200000>; + }; + + cl1_ch1_opp14: opp-2112000000 { + opp-hz = /bits/ 64 <2112000000>; + opp-peak-kBps = <6400000>; + }; + + cl1_ch1_opp15: opp-2201600000 { + opp-hz = /bits/ 64 <2201600000>; + opp-peak-kBps = <6400000>; + }; + }; + + cl2_opp_tbl: opp-table-cluster2 { + compatible = "operating-points-v2"; + opp-shared; + + cl2_ch1_opp1: opp-115200000 { + opp-hz = /bits/ 64 <115200000>; + opp-peak-kBps = <816000>; + }; + + cl2_ch1_opp2: opp-268800000 { + opp-hz = /bits/ 64 <268800000>; + opp-peak-kBps = <816000>; + }; + + cl2_ch1_opp3: opp-422400000 { + opp-hz = /bits/ 64 <422400000>; + opp-peak-kBps = <816000>; + }; + + cl2_ch1_opp4: opp-576000000 { + opp-hz = /bits/ 64 <576000000>; + opp-peak-kBps = <816000>; + }; + + cl2_ch1_opp5: opp-729600000 { + opp-hz = /bits/ 64 <729600000>; + opp-peak-kBps = <816000>; + }; + + cl2_ch1_opp6: opp-883200000 { + opp-hz = /bits/ 64 <883200000>; + opp-peak-kBps = <816000>; + }; + + cl2_ch1_opp7: opp-1036800000 { + opp-hz = /bits/ 64 <1036800000>; + opp-peak-kBps = <816000>; + }; + + cl2_ch1_opp8: opp-1190400000 { + opp-hz = /bits/ 64 <1190400000>; + opp-peak-kBps = <816000>; + }; + + cl2_ch1_opp9: opp-1344000000 { + opp-hz = /bits/ 64 <1344000000>; + opp-peak-kBps = <1632000>; + }; + + cl2_ch1_opp10: opp-1497600000 { + opp-hz = /bits/ 64 <1497600000>; + opp-peak-kBps = <1632000>; + }; + + cl2_ch1_opp11: opp-1651200000 { + opp-hz = /bits/ 64 <1651200000>; + opp-peak-kBps = <2660000>; + }; + + cl2_ch1_opp12: opp-1804800000 { + opp-hz = /bits/ 64 <1804800000>; + opp-peak-kBps = <2660000>; + }; + + cl2_ch1_opp13: opp-1958400000 { + opp-hz = /bits/ 64 <1958400000>; + opp-peak-kBps = <3200000>; + }; + + cl2_ch1_opp14: opp-2112000000 { + opp-hz = /bits/ 64 <2112000000>; + opp-peak-kBps = <6400000>; + }; + + cl2_ch1_opp15: opp-2201600000 { + opp-hz = /bits/ 64 <2201600000>; + opp-peak-kBps = <6400000>; + }; + }; };
Signature · 1 line
2.17.1
Re: [Patch v1 06/10] arm64: tegra: Add cpu OPP tables and interconnects property
Quoted 20 lines · Sumit Gupta
Add OPP table and interconnects property required to scale DDR frequency for better performance. The OPP table has CPU frequency to per MC channel bandwidth mapping in each operating point entry. One table is added for each cluster even though the table data is same because the bandwidth request is per cluster. OPP framework is creating a single icc path if the table is marked 'opp-shared' and shared among all clusters. For us the OPP table is same but the MC client ID argument to interconnects property is different for each cluster which makes different icc path for all.
Signed-off-by Sumit Gupta <[email protected]>--- arch/arm64/boot/dts/nvidia/tegra234.dtsi | 276 +++++++++++++++++++++++ 1 file changed, 276 insertions(+)arch/arm64/boot/dts/nvidia/tegra234.dtsi +0
-0
arch/arm64/boot/dts/nvidia/tegra234.dtsi line 2844
Quoted 5 lines
arch/arm64/boot/dts/nvidia/tegra234.dtsi +2
-0@@ -2840,6 +2840,9 @@ enable-method = "psci"; + operating-points-v2 = <&cl0_opp_tbl>; + interconnects = <&mc TEGRA_ICC_MC_CPU_CLUSTER0 &emc>;
I dislike how this muddies the water between hardware and software description. We don't have a hardware client ID for the CPU clusters, so there's no good way to describe this in a hardware-centric way. We used to have MPCORE read and write clients for this, but as far as I know they used to be for the entire CCPLEX rather than per-cluster. It'd be interesting to know what the BPMP does underneath, perhaps that could give some indication as to what would be a better hardware value to use for this.
Failing that, I wonder if a combination of icc_node_create() and icc_get() can be used for this type of "virtual node" special case.
Thierry
PATCH 7/10 cpufreq: Add Tegra234 to cpufreq-dt-platdev blocklist
In reply to Sumit GuptaTegra234 platform uses the tegra194-cpufreq driver, so add it to the blocklist in cpufreq-dt-platdev driver to avoid the cpufreq driver registration from there.
--- drivers/cpufreq/cpufreq-dt-platdev.c | 1 + 1 file changed, 1 insertion(+)
drivers/cpufreq/cpufreq-dt-platdev.c +1 -0
@@ -137,6 +137,7 @@ static const struct of_device_id blocklist[] __initconst = { { .compatible = "nvidia,tegra30", }, { .compatible = "nvidia,tegra124", }, { .compatible = "nvidia,tegra210", }, + { .compatible = "nvidia,tegra234", }, { .compatible = "qcom,apq8096", }, { .compatible = "qcom,msm8996", },
Signature · 1 line
2.17.1
Re: [Patch v1 07/10] cpufreq: Add Tegra234 to cpufreq-dt-platdev blocklist
Quoted 14 lines · Sumit Gupta
Tegra234 platform uses the tegra194-cpufreq driver, so add it to the blocklist in cpufreq-dt-platdev driver to avoid the cpufreq driver registration from there.
Signed-off-by Sumit Gupta <[email protected]>--- drivers/cpufreq/cpufreq-dt-platdev.c | 1 + 1 file changed, 1 insertion(+)drivers/cpufreq/cpufreq-dt-platdev.c +0
-0
drivers/cpufreq/cpufreq-dt-platdev.c static const struct of_device_id blocklist[] __initconst = { · line 143
Quoted 7 lines
drivers/cpufreq/cpufreq-dt-platdev.c +1
-0@@ -137,6 +137,7 @@ static const struct of_device_id blocklist[] __initconst = { { .compatible = "nvidia,tegra30", }, { .compatible = "nvidia,tegra124", }, { .compatible = "nvidia,tegra210", }, + { .compatible = "nvidia,tegra234", }, { .compatible = "qcom,apq8096", }, { .compatible = "qcom,msm8996", },
Applied. Thanks.
Signature · 1 line
viresh
PATCH 8/10 cpufreq: tegra194: add OPP support and set bandwidth
In reply to Sumit GuptaAdd support to use OPP table from DT in Tegra194 cpufreq driver. Tegra SoC's receive the frequency lookup table (LUT) from BPMP-FW. Cross check the OPP's present in DT against the LUT from BPMP-FW and enable only those DT OPP's which are present in LUT also.
The OPP table in DT has CPU Frequency to bandwidth mapping where the bandwidth value is per MC channel. DRAM bandwidth depends on the number of MC channels which can vary as per the boot configuration. This per channel bandwidth from OPP table will be later converted by MC driver to final bandwidth value by multiplying with number of channels before sending the request to BPMP-FW.
If OPP table is not present in DT, then use the LUT from BPMP-FW directy as the frequency table and not do the DRAM frequency scaling which is same as the current behavior.
Now, as the CPU Frequency table is being controlling through OPP table in DT. Keeping fewer entries in the table will create less frequency steps and scale fast to high frequencies if required.
--- drivers/cpufreq/tegra194-cpufreq.c | 152 ++++++++++++++++++++++++++--- 1 file changed, 139 insertions(+), 13 deletions(-)
drivers/cpufreq/tegra194-cpufreq.c +139 -13
@@ -12,6 +12,7 @@#include <linux/of_platform.h> #include <linux/platform_device.h> #include <linux/slab.h> +#include <linux/units.h> #include <asm/smp_plat.h>
@@ -65,12 +66,32 @@ struct tegra_cpufreq_soc {struct tegra194_cpufreq_data {void __iomem *regs; -struct cpufreq_frequency_table **tables; +struct cpufreq_frequency_table **bpmp_luts;const struct tegra_cpufreq_soc *soc; +bool icc_dram_bw_scaling; };static struct workqueue_struct *read_counters_wq; +static int tegra_cpufreq_set_bw (struct cpufreq_policy *policy,unsigned long freq_khz) +{ +struct dev_pm_opp *opp; +struct device *dev; +int ret; + + dev =get_cpu_device (policy->cpu); +if (!dev) +return -ENODEV ; + + opp =dev_pm_opp_find_freq_exact (dev, freq_khz *KHZ , true); +if (IS_ERR (opp)) +return PTR_ERR (opp); + + ret =dev_pm_opp_set_opp (dev, opp); +dev_pm_opp_put (opp); +return ret; +} +static void tegra_get_cpu_mpidr (void *mpidr) { *((u64 *)mpidr) =read_cpuid_mpidr () &MPIDR_HWID_BITMASK ;
@@ -354,7 +375,7 @@ static unsigned int tegra194_get_speed(u32 cpu)* to the last written ndiv value from freq_table. This is * done to return consistent value. */ -cpufreq_for_each_valid_entry (pos, data->tables[clusterid]) { +cpufreq_for_each_valid_entry (pos, data->bpmp_luts[clusterid]) {if (pos->driver_data != ndiv)continue ;
@@ -369,16 +390,93 @@ static unsigned int tegra194_get_speed(u32 cpu)return rate; } +int tegra_cpufreq_init_cpufreq_table (struct cpufreq_policy *policy, +struct cpufreq_frequency_table *bpmp_lut, +struct cpufreq_frequency_table **opp_table) +{ +struct tegra194_cpufreq_data *data =cpufreq_get_driver_data (); +struct cpufreq_frequency_table *freq_table =NULL ; +struct cpufreq_frequency_table *pos; +int ret =0 , max_opps =0 ; +struct device *cpu_dev; +struct dev_pm_opp *opp; +unsigned long rate; +int j =0 ; + + cpu_dev =get_cpu_device (policy->cpu); +if (!cpu_dev) { +pr_err ("%s: failed to get cpu%d device\n" , __func__, policy->cpu); +return -ENODEV ; + } + +/* Initialize OPP table mentioned in operating-points-v2 property in DT */ + ret =dev_pm_opp_of_add_table_indexed (cpu_dev,0 ); +if (!ret) { + max_opps =dev_pm_opp_get_opp_count (cpu_dev); +if (max_opps <=0 ) { +dev_err (cpu_dev,"Failed to add OPPs\n" ); +return max_opps; + } + +/* Disable all opps and cross-validate against LUT later */ +for (rate =0 ; ; rate++) { + opp =dev_pm_opp_find_freq_ceil (cpu_dev, &rate); +if (IS_ERR (opp)) +break ; + +dev_pm_opp_put (opp); +dev_pm_opp_disable (cpu_dev, rate); + } + }else { +dev_err (cpu_dev,"Invalid or empty opp table in device tree\n" ); + data->icc_dram_bw_scaling = false; +return ret; + } + + freq_table =kcalloc ((max_opps +1 ),sizeof (*freq_table),GFP_KERNEL ); +if (!freq_table) +return -ENOMEM ; + +/* +* Cross check the frequencies from BPMP-FW LUT against the OPP's present in DT. +* Enable only those DT OPP's which are present in LUT also. +*/ +cpufreq_for_each_valid_entry (pos, bpmp_lut) { + opp =dev_pm_opp_find_freq_exact (cpu_dev, pos->frequency *KHZ , false); +if (IS_ERR (opp)) +continue ; + + ret =dev_pm_opp_enable (cpu_dev, pos->frequency *KHZ ); +if (ret <0 ) +return ret; + + freq_table[j].driver_data = pos->driver_data; + freq_table[j].frequency = pos->frequency; + j++; + } + + freq_table[j].driver_data = pos->driver_data; + freq_table[j].frequency =CPUFREQ_TABLE_END ; + + *opp_table = &freq_table[0 ]; + +dev_pm_opp_set_sharing_cpus (cpu_dev, policy->cpus); + +return ret; +} +static int tegra194_cpufreq_init (struct cpufreq_policy *policy) {struct tegra194_cpufreq_data *data =cpufreq_get_driver_data ();int maxcpus_per_cluster = data->soc->maxcpus_per_cluster; +struct cpufreq_frequency_table *freq_table; +struct cpufreq_frequency_table *bpmp_lut;u32 start_cpu, cpu;u32 clusterid; +int ret; data->soc->ops->get_cpu_cluster_id (policy->cpu,NULL , &clusterid); - -if (clusterid >= data->soc->num_clusters || !data->tables[clusterid]) +if (clusterid >= data->soc->num_clusters || !data->bpmp_luts[clusterid])return -EINVAL ; start_cpu =rounddown (policy->cpu, maxcpus_per_cluster);
@@ -387,9 +485,22 @@ static int tegra194_cpufreq_init(struct cpufreq_policy *policy)if (cpu_possible (cpu))cpumask_set_cpu (cpu, policy->cpus); } - policy->freq_table = data->tables[clusterid]; policy->cpuinfo.transition_latency =TEGRA_CPUFREQ_TRANSITION_LATENCY ; + bpmp_lut = data->bpmp_luts[clusterid]; + +if (data->icc_dram_bw_scaling) { + ret =tegra_cpufreq_init_cpufreq_table (policy, bpmp_lut, &freq_table); +if (!ret) { + policy->freq_table = freq_table; +return 0 ; + } + } + + data->icc_dram_bw_scaling = false; + policy->freq_table = bpmp_lut; +pr_info ("OPP tables missing from DT, EMC frequency scaling disabled\n" ); +return 0 ; }
@@ -406,6 +517,9 @@ static int tegra194_cpufreq_set_target(struct cpufreq_policy *policy,*/ data->soc->ops->set_cpu_ndiv (policy, (u64 )tbl->driver_data); +if (data->icc_dram_bw_scaling) +tegra_cpufreq_set_bw (policy, tbl->frequency); +return 0 ; }
@@ -438,8 +552,8 @@ static void tegra194_cpufreq_free_resources(void) }static struct cpufreq_frequency_table * -init_freq_table (struct platform_device *pdev,struct tegra_bpmp *bpmp, -unsigned int cluster_id) +tegra_cpufreq_bpmp_read_lut (struct platform_device *pdev,struct tegra_bpmp *bpmp, +unsigned int cluster_id) {struct cpufreq_frequency_table *freq_table;struct mrq_cpu_ndiv_limits_response resp;
@@ -514,6 +628,7 @@ static int tegra194_cpufreq_probe(struct platform_device *pdev)const struct tegra_cpufreq_soc *soc;struct tegra194_cpufreq_data *data;struct tegra_bpmp *bpmp; +struct device *cpu_dev;int err, i; data =devm_kzalloc (&pdev->dev,sizeof (*data),GFP_KERNEL );
@@ -529,9 +644,9 @@ static int tegra194_cpufreq_probe(struct platform_device *pdev)return -EINVAL ; } - data->tables =devm_kcalloc (&pdev->dev, data->soc->num_clusters, -sizeof (*data->tables),GFP_KERNEL ); -if (!data->tables) + data->bpmp_luts =devm_kcalloc (&pdev->dev, data->soc->num_clusters, +sizeof (*data->bpmp_luts),GFP_KERNEL ); +if (!data->bpmp_luts)return -ENOMEM ;if (soc->actmon_cntr_base) {
@@ -555,15 +670,26 @@ static int tegra194_cpufreq_probe(struct platform_device *pdev) }for (i =0 ; i < data->soc->num_clusters; i++) { - data->tables[i] =init_freq_table (pdev, bpmp, i); -if (IS_ERR (data->tables[i])) { - err =PTR_ERR (data->tables[i]); + data->bpmp_luts[i] =tegra_cpufreq_bpmp_read_lut (pdev, bpmp, i); +if (IS_ERR (data->bpmp_luts[i])) { + err =PTR_ERR (data->bpmp_luts[i]); goto err_free_res; } } tegra194_cpufreq_driver.driver_data = data; +/* Check for optional OPPv2 and interconnect paths on CPU0 to enable ICC scaling */ + cpu_dev =get_cpu_device (0 ); +if (!cpu_dev) +return -EPROBE_DEFER ; + +if (dev_pm_opp_of_get_opp_desc_node (cpu_dev)) { + err =dev_pm_opp_of_find_icc_paths (cpu_dev,NULL ); +if (!err) + data->icc_dram_bw_scaling = true; + } + err =cpufreq_register_driver (&tegra194_cpufreq_driver);if (!err) goto put_bpmp;
Signature · 1 line
2.17.1
Re: [Patch v1 08/10] cpufreq: tegra194: add OPP support and set bandwidth
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 12 lines
Add support to use OPP table from DT in Tegra194 cpufreq driver. Tegra SoC's receive the frequency lookup table (LUT) from BPMP-FW. Cross check the OPP's present in DT against the LUT from BPMP-FW and enable only those DT OPP's which are present in LUT also.
The OPP table in DT has CPU Frequency to bandwidth mapping where the bandwidth value is per MC channel. DRAM bandwidth depends on the number of MC channels which can vary as per the boot configuration. This per channel bandwidth from OPP table will be later converted by MC driver to final bandwidth value by multiplying with number of channels before sending the request to BPMP-FW.
Quoted 7 lines
If OPP table is not present in DT, then use the LUT from BPMP-FW directy as the frequency table and not do the DRAM frequency scaling which is same as the current behavior.
Now, as the CPU Frequency table is being controlling through OPP table in DT. Keeping fewer entries in the table will create less frequency steps and scale fast to high frequencies if required.
It's not exactly clear what you're doing here. Are you going to scale memory BW based on CPU freq? If yes, then this is wrong because CPU freq is independent from the memory subsystem.
All Tegra30+ SoCs have ACTMON hardware unit that monitors CPU memory activity and CPU memory BW should be scaled based on CPU memory events counter. We have ACTMON devfreq driver for older SoCs. I have no clue how ACTMON can be accessed on T186+, perhaps there should be a BPMP FW API for that.
Re: [Patch v1 08/10] cpufreq: tegra194: add OPP support and set bandwidth
Quoted 28 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 12 lines
Add support to use OPP table from DT in Tegra194 cpufreq driver. Tegra SoC's receive the frequency lookup table (LUT) from BPMP-FW. Cross check the OPP's present in DT against the LUT from BPMP-FW and enable only those DT OPP's which are present in LUT also.
The OPP table in DT has CPU Frequency to bandwidth mapping where the bandwidth value is per MC channel. DRAM bandwidth depends on the number of MC channels which can vary as per the boot configuration. This per channel bandwidth from OPP table will be later converted by MC driver to final bandwidth value by multiplying with number of channels before sending the request to BPMP-FW.
Quoted 7 lines
If OPP table is not present in DT, then use the LUT from BPMP-FW directy as the frequency table and not do the DRAM frequency scaling which is same as the current behavior.
Now, as the CPU Frequency table is being controlling through OPP table in DT. Keeping fewer entries in the table will create less frequency steps and scale fast to high frequencies if required.
It's not exactly clear what you're doing here. Are you going to scale memory BW based on CPU freq? If yes, then this is wrong because CPU freq is independent from the memory subsystem.
Quoted 6 lines
All Tegra30+ SoCs have ACTMON hardware unit that monitors CPU memory activity and CPU memory BW should be scaled based on CPU memory events counter. We have ACTMON devfreq driver for older SoCs. I have no clue how ACTMON can be accessed on T186+, perhaps there should be a BPMP FW API for that.
Yes, scaling the memory BW based on CPU freq.
Referred below patch set for previous generation of Tegra Soc's which you mentioned and tried to trace the history.
https://patchwork.ozlabs.org/project/linux-tegra/patch/[email protected]/
In new Tegra Soc's, actmon counter control and usage has been moved to BPMP-FW where only 'MCALL' counter is used and 'MCCPU is not being used. Using the actmon counter was a reactive way to scale the frequency which is less effective due to averaging over a time period. We are now using the proactive way where clients tell their bandwidth
needs to help achieve better performance.
Re: [Patch v1 08/10] cpufreq: tegra194: add OPP support and set bandwidth
Quoted 42 lines · Sumit Gupta
Quoted 28 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 12 lines
Add support to use OPP table from DT in Tegra194 cpufreq driver. Tegra SoC's receive the frequency lookup table (LUT) from BPMP-FW. Cross check the OPP's present in DT against the LUT from BPMP-FW and enable only those DT OPP's which are present in LUT also.
The OPP table in DT has CPU Frequency to bandwidth mapping where the bandwidth value is per MC channel. DRAM bandwidth depends on the number of MC channels which can vary as per the boot configuration. This per channel bandwidth from OPP table will be later converted by MC driver to final bandwidth value by multiplying with number of channels before sending the request to BPMP-FW.
Quoted 7 lines
If OPP table is not present in DT, then use the LUT from BPMP-FW directy as the frequency table and not do the DRAM frequency scaling which is same as the current behavior.
Now, as the CPU Frequency table is being controlling through OPP table in DT. Keeping fewer entries in the table will create less frequency steps and scale fast to high frequencies if required.
It's not exactly clear what you're doing here. Are you going to scale memory BW based on CPU freq? If yes, then this is wrong because CPU freq is independent from the memory subsystem.
Quoted 6 lines
All Tegra30+ SoCs have ACTMON hardware unit that monitors CPU memory activity and CPU memory BW should be scaled based on CPU memory events counter. We have ACTMON devfreq driver for older SoCs. I have no clue how ACTMON can be accessed on T186+, perhaps there should be a BPMP FW API for that.
Yes, scaling the memory BW based on CPU freq.
Referred below patch set for previous generation of Tegra Soc's which you mentioned and tried to trace the history.
Quoted 8 lines
https://patchwork.ozlabs.org/project/linux-tegra/patch/[email protected]/
In new Tegra Soc's, actmon counter control and usage has been moved to BPMP-FW where only 'MCALL' counter is used and 'MCCPU is not being used. Using the actmon counter was a reactive way to scale the frequency which is less effective due to averaging over a time period.
We are now using the proactive way where clients tell their bandwidth needs to help achieve better performance.
You don't know what bandwidth CPU needs, you trying to guess it.
It should be a bad decision to use cpufreq for memory bandwidth scaling. You'll be wasting memory power 90% of time because cpufreq doesn't have relation to the DRAM, your heuristics will be wrong and won't do anything good compared to using ACTMON. The L2 CPU cache + memory prefetching hides memory from CPU. And cpufreq should be less reactive than ACTMON in general.
Scaling memory freq based on cpufreq is what downstream NV kernel did 10+ years ago for the oldest Tegra generations. Today upstream has all the necessary infrastructure for doing memory bandwidth scaling properly and we even using h/w memory counters on T20. It's strange that you want to bring the downstream archaity to the modern upstream for the latest Tegra generations.
If you can skip the BPMP-FW and use ACTMON directly from kernel, then that's what I suggest to do.
Signature · 2 lines
Best regards, Dmitry
Re: [Patch v1 08/10] cpufreq: tegra194: add OPP support and set bandwidth
Quoted 68 lines · Dmitry Osipenko
Quoted 42 lines · Sumit Gupta
Quoted 28 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 12 lines
Add support to use OPP table from DT in Tegra194 cpufreq driver. Tegra SoC's receive the frequency lookup table (LUT) from BPMP-FW. Cross check the OPP's present in DT against the LUT from BPMP-FW and enable only those DT OPP's which are present in LUT also.
The OPP table in DT has CPU Frequency to bandwidth mapping where the bandwidth value is per MC channel. DRAM bandwidth depends on the number of MC channels which can vary as per the boot configuration. This per channel bandwidth from OPP table will be later converted by MC driver to final bandwidth value by multiplying with number of channels before sending the request to BPMP-FW.
Quoted 7 lines
If OPP table is not present in DT, then use the LUT from BPMP-FW directy as the frequency table and not do the DRAM frequency scaling which is same as the current behavior.
Now, as the CPU Frequency table is being controlling through OPP table in DT. Keeping fewer entries in the table will create less frequency steps and scale fast to high frequencies if required.
It's not exactly clear what you're doing here. Are you going to scale memory BW based on CPU freq? If yes, then this is wrong because CPU freq is independent from the memory subsystem.
Quoted 6 lines
All Tegra30+ SoCs have ACTMON hardware unit that monitors CPU memory activity and CPU memory BW should be scaled based on CPU memory events counter. We have ACTMON devfreq driver for older SoCs. I have no clue how ACTMON can be accessed on T186+, perhaps there should be a BPMP FW API for that.
Yes, scaling the memory BW based on CPU freq.
Referred below patch set for previous generation of Tegra Soc's which you mentioned and tried to trace the history.Quoted 8 lines
https://patchwork.ozlabs.org/project/linux-tegra/patch/[email protected]/
In new Tegra Soc's, actmon counter control and usage has been moved to BPMP-FW where only 'MCALL' counter is used and 'MCCPU is not being used. Using the actmon counter was a reactive way to scale the frequency which is less effective due to averaging over a time period.
We are now using the proactive way where clients tell their bandwidth needs to help achieve better performance.You don't know what bandwidth CPU needs, you trying to guess it.
It should be a bad decision to use cpufreq for memory bandwidth scaling. You'll be wasting memory power 90% of time because cpufreq doesn't have relation to the DRAM, your heuristics will be wrong and won't do anything good compared to using ACTMON. The L2 CPU cache + memory prefetching hides memory from CPU. And cpufreq should be less reactive than ACTMON in general.
Scaling memory freq based on cpufreq is what downstream NV kernel did 10+ years ago for the oldest Tegra generations. Today upstream has all the necessary infrastructure for doing memory bandwidth scaling properly and we even using h/w memory counters on T20. It's strange that you want to bring the downstream archaity to the modern upstream for the latest Tegra generations.
Quoted 2 lines
If you can skip the BPMP-FW and use ACTMON directly from kernel, then that's what I suggest to do.
After talking to a few people, it turns out that BPMP is already using ACTMON internally to do the actual scaling of the EMC frequency (or the CPUs contribution to that). So BPMP will use ACTMON counters to monitor the effective memory load of the CPU and adjust the EMC frequency. The bandwidth request that we generate from the cpufreq driver is more of a guideline for the maximum bandwidth we might consume.
Thierry
Re: [Patch v1 08/10] cpufreq: tegra194: add OPP support and set bandwidth
Quoted 72 lines · Thierry Reding
Quoted 68 lines · Dmitry Osipenko
Quoted 42 lines · Sumit Gupta
Quoted 28 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 12 lines
Add support to use OPP table from DT in Tegra194 cpufreq driver. Tegra SoC's receive the frequency lookup table (LUT) from BPMP-FW. Cross check the OPP's present in DT against the LUT from BPMP-FW and enable only those DT OPP's which are present in LUT also.
The OPP table in DT has CPU Frequency to bandwidth mapping where the bandwidth value is per MC channel. DRAM bandwidth depends on the number of MC channels which can vary as per the boot configuration. This per channel bandwidth from OPP table will be later converted by MC driver to final bandwidth value by multiplying with number of channels before sending the request to BPMP-FW.
Quoted 7 lines
If OPP table is not present in DT, then use the LUT from BPMP-FW directy as the frequency table and not do the DRAM frequency scaling which is same as the current behavior.
Now, as the CPU Frequency table is being controlling through OPP table in DT. Keeping fewer entries in the table will create less frequency steps and scale fast to high frequencies if required.
It's not exactly clear what you're doing here. Are you going to scale memory BW based on CPU freq? If yes, then this is wrong because CPU freq is independent from the memory subsystem.
Quoted 6 lines
All Tegra30+ SoCs have ACTMON hardware unit that monitors CPU memory activity and CPU memory BW should be scaled based on CPU memory events counter. We have ACTMON devfreq driver for older SoCs. I have no clue how ACTMON can be accessed on T186+, perhaps there should be a BPMP FW API for that.
Yes, scaling the memory BW based on CPU freq.
Referred below patch set for previous generation of Tegra Soc's which you mentioned and tried to trace the history.Quoted 8 lines
https://patchwork.ozlabs.org/project/linux-tegra/patch/[email protected]/
In new Tegra Soc's, actmon counter control and usage has been moved to BPMP-FW where only 'MCALL' counter is used and 'MCCPU is not being used. Using the actmon counter was a reactive way to scale the frequency which is less effective due to averaging over a time period.
We are now using the proactive way where clients tell their bandwidth needs to help achieve better performance.You don't know what bandwidth CPU needs, you trying to guess it.
It should be a bad decision to use cpufreq for memory bandwidth scaling. You'll be wasting memory power 90% of time because cpufreq doesn't have relation to the DRAM, your heuristics will be wrong and won't do anything good compared to using ACTMON. The L2 CPU cache + memory prefetching hides memory from CPU. And cpufreq should be less reactive than ACTMON in general.
Scaling memory freq based on cpufreq is what downstream NV kernel did 10+ years ago for the oldest Tegra generations. Today upstream has all the necessary infrastructure for doing memory bandwidth scaling properly and we even using h/w memory counters on T20. It's strange that you want to bring the downstream archaity to the modern upstream for the latest Tegra generations.
Quoted 2 lines
If you can skip the BPMP-FW and use ACTMON directly from kernel, then that's what I suggest to do.
Quoted 6 lines
After talking to a few people, it turns out that BPMP is already using ACTMON internally to do the actual scaling of the EMC frequency (or the CPUs contribution to that). So BPMP will use ACTMON counters to monitor the effective memory load of the CPU and adjust the EMC frequency. The bandwidth request that we generate from the cpufreq driver is more of a guideline for the maximum bandwidth we might consume.
Our kernel ACTMON driver uses cpufreq for guiding the EMC freq. Driving EMC rate solely based on cpufreq would be a not good decision. So does it mean you're now going to extend the ACTMON driver with the BPMP support?
Signature · 2 lines
Best regards, Dmitry
Re: [Patch v1 08/10] cpufreq: tegra194: add OPP support and set bandwidth
Quoted 83 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
Quoted 72 lines · Thierry Reding
Quoted 68 lines · Dmitry Osipenko
Quoted 42 lines · Sumit Gupta
Quoted 28 lines · Dmitry Osipenko
External email: Use caution opening links or attachments
20.12.2022 19:02, Sumit Gupta пишет:
Quoted 12 lines
Add support to use OPP table from DT in Tegra194 cpufreq driver. Tegra SoC's receive the frequency lookup table (LUT) from BPMP-FW. Cross check the OPP's present in DT against the LUT from BPMP-FW and enable only those DT OPP's which are present in LUT also.
The OPP table in DT has CPU Frequency to bandwidth mapping where the bandwidth value is per MC channel. DRAM bandwidth depends on the number of MC channels which can vary as per the boot configuration. This per channel bandwidth from OPP table will be later converted by MC driver to final bandwidth value by multiplying with number of channels before sending the request to BPMP-FW.
Quoted 7 lines
If OPP table is not present in DT, then use the LUT from BPMP-FW directy as the frequency table and not do the DRAM frequency scaling which is same as the current behavior.
Now, as the CPU Frequency table is being controlling through OPP table in DT. Keeping fewer entries in the table will create less frequency steps and scale fast to high frequencies if required.
It's not exactly clear what you're doing here. Are you going to scale memory BW based on CPU freq? If yes, then this is wrong because CPU freq is independent from the memory subsystem.
Quoted 6 lines
All Tegra30+ SoCs have ACTMON hardware unit that monitors CPU memory activity and CPU memory BW should be scaled based on CPU memory events counter. We have ACTMON devfreq driver for older SoCs. I have no clue how ACTMON can be accessed on T186+, perhaps there should be a BPMP FW API for that.
Yes, scaling the memory BW based on CPU freq.
Referred below patch set for previous generation of Tegra Soc's which you mentioned and tried to trace the history.Quoted 8 lines
https://patchwork.ozlabs.org/project/linux-tegra/patch/[email protected]/
In new Tegra Soc's, actmon counter control and usage has been moved to BPMP-FW where only 'MCALL' counter is used and 'MCCPU is not being used. Using the actmon counter was a reactive way to scale the frequency which is less effective due to averaging over a time period.
We are now using the proactive way where clients tell their bandwidth needs to help achieve better performance.You don't know what bandwidth CPU needs, you trying to guess it.
It should be a bad decision to use cpufreq for memory bandwidth scaling. You'll be wasting memory power 90% of time because cpufreq doesn't have relation to the DRAM, your heuristics will be wrong and won't do anything good compared to using ACTMON. The L2 CPU cache + memory prefetching hides memory from CPU. And cpufreq should be less reactive than ACTMON in general.
Scaling memory freq based on cpufreq is what downstream NV kernel did 10+ years ago for the oldest Tegra generations. Today upstream has all the necessary infrastructure for doing memory bandwidth scaling properly and we even using h/w memory counters on T20. It's strange that you want to bring the downstream archaity to the modern upstream for the latest Tegra generations.
Quoted 2 lines
If you can skip the BPMP-FW and use ACTMON directly from kernel, then that's what I suggest to do.
Quoted 6 lines
After talking to a few people, it turns out that BPMP is already using ACTMON internally to do the actual scaling of the EMC frequency (or the CPUs contribution to that). So BPMP will use ACTMON counters to monitor the effective memory load of the CPU and adjust the EMC frequency. The bandwidth request that we generate from the cpufreq driver is more of a guideline for the maximum bandwidth we might consume.
Quoted 8 lines
Our kernel ACTMON driver uses cpufreq for guiding the EMC freq. Driving EMC rate solely based on cpufreq would be a not good decision. So does it mean you're now going to extend the ACTMON driver with the BPMP support?
--
Best regards,
Dmitry
As we are using the ACTMON in BPMP-FW now and there is no plan to move it back to kernel for future SoC's. The ACTMON kernel driver won't be used in T234 and later SoC's. The current patches to scale DRAM_FREQ with CPU_FREQ is used in T234.
In future SoC's, we are planning to use a new solution for this in
BPMP-FW which will provide better metric in addition to ACTMON's for scaling DRAM FREQ for CPU Clusters.
Thanks,
Sumit
PATCH 9/10 memory: tegra: get number of enabled mc channels
In reply to Sumit GuptaGet number of MC channels which are actually enabled in current boot configuration.
--- drivers/memory/tegra/mc.c | 19 +++++++++++++++++++ drivers/memory/tegra/mc.h | 1 + include/soc/tegra/mc.h | 1 + 3 files changed, 21 insertions(+)
drivers/memory/tegra/mc.c +19 -0
@@ -881,6 +881,23 @@ static int tegra_mc_interconnect_setup(struct tegra_mc *mc)return err; } +static void tegra_mc_num_channel_enabled (struct tegra_mc *mc) +{ +unsigned int i; +u32 value =0 ; + + value =mc_ch_readl (mc,0 ,MC_EMEM_ADR_CFG_CHANNEL_ENABLE ); +if (value <=0 ) { + mc->num_channels = mc->soc->num_channels; +return ; + } + +for (i =0 ; i <32 ; i++) { +if (value &BIT (i)) + mc->num_channels++; + } +} +static int tegra_mc_probe (struct platform_device *pdev) {struct tegra_mc *mc;
@@ -919,6 +936,8 @@ static int tegra_mc_probe(struct platform_device *pdev)return err; } +tegra_mc_num_channel_enabled (mc); +if (mc->soc->ops && mc->soc->ops->handle_irq) { mc->irq =platform_get_irq (pdev,0 );if (mc->irq <0 )
drivers/memory/tegra/mc.h +1 -0
@@ -53,6 +53,7 @@#define MC_ERR_ROUTE_SANITY_ADR 0x9c4#define MC_ERR_GENERALIZED_CARVEOUT_STATUS 0xc00#define MC_ERR_GENERALIZED_CARVEOUT_ADR 0xc04 +#define MC_EMEM_ADR_CFG_CHANNEL_ENABLE 0xdf8#define MC_GLOBAL_INTSTATUS 0xf24#define MC_ERR_ADR_HI 0x11fc
include/soc/tegra/mc.h +1 -0
@@ -236,6 +236,7 @@ struct tegra_mc {struct tegra_mc_timing *timings;unsigned int num_timings; +unsigned int num_channels;struct reset_controller_dev reset;
Signature · 1 line
2.17.1
Re: [Patch v1 09/10] memory: tegra: get number of enabled mc channels
Quoted 2 lines · Sumit Gupta
Get number of MC channels which are actually enabled in current boot configuration.
Why? You don't do anything with it. Commit msg should give the reason of changes.
Best regards,
Krzysztof
Re: [Patch v1 09/10] memory: tegra: get number of enabled mc channels
Quoted 7 lines · Krzysztof Kozlowski
External email: Use caution opening links or attachments
Quoted 2 lines · Sumit Gupta
Get number of MC channels which are actually enabled in current boot configuration.
Quoted 7 lines
Why? You don't do anything with it. Commit msg should give the reason of changes.
Best regards,
Krzysztof
CPU OPP tables have per channel bandwidth info. The "mc->num_channels" is used in [1] (Patch v1 10/10) to make the per MC channel bandwidth requested by the CPU cluster as a multiple of number of the enabled mc channels.
Will update the commit description with this info.
[1] https://lore.kernel.org/lkml/[email protected]/T/#m3ac150a86977e89b97c5d19c60384f29d7a01d21
Re: [Patch v1 09/10] memory: tegra: get number of enabled mc channels
Quoted 18 lines · Sumit Gupta
Quoted 7 lines · Krzysztof Kozlowski
External email: Use caution opening links or attachments
Quoted 2 lines · Sumit Gupta
Get number of MC channels which are actually enabled in current boot configuration.
Quoted 7 lines
Why? You don't do anything with it. Commit msg should give the reason of changes.
Best regards,
Krzysztof
Quoted 7 lines
CPU OPP tables have per channel bandwidth info. The "
mc->num_channels" is used in [1] (Patch v1 10/10) to make the per MC channel bandwidth requested by the CPU cluster as a multiple of number of the enabled mc channels.Will update the commit description with this info.
[1] https://lore.kernel.org/lkml/[email protected]/T/#m3ac150a86977e89b97c5d19c60384f29d7a01d21
Both patch 9 and 10 are reasonably small, so it would be okay to merge the two patches and avoid any need for an extra explanation.
Thierry
PATCH 10/10 memory: tegra: make cluster bw request a multiple of mc_channels
In reply to Sumit GuptaCPU opp table have bandwidth data per MC channel. The actual bandwidth depends on number of MC channels which can change as per the boot config for a board. So, multiply the bandwidth request for CPU clusters with number of enabled MC channels. This is not required to be done for other MC clients.
--- drivers/memory/tegra/tegra234.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)
drivers/memory/tegra/tegra234.c +24 -1
@@ -834,6 +834,29 @@ static int tegra234_mc_icc_set(struct icc_node *src, struct icc_node *dst)return 0 ; } +static int tegra234_mc_icc_aggregate (struct icc_node *node,u32 tag,u32 avg_bw, +u32 peak_bw,u32 *agg_avg,u32 *agg_peak) +{ +struct tegra_icc_node *tnode =NULL ; +struct tegra_mc *mc =NULL ; + +if (node->id ==TEGRA_ICC_MC_CPU_CLUSTER0 || + node->id ==TEGRA_ICC_MC_CPU_CLUSTER1 || + node->id ==TEGRA_ICC_MC_CPU_CLUSTER2 ) { +if (node->data) { + tnode = node->data; + mc = tnode->mc; +if (mc) + peak_bw = peak_bw * mc->num_channels; + } + } + + *agg_avg += avg_bw; + *agg_peak =max (*agg_peak, peak_bw); + +return 0 ; +} +static struct icc_node *tegra234_mc_of_icc_xlate (struct of_phandle_args *spec,void *data) {
@@ -866,7 +889,7 @@ static int tegra234_mc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peastatic const struct tegra_mc_icc_ops tegra234_mc_icc_ops = { .xlate = tegra234_mc_of_icc_xlate, .get_bw = tegra234_mc_icc_get_init_bw, - .aggregate = icc_std_aggregate, + .aggregate = tegra234_mc_icc_aggregate, .set = tegra234_mc_icc_set, };
Signature · 1 line
2.17.1