Skip to contentSkip to conversation

Introduce memory interconnect for NVIDIA Tegra SoCs

PATCH Applied Dmitry Osipenko 4 participants 54 messages Nov 11, 2020 – Nov 15, 2020

Subsystems (9)

v8 of 12 Applied Latest is v12 →

lore
Share & tools
lore

Dmitry Osipenko

author maintainer
#0 lore raw

v8 cover Introduce memory interconnect for NVIDIA Tegra SoCs

This series brings initial support for memory interconnect to Tegra20, Tegra30 and Tegra124 SoCs.

For the starter only display controllers and devfreq devices are getting interconnect API support, others could be supported later on. The display controllers have the biggest demand for interconnect API right now because dynamic memory frequency scaling can't be done safely without taking into account bandwidth requirement from the displays. In particular this series fixes distorted display output on T30 Ouya and T124 TK1 devices.

Changelog:

v8: - Patches that are already applied by Krzysztof Kozlowski not included.

- Removed tegra20-devfreq.c from MAINTAINERS, suggested by Chanwoo Choi.

- The patch "memory: tegra20-emc: Add devfreq support" now removes PM_OPP for Tegra20 EMC driver from Kconfig, suggested by Chanwoo Choi.

- Now using devm_devfreq_add_device() for Tegra20 devfreq, suggested by Chanwoo Choi.

- Added acks from Chanwoo Choi. Although, devfreq patches probably should go via devfreq tree in order to avoid conflicts with the other previously-applied patches by Chanwoo.

- Dropped superfluous dev_pm_opp_get_opp_table() from patch "devfreq:
tegra30: Support interconnect and OPPs from device-tree".

- Silenced another minor W=1 compilation warning reported by kernel test robot.

- Added couple extra minor clean-up changes, including those that will ease further code changes. Added these new patches:

memory: tegra20-emc: Remove IRQ number from error message
memory: tegra20-emc: Factor out clk initialization
memory: tegra30-emc: Factor out clk initialization

Note that some of the new T124 changes are compile-tested only, but I expect that we will re-test them fully soon.

- Added patch that fixes compilation of a non-Tegra kernels.

memory: tegra: Correct stub of devm_tegra_memory_controller_get()

- Replaced dev_pm_opp_get_opp_table() with dev_pm_opp_set_clkname(), as was suggested by Viresh Kumar in other thread.

- Added patch that will allow T124 EMC driver to probe even if DT doesn't have memory timings, this was missed by accident previously.

memory: tegra124-emc: Continue probing if timings are missing in device-tree

Dmitry Osipenko (26):
memory: tegra: Correct stub of devm_tegra_memory_controller_get()
memory: tegra20-emc: Use dev_pm_opp_set_clkname()
memory: tegra20-emc: Factor out clk initialization
memory: tegra20-emc: Add devfreq support
memory: tegra20-emc: Remove IRQ number from error message
memory: tegra30: Add FIFO sizes to memory clients
memory: tegra30-emc: Make driver modular
memory: tegra30-emc: Continue probing if timings are missing in
device-tree
memory: tegra30: Support interconnect framework
memory: tegra30-emc: Factor out clk initialization
memory: tegra124-emc: Make driver modular
memory: tegra124-emc: Continue probing if timings are missing in
device-tree
memory: tegra124: Support interconnect framework drm/tegra: dc: Support memory bandwidth management drm/tegra: dc: Extend debug stats with total number of events PM / devfreq: tegra30: Support interconnect and OPPs from device-tree PM / devfreq: tegra30: Separate configurations per-SoC generation PM / devfreq: tegra20: Deprecate in a favor of emc-stat based driver
ARM: tegra: Correct EMC registers size in Tegra20 device-tree
ARM: tegra: Add interconnect properties to Tegra20 device-tree
ARM: tegra: Add interconnect properties to Tegra30 device-tree
ARM: tegra: Add interconnect properties to Tegra124 device-tree
ARM: tegra: Add nvidia,memory-controller phandle to Tegra20 EMC
device-tree
ARM: tegra: Add DVFS properties to Tegra20 EMC device-tree node
ARM: tegra: Add DVFS properties to Tegra30 EMC and ACTMON device-tree
nodes
ARM: tegra: Add DVFS properties to Tegra124 EMC and ACTMON device-tree
nodes

 MAINTAINERS                                   |   1 -
 arch/arm/boot/dts/tegra124-apalis-emc.dtsi    |   8 +
 .../arm/boot/dts/tegra124-jetson-tk1-emc.dtsi |   8 +
 arch/arm/boot/dts/tegra124-nyan-big-emc.dtsi  |  10 +
 .../arm/boot/dts/tegra124-nyan-blaze-emc.dtsi |  10 +
 .../boot/dts/tegra124-peripherals-opp.dtsi    | 419 ++++++++++++++++++
 arch/arm/boot/dts/tegra124.dtsi               |  31 ++
 .../boot/dts/tegra20-acer-a500-picasso.dts    |   7 +
 arch/arm/boot/dts/tegra20-colibri.dtsi        |   4 +
 arch/arm/boot/dts/tegra20-paz00.dts           |   6 +
 .../arm/boot/dts/tegra20-peripherals-opp.dtsi |  92 ++++
 arch/arm/boot/dts/tegra20.dtsi                |  33 +-
 .../tegra30-asus-nexus7-grouper-common.dtsi   |   4 +
 ...30-asus-nexus7-grouper-memory-timings.dtsi |  12 +
 .../arm/boot/dts/tegra30-peripherals-opp.dtsi | 383 ++++++++++++++++
 arch/arm/boot/dts/tegra30.dtsi                |  33 +-
 drivers/clk/tegra/Makefile                    |   3 +-
 drivers/clk/tegra/clk-tegra124-emc.c          |  41 +-
 drivers/clk/tegra/clk-tegra124.c              |  27 +-
 drivers/clk/tegra/clk.h                       |  16 +-
 drivers/devfreq/Kconfig                       |  10 -
 drivers/devfreq/Makefile                      |   1 -
 drivers/devfreq/tegra20-devfreq.c             | 210 ---------
 drivers/devfreq/tegra30-devfreq.c             | 154 ++++---
 drivers/gpu/drm/tegra/Kconfig                 |   1 +
 drivers/gpu/drm/tegra/dc.c                    | 359 +++++++++++++++
 drivers/gpu/drm/tegra/dc.h                    |  19 +
 drivers/gpu/drm/tegra/drm.c                   |  14 +
 drivers/gpu/drm/tegra/hub.c                   |   3 +
 drivers/gpu/drm/tegra/plane.c                 | 121 +++++
 drivers/gpu/drm/tegra/plane.h                 |  15 +
 drivers/memory/tegra/Kconfig                  |   9 +-
 drivers/memory/tegra/mc.c                     |   3 +
 drivers/memory/tegra/tegra124-emc.c           | 395 +++++++++++++++--
 drivers/memory/tegra/tegra124.c               |  82 +++-
 drivers/memory/tegra/tegra20-emc.c            | 192 ++++++--
 drivers/memory/tegra/tegra30-emc.c            | 406 +++++++++++++++--
 drivers/memory/tegra/tegra30.c                | 239 +++++++++-
 include/linux/clk/tegra.h                     |   8 +
 include/soc/tegra/emc.h                       |  16 -
 include/soc/tegra/mc.h                        |   2 +-
 41 files changed, 2971 insertions(+), 436 deletions(-)
 create mode 100644 arch/arm/boot/dts/tegra124-peripherals-opp.dtsi
 create mode 100644 arch/arm/boot/dts/tegra20-peripherals-opp.dtsi
 create mode 100644 arch/arm/boot/dts/tegra30-peripherals-opp.dtsi
 delete mode 100644 drivers/devfreq/tegra20-devfreq.c
 delete mode 100644 include/soc/tegra/emc.h
Signature · 1 line
2.29.2
author maintainer
#1 lore raw

PATCH v8 1/26 memory: tegra: Correct stub of devm_tegra_memory_controller_get()

Correct typo in a stub of devm_tegra_memory_controller_get() to fix a non-ARM kernel compile-testing.

---
 include/soc/tegra/mc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/soc/tegra/mc.h +1 -1
index 43876216de34..d731407e23bb 100644
@@ -207,7 +207,7 @@ struct tegra_mc *devm_tegra_memory_controller_get(struct device *dev);
 static inline struct tegra_mc *
 devm_tegra_memory_controller_get(struct device *dev)
 {
-	ERR_PTR(-ENODEV);
+	return ERR_PTR(-ENODEV);
 }
 #endif
 
Signature · 1 line
2.29.2
maintainer Applied
#2 lore raw

Re: [PATCH v8 01/26] memory: tegra: Correct stub of devm_tegra_memory_controller_get()

Quoted 7 lines · Dmitry Osipenko

Correct typo in a stub of devm_tegra_memory_controller_get() to fix a non-ARM kernel compile-testing.

---
 include/soc/tegra/mc.h | 2 +-

Thanks, applied.

Best regards,
Krzysztof

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author maintainer
#3 lore raw

PATCH v8 2/26 memory: tegra20-emc: Use dev_pm_opp_set_clkname()

In reply to Dmitry Osipenko

The dev_pm_opp_get_opp_table() shouldn't be used by drivers, use dev_pm_opp_set_clkname() instead.

---
 drivers/memory/tegra/tegra20-emc.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)
drivers/memory/tegra/tegra20-emc.c +19 -11
index 5e10aa97809f..bb3f315c9587 100644
@@ -902,7 +902,7 @@ static int tegra_emc_interconnect_init(struct tegra_emc *emc)
 
 static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 {
-	struct opp_table *opp_table;
+	struct opp_table *reg_opp_table = NULL, *clk_opp_table;
 	const char *rname = "core";
 	int err;
 
@@ -917,19 +917,24 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 	}
 
 	/* voltage scaling is optional */
-	if (device_property_present(emc->dev, "core-supply"))
-		opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1);
-	else
-		opp_table = dev_pm_opp_get_opp_table(emc->dev);
+	if (device_property_present(emc->dev, "core-supply")) {
+		reg_opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1);
+		if (IS_ERR(reg_opp_table))
+			return dev_err_probe(emc->dev, PTR_ERR(reg_opp_table),
+					     "failed to set OPP regulator\n");
+	}
 
-	if (IS_ERR(opp_table))
-		return dev_err_probe(emc->dev, PTR_ERR(opp_table),
-				     "failed to prepare OPP table\n");
+	clk_opp_table = dev_pm_opp_set_clkname(emc->dev, NULL);
+	err = PTR_ERR_OR_ZERO(clk_opp_table);
+	if (err) {
+		dev_err(emc->dev, "failed to set OPP clk: %d\n", err);
+		goto put_reg_table;
+	}
 
 	err = dev_pm_opp_of_add_table(emc->dev);
 	if (err) {
 		dev_err(emc->dev, "failed to add OPP table: %d\n", err);
-		goto put_table;
+		goto put_clk_table;
 	}
 
 	dev_info(emc->dev, "current clock rate %lu MHz\n",
@@ -946,8 +951,11 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 
 remove_table:
 	dev_pm_opp_of_remove_table(emc->dev);
-put_table:
-	dev_pm_opp_put_regulators(opp_table);
+put_clk_table:
+	dev_pm_opp_put_clkname(clk_opp_table);
+put_reg_table:
+	if (reg_opp_table)
+		dev_pm_opp_put_regulators(reg_opp_table);
 
 	return err;
 }
Signature · 1 line
2.29.2
#4 lore raw

Re: [PATCH v8 02/26] memory: tegra20-emc: Use dev_pm_opp_set_clkname()

Quoted 32 lines · Dmitry Osipenko

The dev_pm_opp_get_opp_table() shouldn't be used by drivers, use dev_pm_opp_set_clkname() instead.

---
 drivers/memory/tegra/tegra20-emc.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)
drivers/memory/tegra/tegra20-emc.c +3 -5
index 5e10aa97809f..bb3f315c9587 100644
@@ -902,7 +902,7 @@ static int tegra_emc_interconnect_init(struct tegra_emc *emc)
 
 static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 {
-	struct opp_table *opp_table;
+	struct opp_table *reg_opp_table = NULL, *clk_opp_table;
 	const char *rname = "core";
 	int err;
 
@@ -917,19 +917,24 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 	}
 
 	/* voltage scaling is optional */
-	if (device_property_present(emc->dev, "core-supply"))
-		opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1);
-	else
-		opp_table = dev_pm_opp_get_opp_table(emc->dev);
+	if (device_property_present(emc->dev, "core-supply")) {
+		reg_opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1);

drivers/memory/tegra/tegra20-emc.c static int tegra_emc_opp_table_init(struct tegra_emc *emc) · line 928

Quoted 10 lines
drivers/memory/tegra/tegra20-emc.c +6 -3
+		if (IS_ERR(reg_opp_table))
+			return dev_err_probe(emc->dev, PTR_ERR(reg_opp_table),
+					     "failed to set OPP regulator\n");
+	}
 
-	if (IS_ERR(opp_table))
-		return dev_err_probe(emc->dev, PTR_ERR(opp_table),
-				     "failed to prepare OPP table\n");
+	clk_opp_table = dev_pm_opp_set_clkname(emc->dev, NULL);
+	err = PTR_ERR_OR_ZERO(clk_opp_table);

Don't check for NULL here.

Quoted 14 lines
drivers/memory/tegra/tegra20-emc.c +5 -1
+	if (err) {
+		dev_err(emc->dev, "failed to set OPP clk: %d\n", err);
+		goto put_reg_table;
+	}
 
 	err = dev_pm_opp_of_add_table(emc->dev);
 	if (err) {
 		dev_err(emc->dev, "failed to add OPP table: %d\n", err);
-		goto put_table;
+		goto put_clk_table;
 	}
 
 	dev_info(emc->dev, "current clock rate %lu MHz\n",

drivers/memory/tegra/tegra20-emc.c static int tegra_emc_opp_table_init(struct tegra_emc *emc) · line 957

Quoted 9 lines
drivers/memory/tegra/tegra20-emc.c +4 -2
@@ -946,8 +951,11 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 
 remove_table:
 	dev_pm_opp_of_remove_table(emc->dev);
-put_table:
-	dev_pm_opp_put_regulators(opp_table);
+put_clk_table:
+	dev_pm_opp_put_clkname(clk_opp_table);
+put_reg_table:
+	if (reg_opp_table)

This won't be required after my other patchset and yeah it is a classic chicken and egg problem we have here :)

Maybe you can fix them separately in 5.11 after everything is applied.

Quoted 6 lines
drivers/memory/tegra/tegra20-emc.c +1 -0
+		dev_pm_opp_put_regulators(reg_opp_table);
 
 	return err;
 }
Signature · 1 line
2.29.2
Signature · 1 line
viresh
#5 lore raw

Re: [PATCH v8 02/26] memory: tegra20-emc: Use dev_pm_opp_set_clkname()

Quoted 38 lines · Viresh Kumar
Quoted 37 lines · Dmitry Osipenko

The dev_pm_opp_get_opp_table() shouldn't be used by drivers, use dev_pm_opp_set_clkname() instead.

---
 drivers/memory/tegra/tegra20-emc.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)
drivers/memory/tegra/tegra20-emc.c +7 -5
index 5e10aa97809f..bb3f315c9587 100644
@@ -902,7 +902,7 @@ static int tegra_emc_interconnect_init(struct tegra_emc *emc)
 
 static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 {
-	struct opp_table *opp_table;
+	struct opp_table *reg_opp_table = NULL, *clk_opp_table;
 	const char *rname = "core";
 	int err;
 
@@ -917,19 +917,24 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 	}
 
 	/* voltage scaling is optional */
-	if (device_property_present(emc->dev, "core-supply"))
-		opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1);
-	else
-		opp_table = dev_pm_opp_get_opp_table(emc->dev);
+	if (device_property_present(emc->dev, "core-supply")) {
+		reg_opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1);
+		if (IS_ERR(reg_opp_table))
+			return dev_err_probe(emc->dev, PTR_ERR(reg_opp_table),
+					     "failed to set OPP regulator\n");
+	}
 
Quoted 7 lines

drivers/memory/tegra/tegra20-emc.c static int tegra_emc_opp_table_init(struct tegra_emc *emc) · line 928

Quoted 5 lines
drivers/memory/tegra/tegra20-emc.c +2 -3
-	if (IS_ERR(opp_table))
-		return dev_err_probe(emc->dev, PTR_ERR(opp_table),
-				     "failed to prepare OPP table\n");
+	clk_opp_table = dev_pm_opp_set_clkname(emc->dev, NULL);
+	err = PTR_ERR_OR_ZERO(clk_opp_table);

Don't check for NULL here.

My bad. You aren't checking but just converting to err. Its fine.

Signature · 1 line
viresh
author maintainer
#6 lore raw

Re: [PATCH v8 02/26] memory: tegra20-emc: Use dev_pm_opp_set_clkname()

In reply to Viresh Kumar

11.11.2020 08:45, Viresh Kumar пишет:

Quoted 7 lines

drivers/memory/tegra/tegra20-emc.c static int tegra_emc_opp_table_init(struct tegra_emc *emc) · line 957

Quoted 2 lines
drivers/memory/tegra/tegra20-emc.c +2 -0
+put_reg_table:
+	if (reg_opp_table)

This won't be required after my other patchset and yeah it is a classic chicken and egg problem we have here :)

Maybe you can fix them separately in 5.11 after everything is applied.

I already prepared patch in the "core voltage scaling" series that will remove this code.

maintainer Applied
#7 lore raw

Re: [PATCH v8 02/26] memory: tegra20-emc: Use dev_pm_opp_set_clkname()

In reply to Dmitry Osipenko
Quoted 8 lines · Dmitry Osipenko

The dev_pm_opp_get_opp_table() shouldn't be used by drivers, use dev_pm_opp_set_clkname() instead.

---
 drivers/memory/tegra/tegra20-emc.c | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

Thanks, applied.

Best regards,
Krzysztof

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author maintainer
#8 lore raw

PATCH v8 3/26 memory: tegra20-emc: Factor out clk initialization

In reply to Dmitry Osipenko

Factor out clk initialization and make it resource-managed. This makes easier to follow code and will help to make further changes cleaner.

---
 drivers/memory/tegra/tegra20-emc.c | 70 ++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 23 deletions(-)
drivers/memory/tegra/tegra20-emc.c +47 -23
index bb3f315c9587..d01b556a6d06 100644
@@ -960,6 +960,49 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 	return err;
 }
 
