[PATCH v4 0/5] memory: tegra: Support EMC dfs on Tegra186/Tegra194

[PATCH v4 0/5] memory: tegra: Support EMC dfs on Tegra186/Tegra194

[Aaron Kling via B4 Relay]

This series borrows the concept used on Tegra234 to scale EMC based on
CPU frequency and applies it to Tegra186 and Tegra194. Except that the
bpmp on those archs does not support bandwidth manager, so the scaling
iteself is handled similar to how Tegra124 currently works.

Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
Changes in v4:
- Rebase on next-20251027
- Track naming convention changes on patch 3
- Link to v3: https://lore.kernel.org/r/20251021-tegra186-icc-p2-v3-0-1a50b526dd40@gmail.com

Changes in v3:
- Split series
- Link to v2: https://lore.kernel.org/r/20250909-tegra186-icc-v2-0-09413724e781@gmail.com

Changes in v2:
- Use opp scoped free in patch 3
- Cleanup as requested in patch 3
- Move patch 3 to the start of the series to keep subsystems grouped
- Link to v1: https://lore.kernel.org/r/20250831-tegra186-icc-v1-0-607ddc53b507@gmail.com

---
Aaron Kling (5):
      dt-bindings: memory: tegra186-mc: Add dummy client IDs for Tegra186
      dt-bindings: memory: tegra194-mc: Add dummy client IDs for Tegra194
      memory: tegra186-emc: Support non-bpmp icc scaling
      memory: tegra186: Support icc scaling
      memory: tegra194: Support icc scaling

 drivers/memory/tegra/tegra186-emc.c      | 132 ++++++++++++++++++++++++++++++-
 drivers/memory/tegra/tegra186.c          |  48 +++++++++++
 drivers/memory/tegra/tegra194.c          |  59 +++++++++++++-
 include/dt-bindings/memory/tegra186-mc.h |   4 +
 include/dt-bindings/memory/tegra194-mc.h |   6 ++
 5 files changed, 246 insertions(+), 3 deletions(-)
---
base-commit: 8fec172c82c2b5f6f8e47ab837c1dc91ee3d1b87
change-id: 20251021-tegra186-icc-p2-864fd8625699

Best regards,
-- 
Aaron Kling <webgeek1234@gmail.com>

[PATCH v4 1/5] dt-bindings: memory: tegra186-mc: Add dummy client IDs for Tegra186

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

Add ICC IDs for dummy software clients representing CCPLEX clusters.

Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
 include/dt-bindings/memory/tegra186-mc.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/dt-bindings/memory/tegra186-mc.h b/include/dt-bindings/memory/tegra186-mc.h
index 82a1e27f73576212bc227c74adff28c5f33c6bb1..8abbc26f3123aad2dffaec6be21f99f8de1ccf89 100644
--- a/include/dt-bindings/memory/tegra186-mc.h
+++ b/include/dt-bindings/memory/tegra186-mc.h
@@ -247,4 +247,8 @@
 #define TEGRA186_MEMORY_CLIENT_VICSRD1 0xa2
 #define TEGRA186_MEMORY_CLIENT_NVDECSRD1 0xa3
 
+/* 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
+
 #endif

-- 
2.51.0

[PATCH v4 2/5] dt-bindings: memory: tegra194-mc: Add dummy client IDs for Tegra194

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

Add ICC IDs for dummy software clients representing CCPLEX clusters.

Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
 include/dt-bindings/memory/tegra194-mc.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/include/dt-bindings/memory/tegra194-mc.h b/include/dt-bindings/memory/tegra194-mc.h
index eed48b746bc94072a6bd0af7f344dbb6f6618859..a7d97a1a470cd3cfb18c7ef45c421426ea3c7abf 100644
--- a/include/dt-bindings/memory/tegra194-mc.h
+++ b/include/dt-bindings/memory/tegra194-mc.h
@@ -407,4 +407,10 @@
 /* MSS internal memqual MIU6 write clients */
 #define TEGRA194_MEMORY_CLIENT_MIU6W 0xff
 
+/* 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
+#define TEGRA_ICC_MC_CPU_CLUSTER3       1006
+
 #endif

-- 
2.51.0

[PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

This adds support for dynamic frequency scaling of external memory on
devices with bpmp firmware that does not support bwmgr.

Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
 drivers/memory/tegra/tegra186-emc.c | 132 +++++++++++++++++++++++++++++++++++-
 1 file changed, 130 insertions(+), 2 deletions(-)

diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
index 9959ad5804b444b269456d1fbae87b4bc111661b..74be09968baa7a0fbdce4359f470ce56b18acb10 100644
--- a/drivers/memory/tegra/tegra186-emc.c
+++ b/drivers/memory/tegra/tegra186-emc.c
@@ -18,6 +18,17 @@ struct tegra186_emc_dvfs {
 	unsigned long rate;
 };
 
+enum emc_rate_request_type {
+	EMC_RATE_DEBUG,
+	EMC_RATE_ICC,
+	EMC_RATE_TYPE_MAX,
+};
+
+struct emc_rate_request {
+	unsigned long min_rate;
+	unsigned long max_rate;
+};
+
 struct tegra186_emc {
 	struct tegra_bpmp *bpmp;
 	struct device *dev;
@@ -33,8 +44,90 @@ struct tegra186_emc {
 	} debugfs;
 
 	struct icc_provider provider;
+
+	/*
+	 * There are multiple sources in the EMC driver which could request
+	 * a min/max clock rate, these rates are contained in this array.
+	 */
+	struct emc_rate_request requested_rate[EMC_RATE_TYPE_MAX];
+
+	/* protect shared rate-change code path */
+	struct mutex rate_lock;
 };
 
+static void tegra186_emc_rate_requests_init(struct tegra186_emc *emc)
+{
+	unsigned int i;
+
+	for (i = 0; i < EMC_RATE_TYPE_MAX; i++) {
+		emc->requested_rate[i].min_rate = 0;
+		emc->requested_rate[i].max_rate = ULONG_MAX;
+	}
+}
+
+static int emc_request_rate(struct tegra186_emc *emc,
+			    unsigned long new_min_rate,
+			    unsigned long new_max_rate,
+			    enum emc_rate_request_type type)
+{
+	struct emc_rate_request *req = emc->requested_rate;
+	unsigned long min_rate = 0, max_rate = ULONG_MAX;
+	unsigned int i;
+	int err;
+
+	/* select minimum and maximum rates among the requested rates */
+	for (i = 0; i < EMC_RATE_TYPE_MAX; i++, req++) {
+		if (i == type) {
+			min_rate = max(new_min_rate, min_rate);
+			max_rate = min(new_max_rate, max_rate);
+		} else {
+			min_rate = max(req->min_rate, min_rate);
+			max_rate = min(req->max_rate, max_rate);
+		}
+	}
+
+	if (min_rate > max_rate) {
+		dev_err_ratelimited(emc->dev, "%s: type %u: out of range: %lu %lu\n",
+				    __func__, type, min_rate, max_rate);
+		return -ERANGE;
+	}
+
+	err = clk_set_rate(emc->clk, min_rate);
+	if (err)
+		return err;
+
+	emc->requested_rate[type].min_rate = new_min_rate;
+	emc->requested_rate[type].max_rate = new_max_rate;
+
+	return 0;
+}
+
+static int emc_set_min_rate(struct tegra186_emc *emc, unsigned long rate,
+			    enum emc_rate_request_type type)
+{
+	struct emc_rate_request *req = &emc->requested_rate[type];
+	int ret;
+
+	mutex_lock(&emc->rate_lock);
+	ret = emc_request_rate(emc, rate, req->max_rate, type);
+	mutex_unlock(&emc->rate_lock);
+
+	return ret;
+}
+
+static int emc_set_max_rate(struct tegra186_emc *emc, unsigned long rate,
+			    enum emc_rate_request_type type)
+{
+	struct emc_rate_request *req = &emc->requested_rate[type];
+	int ret;
+
+	mutex_lock(&emc->rate_lock);
+	ret = emc_request_rate(emc, req->min_rate, rate, type);
+	mutex_unlock(&emc->rate_lock);
+
+	return ret;
+}
+
 /*
  * debugfs interface
  *
@@ -107,7 +200,7 @@ static int tegra186_emc_debug_min_rate_set(void *data, u64 rate)
 	if (!tegra186_emc_validate_rate(emc, rate))
 		return -EINVAL;
 
-	err = clk_set_min_rate(emc->clk, rate);
+	err = emc_set_min_rate(emc, rate, EMC_RATE_DEBUG);
 	if (err < 0)
 		return err;
 
@@ -137,7 +230,7 @@ static int tegra186_emc_debug_max_rate_set(void *data, u64 rate)
 	if (!tegra186_emc_validate_rate(emc, rate))
 		return -EINVAL;
 
-	err = clk_set_max_rate(emc->clk, rate);
+	err = emc_set_max_rate(emc, rate, EMC_RATE_DEBUG);
 	if (err < 0)
 		return err;
 
@@ -217,6 +310,12 @@ static int tegra186_emc_get_emc_dvfs_latency(struct tegra186_emc *emc)
 	return 0;
 }
 
+static inline struct tegra186_emc *
+to_tegra186_emc_provider(struct icc_provider *provider)
+{
+	return container_of(provider, struct tegra186_emc, provider);
+}
+
 /*
  * tegra186_emc_icc_set_bw() - Set BW api for EMC provider
  * @src: ICC node for External Memory Controller (EMC)
@@ -227,6 +326,33 @@ static int tegra186_emc_get_emc_dvfs_latency(struct tegra186_emc *emc)
  */
 static int tegra186_emc_icc_set_bw(struct icc_node *src, struct icc_node *dst)
 {
+	struct tegra186_emc *emc = to_tegra186_emc_provider(dst->provider);
+	struct tegra_mc *mc = dev_get_drvdata(emc->dev->parent);
+	unsigned long long peak_bw = icc_units_to_bps(dst->peak_bw);
+	unsigned long long avg_bw = icc_units_to_bps(dst->avg_bw);
+	unsigned long long rate = max(avg_bw, peak_bw);
+	const unsigned int ddr = 2;
+	int err;
+
+	/*
+	 * Do nothing here if bwmgr is supported in BPMP-FW. BPMP-FW sets the final
+	 * Freq based on the passed values.
+	 */
+	if (mc->bwmgr_mrq_supported)
+		return 0;
+
+	/*
+	 * Tegra186 EMC runs on a clock rate of SDRAM bus. This means that
+	 * EMC clock rate is twice smaller than the peak data rate because
+	 * data is sampled on both EMC clock edges.
+	 */
+	do_div(rate, ddr);
+	rate = min_t(u64, rate, U32_MAX);
+
+	err = emc_set_min_rate(emc, rate, EMC_RATE_ICC);
+	if (err)
+		return err;
+
 	return 0;
 }
 
@@ -329,6 +455,8 @@ static int tegra186_emc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, emc);
 	emc->dev = &pdev->dev;
 
+	tegra186_emc_rate_requests_init(emc);
+
 	if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_EMC_DVFS_LATENCY)) {
 		err = tegra186_emc_get_emc_dvfs_latency(emc);
 		if (err)

-- 
2.51.0

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 27/10/2025 18:55, Aaron Kling via B4 Relay wrote:
> From: Aaron Kling <webgeek1234@gmail.com>
> 
> This adds support for dynamic frequency scaling of external memory on
> devices with bpmp firmware that does not support bwmgr.
> 
> Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
> ---
>   drivers/memory/tegra/tegra186-emc.c | 132 +++++++++++++++++++++++++++++++++++-
>   1 file changed, 130 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
> index 9959ad5804b444b269456d1fbae87b4bc111661b..74be09968baa7a0fbdce4359f470ce56b18acb10 100644
> --- a/drivers/memory/tegra/tegra186-emc.c
> +++ b/drivers/memory/tegra/tegra186-emc.c
> @@ -18,6 +18,17 @@ struct tegra186_emc_dvfs {
>   	unsigned long rate;
>   };
>   
> +enum emc_rate_request_type {
> +	EMC_RATE_DEBUG,
> +	EMC_RATE_ICC,
> +	EMC_RATE_TYPE_MAX,
> +};
> +
> +struct emc_rate_request {
> +	unsigned long min_rate;
> +	unsigned long max_rate;
> +};
> +
>   struct tegra186_emc {
>   	struct tegra_bpmp *bpmp;
>   	struct device *dev;
> @@ -33,8 +44,90 @@ struct tegra186_emc {
>   	} debugfs;
>   
>   	struct icc_provider provider;
> +
> +	/*
> +	 * There are multiple sources in the EMC driver which could request
> +	 * a min/max clock rate, these rates are contained in this array.
> +	 */
> +	struct emc_rate_request requested_rate[EMC_RATE_TYPE_MAX];
> +
> +	/* protect shared rate-change code path */
> +	struct mutex rate_lock;
>   };
>   
> +static void tegra186_emc_rate_requests_init(struct tegra186_emc *emc)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < EMC_RATE_TYPE_MAX; i++) {
> +		emc->requested_rate[i].min_rate = 0;
> +		emc->requested_rate[i].max_rate = ULONG_MAX;
> +	}
> +}
> +
> +static int emc_request_rate(struct tegra186_emc *emc,
> +			    unsigned long new_min_rate,
> +			    unsigned long new_max_rate,
> +			    enum emc_rate_request_type type)
> +{
> +	struct emc_rate_request *req = emc->requested_rate;
> +	unsigned long min_rate = 0, max_rate = ULONG_MAX;
> +	unsigned int i;
> +	int err;
> +
> +	/* select minimum and maximum rates among the requested rates */
> +	for (i = 0; i < EMC_RATE_TYPE_MAX; i++, req++) {
> +		if (i == type) {
> +			min_rate = max(new_min_rate, min_rate);
> +			max_rate = min(new_max_rate, max_rate);
> +		} else {
> +			min_rate = max(req->min_rate, min_rate);
> +			max_rate = min(req->max_rate, max_rate);
> +		}
> +	}
> +
> +	if (min_rate > max_rate) {
> +		dev_err_ratelimited(emc->dev, "%s: type %u: out of range: %lu %lu\n",
> +				    __func__, type, min_rate, max_rate);
> +		return -ERANGE;
> +	}
> +
> +	err = clk_set_rate(emc->clk, min_rate);
> +	if (err)
> +		return err;
> +
> +	emc->requested_rate[type].min_rate = new_min_rate;
> +	emc->requested_rate[type].max_rate = new_max_rate;
> +
> +	return 0;
> +}
> +
> +static int emc_set_min_rate(struct tegra186_emc *emc, unsigned long rate,
> +			    enum emc_rate_request_type type)
> +{
> +	struct emc_rate_request *req = &emc->requested_rate[type];
> +	int ret;
> +
> +	mutex_lock(&emc->rate_lock);
> +	ret = emc_request_rate(emc, rate, req->max_rate, type);
> +	mutex_unlock(&emc->rate_lock);
> +
> +	return ret;
> +}
> +
> +static int emc_set_max_rate(struct tegra186_emc *emc, unsigned long rate,
> +			    enum emc_rate_request_type type)
> +{
> +	struct emc_rate_request *req = &emc->requested_rate[type];
> +	int ret;
> +
> +	mutex_lock(&emc->rate_lock);
> +	ret = emc_request_rate(emc, req->min_rate, rate, type);
> +	mutex_unlock(&emc->rate_lock);
> +
> +	return ret;
> +}
> +
>   /*
>    * debugfs interface
>    *
> @@ -107,7 +200,7 @@ static int tegra186_emc_debug_min_rate_set(void *data, u64 rate)
>   	if (!tegra186_emc_validate_rate(emc, rate))
>   		return -EINVAL;
>   
> -	err = clk_set_min_rate(emc->clk, rate);
> +	err = emc_set_min_rate(emc, rate, EMC_RATE_DEBUG);
>   	if (err < 0)
>   		return err;
>   
> @@ -137,7 +230,7 @@ static int tegra186_emc_debug_max_rate_set(void *data, u64 rate)
>   	if (!tegra186_emc_validate_rate(emc, rate))
>   		return -EINVAL;
>   
> -	err = clk_set_max_rate(emc->clk, rate);
> +	err = emc_set_max_rate(emc, rate, EMC_RATE_DEBUG);
>   	if (err < 0)
>   		return err;
>   
> @@ -217,6 +310,12 @@ static int tegra186_emc_get_emc_dvfs_latency(struct tegra186_emc *emc)
>   	return 0;
>   }
>   
> +static inline struct tegra186_emc *
> +to_tegra186_emc_provider(struct icc_provider *provider)
> +{
> +	return container_of(provider, struct tegra186_emc, provider);
> +}
> +
>   /*
>    * tegra186_emc_icc_set_bw() - Set BW api for EMC provider
>    * @src: ICC node for External Memory Controller (EMC)
> @@ -227,6 +326,33 @@ static int tegra186_emc_get_emc_dvfs_latency(struct tegra186_emc *emc)
>    */
>   static int tegra186_emc_icc_set_bw(struct icc_node *src, struct icc_node *dst)
>   {
> +	struct tegra186_emc *emc = to_tegra186_emc_provider(dst->provider);
> +	struct tegra_mc *mc = dev_get_drvdata(emc->dev->parent);
> +	unsigned long long peak_bw = icc_units_to_bps(dst->peak_bw);
> +	unsigned long long avg_bw = icc_units_to_bps(dst->avg_bw);
> +	unsigned long long rate = max(avg_bw, peak_bw);
> +	const unsigned int ddr = 2;
> +	int err;
> +
> +	/*
> +	 * Do nothing here if bwmgr is supported in BPMP-FW. BPMP-FW sets the final
> +	 * Freq based on the passed values.
> +	 */
> +	if (mc->bwmgr_mrq_supported)
> +		return 0;
> +
> +	/*
> +	 * Tegra186 EMC runs on a clock rate of SDRAM bus. This means that
> +	 * EMC clock rate is twice smaller than the peak data rate because
> +	 * data is sampled on both EMC clock edges.
> +	 */
> +	do_div(rate, ddr);
> +	rate = min_t(u64, rate, U32_MAX);
> +
> +	err = emc_set_min_rate(emc, rate, EMC_RATE_ICC);
> +	if (err)
> +		return err;
> +
>   	return 0;
>   }
>   
> @@ -329,6 +455,8 @@ static int tegra186_emc_probe(struct platform_device *pdev)
>   	platform_set_drvdata(pdev, emc);
>   	emc->dev = &pdev->dev;
>   
> +	tegra186_emc_rate_requests_init(emc);
> +
>   	if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_EMC_DVFS_LATENCY)) {
>   		err = tegra186_emc_get_emc_dvfs_latency(emc);
>   		if (err)
> 


FYI, this patch is causing a boot regression on Tegra194 devices. I 
noticed that tegra194-p2972-0000 and tegra194-p3509-0000+p3668-0000 are 
no longer booting and bisect is pointing to this. I will have a closer 
look and try to see why this is.

Jon

-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Aaron Kling]

On Mon, Nov 10, 2025 at 3:25 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 27/10/2025 18:55, Aaron Kling via B4 Relay wrote:
> > From: Aaron Kling <webgeek1234@gmail.com>
> >
> > This adds support for dynamic frequency scaling of external memory on
> > devices with bpmp firmware that does not support bwmgr.
> >
> > Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
> > ---
> >   drivers/memory/tegra/tegra186-emc.c | 132 +++++++++++++++++++++++++++++++++++-
> >   1 file changed, 130 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
> > index 9959ad5804b444b269456d1fbae87b4bc111661b..74be09968baa7a0fbdce4359f470ce56b18acb10 100644
> > --- a/drivers/memory/tegra/tegra186-emc.c
> > +++ b/drivers/memory/tegra/tegra186-emc.c
> > @@ -18,6 +18,17 @@ struct tegra186_emc_dvfs {
> >       unsigned long rate;
> >   };
> >
> > +enum emc_rate_request_type {
> > +     EMC_RATE_DEBUG,
> > +     EMC_RATE_ICC,
> > +     EMC_RATE_TYPE_MAX,
> > +};
> > +
> > +struct emc_rate_request {
> > +     unsigned long min_rate;
> > +     unsigned long max_rate;
> > +};
> > +
> >   struct tegra186_emc {
> >       struct tegra_bpmp *bpmp;
> >       struct device *dev;
> > @@ -33,8 +44,90 @@ struct tegra186_emc {
> >       } debugfs;
> >
> >       struct icc_provider provider;
> > +
> > +     /*
> > +      * There are multiple sources in the EMC driver which could request
> > +      * a min/max clock rate, these rates are contained in this array.
> > +      */
> > +     struct emc_rate_request requested_rate[EMC_RATE_TYPE_MAX];
> > +
> > +     /* protect shared rate-change code path */
> > +     struct mutex rate_lock;
> >   };
> >
> > +static void tegra186_emc_rate_requests_init(struct tegra186_emc *emc)
> > +{
> > +     unsigned int i;
> > +
> > +     for (i = 0; i < EMC_RATE_TYPE_MAX; i++) {
> > +             emc->requested_rate[i].min_rate = 0;
> > +             emc->requested_rate[i].max_rate = ULONG_MAX;
> > +     }
> > +}
> > +
> > +static int emc_request_rate(struct tegra186_emc *emc,
> > +                         unsigned long new_min_rate,
> > +                         unsigned long new_max_rate,
> > +                         enum emc_rate_request_type type)
> > +{
> > +     struct emc_rate_request *req = emc->requested_rate;
> > +     unsigned long min_rate = 0, max_rate = ULONG_MAX;
> > +     unsigned int i;
> > +     int err;
> > +
> > +     /* select minimum and maximum rates among the requested rates */
> > +     for (i = 0; i < EMC_RATE_TYPE_MAX; i++, req++) {
> > +             if (i == type) {
> > +                     min_rate = max(new_min_rate, min_rate);
> > +                     max_rate = min(new_max_rate, max_rate);
> > +             } else {
> > +                     min_rate = max(req->min_rate, min_rate);
> > +                     max_rate = min(req->max_rate, max_rate);
> > +             }
> > +     }
> > +
> > +     if (min_rate > max_rate) {
> > +             dev_err_ratelimited(emc->dev, "%s: type %u: out of range: %lu %lu\n",
> > +                                 __func__, type, min_rate, max_rate);
> > +             return -ERANGE;
> > +     }
> > +
> > +     err = clk_set_rate(emc->clk, min_rate);
> > +     if (err)
> > +             return err;
> > +
> > +     emc->requested_rate[type].min_rate = new_min_rate;
> > +     emc->requested_rate[type].max_rate = new_max_rate;
> > +
> > +     return 0;
> > +}
> > +
> > +static int emc_set_min_rate(struct tegra186_emc *emc, unsigned long rate,
> > +                         enum emc_rate_request_type type)
> > +{
> > +     struct emc_rate_request *req = &emc->requested_rate[type];
> > +     int ret;
> > +
> > +     mutex_lock(&emc->rate_lock);
> > +     ret = emc_request_rate(emc, rate, req->max_rate, type);
> > +     mutex_unlock(&emc->rate_lock);
> > +
> > +     return ret;
> > +}
> > +
> > +static int emc_set_max_rate(struct tegra186_emc *emc, unsigned long rate,
> > +                         enum emc_rate_request_type type)
> > +{
> > +     struct emc_rate_request *req = &emc->requested_rate[type];
> > +     int ret;
> > +
> > +     mutex_lock(&emc->rate_lock);
> > +     ret = emc_request_rate(emc, req->min_rate, rate, type);
> > +     mutex_unlock(&emc->rate_lock);
> > +
> > +     return ret;
> > +}
> > +
> >   /*
> >    * debugfs interface
> >    *
> > @@ -107,7 +200,7 @@ static int tegra186_emc_debug_min_rate_set(void *data, u64 rate)
> >       if (!tegra186_emc_validate_rate(emc, rate))
> >               return -EINVAL;
> >
> > -     err = clk_set_min_rate(emc->clk, rate);
> > +     err = emc_set_min_rate(emc, rate, EMC_RATE_DEBUG);
> >       if (err < 0)
> >               return err;
> >
> > @@ -137,7 +230,7 @@ static int tegra186_emc_debug_max_rate_set(void *data, u64 rate)
> >       if (!tegra186_emc_validate_rate(emc, rate))
> >               return -EINVAL;
> >
> > -     err = clk_set_max_rate(emc->clk, rate);
> > +     err = emc_set_max_rate(emc, rate, EMC_RATE_DEBUG);
> >       if (err < 0)
> >               return err;
> >
> > @@ -217,6 +310,12 @@ static int tegra186_emc_get_emc_dvfs_latency(struct tegra186_emc *emc)
> >       return 0;
> >   }
> >
> > +static inline struct tegra186_emc *
> > +to_tegra186_emc_provider(struct icc_provider *provider)
> > +{
> > +     return container_of(provider, struct tegra186_emc, provider);
> > +}
> > +
> >   /*
> >    * tegra186_emc_icc_set_bw() - Set BW api for EMC provider
> >    * @src: ICC node for External Memory Controller (EMC)
> > @@ -227,6 +326,33 @@ static int tegra186_emc_get_emc_dvfs_latency(struct tegra186_emc *emc)
> >    */
> >   static int tegra186_emc_icc_set_bw(struct icc_node *src, struct icc_node *dst)
> >   {
> > +     struct tegra186_emc *emc = to_tegra186_emc_provider(dst->provider);
> > +     struct tegra_mc *mc = dev_get_drvdata(emc->dev->parent);
> > +     unsigned long long peak_bw = icc_units_to_bps(dst->peak_bw);
> > +     unsigned long long avg_bw = icc_units_to_bps(dst->avg_bw);
> > +     unsigned long long rate = max(avg_bw, peak_bw);
> > +     const unsigned int ddr = 2;
> > +     int err;
> > +
> > +     /*
> > +      * Do nothing here if bwmgr is supported in BPMP-FW. BPMP-FW sets the final
> > +      * Freq based on the passed values.
> > +      */
> > +     if (mc->bwmgr_mrq_supported)
> > +             return 0;
> > +
> > +     /*
> > +      * Tegra186 EMC runs on a clock rate of SDRAM bus. This means that
> > +      * EMC clock rate is twice smaller than the peak data rate because
> > +      * data is sampled on both EMC clock edges.
> > +      */
> > +     do_div(rate, ddr);
> > +     rate = min_t(u64, rate, U32_MAX);
> > +
> > +     err = emc_set_min_rate(emc, rate, EMC_RATE_ICC);
> > +     if (err)
> > +             return err;
> > +
> >       return 0;
> >   }
> >
> > @@ -329,6 +455,8 @@ static int tegra186_emc_probe(struct platform_device *pdev)
> >       platform_set_drvdata(pdev, emc);
> >       emc->dev = &pdev->dev;
> >
> > +     tegra186_emc_rate_requests_init(emc);
> > +
> >       if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_EMC_DVFS_LATENCY)) {
> >               err = tegra186_emc_get_emc_dvfs_latency(emc);
> >               if (err)
> >
>
>
> FYI, this patch is causing a boot regression on Tegra194 devices. I
> noticed that tegra194-p2972-0000 and tegra194-p3509-0000+p3668-0000 are
> no longer booting and bisect is pointing to this. I will have a closer
> look and try to see why this is.

Interesting. Both were booting for me during my verification, though
my use case involves the dt changes that I don't believe have been
picked up yet. Thought I had explicitly verified without the dt
changes too, though. Since I was asked to do so on this or one of the
other similar series. I will try to check linux-next as-is soon.

Aaron

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Aaron Kling]