+static void devm_tegra_emc_unset_callback(void *data)
+{
+	tegra20_clk_set_emc_round_callback(NULL, NULL);
+}
+
+static void devm_tegra_emc_unreg_clk_notifier(void *data)
+{
+	struct tegra_emc *emc = data;
+
+	clk_notifier_unregister(emc->clk, &emc->clk_nb);
+}
+
+static int tegra_emc_init_clk(struct tegra_emc *emc)
+{
+	int err;
+
+	tegra20_clk_set_emc_round_callback(emc_round_rate, emc);
+
+	err = devm_add_action_or_reset(emc->dev, devm_tegra_emc_unset_callback,
+				       NULL);
+	if (err)
+		return err;
+
+	emc->clk = devm_clk_get(emc->dev, NULL);
+	if (IS_ERR(emc->clk)) {
+		dev_err(emc->dev, "failed to get EMC clock: %pe\n", emc->clk);
+		return PTR_ERR(emc->clk);
+	}
+
+	err = clk_notifier_register(emc->clk, &emc->clk_nb);
+	if (err) {
+		dev_err(emc->dev, "failed to register clk notifier: %d\n", err);
+		return err;
+	}
+
+	err = devm_add_action_or_reset(emc->dev,
+				       devm_tegra_emc_unreg_clk_notifier, emc);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 static int tegra_emc_probe(struct platform_device *pdev)
 {
 	struct device_node *np;
@@ -1003,25 +1046,13 @@ static int tegra_emc_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	tegra20_clk_set_emc_round_callback(emc_round_rate, emc);
-
-	emc->clk = devm_clk_get(&pdev->dev, "emc");
-	if (IS_ERR(emc->clk)) {
-		err = PTR_ERR(emc->clk);
-		dev_err(&pdev->dev, "failed to get emc clock: %d\n", err);
-		goto unset_cb;
-	}
-
-	err = clk_notifier_register(emc->clk, &emc->clk_nb);
-	if (err) {
-		dev_err(&pdev->dev, "failed to register clk notifier: %d\n",
-			err);
-		goto unset_cb;
-	}
+	err = tegra_emc_init_clk(emc);
+	if (err)
+		return err;
 
 	err = tegra_emc_opp_table_init(emc);
 	if (err)
-		goto unreg_notifier;
+		return err;
 
 	platform_set_drvdata(pdev, emc);
 	tegra_emc_rate_requests_init(emc);
@@ -1036,13 +1067,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
 	try_module_get(THIS_MODULE);
 
 	return 0;
-
-unreg_notifier:
-	clk_notifier_unregister(emc->clk, &emc->clk_nb);
-unset_cb:
-	tegra20_clk_set_emc_round_callback(NULL, NULL);
-
-	return err;
 }
 
 static const struct of_device_id tegra_emc_of_match[] = {
Signature · 1 line
2.29.2
maintainer Applied
#9 lore raw

Re: [PATCH v8 03/26] memory: tegra20-emc: Factor out clk initialization

Quoted 7 lines · Dmitry Osipenko

Factor out clk initialization and make it resource-managed. This makes easier to follow code and will help to make further changes cleaner.

---
 drivers/memory/tegra/tegra20-emc.c | 70 ++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 23 deletions(-)

Thanks, applied.

Best regards,
Krzysztof

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author maintainer
#10 lore raw

PATCH v8 4/26 memory: tegra20-emc: Add devfreq support

In reply to Dmitry Osipenko

Add devfreq support to the Tegra20 EMC driver. Memory utilization statistics will be periodically polled from the memory controller and appropriate minimum clock rate will be selected by the devfreq governor.

---
 drivers/memory/tegra/Kconfig       |  3 +-
 drivers/memory/tegra/tegra20-emc.c | 90 ++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 1 deletion(-)
drivers/memory/tegra/Kconfig +2 -1
index ac3dfe155505..8cc1ec5be443 100644
@@ -12,7 +12,8 @@ config TEGRA20_EMC
 	tristate "NVIDIA Tegra20 External Memory Controller driver"
 	default y
 	depends on TEGRA_MC && ARCH_TEGRA_2x_SOC
-	select PM_OPP
+	select DEVFREQ_GOV_SIMPLE_ONDEMAND
+	select PM_DEVFREQ
 	help
 	  This driver is for the External Memory Controller (EMC) found on
 	  Tegra20 chips. The EMC controls the external DRAM on the board.
drivers/memory/tegra/tegra20-emc.c +90 -0

126 lines not on this page · raw message

Signature · 1 line
2.29.2
maintainer Reviewed-by on v8 4/26
#11 lore raw

Re: [PATCH v8 04/26] memory: tegra20-emc: Add devfreq support

Hi Dmitry,

Quoted 154 lines · Dmitry Osipenko

Add devfreq support to the Tegra20 EMC driver. Memory utilization statistics will be periodically polled from the memory controller and appropriate minimum clock rate will be selected by the devfreq governor.

---
 drivers/memory/tegra/Kconfig       |  3 +-
 drivers/memory/tegra/tegra20-emc.c | 90 ++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 1 deletion(-)
drivers/memory/tegra/Kconfig +2 -1
index ac3dfe155505..8cc1ec5be443 100644
@@ -12,7 +12,8 @@ config TEGRA20_EMC
 	tristate "NVIDIA Tegra20 External Memory Controller driver"
 	default y
 	depends on TEGRA_MC && ARCH_TEGRA_2x_SOC
-	select PM_OPP
+	select DEVFREQ_GOV_SIMPLE_ONDEMAND
+	select PM_DEVFREQ
 	help
 	  This driver is for the External Memory Controller (EMC) found on
 	  Tegra20 chips. The EMC controls the external DRAM on the board.
drivers/memory/tegra/tegra20-emc.c +89 -0
index d01b556a6d06..b9cd965980e2 100644
@@ -8,6 +8,7 @@
 #include <linux/clk.h>
 #include <linux/clk/tegra.h>
 #include <linux/debugfs.h>
+#include <linux/devfreq.h>
 #include <linux/err.h>
 #include <linux/interconnect-provider.h>
 #include <linux/interrupt.h>
@@ -102,6 +103,10 @@
 
 #define EMC_FBIO_CFG5_DRAM_WIDTH_X16		BIT(4)
 
+#define EMC_PWR_GATHER_CLEAR			(1 << 8)
+#define EMC_PWR_GATHER_DISABLE			(2 << 8)
+#define EMC_PWR_GATHER_ENABLE			(3 << 8)
+
 static const u16 emc_timing_registers[] = {
 	EMC_RC,
 	EMC_RFC,
@@ -157,6 +162,7 @@ struct emc_timing {
 };
 
 enum emc_rate_request_type {
+	EMC_RATE_DEVFREQ,
 	EMC_RATE_DEBUG,
 	EMC_RATE_ICC,
 	EMC_RATE_TYPE_MAX,
@@ -193,6 +199,8 @@ struct tegra_emc {
 
 	/* protect shared rate-change code path */
 	struct mutex rate_lock;
+
+	struct devfreq_simple_ondemand_data ondemand_data;
 };
 
 static irqreturn_t tegra_emc_isr(int irq, void *data)
@@ -1003,6 +1011,87 @@ static int tegra_emc_init_clk(struct tegra_emc *emc)
 	return 0;
 }
 
+static int tegra_emc_devfreq_target(struct device *dev, unsigned long *freq,
+				    u32 flags)
+{
+	struct tegra_emc *emc = dev_get_drvdata(dev);
+	struct dev_pm_opp *opp;
+	unsigned long rate;
+
+	opp = devfreq_recommended_opp(dev, freq, flags);
+	if (IS_ERR(opp)) {
+		dev_err(dev, "failed to find opp for %lu Hz\n", *freq);
+		return PTR_ERR(opp);
+	}
+
+	rate = dev_pm_opp_get_freq(opp);
+	dev_pm_opp_put(opp);
+
+	return emc_set_min_rate(emc, rate, EMC_RATE_DEVFREQ);
+}
+
+static int tegra_emc_devfreq_get_dev_status(struct device *dev,
+					    struct devfreq_dev_status *stat)
+{
+	struct tegra_emc *emc = dev_get_drvdata(dev);
+
+	/* freeze counters */
+	writel_relaxed(EMC_PWR_GATHER_DISABLE, emc->regs + EMC_STAT_CONTROL);
+
+	/*
+	 *  busy_time: number of clocks EMC request was accepted
+	 * total_time: number of clocks PWR_GATHER control was set to ENABLE
+	 */
+	stat->busy_time = readl_relaxed(emc->regs + EMC_STAT_PWR_COUNT);
+	stat->total_time = readl_relaxed(emc->regs + EMC_STAT_PWR_CLOCKS);
+	stat->current_frequency = clk_get_rate(emc->clk);
+
+	/* clear counters and restart */
+	writel_relaxed(EMC_PWR_GATHER_CLEAR, emc->regs + EMC_STAT_CONTROL);
+	writel_relaxed(EMC_PWR_GATHER_ENABLE, emc->regs + EMC_STAT_CONTROL);
+
+	return 0;
+}
+
+static struct devfreq_dev_profile tegra_emc_devfreq_profile = {
+	.polling_ms = 30,
+	.target = tegra_emc_devfreq_target,
+	.get_dev_status = tegra_emc_devfreq_get_dev_status,
+};
+
+static int tegra_emc_devfreq_init(struct tegra_emc *emc)
+{
+	struct devfreq *devfreq;
+
+	/*
+	 * PWR_COUNT is 1/2 of PWR_CLOCKS at max, and thus, the up-threshold
+	 * should be less than 50.  Secondly, multiple active memory clients
+	 * may cause over 20% of lost clock cycles due to stalls caused by
+	 * competing memory accesses.  This means that threshold should be
+	 * set to a less than 30 in order to have a properly working governor.
+	 */
+	emc->ondemand_data.upthreshold = 20;
+
+	/*
+	 * Reset statistic gathers state, select global bandwidth for the
+	 * statistics collection mode and set clocks counter saturation
+	 * limit to maximum.
+	 */
+	writel_relaxed(0x00000000, emc->regs + EMC_STAT_CONTROL);
+	writel_relaxed(0x00000000, emc->regs + EMC_STAT_LLMC_CONTROL);
+	writel_relaxed(0xffffffff, emc->regs + EMC_STAT_PWR_CLOCK_LIMIT);
+
+	devfreq = devm_devfreq_add_device(emc->dev, &tegra_emc_devfreq_profile,
+					  DEVFREQ_GOV_SIMPLE_ONDEMAND,
+					  &emc->ondemand_data);
+	if (IS_ERR(devfreq)) {
+		dev_err(emc->dev, "failed to initialize devfreq: %pe", devfreq);
+		return PTR_ERR(devfreq);
+	}
+
+	return 0;
+}
+
 static int tegra_emc_probe(struct platform_device *pdev)
 {
 	struct device_node *np;

drivers/memory/tegra/tegra20-emc.c static int tegra_emc_probe(struct platform_device *pdev) · line 1153

Quoted 7 lines
drivers/memory/tegra/tegra20-emc.c +1 -0
@@ -1058,6 +1147,7 @@ static int tegra_emc_probe(struct platform_device *pdev)
 	tegra_emc_rate_requests_init(emc);
 	tegra_emc_debugfs_init(emc);
 	tegra_emc_interconnect_init(emc);
+	tegra_emc_devfreq_init(emc);
 
 	/*
 	 * Don't allow the kernel module to be unloaded. Unloading adds some
Signature · 7 lines
Best Regards,
Chanwoo Choi
Samsung Electronics
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
maintainer Applied
#12 lore raw

Re: [PATCH v8 04/26] memory: tegra20-emc: Add devfreq support

In reply to Dmitry Osipenko
Quoted 9 lines · Dmitry Osipenko

Add devfreq support to the Tegra20 EMC driver. Memory utilization statistics will be periodically polled from the memory controller and appropriate minimum clock rate will be selected by the devfreq governor.

---
 drivers/memory/tegra/Kconfig       |  3 +-
 drivers/memory/tegra/tegra20-emc.c | 90 ++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 1 deletion(-)

Thanks, applied.

Best regards,
Krzysztof

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author maintainer
#13 lore raw

PATCH v8 5/26 memory: tegra20-emc: Remove IRQ number from error message

In reply to Dmitry Osipenko

Remove IRQ number from error message since it doesn't add any useful information, especially because this number is virtual.

---
 drivers/memory/tegra/tegra20-emc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/memory/tegra/tegra20-emc.c +1 -1
index b9cd965980e2..0320d9df4a20 100644
@@ -1131,7 +1131,7 @@ static int tegra_emc_probe(struct platform_device *pdev)
 	err = devm_request_irq(&pdev->dev, irq, tegra_emc_isr, 0,
 			       dev_name(&pdev->dev), emc);
 	if (err) {
-		dev_err(&pdev->dev, "failed to request IRQ#%u: %d\n", irq, err);
+		dev_err(&pdev->dev, "failed to request IRQ: %d\n", err);
 		return err;
 	}
 
Signature · 1 line
2.29.2
maintainer Applied
#14 lore raw

Re: [PATCH v8 05/26] memory: tegra20-emc: Remove IRQ number from error message

Quoted 7 lines · Dmitry Osipenko

Remove IRQ number from error message since it doesn't add any useful information, especially because this number is virtual.

---
 drivers/memory/tegra/tegra20-emc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Thanks, applied.

Best regards,
Krzysztof

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author maintainer
#15 lore raw

PATCH v8 6/26 memory: tegra30: Add FIFO sizes to memory clients

In reply to Dmitry Osipenko

The latency allowness is calculated based on buffering capabilities of memory clients. Add FIFO sizes to the Tegra30 memory clients.

---
 drivers/memory/tegra/tegra30.c | 66 ++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)
drivers/memory/tegra/tegra30.c +66 -0
index b1990b4133d8..d0314f29608d 100644
@@ -42,6 +42,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x0,
 		},
+		.fifo_size = 16 * 2,
 	}, {
 		.id = 0x01,
 		.name = "display0a",
@@ -56,6 +57,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x4e,
 		},
+		.fifo_size = 16 * 128,
 	}, {
 		.id = 0x02,
 		.name = "display0ab",
@@ -70,6 +72,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x4e,
 		},
+		.fifo_size = 16 * 128,
 	}, {
 		.id = 0x03,
 		.name = "display0b",
@@ -84,6 +87,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x4e,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x04,
 		.name = "display0bb",
@@ -98,6 +102,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x4e,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x05,
 		.name = "display0c",
@@ -112,6 +117,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x4e,
 		},
+		.fifo_size = 16 * 128,
 	}, {
 		.id = 0x06,
 		.name = "display0cb",
@@ -126,6 +132,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x4e,
 		},
+		.fifo_size = 16 * 128,
 	}, {
 		.id = 0x07,
 		.name = "display1b",
@@ -140,6 +147,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x4e,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x08,
 		.name = "display1bb",
@@ -154,6 +162,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x4e,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x09,
 		.name = "eppup",
@@ -168,6 +177,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x17,
 		},
+		.fifo_size = 16 * 8,
 	}, {
 		.id = 0x0a,
 		.name = "g2pr",
@@ -182,6 +192,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x09,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x0b,
 		.name = "g2sr",
@@ -196,6 +207,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x09,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x0c,
 		.name = "mpeunifbr",
@@ -210,6 +222,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x50,
 		},
+		.fifo_size = 16 * 8,
 	}, {
 		.id = 0x0d,
 		.name = "viruv",
@@ -224,6 +237,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x2c,
 		},
+		.fifo_size = 16 * 8,
 	}, {
 		.id = 0x0e,
 		.name = "afir",
@@ -238,6 +252,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x10,
 		},
+		.fifo_size = 16 * 32,
 	}, {
 		.id = 0x0f,
 		.name = "avpcarm7r",
@@ -252,6 +267,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x04,
 		},
+		.fifo_size = 16 * 2,
 	}, {
 		.id = 0x10,
 		.name = "displayhc",
@@ -266,6 +282,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0xff,
 		},
+		.fifo_size = 16 * 2,
 	}, {
 		.id = 0x11,
 		.name = "displayhcb",
@@ -280,6 +297,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0xff,
 		},
+		.fifo_size = 16 * 2,
 	}, {
 		.id = 0x12,
 		.name = "fdcdrd",
@@ -294,6 +312,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x0a,
 		},
+		.fifo_size = 16 * 48,
 	}, {
 		.id = 0x13,
 		.name = "fdcdrd2",
@@ -308,6 +327,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x0a,
 		},
+		.fifo_size = 16 * 48,
 	}, {
 		.id = 0x14,
 		.name = "g2dr",
@@ -322,6 +342,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x0a,
 		},
+		.fifo_size = 16 * 48,
 	}, {
 		.id = 0x15,
 		.name = "hdar",
@@ -336,6 +357,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0xff,
 		},
+		.fifo_size = 16 * 16,
 	}, {
 		.id = 0x16,
 		.name = "host1xdmar",
@@ -350,6 +372,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x05,
 		},
+		.fifo_size = 16 * 16,
 	}, {
 		.id = 0x17,
 		.name = "host1xr",
@@ -364,6 +387,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x50,
 		},
+		.fifo_size = 16 * 8,
 	}, {
 		.id = 0x18,
 		.name = "idxsrd",
@@ -378,6 +402,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x13,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x19,
 		.name = "idxsrd2",
@@ -392,6 +417,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x13,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x1a,
 		.name = "mpe_ipred",
@@ -406,6 +432,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x80,
 		},
+		.fifo_size = 16 * 2,
 	}, {
 		.id = 0x1b,
 		.name = "mpeamemrd",
@@ -420,6 +447,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x42,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x1c,
 		.name = "mpecsrd",
@@ -434,6 +462,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0xff,
 		},
+		.fifo_size = 16 * 8,
 	}, {
 		.id = 0x1d,
 		.name = "ppcsahbdmar",
@@ -448,6 +477,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x10,
 		},
+		.fifo_size = 16 * 2,
 	}, {
 		.id = 0x1e,
 		.name = "ppcsahbslvr",
@@ -462,6 +492,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x12,
 		},
+		.fifo_size = 16 * 8,
 	}, {
 		.id = 0x1f,
 		.name = "satar",
@@ -476,6 +507,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x33,
 		},
+		.fifo_size = 16 * 32,
 	}, {
 		.id = 0x20,
 		.name = "texsrd",
@@ -490,6 +522,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x13,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x21,
 		.name = "texsrd2",
@@ -504,6 +537,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x13,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x22,
 		.name = "vdebsevr",
@@ -518,6 +552,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0xff,
 		},
+		.fifo_size = 16 * 8,
 	}, {
 		.id = 0x23,
 		.name = "vdember",
@@ -532,6 +567,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0xd0,
 		},
+		.fifo_size = 16 * 4,
 	}, {
 		.id = 0x24,
 		.name = "vdemcer",
@@ -546,6 +582,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x2a,
 		},
+		.fifo_size = 16 * 16,
 	}, {
 		.id = 0x25,
 		.name = "vdetper",
@@ -560,6 +597,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x74,
 		},
+		.fifo_size = 16 * 16,
 	}, {
 		.id = 0x26,
 		.name = "mpcorelpr",
@@ -570,6 +608,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x04,
 		},
+		.fifo_size = 16 * 14,
 	}, {
 		.id = 0x27,
 		.name = "mpcorer",
@@ -580,6 +619,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x04,
 		},
+		.fifo_size = 16 * 14,
 	}, {
 		.id = 0x28,
 		.name = "eppu",
@@ -594,6 +634,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x6c,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x29,
 		.name = "eppv",
@@ -608,6 +649,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x6c,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x2a,
 		.name = "eppy",
@@ -622,6 +664,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x6c,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x2b,
 		.name = "mpeunifbw",
@@ -636,6 +679,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x13,
 		},
+		.fifo_size = 16 * 8,
 	}, {
 		.id = 0x2c,
 		.name = "viwsb",
@@ -650,6 +694,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x12,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x2d,
 		.name = "viwu",
@@ -664,6 +709,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0xb2,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x2e,
 		.name = "viwv",
@@ -678,6 +724,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0xb2,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x2f,
 		.name = "viwy",
@@ -692,6 +739,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x12,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x30,
 		.name = "g2dw",
@@ -706,6 +754,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x9,
 		},
+		.fifo_size = 16 * 128,
 	}, {
 		.id = 0x31,
 		.name = "afiw",
@@ -720,6 +769,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x0c,
 		},
+		.fifo_size = 16 * 32,
 	}, {
 		.id = 0x32,
 		.name = "avpcarm7w",
@@ -734,6 +784,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x0e,
 		},
+		.fifo_size = 16 * 2,
 	}, {
 		.id = 0x33,
 		.name = "fdcdwr",
@@ -748,6 +799,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x0a,
 		},
+		.fifo_size = 16 * 48,
 	}, {
 		.id = 0x34,
 		.name = "fdcdwr2",
@@ -762,6 +814,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x0a,
 		},
+		.fifo_size = 16 * 48,
 	}, {
 		.id = 0x35,
 		.name = "hdaw",
@@ -776,6 +829,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0xff,
 		},
+		.fifo_size = 16 * 16,
 	}, {
 		.id = 0x36,
 		.name = "host1xw",
@@ -790,6 +844,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x10,
 		},
+		.fifo_size = 16 * 32,
 	}, {
 		.id = 0x37,
 		.name = "ispw",
@@ -804,6 +859,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0xff,
 		},
+		.fifo_size = 16 * 64,
 	}, {
 		.id = 0x38,
 		.name = "mpcorelpw",
@@ -814,6 +870,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x0e,
 		},
+		.fifo_size = 16 * 24,
 	}, {
 		.id = 0x39,
 		.name = "mpcorew",
@@ -824,6 +881,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x0e,
 		},
+		.fifo_size = 16 * 24,
 	}, {
 		.id = 0x3a,
 		.name = "mpecswr",
@@ -838,6 +896,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0xff,
 		},
+		.fifo_size = 16 * 8,
 	}, {
 		.id = 0x3b,
 		.name = "ppcsahbdmaw",
@@ -852,6 +911,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x10,
 		},
+		.fifo_size = 16 * 2,
 	}, {
 		.id = 0x3c,
 		.name = "ppcsahbslvw",
@@ -866,6 +926,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x06,
 		},
+		.fifo_size = 16 * 4,
 	}, {
 		.id = 0x3d,
 		.name = "sataw",
@@ -880,6 +941,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x33,
 		},
+		.fifo_size = 16 * 32,
 	}, {
 		.id = 0x3e,
 		.name = "vdebsevw",
@@ -894,6 +956,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0xff,
 		},
+		.fifo_size = 16 * 4,
 	}, {
 		.id = 0x3f,
 		.name = "vdedbgw",
@@ -908,6 +971,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0xff,
 		},
+		.fifo_size = 16 * 16,
 	}, {
 		.id = 0x40,
 		.name = "vdembew",
@@ -922,6 +986,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x42,
 		},
+		.fifo_size = 16 * 2,
 	}, {
 		.id = 0x41,
 		.name = "vdetpmw",
@@ -936,6 +1001,7 @@ static const struct tegra_mc_client tegra30_mc_clients[] = {
 			.mask = 0xff,
 			.def = 0x2a,
 		},
+		.fifo_size = 16 * 16,
 	},
 };
 
Signature · 1 line
2.29.2
maintainer Applied
#16 lore raw

Re: [PATCH v8 06/26] memory: tegra30: Add FIFO sizes to memory clients

Quoted 7 lines · Dmitry Osipenko

The latency allowness is calculated based on buffering capabilities of memory clients. Add FIFO sizes to the Tegra30 memory clients.

---
 drivers/memory/tegra/tegra30.c | 66 ++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

Thanks, applied.

Best regards,
Krzysztof

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author maintainer
#17 lore raw

PATCH v8 7/26 memory: tegra30-emc: Make driver modular

In reply to Dmitry Osipenko

Add modularization support to the Tegra30 EMC driver, which now can be compiled as a loadable kernel module.

---
 drivers/memory/tegra/Kconfig       |  2 +-
 drivers/memory/tegra/mc.c          |  3 +++
 drivers/memory/tegra/tegra30-emc.c | 17 ++++++++++++-----
 3 files changed, 16 insertions(+), 6 deletions(-)
drivers/memory/tegra/Kconfig +1 -1
index 8cc1ec5be443..2a4a16bcf91c 100644
@@ -21,7 +21,7 @@ config TEGRA20_EMC
 	  external memory.
 
 config TEGRA30_EMC
-	bool "NVIDIA Tegra30 External Memory Controller driver"
+	tristate "NVIDIA Tegra30 External Memory Controller driver"
 	default y
 	depends on TEGRA_MC && ARCH_TEGRA_3x_SOC
 	help
drivers/memory/tegra/mc.c +3 -0
index a7e6a8e4c95a..44064de962c2 100644
@@ -6,6 +6,7 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/dma-mapping.h>
+#include <linux/export.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -346,6 +347,7 @@ int tegra_mc_write_emem_configuration(struct tegra_mc *mc, unsigned long rate)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(tegra_mc_write_emem_configuration);
 
 unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc)
 {
@@ -357,6 +359,7 @@ unsigned int tegra_mc_get_emem_device_count(struct tegra_mc *mc)
 
 	return dram_count;
 }
+EXPORT_SYMBOL_GPL(tegra_mc_get_emem_device_count);
 
 static int load_one_timing(struct tegra_mc *mc,
 			   struct tegra_mc_timing *timing,
drivers/memory/tegra/tegra30-emc.c +12 -5
index 1be28e28ec34..d0926088360a 100644
@@ -1330,6 +1330,13 @@ static int tegra_emc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, emc);
 	tegra_emc_debugfs_init(emc);
 
+	/*
+	 * Don't allow the kernel module to be unloaded. Unloading adds some
+	 * extra complexity which doesn't really worth the effort in a case of
+	 * this driver.
+	 */
+	try_module_get(THIS_MODULE);
+
 	return 0;
 
 unset_cb:
@@ -1380,6 +1387,7 @@ static const struct of_device_id tegra_emc_of_match[] = {
 	{ .compatible = "nvidia,tegra30-emc", },
 	{},
 };
+MODULE_DEVICE_TABLE(of, tegra_emc_of_match);
 
 static struct platform_driver tegra_emc_driver = {
 	.probe = tegra_emc_probe,
@@ -1390,9 +1398,8 @@ static struct platform_driver tegra_emc_driver = {
 		.suppress_bind_attrs = true,
 	},
 };
+module_platform_driver(tegra_emc_driver);
 
-static int __init tegra_emc_init(void)
-{
-	return platform_driver_register(&tegra_emc_driver);
-}
-subsys_initcall(tegra_emc_init);
+MODULE_AUTHOR("Dmitry Osipenko <[email protected]>");
+MODULE_DESCRIPTION("NVIDIA Tegra30 EMC driver");
+MODULE_LICENSE("GPL v2");
Signature · 1 line
2.29.2
maintainer Applied
#18 lore raw

Re: [PATCH v8 07/26] memory: tegra30-emc: Make driver modular

Quoted 8 lines · Dmitry Osipenko

Add modularization support to the Tegra30 EMC driver, which now can be compiled as a loadable kernel module.

---
 drivers/memory/tegra/Kconfig       |  2 +-
 drivers/memory/tegra/mc.c          |  3 +++
 drivers/memory/tegra/tegra30-emc.c | 17 ++++++++++++-----

Thanks, applied.

Best regards,
Krzysztof

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author maintainer
#19 lore raw

PATCH v8 8/26 memory: tegra30-emc: Continue probing if timings are missing in device-tree

In reply to Dmitry Osipenko

EMC driver will become mandatory after turning it into interconnect provider because interconnect users, like display controller driver, will fail to probe using newer device-trees that have interconnect properties. Thus make EMC driver to probe even if timings are missing in device-tree.

---
 drivers/memory/tegra/tegra30-emc.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)
drivers/memory/tegra/tegra30-emc.c +15 -14
index d0926088360a..3488786da03b 100644
@@ -988,6 +988,11 @@ static struct device_node *emc_find_node_by_ram_code(struct device *dev)
 	u32 value, ram_code;
 	int err;
 
+	if (of_get_child_count(dev->of_node) == 0) {
+		dev_info(dev, "device-tree doesn't have memory timings\n");
+		return NULL;
+	}
+
 	ram_code = tegra_read_ram_code();
 
 	for_each_child_of_node(dev->of_node, np) {
@@ -1057,6 +1062,9 @@ static long emc_round_rate(unsigned long rate,
 	struct tegra_emc *emc = arg;
 	unsigned int i;
 
+	if (!emc->num_timings)
+		return clk_get_rate(emc->clk);
+
 	min_rate = min(min_rate, emc->timings[emc->num_timings - 1].rate);
 
 	for (i = 0; i < emc->num_timings; i++) {
@@ -1262,16 +1270,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
 	struct tegra_emc *emc;
 	int err;
 
-	if (of_get_child_count(pdev->dev.of_node) == 0) {
-		dev_info(&pdev->dev,
-			 "device-tree node doesn't have memory timings\n");
-		return -ENODEV;
-	}
-
-	np = emc_find_node_by_ram_code(&pdev->dev);
-	if (!np)
-		return -EINVAL;
-
 	emc = devm_kzalloc(&pdev->dev, sizeof(*emc), GFP_KERNEL);
 	if (!emc) {
 		of_node_put(np);
@@ -1285,10 +1283,13 @@ static int tegra_emc_probe(struct platform_device *pdev)
 	emc->clk_nb.notifier_call = emc_clk_change_notify;
 	emc->dev = &pdev->dev;
 
-	err = emc_load_timings_from_dt(emc, np);
-	of_node_put(np);
-	if (err)
-		return err;
+	np = emc_find_node_by_ram_code(&pdev->dev);
+	if (np) {
+		err = emc_load_timings_from_dt(emc, np);
+		of_node_put(np);
+		if (err)
+			return err;
+	}
 
 	emc->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(emc->regs))
Signature · 1 line
2.29.2
maintainer Applied
#20 lore raw

Re: [PATCH v8 08/26] memory: tegra30-emc: Continue probing if timings are missing in device-tree

Quoted 9 lines · Dmitry Osipenko

EMC driver will become mandatory after turning it into interconnect provider because interconnect users, like display controller driver, will fail to probe using newer device-trees that have interconnect properties. Thus make EMC driver to probe even if timings are missing in device-tree.

---
 drivers/memory/tegra/tegra30-emc.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

Thanks, applied 1-8. For the rest I see discussion on going, so I guess there will be a v9.

Best regards,
Krzysztof

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author maintainer
#21 lore raw

Re: [PATCH v8 08/26] memory: tegra30-emc: Continue probing if timings are missing in device-tree

14.11.2020 18:42, Krzysztof Kozlowski пишет:

Quoted 6 lines · Dmitry Osipenko
Quoted 5 lines

EMC driver will become mandatory after turning it into interconnect provider because interconnect users, like display controller driver, will fail to probe using newer device-trees that have interconnect properties. Thus make EMC driver to probe even if timings are missing in device-tree.

Quoted 7 lines
Quoted 4 lines
---
 drivers/memory/tegra/tegra30-emc.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

Thanks, applied 1-8. For the rest I see discussion on going, so I guess there will be a v9.

The v9 will be ready soon, thank you.

author maintainer
#22 lore raw

PATCH v8 9/26 memory: tegra30: Support interconnect framework

In reply to Dmitry Osipenko

Now Internal and External memory controllers are memory interconnection providers. This allows us to use interconnect API for tuning of memory configuration. EMC driver now supports OPPs and DVFS. MC driver now supports tuning of memory arbitration latency, which needs to be done for ISO memory clients, like a Display client for example.

---
 drivers/memory/tegra/Kconfig       |   1 +
 drivers/memory/tegra/tegra30-emc.c | 300 ++++++++++++++++++++++++++++-
 drivers/memory/tegra/tegra30.c     | 173 ++++++++++++++++-
 3 files changed, 471 insertions(+), 3 deletions(-)
drivers/memory/tegra/Kconfig +1 -0
index 2a4a16bcf91c..ca7077a06f4c 100644
@@ -24,6 +24,7 @@ config TEGRA30_EMC
 	tristate "NVIDIA Tegra30 External Memory Controller driver"
 	default y
 	depends on TEGRA_MC && ARCH_TEGRA_3x_SOC
+	select PM_OPP
 	help
 	  This driver is for the External Memory Controller (EMC) found on
 	  Tegra30 chips. The EMC controls the external DRAM on the board.
drivers/memory/tegra/tegra30-emc.c +298 -2

381 lines not on this page · raw message

drivers/memory/tegra/tegra30.c +172 -1

189 lines not on this page · raw message

Signature · 1 line
2.29.2
#23 lore raw

Re: [PATCH v8 09/26] memory: tegra30: Support interconnect framework

drivers/memory/tegra/tegra30-emc.c static void tegra_emc_debugfs_init(struct tegra_emc *emc) · line 1494

Quoted 12 lines · Dmitry Osipenko
drivers/memory/tegra/tegra30-emc.c +12 -0
+static int tegra_emc_opp_table_init(struct tegra_emc *emc)
+{
+	struct opp_table *reg_opp_table = NULL, *clk_opp_table, *hw_opp_table;
+	u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
+	const char *rname = "core";
+	int err;
+
+	/*
+	 * Legacy device-trees don't have OPP table and EMC driver isn't
+	 * useful in this case.
+	 */
+	if (!device_property_present(emc->dev, "operating-points-v2")) {

I don't understand why you want to check this ? The below call to dev_pm_opp_of_add_table() will fail anyway and that should be good for you.

Quoted 4 lines
drivers/memory/tegra/tegra30-emc.c +4 -0
+		dev_err(emc->dev,
+			"OPP table not found, please update your device tree\n");
+		return -ENODEV;
+	}

drivers/memory/tegra/tegra30-emc.c static void tegra_emc_debugfs_init(struct tegra_emc *emc) · line 1508

Quoted 10 lines
drivers/memory/tegra/tegra30-emc.c +10 -0
+
+	/* voltage scaling is optional */
+	if (device_property_present(emc->dev, "core-supply")) {
+		reg_opp_table = dev_pm_opp_set_regulators(emc->dev, &rname, 1);
+		if (IS_ERR(reg_opp_table))
+			return dev_err_probe(emc->dev, PTR_ERR(reg_opp_table),
+					     "failed to set OPP regulator\n");
+	}
+
+	clk_opp_table = dev_pm_opp_set_clkname(emc->dev, NULL);

I think there is still some misunderstanding here. This call with a NULL connection id is useful only if the DT OPP table is optional for your platform and you want the same driver to work with and without the DT OPP table. Clearly in your case you want the OPP table in the DT to be there and so this call is not required at all.

Quoted 43 lines
drivers/memory/tegra/tegra30-emc.c +43 -0
+	err = PTR_ERR_OR_ZERO(clk_opp_table);
+	if (err) {
+		dev_err(emc->dev, "failed to set OPP clk: %d\n", err);
+		goto put_reg_table;
+	}
+
+	hw_opp_table = dev_pm_opp_set_supported_hw(emc->dev, &hw_version, 1);
+	err = PTR_ERR_OR_ZERO(hw_opp_table);
+	if (err) {
+		dev_err(emc->dev, "failed to set OPP supported HW: %d\n", err);
+		goto put_clk_table;
+	}
+
+	err = dev_pm_opp_of_add_table(emc->dev);
+	if (err) {
+		dev_err(emc->dev, "failed to add OPP table: %d\n", err);
+		goto put_hw_table;
+	}
+
+	dev_info(emc->dev, "OPP HW ver. 0x%x, current clock rate %lu MHz\n",
+		 hw_version, clk_get_rate(emc->clk) / 1000000);
+
+	/* first dummy rate-set initializes voltage state */
+	err = dev_pm_opp_set_rate(emc->dev, clk_get_rate(emc->clk));
+	if (err) {
+		dev_err(emc->dev, "failed to initialize OPP clock: %d\n", err);
+		goto remove_table;
+	}
+
+	return 0;
+
+remove_table:
+	dev_pm_opp_of_remove_table(emc->dev);
+put_hw_table:
+	dev_pm_opp_put_supported_hw(hw_opp_table);
+put_clk_table:
+	dev_pm_opp_put_clkname(clk_opp_table);
+put_reg_table:
+	if (reg_opp_table)
+		dev_pm_opp_put_regulators(reg_opp_table);
+
+	return err;
+}
Signature · 1 line
viresh
author maintainer
#24 lore raw

Re: [PATCH v8 09/26] memory: tegra30: Support interconnect framework

11.11.2020 08:53, Viresh Kumar пишет:

Quoted 7 lines
Quoted 7 lines
drivers/memory/tegra/tegra30-emc.c +7 -0
+static int tegra_emc_opp_table_init(struct tegra_emc *emc)
+{
+	struct opp_table *reg_opp_table = NULL, *clk_opp_table, *hw_opp_table;
+	u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
+	const char *rname = "core";
+	int err;
+
Quoted 9 lines

drivers/memory/tegra/tegra30-emc.c static void tegra_emc_debugfs_init(struct tegra_emc *emc) · line 1494

Quoted 5 lines
drivers/memory/tegra/tegra30-emc.c +5 -0
+	/*
+	 * Legacy device-trees don't have OPP table and EMC driver isn't
+	 * useful in this case.
+	 */
+	if (!device_property_present(emc->dev, "operating-points-v2")) {

I don't understand why you want to check this ? The below call to dev_pm_opp_of_add_table() will fail anyway and that should be good for you.

The dev_pm_opp_of_add_table() will produce a error message which doesn't give a clue about what's wrong, i.e. that device-tree needs to be updated.

#25 lore raw

Re: [PATCH v8 09/26] memory: tegra30: Support interconnect framework

Quoted 18 lines · Dmitry Osipenko

11.11.2020 08:53, Viresh Kumar пишет:

Quoted 7 lines
Quoted 7 lines
drivers/memory/tegra/tegra30-emc.c +7 -0
+static int tegra_emc_opp_table_init(struct tegra_emc *emc)
+{
+	struct opp_table *reg_opp_table = NULL, *clk_opp_table, *hw_opp_table;
+	u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
+	const char *rname = "core";
+	int err;
+
Quoted 9 lines

drivers/memory/tegra/tegra30-emc.c static void tegra_emc_debugfs_init(struct tegra_emc *emc) · line 1494

Quoted 5 lines
drivers/memory/tegra/tegra30-emc.c +5 -0
+	/*
+	 * Legacy device-trees don't have OPP table and EMC driver isn't
+	 * useful in this case.
+	 */
+	if (!device_property_present(emc->dev, "operating-points-v2")) {

I don't understand why you want to check this ? The below call to dev_pm_opp_of_add_table() will fail anyway and that should be good for you.

Quoted 2 lines

The dev_pm_opp_of_add_table() will produce a error message which doesn't give a clue about what's wrong, i.e. that device-tree needs to be updated.

If you think that you need to print something more, then you can do that in the error message you print when dev_pm_opp_of_add_table() fails. I would suggest to drop this redundant check here.

Signature · 1 line
viresh
author maintainer
#26 lore raw

Re: [PATCH v8 09/26] memory: tegra30: Support interconnect framework

11.11.2020 09:18, Viresh Kumar пишет:

Quoted 19 lines · Dmitry Osipenko
Quoted 18 lines

11.11.2020 08:53, Viresh Kumar пишет:

Quoted 16 lines

drivers/memory/tegra/tegra30-emc.c static void tegra_emc_debugfs_init(struct tegra_emc *emc) · line 1494

Quoted 12 lines
drivers/memory/tegra/tegra30-emc.c +12 -0
+static int tegra_emc_opp_table_init(struct tegra_emc *emc)
+{
+	struct opp_table *reg_opp_table = NULL, *clk_opp_table, *hw_opp_table;
+	u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
+	const char *rname = "core";
+	int err;
+
+	/*
+	 * Legacy device-trees don't have OPP table and EMC driver isn't
+	 * useful in this case.
+	 */
+	if (!device_property_present(emc->dev, "operating-points-v2")) {

I don't understand why you want to check this ? The below call to dev_pm_opp_of_add_table() will fail anyway and that should be good for you.

Quoted 7 lines
Quoted 2 lines

The dev_pm_opp_of_add_table() will produce a error message which doesn't give a clue about what's wrong, i.e. that device-tree needs to be updated.

If you think that you need to print something more, then you can do that in the error message you print when dev_pm_opp_of_add_table() fails. I would suggest to drop this redundant check here.

Please give the rationale.

#27 lore raw

Re: [PATCH v8 09/26] memory: tegra30: Support interconnect framework

Quoted 11 lines · Dmitry Osipenko

11.11.2020 09:18, Viresh Kumar пишет:

Quoted 8 lines · Dmitry Osipenko
Quoted 2 lines

The dev_pm_opp_of_add_table() will produce a error message which doesn't give a clue about what's wrong, i.e. that device-tree needs to be updated.

If you think that you need to print something more, then you can do that in the error message you print when dev_pm_opp_of_add_table() fails. I would suggest to drop this redundant check here.

Please give the rationale.

The rationale is that the check is already performed by dev_pm_opp_of_add_table() and it isn't going to add *any* benefit to check it again here. Such a check for matching compatible platforms is normally fine, but not for this. This is like open coding part of dev_pm_opp_of_add_table(), and so is redundant. The dev_pm_opp_of_add_table() helper also checks for OPPv1 bindings in the DT (yes you won't be using them on your platform) and so relying on that API is a better thing to do.

As you already said, you just wanted a better print message and so you have added this check. If you really care only about the print message, then you can add a print of your choice in the driver but otherwise this check is not going to benefit you much I am afraid.

Having said that, this isn't the code I maintain. I need to guarantee that the OPP core APIs are used properly and are not misused and so I have a higher say there. But in this case all I can do is _suggest_ and not enforce. And as I said earlier, I suggest to drop this redundant check in order to make your code better and faster.

Thanks.

Signature · 1 line
viresh
author maintainer
#28 lore raw

Re: [PATCH v8 09/26] memory: tegra30: Support interconnect framework

11.11.2020 10:54, Viresh Kumar пишет:

Quoted 27 lines · Dmitry Osipenko
Quoted 11 lines

11.11.2020 09:18, Viresh Kumar пишет:

Quoted 8 lines · Dmitry Osipenko
Quoted 2 lines

The dev_pm_opp_of_add_table() will produce a error message which doesn't give a clue about what's wrong, i.e. that device-tree needs to be updated.

If you think that you need to print something more, then you can do that in the error message you print when dev_pm_opp_of_add_table() fails. I would suggest to drop this redundant check here.

Please give the rationale.

The rationale is that the check is already performed by dev_pm_opp_of_add_table() and it isn't going to add *any* benefit to check it again here. Such a check for matching compatible platforms is normally fine, but not for this. This is like open coding part of dev_pm_opp_of_add_table(), and so is redundant. The dev_pm_opp_of_add_table() helper also checks for OPPv1 bindings in the DT (yes you won't be using them on your platform) and so relying on that API is a better thing to do.

As you already said, you just wanted a better print message and so you have added this check. If you really care only about the print message, then you can add a print of your choice in the driver but otherwise this check is not going to benefit you much I am afraid.

Quoted 8 lines

Having said that, this isn't the code I maintain. I need to guarantee that the OPP core APIs are used properly and are not misused and so I have a higher say there. But in this case all I can do is _suggest_ and not enforce. And as I said earlier, I suggest to drop this redundant check in order to make your code better and faster.

Thanks.

I took a closer look and turned out that devm_pm_opp_of_add_table() silently returns -ENODEV if OPP is missing in a DT. Hence indeed it should be good to drop the property-check. I'll improve it in the next revision, thank you.

author maintainer
#29 lore raw

PATCH v8 10/26 memory: tegra30-emc: Factor out clk initialization

In reply to Dmitry Osipenko

Factor out clk initialization and make it resource-managed. This makes easier to follow code and will help to make further changes cleaner.

---
 drivers/memory/tegra/tegra30-emc.c | 70 ++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 23 deletions(-)
drivers/memory/tegra/tegra30-emc.c +47 -23
index d27df842a667..1df42e212d73 100644
@@ -1550,6 +1550,49 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 	return err;
 }
 
+static void devm_tegra_emc_unset_callback(void *data)
+{
+	tegra20_clk_set_emc_round_callback(NULL, NULL);
+}
+
+static void devm_tegra_emc_unreg_clk_notifier(void *data)
+{
+	struct tegra_emc *emc = data;
+
+	clk_notifier_unregister(emc->clk, &emc->clk_nb);
+}
+
+static int tegra_emc_init_clk(struct tegra_emc *emc)
+{
+	int err;
+
+	tegra20_clk_set_emc_round_callback(emc_round_rate, emc);
+
+	err = devm_add_action_or_reset(emc->dev, devm_tegra_emc_unset_callback,
+				       NULL);
+	if (err)
+		return err;
+
+	emc->clk = devm_clk_get(emc->dev, NULL);
+	if (IS_ERR(emc->clk)) {
+		dev_err(emc->dev, "failed to get EMC clock: %pe\n", emc->clk);
+		return PTR_ERR(emc->clk);
+	}
+
+	err = clk_notifier_register(emc->clk, &emc->clk_nb);
+	if (err) {
+		dev_err(emc->dev, "failed to register clk notifier: %d\n", err);
+		return err;
+	}
+
+	err = devm_add_action_or_reset(emc->dev,
+				       devm_tegra_emc_unreg_clk_notifier, emc);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 static int tegra_emc_probe(struct platform_device *pdev)
 {
 	struct device_node *np;
@@ -1599,25 +1642,13 @@ static int tegra_emc_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	tegra20_clk_set_emc_round_callback(emc_round_rate, emc);
-
-	emc->clk = devm_clk_get(&pdev->dev, "emc");
-	if (IS_ERR(emc->clk)) {
-		err = PTR_ERR(emc->clk);
-		dev_err(&pdev->dev, "failed to get emc clock: %d\n", err);
-		goto unset_cb;
-	}
-
-	err = clk_notifier_register(emc->clk, &emc->clk_nb);
-	if (err) {
-		dev_err(&pdev->dev, "failed to register clk notifier: %d\n",
-			err);
-		goto unset_cb;
-	}
+	err = tegra_emc_init_clk(emc);
+	if (err)
+		return err;
 
 	err = tegra_emc_opp_table_init(emc);
 	if (err)
-		goto unreg_notifier;
+		return err;
 
 	platform_set_drvdata(pdev, emc);
 	tegra_emc_rate_requests_init(emc);
@@ -1632,13 +1663,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
 	try_module_get(THIS_MODULE);
 
 	return 0;
-
-unreg_notifier:
-	clk_notifier_unregister(emc->clk, &emc->clk_nb);
-unset_cb:
-	tegra20_clk_set_emc_round_callback(NULL, NULL);
-
-	return err;
 }
 
 static int tegra_emc_suspend(struct device *dev)
Signature · 1 line
2.29.2
#30 lore raw

Re: [PATCH v8 10/26] memory: tegra30-emc: Factor out clk initialization

Quoted 93 lines · Dmitry Osipenko

Factor out clk initialization and make it resource-managed. This makes easier to follow code and will help to make further changes cleaner.

---
 drivers/memory/tegra/tegra30-emc.c | 70 ++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 23 deletions(-)
drivers/memory/tegra/tegra30-emc.c +47 -16
index d27df842a667..1df42e212d73 100644
@@ -1550,6 +1550,49 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 	return err;
 }
 
+static void devm_tegra_emc_unset_callback(void *data)
+{
+	tegra20_clk_set_emc_round_callback(NULL, NULL);
+}
+
+static void devm_tegra_emc_unreg_clk_notifier(void *data)
+{
+	struct tegra_emc *emc = data;
+
+	clk_notifier_unregister(emc->clk, &emc->clk_nb);
+}
+
+static int tegra_emc_init_clk(struct tegra_emc *emc)
+{
+	int err;
+
+	tegra20_clk_set_emc_round_callback(emc_round_rate, emc);
+
+	err = devm_add_action_or_reset(emc->dev, devm_tegra_emc_unset_callback,
+				       NULL);
+	if (err)
+		return err;
+
+	emc->clk = devm_clk_get(emc->dev, NULL);
+	if (IS_ERR(emc->clk)) {
+		dev_err(emc->dev, "failed to get EMC clock: %pe\n", emc->clk);
+		return PTR_ERR(emc->clk);
+	}
+
+	err = clk_notifier_register(emc->clk, &emc->clk_nb);
+	if (err) {
+		dev_err(emc->dev, "failed to register clk notifier: %d\n", err);
+		return err;
+	}
+
+	err = devm_add_action_or_reset(emc->dev,
+				       devm_tegra_emc_unreg_clk_notifier, emc);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 static int tegra_emc_probe(struct platform_device *pdev)
 {
 	struct device_node *np;
@@ -1599,25 +1642,13 @@ static int tegra_emc_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	tegra20_clk_set_emc_round_callback(emc_round_rate, emc);
-
-	emc->clk = devm_clk_get(&pdev->dev, "emc");
-	if (IS_ERR(emc->clk)) {
-		err = PTR_ERR(emc->clk);
-		dev_err(&pdev->dev, "failed to get emc clock: %d\n", err);
-		goto unset_cb;
-	}
-
-	err = clk_notifier_register(emc->clk, &emc->clk_nb);
-	if (err) {
-		dev_err(&pdev->dev, "failed to register clk notifier: %d\n",
-			err);
-		goto unset_cb;
-	}
+	err = tegra_emc_init_clk(emc);
+	if (err)
+		return err;
 
 	err = tegra_emc_opp_table_init(emc);
 	if (err)
-		goto unreg_notifier;
+		return err;
 
 	platform_set_drvdata(pdev, emc);
 	tegra_emc_rate_requests_init(emc);

drivers/memory/tegra/tegra30-emc.c static int tegra_emc_probe(struct platform_device *pdev) · line 1637

Quoted 6 lines
drivers/memory/tegra/tegra30-emc.c +0 -3
@@ -1632,13 +1663,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
 	try_module_get(THIS_MODULE);
 
 	return 0;
-
-unreg_notifier:
-	clk_notifier_unregister(emc->clk, &emc->clk_nb);

You added this code in patch #8, so adding-and-removing a piece of code is a nice hint that this patch should be before. Don't add new code which later you simplify. Move all bugfixes and all simplifications to beginning of patchset.

That's quite similar case to v6 where you put bugfixes in the middle of patchset.

Best regards,
Krzysztof
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

#31 lore raw

Re: [PATCH v8 10/26] memory: tegra30-emc: Factor out clk initialization

Quoted 96 lines · Krzysztof Kozlowski
Quoted 95 lines · Dmitry Osipenko

Factor out clk initialization and make it resource-managed. This makes easier to follow code and will help to make further changes cleaner.

---
 drivers/memory/tegra/tegra30-emc.c | 70 ++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 23 deletions(-)
drivers/memory/tegra/tegra30-emc.c +47 -16
index d27df842a667..1df42e212d73 100644
@@ -1550,6 +1550,49 @@ static int tegra_emc_opp_table_init(struct tegra_emc *emc)
 	return err;
 }
 
+static void devm_tegra_emc_unset_callback(void *data)
+{
+	tegra20_clk_set_emc_round_callback(NULL, NULL);
+}
+
+static void devm_tegra_emc_unreg_clk_notifier(void *data)
+{
+	struct tegra_emc *emc = data;
+
+	clk_notifier_unregister(emc->clk, &emc->clk_nb);
+}
+
+static int tegra_emc_init_clk(struct tegra_emc *emc)
+{
+	int err;
+
+	tegra20_clk_set_emc_round_callback(emc_round_rate, emc);
+
+	err = devm_add_action_or_reset(emc->dev, devm_tegra_emc_unset_callback,
+				       NULL);
+	if (err)
+		return err;
+
+	emc->clk = devm_clk_get(emc->dev, NULL);
+	if (IS_ERR(emc->clk)) {
+		dev_err(emc->dev, "failed to get EMC clock: %pe\n", emc->clk);
+		return PTR_ERR(emc->clk);
+	}
+
+	err = clk_notifier_register(emc->clk, &emc->clk_nb);
+	if (err) {
+		dev_err(emc->dev, "failed to register clk notifier: %d\n", err);
+		return err;
+	}
+
+	err = devm_add_action_or_reset(emc->dev,
+				       devm_tegra_emc_unreg_clk_notifier, emc);
+	if (err)
+		return err;
+
+	return 0;
+}
+
 static int tegra_emc_probe(struct platform_device *pdev)
 {
 	struct device_node *np;
@@ -1599,25 +1642,13 @@ static int tegra_emc_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	tegra20_clk_set_emc_round_callback(emc_round_rate, emc);
-
-	emc->clk = devm_clk_get(&pdev->dev, "emc");
-	if (IS_ERR(emc->clk)) {
-		err = PTR_ERR(emc->clk);
-		dev_err(&pdev->dev, "failed to get emc clock: %d\n", err);
-		goto unset_cb;
-	}
-
-	err = clk_notifier_register(emc->clk, &emc->clk_nb);
-	if (err) {
-		dev_err(&pdev->dev, "failed to register clk notifier: %d\n",
-			err);
-		goto unset_cb;
-	}
+	err = tegra_emc_init_clk(emc);
+	if (err)
+		return err;
 
 	err = tegra_emc_opp_table_init(emc);
 	if (err)
-		goto unreg_notifier;
+		return err;
 
 	platform_set_drvdata(pdev, emc);
 	tegra_emc_rate_requests_init(emc);
@@ -1632,13 +1663,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
 	try_module_get(THIS_MODULE);
 
Quoted 6 lines

drivers/memory/tegra/tegra30-emc.c static int tegra_emc_probe(struct platform_device *pdev) · line 1637

Quoted 4 lines
drivers/memory/tegra/tegra30-emc.c +0 -3
 	return 0;
-
-unreg_notifier:
-	clk_notifier_unregister(emc->clk, &emc->clk_nb);

You added this code in patch #8, so adding-and-removing a piece of code

Correction: you added this in patch #9.

Best regards,
Krzysztof

Quoted 7 lines

is a nice hint that this patch should be before. Don't add new code which later you simplify. Move all bugfixes and all simplifications to beginning of patchset.

That's quite similar case to v6 where you put bugfixes in the middle of patchset.

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author maintainer
#32 lore raw

Re: [PATCH v8 10/26] memory: tegra30-emc: Factor out clk initialization

11.11.2020 11:52, Krzysztof Kozlowski пишет:

Quoted 14 lines
Quoted 1 line

You added this code in patch #8, so adding-and-removing a piece of code

Correction: you added this in patch #9.

Best regards,
Krzysztof

Quoted 7 lines

is a nice hint that this patch should be before. Don't add new code which later you simplify. Move all bugfixes and all simplifications to beginning of patchset.

That's quite similar case to v6 where you put bugfixes in the middle of patchset.

Indeed, I squashed a similar change in the T124 patch #13, but forgot to squash it for the T30.

author maintainer
#33 lore raw

PATCH v8 11/26 memory: tegra124-emc: Make driver modular

In reply to Dmitry Osipenko

Add modularization support to the Tegra124 EMC driver, which now can be compiled as a loadable kernel module.

Note that EMC clock must be registered at clk-init time, otherwise PLLM will be disabled as unused clock at boot time if EMC driver is compiled as a module. Hence add a prepare/complete callbacks. similarly to what is done for the Tegra20/30 EMC drivers.

---
 drivers/clk/tegra/Makefile           |  3 +-
 drivers/clk/tegra/clk-tegra124-emc.c | 41 ++++++++++++++++++++++++----
 drivers/clk/tegra/clk-tegra124.c     | 27 ++++++++++++++++--
 drivers/clk/tegra/clk.h              | 16 +++--------
 drivers/memory/tegra/Kconfig         |  2 +-
 drivers/memory/tegra/tegra124-emc.c  | 31 ++++++++++++++-------
 include/linux/clk/tegra.h            |  8 ++++++
 include/soc/tegra/emc.h              | 16 -----------
 8 files changed, 96 insertions(+), 48 deletions(-)
 delete mode 100644 include/soc/tegra/emc.h
drivers/clk/tegra/Makefile +2 -1
index eec2313fd37e..53b76133e905 100644
@@ -22,7 +22,8 @@ obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= clk-tegra20-emc.o
 obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= clk-tegra114.o
 obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124.o
 obj-$(CONFIG_TEGRA_CLK_DFLL)		+= clk-tegra124-dfll-fcpu.o
-obj-$(CONFIG_TEGRA124_EMC)		+= clk-tegra124-emc.o
+obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124-emc.o
+obj-$(CONFIG_ARCH_TEGRA_132_SOC)	+= clk-tegra124-emc.o
 obj-$(CONFIG_ARCH_TEGRA_132_SOC)	+= clk-tegra124.o
 obj-y					+= cvb.o
 obj-$(CONFIG_ARCH_TEGRA_210_SOC)	+= clk-tegra210.o
drivers/clk/tegra/clk-tegra124-emc.c +36 -5

87 lines not on this page · raw message

drivers/clk/tegra/clk-tegra124.c +24 -3
index e931319dcc9d..b4f2ae4066a6 100644
@@ -929,6 +929,7 @@ static struct tegra_clk tegra124_clks[tegra_clk_max] __initdata = {
 	[tegra_clk_audio4_mux] = { .dt_id = TEGRA124_CLK_AUDIO4_MUX, .present = true },
 	[tegra_clk_spdif_mux] = { .dt_id = TEGRA124_CLK_SPDIF_MUX, .present = true },
 	[tegra_clk_cec] = { .dt_id = TEGRA124_CLK_CEC, .present = true },
+	[tegra_clk_emc] = { .dt_id = TEGRA124_CLK_EMC, .present = false },
 };
 
 static struct tegra_devclk devclks[] __initdata = {
@@ -1500,6 +1501,26 @@ static void __init tegra124_132_clock_init_pre(struct device_node *np)
 	writel(plld_base, clk_base + PLLD_BASE);
 }
 
+static struct clk *tegra124_clk_src_onecell_get(struct of_phandle_args *clkspec,
+						void *data)
+{
+	struct clk_hw *hw;
+	struct clk *clk;
+
+	clk = of_clk_src_onecell_get(clkspec, data);
+	if (IS_ERR(clk))
+		return clk;
+
+	hw = __clk_get_hw(clk);
+
+	if (clkspec->args[0] == TEGRA124_CLK_EMC) {
+		if (!tegra124_clk_emc_driver_available(hw))
+			return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	return clk;
+}
+
 /**
  * tegra124_132_clock_init_post - clock initialization postamble for T124/T132
  * @np: struct device_node * of the DT node for the SoC CAR IP block
@@ -1516,10 +1537,10 @@ static void __init tegra124_132_clock_init_post(struct device_node *np)
 				  &pll_x_params);
 	tegra_init_special_resets(1, tegra124_reset_assert,
 				  tegra124_reset_deassert);
-	tegra_add_of_provider(np, of_clk_src_onecell_get);
+	tegra_add_of_provider(np, tegra124_clk_src_onecell_get);
 
-	clks[TEGRA124_CLK_EMC] = tegra_clk_register_emc(clk_base, np,
-							&emc_lock);
+	clks[TEGRA124_CLK_EMC] = tegra124_clk_register_emc(clk_base, np,
+							   &emc_lock);
 
 	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
 
drivers/clk/tegra/clk.h +4 -12
index 6b565f6b5f66..2da7c93c1a6c 100644
@@ -881,18 +881,6 @@ void tegra_super_clk_gen5_init(void __iomem *clk_base,
 			void __iomem *pmc_base, struct tegra_clk *tegra_clks,
 			struct tegra_clk_pll_params *pll_params);
 
-#ifdef CONFIG_TEGRA124_EMC
-struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np,
-				   spinlock_t *lock);
-#else
-static inline struct clk *tegra_clk_register_emc(void __iomem *base,
-						 struct device_node *np,
-						 spinlock_t *lock)
-{
-	return NULL;
-}
-#endif
-
 void tegra114_clock_tune_cpu_trimmers_high(void);
 void tegra114_clock_tune_cpu_trimmers_low(void);
 void tegra114_clock_tune_cpu_trimmers_init(void);
@@ -922,6 +910,10 @@ void tegra_clk_periph_resume(void);
 bool tegra20_clk_emc_driver_available(struct clk_hw *emc_hw);
 struct clk *tegra20_clk_register_emc(void __iomem *ioaddr, bool low_jitter);
 
+struct clk *tegra124_clk_register_emc(void __iomem *base, struct device_node *np,
+				      spinlock_t *lock);
+bool tegra124_clk_emc_driver_available(struct clk_hw *emc_hw);
+
 struct clk *tegra210_clk_register_emc(struct device_node *np,
 				      void __iomem *regs);
 
drivers/memory/tegra/Kconfig +1 -1
index ca7077a06f4c..6265aa39674f 100644
@@ -32,7 +32,7 @@ config TEGRA30_EMC
 	  external memory.
 
 config TEGRA124_EMC
-	bool "NVIDIA Tegra124 External Memory Controller driver"
+	tristate "NVIDIA Tegra124 External Memory Controller driver"
 	default y
 	depends on TEGRA_MC && ARCH_TEGRA_124_SOC
 	help
drivers/memory/tegra/tegra124-emc.c +21 -10
index ee8ee39e98ed..edfbf6d6d357 100644
@@ -9,16 +9,17 @@
 #include <linux/clk-provider.h>
 #include <linux/clk.h>
 #include <linux/clkdev.h>
+#include <linux/clk/tegra.h>
 #include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/io.h>
+#include <linux/module.h>
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/sort.h>
 #include <linux/string.h>
 
-#include <soc/tegra/emc.h>
 #include <soc/tegra/fuse.h>
 #include <soc/tegra/mc.h>
 
@@ -562,8 +563,8 @@ static struct emc_timing *tegra_emc_find_timing(struct tegra_emc *emc,
 	return timing;
 }
 
-int tegra_emc_prepare_timing_change(struct tegra_emc *emc,
-				    unsigned long rate)
+static int tegra_emc_prepare_timing_change(struct tegra_emc *emc,
+					   unsigned long rate)
 {
 	struct emc_timing *timing = tegra_emc_find_timing(emc, rate);
 	struct emc_timing *last = &emc->last_timing;
@@ -790,8 +791,8 @@ int tegra_emc_prepare_timing_change(struct tegra_emc *emc,
 	return 0;
 }
 
-void tegra_emc_complete_timing_change(struct tegra_emc *emc,
-				      unsigned long rate)
+static void tegra_emc_complete_timing_change(struct tegra_emc *emc,
+					     unsigned long rate)
 {
 	struct emc_timing *timing = tegra_emc_find_timing(emc, rate);
 	struct emc_timing *last = &emc->last_timing;
@@ -987,6 +988,7 @@ static const struct of_device_id tegra_emc_of_match[] = {
 	{ .compatible = "nvidia,tegra132-emc" },
 	{}
 };
+MODULE_DEVICE_TABLE(of, tegra_emc_of_match);
 
 static struct device_node *
 tegra_emc_find_node_by_ram_code(struct device_node *node, u32 ram_code)
@@ -1226,9 +1228,19 @@ static int tegra_emc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, emc);
 
+	tegra124_clk_set_emc_callbacks(tegra_emc_prepare_timing_change,
+				       tegra_emc_complete_timing_change);
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS))
 		emc_debugfs_init(&pdev->dev, emc);
 
+	/*
+	 * Don't allow the kernel module to be unloaded. Unloading adds some
+	 * extra complexity which doesn't really worth the effort in a case of
+	 * this driver.
+	 */
+	try_module_get(THIS_MODULE);
+
 	return 0;
 };
 
@@ -1240,9 +1252,8 @@ static struct platform_driver tegra_emc_driver = {
 		.suppress_bind_attrs = true,
 	},
 };
+module_platform_driver(tegra_emc_driver);
 
-static int tegra_emc_init(void)
-{
-	return platform_driver_register(&tegra_emc_driver);
-}
-subsys_initcall(tegra_emc_init);
+MODULE_AUTHOR("Mikko Perttunen <[email protected]>");
+MODULE_DESCRIPTION("NVIDIA Tegra124 EMC driver");
+MODULE_LICENSE("GPL v2");
include/linux/clk/tegra.h +8 -0
index 3f01d43f0598..eb016fc9cc0b 100644
@@ -136,6 +136,7 @@ extern void tegra210_clk_emc_dll_update_setting(u32 emc_dll_src_value);
 extern void tegra210_clk_emc_update_setting(u32 emc_src_value);
 
 struct clk;
+struct tegra_emc;
 
 typedef long (tegra20_clk_emc_round_cb)(unsigned long rate,
 					unsigned long min_rate,
@@ -146,6 +147,13 @@ void tegra20_clk_set_emc_round_callback(tegra20_clk_emc_round_cb *round_cb,
 					void *cb_arg);
 int tegra20_clk_prepare_emc_mc_same_freq(struct clk *emc_clk, bool same);
 
+typedef int (tegra124_emc_prepare_timing_change_cb)(struct tegra_emc *emc,
+						    unsigned long rate);
+typedef void (tegra124_emc_complete_timing_change_cb)(struct tegra_emc *emc,
+						      unsigned long rate);
+void tegra124_clk_set_emc_callbacks(tegra124_emc_prepare_timing_change_cb *prep_cb,
+				    tegra124_emc_complete_timing_change_cb *complete_cb);
+
 struct tegra210_clk_emc_config {
 	unsigned long rate;
 	bool same_freq;
include/soc/tegra/emc.h deleted +0 -16
deleted file mode 100644
index 05199a97ccf4..000000000000
@@ -1,16 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (c) 2014 NVIDIA Corporation. All rights reserved.
- */
-
-#ifndef __SOC_TEGRA_EMC_H__
-#define __SOC_TEGRA_EMC_H__
-
-struct tegra_emc;
-
-int tegra_emc_prepare_timing_change(struct tegra_emc *emc,
-				    unsigned long rate);
-void tegra_emc_complete_timing_change(struct tegra_emc *emc,
-				      unsigned long rate);
-
-#endif /* __SOC_TEGRA_EMC_H__ */
Signature · 1 line
2.29.2
#34 lore raw

Re: [PATCH v8 11/26] memory: tegra124-emc: Make driver modular

Quoted 27 lines · Dmitry Osipenko

Add modularization support to the Tegra124 EMC driver, which now can be compiled as a loadable kernel module.

Note that EMC clock must be registered at clk-init time, otherwise PLLM will be disabled as unused clock at boot time if EMC driver is compiled as a module. Hence add a prepare/complete callbacks. similarly to what is done for the Tegra20/30 EMC drivers.

---
 drivers/clk/tegra/Makefile           |  3 +-
 drivers/clk/tegra/clk-tegra124-emc.c | 41 ++++++++++++++++++++++++----
 drivers/clk/tegra/clk-tegra124.c     | 27 ++++++++++++++++--
 drivers/clk/tegra/clk.h              | 16 +++--------
 drivers/memory/tegra/Kconfig         |  2 +-
 drivers/memory/tegra/tegra124-emc.c  | 31 ++++++++++++++-------
 include/linux/clk/tegra.h            |  8 ++++++
 include/soc/tegra/emc.h              | 16 -----------
 8 files changed, 96 insertions(+), 48 deletions(-)
 delete mode 100644 include/soc/tegra/emc.h
drivers/clk/tegra/Makefile +0 -0
index eec2313fd37e..53b76133e905 100644

drivers/clk/tegra/Makefile obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += clk-tegra20-emc.o · line 26

Quoted 6 lines
drivers/clk/tegra/Makefile +2 -1
@@ -22,7 +22,8 @@ obj-$(CONFIG_ARCH_TEGRA_3x_SOC)		+= clk-tegra20-emc.o
 obj-$(CONFIG_ARCH_TEGRA_114_SOC)	+= clk-tegra114.o
 obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124.o
 obj-$(CONFIG_TEGRA_CLK_DFLL)		+= clk-tegra124-dfll-fcpu.o
-obj-$(CONFIG_TEGRA124_EMC)		+= clk-tegra124-emc.o
+obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124-emc.o
+obj-$(CONFIG_ARCH_TEGRA_132_SOC)	+= clk-tegra124-emc.o

How is it related to modularization? It looks like different issue is fixed here.

Quoted 164 lines
drivers/clk/tegra/Makefile +0 -0
 obj-$(CONFIG_ARCH_TEGRA_132_SOC)	+= clk-tegra124.o
 obj-y					+= cvb.o
 obj-$(CONFIG_ARCH_TEGRA_210_SOC)	+= clk-tegra210.o
drivers/clk/tegra/clk-tegra124-emc.c +36 -5
index 745f9faa98d8..bdf6f4a51617 100644
@@ -11,7 +11,9 @@
 #include <linux/clk-provider.h>
 #include <linux/clk.h>
 #include <linux/clkdev.h>
+#include <linux/clk/tegra.h>
 #include <linux/delay.h>
+#include <linux/export.h>
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
@@ -21,7 +23,6 @@
 #include <linux/string.h>
 
 #include <soc/tegra/fuse.h>
-#include <soc/tegra/emc.h>
 
 #include "clk.h"
 
@@ -80,6 +81,9 @@ struct tegra_clk_emc {
 	int num_timings;
 	struct emc_timing *timings;
 	spinlock_t *lock;
+
+	tegra124_emc_prepare_timing_change_cb *prepare_timing_change;
+	tegra124_emc_complete_timing_change_cb *complete_timing_change;
 };
 
 /* Common clock framework callback implementations */
@@ -176,6 +180,9 @@ static struct tegra_emc *emc_ensure_emc_driver(struct tegra_clk_emc *tegra)
 	if (tegra->emc)
 		return tegra->emc;
 
+	if (!tegra->prepare_timing_change || !tegra->complete_timing_change)
+		return NULL;
+
 	if (!tegra->emc_node)
 		return NULL;
 
@@ -241,7 +248,7 @@ static int emc_set_timing(struct tegra_clk_emc *tegra,
 
 	div = timing->parent_rate / (timing->rate / 2) - 2;
 
-	err = tegra_emc_prepare_timing_change(emc, timing->rate);
+	err = tegra->prepare_timing_change(emc, timing->rate);
 	if (err)
 		return err;
 
@@ -259,7 +266,7 @@ static int emc_set_timing(struct tegra_clk_emc *tegra,
 
 	spin_unlock_irqrestore(tegra->lock, flags);
 
-	tegra_emc_complete_timing_change(emc, timing->rate);
+	tegra->complete_timing_change(emc, timing->rate);
 
 	clk_hw_reparent(&tegra->hw, __clk_get_hw(timing->parent));
 	clk_disable_unprepare(tegra->prev_parent);
@@ -473,8 +480,8 @@ static const struct clk_ops tegra_clk_emc_ops = {
 	.get_parent = emc_get_parent,
 };
 
-struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np,
-				   spinlock_t *lock)
+struct clk *tegra124_clk_register_emc(void __iomem *base, struct device_node *np,
+				      spinlock_t *lock)
 {
 	struct tegra_clk_emc *tegra;
 	struct clk_init_data init;
@@ -538,3 +545,27 @@ struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np,
 
 	return clk;
 };
+
+void tegra124_clk_set_emc_callbacks(tegra124_emc_prepare_timing_change_cb *prep_cb,
+				    tegra124_emc_complete_timing_change_cb *complete_cb)
+{
+	struct clk *clk = __clk_lookup("emc");
+	struct tegra_clk_emc *tegra;
+	struct clk_hw *hw;
+
+	if (clk) {
+		hw = __clk_get_hw(clk);
+		tegra = container_of(hw, struct tegra_clk_emc, hw);
+
+		tegra->prepare_timing_change = prep_cb;
+		tegra->complete_timing_change = complete_cb;
+	}
+}
+EXPORT_SYMBOL_GPL(tegra124_clk_set_emc_callbacks);
+
+bool tegra124_clk_emc_driver_available(struct clk_hw *hw)
+{
+	struct tegra_clk_emc *tegra = container_of(hw, struct tegra_clk_emc, hw);
+
+	return tegra->prepare_timing_change && tegra->complete_timing_change;
+}
drivers/clk/tegra/clk-tegra124.c +24 -3
index e931319dcc9d..b4f2ae4066a6 100644
@@ -929,6 +929,7 @@ static struct tegra_clk tegra124_clks[tegra_clk_max] __initdata = {
 	[tegra_clk_audio4_mux] = { .dt_id = TEGRA124_CLK_AUDIO4_MUX, .present = true },
 	[tegra_clk_spdif_mux] = { .dt_id = TEGRA124_CLK_SPDIF_MUX, .present = true },
 	[tegra_clk_cec] = { .dt_id = TEGRA124_CLK_CEC, .present = true },
+	[tegra_clk_emc] = { .dt_id = TEGRA124_CLK_EMC, .present = false },
 };
 
 static struct tegra_devclk devclks[] __initdata = {
@@ -1500,6 +1501,26 @@ static void __init tegra124_132_clock_init_pre(struct device_node *np)
 	writel(plld_base, clk_base + PLLD_BASE);
 }
 
+static struct clk *tegra124_clk_src_onecell_get(struct of_phandle_args *clkspec,
+						void *data)
+{
+	struct clk_hw *hw;
+	struct clk *clk;
+
+	clk = of_clk_src_onecell_get(clkspec, data);
+	if (IS_ERR(clk))
+		return clk;
+
+	hw = __clk_get_hw(clk);
+
+	if (clkspec->args[0] == TEGRA124_CLK_EMC) {
+		if (!tegra124_clk_emc_driver_available(hw))
+			return ERR_PTR(-EPROBE_DEFER);
+	}
+
+	return clk;
+}
+
 /**
  * tegra124_132_clock_init_post - clock initialization postamble for T124/T132
  * @np: struct device_node * of the DT node for the SoC CAR IP block
@@ -1516,10 +1537,10 @@ static void __init tegra124_132_clock_init_post(struct device_node *np)
 				  &pll_x_params);
 	tegra_init_special_resets(1, tegra124_reset_assert,
 				  tegra124_reset_deassert);
-	tegra_add_of_provider(np, of_clk_src_onecell_get);
+	tegra_add_of_provider(np, tegra124_clk_src_onecell_get);
 
-	clks[TEGRA124_CLK_EMC] = tegra_clk_register_emc(clk_base, np,
-							&emc_lock);
+	clks[TEGRA124_CLK_EMC] = tegra124_clk_register_emc(clk_base, np,
+							   &emc_lock);
 
 	tegra_register_devclks(devclks, ARRAY_SIZE(devclks));
 
drivers/clk/tegra/clk.h +0 -1
index 6b565f6b5f66..2da7c93c1a6c 100644
@@ -881,18 +881,6 @@ void tegra_super_clk_gen5_init(void __iomem *clk_base,
 			void __iomem *pmc_base, struct tegra_clk *tegra_clks,
 			struct tegra_clk_pll_params *pll_params);
 
-#ifdef CONFIG_TEGRA124_EMC

drivers/clk/tegra/clk.h void tegra_super_clk_gen5_init(void __iomem *clk_base, · line 894

Quoted 10 lines
drivers/clk/tegra/clk.h +0 -10
-struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np,
-				   spinlock_t *lock);
-#else
-static inline struct clk *tegra_clk_register_emc(void __iomem *base,
-						 struct device_node *np,
-						 spinlock_t *lock)
-{
-	return NULL;
-}
-#endif

Why clock changes are so tightly coupled with making an EMC driver modular? Usually this should be a separate change - you adjust any dependencies to accept late or deferred probing, exported symbols, loosen the coupling between drivers, etc. and then you convert something to module.

Best regards,
Krzysztof
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author maintainer
#35 lore raw

Re: [PATCH v8 11/26] memory: tegra124-emc: Make driver modular

11.11.2020 12:04, Krzysztof Kozlowski пишет:

Quoted 5 lines

drivers/clk/tegra/Makefile obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += clk-tegra20-emc.o · line 26

Quoted 3 lines
drivers/clk/tegra/Makefile +2 -1
-obj-$(CONFIG_TEGRA124_EMC)		+= clk-tegra124-emc.o
+obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124-emc.o
+obj-$(CONFIG_ARCH_TEGRA_132_SOC)	+= clk-tegra124-emc.o

How is it related to modularization? It looks like different issue is fixed here.

The CONFIG_TEGRA124_EMC now could be 'm', while the clock code must be built-in. The TEGRA124 EMC driver is used by T124 and T132 SoCs.

...

Quoted 8 lines
Quoted 8 lines
drivers/clk/tegra/clk.h +0 -0
index 6b565f6b5f66..2da7c93c1a6c 100644
@@ -881,18 +881,6 @@ void tegra_super_clk_gen5_init(void __iomem *clk_base,
 			void __iomem *pmc_base, struct tegra_clk *tegra_clks,
 			struct tegra_clk_pll_params *pll_params);
 
Quoted 16 lines

drivers/clk/tegra/clk.h void tegra_super_clk_gen5_init(void __iomem *clk_base, · line 894

Quoted 11 lines
drivers/clk/tegra/clk.h +0 -11
-#ifdef CONFIG_TEGRA124_EMC
-struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np,
-				   spinlock_t *lock);
-#else
-static inline struct clk *tegra_clk_register_emc(void __iomem *base,
-						 struct device_node *np,
-						 spinlock_t *lock)
-{
-	return NULL;
-}
-#endif

Why clock changes are so tightly coupled with making an EMC driver modular? Usually this should be a separate change - you adjust any dependencies to accept late or deferred probing, exported symbols, loosen the coupling between drivers, etc. and then you convert something to module.

Because the clock and EMC driver were not separated from each other previously. The clock part can't be modularized easily and probably shouldn't.

I'm not sure whether it's actually possible to split this patch without taking a closer a look.

I'm also doubt that it would really worth the effort for a 100 lines of a changed code.

#36 lore raw

Re: [PATCH v8 11/26] memory: tegra124-emc: Make driver modular

Quoted 9 lines · Dmitry Osipenko

11.11.2020 12:04, Krzysztof Kozlowski пишет:

Quoted 5 lines

drivers/clk/tegra/Makefile obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += clk-tegra20-emc.o · line 26

Quoted 3 lines
drivers/clk/tegra/Makefile +2 -1
-obj-$(CONFIG_TEGRA124_EMC)		+= clk-tegra124-emc.o
+obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124-emc.o
+obj-$(CONFIG_ARCH_TEGRA_132_SOC)	+= clk-tegra124-emc.o

How is it related to modularization? It looks like different issue is fixed here.

The CONFIG_TEGRA124_EMC now could be 'm', while the clock code must be built-in. The TEGRA124 EMC driver is used by T124 and T132 SoCs.\

Mhmm, the CONFIG_TEGRA124_EMC depends on ARCH_TEGRA_124_SOC so on the config !ARCH_TEGRA_124_SOC && ARCH_TEGRA_132_SOC this was not selected. Now it will be selected.

Quoted 31 lines

...

Quoted 8 lines
Quoted 8 lines
drivers/clk/tegra/clk.h +0 -0
index 6b565f6b5f66..2da7c93c1a6c 100644
@@ -881,18 +881,6 @@ void tegra_super_clk_gen5_init(void __iomem *clk_base,
 			void __iomem *pmc_base, struct tegra_clk *tegra_clks,
 			struct tegra_clk_pll_params *pll_params);
 
Quoted 16 lines

drivers/clk/tegra/clk.h void tegra_super_clk_gen5_init(void __iomem *clk_base, · line 894

Quoted 11 lines
drivers/clk/tegra/clk.h +0 -11
-#ifdef CONFIG_TEGRA124_EMC
-struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np,
-				   spinlock_t *lock);
-#else
-static inline struct clk *tegra_clk_register_emc(void __iomem *base,
-						 struct device_node *np,
-						 spinlock_t *lock)
-{
-	return NULL;
-}
-#endif

Why clock changes are so tightly coupled with making an EMC driver modular? Usually this should be a separate change - you adjust any dependencies to accept late or deferred probing, exported symbols, loosen the coupling between drivers, etc. and then you convert something to module.

Because the clock and EMC driver were not separated from each other previously. The clock part can't be modularized easily and probably shouldn't.

Quoted 5 lines

I'm not sure whether it's actually possible to split this patch without taking a closer a look.

I'm also doubt that it would really worth the effort for a 100 lines of a changed code.

Indeed, the clk and emc drivers are so tightly coupled that anyway it would be big patch to change the interfaces followed up a small one making emc module. OK.

Best regards,
Krzysztof

_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel

author maintainer
#37 lore raw

Re: [PATCH v8 11/26] memory: tegra124-emc: Make driver modular

11.11.2020 12:26, Krzysztof Kozlowski пишет:

Quoted 6 lines
Quoted 6 lines

11.11.2020 12:04, Krzysztof Kozlowski пишет:

Quoted 5 lines

drivers/clk/tegra/Makefile obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += clk-tegra20-emc.o · line 26

Quoted 3 lines
drivers/clk/tegra/Makefile +2 -1
-obj-$(CONFIG_TEGRA124_EMC)		+= clk-tegra124-emc.o
+obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124-emc.o
+obj-$(CONFIG_ARCH_TEGRA_132_SOC)	+= clk-tegra124-emc.o

How is it related to modularization? It looks like different issue is fixed here.

Quoted 6 lines
Quoted 2 lines

The CONFIG_TEGRA124_EMC now could be 'm', while the clock code must be built-in. The TEGRA124 EMC driver is used by T124 and T132 SoCs.\

Mhmm, the CONFIG_TEGRA124_EMC depends on ARCH_TEGRA_124_SOC so on the config !ARCH_TEGRA_124_SOC && ARCH_TEGRA_132_SOC this was not selected. Now it will be selected.

The driver isn't exposed on ARM64 probably because nobody bothered to do it so far. But it's not also the memory driver which depends on clk-tegra124-emc.o, it's also the main clk/tegra/clk-tegra124.c driver itself, which is also used by both T124/132.

author maintainer
#38 lore raw

Re: [PATCH v8 11/26] memory: tegra124-emc: Make driver modular

11.11.2020 13:25, Dmitry Osipenko пишет:

Quoted 14 lines

11.11.2020 12:26, Krzysztof Kozlowski пишет:

Quoted 6 lines
Quoted 6 lines

11.11.2020 12:04, Krzysztof Kozlowski пишет:

Quoted 5 lines

drivers/clk/tegra/Makefile obj-$(CONFIG_ARCH_TEGRA_3x_SOC) += clk-tegra20-emc.o · line 26

Quoted 3 lines
drivers/clk/tegra/Makefile +2 -1
-obj-$(CONFIG_TEGRA124_EMC)		+= clk-tegra124-emc.o
+obj-$(CONFIG_ARCH_TEGRA_124_SOC)	+= clk-tegra124-emc.o
+obj-$(CONFIG_ARCH_TEGRA_132_SOC)	+= clk-tegra124-emc.o

How is it related to modularization? It looks like different issue is fixed here.

Quoted 6 lines
Quoted 2 lines

The CONFIG_TEGRA124_EMC now could be 'm', while the clock code must be built-in. The TEGRA124 EMC driver is used by T124 and T132 SoCs.\

Mhmm, the CONFIG_TEGRA124_EMC depends on ARCH_TEGRA_124_SOC so on the config !ARCH_TEGRA_124_SOC && ARCH_TEGRA_132_SOC this was not selected. Now it will be selected.

Quoted 5 lines

The driver isn't exposed on ARM64 probably because nobody bothered to do it so far. But it's not also the memory driver which depends on clk-tegra124-emc.o, it's also the main clk/tegra/clk-tegra124.c driver itself, which is also used by both T124/132.

Actually, it shouldn't be difficult to keep the old condition for clk-tegra124-emc.o by slightly improving the Kconfig. I'll do it in the next revision.

author maintainer
#39 lore raw

PATCH v8 12/26 memory: tegra124-emc: Continue probing if timings are missing in device-tree

In reply to Dmitry Osipenko

EMC driver will become mandatory after turning it into interconnect provider because interconnect users, like display controller driver, will fail to probe using newer device-trees that have interconnect properties. Thus make EMC driver to probe even if timings are missing in device-tree.

---
 drivers/memory/tegra/tegra124-emc.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)
drivers/memory/tegra/tegra124-emc.c +9 -17
index edfbf6d6d357..8fb8c1af25c9 100644
@@ -1201,23 +1201,15 @@ static int tegra_emc_probe(struct platform_device *pdev)
 	ram_code = tegra_read_ram_code();
 
 	np = tegra_emc_find_node_by_ram_code(pdev->dev.of_node, ram_code);
-	if (!np) {
-		dev_err(&pdev->dev,
-			"no memory timings for RAM code %u found in DT\n",
-			ram_code);
-		return -ENOENT;
-	}
-
-	err = tegra_emc_load_timings_from_dt(emc, np);
-	of_node_put(np);
-	if (err)
-		return err;
-
-	if (emc->num_timings == 0) {
-		dev_err(&pdev->dev,
-			"no memory timings for RAM code %u registered\n",
-			ram_code);
-		return -ENOENT;
+	if (np) {
+		err = tegra_emc_load_timings_from_dt(emc, np);
+		of_node_put(np);
+		if (err)
+			return err;
+	} else {
+		dev_info(&pdev->dev,
+			 "no memory timings for RAM code %u found in DT\n",
+			 ram_code);
 	}
 
 	err = emc_init(emc);
Signature · 1 line
2.29.2
author maintainer
#40 lore raw

PATCH v8 13/26 memory: tegra124: Support interconnect framework

In reply to Dmitry Osipenko

Now Internal and External memory controllers are memory interconnection providers. This allows us to use interconnect API for tuning of memory configuration. EMC driver now supports OPPs and DVFS.

---
 drivers/memory/tegra/Kconfig        |   1 +
 drivers/memory/tegra/tegra124-emc.c | 338 +++++++++++++++++++++++++++-
 drivers/memory/tegra/tegra124.c     |  82 ++++++-
 3 files changed, 409 insertions(+), 12 deletions(-)
drivers/memory/tegra/Kconfig +1 -0
index 6265aa39674f..076482e1787b 100644
@@ -35,6 +35,7 @@ config TEGRA124_EMC
 	tristate "NVIDIA Tegra124 External Memory Controller driver"
 	default y
 	depends on TEGRA_MC && ARCH_TEGRA_124_SOC
+	select PM_OPP
 	help
 	  This driver is for the External Memory Controller (EMC) found on
 	  Tegra124 chips. The EMC controls the external DRAM on the board.
drivers/memory/tegra/tegra124-emc.c +327 -11

433 lines not on this page · raw message

drivers/memory/tegra/tegra124.c +81 -1

105 lines not on this page · raw message

Signature · 1 line
2.29.2
author maintainer
#41 lore raw

PATCH v8 14/26 drm/tegra: dc: Support memory bandwidth management

In reply to Dmitry Osipenko

Display controller (DC) performs isochronous memory transfers, and thus, has a requirement for a minimum memory bandwidth that shall be fulfilled, otherwise framebuffer data can't be fetched fast enough and this results in a DC's data-FIFO underflow that follows by a visual corruption.

The Memory Controller drivers provide facility for memory bandwidth management via interconnect API. Let's wire up the interconnect API support to the DC driver in order to fix the distorted display output on T30 Ouya, T124 TK1 and other Tegra devices.

---
 drivers/gpu/drm/tegra/Kconfig |   1 +
 drivers/gpu/drm/tegra/dc.c    | 349 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/tegra/dc.h    |  14 ++
 drivers/gpu/drm/tegra/drm.c   |  14 ++
 drivers/gpu/drm/tegra/hub.c   |   3 +
 drivers/gpu/drm/tegra/plane.c | 121 ++++++++++++
 drivers/gpu/drm/tegra/plane.h |  15 ++
 7 files changed, 517 insertions(+)
drivers/gpu/drm/tegra/Kconfig +1 -0
index 5043dcaf1cf9..1650a448eabd 100644
@@ -9,6 +9,7 @@ config DRM_TEGRA
 	select DRM_MIPI_DSI
 	select DRM_PANEL
 	select TEGRA_HOST1X
+	select INTERCONNECT
 	select IOMMU_IOVA
 	select CEC_CORE if CEC_NOTIFIER
 	help
drivers/gpu/drm/tegra/dc.c +349 -0

466 lines not on this page · raw message

drivers/gpu/drm/tegra/dc.h +14 -0
index 051d03dcb9b0..0d7bdf66a1ec 100644
@@ -15,6 +15,8 @@
 
 struct tegra_output;
 
+#define TEGRA_DC_LEGACY_PLANES_NUM	6
+
 struct tegra_dc_state {
 	struct drm_crtc_state base;
 
@@ -23,6 +25,8 @@ struct tegra_dc_state {
 	unsigned int div;
 
 	u32 planes;
+
+	unsigned long plane_peak_bw[TEGRA_DC_LEGACY_PLANES_NUM];
 };
 
 static inline struct tegra_dc_state *to_dc_state(struct drm_crtc_state *state)
@@ -33,6 +37,12 @@ static inline struct tegra_dc_state *to_dc_state(struct drm_crtc_state *state)
 	return NULL;
 }
 
+static inline const struct tegra_dc_state *
+to_const_dc_state(const struct drm_crtc_state *state)
+{
+	return to_dc_state((struct drm_crtc_state *)state);
+}
+
 struct tegra_dc_stats {
 	unsigned long frames;
 	unsigned long vblank;
@@ -65,7 +75,9 @@ struct tegra_dc_soc_info {
 	unsigned int num_overlay_formats;
 	const u64 *modifiers;
 	bool has_win_a_without_filters;
+	bool has_win_b_vfilter_mem_client;
 	bool has_win_c_without_vert_filter;
+	unsigned int plane_tiled_memory_bandwidth_x2;
 };
 
 struct tegra_dc {
@@ -151,6 +163,8 @@ int tegra_dc_state_setup_clock(struct tegra_dc *dc,
 			       struct drm_crtc_state *crtc_state,
 			       struct clk *clk, unsigned long pclk,
 			       unsigned int div);
+void tegra_crtc_atomic_post_commit(struct drm_crtc *crtc,
+				   struct drm_atomic_state *state);
 
 /* from rgb.c */
 int tegra_dc_rgb_probe(struct tegra_dc *dc);
drivers/gpu/drm/tegra/drm.c +14 -0
index f0f581cd345e..fc16c8525d54 100644
@@ -20,6 +20,7 @@
 #include <drm/drm_prime.h>
 #include <drm/drm_vblank.h>
 
+#include "dc.h"
 #include "drm.h"
 #include "gem.h"
 
@@ -59,6 +60,17 @@ static const struct drm_mode_config_funcs tegra_drm_mode_config_funcs = {
 	.atomic_commit = drm_atomic_helper_commit,
 };
 
+static void tegra_atomic_post_commit(struct drm_device *drm,
+				     struct drm_atomic_state *old_state)
+{
+	struct drm_crtc_state *old_crtc_state __maybe_unused;
+	struct drm_crtc *crtc;
+	unsigned int i;
+
+	for_each_old_crtc_in_state(old_state, crtc, old_crtc_state, i)
+		tegra_crtc_atomic_post_commit(crtc, old_state);
+}
+
 static void tegra_atomic_commit_tail(struct drm_atomic_state *old_state)
 {
 	struct drm_device *drm = old_state->dev;
@@ -75,6 +87,8 @@ static void tegra_atomic_commit_tail(struct drm_atomic_state *old_state)
 	} else {
 		drm_atomic_helper_commit_tail_rpm(old_state);
 	}
+
+	tegra_atomic_post_commit(drm, old_state);
 }
 
 static const struct drm_mode_config_helper_funcs
drivers/gpu/drm/tegra/hub.c +3 -0
index 22a03f7ffdc1..4fa338dc7eb2 100644
@@ -344,6 +344,9 @@ static int tegra_shared_plane_atomic_check(struct drm_plane *plane,
 	struct tegra_dc *dc = to_tegra_dc(state->crtc);
 	int err;
 
+	plane_state->peak_memory_bandwidth = 0;
+	plane_state->avg_memory_bandwidth = 0;
+
 	/* no need for further checks if the plane is being disabled */
 	if (!state->crtc || !state->fb)
 		return 0;
drivers/gpu/drm/tegra/plane.c +121 -0

148 lines not on this page · raw message

drivers/gpu/drm/tegra/plane.h +15 -0
index c691dd79b27b..f2731aae7d01 100644
@@ -8,6 +8,7 @@
 
 #include <drm/drm_plane.h>
 
+struct icc_path;
 struct tegra_bo;
 struct tegra_dc;
 
@@ -16,6 +17,9 @@ struct tegra_plane {
 	struct tegra_dc *dc;
 	unsigned int offset;
 	unsigned int index;
+
+	struct icc_path *icc_mem;
+	struct icc_path *icc_mem_vfilter;
 };
 
 struct tegra_cursor {
@@ -52,6 +56,10 @@ struct tegra_plane_state {
 	/* used for legacy blending support only */
 	struct tegra_plane_legacy_blending_state blending[2];
 	bool opaque;
+
+	/* bandwidths are in ICC units, i.e. kbytes/sec */
+	u32 peak_memory_bandwidth;
+	u32 avg_memory_bandwidth;
 };
 
 static inline struct tegra_plane_state *
@@ -63,6 +71,12 @@ to_tegra_plane_state(struct drm_plane_state *state)
 	return NULL;
 }
 
+static inline const struct tegra_plane_state *
+to_const_tegra_plane_state(const struct drm_plane_state *state)
+{
+	return to_tegra_plane_state((struct drm_plane_state *)state);
+}
+
 extern const struct drm_plane_funcs tegra_plane_funcs;
 
 int tegra_plane_prepare_fb(struct drm_plane *plane,
@@ -77,5 +91,6 @@ int tegra_plane_format(u32 fourcc, u32 *format, u32 *swap);
 bool tegra_plane_format_is_yuv(unsigned int format, bool *planar);
 int tegra_plane_setup_legacy_state(struct tegra_plane *tegra,
 				   struct tegra_plane_state *state);
+int tegra_plane_interconnect_init(struct tegra_plane *plane);
 
 #endif /* TEGRA_PLANE_H */
Signature · 1 line
2.29.2
author maintainer
#42 lore raw

PATCH v8 15/26 drm/tegra: dc: Extend debug stats with total number of events

In reply to Dmitry Osipenko

It's useful to know the total number of underflow events and currently the debug stats are getting reset each time CRTC is being disabled. Let's account the overall number of events that doesn't get a reset.

---
 drivers/gpu/drm/tegra/dc.c | 10 ++++++++++
 drivers/gpu/drm/tegra/dc.h |  5 +++++
 2 files changed, 15 insertions(+)
drivers/gpu/drm/tegra/dc.c +10 -0
index 14168f792977..fd7c8828652d 100644
@@ -1539,6 +1539,11 @@ static int tegra_dc_show_stats(struct seq_file *s, void *data)
 	seq_printf(s, "underflow: %lu\n", dc->stats.underflow);
 	seq_printf(s, "overflow: %lu\n", dc->stats.overflow);
 
+	seq_printf(s, "frames total: %lu\n", dc->stats.frames_total);
+	seq_printf(s, "vblank total: %lu\n", dc->stats.vblank_total);
+	seq_printf(s, "underflow total: %lu\n", dc->stats.underflow_total);
+	seq_printf(s, "overflow total: %lu\n", dc->stats.overflow_total);
+
 	return 0;
 }
 
@@ -2308,6 +2313,7 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)
 		/*
 		dev_dbg(dc->dev, "%s(): frame end\n", __func__);
 		*/
+		dc->stats.frames_total++;
 		dc->stats.frames++;
 	}
 
@@ -2316,6 +2322,7 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)
 		dev_dbg(dc->dev, "%s(): vertical blank\n", __func__);
 		*/
 		drm_crtc_handle_vblank(&dc->base);
+		dc->stats.vblank_total++;
 		dc->stats.vblank++;
 	}
 
@@ -2323,6 +2330,7 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)
 		/*
 		dev_dbg(dc->dev, "%s(): underflow\n", __func__);
 		*/
+		dc->stats.underflow_total++;
 		dc->stats.underflow++;
 	}
 
@@ -2330,11 +2338,13 @@ static irqreturn_t tegra_dc_irq(int irq, void *data)
 		/*
 		dev_dbg(dc->dev, "%s(): overflow\n", __func__);
 		*/
+		dc->stats.overflow_total++;
 		dc->stats.overflow++;
 	}
 
 	if (status & HEAD_UF_INT) {
 		dev_dbg_ratelimited(dc->dev, "%s(): head underflow\n", __func__);
+		dc->stats.underflow_total++;
 		dc->stats.underflow++;
 	}
 
drivers/gpu/drm/tegra/dc.h +5 -0
index 0d7bdf66a1ec..ba4ed35139fb 100644
@@ -48,6 +48,11 @@ struct tegra_dc_stats {
 	unsigned long vblank;
 	unsigned long underflow;
 	unsigned long overflow;
+
+	unsigned long frames_total;
+	unsigned long vblank_total;
+	unsigned long underflow_total;
+	unsigned long overflow_total;
 };
 
 struct tegra_windowgroup_soc {
Signature · 1 line
2.29.2
author maintainer
#43 lore raw

PATCH v8 16/26 PM / devfreq: tegra30: Support interconnect and OPPs from device-tree

In reply to Dmitry Osipenko

This patch moves ACTMON driver away from generating OPP table by itself, transitioning it to use the table which comes from device-tree. This change breaks compatibility with older device-trees in order to bring support for the interconnect framework to the driver. This is a mandatory change which needs to be done in order to implement interconnect-based memory DVFS. Users of legacy device-trees will get a message telling that theirs DT needs to be upgraded. Now ACTMON issues memory bandwidth request using dev_pm_opp_set_bw(), instead of driving EMC clock rate directly.

---
 drivers/devfreq/tegra30-devfreq.c | 86 ++++++++++++++++---------------
 1 file changed, 44 insertions(+), 42 deletions(-)
drivers/devfreq/tegra30-devfreq.c +44 -42
index 38cc0d014738..ed6d4469c8c7 100644
@@ -19,6 +19,8 @@
 #include <linux/reset.h>
 #include <linux/workqueue.h>
 
+#include <soc/tegra/fuse.h>
+
 #include "governor.h"
 
 #define ACTMON_GLB_STATUS					0x0
@@ -155,6 +157,7 @@ struct tegra_devfreq_device {
 
 struct tegra_devfreq {
 	struct devfreq		*devfreq;
+	struct opp_table	*opp_table;
 
 	struct reset_control	*reset;
 	struct clk		*clock;
@@ -612,34 +615,19 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra)
 static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
 				u32 flags)
 {
-	struct tegra_devfreq *tegra = dev_get_drvdata(dev);
-	struct devfreq *devfreq = tegra->devfreq;
 	struct dev_pm_opp *opp;
-	unsigned long rate;
-	int err;
+	int ret;
 
 	opp = devfreq_recommended_opp(dev, freq, flags);
 	if (IS_ERR(opp)) {
 		dev_err(dev, "Failed to find opp for %lu Hz\n", *freq);
 		return PTR_ERR(opp);
 	}
-	rate = dev_pm_opp_get_freq(opp);
-	dev_pm_opp_put(opp);
-
-	err = clk_set_min_rate(tegra->emc_clock, rate * KHZ);
-	if (err)
-		return err;
-
-	err = clk_set_rate(tegra->emc_clock, 0);
-	if (err)
-		goto restore_min_rate;
 
-	return 0;
-
-restore_min_rate:
-	clk_set_min_rate(tegra->emc_clock, devfreq->previous_freq);
+	ret = dev_pm_opp_set_bw(dev, opp);
+	dev_pm_opp_put(opp);
 
-	return err;
+	return ret;
 }
 
 static int tegra_devfreq_get_dev_status(struct device *dev,
@@ -655,7 +643,7 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
 	stat->private_data = tegra;
 
 	/* The below are to be used by the other governors */
-	stat->current_frequency = cur_freq;
+	stat->current_frequency = cur_freq * KHZ;
 
 	actmon_dev = &tegra->devices[MCALL];
 
@@ -705,7 +693,12 @@ static int tegra_governor_get_target(struct devfreq *devfreq,
 		target_freq = max(target_freq, dev->target_freq);
 	}
 
-	*freq = target_freq;
+	/*
+	 * tegra-devfreq driver operates with KHz units, while OPP table
+	 * entries use Hz units. Hence we need to convert the units for the
+	 * devfreq core.
+	 */
+	*freq = target_freq * KHZ;
 
 	return 0;
 }
@@ -774,6 +767,7 @@ static struct devfreq_governor tegra_devfreq_governor = {
 
 static int tegra_devfreq_probe(struct platform_device *pdev)
 {
+	u32 hw_version = BIT(tegra_sku_info.soc_speedo_id);
 	struct tegra_devfreq_device *dev;
 	struct tegra_devfreq *tegra;
 	struct devfreq *devfreq;
@@ -781,6 +775,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	long rate;
 	int err;
 
+	/* legacy device-trees don't have OPP table and must be updated */
+	if (!device_property_present(&pdev->dev, "operating-points-v2")) {
+		dev_err(&pdev->dev,
+			"OPP table not found, please update your device tree\n");
+		return -ENODEV;
+	}
+
 	tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
 	if (!tegra)
 		return -ENOMEM;
@@ -822,11 +823,25 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	tegra->opp_table = dev_pm_opp_set_supported_hw(&pdev->dev,
+						       &hw_version, 1);
+	err = PTR_ERR_OR_ZERO(tegra->opp_table);
+	if (err) {
+		dev_err(&pdev->dev, "Failed to set supported HW: %d\n", err);
+		return err;
+	}
+
+	err = dev_pm_opp_of_add_table(&pdev->dev);
+	if (err) {
+		dev_err(&pdev->dev, "Failed to add OPP table: %d\n", err);
+		goto put_hw;
+	}
+
 	err = clk_prepare_enable(tegra->clock);
 	if (err) {
 		dev_err(&pdev->dev,
 			"Failed to prepare and enable ACTMON clock\n");
-		return err;
+		goto remove_table;
 	}
 
 	err = reset_control_reset(tegra->reset);
@@ -850,23 +865,6 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 		dev->regs = tegra->regs + dev->config->offset;
 	}
 
-	for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) {
-		rate = clk_round_rate(tegra->emc_clock, rate);
-
-		if (rate < 0) {
-			dev_err(&pdev->dev,
-				"Failed to round clock rate: %ld\n", rate);
-			err = rate;
-			goto remove_opps;
-		}
-
-		err = dev_pm_opp_add(&pdev->dev, rate / KHZ, 0);
-		if (err) {
-			dev_err(&pdev->dev, "Failed to add OPP: %d\n", err);
-			goto remove_opps;
-		}
-	}
-
 	platform_set_drvdata(pdev, tegra);
 
 	tegra->clk_rate_change_nb.notifier_call = tegra_actmon_clk_notify_cb;
@@ -882,7 +880,6 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	}
 
 	tegra_devfreq_profile.initial_freq = clk_get_rate(tegra->emc_clock);
-	tegra_devfreq_profile.initial_freq /= KHZ;
 
 	devfreq = devfreq_add_device(&pdev->dev, &tegra_devfreq_profile,
 				     "tegra_actmon", NULL);
@@ -902,6 +899,10 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	reset_control_reset(tegra->reset);
 disable_clk:
 	clk_disable_unprepare(tegra->clock);
+remove_table:
+	dev_pm_opp_of_remove_table(&pdev->dev);
+put_hw:
+	dev_pm_opp_put_supported_hw(tegra->opp_table);
 
 	return err;
 }
@@ -913,11 +914,12 @@ static int tegra_devfreq_remove(struct platform_device *pdev)
 	devfreq_remove_device(tegra->devfreq);
 	devfreq_remove_governor(&tegra_devfreq_governor);
 
-	dev_pm_opp_remove_all_dynamic(&pdev->dev);
-
 	reset_control_reset(tegra->reset);
 	clk_disable_unprepare(tegra->clock);
 
+	dev_pm_opp_of_remove_table(&pdev->dev);
+	dev_pm_opp_put_supported_hw(tegra->opp_table);
+
 	return 0;
 }
 
Signature · 1 line
2.29.2
author maintainer
#44 lore raw

PATCH v8 17/26 PM / devfreq: tegra30: Separate configurations per-SoC generation

In reply to Dmitry Osipenko

Previously we were using count-weight of the T124 for T30 in order to get EMC clock rate that was reasonable for T30. In fact the count-weight should be x2 times smaller on T30, but then devfreq was producing a bit too low EMC clock rate for ISO memory clients, like display controller for example.

Now both Tegra ACTMON and Tegra DRM display drivers support interconnect framework and display driver tells to ICC what a minimum memory bandwidth is needed, preventing FIFO underflows. Thus, now we can use a proper count-weight value for Tegra30 and MC_ALL device config needs a bit more aggressive boosting.

Add a separate ACTMON driver configuration that is specific to Tegra30.

---
 drivers/devfreq/tegra30-devfreq.c | 68 ++++++++++++++++++++++++-------
 1 file changed, 54 insertions(+), 14 deletions(-)
drivers/devfreq/tegra30-devfreq.c +54 -14
index ed6d4469c8c7..36ba82c3898c 100644
@@ -57,13 +57,6 @@
 #define ACTMON_BELOW_WMARK_WINDOW				3
 #define ACTMON_BOOST_FREQ_STEP					16000
 
-/*
- * Activity counter is incremented every 256 memory transactions, and each
- * transaction takes 4 EMC clocks for Tegra124; So the COUNT_WEIGHT is
- * 4 * 256 = 1024.
- */
-#define ACTMON_COUNT_WEIGHT					0x400
-
 /*
  * ACTMON_AVERAGE_WINDOW_LOG2: default value for @DEV_CTRL_K_VAL, which
  * translates to 2 ^ (K_VAL + 1). ex: 2 ^ (6 + 1) = 128
@@ -111,7 +104,7 @@ enum tegra_actmon_device {
 	MCCPU,
 };
 
-static const struct tegra_devfreq_device_config actmon_device_configs[] = {
+static const struct tegra_devfreq_device_config tegra124_device_configs[] = {
 	{
 		/* MCALL: All memory accesses (including from the CPUs) */
 		.offset = 0x1c0,
@@ -133,6 +126,28 @@ static const struct tegra_devfreq_device_config actmon_device_configs[] = {
 	},
 };
 
+static const struct tegra_devfreq_device_config tegra30_device_configs[] = {
+	{
+		/* MCALL: All memory accesses (including from the CPUs) */
+		.offset = 0x1c0,
+		.irq_mask = 1 << 26,
+		.boost_up_coeff = 200,
+		.boost_down_coeff = 50,
+		.boost_up_threshold = 20,
+		.boost_down_threshold = 10,
+	},
+	{
+		/* MCCPU: memory accesses from the CPUs */
+		.offset = 0x200,
+		.irq_mask = 1 << 25,
+		.boost_up_coeff = 800,
+		.boost_down_coeff = 40,
+		.boost_up_threshold = 27,
+		.boost_down_threshold = 10,
+		.avg_dependency_threshold = 16000, /* 16MHz in kHz units */
+	},
+};
+
 /**
  * struct tegra_devfreq_device - state specific to an ACTMON device
  *
@@ -155,6 +170,12 @@ struct tegra_devfreq_device {
 	unsigned long target_freq;
 };
 
+struct tegra_devfreq_soc_data {
+	const struct tegra_devfreq_device_config *configs;
+	/* Weight value for count measurements */
+	unsigned int count_weight;
+};
+
 struct tegra_devfreq {
 	struct devfreq		*devfreq;
 	struct opp_table	*opp_table;
@@ -171,11 +192,13 @@ struct tegra_devfreq {
 	struct delayed_work	cpufreq_update_work;
 	struct notifier_block	cpu_rate_change_nb;
 
-	struct tegra_devfreq_device devices[ARRAY_SIZE(actmon_device_configs)];
+	struct tegra_devfreq_device devices[2];
 
 	unsigned int		irq;
 
 	bool			started;
+
+	const struct tegra_devfreq_soc_data *soc;
 };
 
 struct tegra_actmon_emc_ratio {
@@ -488,7 +511,7 @@ static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
 	tegra_devfreq_update_avg_wmark(tegra, dev);
 	tegra_devfreq_update_wmark(tegra, dev);
 
-	device_writel(dev, ACTMON_COUNT_WEIGHT, ACTMON_DEV_COUNT_WEIGHT);
+	device_writel(dev, tegra->soc->count_weight, ACTMON_DEV_COUNT_WEIGHT);
 	device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
 
 	val |= ACTMON_DEV_CTRL_ENB_PERIODIC;
@@ -786,6 +809,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	if (!tegra)
 		return -ENOMEM;
 
+	tegra->soc = of_device_get_match_data(&pdev->dev);
+
 	tegra->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(tegra->regs))
 		return PTR_ERR(tegra->regs);
@@ -859,9 +884,9 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 
 	tegra->max_freq = rate / KHZ;
 
-	for (i = 0; i < ARRAY_SIZE(actmon_device_configs); i++) {
+	for (i = 0; i < ARRAY_SIZE(tegra->devices); i++) {
 		dev = tegra->devices + i;
-		dev->config = actmon_device_configs + i;
+		dev->config = tegra->soc->configs + i;
 		dev->regs = tegra->regs + dev->config->offset;
 	}
 
@@ -923,9 +948,24 @@ static int tegra_devfreq_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct tegra_devfreq_soc_data tegra124_soc = {
+	.configs = tegra124_device_configs,
+
+	/*
+	 * Activity counter is incremented every 256 memory transactions,
+	 * and each transaction takes 4 EMC clocks.
+	 */
+	.count_weight = 4 * 256,
+};
+
+static const struct tegra_devfreq_soc_data tegra30_soc = {
+	.configs = tegra30_device_configs,
+	.count_weight = 2 * 256,
+};
+
 static const struct of_device_id tegra_devfreq_of_match[] = {
-	{ .compatible = "nvidia,tegra30-actmon" },
-	{ .compatible = "nvidia,tegra124-actmon" },
+	{ .compatible = "nvidia,tegra30-actmon",  .data = &tegra30_soc, },
+	{ .compatible = "nvidia,tegra124-actmon", .data = &tegra124_soc, },
 	{ },
 };
 
Signature · 1 line
2.29.2
author maintainer
#45 lore raw

PATCH v8 18/26 PM / devfreq: tegra20: Deprecate in a favor of emc-stat based driver

In reply to Dmitry Osipenko

Remove tegra20-devfreq in order to replace it with a EMC_STAT based devfreq driver. Previously we were going to use MC_STAT based tegra20-devfreq driver because EMC_STAT wasn't working properly, but now that problem is resolved. This resolves complications imposed by the removed driver since it was depending on both EMC and MC drivers simultaneously.

---
 MAINTAINERS                       |   1 -
 drivers/devfreq/Kconfig           |  10 --
 drivers/devfreq/Makefile          |   1 -
 drivers/devfreq/tegra20-devfreq.c | 210 ------------------------------
 4 files changed, 222 deletions(-)
 delete mode 100644 drivers/devfreq/tegra20-devfreq.c
MAINTAINERS +0 -1
index 4bbd866dec8e..94d8bebfda60 100644
@@ -11323,7 +11323,6 @@ L:	[email protected]
 L:	[email protected]
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git
 S:	Maintained
-F:	drivers/devfreq/tegra20-devfreq.c
 F:	drivers/devfreq/tegra30-devfreq.c
 
 MEMORY MANAGEMENT
drivers/devfreq/Kconfig +0 -10
index 0ee36ae2fa79..00704efe6398 100644
@@ -121,16 +121,6 @@ config ARM_TEGRA_DEVFREQ
 	  It reads ACTMON counters of memory controllers and adjusts the
 	  operating frequencies and voltages with OPP support.
 
-config ARM_TEGRA20_DEVFREQ
-	tristate "NVIDIA Tegra20 DEVFREQ Driver"
-	depends on ARCH_TEGRA_2x_SOC || COMPILE_TEST
-	depends on COMMON_CLK
-	select DEVFREQ_GOV_SIMPLE_ONDEMAND
-	help
-	  This adds the DEVFREQ driver for the Tegra20 family of SoCs.
-	  It reads Memory Controller counters and adjusts the operating
-	  frequencies and voltages with OPP support.
-
 config ARM_RK3399_DMC_DEVFREQ
 	tristate "ARM RK3399 DMC DEVFREQ Driver"
 	depends on (ARCH_ROCKCHIP && HAVE_ARM_SMCCC) || \
drivers/devfreq/Makefile +0 -1
index 3ca1ad0ecb97..a16333ea7034 100644
@@ -13,7 +13,6 @@ obj-$(CONFIG_ARM_IMX_BUS_DEVFREQ)	+= imx-bus.o
 obj-$(CONFIG_ARM_IMX8M_DDRC_DEVFREQ)	+= imx8m-ddrc.o
 obj-$(CONFIG_ARM_RK3399_DMC_DEVFREQ)	+= rk3399_dmc.o
 obj-$(CONFIG_ARM_TEGRA_DEVFREQ)		+= tegra30-devfreq.o
-obj-$(CONFIG_ARM_TEGRA20_DEVFREQ)	+= tegra20-devfreq.o
 
 # DEVFREQ Event Drivers
 obj-$(CONFIG_PM_DEVFREQ_EVENT)		+= event/
drivers/devfreq/tegra20-devfreq.c deleted +0 -210

210 lines not on this page · raw message

Signature · 1 line
2.29.2
author maintainer
#46 lore raw

PATCH v8 19/26 ARM: tegra: Correct EMC registers size in Tegra20 device-tree

In reply to Dmitry Osipenko

Fix the size of Tegra20 EMC registers, which should be twice bigger.

---
 arch/arm/boot/dts/tegra20.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/tegra20.dtsi +1 -1
index 72a4211a618f..9347f7789245 100644
@@ -634,7 +634,7 @@ mc: memory-controller@7000f000 {
 
 	memory-controller@7000f400 {
 		compatible = "nvidia,tegra20-emc";
-		reg = <0x7000f400 0x200>;
+		reg = <0x7000f400 0x400>;
 		interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA20_CLK_EMC>;
 		#address-cells = <1>;
Signature · 1 line
2.29.2
author maintainer
#47 lore raw

PATCH v8 20/26 ARM: tegra: Add interconnect properties to Tegra20 device-tree

In reply to Dmitry Osipenko

Add interconnect properties to the Memory Controller, External Memory Controller and the Display Controller nodes in order to describe hardware interconnection.

---
 arch/arm/boot/dts/tegra20.dtsi | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)
arch/arm/boot/dts/tegra20.dtsi +25 -1
index 9347f7789245..2e1304493f7d 100644
@@ -111,6 +111,17 @@ dc@54200000 {
 
 			nvidia,head = <0>;
 
+			interconnects = <&mc TEGRA20_MC_DISPLAY0A &emc>,
+					<&mc TEGRA20_MC_DISPLAY0B &emc>,
+					<&mc TEGRA20_MC_DISPLAY1B &emc>,
+					<&mc TEGRA20_MC_DISPLAY0C &emc>,
+					<&mc TEGRA20_MC_DISPLAYHC &emc>;
+			interconnect-names = "wina",
+					     "winb",
+					     "winb-vfilter",
+					     "winc",
+					     "cursor";
+
 			rgb {
 				status = "disabled";
 			};
@@ -128,6 +139,17 @@ dc@54240000 {
 
 			nvidia,head = <1>;
 
+			interconnects = <&mc TEGRA20_MC_DISPLAY0AB &emc>,
+					<&mc TEGRA20_MC_DISPLAY0BB &emc>,
+					<&mc TEGRA20_MC_DISPLAY1BB &emc>,
+					<&mc TEGRA20_MC_DISPLAY0CB &emc>,
+					<&mc TEGRA20_MC_DISPLAYHCB &emc>;
+			interconnect-names = "wina",
+					     "winb",
+					     "winb-vfilter",
+					     "winc",
+					     "cursor";
+
 			rgb {
 				status = "disabled";
 			};
@@ -630,15 +652,17 @@ mc: memory-controller@7000f000 {
 		interrupts = <GIC_SPI 77 IRQ_TYPE_LEVEL_HIGH>;
 		#reset-cells = <1>;
 		#iommu-cells = <0>;
+		#interconnect-cells = <1>;
 	};
 
-	memory-controller@7000f400 {
+	emc: memory-controller@7000f400 {
 		compatible = "nvidia,tegra20-emc";
 		reg = <0x7000f400 0x400>;
 		interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA20_CLK_EMC>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		#interconnect-cells = <0>;
 	};
 
 	fuse@7000f800 {
Signature · 1 line
2.29.2
author maintainer
#48 lore raw

PATCH v8 21/26 ARM: tegra: Add interconnect properties to Tegra30 device-tree

In reply to Dmitry Osipenko

Add interconnect properties to the Memory Controller, External Memory Controller and the Display Controller nodes in order to describe hardware interconnection.

---
 arch/arm/boot/dts/tegra30.dtsi | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)
arch/arm/boot/dts/tegra30.dtsi +26 -1
index aeae8c092d41..2caf6cc6f4b1 100644
@@ -210,6 +210,17 @@ dc@54200000 {
 
 			nvidia,head = <0>;
 
+			interconnects = <&mc TEGRA30_MC_DISPLAY0A &emc>,
+					<&mc TEGRA30_MC_DISPLAY0B &emc>,
+					<&mc TEGRA30_MC_DISPLAY1B &emc>,
+					<&mc TEGRA30_MC_DISPLAY0C &emc>,
+					<&mc TEGRA30_MC_DISPLAYHC &emc>;
+			interconnect-names = "wina",
+					     "winb",
+					     "winb-vfilter",
+					     "winc",
+					     "cursor";
+
 			rgb {
 				status = "disabled";
 			};
@@ -229,6 +240,17 @@ dc@54240000 {
 
 			nvidia,head = <1>;
 
+			interconnects = <&mc TEGRA30_MC_DISPLAY0AB &emc>,
+					<&mc TEGRA30_MC_DISPLAY0BB &emc>,
+					<&mc TEGRA30_MC_DISPLAY1BB &emc>,
+					<&mc TEGRA30_MC_DISPLAY0CB &emc>,
+					<&mc TEGRA30_MC_DISPLAYHCB &emc>;
+			interconnect-names = "wina",
+					     "winb",
+					     "winb-vfilter",
+					     "winc",
+					     "cursor";
+
 			rgb {
 				status = "disabled";
 			};
@@ -748,15 +770,18 @@ mc: memory-controller@7000f000 {
 
 		#iommu-cells = <1>;
 		#reset-cells = <1>;
+		#interconnect-cells = <1>;
 	};
 
-	memory-controller@7000f400 {
+	emc: memory-controller@7000f400 {
 		compatible = "nvidia,tegra30-emc";
 		reg = <0x7000f400 0x400>;
 		interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&tegra_car TEGRA30_CLK_EMC>;
 
 		nvidia,memory-controller = <&mc>;
+
+		#interconnect-cells = <0>;
 	};
 
 	fuse@7000f800 {
Signature · 1 line
2.29.2
author maintainer
#49 lore raw

PATCH v8 22/26 ARM: tegra: Add interconnect properties to Tegra124 device-tree

In reply to Dmitry Osipenko

Add interconnect properties to the Memory Controller, External Memory Controller and the Display Controller nodes in order to describe hardware interconnection.

---
 arch/arm/boot/dts/tegra124.dtsi | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
arch/arm/boot/dts/tegra124.dtsi +25 -0
index 64f488ba1e72..1801e30b1d3a 100644
@@ -113,6 +113,19 @@ dc@54200000 {
 			iommus = <&mc TEGRA_SWGROUP_DC>;
 
 			nvidia,head = <0>;
+
+			interconnects = <&mc TEGRA124_MC_DISPLAY0A &emc>,
+					<&mc TEGRA124_MC_DISPLAY0B &emc>,
+					<&mc TEGRA124_MC_DISPLAY0C &emc>,
+					<&mc TEGRA124_MC_DISPLAYHC &emc>,
+					<&mc TEGRA124_MC_DISPLAYD &emc>,
+					<&mc TEGRA124_MC_DISPLAYT &emc>;
+			interconnect-names = "wina",
+					     "winb",
+					     "winc",
+					     "cursor",
+					     "wind",
+					     "wint";
 		};
 
 		dc@54240000 {
@@ -127,6 +140,15 @@ dc@54240000 {
 			iommus = <&mc TEGRA_SWGROUP_DCB>;
 
 			nvidia,head = <1>;
+
+			interconnects = <&mc TEGRA124_MC_DISPLAY0AB &emc>,
+					<&mc TEGRA124_MC_DISPLAY0BB &emc>,
+					<&mc TEGRA124_MC_DISPLAY0CB &emc>,
+					<&mc TEGRA124_MC_DISPLAYHCB &emc>;
+			interconnect-names = "wina",
+					     "winb",
+					     "winc",
+					     "cursor";
 		};
 
 		hdmi: hdmi@54280000 {
@@ -628,6 +650,7 @@ mc: memory-controller@70019000 {
 
 		#iommu-cells = <1>;
 		#reset-cells = <1>;
+		#interconnect-cells = <1>;
 	};
 
 	emc: external-memory-controller@7001b000 {
@@ -637,6 +660,8 @@ emc: external-memory-controller@7001b000 {
 		clock-names = "emc";
 
 		nvidia,memory-controller = <&mc>;
+
+		#interconnect-cells = <0>;
 	};
 
 	sata@70020000 {
Signature · 1 line
2.29.2
author maintainer
#50 lore raw

PATCH v8 23/26 ARM: tegra: Add nvidia,memory-controller phandle to Tegra20 EMC device-tree

In reply to Dmitry Osipenko

Add nvidia,memory-controller to the Tegra20 External Memory Controller node. This allows to perform a direct lookup of the Memory Controller instead of walking up the whole tree. This puts Tegra20 device-tree on par with Tegra30+.

---
 arch/arm/boot/dts/tegra20.dtsi | 2 ++
 1 file changed, 2 insertions(+)
arch/arm/boot/dts/tegra20.dtsi +2 -0
index 2e1304493f7d..8f8ad81916e7 100644
@@ -663,6 +663,8 @@ emc: memory-controller@7000f400 {
 		#address-cells = <1>;
 		#size-cells = <0>;
 		#interconnect-cells = <0>;
+
+		nvidia,memory-controller = <&mc>;
 	};
 
 	fuse@7000f800 {
Signature · 1 line
2.29.2
author maintainer
#51 lore raw

PATCH v8 24/26 ARM: tegra: Add DVFS properties to Tegra20 EMC device-tree node

In reply to Dmitry Osipenko

Add EMC OPP DVFS table that will be used for dynamic scaling of memory frequency/voltage. Update board device-trees with optional EMC core supply and remove unsupported OPPs.

---
 .../boot/dts/tegra20-acer-a500-picasso.dts    |  7 ++
 arch/arm/boot/dts/tegra20-colibri.dtsi        |  4 +
 arch/arm/boot/dts/tegra20-paz00.dts           |  6 ++
 .../arm/boot/dts/tegra20-peripherals-opp.dtsi | 92 +++++++++++++++++++
 arch/arm/boot/dts/tegra20.dtsi                |  3 +
 5 files changed, 112 insertions(+)
 create mode 100644 arch/arm/boot/dts/tegra20-peripherals-opp.dtsi
arch/arm/boot/dts/tegra20-acer-a500-picasso.dts +7 -0
index a0b829738e8f..b4ed88802387 100644
@@ -1061,6 +1061,8 @@ map0 {
 	memory-controller@7000f400 {
 		nvidia,use-ram-code;
 
+		core-supply = <&vdd_core>;
+
 		emc-tables@0 {
 			nvidia,ram-code = <0>; /* elpida-8gb */
 
@@ -1450,3 +1452,8 @@ emc-table@300000 {
 		};
 	};
 };
+
+&emc_icc_dvfs_opp_table {
+	/delete-node/ opp@666000000;
+	/delete-node/ opp@760000000;
+};
arch/arm/boot/dts/tegra20-colibri.dtsi +4 -0
index 6162d193e12c..585a5b441cf6 100644
@@ -742,6 +742,10 @@ sound {
 	};
 };
 
+&emc_icc_dvfs_opp_table {
+	/delete-node/ opp@760000000;
+};
+
 &gpio {
 	lan-reset-n {
 		gpio-hog;
arch/arm/boot/dts/tegra20-paz00.dts +6 -0
index ada2bed8b1b5..52a81d888424 100644
@@ -314,6 +314,8 @@ nvec@7000c500 {
 	memory-controller@7000f400 {
 		nvidia,use-ram-code;
 
+		core-supply = <&core_vdd_reg>;
+
 		emc-tables@0 {
 			nvidia,ram-code = <0x0>;
 			#address-cells = <1>;
@@ -662,3 +664,7 @@ cpu@1 {
 		};
 	};
 };
+
+&emc_icc_dvfs_opp_table {
+	/delete-node/ opp@760000000;
+};
arch/arm/boot/dts/tegra20-peripherals-opp.dtsi new file +92 -0

92 lines not on this page · raw message

arch/arm/boot/dts/tegra20.dtsi +3 -0
index 8f8ad81916e7..6ce498178105 100644
@@ -6,6 +6,8 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/soc/tegra-pmc.h>
 
+#include "tegra20-peripherals-opp.dtsi"
+
 / {
 	compatible = "nvidia,tegra20";
 	interrupt-parent = <&lic>;
@@ -664,6 +666,7 @@ emc: memory-controller@7000f400 {
 		#size-cells = <0>;
 		#interconnect-cells = <0>;
 
+		operating-points-v2 = <&emc_icc_dvfs_opp_table>;
 		nvidia,memory-controller = <&mc>;
 	};
 
Signature · 1 line
2.29.2
author maintainer
#52 lore raw

PATCH v8 25/26 ARM: tegra: Add DVFS properties to Tegra30 EMC and ACTMON device-tree nodes

In reply to Dmitry Osipenko

Add EMC OPP DVFS/DFS tables and interconnect paths that will be used for dynamic memory bandwidth scaling based on memory utilization statistics. Update board device-trees with optional EMC core supply and remove unsupported OPPs.

Note that ACTMON watches all memory interconnect paths, but we use a single CPU-READ interconnect path for driving memory bandwidth, for simplicity.

---
 .../tegra30-asus-nexus7-grouper-common.dtsi   |   4 +
 ...30-asus-nexus7-grouper-memory-timings.dtsi |  12 +
 .../arm/boot/dts/tegra30-peripherals-opp.dtsi | 383 ++++++++++++++++++
 arch/arm/boot/dts/tegra30.dtsi                |   6 +
 4 files changed, 405 insertions(+)
 create mode 100644 arch/arm/boot/dts/tegra30-peripherals-opp.dtsi
arch/arm/boot/dts/tegra30-asus-nexus7-grouper-common.dtsi +4 -0
index 88ca03f57b3b..261e266c61d8 100644
@@ -956,6 +956,10 @@ pmc@7000e400 {
 		nvidia,sys-clock-req-active-high;
 	};
 
+	memory-controller@7000f400 {
+		core-supply = <&vdd_core>;
+	};
+
 	ahub@70080000 {
 		i2s@70080400 {
 			status = "okay";
arch/arm/boot/dts/tegra30-asus-nexus7-grouper-memory-timings.dtsi +12 -0
index bc0f6f29b956..bcff0997ee51 100644
@@ -1563,3 +1563,15 @@ timing-667000000 {
 		};
 	};
 };
+
+&emc_icc_dvfs_opp_table {
+	/delete-node/ opp@750000000,1300;
+	/delete-node/ opp@800000000,1300;
+	/delete-node/ opp@900000000,1350;
+};
+
+&emc_bw_dfs_opp_table {
+	/delete-node/ opp@750000000;
+	/delete-node/ opp@800000000;
+	/delete-node/ opp@900000000;
+};
arch/arm/boot/dts/tegra30-peripherals-opp.dtsi new file +383 -0

383 lines not on this page · raw message

arch/arm/boot/dts/tegra30.dtsi +6 -0
index 2caf6cc6f4b1..44a6dbba7081 100644
@@ -6,6 +6,8 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/soc/tegra-pmc.h>
 
+#include "tegra30-peripherals-opp.dtsi"
+
 / {
 	compatible = "nvidia,tegra30";
 	interrupt-parent = <&lic>;
@@ -417,6 +419,9 @@ actmon@6000c800 {
 		clock-names = "actmon", "emc";
 		resets = <&tegra_car TEGRA30_CLK_ACTMON>;
 		reset-names = "actmon";
+		operating-points-v2 = <&emc_bw_dfs_opp_table>;
+		interconnects = <&mc TEGRA30_MC_MPCORER &emc>;
+		interconnect-names = "cpu-read";
 	};
 
 	gpio: gpio@6000d000 {
@@ -780,6 +785,7 @@ emc: memory-controller@7000f400 {
 		clocks = <&tegra_car TEGRA30_CLK_EMC>;
 
 		nvidia,memory-controller = <&mc>;
+		operating-points-v2 = <&emc_icc_dvfs_opp_table>;
 
 		#interconnect-cells = <0>;
 	};
Signature · 1 line
2.29.2
author maintainer
#53 lore raw

PATCH v8 26/26 ARM: tegra: Add DVFS properties to Tegra124 EMC and ACTMON device-tree nodes

In reply to Dmitry Osipenko

Add EMC OPP DVFS/DFS tables and interconnect paths that will be used for dynamic memory bandwidth scaling based on memory utilization statistics. Remove unsupported EMC OPPs from board device-trees.

Note that ACTMON watches all memory interconnect paths, but we use a single CPU-READ interconnect path for driving memory bandwidth, for simplicity.

---
 arch/arm/boot/dts/tegra124-apalis-emc.dtsi    |   8 +
 .../arm/boot/dts/tegra124-jetson-tk1-emc.dtsi |   8 +
 arch/arm/boot/dts/tegra124-nyan-big-emc.dtsi  |  10 +
 .../arm/boot/dts/tegra124-nyan-blaze-emc.dtsi |  10 +
 .../boot/dts/tegra124-peripherals-opp.dtsi    | 419 ++++++++++++++++++
 arch/arm/boot/dts/tegra124.dtsi               |   6 +
 6 files changed, 461 insertions(+)
 create mode 100644 arch/arm/boot/dts/tegra124-peripherals-opp.dtsi
arch/arm/boot/dts/tegra124-apalis-emc.dtsi +8 -0
index 32401457ae71..a7ac805eeed5 100644
@@ -1465,3 +1465,11 @@ timing-924000000 {
 		};
 	};
 };
+
+&emc_icc_dvfs_opp_table {
+	/delete-node/ opp@1200000000,1100;
+};
+
+&emc_bw_dfs_opp_table {
+	/delete-node/ opp@1200000000;
+};
arch/arm/boot/dts/tegra124-jetson-tk1-emc.dtsi +8 -0
index 861d3f22116b..df4e463afbd1 100644
@@ -2420,3 +2420,11 @@ timing-924000000 {
 		};
 	};
 };
+
+&emc_icc_dvfs_opp_table {
+	/delete-node/ opp@1200000000,1100;
+};
+
+&emc_bw_dfs_opp_table {
+	/delete-node/ opp@1200000000;
+};
arch/arm/boot/dts/tegra124-nyan-big-emc.dtsi +10 -0
index c91647d13a50..a0f56cc9da5c 100644
@@ -6649,3 +6649,13 @@ timing-792000000 {
 		};
 	};
 };
+
+&emc_icc_dvfs_opp_table {
+	/delete-node/ opp@924000000,1100;
+	/delete-node/ opp@1200000000,1100;
+};
+
+&emc_bw_dfs_opp_table {
+	/delete-node/ opp@924000000;
+	/delete-node/ opp@1200000000;
+};
arch/arm/boot/dts/tegra124-nyan-blaze-emc.dtsi +10 -0
index d2beea0bd15f..35c98734d35f 100644
@@ -2048,3 +2048,13 @@ timing-792000000 {
 		};
 	};
 };
+
+&emc_icc_dvfs_opp_table {
+	/delete-node/ opp@924000000,1100;
+	/delete-node/ opp@1200000000,1100;
+};
+
+&emc_bw_dfs_opp_table {
+	/delete-node/ opp@924000000;
+	/delete-node/ opp@1200000000;
+};
arch/arm/boot/dts/tegra124-peripherals-opp.dtsi new file +419 -0

419 lines not on this page · raw message

arch/arm/boot/dts/tegra124.dtsi +6 -0
index 1801e30b1d3a..46441d10a3fc 100644
@@ -8,6 +8,8 @@
 #include <dt-bindings/thermal/tegra124-soctherm.h>
 #include <dt-bindings/soc/tegra-pmc.h>
 
+#include "tegra124-peripherals-opp.dtsi"
+
 / {
 	compatible = "nvidia,tegra124";
 	interrupt-parent = <&lic>;
@@ -290,6 +292,9 @@ actmon@6000c800 {
 		clock-names = "actmon", "emc";
 		resets = <&tegra_car 119>;
 		reset-names = "actmon";
+		operating-points-v2 = <&emc_bw_dfs_opp_table>;
+		interconnects = <&mc TEGRA124_MC_MPCORER &emc>;
+		interconnect-names = "cpu-read";
 	};
 
 	gpio: gpio@6000d000 {
@@ -660,6 +665,7 @@ emc: external-memory-controller@7001b000 {
 		clock-names = "emc";
 
 		nvidia,memory-controller = <&mc>;
+		operating-points-v2 = <&emc_icc_dvfs_opp_table>;
 
 		#interconnect-cells = <0>;
 	};
Signature · 1 line
2.29.2