On Mon, Nov 10, 2025 at 3:55 PM Aaron Kling <webgeek1234@gmail.com> wrote:
>
> On Mon, Nov 10, 2025 at 3:25 PM Jon Hunter <jonathanh@nvidia.com> wrote:
> >
> >
> > On 27/10/2025 18:55, Aaron Kling via B4 Relay wrote:
> > > From: Aaron Kling <webgeek1234@gmail.com>
> > >
> > > This adds support for dynamic frequency scaling of external memory on
> > > devices with bpmp firmware that does not support bwmgr.
> > >
> > > Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
> > > ---
> > >   drivers/memory/tegra/tegra186-emc.c | 132 +++++++++++++++++++++++++++++++++++-
> > >   1 file changed, 130 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
> > > index 9959ad5804b444b269456d1fbae87b4bc111661b..74be09968baa7a0fbdce4359f470ce56b18acb10 100644
> > > --- a/drivers/memory/tegra/tegra186-emc.c
> > > +++ b/drivers/memory/tegra/tegra186-emc.c
> > > @@ -18,6 +18,17 @@ struct tegra186_emc_dvfs {
> > >       unsigned long rate;
> > >   };
> > >
> > > +enum emc_rate_request_type {
> > > +     EMC_RATE_DEBUG,
> > > +     EMC_RATE_ICC,
> > > +     EMC_RATE_TYPE_MAX,
> > > +};
> > > +
> > > +struct emc_rate_request {
> > > +     unsigned long min_rate;
> > > +     unsigned long max_rate;
> > > +};
> > > +
> > >   struct tegra186_emc {
> > >       struct tegra_bpmp *bpmp;
> > >       struct device *dev;
> > > @@ -33,8 +44,90 @@ struct tegra186_emc {
> > >       } debugfs;
> > >
> > >       struct icc_provider provider;
> > > +
> > > +     /*
> > > +      * There are multiple sources in the EMC driver which could request
> > > +      * a min/max clock rate, these rates are contained in this array.
> > > +      */
> > > +     struct emc_rate_request requested_rate[EMC_RATE_TYPE_MAX];
> > > +
> > > +     /* protect shared rate-change code path */
> > > +     struct mutex rate_lock;
> > >   };
> > >
> > > +static void tegra186_emc_rate_requests_init(struct tegra186_emc *emc)
> > > +{
> > > +     unsigned int i;
> > > +
> > > +     for (i = 0; i < EMC_RATE_TYPE_MAX; i++) {
> > > +             emc->requested_rate[i].min_rate = 0;
> > > +             emc->requested_rate[i].max_rate = ULONG_MAX;
> > > +     }
> > > +}
> > > +
> > > +static int emc_request_rate(struct tegra186_emc *emc,
> > > +                         unsigned long new_min_rate,
> > > +                         unsigned long new_max_rate,
> > > +                         enum emc_rate_request_type type)
> > > +{
> > > +     struct emc_rate_request *req = emc->requested_rate;
> > > +     unsigned long min_rate = 0, max_rate = ULONG_MAX;
> > > +     unsigned int i;
> > > +     int err;
> > > +
> > > +     /* select minimum and maximum rates among the requested rates */
> > > +     for (i = 0; i < EMC_RATE_TYPE_MAX; i++, req++) {
> > > +             if (i == type) {
> > > +                     min_rate = max(new_min_rate, min_rate);
> > > +                     max_rate = min(new_max_rate, max_rate);
> > > +             } else {
> > > +                     min_rate = max(req->min_rate, min_rate);
> > > +                     max_rate = min(req->max_rate, max_rate);
> > > +             }
> > > +     }
> > > +
> > > +     if (min_rate > max_rate) {
> > > +             dev_err_ratelimited(emc->dev, "%s: type %u: out of range: %lu %lu\n",
> > > +                                 __func__, type, min_rate, max_rate);
> > > +             return -ERANGE;
> > > +     }
> > > +
> > > +     err = clk_set_rate(emc->clk, min_rate);
> > > +     if (err)
> > > +             return err;
> > > +
> > > +     emc->requested_rate[type].min_rate = new_min_rate;
> > > +     emc->requested_rate[type].max_rate = new_max_rate;
> > > +
> > > +     return 0;
> > > +}
> > > +
> > > +static int emc_set_min_rate(struct tegra186_emc *emc, unsigned long rate,
> > > +                         enum emc_rate_request_type type)
> > > +{
> > > +     struct emc_rate_request *req = &emc->requested_rate[type];
> > > +     int ret;
> > > +
> > > +     mutex_lock(&emc->rate_lock);
> > > +     ret = emc_request_rate(emc, rate, req->max_rate, type);
> > > +     mutex_unlock(&emc->rate_lock);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > > +static int emc_set_max_rate(struct tegra186_emc *emc, unsigned long rate,
> > > +                         enum emc_rate_request_type type)
> > > +{
> > > +     struct emc_rate_request *req = &emc->requested_rate[type];
> > > +     int ret;
> > > +
> > > +     mutex_lock(&emc->rate_lock);
> > > +     ret = emc_request_rate(emc, req->min_rate, rate, type);
> > > +     mutex_unlock(&emc->rate_lock);
> > > +
> > > +     return ret;
> > > +}
> > > +
> > >   /*
> > >    * debugfs interface
> > >    *
> > > @@ -107,7 +200,7 @@ static int tegra186_emc_debug_min_rate_set(void *data, u64 rate)
> > >       if (!tegra186_emc_validate_rate(emc, rate))
> > >               return -EINVAL;
> > >
> > > -     err = clk_set_min_rate(emc->clk, rate);
> > > +     err = emc_set_min_rate(emc, rate, EMC_RATE_DEBUG);
> > >       if (err < 0)
> > >               return err;
> > >
> > > @@ -137,7 +230,7 @@ static int tegra186_emc_debug_max_rate_set(void *data, u64 rate)
> > >       if (!tegra186_emc_validate_rate(emc, rate))
> > >               return -EINVAL;
> > >
> > > -     err = clk_set_max_rate(emc->clk, rate);
> > > +     err = emc_set_max_rate(emc, rate, EMC_RATE_DEBUG);
> > >       if (err < 0)
> > >               return err;
> > >
> > > @@ -217,6 +310,12 @@ static int tegra186_emc_get_emc_dvfs_latency(struct tegra186_emc *emc)
> > >       return 0;
> > >   }
> > >
> > > +static inline struct tegra186_emc *
> > > +to_tegra186_emc_provider(struct icc_provider *provider)
> > > +{
> > > +     return container_of(provider, struct tegra186_emc, provider);
> > > +}
> > > +
> > >   /*
> > >    * tegra186_emc_icc_set_bw() - Set BW api for EMC provider
> > >    * @src: ICC node for External Memory Controller (EMC)
> > > @@ -227,6 +326,33 @@ static int tegra186_emc_get_emc_dvfs_latency(struct tegra186_emc *emc)
> > >    */
> > >   static int tegra186_emc_icc_set_bw(struct icc_node *src, struct icc_node *dst)
> > >   {
> > > +     struct tegra186_emc *emc = to_tegra186_emc_provider(dst->provider);
> > > +     struct tegra_mc *mc = dev_get_drvdata(emc->dev->parent);
> > > +     unsigned long long peak_bw = icc_units_to_bps(dst->peak_bw);
> > > +     unsigned long long avg_bw = icc_units_to_bps(dst->avg_bw);
> > > +     unsigned long long rate = max(avg_bw, peak_bw);
> > > +     const unsigned int ddr = 2;
> > > +     int err;
> > > +
> > > +     /*
> > > +      * Do nothing here if bwmgr is supported in BPMP-FW. BPMP-FW sets the final
> > > +      * Freq based on the passed values.
> > > +      */
> > > +     if (mc->bwmgr_mrq_supported)
> > > +             return 0;
> > > +
> > > +     /*
> > > +      * Tegra186 EMC runs on a clock rate of SDRAM bus. This means that
> > > +      * EMC clock rate is twice smaller than the peak data rate because
> > > +      * data is sampled on both EMC clock edges.
> > > +      */
> > > +     do_div(rate, ddr);
> > > +     rate = min_t(u64, rate, U32_MAX);
> > > +
> > > +     err = emc_set_min_rate(emc, rate, EMC_RATE_ICC);
> > > +     if (err)
> > > +             return err;
> > > +
> > >       return 0;
> > >   }
> > >
> > > @@ -329,6 +455,8 @@ static int tegra186_emc_probe(struct platform_device *pdev)
> > >       platform_set_drvdata(pdev, emc);
> > >       emc->dev = &pdev->dev;
> > >
> > > +     tegra186_emc_rate_requests_init(emc);
> > > +
> > >       if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_EMC_DVFS_LATENCY)) {
> > >               err = tegra186_emc_get_emc_dvfs_latency(emc);
> > >               if (err)
> > >
> >
> >
> > FYI, this patch is causing a boot regression on Tegra194 devices. I
> > noticed that tegra194-p2972-0000 and tegra194-p3509-0000+p3668-0000 are
> > no longer booting and bisect is pointing to this. I will have a closer
> > look and try to see why this is.
>
> Interesting. Both were booting for me during my verification, though
> my use case involves the dt changes that I don't believe have been
> picked up yet. Thought I had explicitly verified without the dt
> changes too, though. Since I was asked to do so on this or one of the
> other similar series. I will try to check linux-next as-is soon.

I just built next-20251110 using the standard arm64 defconfig and
flashed the resulting Image and dtb's to p2972 and p3518 (p3509+p3668)
and both booted to cli on a barebones busybox ramdisk. I do not see
any errors from tegra-mc, and the only error I see from tegra186-emc
is that it can't find the opp tables, which is expected without the dt
changes, and is not fatal.

Aaron

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 11/11/2025 01:39, Aaron Kling wrote:
> On Mon, Nov 10, 2025 at 3:55 PM Aaron Kling <webgeek1234@gmail.com> wrote:
>>
>> On Mon, Nov 10, 2025 at 3:25 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>>>
>>>
>>> On 27/10/2025 18:55, Aaron Kling via B4 Relay wrote:
>>>> From: Aaron Kling <webgeek1234@gmail.com>
>>>>
>>>> This adds support for dynamic frequency scaling of external memory on
>>>> devices with bpmp firmware that does not support bwmgr.
>>>>
>>>> Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
>>>> ---
>>>>    drivers/memory/tegra/tegra186-emc.c | 132 +++++++++++++++++++++++++++++++++++-
>>>>    1 file changed, 130 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/memory/tegra/tegra186-emc.c b/drivers/memory/tegra/tegra186-emc.c
>>>> index 9959ad5804b444b269456d1fbae87b4bc111661b..74be09968baa7a0fbdce4359f470ce56b18acb10 100644
>>>> --- a/drivers/memory/tegra/tegra186-emc.c
>>>> +++ b/drivers/memory/tegra/tegra186-emc.c
>>>> @@ -18,6 +18,17 @@ struct tegra186_emc_dvfs {
>>>>        unsigned long rate;
>>>>    };
>>>>
>>>> +enum emc_rate_request_type {
>>>> +     EMC_RATE_DEBUG,
>>>> +     EMC_RATE_ICC,
>>>> +     EMC_RATE_TYPE_MAX,
>>>> +};
>>>> +
>>>> +struct emc_rate_request {
>>>> +     unsigned long min_rate;
>>>> +     unsigned long max_rate;
>>>> +};
>>>> +
>>>>    struct tegra186_emc {
>>>>        struct tegra_bpmp *bpmp;
>>>>        struct device *dev;
>>>> @@ -33,8 +44,90 @@ struct tegra186_emc {
>>>>        } debugfs;
>>>>
>>>>        struct icc_provider provider;
>>>> +
>>>> +     /*
>>>> +      * There are multiple sources in the EMC driver which could request
>>>> +      * a min/max clock rate, these rates are contained in this array.
>>>> +      */
>>>> +     struct emc_rate_request requested_rate[EMC_RATE_TYPE_MAX];
>>>> +
>>>> +     /* protect shared rate-change code path */
>>>> +     struct mutex rate_lock;
>>>>    };
>>>>
>>>> +static void tegra186_emc_rate_requests_init(struct tegra186_emc *emc)
>>>> +{
>>>> +     unsigned int i;
>>>> +
>>>> +     for (i = 0; i < EMC_RATE_TYPE_MAX; i++) {
>>>> +             emc->requested_rate[i].min_rate = 0;
>>>> +             emc->requested_rate[i].max_rate = ULONG_MAX;
>>>> +     }
>>>> +}
>>>> +
>>>> +static int emc_request_rate(struct tegra186_emc *emc,
>>>> +                         unsigned long new_min_rate,
>>>> +                         unsigned long new_max_rate,
>>>> +                         enum emc_rate_request_type type)
>>>> +{
>>>> +     struct emc_rate_request *req = emc->requested_rate;
>>>> +     unsigned long min_rate = 0, max_rate = ULONG_MAX;
>>>> +     unsigned int i;
>>>> +     int err;
>>>> +
>>>> +     /* select minimum and maximum rates among the requested rates */
>>>> +     for (i = 0; i < EMC_RATE_TYPE_MAX; i++, req++) {
>>>> +             if (i == type) {
>>>> +                     min_rate = max(new_min_rate, min_rate);
>>>> +                     max_rate = min(new_max_rate, max_rate);
>>>> +             } else {
>>>> +                     min_rate = max(req->min_rate, min_rate);
>>>> +                     max_rate = min(req->max_rate, max_rate);
>>>> +             }
>>>> +     }
>>>> +
>>>> +     if (min_rate > max_rate) {
>>>> +             dev_err_ratelimited(emc->dev, "%s: type %u: out of range: %lu %lu\n",
>>>> +                                 __func__, type, min_rate, max_rate);
>>>> +             return -ERANGE;
>>>> +     }
>>>> +
>>>> +     err = clk_set_rate(emc->clk, min_rate);
>>>> +     if (err)
>>>> +             return err;
>>>> +
>>>> +     emc->requested_rate[type].min_rate = new_min_rate;
>>>> +     emc->requested_rate[type].max_rate = new_max_rate;
>>>> +
>>>> +     return 0;
>>>> +}
>>>> +
>>>> +static int emc_set_min_rate(struct tegra186_emc *emc, unsigned long rate,
>>>> +                         enum emc_rate_request_type type)
>>>> +{
>>>> +     struct emc_rate_request *req = &emc->requested_rate[type];
>>>> +     int ret;
>>>> +
>>>> +     mutex_lock(&emc->rate_lock);
>>>> +     ret = emc_request_rate(emc, rate, req->max_rate, type);
>>>> +     mutex_unlock(&emc->rate_lock);
>>>> +
>>>> +     return ret;
>>>> +}
>>>> +
>>>> +static int emc_set_max_rate(struct tegra186_emc *emc, unsigned long rate,
>>>> +                         enum emc_rate_request_type type)
>>>> +{
>>>> +     struct emc_rate_request *req = &emc->requested_rate[type];
>>>> +     int ret;
>>>> +
>>>> +     mutex_lock(&emc->rate_lock);
>>>> +     ret = emc_request_rate(emc, req->min_rate, rate, type);
>>>> +     mutex_unlock(&emc->rate_lock);
>>>> +
>>>> +     return ret;
>>>> +}
>>>> +
>>>>    /*
>>>>     * debugfs interface
>>>>     *
>>>> @@ -107,7 +200,7 @@ static int tegra186_emc_debug_min_rate_set(void *data, u64 rate)
>>>>        if (!tegra186_emc_validate_rate(emc, rate))
>>>>                return -EINVAL;
>>>>
>>>> -     err = clk_set_min_rate(emc->clk, rate);
>>>> +     err = emc_set_min_rate(emc, rate, EMC_RATE_DEBUG);
>>>>        if (err < 0)
>>>>                return err;
>>>>
>>>> @@ -137,7 +230,7 @@ static int tegra186_emc_debug_max_rate_set(void *data, u64 rate)
>>>>        if (!tegra186_emc_validate_rate(emc, rate))
>>>>                return -EINVAL;
>>>>
>>>> -     err = clk_set_max_rate(emc->clk, rate);
>>>> +     err = emc_set_max_rate(emc, rate, EMC_RATE_DEBUG);
>>>>        if (err < 0)
>>>>                return err;
>>>>
>>>> @@ -217,6 +310,12 @@ static int tegra186_emc_get_emc_dvfs_latency(struct tegra186_emc *emc)
>>>>        return 0;
>>>>    }
>>>>
>>>> +static inline struct tegra186_emc *
>>>> +to_tegra186_emc_provider(struct icc_provider *provider)
>>>> +{
>>>> +     return container_of(provider, struct tegra186_emc, provider);
>>>> +}
>>>> +
>>>>    /*
>>>>     * tegra186_emc_icc_set_bw() - Set BW api for EMC provider
>>>>     * @src: ICC node for External Memory Controller (EMC)
>>>> @@ -227,6 +326,33 @@ static int tegra186_emc_get_emc_dvfs_latency(struct tegra186_emc *emc)
>>>>     */
>>>>    static int tegra186_emc_icc_set_bw(struct icc_node *src, struct icc_node *dst)
>>>>    {
>>>> +     struct tegra186_emc *emc = to_tegra186_emc_provider(dst->provider);
>>>> +     struct tegra_mc *mc = dev_get_drvdata(emc->dev->parent);
>>>> +     unsigned long long peak_bw = icc_units_to_bps(dst->peak_bw);
>>>> +     unsigned long long avg_bw = icc_units_to_bps(dst->avg_bw);
>>>> +     unsigned long long rate = max(avg_bw, peak_bw);
>>>> +     const unsigned int ddr = 2;
>>>> +     int err;
>>>> +
>>>> +     /*
>>>> +      * Do nothing here if bwmgr is supported in BPMP-FW. BPMP-FW sets the final
>>>> +      * Freq based on the passed values.
>>>> +      */
>>>> +     if (mc->bwmgr_mrq_supported)
>>>> +             return 0;
>>>> +
>>>> +     /*
>>>> +      * Tegra186 EMC runs on a clock rate of SDRAM bus. This means that
>>>> +      * EMC clock rate is twice smaller than the peak data rate because
>>>> +      * data is sampled on both EMC clock edges.
>>>> +      */
>>>> +     do_div(rate, ddr);
>>>> +     rate = min_t(u64, rate, U32_MAX);
>>>> +
>>>> +     err = emc_set_min_rate(emc, rate, EMC_RATE_ICC);
>>>> +     if (err)
>>>> +             return err;
>>>> +
>>>>        return 0;
>>>>    }
>>>>
>>>> @@ -329,6 +455,8 @@ static int tegra186_emc_probe(struct platform_device *pdev)
>>>>        platform_set_drvdata(pdev, emc);
>>>>        emc->dev = &pdev->dev;
>>>>
>>>> +     tegra186_emc_rate_requests_init(emc);
>>>> +
>>>>        if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_EMC_DVFS_LATENCY)) {
>>>>                err = tegra186_emc_get_emc_dvfs_latency(emc);
>>>>                if (err)
>>>>
>>>
>>>
>>> FYI, this patch is causing a boot regression on Tegra194 devices. I
>>> noticed that tegra194-p2972-0000 and tegra194-p3509-0000+p3668-0000 are
>>> no longer booting and bisect is pointing to this. I will have a closer
>>> look and try to see why this is.
>>
>> Interesting. Both were booting for me during my verification, though
>> my use case involves the dt changes that I don't believe have been
>> picked up yet. Thought I had explicitly verified without the dt
>> changes too, though. Since I was asked to do so on this or one of the
>> other similar series. I will try to check linux-next as-is soon.
> 
> I just built next-20251110 using the standard arm64 defconfig and
> flashed the resulting Image and dtb's to p2972 and p3518 (p3509+p3668)
> and both booted to cli on a barebones busybox ramdisk. I do not see
> any errors from tegra-mc, and the only error I see from tegra186-emc
> is that it can't find the opp tables, which is expected without the dt
> changes, and is not fatal.

Thanks for testing. Something is not right because our boards are 
failing. So may be we are doing/testing something different. However, 
this should not break. So there is a problem here.

Jon

-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Krzysztof Kozlowski]

On 11/11/2025 12:13, Jon Hunter wrote:
>>>>> +
>>>>>        if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_EMC_DVFS_LATENCY)) {
>>>>>                err = tegra186_emc_get_emc_dvfs_latency(emc);
>>>>>                if (err)
>>>>>
>>>>
>>>>
>>>> FYI, this patch is causing a boot regression on Tegra194 devices. I
>>>> noticed that tegra194-p2972-0000 and tegra194-p3509-0000+p3668-0000 are
>>>> no longer booting and bisect is pointing to this. I will have a closer
>>>> look and try to see why this is.
>>>
>>> Interesting. Both were booting for me during my verification, though
>>> my use case involves the dt changes that I don't believe have been
>>> picked up yet. Thought I had explicitly verified without the dt
>>> changes too, though. Since I was asked to do so on this or one of the
>>> other similar series. I will try to check linux-next as-is soon.
>>
>> I just built next-20251110 using the standard arm64 defconfig and
>> flashed the resulting Image and dtb's to p2972 and p3518 (p3509+p3668)
>> and both booted to cli on a barebones busybox ramdisk. I do not see
>> any errors from tegra-mc, and the only error I see from tegra186-emc
>> is that it can't find the opp tables, which is expected without the dt
>> changes, and is not fatal.
> 
> Thanks for testing. Something is not right because our boards are 
> failing. So may be we are doing/testing something different. However, 
> this should not break. So there is a problem here.


Did you meant: "So there is NO problem here"?

I kept these for 10 days in linux-next and yesterday sent them in pull
request. If some patches are needed on top, they can still fit coming
merge window if sent soon.


Best regards,
Krzysztof

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 11/11/2025 11:16, Krzysztof Kozlowski wrote:
> On 11/11/2025 12:13, Jon Hunter wrote:
>>>>>> +
>>>>>>         if (tegra_bpmp_mrq_is_supported(emc->bpmp, MRQ_EMC_DVFS_LATENCY)) {
>>>>>>                 err = tegra186_emc_get_emc_dvfs_latency(emc);
>>>>>>                 if (err)
>>>>>>
>>>>>
>>>>>
>>>>> FYI, this patch is causing a boot regression on Tegra194 devices. I
>>>>> noticed that tegra194-p2972-0000 and tegra194-p3509-0000+p3668-0000 are
>>>>> no longer booting and bisect is pointing to this. I will have a closer
>>>>> look and try to see why this is.
>>>>
>>>> Interesting. Both were booting for me during my verification, though
>>>> my use case involves the dt changes that I don't believe have been
>>>> picked up yet. Thought I had explicitly verified without the dt
>>>> changes too, though. Since I was asked to do so on this or one of the
>>>> other similar series. I will try to check linux-next as-is soon.
>>>
>>> I just built next-20251110 using the standard arm64 defconfig and
>>> flashed the resulting Image and dtb's to p2972 and p3518 (p3509+p3668)
>>> and both booted to cli on a barebones busybox ramdisk. I do not see
>>> any errors from tegra-mc, and the only error I see from tegra186-emc
>>> is that it can't find the opp tables, which is expected without the dt
>>> changes, and is not fatal.
>>
>> Thanks for testing. Something is not right because our boards are
>> failing. So may be we are doing/testing something different. However,
>> this should not break. So there is a problem here.
> 
> 
> Did you meant: "So there is NO problem here"?

Nope. I mean that this is a problem here.

> I kept these for 10 days in linux-next and yesterday sent them in pull
> request. If some patches are needed on top, they can still fit coming
> merge window if sent soon.

Looking back I see it started failing with next-20251103. next-20251031 
was fine. Reverting this commit on top of next-20251110 fixes the issue.

There may be a difference in the firmware being used. Our testing is 
based upon an older NVIDIA L4T r32.5.1 release but nonetheless, we 
should not break that.

Jon
-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 11/11/2025 12:05, Jon Hunter wrote:

...

>>> Thanks for testing. Something is not right because our boards are
>>> failing. So may be we are doing/testing something different. However,
>>> this should not break. So there is a problem here.
>>
>>
>> Did you meant: "So there is NO problem here"?
> 
> Nope. I mean that this is a problem here.
> 
>> I kept these for 10 days in linux-next and yesterday sent them in pull
>> request. If some patches are needed on top, they can still fit coming
>> merge window if sent soon.
> 
> Looking back I see it started failing with next-20251103. next-20251031 
> was fine. Reverting this commit on top of next-20251110 fixes the issue.
> 
> There may be a difference in the firmware being used. Our testing is 
> based upon an older NVIDIA L4T r32.5.1 release but nonetheless, we 
> should not break that.


OK, so I see what is happening here. The boot test that we are running 
has a 2 minute timeout and the board is now failing to boot within that 
time.

Adding some debug prints, I can see that initially the EMC clock 
frequency is 1600MHz and now after this change, on boot the EMC clock 
get set to 250MHz. Hence, the booting is now taking significantly longer 
and the test times out.

We definitely don't want to increase the timeout of the test. Any thoughts?

Jon

-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Aaron Kling]

On Tue, Nov 11, 2025 at 8:35 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 11/11/2025 12:05, Jon Hunter wrote:
>
> ...
>
> >>> Thanks for testing. Something is not right because our boards are
> >>> failing. So may be we are doing/testing something different. However,
> >>> this should not break. So there is a problem here.
> >>
> >>
> >> Did you meant: "So there is NO problem here"?
> >
> > Nope. I mean that this is a problem here.
> >
> >> I kept these for 10 days in linux-next and yesterday sent them in pull
> >> request. If some patches are needed on top, they can still fit coming
> >> merge window if sent soon.
> >
> > Looking back I see it started failing with next-20251103. next-20251031
> > was fine. Reverting this commit on top of next-20251110 fixes the issue.
> >
> > There may be a difference in the firmware being used. Our testing is
> > based upon an older NVIDIA L4T r32.5.1 release but nonetheless, we
> > should not break that.
>
>
> OK, so I see what is happening here. The boot test that we are running
> has a 2 minute timeout and the board is now failing to boot within that
> time.
>
> Adding some debug prints, I can see that initially the EMC clock
> frequency is 1600MHz and now after this change, on boot the EMC clock
> get set to 250MHz. Hence, the booting is now taking significantly longer
> and the test times out.
>
> We definitely don't want to increase the timeout of the test. Any thoughts?

My setup uses the boot stack from L4T r32.7.6, though cboot is source
built and has had changes over time to support newer Android versions.
There shouldn't be anything there that would affect emc clock, though.

I'm seeing the emc clock stay at the boot value, namely 1600MHz. Per
both debugfs clk/emc/clk_rate and bpmp/debug/clk/emc/rate. I don't
even see 250MHz as an option. Debugfs emc/available_rates lists 204MHz
as the closest entry.

I'm trying to think what could cause a drop in the selected clock
rate. This patch should only dynamically change the rate if the opp
tables exist, enabling the cpufreq based scaling via icc. But those
tables don't exist on linux-next right now. My test ramdisk does
nothing except set up sysfs/procfs/etc just enough to run a busybox
shell for debugging. Do the Nvidia regression testing boot scripts do
anything to sysfs or debugfs that would affect emc?

Aaron

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 11/11/2025 17:04, Aaron Kling wrote:

...

> My setup uses the boot stack from L4T r32.7.6, though cboot is source
> built and has had changes over time to support newer Android versions.
> There shouldn't be anything there that would affect emc clock, though.
> 
> I'm seeing the emc clock stay at the boot value, namely 1600MHz. Per
> both debugfs clk/emc/clk_rate and bpmp/debug/clk/emc/rate. I don't
> even see 250MHz as an option. Debugfs emc/available_rates lists 204MHz
> as the closest entry.
> 
> I'm trying to think what could cause a drop in the selected clock
> rate. This patch should only dynamically change the rate if the opp
> tables exist, enabling the cpufreq based scaling via icc. But those
> tables don't exist on linux-next right now. My test ramdisk does
> nothing except set up sysfs/procfs/etc just enough to run a busybox
> shell for debugging. Do the Nvidia regression testing boot scripts do
> anything to sysfs or debugfs that would affect emc?

So this is definitely coming from ICC. On boot I see a request for
250MHz coming from the PCIe driver ...

[   13.861227] tegra186_emc_icc_set_bw-356: rate 250000000
[   13.861350] CPU: 1 UID: 0 PID: 68 Comm: kworker/u32:1 Not tainted 6.18.0-rc4-next-20251110-00001-gfc12493c80fb-dirty #9 PREEMPT
[   13.861362] Hardware name: NVIDIA Jetson AGX Xavier Developer Kit (DT)
[   13.861370] Workqueue: events_unbound deferred_probe_work_func
[   13.861388] Call trace:
[   13.861393]  show_stack+0x18/0x24 (C)
[   13.861407]  dump_stack_lvl+0x74/0x8c
[   13.861419]  dump_stack+0x18/0x24
[   13.861426]  tegra186_emc_icc_set_bw+0xc8/0x14c
[   13.861438]  apply_constraints+0x70/0xb0
[   13.861451]  icc_set_bw+0x88/0x128
[   13.861461]  tegra_pcie_icc_set+0x7c/0x10c [pcie_tegra194]
[   13.861499]  tegra_pcie_dw_start_link+0x178/0x2b0 [pcie_tegra194]
[   13.861510]  dw_pcie_host_init+0x664/0x6e0
[   13.861523]  tegra_pcie_dw_probe+0x6d4/0xbfc [pcie_tegra194]
[   13.861534]  platform_probe+0x5c/0x98
[   13.861547]  really_probe+0xbc/0x2a8
[   13.861555]  __driver_probe_device+0x78/0x12c
[   13.861563]  driver_probe_device+0x3c/0x15c
[   13.861572]  __device_attach_driver+0xb8/0x134
[   13.861580]  bus_for_each_drv+0x84/0xe0
[   13.861588]  __device_attach+0x9c/0x188
[   13.861596]  device_initial_probe+0x14/0x20
[   13.861610]  bus_probe_device+0xac/0xb0
[   13.861619]  deferred_probe_work_func+0x88/0xc0
[   13.861627]  process_one_work+0x148/0x28c
[   13.861640]  worker_thread+0x2d0/0x3d8
[   13.861648]  kthread+0x128/0x200
[   13.861659]  ret_from_fork+0x10/0x20

The actual rate that is set is 408MHz if I read the rate after
it is set ...

[   13.912099] tegra186_emc_icc_set_bw-362: rate 408000000

This is a simple boot test and so nothing we are doing via
debugfs/sysfs to influence this.

Jon

-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Aaron Kling]

On Tue, Nov 11, 2025 at 3:29 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 11/11/2025 17:04, Aaron Kling wrote:
>
> ...
>
> > My setup uses the boot stack from L4T r32.7.6, though cboot is source
> > built and has had changes over time to support newer Android versions.
> > There shouldn't be anything there that would affect emc clock, though.
> >
> > I'm seeing the emc clock stay at the boot value, namely 1600MHz. Per
> > both debugfs clk/emc/clk_rate and bpmp/debug/clk/emc/rate. I don't
> > even see 250MHz as an option. Debugfs emc/available_rates lists 204MHz
> > as the closest entry.
> >
> > I'm trying to think what could cause a drop in the selected clock
> > rate. This patch should only dynamically change the rate if the opp
> > tables exist, enabling the cpufreq based scaling via icc. But those
> > tables don't exist on linux-next right now. My test ramdisk does
> > nothing except set up sysfs/procfs/etc just enough to run a busybox
> > shell for debugging. Do the Nvidia regression testing boot scripts do
> > anything to sysfs or debugfs that would affect emc?
>
> So this is definitely coming from ICC. On boot I see a request for
> 250MHz coming from the PCIe driver ...
>
> [   13.861227] tegra186_emc_icc_set_bw-356: rate 250000000
> [   13.861350] CPU: 1 UID: 0 PID: 68 Comm: kworker/u32:1 Not tainted 6.18.0-rc4-next-20251110-00001-gfc12493c80fb-dirty #9 PREEMPT
> [   13.861362] Hardware name: NVIDIA Jetson AGX Xavier Developer Kit (DT)
> [   13.861370] Workqueue: events_unbound deferred_probe_work_func
> [   13.861388] Call trace:
> [   13.861393]  show_stack+0x18/0x24 (C)
> [   13.861407]  dump_stack_lvl+0x74/0x8c
> [   13.861419]  dump_stack+0x18/0x24
> [   13.861426]  tegra186_emc_icc_set_bw+0xc8/0x14c
> [   13.861438]  apply_constraints+0x70/0xb0
> [   13.861451]  icc_set_bw+0x88/0x128
> [   13.861461]  tegra_pcie_icc_set+0x7c/0x10c [pcie_tegra194]
> [   13.861499]  tegra_pcie_dw_start_link+0x178/0x2b0 [pcie_tegra194]
> [   13.861510]  dw_pcie_host_init+0x664/0x6e0
> [   13.861523]  tegra_pcie_dw_probe+0x6d4/0xbfc [pcie_tegra194]
> [   13.861534]  platform_probe+0x5c/0x98
> [   13.861547]  really_probe+0xbc/0x2a8
> [   13.861555]  __driver_probe_device+0x78/0x12c
> [   13.861563]  driver_probe_device+0x3c/0x15c
> [   13.861572]  __device_attach_driver+0xb8/0x134
> [   13.861580]  bus_for_each_drv+0x84/0xe0
> [   13.861588]  __device_attach+0x9c/0x188
> [   13.861596]  device_initial_probe+0x14/0x20
> [   13.861610]  bus_probe_device+0xac/0xb0
> [   13.861619]  deferred_probe_work_func+0x88/0xc0
> [   13.861627]  process_one_work+0x148/0x28c
> [   13.861640]  worker_thread+0x2d0/0x3d8
> [   13.861648]  kthread+0x128/0x200
> [   13.861659]  ret_from_fork+0x10/0x20
>
> The actual rate that is set is 408MHz if I read the rate after
> it is set ...
>
> [   13.912099] tegra186_emc_icc_set_bw-362: rate 408000000
>
> This is a simple boot test and so nothing we are doing via
> debugfs/sysfs to influence this.

Alright, I think I've got the picture of what's going on now. The
standard arm64 defconfig enables the t194 pcie driver as a module. And
my simple busybox ramdisk that I use for mainline regression testing
isn't loading any modules. If I set the pcie driver to built-in, I
replicate the issue. And I don't see the issue on my normal use case,
because I have the dt changes as well.

So it appears that the pcie driver submits icc bandwidth. And without
cpufreq submitting bandwidth as well, the emc driver gets a very low
number and thus sets a very low emc freq. The question becomes... what
to do about it? If the related dt changes were submitted to
linux-next, everything should fall into place. And I'm not sure where
this falls on the severity scale since it doesn't full out break boot
or prevent operation.

Aaron

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 11/11/2025 23:17, Aaron Kling wrote:

...

> Alright, I think I've got the picture of what's going on now. The
> standard arm64 defconfig enables the t194 pcie driver as a module. And
> my simple busybox ramdisk that I use for mainline regression testing
> isn't loading any modules. If I set the pcie driver to built-in, I
> replicate the issue. And I don't see the issue on my normal use case,
> because I have the dt changes as well.
> 
> So it appears that the pcie driver submits icc bandwidth. And without
> cpufreq submitting bandwidth as well, the emc driver gets a very low
> number and thus sets a very low emc freq. The question becomes... what
> to do about it? If the related dt changes were submitted to
> linux-next, everything should fall into place. And I'm not sure where
> this falls on the severity scale since it doesn't full out break boot
> or prevent operation.

Where are the related DT changes? If we can get these into -next and 
lined up to be merged for v6.19, then that is fine. However, we should 
not merge this for v6.19 without the DT changes.

I will also talk with Thierry to see if he has any concerns about users 
seeing slow performance if they don't have an up-to-date DTB.

Is there any easy way to detect if the DTB has he necessary properties 
to enable ICC scaling?

Jon

-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Aaron Kling]

On Wed, Nov 12, 2025 at 12:18 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 11/11/2025 23:17, Aaron Kling wrote:
>
> ...
>
> > Alright, I think I've got the picture of what's going on now. The
> > standard arm64 defconfig enables the t194 pcie driver as a module. And
> > my simple busybox ramdisk that I use for mainline regression testing
> > isn't loading any modules. If I set the pcie driver to built-in, I
> > replicate the issue. And I don't see the issue on my normal use case,
> > because I have the dt changes as well.
> >
> > So it appears that the pcie driver submits icc bandwidth. And without
> > cpufreq submitting bandwidth as well, the emc driver gets a very low
> > number and thus sets a very low emc freq. The question becomes... what
> > to do about it? If the related dt changes were submitted to
> > linux-next, everything should fall into place. And I'm not sure where
> > this falls on the severity scale since it doesn't full out break boot
> > or prevent operation.
>
> Where are the related DT changes? If we can get these into -next and
> lined up to be merged for v6.19, then that is fine. However, we should
> not merge this for v6.19 without the DT changes.

The dt changes are here [0].

This was all part of the same series, keeping everything logically
related together. But on v2, Krzysztof said that none of this should
have ever been together and that each subsystem should get a separate
series, even if the changes are related. Which I did, and now this is
split across three series. The actmon series for tegra210 is in a
similar state. Split across four series and only one has been pulled
to linux-next.

> I will also talk with Thierry to see if he has any concerns about users
> seeing slow performance if they don't have an up-to-date DTB.
>
> Is there any easy way to detect if the DTB has he necessary properties
> to enable ICC scaling?

I'm not sure there is any simple way, given how I set up tegra186 and
tegra194. The new dt properties are on the cpu nodes, there's nothing
new for the emc node. So the emc driver just unconditionally declares
itself to icc. It was doing this before too, but wouldn't do anything
on tegra186 or tegra194 because the set_bw function was just a stub
and the real logic happened in the bpmp bw mgr, which only exists on
tegra234+. Now the set_bw function will directly calculate and set the
emc clock as long as the bpmp bw mgr is not supported. Offhand, I
can't think of anything existing to check to skip this, because
nothing new in the dt has been added in the scope of emc.

Aaron

[0] https://lore.kernel.org/r/20251021-tegra186-icc-p3-v3-0-68184ee8a89c@gmail.com

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Krzysztof Kozlowski]

On 12/11/2025 08:21, Aaron Kling wrote:
> On Wed, Nov 12, 2025 at 12:18 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>>
>> On 11/11/2025 23:17, Aaron Kling wrote:
>>
>> ...
>>
>>> Alright, I think I've got the picture of what's going on now. The
>>> standard arm64 defconfig enables the t194 pcie driver as a module. And
>>> my simple busybox ramdisk that I use for mainline regression testing
>>> isn't loading any modules. If I set the pcie driver to built-in, I
>>> replicate the issue. And I don't see the issue on my normal use case,
>>> because I have the dt changes as well.
>>>
>>> So it appears that the pcie driver submits icc bandwidth. And without
>>> cpufreq submitting bandwidth as well, the emc driver gets a very low
>>> number and thus sets a very low emc freq. The question becomes... what
>>> to do about it? If the related dt changes were submitted to
>>> linux-next, everything should fall into place. And I'm not sure where
>>> this falls on the severity scale since it doesn't full out break boot
>>> or prevent operation.
>>
>> Where are the related DT changes? If we can get these into -next and
>> lined up to be merged for v6.19, then that is fine. However, we should
>> not merge this for v6.19 without the DT changes.
> 
> The dt changes are here [0].
> 
> This was all part of the same series, keeping everything logically
> related together. But on v2, Krzysztof said that none of this should

I asked you about dependencies between the patches and you said there
are none, so collecting different subsystems into one is wrong. That's
nothing new, standard Linux kernel process.

What is non-standard here is keeping secret that there is impact on users.

> have ever been together and that each subsystem should get a separate
> series, even if the changes are related. Which I did, and now this is
> split across three series. The actmon series for tegra210 is in a
> similar state. Split across four series and only one has been pulled
> to linux-next.
> 
>> I will also talk with Thierry to see if he has any concerns about users
>> seeing slow performance if they don't have an up-to-date DTB.
>>
>> Is there any easy way to detect if the DTB has he necessary properties
>> to enable ICC scaling?
> 
> I'm not sure there is any simple way, given how I set up tegra186 and
> tegra194. The new dt properties are on the cpu nodes, there's nothing
> new for the emc node. So the emc driver just unconditionally declares
> itself to icc. It was doing this before too, but wouldn't do anything
> on tegra186 or tegra194 because the set_bw function was just a stub
> and the real logic happened in the bpmp bw mgr, which only exists on
> tegra234+. Now the set_bw function will directly calculate and set the
> emc clock as long as the bpmp bw mgr is not supported. Offhand, I
> can't think of anything existing to check to skip this, because
> nothing new in the dt has been added in the scope of emc.
If your ICC triggers without users, I think it is usual case - you
should not enable the sync_state but instead keep it disabled till you
have all the consumers in place.

Best regards,
Krzysztof

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 12/11/2025 07:21, Aaron Kling wrote:
> On Wed, Nov 12, 2025 at 12:18 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>>
>> On 11/11/2025 23:17, Aaron Kling wrote:
>>
>> ...
>>
>>> Alright, I think I've got the picture of what's going on now. The
>>> standard arm64 defconfig enables the t194 pcie driver as a module. And
>>> my simple busybox ramdisk that I use for mainline regression testing
>>> isn't loading any modules. If I set the pcie driver to built-in, I
>>> replicate the issue. And I don't see the issue on my normal use case,
>>> because I have the dt changes as well.
>>>
>>> So it appears that the pcie driver submits icc bandwidth. And without
>>> cpufreq submitting bandwidth as well, the emc driver gets a very low
>>> number and thus sets a very low emc freq. The question becomes... what
>>> to do about it? If the related dt changes were submitted to
>>> linux-next, everything should fall into place. And I'm not sure where
>>> this falls on the severity scale since it doesn't full out break boot
>>> or prevent operation.
>>
>> Where are the related DT changes? If we can get these into -next and
>> lined up to be merged for v6.19, then that is fine. However, we should
>> not merge this for v6.19 without the DT changes.
> 
> The dt changes are here [0].

To confirm, applying the DT changes do not fix this for me. Thierry is 
having a look at this to see if there is a way to fix this.

BTW, I have also noticed that Thierry's memory frequency test [0] is 
also failing on Tegra186. The test simply tries to set the frequency via 
the sysfs and this is now failing. I am seeing ...

memory: emc: - available rates: (* = current)
memory: emc:   -   40800000
memory: emc:   -   68000000
memory: emc:   -  102000000
memory: emc:   -  204000000
memory: emc:   -  408000000
memory: emc:   -  665600000
memory: emc:   -  800000000
memory: emc:   - 1062400000
memory: emc:   - 1331200000
memory: emc:   - 1600000000
memory: emc:   - 1866000000 *
memory: emc: - testing:
memory: emc:   -   40800000...OSError: [Errno 34] Numerical result out 
of range

Jon

[0] https://github.com/thierryreding/tegra-tests

-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Aaron Kling]

On Fri, Nov 21, 2025 at 5:21 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 12/11/2025 07:21, Aaron Kling wrote:
> > On Wed, Nov 12, 2025 at 12:18 AM Jon Hunter <jonathanh@nvidia.com> wrote:
> >>
> >>
> >> On 11/11/2025 23:17, Aaron Kling wrote:
> >>
> >> ...
> >>
> >>> Alright, I think I've got the picture of what's going on now. The
> >>> standard arm64 defconfig enables the t194 pcie driver as a module. And
> >>> my simple busybox ramdisk that I use for mainline regression testing
> >>> isn't loading any modules. If I set the pcie driver to built-in, I
> >>> replicate the issue. And I don't see the issue on my normal use case,
> >>> because I have the dt changes as well.
> >>>
> >>> So it appears that the pcie driver submits icc bandwidth. And without
> >>> cpufreq submitting bandwidth as well, the emc driver gets a very low
> >>> number and thus sets a very low emc freq. The question becomes... what
> >>> to do about it? If the related dt changes were submitted to
> >>> linux-next, everything should fall into place. And I'm not sure where
> >>> this falls on the severity scale since it doesn't full out break boot
> >>> or prevent operation.
> >>
> >> Where are the related DT changes? If we can get these into -next and
> >> lined up to be merged for v6.19, then that is fine. However, we should
> >> not merge this for v6.19 without the DT changes.
> >
> > The dt changes are here [0].
>
> To confirm, applying the DT changes do not fix this for me. Thierry is
> having a look at this to see if there is a way to fix this.
>
> BTW, I have also noticed that Thierry's memory frequency test [0] is
> also failing on Tegra186. The test simply tries to set the frequency via
> the sysfs and this is now failing. I am seeing ...
>
> memory: emc: - available rates: (* = current)
> memory: emc:   -   40800000
> memory: emc:   -   68000000
> memory: emc:   -  102000000
> memory: emc:   -  204000000
> memory: emc:   -  408000000
> memory: emc:   -  665600000
> memory: emc:   -  800000000
> memory: emc:   - 1062400000
> memory: emc:   - 1331200000
> memory: emc:   - 1600000000
> memory: emc:   - 1866000000 *
> memory: emc: - testing:
> memory: emc:   -   40800000...OSError: [Errno 34] Numerical result out
> of range

Question. Does this test run and pass on jetson-tk1? I based the
tegra210 and tegra186 [0] code on tegra124 [1]. And I don't see a
difference in the flow now. What appears to be happening is that icc
is reporting a high bandwidth, setting the emc min_freq to something
like 1600MHz. Then debugfs is having max_freq set to something low
like 40.8MHz. Then the linked code block fails because the higher of
the min_freqs is greater than the lower of the max_freqs. But if this
same test is run on jetson-tk1, I don't see how it passes. Unless
maybe the t124 actmon is consistently setting min freqs during the
tests.

An argument could be made that any attempt to set debugfs should win a
conflict with icc. That could be done. But if that needs done here,
I'd argue that it needs replicated across all other applicable emc
drivers too.

Aaron

[0] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/memory/tegra/tegra186-emc.c?h=next-20251121#n78
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/memory/tegra/tegra124-emc.c?h=v6.18-rc6#n1066

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 21/11/2025 18:17, Aaron Kling wrote:
> On Fri, Nov 21, 2025 at 5:21 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>>
>> On 12/11/2025 07:21, Aaron Kling wrote:
>>> On Wed, Nov 12, 2025 at 12:18 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>>>>
>>>>
>>>> On 11/11/2025 23:17, Aaron Kling wrote:
>>>>
>>>> ...
>>>>
>>>>> Alright, I think I've got the picture of what's going on now. The
>>>>> standard arm64 defconfig enables the t194 pcie driver as a module. And
>>>>> my simple busybox ramdisk that I use for mainline regression testing
>>>>> isn't loading any modules. If I set the pcie driver to built-in, I
>>>>> replicate the issue. And I don't see the issue on my normal use case,
>>>>> because I have the dt changes as well.
>>>>>
>>>>> So it appears that the pcie driver submits icc bandwidth. And without
>>>>> cpufreq submitting bandwidth as well, the emc driver gets a very low
>>>>> number and thus sets a very low emc freq. The question becomes... what
>>>>> to do about it? If the related dt changes were submitted to
>>>>> linux-next, everything should fall into place. And I'm not sure where
>>>>> this falls on the severity scale since it doesn't full out break boot
>>>>> or prevent operation.
>>>>
>>>> Where are the related DT changes? If we can get these into -next and
>>>> lined up to be merged for v6.19, then that is fine. However, we should
>>>> not merge this for v6.19 without the DT changes.
>>>
>>> The dt changes are here [0].
>>
>> To confirm, applying the DT changes do not fix this for me. Thierry is
>> having a look at this to see if there is a way to fix this.
>>
>> BTW, I have also noticed that Thierry's memory frequency test [0] is
>> also failing on Tegra186. The test simply tries to set the frequency via
>> the sysfs and this is now failing. I am seeing ...
>>
>> memory: emc: - available rates: (* = current)
>> memory: emc:   -   40800000
>> memory: emc:   -   68000000
>> memory: emc:   -  102000000
>> memory: emc:   -  204000000
>> memory: emc:   -  408000000
>> memory: emc:   -  665600000
>> memory: emc:   -  800000000
>> memory: emc:   - 1062400000
>> memory: emc:   - 1331200000
>> memory: emc:   - 1600000000
>> memory: emc:   - 1866000000 *
>> memory: emc: - testing:
>> memory: emc:   -   40800000...OSError: [Errno 34] Numerical result out
>> of range
> 
> Question. Does this test run and pass on jetson-tk1? I based the
> tegra210 and tegra186 [0] code on tegra124 [1]. And I don't see a
> difference in the flow now. What appears to be happening is that icc
> is reporting a high bandwidth, setting the emc min_freq to something
> like 1600MHz. Then debugfs is having max_freq set to something low
> like 40.8MHz. Then the linked code block fails because the higher of
> the min_freqs is greater than the lower of the max_freqs. But if this
> same test is run on jetson-tk1, I don't see how it passes. Unless
> maybe the t124 actmon is consistently setting min freqs during the
> tests.

So we don't currently run this test on Tegra124. We could certainly try. 
I don't recall if there was an issue that prevented us from doing so now.

> An argument could be made that any attempt to set debugfs should win a
> conflict with icc. That could be done. But if that needs done here,
> I'd argue that it needs replicated across all other applicable emc
> drivers too.

The bottom line is that we cannot regress anything that was working before.

Jon

-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Aaron Kling]

On Tue, Dec 9, 2025 at 10:08 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 21/11/2025 18:17, Aaron Kling wrote:
> > On Fri, Nov 21, 2025 at 5:21 AM Jon Hunter <jonathanh@nvidia.com> wrote:
> >>
> >>
> >> On 12/11/2025 07:21, Aaron Kling wrote:
> >>> On Wed, Nov 12, 2025 at 12:18 AM Jon Hunter <jonathanh@nvidia.com> wrote:
> >>>>
> >>>>
> >>>> On 11/11/2025 23:17, Aaron Kling wrote:
> >>>>
> >>>> ...
> >>>>
> >>>>> Alright, I think I've got the picture of what's going on now. The
> >>>>> standard arm64 defconfig enables the t194 pcie driver as a module. And
> >>>>> my simple busybox ramdisk that I use for mainline regression testing
> >>>>> isn't loading any modules. If I set the pcie driver to built-in, I
> >>>>> replicate the issue. And I don't see the issue on my normal use case,
> >>>>> because I have the dt changes as well.
> >>>>>
> >>>>> So it appears that the pcie driver submits icc bandwidth. And without
> >>>>> cpufreq submitting bandwidth as well, the emc driver gets a very low
> >>>>> number and thus sets a very low emc freq. The question becomes... what
> >>>>> to do about it? If the related dt changes were submitted to
> >>>>> linux-next, everything should fall into place. And I'm not sure where
> >>>>> this falls on the severity scale since it doesn't full out break boot
> >>>>> or prevent operation.
> >>>>
> >>>> Where are the related DT changes? If we can get these into -next and
> >>>> lined up to be merged for v6.19, then that is fine. However, we should
> >>>> not merge this for v6.19 without the DT changes.
> >>>
> >>> The dt changes are here [0].
> >>
> >> To confirm, applying the DT changes do not fix this for me. Thierry is
> >> having a look at this to see if there is a way to fix this.
> >>
> >> BTW, I have also noticed that Thierry's memory frequency test [0] is
> >> also failing on Tegra186. The test simply tries to set the frequency via
> >> the sysfs and this is now failing. I am seeing ...
> >>
> >> memory: emc: - available rates: (* = current)
> >> memory: emc:   -   40800000
> >> memory: emc:   -   68000000
> >> memory: emc:   -  102000000
> >> memory: emc:   -  204000000
> >> memory: emc:   -  408000000
> >> memory: emc:   -  665600000
> >> memory: emc:   -  800000000
> >> memory: emc:   - 1062400000
> >> memory: emc:   - 1331200000
> >> memory: emc:   - 1600000000
> >> memory: emc:   - 1866000000 *
> >> memory: emc: - testing:
> >> memory: emc:   -   40800000...OSError: [Errno 34] Numerical result out
> >> of range
> >
> > Question. Does this test run and pass on jetson-tk1? I based the
> > tegra210 and tegra186 [0] code on tegra124 [1]. And I don't see a
> > difference in the flow now. What appears to be happening is that icc
> > is reporting a high bandwidth, setting the emc min_freq to something
> > like 1600MHz. Then debugfs is having max_freq set to something low
> > like 40.8MHz. Then the linked code block fails because the higher of
> > the min_freqs is greater than the lower of the max_freqs. But if this
> > same test is run on jetson-tk1, I don't see how it passes. Unless
> > maybe the t124 actmon is consistently setting min freqs during the
> > tests.
>
> So we don't currently run this test on Tegra124. We could certainly try.
> I don't recall if there was an issue that prevented us from doing so now.
>
> > An argument could be made that any attempt to set debugfs should win a
> > conflict with icc. That could be done. But if that needs done here,
> > I'd argue that it needs replicated across all other applicable emc
> > drivers too.
>
> The bottom line is that we cannot regress anything that was working before.

Let me try to iterate the potential issues I've seen stated here. If
I'm missing anything, please fill in the blanks.

1) If this change is applied without the related dt change and the
pcie drvier is loaded, the emc clock can become stuck at the lowest
rate. This is caused by the pcie driver providing icc data, but
nothing else is. So the very low requested bandwidth results in the
emc clock being set very low. I'm not sure there is a 'fix' for this,
beyond making sure the dt change is merged to ensure that the cpufreq
driver provides bandwidth info, causing the emc driver to select a
more reasonable emc clock rate. This is a similar situation to what's
currently blocking the tegra210 actmon series. I don't think there is
a way for the drivers to know if icc data is missing/wrong. The
scaling is doing exactly what it's told based on the icc routing given
in the dt.

2) Jon, you report that even with both this change and the related dt
change, that the issue is still not fixed. But then posted a log
showing that the emc rate is set to max. If the issue is that emc rate
is too low, then how can debugfs report that the rate is max? For
reference, everything scales as expected for me given this change plus
the dt change on both p2771 and p3636+p3509.

3) If icc is requesting enough bandwidth to set the emc clock to a
high value, then a user tries to set debugfs max_freq to a lower
value, this code will reject the change. I do not believe this is an
issue unique to this code. tegra20-emc, tegra30-emc, and tegra124-emc
all have this same flow. And so does my proposed change to
tegra210-emc-core in the actmon series. This is why I asked if
tegra124 ran this test, to see if the failure was unique. If this is
not a unique failure, then I'd argue that all instances need changed,
not just this one causing diverging results depending on the soc being
utilized. A lot of the work I'm doing is to try to bring unity and
feature parity to all the tegra socs I'm working on. I don't want to
cause even more divergence.

What actions need taken for which issue?

Aaron

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 10/12/2025 05:06, Aaron Kling wrote:

...

> Let me try to iterate the potential issues I've seen stated here. If
> I'm missing anything, please fill in the blanks.
> 
> 1) If this change is applied without the related dt change and the
> pcie drvier is loaded, the emc clock can become stuck at the lowest
> rate. This is caused by the pcie driver providing icc data, but
> nothing else is. So the very low requested bandwidth results in the
> emc clock being set very low. I'm not sure there is a 'fix' for this,
> beyond making sure the dt change is merged to ensure that the cpufreq
> driver provides bandwidth info, causing the emc driver to select a
> more reasonable emc clock rate. This is a similar situation to what's
> currently blocking the tegra210 actmon series. I don't think there is
> a way for the drivers to know if icc data is missing/wrong. The
> scaling is doing exactly what it's told based on the icc routing given
> in the dt.

So this is the fundamental issue with this that must be fixed. We can't 
allow the PCIe driver to slow the system down. I think that Krzysztof 
suggested we need some way to determine if the necessary ICC clients are 
present/registered for ICC to work. Admittedly, I have no idea if there 
is a simple way to do this, but we need something like that.

> 2) Jon, you report that even with both this change and the related dt
> change, that the issue is still not fixed. But then posted a log
> showing that the emc rate is set to max. If the issue is that emc rate
> is too low, then how can debugfs report that the rate is max? For
> reference, everything scales as expected for me given this change plus
> the dt change on both p2771 and p3636+p3509.

To clarify, this broke the boot test on Tegra194 because the boot was 
too slow. However, this also broke the EMC test on Tegra186 because 
setting the frequency from the debugfs failed. So two different failures 
on two different devices. I am guessing the EMC test would also fail on 
Tegra194, but given that it does not boot, we did not get that far.

> 3) If icc is requesting enough bandwidth to set the emc clock to a
> high value, then a user tries to set debugfs max_freq to a lower
> value, this code will reject the change. I do not believe this is an
> issue unique to this code. tegra20-emc, tegra30-emc, and tegra124-emc
> all have this same flow. And so does my proposed change to
> tegra210-emc-core in the actmon series. This is why I asked if
> tegra124 ran this test, to see if the failure was unique. If this is
> not a unique failure, then I'd argue that all instances need changed,
> not just this one causing diverging results depending on the soc being
> utilized. A lot of the work I'm doing is to try to bring unity and
> feature parity to all the tegra socs I'm working on. I don't want to
> cause even more divergence.

Yes that is fair point, however, we need to detect this in the 
tegra-tests so that we know that this will not work. It would be nice if 
we could disable ICC from userspace and then run the test.

Bottom line here is that #1 is the problem that needs to be fixed.

Jon

-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Aaron Kling]

On Wed, Dec 10, 2025 at 9:04 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 10/12/2025 05:06, Aaron Kling wrote:
>
> ...
>
> > Let me try to iterate the potential issues I've seen stated here. If
> > I'm missing anything, please fill in the blanks.
> >
> > 1) If this change is applied without the related dt change and the
> > pcie drvier is loaded, the emc clock can become stuck at the lowest
> > rate. This is caused by the pcie driver providing icc data, but
> > nothing else is. So the very low requested bandwidth results in the
> > emc clock being set very low. I'm not sure there is a 'fix' for this,
> > beyond making sure the dt change is merged to ensure that the cpufreq
> > driver provides bandwidth info, causing the emc driver to select a
> > more reasonable emc clock rate. This is a similar situation to what's
> > currently blocking the tegra210 actmon series. I don't think there is
> > a way for the drivers to know if icc data is missing/wrong. The
> > scaling is doing exactly what it's told based on the icc routing given
> > in the dt.
>
> So this is the fundamental issue with this that must be fixed. We can't
> allow the PCIe driver to slow the system down. I think that Krzysztof
> suggested we need some way to determine if the necessary ICC clients are
> present/registered for ICC to work. Admittedly, I have no idea if there
> is a simple way to do this, but we need something like that.

I'm not sure I understand how checking clients would work. Is there a
mechanism for the emc driver to know if cpufreq is registered to icc
in a way that works with probe deferrals, but also allows for it to be
optional?

Alternatively if there is not, can we just accept the abi break and
have this and the dt change depend on each other? I know it's not
desirable or the first choice, but if the other option is to rewrite
part of the icc system, then perhaps it should be an option.

> > 2) Jon, you report that even with both this change and the related dt
> > change, that the issue is still not fixed. But then posted a log
> > showing that the emc rate is set to max. If the issue is that emc rate
> > is too low, then how can debugfs report that the rate is max? For
> > reference, everything scales as expected for me given this change plus
> > the dt change on both p2771 and p3636+p3509.
>
> To clarify, this broke the boot test on Tegra194 because the boot was
> too slow. However, this also broke the EMC test on Tegra186 because
> setting the frequency from the debugfs failed. So two different failures
> on two different devices. I am guessing the EMC test would also fail on
> Tegra194, but given that it does not boot, we did not get that far.

So you're saying that even with the dt changes, this change on
tegra194 still does not boot before the regression test framework
times out? If so, I need some more details about this. I have not seen
issues on p2972 or p3518. For example, if I boot to android recovery
where I set the cpufreq governor to performance, I see emc clock rate
set to 2133 MHz and 1600 MHz respectively. And boot time from kernel
start to pixels on display is 15 seconds, give or take a couple
seconds. This is using the boot stack from l4t r32.7.6.

> > 3) If icc is requesting enough bandwidth to set the emc clock to a
> > high value, then a user tries to set debugfs max_freq to a lower
> > value, this code will reject the change. I do not believe this is an
> > issue unique to this code. tegra20-emc, tegra30-emc, and tegra124-emc
> > all have this same flow. And so does my proposed change to
> > tegra210-emc-core in the actmon series. This is why I asked if
> > tegra124 ran this test, to see if the failure was unique. If this is
> > not a unique failure, then I'd argue that all instances need changed,
> > not just this one causing diverging results depending on the soc being
> > utilized. A lot of the work I'm doing is to try to bring unity and
> > feature parity to all the tegra socs I'm working on. I don't want to
> > cause even more divergence.
>
> Yes that is fair point, however, we need to detect this in the
> tegra-tests so that we know that this will not work. It would be nice if
> we could disable ICC from userspace and then run the test.

I am unaware of a way to disable icc from userspace. That would be
useful to me as well. And for the record, I'm not refusing to make
such a change. I would just want to have a series to change all the
others uploaded and merged concurrently. But I cannot test t20 or t30.
Only t124+.

> Bottom line here is that #1 is the problem that needs to be fixed.

Aaron

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 10/12/2025 18:32, Aaron Kling wrote:
> On Wed, Dec 10, 2025 at 9:04 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>>
>> On 10/12/2025 05:06, Aaron Kling wrote:
>>
>> ...
>>
>>> Let me try to iterate the potential issues I've seen stated here. If
>>> I'm missing anything, please fill in the blanks.
>>>
>>> 1) If this change is applied without the related dt change and the
>>> pcie drvier is loaded, the emc clock can become stuck at the lowest
>>> rate. This is caused by the pcie driver providing icc data, but
>>> nothing else is. So the very low requested bandwidth results in the
>>> emc clock being set very low. I'm not sure there is a 'fix' for this,
>>> beyond making sure the dt change is merged to ensure that the cpufreq
>>> driver provides bandwidth info, causing the emc driver to select a
>>> more reasonable emc clock rate. This is a similar situation to what's
>>> currently blocking the tegra210 actmon series. I don't think there is
>>> a way for the drivers to know if icc data is missing/wrong. The
>>> scaling is doing exactly what it's told based on the icc routing given
>>> in the dt.
>>
>> So this is the fundamental issue with this that must be fixed. We can't
>> allow the PCIe driver to slow the system down. I think that Krzysztof
>> suggested we need some way to determine if the necessary ICC clients are
>> present/registered for ICC to work. Admittedly, I have no idea if there
>> is a simple way to do this, but we need something like that.
> 
> I'm not sure I understand how checking clients would work. Is there a
> mechanism for the emc driver to know if cpufreq is registered to icc
> in a way that works with probe deferrals, but also allows for it to be
> optional?

I am not sure if such a mechanism exists either, but it seems that we 
need something like this.

> Alternatively if there is not, can we just accept the abi break and
> have this and the dt change depend on each other? I know it's not
> desirable or the first choice, but if the other option is to rewrite
> part of the icc system, then perhaps it should be an option.

I am not sure it is an ABI break, but the default performance might be 
worse. I am not sure if you are proposing a way to enforce the 
dependency or just saying that there is a dependency. We can't do the 
latter, but if there is a way for the kernel to check the dependency and 
make the right choice, then that should work.

>>> 2) Jon, you report that even with both this change and the related dt
>>> change, that the issue is still not fixed. But then posted a log
>>> showing that the emc rate is set to max. If the issue is that emc rate
>>> is too low, then how can debugfs report that the rate is max? For
>>> reference, everything scales as expected for me given this change plus
>>> the dt change on both p2771 and p3636+p3509.
>>
>> To clarify, this broke the boot test on Tegra194 because the boot was
>> too slow. However, this also broke the EMC test on Tegra186 because
>> setting the frequency from the debugfs failed. So two different failures
>> on two different devices. I am guessing the EMC test would also fail on
>> Tegra194, but given that it does not boot, we did not get that far.
> 
> So you're saying that even with the dt changes, this change on
> tegra194 still does not boot before the regression test framework
> times out? If so, I need some more details about this. I have not seen
> issues on p2972 or p3518. For example, if I boot to android recovery
> where I set the cpufreq governor to performance, I see emc clock rate
> set to 2133 MHz and 1600 MHz respectively. And boot time from kernel
> start to pixels on display is 15 seconds, give or take a couple
> seconds. This is using the boot stack from l4t r32.7.6.

Yes. The boot failure here is not a hard boot failure, but the device 
takes too long to boot and the boot test times out. And no we will not 
increase the timeout as it is there for a reason. It could well be 
because the default governor is not set to performance. If you boot with 
just using the stock 'defconfig' for ARM64 without setting the governor 
does it take longer?

Jon

-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Aaron Kling]

On Wed, Dec 10, 2025 at 3:24 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 10/12/2025 18:32, Aaron Kling wrote:
> > On Wed, Dec 10, 2025 at 9:04 AM Jon Hunter <jonathanh@nvidia.com> wrote:
> >>
> >>
> >> On 10/12/2025 05:06, Aaron Kling wrote:
> >>
> >> ...
> >>
> >>> Let me try to iterate the potential issues I've seen stated here. If
> >>> I'm missing anything, please fill in the blanks.
> >>>
> >>> 1) If this change is applied without the related dt change and the
> >>> pcie drvier is loaded, the emc clock can become stuck at the lowest
> >>> rate. This is caused by the pcie driver providing icc data, but
> >>> nothing else is. So the very low requested bandwidth results in the
> >>> emc clock being set very low. I'm not sure there is a 'fix' for this,
> >>> beyond making sure the dt change is merged to ensure that the cpufreq
> >>> driver provides bandwidth info, causing the emc driver to select a
> >>> more reasonable emc clock rate. This is a similar situation to what's
> >>> currently blocking the tegra210 actmon series. I don't think there is
> >>> a way for the drivers to know if icc data is missing/wrong. The
> >>> scaling is doing exactly what it's told based on the icc routing given
> >>> in the dt.
> >>
> >> So this is the fundamental issue with this that must be fixed. We can't
> >> allow the PCIe driver to slow the system down. I think that Krzysztof
> >> suggested we need some way to determine if the necessary ICC clients are
> >> present/registered for ICC to work. Admittedly, I have no idea if there
> >> is a simple way to do this, but we need something like that.
> >
> > I'm not sure I understand how checking clients would work. Is there a
> > mechanism for the emc driver to know if cpufreq is registered to icc
> > in a way that works with probe deferrals, but also allows for it to be
> > optional?
>
> I am not sure if such a mechanism exists either, but it seems that we
> need something like this.
>
> > Alternatively if there is not, can we just accept the abi break and
> > have this and the dt change depend on each other? I know it's not
> > desirable or the first choice, but if the other option is to rewrite
> > part of the icc system, then perhaps it should be an option.
>
> I am not sure it is an ABI break, but the default performance might be
> worse. I am not sure if you are proposing a way to enforce the
> dependency or just saying that there is a dependency. We can't do the
> latter, but if there is a way for the kernel to check the dependency and
> make the right choice, then that should work.

So we can't accept that older dt's will run slower on a newer kernel
and say that a newer dt is needed for full performance?

If that's not an option, then I have no idea how to resolve this. I'm
not greatly knowledgeable about the icc subsystem. I can try to look
into options, but I'm not greatly optimistic about me finding one. If
someone could suggest a concept on how to make it work, I could
implement it. But I'm not even seeing the concept right now.

> >>> 2) Jon, you report that even with both this change and the related dt
> >>> change, that the issue is still not fixed. But then posted a log
> >>> showing that the emc rate is set to max. If the issue is that emc rate
> >>> is too low, then how can debugfs report that the rate is max? For
> >>> reference, everything scales as expected for me given this change plus
> >>> the dt change on both p2771 and p3636+p3509.
> >>
> >> To clarify, this broke the boot test on Tegra194 because the boot was
> >> too slow. However, this also broke the EMC test on Tegra186 because
> >> setting the frequency from the debugfs failed. So two different failures
> >> on two different devices. I am guessing the EMC test would also fail on
> >> Tegra194, but given that it does not boot, we did not get that far.
> >
> > So you're saying that even with the dt changes, this change on
> > tegra194 still does not boot before the regression test framework
> > times out? If so, I need some more details about this. I have not seen
> > issues on p2972 or p3518. For example, if I boot to android recovery
> > where I set the cpufreq governor to performance, I see emc clock rate
> > set to 2133 MHz and 1600 MHz respectively. And boot time from kernel
> > start to pixels on display is 15 seconds, give or take a couple
> > seconds. This is using the boot stack from l4t r32.7.6.
>
> Yes. The boot failure here is not a hard boot failure, but the device
> takes too long to boot and the boot test times out. And no we will not
> increase the timeout as it is there for a reason. It could well be
> because the default governor is not set to performance. If you boot with
> just using the stock 'defconfig' for ARM64 without setting the governor
> does it take longer?

So, I checked out next-20251210, then b4 shazam'ed this series and the
matching dt series,
20251021-tegra186-icc-p3-v3-0-68184ee8a89c@gmail.com. Then built with
LLVM=1 ARCH=arm64 make defconfig
LLVM=1 ARCH=arm64 make -j33 Image nvidia/tegra194-p2972-0000.dtb

I packaged them into an android boot image using a lightly modified
copy of Gnurou's bbinitramfs which just drops to a busybox shell. Note
that this includes no modules, and since the pcie driver is =m in
defconfig, it is not included. Then I flashed that with the l4t
r32.7.6 boot stack to p2972. I got the shell on uart after 4.275
seconds in the kernel. Per sysfs, the cpufreq governor is schedutil
and all policies are idling at min freq, 115200. And per debugfs, the
emc clock is 800000000. All this looks to be as expected.

I have no idea why the regression test setup is timing out. I have not
seen the issue through any of my testing. On pure mainline as per the
above paragraph, or with the patches on the android common kernel, as
per my target use case. I don't know what to do if I can't replicate
the issue. I don't suppose the flash package for the regression test
setup is something that could be released?

Aaron

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 10/12/2025 22:41, Aaron Kling wrote:
> On Wed, Dec 10, 2025 at 3:24 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>>
>> On 10/12/2025 18:32, Aaron Kling wrote:
>>> On Wed, Dec 10, 2025 at 9:04 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>>>>
>>>>
>>>> On 10/12/2025 05:06, Aaron Kling wrote:
>>>>
>>>> ...
>>>>
>>>>> Let me try to iterate the potential issues I've seen stated here. If
>>>>> I'm missing anything, please fill in the blanks.
>>>>>
>>>>> 1) If this change is applied without the related dt change and the
>>>>> pcie drvier is loaded, the emc clock can become stuck at the lowest
>>>>> rate. This is caused by the pcie driver providing icc data, but
>>>>> nothing else is. So the very low requested bandwidth results in the
>>>>> emc clock being set very low. I'm not sure there is a 'fix' for this,
>>>>> beyond making sure the dt change is merged to ensure that the cpufreq
>>>>> driver provides bandwidth info, causing the emc driver to select a
>>>>> more reasonable emc clock rate. This is a similar situation to what's
>>>>> currently blocking the tegra210 actmon series. I don't think there is
>>>>> a way for the drivers to know if icc data is missing/wrong. The
>>>>> scaling is doing exactly what it's told based on the icc routing given
>>>>> in the dt.
>>>>
>>>> So this is the fundamental issue with this that must be fixed. We can't
>>>> allow the PCIe driver to slow the system down. I think that Krzysztof
>>>> suggested we need some way to determine if the necessary ICC clients are
>>>> present/registered for ICC to work. Admittedly, I have no idea if there
>>>> is a simple way to do this, but we need something like that.
>>>
>>> I'm not sure I understand how checking clients would work. Is there a
>>> mechanism for the emc driver to know if cpufreq is registered to icc
>>> in a way that works with probe deferrals, but also allows for it to be
>>> optional?
>>
>> I am not sure if such a mechanism exists either, but it seems that we
>> need something like this.
>>
>>> Alternatively if there is not, can we just accept the abi break and
>>> have this and the dt change depend on each other? I know it's not
>>> desirable or the first choice, but if the other option is to rewrite
>>> part of the icc system, then perhaps it should be an option.
>>
>> I am not sure it is an ABI break, but the default performance might be
>> worse. I am not sure if you are proposing a way to enforce the
>> dependency or just saying that there is a dependency. We can't do the
>> latter, but if there is a way for the kernel to check the dependency and
>> make the right choice, then that should work.
> 
> So we can't accept that older dt's will run slower on a newer kernel
> and say that a newer dt is needed for full performance?
> 
> If that's not an option, then I have no idea how to resolve this. I'm
> not greatly knowledgeable about the icc subsystem. I can try to look
> into options, but I'm not greatly optimistic about me finding one. If
> someone could suggest a concept on how to make it work, I could
> implement it. But I'm not even seeing the concept right now.
> 
>>>>> 2) Jon, you report that even with both this change and the related dt
>>>>> change, that the issue is still not fixed. But then posted a log
>>>>> showing that the emc rate is set to max. If the issue is that emc rate
>>>>> is too low, then how can debugfs report that the rate is max? For
>>>>> reference, everything scales as expected for me given this change plus
>>>>> the dt change on both p2771 and p3636+p3509.
>>>>
>>>> To clarify, this broke the boot test on Tegra194 because the boot was
>>>> too slow. However, this also broke the EMC test on Tegra186 because
>>>> setting the frequency from the debugfs failed. So two different failures
>>>> on two different devices. I am guessing the EMC test would also fail on
>>>> Tegra194, but given that it does not boot, we did not get that far.
>>>
>>> So you're saying that even with the dt changes, this change on
>>> tegra194 still does not boot before the regression test framework
>>> times out? If so, I need some more details about this. I have not seen
>>> issues on p2972 or p3518. For example, if I boot to android recovery
>>> where I set the cpufreq governor to performance, I see emc clock rate
>>> set to 2133 MHz and 1600 MHz respectively. And boot time from kernel
>>> start to pixels on display is 15 seconds, give or take a couple
>>> seconds. This is using the boot stack from l4t r32.7.6.
>>
>> Yes. The boot failure here is not a hard boot failure, but the device
>> takes too long to boot and the boot test times out. And no we will not
>> increase the timeout as it is there for a reason. It could well be
>> because the default governor is not set to performance. If you boot with
>> just using the stock 'defconfig' for ARM64 without setting the governor
>> does it take longer?
> 
> So, I checked out next-20251210, then b4 shazam'ed this series and the
> matching dt series,
> 20251021-tegra186-icc-p3-v3-0-68184ee8a89c@gmail.com. Then built with
> LLVM=1 ARCH=arm64 make defconfig
> LLVM=1 ARCH=arm64 make -j33 Image nvidia/tegra194-p2972-0000.dtb
> 
> I packaged them into an android boot image using a lightly modified
> copy of Gnurou's bbinitramfs which just drops to a busybox shell. Note
> that this includes no modules, and since the pcie driver is =m in
> defconfig, it is not included. Then I flashed that with the l4t
> r32.7.6 boot stack to p2972. I got the shell on uart after 4.275
> seconds in the kernel. Per sysfs, the cpufreq governor is schedutil
> and all policies are idling at min freq, 115200. And per debugfs, the
> emc clock is 800000000. All this looks to be as expected.
> 
> I have no idea why the regression test setup is timing out. I have not
> seen the issue through any of my testing. On pure mainline as per the
> above paragraph, or with the patches on the android common kernel, as
> per my target use case. I don't know what to do if I can't replicate
> the issue. I don't suppose the flash package for the regression test
> setup is something that could be released?

I thought we already concluded that you did not see this because you did 
not have the PCIe module present in your testing? From the above its 
sounds like you still don't have that driver present and so you don't 
see the issue. I guess I am not surprised by that but I am not sure why 
you are now saying you have no idea why this is timing out? I thought 
this was understood.

-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Aaron Kling]

On Thu, Dec 11, 2025 at 1:47 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>
>
> On 10/12/2025 22:41, Aaron Kling wrote:
> > On Wed, Dec 10, 2025 at 3:24 PM Jon Hunter <jonathanh@nvidia.com> wrote:
> >>
> >>
> >> On 10/12/2025 18:32, Aaron Kling wrote:
> >>> On Wed, Dec 10, 2025 at 9:04 AM Jon Hunter <jonathanh@nvidia.com> wrote:
> >>>>
> >>>>
> >>>> On 10/12/2025 05:06, Aaron Kling wrote:
> >>>>
> >>>> ...
> >>>>
> >>>>> Let me try to iterate the potential issues I've seen stated here. If
> >>>>> I'm missing anything, please fill in the blanks.
> >>>>>
> >>>>> 1) If this change is applied without the related dt change and the
> >>>>> pcie drvier is loaded, the emc clock can become stuck at the lowest
> >>>>> rate. This is caused by the pcie driver providing icc data, but
> >>>>> nothing else is. So the very low requested bandwidth results in the
> >>>>> emc clock being set very low. I'm not sure there is a 'fix' for this,
> >>>>> beyond making sure the dt change is merged to ensure that the cpufreq
> >>>>> driver provides bandwidth info, causing the emc driver to select a
> >>>>> more reasonable emc clock rate. This is a similar situation to what's
> >>>>> currently blocking the tegra210 actmon series. I don't think there is
> >>>>> a way for the drivers to know if icc data is missing/wrong. The
> >>>>> scaling is doing exactly what it's told based on the icc routing given
> >>>>> in the dt.
> >>>>
> >>>> So this is the fundamental issue with this that must be fixed. We can't
> >>>> allow the PCIe driver to slow the system down. I think that Krzysztof
> >>>> suggested we need some way to determine if the necessary ICC clients are
> >>>> present/registered for ICC to work. Admittedly, I have no idea if there
> >>>> is a simple way to do this, but we need something like that.
> >>>
> >>> I'm not sure I understand how checking clients would work. Is there a
> >>> mechanism for the emc driver to know if cpufreq is registered to icc
> >>> in a way that works with probe deferrals, but also allows for it to be
> >>> optional?
> >>
> >> I am not sure if such a mechanism exists either, but it seems that we
> >> need something like this.
> >>
> >>> Alternatively if there is not, can we just accept the abi break and
> >>> have this and the dt change depend on each other? I know it's not
> >>> desirable or the first choice, but if the other option is to rewrite
> >>> part of the icc system, then perhaps it should be an option.
> >>
> >> I am not sure it is an ABI break, but the default performance might be
> >> worse. I am not sure if you are proposing a way to enforce the
> >> dependency or just saying that there is a dependency. We can't do the
> >> latter, but if there is a way for the kernel to check the dependency and
> >> make the right choice, then that should work.
> >
> > So we can't accept that older dt's will run slower on a newer kernel
> > and say that a newer dt is needed for full performance?
> >
> > If that's not an option, then I have no idea how to resolve this. I'm
> > not greatly knowledgeable about the icc subsystem. I can try to look
> > into options, but I'm not greatly optimistic about me finding one. If
> > someone could suggest a concept on how to make it work, I could
> > implement it. But I'm not even seeing the concept right now.
> >
> >>>>> 2) Jon, you report that even with both this change and the related dt
> >>>>> change, that the issue is still not fixed. But then posted a log
> >>>>> showing that the emc rate is set to max. If the issue is that emc rate
> >>>>> is too low, then how can debugfs report that the rate is max? For
> >>>>> reference, everything scales as expected for me given this change plus
> >>>>> the dt change on both p2771 and p3636+p3509.
> >>>>
> >>>> To clarify, this broke the boot test on Tegra194 because the boot was
> >>>> too slow. However, this also broke the EMC test on Tegra186 because
> >>>> setting the frequency from the debugfs failed. So two different failures
> >>>> on two different devices. I am guessing the EMC test would also fail on
> >>>> Tegra194, but given that it does not boot, we did not get that far.
> >>>
> >>> So you're saying that even with the dt changes, this change on
> >>> tegra194 still does not boot before the regression test framework
> >>> times out? If so, I need some more details about this. I have not seen
> >>> issues on p2972 or p3518. For example, if I boot to android recovery
> >>> where I set the cpufreq governor to performance, I see emc clock rate
> >>> set to 2133 MHz and 1600 MHz respectively. And boot time from kernel
> >>> start to pixels on display is 15 seconds, give or take a couple
> >>> seconds. This is using the boot stack from l4t r32.7.6.
> >>
> >> Yes. The boot failure here is not a hard boot failure, but the device
> >> takes too long to boot and the boot test times out. And no we will not
> >> increase the timeout as it is there for a reason. It could well be
> >> because the default governor is not set to performance. If you boot with
> >> just using the stock 'defconfig' for ARM64 without setting the governor
> >> does it take longer?
> >
> > So, I checked out next-20251210, then b4 shazam'ed this series and the
> > matching dt series,
> > 20251021-tegra186-icc-p3-v3-0-68184ee8a89c@gmail.com. Then built with
> > LLVM=1 ARCH=arm64 make defconfig
> > LLVM=1 ARCH=arm64 make -j33 Image nvidia/tegra194-p2972-0000.dtb
> >
> > I packaged them into an android boot image using a lightly modified
> > copy of Gnurou's bbinitramfs which just drops to a busybox shell. Note
> > that this includes no modules, and since the pcie driver is =m in
> > defconfig, it is not included. Then I flashed that with the l4t
> > r32.7.6 boot stack to p2972. I got the shell on uart after 4.275
> > seconds in the kernel. Per sysfs, the cpufreq governor is schedutil
> > and all policies are idling at min freq, 115200. And per debugfs, the
> > emc clock is 800000000. All this looks to be as expected.
> >
> > I have no idea why the regression test setup is timing out. I have not
> > seen the issue through any of my testing. On pure mainline as per the
> > above paragraph, or with the patches on the android common kernel, as
> > per my target use case. I don't know what to do if I can't replicate
> > the issue. I don't suppose the flash package for the regression test
> > setup is something that could be released?
>
> I thought we already concluded that you did not see this because you did
> not have the PCIe module present in your testing? From the above its
> sounds like you still don't have that driver present and so you don't
> see the issue. I guess I am not surprised by that but I am not sure why
> you are now saying you have no idea why this is timing out? I thought
> this was understood.

Oh, come on... The issue is a combination of old dt AND the pcie
driver. I can reproduce low emc clock with that. But then you said
t194 on the regression bench was still timing out even with the new
dt. And that's what I cannot reproduce. And then you asked me to test
with pure mainline and a stock/unmodified defconfig. So I did, using
-next and the two open series, but clarified what an unmodified
defconfig meant.

So, I modified the .config to enable the pcie driver as built-in, then
reflashed. Otherwise the same as my previous post. I got the shell
after 11 seconds. And clocks are still as reported before, cpu at min,
emc at 800000000.

Aaron

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Krzysztof Kozlowski]

On 21/11/2025 12:21, Jon Hunter wrote:
> 
> On 12/11/2025 07:21, Aaron Kling wrote:
>> On Wed, Nov 12, 2025 at 12:18 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>>>
>>>
>>> On 11/11/2025 23:17, Aaron Kling wrote:
>>>
>>> ...
>>>
>>>> Alright, I think I've got the picture of what's going on now. The
>>>> standard arm64 defconfig enables the t194 pcie driver as a module. And
>>>> my simple busybox ramdisk that I use for mainline regression testing
>>>> isn't loading any modules. If I set the pcie driver to built-in, I
>>>> replicate the issue. And I don't see the issue on my normal use case,
>>>> because I have the dt changes as well.
>>>>
>>>> So it appears that the pcie driver submits icc bandwidth. And without
>>>> cpufreq submitting bandwidth as well, the emc driver gets a very low
>>>> number and thus sets a very low emc freq. The question becomes... what
>>>> to do about it? If the related dt changes were submitted to
>>>> linux-next, everything should fall into place. And I'm not sure where
>>>> this falls on the severity scale since it doesn't full out break boot
>>>> or prevent operation.
>>>
>>> Where are the related DT changes? If we can get these into -next and
>>> lined up to be merged for v6.19, then that is fine. However, we should
>>> not merge this for v6.19 without the DT changes.
>>
>> The dt changes are here [0].
> 
> To confirm, applying the DT changes do not fix this for me. Thierry is 
> having a look at this to see if there is a way to fix this.
> 
> BTW, I have also noticed that Thierry's memory frequency test [0] is 
> also failing on Tegra186. The test simply tries to set the frequency via 
> the sysfs and this is now failing. I am seeing ..


The pull request was not yet merged, so I can amend it. The issue was
reported 12 days ago, so if this cannot be fixed in for such time, then
it is not yet ready and I will drop the changes.

Best regards,
Krzysztof

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Aaron Kling]

On Sat, Nov 22, 2025 at 6:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 21/11/2025 12:21, Jon Hunter wrote:
> >
> > On 12/11/2025 07:21, Aaron Kling wrote:
> >> On Wed, Nov 12, 2025 at 12:18 AM Jon Hunter <jonathanh@nvidia.com> wrote:
> >>>
> >>>
> >>> On 11/11/2025 23:17, Aaron Kling wrote:
> >>>
> >>> ...
> >>>
> >>>> Alright, I think I've got the picture of what's going on now. The
> >>>> standard arm64 defconfig enables the t194 pcie driver as a module. And
> >>>> my simple busybox ramdisk that I use for mainline regression testing
> >>>> isn't loading any modules. If I set the pcie driver to built-in, I
> >>>> replicate the issue. And I don't see the issue on my normal use case,
> >>>> because I have the dt changes as well.
> >>>>
> >>>> So it appears that the pcie driver submits icc bandwidth. And without
> >>>> cpufreq submitting bandwidth as well, the emc driver gets a very low
> >>>> number and thus sets a very low emc freq. The question becomes... what
> >>>> to do about it? If the related dt changes were submitted to
> >>>> linux-next, everything should fall into place. And I'm not sure where
> >>>> this falls on the severity scale since it doesn't full out break boot
> >>>> or prevent operation.
> >>>
> >>> Where are the related DT changes? If we can get these into -next and
> >>> lined up to be merged for v6.19, then that is fine. However, we should
> >>> not merge this for v6.19 without the DT changes.
> >>
> >> The dt changes are here [0].
> >
> > To confirm, applying the DT changes do not fix this for me. Thierry is
> > having a look at this to see if there is a way to fix this.
> >
> > BTW, I have also noticed that Thierry's memory frequency test [0] is
> > also failing on Tegra186. The test simply tries to set the frequency via
> > the sysfs and this is now failing. I am seeing ..

With this patch dropped from -next, what needs to happen to get it
requeued? I gave an analysis over two weeks ago and have seen no
response since.

Aaron

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Krzysztof Kozlowski]

On 09/12/2025 05:26, Aaron Kling wrote:
> On Sat, Nov 22, 2025 at 6:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 21/11/2025 12:21, Jon Hunter wrote:
>>>
>>> On 12/11/2025 07:21, Aaron Kling wrote:
>>>> On Wed, Nov 12, 2025 at 12:18 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>>>>>
>>>>>
>>>>> On 11/11/2025 23:17, Aaron Kling wrote:
>>>>>
>>>>> ...
>>>>>
>>>>>> Alright, I think I've got the picture of what's going on now. The
>>>>>> standard arm64 defconfig enables the t194 pcie driver as a module. And
>>>>>> my simple busybox ramdisk that I use for mainline regression testing
>>>>>> isn't loading any modules. If I set the pcie driver to built-in, I
>>>>>> replicate the issue. And I don't see the issue on my normal use case,
>>>>>> because I have the dt changes as well.
>>>>>>
>>>>>> So it appears that the pcie driver submits icc bandwidth. And without
>>>>>> cpufreq submitting bandwidth as well, the emc driver gets a very low
>>>>>> number and thus sets a very low emc freq. The question becomes... what
>>>>>> to do about it? If the related dt changes were submitted to
>>>>>> linux-next, everything should fall into place. And I'm not sure where
>>>>>> this falls on the severity scale since it doesn't full out break boot
>>>>>> or prevent operation.
>>>>>
>>>>> Where are the related DT changes? If we can get these into -next and
>>>>> lined up to be merged for v6.19, then that is fine. However, we should
>>>>> not merge this for v6.19 without the DT changes.
>>>>
>>>> The dt changes are here [0].
>>>
>>> To confirm, applying the DT changes do not fix this for me. Thierry is
>>> having a look at this to see if there is a way to fix this.
>>>
>>> BTW, I have also noticed that Thierry's memory frequency test [0] is
>>> also failing on Tegra186. The test simply tries to set the frequency via
>>> the sysfs and this is now failing. I am seeing ..
> 
> With this patch dropped from -next, what needs to happen to get it
> requeued? I gave an analysis over two weeks ago and have seen no
> response since.

Hm, I did not see the root cause identified, so maybe I missed something.

Anyway, I am waiting for the patchset to be retested and resent. And
testing MUST include kernel development process rules, including how
patches are taken - see maintainer soc profile. Any dependencies must be
clearly marked.

Best regards,
Krzysztof

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 09/12/2025 05:53, Krzysztof Kozlowski wrote:
> On 09/12/2025 05:26, Aaron Kling wrote:
>> On Sat, Nov 22, 2025 at 6:01 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>
>>> On 21/11/2025 12:21, Jon Hunter wrote:
>>>>
>>>> On 12/11/2025 07:21, Aaron Kling wrote:
>>>>> On Wed, Nov 12, 2025 at 12:18 AM Jon Hunter <jonathanh@nvidia.com> wrote:
>>>>>>
>>>>>>
>>>>>> On 11/11/2025 23:17, Aaron Kling wrote:
>>>>>>
>>>>>> ...
>>>>>>
>>>>>>> Alright, I think I've got the picture of what's going on now. The
>>>>>>> standard arm64 defconfig enables the t194 pcie driver as a module. And
>>>>>>> my simple busybox ramdisk that I use for mainline regression testing
>>>>>>> isn't loading any modules. If I set the pcie driver to built-in, I
>>>>>>> replicate the issue. And I don't see the issue on my normal use case,
>>>>>>> because I have the dt changes as well.
>>>>>>>
>>>>>>> So it appears that the pcie driver submits icc bandwidth. And without
>>>>>>> cpufreq submitting bandwidth as well, the emc driver gets a very low
>>>>>>> number and thus sets a very low emc freq. The question becomes... what
>>>>>>> to do about it? If the related dt changes were submitted to
>>>>>>> linux-next, everything should fall into place. And I'm not sure where
>>>>>>> this falls on the severity scale since it doesn't full out break boot
>>>>>>> or prevent operation.
>>>>>>
>>>>>> Where are the related DT changes? If we can get these into -next and
>>>>>> lined up to be merged for v6.19, then that is fine. However, we should
>>>>>> not merge this for v6.19 without the DT changes.
>>>>>
>>>>> The dt changes are here [0].
>>>>
>>>> To confirm, applying the DT changes do not fix this for me. Thierry is
>>>> having a look at this to see if there is a way to fix this.
>>>>
>>>> BTW, I have also noticed that Thierry's memory frequency test [0] is
>>>> also failing on Tegra186. The test simply tries to set the frequency via
>>>> the sysfs and this is now failing. I am seeing ..
>>
>> With this patch dropped from -next, what needs to happen to get it
>> requeued? I gave an analysis over two weeks ago and have seen no
>> response since.
> 
> Hm, I did not see the root cause identified, so maybe I missed something.
> 
> Anyway, I am waiting for the patchset to be retested and resent. And
> testing MUST include kernel development process rules, including how
> patches are taken - see maintainer soc profile. Any dependencies must be
> clearly marked.

Yes me too. I am happy to re-test any updates.

Jon

-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Krzysztof Kozlowski]

On 12/11/2025 07:18, Jon Hunter wrote:
> 
> On 11/11/2025 23:17, Aaron Kling wrote:
> 
> ...
> 
>> Alright, I think I've got the picture of what's going on now. The
>> standard arm64 defconfig enables the t194 pcie driver as a module. And
>> my simple busybox ramdisk that I use for mainline regression testing
>> isn't loading any modules. If I set the pcie driver to built-in, I
>> replicate the issue. And I don't see the issue on my normal use case,
>> because I have the dt changes as well.
>>
>> So it appears that the pcie driver submits icc bandwidth. And without
>> cpufreq submitting bandwidth as well, the emc driver gets a very low
>> number and thus sets a very low emc freq. The question becomes... what
>> to do about it? If the related dt changes were submitted to
>> linux-next, everything should fall into place. And I'm not sure where
>> this falls on the severity scale since it doesn't full out break boot
>> or prevent operation.
> 
> Where are the related DT changes? If we can get these into -next and 
> lined up to be merged for v6.19, then that is fine. However, we should 

It's still breaking all the users then.

> not merge this for v6.19 without the DT changes.
> 
> I will also talk with Thierry to see if he has any concerns about users 
> seeing slow performance if they don't have an up-to-date DTB.
> 
> Is there any easy way to detect if the DTB has he necessary properties 
> to enable ICC scaling?
> 
> Jon
> 


Best regards,
Krzysztof

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 12/11/2025 07:26, Krzysztof Kozlowski wrote:
> On 12/11/2025 07:18, Jon Hunter wrote:
>>
>> On 11/11/2025 23:17, Aaron Kling wrote:
>>
>> ...
>>
>>> Alright, I think I've got the picture of what's going on now. The
>>> standard arm64 defconfig enables the t194 pcie driver as a module. And
>>> my simple busybox ramdisk that I use for mainline regression testing
>>> isn't loading any modules. If I set the pcie driver to built-in, I
>>> replicate the issue. And I don't see the issue on my normal use case,
>>> because I have the dt changes as well.
>>>
>>> So it appears that the pcie driver submits icc bandwidth. And without
>>> cpufreq submitting bandwidth as well, the emc driver gets a very low
>>> number and thus sets a very low emc freq. The question becomes... what
>>> to do about it? If the related dt changes were submitted to
>>> linux-next, everything should fall into place. And I'm not sure where
>>> this falls on the severity scale since it doesn't full out break boot
>>> or prevent operation.
>>
>> Where are the related DT changes? If we can get these into -next and
>> lined up to be merged for v6.19, then that is fine. However, we should
> 
> It's still breaking all the users then.

Yes indeed.

Jon

-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Krzysztof Kozlowski]

On 12/11/2025 11:59, Jon Hunter wrote:
> 
> On 12/11/2025 07:26, Krzysztof Kozlowski wrote:
>> On 12/11/2025 07:18, Jon Hunter wrote:
>>>
>>> On 11/11/2025 23:17, Aaron Kling wrote:
>>>
>>> ...
>>>
>>>> Alright, I think I've got the picture of what's going on now. The
>>>> standard arm64 defconfig enables the t194 pcie driver as a module. And
>>>> my simple busybox ramdisk that I use for mainline regression testing
>>>> isn't loading any modules. If I set the pcie driver to built-in, I
>>>> replicate the issue. And I don't see the issue on my normal use case,
>>>> because I have the dt changes as well.
>>>>
>>>> So it appears that the pcie driver submits icc bandwidth. And without
>>>> cpufreq submitting bandwidth as well, the emc driver gets a very low
>>>> number and thus sets a very low emc freq. The question becomes... what
>>>> to do about it? If the related dt changes were submitted to
>>>> linux-next, everything should fall into place. And I'm not sure where
>>>> this falls on the severity scale since it doesn't full out break boot
>>>> or prevent operation.
>>>
>>> Where are the related DT changes? If we can get these into -next and
>>> lined up to be merged for v6.19, then that is fine. However, we should
>>
>> It's still breaking all the users then.
> 
> Yes indeed.


Please test if dropping sync_state from memory controller drivers helps
you. This might be the easiest fix and it is also known solution when
there are no users.

Best regards,
Krzysztof

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Jon Hunter]

On 12/11/2025 11:42, Krzysztof Kozlowski wrote:
> On 12/11/2025 11:59, Jon Hunter wrote:
>>
>> On 12/11/2025 07:26, Krzysztof Kozlowski wrote:
>>> On 12/11/2025 07:18, Jon Hunter wrote:
>>>>
>>>> On 11/11/2025 23:17, Aaron Kling wrote:
>>>>
>>>> ...
>>>>
>>>>> Alright, I think I've got the picture of what's going on now. The
>>>>> standard arm64 defconfig enables the t194 pcie driver as a module. And
>>>>> my simple busybox ramdisk that I use for mainline regression testing
>>>>> isn't loading any modules. If I set the pcie driver to built-in, I
>>>>> replicate the issue. And I don't see the issue on my normal use case,
>>>>> because I have the dt changes as well.
>>>>>
>>>>> So it appears that the pcie driver submits icc bandwidth. And without
>>>>> cpufreq submitting bandwidth as well, the emc driver gets a very low
>>>>> number and thus sets a very low emc freq. The question becomes... what
>>>>> to do about it? If the related dt changes were submitted to
>>>>> linux-next, everything should fall into place. And I'm not sure where
>>>>> this falls on the severity scale since it doesn't full out break boot
>>>>> or prevent operation.
>>>>
>>>> Where are the related DT changes? If we can get these into -next and
>>>> lined up to be merged for v6.19, then that is fine. However, we should
>>>
>>> It's still breaking all the users then.
>>
>> Yes indeed.
> 
> 
> Please test if dropping sync_state from memory controller drivers helps
> you. This might be the easiest fix and it is also known solution when
> there are no users.

I had a quick look, but I believe that sync_state was first added for 
Tegra234 devices. The current issue is with Tegra194, so I am not sure 
we can simply drop it.

Jon

-- 
nvpublic

Re: [PATCH v4 3/5] memory: tegra186-emc: Support non-bpmp icc scaling

[Krzysztof Kozlowski]

On 12/11/2025 00:17, Aaron Kling wrote:
>>
>> The actual rate that is set is 408MHz if I read the rate after
>> it is set ...
>>
>> [   13.912099] tegra186_emc_icc_set_bw-362: rate 408000000
>>
>> This is a simple boot test and so nothing we are doing via
>> debugfs/sysfs to influence this.
> 
> Alright, I think I've got the picture of what's going on now. The
> standard arm64 defconfig enables the t194 pcie driver as a module. And
> my simple busybox ramdisk that I use for mainline regression testing
> isn't loading any modules. If I set the pcie driver to built-in, I
> replicate the issue. And I don't see the issue on my normal use case,
> because I have the dt changes as well.
> 
> So it appears that the pcie driver submits icc bandwidth. And without
> cpufreq submitting bandwidth as well, the emc driver gets a very low
> number and thus sets a very low emc freq. The question becomes... what

If this depends on DT changes then it is obvious ABI break. Nothing in
commit msgs explained ABI impact.

> to do about it? If the related dt changes were submitted to
> linux-next, everything should fall into place. And I'm not sure where
> this falls on the severity scale since it doesn't full out break boot
> or prevent operation.
> 
> Aaron


Best regards,
Krzysztof

[PATCH v4 4/5] memory: tegra186: Support icc scaling

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

Add Interconnect framework support to dynamically set the DRAM
bandwidth from different clients. The MC driver is added as an ICC
provider and the EMC driver is already a provider.

Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
 drivers/memory/tegra/tegra186.c | 48 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/memory/tegra/tegra186.c b/drivers/memory/tegra/tegra186.c
index aee11457bf8e032637d1772affb87da0cac68494..1384164f624af5d4aaccedc84443d203ba3db2c6 100644
--- a/drivers/memory/tegra/tegra186.c
+++ b/drivers/memory/tegra/tegra186.c
@@ -899,9 +899,56 @@ static const struct tegra_mc_client tegra186_mc_clients[] = {
 				.security = 0x51c,
 			},
 		},
+	}, {
+		.id = TEGRA_ICC_MC_CPU_CLUSTER0,
+		.name = "sw_cluster0",
+		.type = TEGRA_ICC_NISO,
+	}, {
+		.id = TEGRA_ICC_MC_CPU_CLUSTER1,
+		.name = "sw_cluster1",
+		.type = TEGRA_ICC_NISO,
 	},
 };
 
+static int tegra186_mc_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+	/* TODO: program PTSA */
+	return 0;
+}
+
+static int tegra186_mc_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
+				     u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
+{
+	struct icc_provider *p = node->provider;
+	struct tegra_mc *mc = icc_provider_to_tegra_mc(p);
+
+	if (node->id == TEGRA_ICC_MC_CPU_CLUSTER0 ||
+	    node->id == TEGRA_ICC_MC_CPU_CLUSTER1) {
+		if (mc)
+			peak_bw = peak_bw * mc->num_channels;
+	}
+
+	*agg_avg += avg_bw;
+	*agg_peak = max(*agg_peak, peak_bw);
+
+	return 0;
+}
+
+static int tegra186_mc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peak)
+{
+	*avg = 0;
+	*peak = 0;
+
+	return 0;
+}
+
+static const struct tegra_mc_icc_ops tegra186_mc_icc_ops = {
+	.xlate = tegra_mc_icc_xlate,
+	.aggregate = tegra186_mc_icc_aggregate,
+	.get_bw = tegra186_mc_icc_get_init_bw,
+	.set = tegra186_mc_icc_set,
+};
+
 const struct tegra_mc_soc tegra186_mc_soc = {
 	.num_clients = ARRAY_SIZE(tegra186_mc_clients),
 	.clients = tegra186_mc_clients,
@@ -912,6 +959,7 @@ const struct tegra_mc_soc tegra186_mc_soc = {
 		   MC_INT_SECERR_SEC | MC_INT_DECERR_VPR |
 		   MC_INT_SECURITY_VIOLATION | MC_INT_DECERR_EMEM,
 	.ops = &tegra186_mc_ops,
+	.icc_ops = &tegra186_mc_icc_ops,
 	.ch_intmask = 0x0000000f,
 	.global_intstatus_channel_shift = 0,
 };

-- 
2.51.0

[PATCH v4 5/5] memory: tegra194: Support icc scaling

[Aaron Kling via B4 Relay]

From: Aaron Kling <webgeek1234@gmail.com>

Add Interconnect framework support to dynamically set the DRAM
bandwidth from different clients. The MC driver is added as an ICC
provider and the EMC driver is already a provider.

Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
 drivers/memory/tegra/tegra194.c | 59 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/drivers/memory/tegra/tegra194.c b/drivers/memory/tegra/tegra194.c
index 26035ac3a1eb51a3d8ce3830427b4412b48baf3c..e478587586e7f01afd41ff74d26a9a3f1d881347 100644
--- a/drivers/memory/tegra/tegra194.c
+++ b/drivers/memory/tegra/tegra194.c
@@ -1340,9 +1340,66 @@ static const struct tegra_mc_client tegra194_mc_clients[] = {
 				.security = 0x7fc,
 			},
 		},
+	}, {
+		.id = TEGRA_ICC_MC_CPU_CLUSTER0,
+		.name = "sw_cluster0",
+		.type = TEGRA_ICC_NISO,
+	}, {
+		.id = TEGRA_ICC_MC_CPU_CLUSTER1,
+		.name = "sw_cluster1",
+		.type = TEGRA_ICC_NISO,
+	}, {
+		.id = TEGRA_ICC_MC_CPU_CLUSTER2,
+		.name = "sw_cluster2",
+		.type = TEGRA_ICC_NISO,
+	}, {
+		.id = TEGRA_ICC_MC_CPU_CLUSTER3,
+		.name = "sw_cluster3",
+		.type = TEGRA_ICC_NISO,
 	},
 };
 
+static int tegra194_mc_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+	/* TODO: program PTSA */
+	return 0;
+}
+
+static int tegra194_mc_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
+				     u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
+{
+	struct icc_provider *p = node->provider;
+	struct tegra_mc *mc = icc_provider_to_tegra_mc(p);
+
+	if (node->id == TEGRA_ICC_MC_CPU_CLUSTER0 ||
+	    node->id == TEGRA_ICC_MC_CPU_CLUSTER1 ||
+	    node->id == TEGRA_ICC_MC_CPU_CLUSTER2 ||
+	    node->id == TEGRA_ICC_MC_CPU_CLUSTER3) {
+		if (mc)
+			peak_bw = peak_bw * mc->num_channels;
+	}
+
+	*agg_avg += avg_bw;
+	*agg_peak = max(*agg_peak, peak_bw);
+
+	return 0;
+}
+
+static int tegra194_mc_icc_get_init_bw(struct icc_node *node, u32 *avg, u32 *peak)
+{
+	*avg = 0;
+	*peak = 0;
+
+	return 0;
+}
+
+static const struct tegra_mc_icc_ops tegra194_mc_icc_ops = {
+	.xlate = tegra_mc_icc_xlate,
+	.aggregate = tegra194_mc_icc_aggregate,
+	.get_bw = tegra194_mc_icc_get_init_bw,
+	.set = tegra194_mc_icc_set,
+};
+
 const struct tegra_mc_soc tegra194_mc_soc = {
 	.num_clients = ARRAY_SIZE(tegra194_mc_clients),
 	.clients = tegra194_mc_clients,
@@ -1355,7 +1412,7 @@ const struct tegra_mc_soc tegra194_mc_soc = {
 		   MC_INT_SECURITY_VIOLATION | MC_INT_DECERR_EMEM,
 	.has_addr_hi_reg = true,
 	.ops = &tegra186_mc_ops,
-	.icc_ops = &tegra_mc_icc_ops,
+	.icc_ops = &tegra194_mc_icc_ops,
 	.ch_intmask = 0x00000f00,
 	.global_intstatus_channel_shift = 8,
 };

-- 
2.51.0

Re: [PATCH v4 0/5] memory: tegra: Support EMC dfs on Tegra186/Tegra194

[Krzysztof Kozlowski]

On Mon, 27 Oct 2025 13:55:14 -0500, Aaron Kling wrote:
> This series borrows the concept used on Tegra234 to scale EMC based on
> CPU frequency and applies it to Tegra186 and Tegra194. Except that the
> bpmp on those archs does not support bandwidth manager, so the scaling
> iteself is handled similar to how Tegra124 currently works.
> 
> 

Applied, thanks!

[1/5] dt-bindings: memory: tegra186-mc: Add dummy client IDs for Tegra186
      https://git.kernel.org/krzk/linux-mem-ctrl/c/c15b28b1b3befb7ebf1c01c42623c3cede4cf9d1
[2/5] dt-bindings: memory: tegra194-mc: Add dummy client IDs for Tegra194
      https://git.kernel.org/krzk/linux-mem-ctrl/c/2aad3b30a7df710ff281d12a81bf84aa4bd98500
[3/5] memory: tegra186-emc: Support non-bpmp icc scaling
      https://git.kernel.org/krzk/linux-mem-ctrl/c/1004666bc437e234910c660f9d03a71ad44c027a
[4/5] memory: tegra186: Support icc scaling
      https://git.kernel.org/krzk/linux-mem-ctrl/c/dd6814eefbe524e433e1dbc25229c9338cb50027
[5/5] memory: tegra194: Support icc scaling
      https://git.kernel.org/krzk/linux-mem-ctrl/c/c9e39dd13ad6650b46ff3288ed33130a8bc771f8

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>