2020-06-09 13:17:14

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 00/37] Introduce memory interconnect for NVIDIA Tegra SoCs

Hello,

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

For the starter only display controllers 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.

Changelog:

v4: - All drivers that use interconnect API now select it in the Kconfig in
order to properly express the build dependency.

- The IS_ENABLED(CONFIG_INTERCONNECT) is dropped now from all patches.

- Added MODULE_AUTHOR() to the modularized drivers, for completeness.

- Added missed TEGRA_MC Kconfig dependency for the Tegra20 EMC driver.

- Added more acks from Rob Herring that I accidentally missed to add in v3.

v3: - Added acks from Rob Herring that were given to some of the v2 patches.

- Specified name of the TRM documentation chapter in the patch
"dt-bindings: host1x: Document new interconnect properties", which was
suggested by Rob Herring in the review comment to v2.

- Added patches that allow EMC drivers to be compiled as a loadable kernel
modules. This came up during of the v2 review when Georgi Djakov pointed
out that interconnect-core could be compiled as a kernel module. Please
note that the Tegra124 EMC driver is compile-tested only, I don't have
Tegra124 HW.

- In the review comment to [1] Stephen Boyd suggested that it will be
better not to make changes to clk API, which was needed in order to
avoid clashing of the interconnect driver with the devfreq in regards
to memory clk-rate rounding.

[1] https://patchwork.ozlabs.org/project/linux-tegra/patch/[email protected]/

Stephen Boyd suggested that instead we should provide OPP table via DT.
I tried to investigate whether this could be done and turned out
it's a bit complicated. Technically it should be doable, but:

1. For now we don't fully support voltage scaling of the CORE regulator
and so OPP table in the DT isn't really needed today. We can
generate table from the memory timings, which is what Tegra devfreq
drivers already do.

2. The OPP table should be defined in the DT for the Memory Controller
node and then its usage somehow should be shared by both interconnect
and devfreq drivers. It's not obvious what's the best way to do it.

So, it will be much better to postpone the DT OPP table addition
until these questions are resolved. We can infer OPPs from the
memory timings and we could get the memory rates from the memory
driver directly, avoiding the problems induced by the clk API usage.
This idea is implemented in v3, see these patches:

PM / devfreq: tegra20: Use MC timings for building OPP table
PM / devfreq: tegra30: Use MC timings for building OPP table

v2: - Instead of a single dma-mem interconnect path, the paths are now
defined per memory client.

- The EMC provider now uses #interconnect-cells=<0>.

- Dropped Tegra124 because there is no enough information about how to
properly calculate required EMC clock rate for it and I don't have
hardware for testing. Somebody else will have to work on it.

- Moved interconnect providers code into drivers/memory/tegra/*.

- Added "Create tegra20-devfreq device" patch because interconnect
is not very usable without the devfreq memory auto-scaling since
memory freq will be fixed to the display's requirement.

Artur Świgoń (1):
interconnect: Relax requirement in of_icc_get_from_provider()

Dmitry Osipenko (36):
clk: Export clk_hw_reparent()
clk: tegra: Remove Memory Controller lock
clk: tegra: Export Tegra20 EMC kernel symbols
memory: tegra20-emc: Make driver modular
memory: tegra30-emc: Make driver modular
memory: tegra124-emc: Make driver modular
memory: tegra124-emc: Use devm_platform_ioremap_resource
soc/tegra: fuse: Export tegra_read_ram_code()
memory: tegra20-emc: Initialize MC timings
PM / devfreq: tegra20: Silence deferred probe error
PM / devfreq: tegra30: Silence deferred probe error
PM / devfreq: tegra20: Use MC timings for building OPP table
PM / devfreq: tegra30: Use MC timings for building OPP table
PM / devfreq: tegra20: Add error messages to tegra_devfreq_target()
PM / devfreq: tegra30: Add error messages to tegra_devfreq_target()
PM / devfreq: tegra20: Adjust clocks conversion ratio and polling
interval
PM / devfreq: tegra20: Relax Kconfig dependency
dt-bindings: memory: tegra20: mc: Document new interconnect property
dt-bindings: memory: tegra20: emc: Document new interconnect property
dt-bindings: memory: tegra30: mc: Document new interconnect property
dt-bindings: memory: tegra30: emc: Document new interconnect property
dt-bindings: host1x: Document new interconnect properties
dt-bindings: memory: tegra20: Add memory client IDs
dt-bindings: memory: tegra30: Add memory client IDs
ARM: tegra: Add interconnect properties to Tegra20 device-tree
ARM: tegra: Add interconnect properties to Tegra30 device-tree
memory: tegra: Register as interconnect provider
memory: tegra20-emc: Use devm_platform_ioremap_resource
memory: tegra20-emc: Continue probing if timings are missing in
device-tree
memory: tegra20-emc: Register as interconnect provider
memory: tegra20-emc: Create tegra20-devfreq device
memory: tegra30-emc: Continue probing if timings are missing in
device-tree
memory: tegra30-emc: Register as interconnect provider
drm/tegra: dc: Support memory bandwidth management
drm/tegra: dc: Tune up high priority request controls for Tegra20
drm/tegra: dc: Extend debug stats with total number of events

.../display/tegra/nvidia,tegra20-host1x.txt | 68 +++++
.../memory-controllers/nvidia,tegra20-emc.txt | 2 +
.../memory-controllers/nvidia,tegra20-mc.txt | 3 +
.../nvidia,tegra30-emc.yaml | 6 +
.../memory-controllers/nvidia,tegra30-mc.yaml | 5 +
arch/arm/boot/dts/tegra20.dtsi | 22 +-
arch/arm/boot/dts/tegra30.dtsi | 23 +-
drivers/clk/clk.c | 1 +
drivers/clk/tegra/clk-divider.c | 4 +-
drivers/clk/tegra/clk-tegra114.c | 6 +-
drivers/clk/tegra/clk-tegra124-emc.c | 63 ++--
drivers/clk/tegra/clk-tegra124.c | 8 +-
drivers/clk/tegra/clk-tegra20-emc.c | 3 +
drivers/clk/tegra/clk-tegra20.c | 3 +-
drivers/clk/tegra/clk-tegra30.c | 3 +-
drivers/clk/tegra/clk.h | 14 +-
drivers/devfreq/Kconfig | 2 +-
drivers/devfreq/tegra20-devfreq.c | 52 ++--
drivers/devfreq/tegra30-devfreq.c | 115 +++++--
drivers/gpu/drm/tegra/Kconfig | 1 +
drivers/gpu/drm/tegra/dc.c | 289 +++++++++++++++++-
drivers/gpu/drm/tegra/dc.h | 13 +
drivers/gpu/drm/tegra/drm.c | 19 ++
drivers/gpu/drm/tegra/plane.c | 1 +
drivers/gpu/drm/tegra/plane.h | 4 +-
drivers/interconnect/core.c | 11 +-
drivers/memory/tegra/Kconfig | 9 +-
drivers/memory/tegra/mc.c | 117 +++++++
drivers/memory/tegra/mc.h | 8 +
drivers/memory/tegra/tegra124-emc.c | 36 ++-
drivers/memory/tegra/tegra20-emc.c | 215 +++++++++++--
drivers/memory/tegra/tegra30-emc.c | 156 ++++++++--
drivers/soc/tegra/fuse/tegra-apbmisc.c | 2 +
include/dt-bindings/memory/tegra20-mc.h | 53 ++++
include/dt-bindings/memory/tegra30-mc.h | 67 ++++
include/linux/clk/tegra.h | 11 +
include/soc/tegra/emc.h | 16 -
include/soc/tegra/mc.h | 3 +
38 files changed, 1235 insertions(+), 199 deletions(-)
delete mode 100644 include/soc/tegra/emc.h

--
2.26.0


2020-06-09 13:17:41

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 12/37] PM / devfreq: tegra20: Use MC timings for building OPP table

The clk_round_rate() won't be usable for building OPP table once
interconnect support will be added to the EMC driver because that CLK API
function limits the rounded rate based on the clk rate that is imposed by
active clk-users, and thus, the rounding won't work as expected if
interconnect will set the minimum EMC clock rate before devfreq driver is
loaded. The struct tegra_mc contains memory timings which could be used by
the devfreq driver for building up OPP table instead of rounding clock
rate, this patch implements this idea.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/devfreq/tegra20-devfreq.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
index 6469dc69c5e0..bf504ca4dea2 100644
--- a/drivers/devfreq/tegra20-devfreq.c
+++ b/drivers/devfreq/tegra20-devfreq.c
@@ -123,8 +123,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
{
struct tegra_devfreq *tegra;
struct tegra_mc *mc;
- unsigned long max_rate;
- unsigned long rate;
+ unsigned int i;
int err;

mc = tegra_get_memory_controller();
@@ -151,12 +150,17 @@ static int tegra_devfreq_probe(struct platform_device *pdev)

tegra->regs = mc->regs;

- max_rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
-
- for (rate = 0; rate <= max_rate; rate++) {
- rate = clk_round_rate(tegra->emc_clock, rate);
+ if (!mc->num_timings) {
+ err = dev_pm_opp_add(&pdev->dev,
+ clk_get_rate(tegra->emc_clock), 0);
+ if (err) {
+ dev_err(&pdev->dev, "failed to add OPP: %d\n", err);
+ return err;
+ }
+ }

- err = dev_pm_opp_add(&pdev->dev, rate, 0);
+ for (i = 0; i < mc->num_timings; i++) {
+ err = dev_pm_opp_add(&pdev->dev, mc->timings[i].rate, 0);
if (err) {
dev_err(&pdev->dev, "failed to add opp: %d\n", err);
goto remove_opps;
--
2.26.0

2020-06-09 13:18:11

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 15/37] PM / devfreq: tegra30: Add error messages to tegra_devfreq_target()

It's useful to now when something goes wrong instead of failing silently,
so let's add error messages to tegra_devfreq_target() to prevent situation
where it fails silently.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/devfreq/tegra30-devfreq.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 13f93c6038ab..a03fb16c5c4c 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -641,12 +641,16 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
dev_pm_opp_put(opp);

err = clk_set_min_rate(tegra->emc_clock, rate * KHZ);
- if (err)
+ if (err) {
+ dev_err(dev, "Failed to set min rate: %d\n", err);
return err;
+ }

err = clk_set_rate(tegra->emc_clock, 0);
- if (err)
+ if (err) {
+ dev_err(dev, "Failed to set rate: %d\n", err);
goto restore_min_rate;
+ }

return 0;

--
2.26.0

2020-06-09 13:18:15

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 27/37] interconnect: Relax requirement in of_icc_get_from_provider()

From: Artur Świgoń <[email protected]>

This patch relaxes the condition in of_icc_get_from_provider() so that it
is no longer required to set #interconnect-cells = <1> in the DT. In case
of the devfreq driver for exynos-bus, #interconnect-cells is always zero.

Signed-off-by: Artur Świgoń <[email protected]>
[[email protected]: added cells_num checking for of_icc_xlate_onecell()]
Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/interconnect/core.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index e5f998744501..cb143421ca67 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -339,7 +339,7 @@ static struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec)
struct icc_node *node = ERR_PTR(-EPROBE_DEFER);
struct icc_provider *provider;

- if (!spec || spec->args_count != 1)
+ if (!spec)
return ERR_PTR(-EINVAL);

mutex_lock(&icc_lock);
@@ -967,6 +967,15 @@ EXPORT_SYMBOL_GPL(icc_nodes_remove);
*/
int icc_provider_add(struct icc_provider *provider)
{
+ struct device_node *np = provider->dev->of_node;
+ u32 cells_num;
+ int err;
+
+ err = of_property_read_u32(np, "#interconnect-cells", &cells_num);
+ if (WARN_ON(err))
+ return err;
+ if (WARN_ON(provider->xlate == of_icc_xlate_onecell && cells_num != 1))
+ return -EINVAL;
if (WARN_ON(!provider->set))
return -EINVAL;
if (WARN_ON(!provider->xlate))
--
2.26.0

2020-06-09 13:18:27

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 16/37] PM / devfreq: tegra20: Adjust clocks conversion ratio and polling interval

The current conversion ratio results in a higher frequency than needed,
that is not very actual now since the Display Controller driver got
support for memory bandwidth management and hence memory frequency can
go lower now without bad consequences. Since memory freq now goes to a
lower rates, the responsiveness of interactive applications become worse
due to a quite high polling interval value that is currently set to 500ms.
Changing polling interval to 30ms results in a good responsiveness of the
system.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/devfreq/tegra20-devfreq.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
index 249d0dc44f6c..7cdea4ba38f7 100644
--- a/drivers/devfreq/tegra20-devfreq.c
+++ b/drivers/devfreq/tegra20-devfreq.c
@@ -79,16 +79,12 @@ static int tegra_devfreq_get_dev_status(struct device *dev,

/*
* EMC_COUNT returns number of memory events, that number is lower
- * than the number of clocks. Conversion ratio of 1/8 results in a
- * bit higher bandwidth than actually needed, it is good enough for
- * the time being because drivers don't support requesting minimum
- * needed memory bandwidth yet.
- *
- * TODO: adjust the ratio value once relevant drivers will support
- * memory bandwidth management.
+ * than the number of total EMC clocks over the sampling period.
+ * The clocks number is converted to maximum possible number of
+ * memory events using the ratio of 1/4.
*/
stat->busy_time = readl_relaxed(tegra->regs + MC_STAT_EMC_COUNT);
- stat->total_time = readl_relaxed(tegra->regs + MC_STAT_EMC_CLOCKS) / 8;
+ stat->total_time = readl_relaxed(tegra->regs + MC_STAT_EMC_CLOCKS) / 4;
stat->current_frequency = clk_get_rate(tegra->emc_clock);

writel_relaxed(EMC_GATHER_CLEAR, tegra->regs + MC_STAT_CONTROL);
@@ -98,7 +94,7 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
}

static struct devfreq_dev_profile tegra_devfreq_profile = {
- .polling_ms = 500,
+ .polling_ms = 30,
.target = tegra_devfreq_target,
.get_dev_status = tegra_devfreq_get_dev_status,
};
--
2.26.0

2020-06-09 13:18:32

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 18/37] dt-bindings: memory: tegra20: mc: Document new interconnect property

Memory controller is interconnected with memory clients and with the
external memory controller. Document new interconnect property which
turns memory controller into interconnect provider.

Acked-by: Rob Herring <[email protected]>
Signed-off-by: Dmitry Osipenko <[email protected]>
---
.../bindings/memory-controllers/nvidia,tegra20-mc.txt | 3 +++
1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
index e55328237df4..739b7c6f2e26 100644
--- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-mc.txt
@@ -16,6 +16,8 @@ Required properties:
IOMMU specifier needed to encode an address. GART supports only a single
address space that is shared by all devices, therefore no additional
information needed for the address encoding.
+- #interconnect-cells : Should be 1. This cell represents memory client.
+ The assignments may be found in header file <dt-bindings/memory/tegra20-mc.h>.

Example:
mc: memory-controller@7000f000 {
@@ -27,6 +29,7 @@ Example:
interrupts = <GIC_SPI 77 0x04>;
#reset-cells = <1>;
#iommu-cells = <0>;
+ #interconnect-cells = <1>;
};

video-codec@6001a000 {
--
2.26.0

2020-06-09 13:18:41

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 35/37] drm/tegra: dc: Support memory bandwidth management

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. This patch wires up the interconnect
API support to the DC driver.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/gpu/drm/tegra/Kconfig | 1 +
drivers/gpu/drm/tegra/dc.c | 271 +++++++++++++++++++++++++++++++++-
drivers/gpu/drm/tegra/dc.h | 8 +
drivers/gpu/drm/tegra/drm.c | 19 +++
drivers/gpu/drm/tegra/plane.c | 1 +
drivers/gpu/drm/tegra/plane.h | 4 +-
6 files changed, 300 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/Kconfig b/drivers/gpu/drm/tegra/Kconfig
index 5043dcaf1cf9..1650a448eabd 100644
--- a/drivers/gpu/drm/tegra/Kconfig
+++ b/drivers/gpu/drm/tegra/Kconfig
@@ -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
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 83f31c6e891c..12b318bb8475 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -519,6 +519,136 @@ static void tegra_dc_setup_window(struct tegra_plane *plane,
tegra_plane_setup_blending(plane, window);
}

+static unsigned long
+tegra_plane_memory_bandwidth(struct drm_plane_state *state,
+ struct tegra_dc_window *window,
+ unsigned int num,
+ unsigned int denum)
+{
+ struct tegra_plane_state *tegra_state;
+ struct drm_crtc_state *crtc_state;
+ const struct drm_format_info *fmt;
+ struct tegra_dc_window win;
+ unsigned long long bandwidth;
+ unsigned int bpp_plane;
+ unsigned int bpp;
+ unsigned int mul;
+ unsigned int i;
+
+ if (!state->fb || !state->visible)
+ return 0;
+
+ crtc_state = drm_atomic_get_new_crtc_state(state->state, state->crtc);
+ tegra_state = to_tegra_plane_state(state);
+
+ if (!window)
+ window = &win;
+
+ window->src.w = drm_rect_width(&state->src) >> 16;
+ window->src.h = drm_rect_height(&state->src) >> 16;
+ window->dst.w = drm_rect_width(&state->dst);
+ window->dst.h = drm_rect_height(&state->dst);
+ window->tiling = tegra_state->tiling;
+
+ fmt = state->fb->format;
+
+ /*
+ * Note that real memory bandwidth vary depending on format and
+ * memory layout, we are not taking that into account because small
+ * estimation error isn't important since bandwidth is rounded up
+ * anyway.
+ */
+ for (i = 0, bpp = 0; i < fmt->num_planes; i++) {
+ bpp_plane = fmt->cpp[i] * 8;
+
+ /*
+ * Sub-sampling is relevant for chroma planes only and vertical
+ * readouts are not cached, hence only horizontal sub-sampling
+ * matters.
+ */
+ if (i > 0)
+ bpp_plane /= fmt->hsub;
+
+ bpp += bpp_plane;
+ }
+
+ /*
+ * Horizontal downscale takes extra bandwidth which roughly depends
+ * on the scaled width.
+ */
+ if (window->src.w > window->dst.w)
+ mul = (window->src.w - window->dst.w) * bpp / 2048 + 1;
+ else
+ mul = 1;
+
+ /*
+ * Ignore cursor window if its width is small enough such that
+ * data-prefetch FIFO will easily help to overcome temporal memory
+ * pressure.
+ *
+ * Window A has a 128bit x 128 deep read FIFO, while windows B/C
+ * have a 128bit x 64 deep read FIFO.
+ *
+ * This allows us to not overestimate memory frequency requirement.
+ * Even if it will happen that cursor gets a temporal underflow, this
+ * won't be fatal.
+ */
+ if (state->plane->type == DRM_PLANE_TYPE_CURSOR &&
+ mul == 1 && window->src.w * bpp <= 128 * 16)
+ return 0;
+
+ /* mode.clock in kHz, bandwidth in kbit/s */
+ bandwidth = kbps_to_icc(crtc_state->mode.clock * bpp * mul);
+
+ /* the requested bandwidth should be higher than required */
+ bandwidth *= num;
+ do_div(bandwidth, denum);
+
+ return min_t(u64, bandwidth, ULONG_MAX);
+}
+
+static unsigned long
+tegra20_plane_memory_bandwidth(struct drm_plane_state *state)
+{
+ return tegra_plane_memory_bandwidth(state, NULL, 29, 10);
+}
+
+static unsigned long
+tegra30_plane_memory_bandwidth(struct drm_plane_state *state)
+{
+ struct tegra_dc_window window;
+ unsigned long bandwidth;
+
+ bandwidth = tegra_plane_memory_bandwidth(state, &window, 29, 10);
+
+ /* x2: memory overfetch for tiled framebuffer and DDR3 */
+ if (window.tiling.mode == TEGRA_BO_TILING_MODE_TILED)
+ bandwidth *= 2;
+
+ return bandwidth;
+}
+
+static unsigned long
+tegra114_plane_memory_bandwidth(struct drm_plane_state *state)
+{
+ struct tegra_dc_window window;
+ unsigned long bandwidth;
+
+ bandwidth = tegra_plane_memory_bandwidth(state, &window, 12, 10);
+
+ /* x2: memory overfetch for tiled framebuffer and DDR3 */
+ if (window.tiling.mode == TEGRA_BO_TILING_MODE_TILED)
+ bandwidth *= 2;
+
+ return bandwidth;
+}
+
+static unsigned long
+tegra124_plane_memory_bandwidth(struct drm_plane_state *state)
+{
+ return tegra_plane_memory_bandwidth(state, NULL, 12, 10);
+}
+
static const u32 tegra20_primary_formats[] = {
DRM_FORMAT_ARGB4444,
DRM_FORMAT_ARGB1555,
@@ -608,8 +738,10 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
int err;

/* no need for further checks if the plane is being disabled */
- if (!state->crtc)
+ if (!state->crtc) {
+ plane_state->memory_bandwidth = 0;
return 0;
+ }

err = tegra_plane_format(state->fb->format->format,
&plane_state->format,
@@ -662,6 +794,8 @@ static int tegra_plane_atomic_check(struct drm_plane *plane,
if (err < 0)
return err;

+ plane_state->memory_bandwidth = dc->soc->plane_memory_bandwidth(state);
+
return 0;
}

@@ -1186,6 +1320,7 @@ tegra_crtc_atomic_duplicate_state(struct drm_crtc *crtc)
copy->pclk = state->pclk;
copy->div = state->div;
copy->planes = state->planes;
+ copy->memory_bandwidth = state->memory_bandwidth;

return &copy->base;
}
@@ -1768,6 +1903,8 @@ static void tegra_crtc_atomic_disable(struct drm_crtc *crtc,
err = host1x_client_suspend(&dc->client);
if (err < 0)
dev_err(dc->dev, "failed to suspend: %d\n", err);
+
+ icc_set_bw(dc->icc_bandwidth, 0, 0);
}

static void tegra_crtc_atomic_enable(struct drm_crtc *crtc,
@@ -1779,6 +1916,9 @@ static void tegra_crtc_atomic_enable(struct drm_crtc *crtc,
u32 value;
int err;

+ icc_set_bw(dc->icc_bandwidth, state->memory_bandwidth,
+ state->memory_bandwidth);
+
err = host1x_client_resume(&dc->client);
if (err < 0) {
dev_err(dc->dev, "failed to resume: %d\n", err);
@@ -1892,6 +2032,9 @@ static void tegra_crtc_atomic_enable(struct drm_crtc *crtc,
static void tegra_crtc_atomic_begin(struct drm_crtc *crtc,
struct drm_crtc_state *old_crtc_state)
{
+ struct tegra_dc_state *dc_old_state = to_dc_state(old_crtc_state);
+ struct tegra_dc_state *dc_state = to_dc_state(crtc->state);
+ struct tegra_dc *dc = to_tegra_dc(crtc);
unsigned long flags;

if (crtc->state->event) {
@@ -1906,6 +2049,25 @@ static void tegra_crtc_atomic_begin(struct drm_crtc *crtc,

crtc->state->event = NULL;
}
+
+ if (old_crtc_state && old_crtc_state->active) {
+ /*
+ * Raise memory bandwidth before changes take effect if it
+ * goes from low to high.
+ */
+ if (dc_old_state->memory_bandwidth < dc_state->memory_bandwidth)
+ icc_set_bw(dc->icc_bandwidth,
+ dc_state->memory_bandwidth,
+ dc_state->memory_bandwidth);
+ } else {
+ /*
+ * Raise memory bandwidth before changes take effect if
+ * CRTC is turning on.
+ */
+ icc_set_bw(dc->icc_bandwidth,
+ dc_state->memory_bandwidth,
+ dc_state->memory_bandwidth);
+ }
}

static void tegra_crtc_atomic_flush(struct drm_crtc *crtc,
@@ -1924,7 +2086,80 @@ static void tegra_crtc_atomic_flush(struct drm_crtc *crtc,
value = tegra_dc_readl(dc, DC_CMD_STATE_CONTROL);
}

+static bool
+tegra_plane_overlaps_other_plane(struct drm_crtc_state *state,
+ const struct drm_plane_state *plane_state)
+{
+ const struct drm_plane_state *other_state;
+ struct drm_plane *plane;
+ struct drm_rect rect;
+
+ drm_atomic_crtc_state_for_each_plane_state(plane, other_state, state) {
+ rect = plane_state->dst;
+
+ if (other_state == plane_state)
+ continue;
+
+ if (!other_state->visible || !other_state->fb)
+ continue;
+
+ if (drm_rect_intersect(&rect, &other_state->dst))
+ return true;
+ }
+
+ return false;
+}
+
+static int tegra_crtc_atomic_check(struct drm_crtc *crtc,
+ struct drm_crtc_state *state)
+{
+ struct tegra_dc_state *dc_state = to_dc_state(state);
+ const struct drm_plane_state *plane_state;
+ const struct tegra_plane_state *tegra;
+ unsigned long long bandwidth = 0;
+ struct drm_plane *plane;
+
+ /*
+ * For overlapping planes pixel's data is fetched for each plane at
+ * the same time, hence bandwidth is accumulated in this case.
+ */
+ drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, state) {
+ tegra = to_tegra_plane_state(plane_state);
+
+ if (tegra_plane_overlaps_other_plane(state, plane_state))
+ bandwidth += tegra->memory_bandwidth;
+ else
+ bandwidth = max_t(u64, bandwidth,
+ tegra->memory_bandwidth);
+ }
+
+ dc_state->memory_bandwidth = min_t(u64, bandwidth, U32_MAX);
+
+ return 0;
+}
+
+void tegra_crtc_atomic_post_commit(struct drm_crtc *crtc,
+ struct drm_crtc_state *old_crtc_state)
+{
+ struct tegra_dc_state *dc_old_state = to_dc_state(old_crtc_state);
+ struct tegra_dc_state *dc_state = to_dc_state(crtc->state);
+ struct tegra_dc *dc = to_tegra_dc(crtc);
+
+ if (!dc_old_state)
+ return;
+
+ /*
+ * Drop memory bandwidth after changes take effect if it goes from
+ * high to low.
+ */
+ if (dc_old_state->memory_bandwidth > dc_state->memory_bandwidth)
+ icc_set_bw(dc->icc_bandwidth,
+ dc_state->memory_bandwidth,
+ dc_state->memory_bandwidth);
+}
+
static const struct drm_crtc_helper_funcs tegra_crtc_helper_funcs = {
+ .atomic_check = tegra_crtc_atomic_check,
.atomic_begin = tegra_crtc_atomic_begin,
.atomic_flush = tegra_crtc_atomic_flush,
.atomic_enable = tegra_crtc_atomic_enable,
@@ -2216,6 +2451,7 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
.modifiers = tegra20_modifiers,
.has_win_a_without_filters = true,
.has_win_c_without_vert_filter = true,
+ .plane_memory_bandwidth = tegra20_plane_memory_bandwidth,
};

static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
@@ -2235,6 +2471,7 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
.modifiers = tegra20_modifiers,
.has_win_a_without_filters = false,
.has_win_c_without_vert_filter = false,
+ .plane_memory_bandwidth = tegra30_plane_memory_bandwidth,
};

static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
@@ -2254,6 +2491,7 @@ static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
.modifiers = tegra20_modifiers,
.has_win_a_without_filters = false,
.has_win_c_without_vert_filter = false,
+ .plane_memory_bandwidth = tegra114_plane_memory_bandwidth,
};

static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
@@ -2273,6 +2511,7 @@ static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
.modifiers = tegra124_modifiers,
.has_win_a_without_filters = false,
.has_win_c_without_vert_filter = false,
+ .plane_memory_bandwidth = tegra124_plane_memory_bandwidth,
};

static const struct tegra_dc_soc_info tegra210_dc_soc_info = {
@@ -2292,6 +2531,7 @@ static const struct tegra_dc_soc_info tegra210_dc_soc_info = {
.modifiers = tegra124_modifiers,
.has_win_a_without_filters = false,
.has_win_c_without_vert_filter = false,
+ .plane_memory_bandwidth = tegra124_plane_memory_bandwidth,
};

static const struct tegra_windowgroup_soc tegra186_dc_wgrps[] = {
@@ -2340,6 +2580,7 @@ static const struct tegra_dc_soc_info tegra186_dc_soc_info = {
.has_nvdisplay = true,
.wgrps = tegra186_dc_wgrps,
.num_wgrps = ARRAY_SIZE(tegra186_dc_wgrps),
+ .plane_memory_bandwidth = tegra124_plane_memory_bandwidth,
};

static const struct tegra_windowgroup_soc tegra194_dc_wgrps[] = {
@@ -2388,6 +2629,7 @@ static const struct tegra_dc_soc_info tegra194_dc_soc_info = {
.has_nvdisplay = true,
.wgrps = tegra194_dc_wgrps,
.num_wgrps = ARRAY_SIZE(tegra194_dc_wgrps),
+ .plane_memory_bandwidth = tegra124_plane_memory_bandwidth,
};

static const struct of_device_id tegra_dc_of_match[] = {
@@ -2494,6 +2736,7 @@ static int tegra_dc_couple(struct tegra_dc *dc)

static int tegra_dc_probe(struct platform_device *pdev)
{
+ const char *level = KERN_ERR;
struct tegra_dc *dc;
int err;

@@ -2562,8 +2805,6 @@ static int tegra_dc_probe(struct platform_device *pdev)

err = tegra_dc_rgb_probe(dc);
if (err < 0 && err != -ENODEV) {
- const char *level = KERN_ERR;
-
if (err == -EPROBE_DEFER)
level = KERN_DEBUG;

@@ -2572,6 +2813,25 @@ static int tegra_dc_probe(struct platform_device *pdev)
return err;
}

+ /*
+ * The display controller memory bandwidth management isn't trivial
+ * because it requires the knowledge about the DC hardware state
+ * in order to make a proper decisions. It's not easy to convey
+ * that information to the ICC provider, so we will just use the
+ * first interconnect path for the memory bandwidth management and
+ * make all the decisions within the DC driver, for simplicity.
+ */
+ dc->icc_bandwidth = of_icc_get(dc->dev, "display0a");
+ err = PTR_ERR_OR_ZERO(dc->icc_bandwidth);
+ if (err) {
+ if (err == -EPROBE_DEFER)
+ level = KERN_DEBUG;
+
+ dev_printk(level, dc->dev,
+ "failed to get display0a interconnect: %d\n", err);
+ goto remove_rgb;
+ }
+
platform_set_drvdata(pdev, dc);
pm_runtime_enable(&pdev->dev);

@@ -2590,6 +2850,9 @@ static int tegra_dc_probe(struct platform_device *pdev)

disable_pm:
pm_runtime_disable(&pdev->dev);
+ icc_put(dc->icc_bandwidth);
+
+remove_rgb:
tegra_dc_rgb_remove(dc);

return err;
@@ -2607,6 +2870,8 @@ static int tegra_dc_remove(struct platform_device *pdev)
return err;
}

+ icc_put(dc->icc_bandwidth);
+
err = tegra_dc_rgb_remove(dc);
if (err < 0) {
dev_err(&pdev->dev, "failed to remove RGB output: %d\n", err);
diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index 3d8ddccd758f..3a0ff57c5169 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -8,6 +8,7 @@
#define TEGRA_DC_H 1

#include <linux/host1x.h>
+#include <linux/interconnect.h>

#include <drm/drm_crtc.h>

@@ -23,6 +24,8 @@ struct tegra_dc_state {
unsigned int div;

u32 planes;
+
+ unsigned long memory_bandwidth;
};

static inline struct tegra_dc_state *to_dc_state(struct drm_crtc_state *state)
@@ -66,6 +69,7 @@ struct tegra_dc_soc_info {
const u64 *modifiers;
bool has_win_a_without_filters;
bool has_win_c_without_vert_filter;
+ unsigned long (*plane_memory_bandwidth)(struct drm_plane_state *state);
};

struct tegra_dc {
@@ -90,6 +94,8 @@ struct tegra_dc {
struct drm_info_list *debugfs_files;

const struct tegra_dc_soc_info *soc;
+
+ struct icc_path *icc_bandwidth;
};

static inline struct tegra_dc *
@@ -150,6 +156,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_crtc_state *old_crtc_state);

/* from rgb.c */
int tegra_dc_rgb_probe(struct tegra_dc *dc);
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 211906347f3f..81e58f90f5c9 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -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,22 @@ 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, *new_crtc_state;
+ struct drm_crtc *crtc;
+ unsigned int i;
+
+ for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
+ new_crtc_state, i) {
+ if (!new_crtc_state->active)
+ continue;
+
+ tegra_crtc_atomic_post_commit(crtc, old_crtc_state);
+ }
+}
+
static void tegra_atomic_commit_tail(struct drm_atomic_state *old_state)
{
struct drm_device *drm = old_state->dev;
@@ -75,6 +92,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
diff --git a/drivers/gpu/drm/tegra/plane.c b/drivers/gpu/drm/tegra/plane.c
index 9ccfb56e9b01..f94925744105 100644
--- a/drivers/gpu/drm/tegra/plane.c
+++ b/drivers/gpu/drm/tegra/plane.c
@@ -63,6 +63,7 @@ tegra_plane_atomic_duplicate_state(struct drm_plane *plane)
copy->swap = state->swap;
copy->bottom_up = state->bottom_up;
copy->opaque = state->opaque;
+ copy->memory_bandwidth = state->memory_bandwidth;

for (i = 0; i < 2; i++)
copy->blending[i] = state->blending[i];
diff --git a/drivers/gpu/drm/tegra/plane.h b/drivers/gpu/drm/tegra/plane.h
index a158a915109a..5227c7a9ad8b 100644
--- a/drivers/gpu/drm/tegra/plane.h
+++ b/drivers/gpu/drm/tegra/plane.h
@@ -51,10 +51,12 @@ struct tegra_plane_state {
/* used for legacy blending support only */
struct tegra_plane_legacy_blending_state blending[2];
bool opaque;
+
+ unsigned long memory_bandwidth;
};

static inline struct tegra_plane_state *
-to_tegra_plane_state(struct drm_plane_state *state)
+to_tegra_plane_state(const struct drm_plane_state *state)
{
if (state)
return container_of(state, struct tegra_plane_state, base);
--
2.26.0

2020-06-09 13:18:42

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 34/37] memory: tegra30-emc: Register as interconnect provider

Now external memory controller is a memory interconnection provider.
This allows us to use interconnect API to change memory configuration.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/memory/tegra/tegra30-emc.c | 110 +++++++++++++++++++++++++++++
1 file changed, 110 insertions(+)

diff --git a/drivers/memory/tegra/tegra30-emc.c b/drivers/memory/tegra/tegra30-emc.c
index f6c688bfe41f..37cea4435d55 100644
--- a/drivers/memory/tegra/tegra30-emc.c
+++ b/drivers/memory/tegra/tegra30-emc.c
@@ -14,6 +14,7 @@
#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/err.h>
+#include <linux/interconnect-provider.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/iopoll.h>
@@ -327,6 +328,7 @@ struct tegra_emc {
struct device *dev;
struct tegra_mc *mc;
struct notifier_block clk_nb;
+ struct icc_provider provider;
struct clk *clk;
void __iomem *regs;
unsigned int irq;
@@ -1264,6 +1266,113 @@ static void tegra_emc_debugfs_init(struct tegra_emc *emc)
emc, &tegra_emc_debug_max_rate_fops);
}

+static inline struct tegra_emc *
+to_tegra_emc_provider(struct icc_provider *provider)
+{
+ return container_of(provider, struct tegra_emc, provider);
+}
+
+static struct icc_node *
+emc_of_icc_xlate(struct of_phandle_args *spec, void *data)
+{
+ struct icc_provider *provider = data;
+ struct icc_node *node;
+
+ /* External Memory is the only possible ICC route */
+ list_for_each_entry(node, &provider->nodes, node_list) {
+ if (node->id == TEGRA_ICC_EMEM)
+ return node;
+ }
+
+ return ERR_PTR(-EINVAL);
+}
+
+static int emc_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+ struct tegra_emc *emc = to_tegra_emc_provider(dst->provider);
+ unsigned long long rate = icc_units_to_bps(dst->avg_bw);
+ unsigned int dram_data_bus_width_bytes = 4;
+ unsigned int ddr = 2;
+ int err;
+
+ do_div(rate, ddr * dram_data_bus_width_bytes);
+ rate = min_t(u64, rate, U32_MAX);
+
+ err = clk_set_min_rate(emc->clk, rate);
+ if (err)
+ return err;
+
+ err = clk_set_rate(emc->clk, rate);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+static int emc_icc_aggregate(struct icc_node *node,
+ u32 tag, u32 avg_bw, u32 peak_bw,
+ u32 *agg_avg, u32 *agg_peak)
+{
+ *agg_avg = min((u64)avg_bw + (*agg_avg), (u64)U32_MAX);
+ *agg_peak = max(*agg_peak, peak_bw);
+
+ return 0;
+}
+
+static int tegra_emc_interconnect_init(struct tegra_emc *emc)
+{
+ struct icc_node *node;
+ int err;
+
+ /* older device-trees don't have interconnect properties */
+ if (!of_find_property(emc->dev->of_node, "#interconnect-cells", NULL))
+ return 0;
+
+ emc->provider.dev = emc->dev;
+ emc->provider.set = emc_icc_set;
+ emc->provider.data = &emc->provider;
+ emc->provider.xlate = emc_of_icc_xlate;
+ emc->provider.aggregate = emc_icc_aggregate;
+
+ err = icc_provider_add(&emc->provider);
+ if (err)
+ goto err_msg;
+
+ /* create External Memory Controller node */
+ node = icc_node_create(TEGRA_ICC_EMC);
+ err = PTR_ERR_OR_ZERO(node);
+ if (err)
+ goto del_provider;
+
+ node->name = "External Memory Controller";
+ icc_node_add(node, &emc->provider);
+
+ /* link External Memory Controller to External Memory (DRAM) */
+ err = icc_link_create(node, TEGRA_ICC_EMEM);
+ if (err)
+ goto remove_nodes;
+
+ /* create External Memory node */
+ node = icc_node_create(TEGRA_ICC_EMEM);
+ err = PTR_ERR_OR_ZERO(node);
+ if (err)
+ goto remove_nodes;
+
+ node->name = "External Memory (DRAM)";
+ icc_node_add(node, &emc->provider);
+
+ return 0;
+
+remove_nodes:
+ icc_nodes_remove(&emc->provider);
+del_provider:
+ icc_provider_del(&emc->provider);
+err_msg:
+ dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
+
+ return err;
+}
+
static int tegra_emc_probe(struct platform_device *pdev)
{
struct platform_device *mc;
@@ -1343,6 +1452,7 @@ static int tegra_emc_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, emc);
tegra_emc_debugfs_init(emc);
+ tegra_emc_interconnect_init(emc);

/*
* Don't allow the kernel module to be unloaded. Unloading adds some
--
2.26.0

2020-06-09 13:18:48

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 33/37] memory: tegra30-emc: Continue probing if timings are missing in device-tree

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.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/memory/tegra/tegra30-emc.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/memory/tegra/tegra30-emc.c b/drivers/memory/tegra/tegra30-emc.c
index 85d4effb8e6f..f6c688bfe41f 100644
--- a/drivers/memory/tegra/tegra30-emc.c
+++ b/drivers/memory/tegra/tegra30-emc.c
@@ -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++) {
@@ -1263,12 +1271,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 = of_parse_phandle(pdev->dev.of_node, "nvidia,memory-controller", 0);
if (!np) {
dev_err(&pdev->dev, "could not get memory controller node\n");
@@ -1280,10 +1282,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
if (!mc)
return -ENOENT;

- 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);
@@ -1297,10 +1295,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))
--
2.26.0

2020-06-09 13:18:54

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 32/37] memory: tegra20-emc: Create tegra20-devfreq device

The tegra20-devfreq driver provides memory frequency scaling functionality
and it uses EMC clock for the scaling. Since tegra20-devfreq is a software
driver, the device for the driver needs to be created manually. Let's do
it from EMC driver since it provides the clk rate-change functionality.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/memory/tegra/tegra20-emc.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c
index ef0f5a69f24b..59d85e527516 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -877,6 +877,9 @@ static int tegra_emc_probe(struct platform_device *pdev)
tegra_emc_debugfs_init(emc);
tegra_emc_interconnect_init(emc);

+ if (IS_ENABLED(CONFIG_ARM_TEGRA20_DEVFREQ))
+ platform_device_register_simple("tegra20-devfreq", -1, NULL, 0);
+
/*
* 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
--
2.26.0

2020-06-09 13:18:55

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 37/37] drm/tegra: dc: Extend debug stats with total number of events

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 reset.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/gpu/drm/tegra/dc.c | 10 ++++++++++
drivers/gpu/drm/tegra/dc.h | 5 +++++
2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 48dad375b470..6a5a017e37d5 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -1616,6 +1616,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;
}

@@ -2178,6 +2183,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++;
}

@@ -2186,6 +2192,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++;
}

@@ -2193,6 +2200,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++;
}

@@ -2200,11 +2208,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++;
}

diff --git a/drivers/gpu/drm/tegra/dc.h b/drivers/gpu/drm/tegra/dc.h
index 3a0ff57c5169..3eb4eddc2288 100644
--- a/drivers/gpu/drm/tegra/dc.h
+++ b/drivers/gpu/drm/tegra/dc.h
@@ -41,6 +41,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 {
--
2.26.0

2020-06-09 13:19:00

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 31/37] memory: tegra20-emc: Register as interconnect provider

Now memory controller is a memory interconnection provider. This allows us
to use interconnect API in order to change memory configuration.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/memory/tegra/tegra20-emc.c | 110 +++++++++++++++++++++++++++++
1 file changed, 110 insertions(+)

diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c
index 6cd3d50145dc..ef0f5a69f24b 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -9,6 +9,7 @@
#include <linux/clk/tegra.h>
#include <linux/debugfs.h>
#include <linux/err.h>
+#include <linux/interconnect-provider.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/iopoll.h>
@@ -148,6 +149,7 @@ struct emc_timing {
struct tegra_emc {
struct device *dev;
struct notifier_block clk_nb;
+ struct icc_provider provider;
struct clk *clk;
void __iomem *regs;

@@ -661,6 +663,113 @@ static void tegra_emc_debugfs_init(struct tegra_emc *emc)
emc, &tegra_emc_debug_max_rate_fops);
}

+static inline struct tegra_emc *
+to_tegra_emc_provider(struct icc_provider *provider)
+{
+ return container_of(provider, struct tegra_emc, provider);
+}
+
+static struct icc_node *
+emc_of_icc_xlate(struct of_phandle_args *spec, void *data)
+{
+ struct icc_provider *provider = data;
+ struct icc_node *node;
+
+ /* External Memory is the only possible ICC route */
+ list_for_each_entry(node, &provider->nodes, node_list) {
+ if (node->id == TEGRA_ICC_EMEM)
+ return node;
+ }
+
+ return ERR_PTR(-EINVAL);
+}
+
+static int emc_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+ struct tegra_emc *emc = to_tegra_emc_provider(dst->provider);
+ unsigned long long rate = icc_units_to_bps(dst->avg_bw);
+ unsigned int dram_data_bus_width_bytes = 4;
+ unsigned int ddr = 2;
+ int err;
+
+ do_div(rate, ddr * dram_data_bus_width_bytes);
+ rate = min_t(u64, rate, U32_MAX);
+
+ err = clk_set_min_rate(emc->clk, rate);
+ if (err)
+ return err;
+
+ err = clk_set_rate(emc->clk, rate);
+ if (err)
+ return err;
+
+ return 0;
+}
+
+static int emc_icc_aggregate(struct icc_node *node,
+ u32 tag, u32 avg_bw, u32 peak_bw,
+ u32 *agg_avg, u32 *agg_peak)
+{
+ *agg_avg = min((u64)avg_bw + (*agg_avg), (u64)U32_MAX);
+ *agg_peak = max(*agg_peak, peak_bw);
+
+ return 0;
+}
+
+static int tegra_emc_interconnect_init(struct tegra_emc *emc)
+{
+ struct icc_node *node;
+ int err;
+
+ /* older device-trees don't have interconnect properties */
+ if (!of_find_property(emc->dev->of_node, "#interconnect-cells", NULL))
+ return 0;
+
+ emc->provider.dev = emc->dev;
+ emc->provider.set = emc_icc_set;
+ emc->provider.data = &emc->provider;
+ emc->provider.xlate = emc_of_icc_xlate;
+ emc->provider.aggregate = emc_icc_aggregate;
+
+ err = icc_provider_add(&emc->provider);
+ if (err)
+ goto err_msg;
+
+ /* create External Memory Controller node */
+ node = icc_node_create(TEGRA_ICC_EMC);
+ err = PTR_ERR_OR_ZERO(node);
+ if (err)
+ goto del_provider;
+
+ node->name = "External Memory Controller";
+ icc_node_add(node, &emc->provider);
+
+ /* link External Memory Controller to External Memory (DRAM) */
+ err = icc_link_create(node, TEGRA_ICC_EMEM);
+ if (err)
+ goto remove_nodes;
+
+ /* create External Memory node */
+ node = icc_node_create(TEGRA_ICC_EMEM);
+ err = PTR_ERR_OR_ZERO(node);
+ if (err)
+ goto remove_nodes;
+
+ node->name = "External Memory (DRAM)";
+ icc_node_add(node, &emc->provider);
+
+ return 0;
+
+remove_nodes:
+ icc_nodes_remove(&emc->provider);
+del_provider:
+ icc_provider_del(&emc->provider);
+err_msg:
+ dev_err(emc->dev, "failed to initialize ICC: %d\n", err);
+
+ return err;
+}
+
static int tegra_emc_init_mc_timings(struct tegra_emc *emc)
{
struct tegra_mc_timing *timing;
@@ -766,6 +875,7 @@ static int tegra_emc_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, emc);
tegra_emc_debugfs_init(emc);
+ tegra_emc_interconnect_init(emc);

/*
* Don't allow the kernel module to be unloaded. Unloading adds some
--
2.26.0

2020-06-09 13:19:09

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 36/37] drm/tegra: dc: Tune up high priority request controls for Tegra20

Tegra20 has a high-priority-request control that allows to configure
when display's memory client should perform read requests with a higher
priority (Tegra30+ uses other means like Latency Allowance).

This patch changes the controls configuration in order to get a more
aggressive memory prefetching, which allows to reliably avoid FIFO
underflow when running on a lower memory frequency. This allow us
safely drop the memory bandwidth requirement by about two times in a
most popular use-cases (only one display active, video overlay inactive,
no scaling is done).

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/gpu/drm/tegra/dc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 12b318bb8475..48dad375b470 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -1971,12 +1971,12 @@ static void tegra_crtc_atomic_enable(struct drm_crtc *crtc,
tegra_dc_writel(dc, value, DC_CMD_INT_POLARITY);

/* initialize timer */
- value = CURSOR_THRESHOLD(0) | WINDOW_A_THRESHOLD(0x20) |
- WINDOW_B_THRESHOLD(0x20) | WINDOW_C_THRESHOLD(0x20);
+ value = CURSOR_THRESHOLD(0) | WINDOW_A_THRESHOLD(0x70) |
+ WINDOW_B_THRESHOLD(0x30) | WINDOW_C_THRESHOLD(0x70);
tegra_dc_writel(dc, value, DC_DISP_DISP_MEM_HIGH_PRIORITY);

- value = CURSOR_THRESHOLD(0) | WINDOW_A_THRESHOLD(1) |
- WINDOW_B_THRESHOLD(1) | WINDOW_C_THRESHOLD(1);
+ value = CURSOR_THRESHOLD(0) | WINDOW_A_THRESHOLD(0) |
+ WINDOW_B_THRESHOLD(0) | WINDOW_C_THRESHOLD(0);
tegra_dc_writel(dc, value, DC_DISP_DISP_MEM_HIGH_PRIORITY_TIMER);

value = VBLANK_INT | WIN_A_UF_INT | WIN_B_UF_INT | WIN_C_UF_INT |
--
2.26.0

2020-06-09 13:19:14

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 01/37] clk: Export clk_hw_reparent()

We're going to turn Tegra124 Memory Controller driver into a loadable
kernel module. The driver uses clk_hw_reparent(), which isn't an exported
kernel symbol. Let's export that function in order to allow modularization
of the Tegra driver.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clk/clk.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 3f588ed06ce3..2fa6394d9a1b 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2439,6 +2439,7 @@ void clk_hw_reparent(struct clk_hw *hw, struct clk_hw *new_parent)

clk_core_reparent(hw->core, !new_parent ? NULL : new_parent->core);
}
+EXPORT_SYMBOL_GPL(clk_hw_reparent);

/**
* clk_has_parent - check if a clock is a possible parent for another
--
2.26.0

2020-06-09 13:19:26

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 30/37] memory: tegra20-emc: Continue probing if timings are missing in device-tree

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.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/memory/tegra/tegra20-emc.c | 34 ++++++++++++++----------------
1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c
index 507065c2f452..6cd3d50145dc 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -386,6 +386,11 @@ tegra_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;
+ }
+
if (!of_property_read_bool(dev->of_node, "nvidia,use-ram-code"))
return of_node_get(dev->of_node);

@@ -454,6 +459,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++) {
@@ -691,13 +699,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
struct tegra_emc *emc;
int irq, err;

- /* driver has nothing to do in a case of memory timing absence */
- if (of_get_child_count(pdev->dev.of_node) == 0) {
- dev_info(&pdev->dev,
- "EMC device tree node doesn't have memory timings\n");
- return 0;
- }
-
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(&pdev->dev, "interrupt not specified\n");
@@ -705,23 +706,20 @@ static int tegra_emc_probe(struct platform_device *pdev)
return irq;
}

- np = tegra_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);
+ if (!emc)
return -ENOMEM;
- }

emc->clk_nb.notifier_call = tegra_emc_clk_change_notify;
emc->dev = &pdev->dev;

- err = tegra_emc_load_timings_from_dt(emc, np);
- of_node_put(np);
- if (err)
- return err;
+ np = tegra_emc_find_node_by_ram_code(&pdev->dev);
+ if (np) {
+ err = tegra_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))
--
2.26.0

2020-06-09 13:19:30

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 09/37] memory: tegra20-emc: Initialize MC timings

We're going to add interconnect support to the EMC driver. Once this
support will be added, the Tegra20 devfreq driver will no longer be
able to use clk_round_rate(emc) for building up OPP table. It's quite
handy that struct tegra_mc contains memory timings which could be used
by the devfreq drivers instead of the clk rate-rounding. The tegra_mc
timings are populated by the MC driver only for Tegra30+ SoCs, hence
the Tegra20 EMC could populate timings by itself.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/memory/tegra/Kconfig | 2 +-
drivers/memory/tegra/tegra20-emc.c | 47 ++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index c1cad4ce6251..5bf75b316a2f 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -10,7 +10,7 @@ config TEGRA_MC
config TEGRA20_EMC
tristate "NVIDIA Tegra20 External Memory Controller driver"
default y
- depends on ARCH_TEGRA_2x_SOC
+ depends on TEGRA_MC && ARCH_TEGRA_2x_SOC
help
This driver is for the External Memory Controller (EMC) found on
Tegra20 chips. The EMC controls the external DRAM on the board.
diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c
index 0baa6590adea..2e310c51c599 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -15,12 +15,15 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/sort.h>
#include <linux/types.h>

#include <soc/tegra/fuse.h>

+#include "mc.h"
+
#define EMC_INTSTATUS 0x000
#define EMC_INTMASK 0x004
#define EMC_DBG 0x008
@@ -650,6 +653,38 @@ static void tegra_emc_debugfs_init(struct tegra_emc *emc)
emc, &tegra_emc_debug_max_rate_fops);
}

+static int tegra_emc_init_mc_timings(struct tegra_emc *emc)
+{
+ struct tegra_mc_timing *timing;
+ struct platform_device *pdev;
+ struct device_node *np;
+ struct tegra_mc *mc;
+ unsigned int i;
+
+ np = of_find_compatible_node(NULL, NULL, "nvidia,tegra20-mc-gart");
+ if (!np)
+ return -ENOENT;
+
+ pdev = of_find_device_by_node(np);
+ of_node_put(np);
+ if (!pdev)
+ return -ENOENT;
+
+ mc = platform_get_drvdata(pdev);
+ if (!mc)
+ return -EPROBE_DEFER;
+
+ mc->timings = devm_kcalloc(mc->dev, emc->num_timings, sizeof(*timing),
+ GFP_KERNEL);
+ if (!mc->timings)
+ return -ENOMEM;
+
+ for (i = 0; i < emc->num_timings; i++)
+ mc->timings[mc->num_timings++].rate = emc->timings[i].rate;
+
+ return 0;
+}
+
static int tegra_emc_probe(struct platform_device *pdev)
{
struct device_node *np;
@@ -705,6 +740,18 @@ static int tegra_emc_probe(struct platform_device *pdev)
return err;
}

+ /*
+ * Only Tegra30+ SoCs are having Memory Controller timings initialized
+ * by the MC driver. For Tegra20 we need to populate the MC timings
+ * from here. The MC timings will be used by the Tegra20 devfreq driver.
+ */
+ err = tegra_emc_init_mc_timings(emc);
+ if (err) {
+ dev_err(&pdev->dev, "failed to initialize mc timings: %d\n",
+ err);
+ return err;
+ }
+
tegra20_clk_set_emc_round_callback(emc_round_rate, emc);

emc->clk = devm_clk_get(&pdev->dev, "emc");
--
2.26.0

2020-06-09 13:19:31

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 29/37] memory: tegra20-emc: Use devm_platform_ioremap_resource

Utilize that relatively new helper which makes code a bit cleaner.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/memory/tegra/tegra20-emc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c
index 2e310c51c599..507065c2f452 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -689,7 +689,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
{
struct device_node *np;
struct tegra_emc *emc;
- struct resource *res;
int irq, err;

/* driver has nothing to do in a case of memory timing absence */
@@ -724,8 +723,7 @@ static int tegra_emc_probe(struct platform_device *pdev)
if (err)
return err;

- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- emc->regs = devm_ioremap_resource(&pdev->dev, res);
+ emc->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(emc->regs))
return PTR_ERR(emc->regs);

--
2.26.0

2020-06-09 13:19:36

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 26/37] ARM: tegra: Add interconnect properties to Tegra30 device-tree

Add interconnect properties to the memory controller, external memory
controller and the display controller nodes in order to describe hardware
interconnection.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
arch/arm/boot/dts/tegra30.dtsi | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index d2d05f1da274..2b183025629f 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -208,6 +208,15 @@ dc@54200000 {

nvidia,head = <0>;

+ interconnects = <&mc TEGRA30_MC_DISPLAY0A &emc>,
+ <&mc TEGRA30_MC_DISPLAY0B &emc>,
+ <&mc TEGRA30_MC_DISPLAY0C &emc>,
+ <&mc TEGRA30_MC_DISPLAY1B &emc>;
+ interconnect-names = "display0a",
+ "display0b",
+ "display0c",
+ "display1b";
+
rgb {
status = "disabled";
};
@@ -227,6 +236,15 @@ dc@54240000 {

nvidia,head = <1>;

+ interconnects = <&mc TEGRA30_MC_DISPLAY0AB &emc>,
+ <&mc TEGRA30_MC_DISPLAY0BB &emc>,
+ <&mc TEGRA30_MC_DISPLAY0CB &emc>,
+ <&mc TEGRA30_MC_DISPLAY1BB &emc>;
+ interconnect-names = "display0a",
+ "display0b",
+ "display0c",
+ "display1b";
+
rgb {
status = "disabled";
};
@@ -733,15 +751,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 {
--
2.26.0

2020-06-09 13:19:46

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 28/37] memory: tegra: Register as interconnect provider

Now memory controller is a memory interconnection provider. This allows us
to use interconnect API in order to change memory configuration.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/memory/tegra/Kconfig | 1 +
drivers/memory/tegra/mc.c | 114 +++++++++++++++++++++++++++++++++++
drivers/memory/tegra/mc.h | 8 +++
include/soc/tegra/mc.h | 3 +
4 files changed, 126 insertions(+)

diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index 5bf75b316a2f..7055fdef2c32 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -3,6 +3,7 @@ config TEGRA_MC
bool "NVIDIA Tegra Memory Controller support"
default y
depends on ARCH_TEGRA
+ select INTERCONNECT
help
This driver supports the Memory Controller (MC) hardware found on
NVIDIA Tegra SoCs.
diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index 772aa021b5f6..7ef7ac9e103e 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -594,6 +594,118 @@ static __maybe_unused irqreturn_t tegra20_mc_irq(int irq, void *data)
return IRQ_HANDLED;
}

+static int tegra_mc_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+ return 0;
+}
+
+static int tegra_mc_icc_aggregate(struct icc_node *node,
+ u32 tag, u32 avg_bw, u32 peak_bw,
+ u32 *agg_avg, u32 *agg_peak)
+{
+ *agg_avg = min((u64)avg_bw + (*agg_avg), (u64)U32_MAX);
+ *agg_peak = max(*agg_peak, peak_bw);
+
+ return 0;
+}
+
+/*
+ * Memory Controller (MC) has few Memory Clients that are issuing memory
+ * bandwidth allocation requests to the MC interconnect provider. The MC
+ * provider aggregates the requests and then sends the aggregated request
+ * up to the External Memory Controller (EMC) interconnect provider which
+ * re-configures hardware interface to External Memory (EMEM) in accordance
+ * to the required bandwidth. Each MC interconnect node represents an
+ * individual Memory Client.
+ *
+ * Memory interconnect topology:
+ *
+ * +----+
+ * +--------+ | |
+ * | TEXSRD +--->+ |
+ * +--------+ | |
+ * | | +-----+ +------+
+ * ... | MC +--->+ EMC +--->+ EMEM |
+ * | | +-----+ +------+
+ * +--------+ | |
+ * | DISP.. +--->+ |
+ * +--------+ | |
+ * +----+
+ */
+static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
+{
+ struct icc_onecell_data *data;
+ struct icc_node *node;
+ unsigned int num_nodes;
+ unsigned int i;
+ int err;
+
+ /* older device-trees don't have interconnect properties */
+ if (!of_find_property(mc->dev->of_node, "#interconnect-cells", NULL))
+ return 0;
+
+ num_nodes = mc->soc->num_clients;
+
+ data = devm_kzalloc(mc->dev, struct_size(data, nodes, num_nodes),
+ GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ mc->provider.dev = mc->dev;
+ mc->provider.set = tegra_mc_icc_set;
+ mc->provider.data = data;
+ mc->provider.xlate = of_icc_xlate_onecell;
+ mc->provider.aggregate = tegra_mc_icc_aggregate;
+
+ err = icc_provider_add(&mc->provider);
+ if (err)
+ goto err_msg;
+
+ /* create Memory Controller node */
+ node = icc_node_create(TEGRA_ICC_MC);
+ err = PTR_ERR_OR_ZERO(node);
+ if (err)
+ goto del_provider;
+
+ node->name = "Memory Controller";
+ icc_node_add(node, &mc->provider);
+
+ /* link Memory Controller to External Memory Controller */
+ err = icc_link_create(node, TEGRA_ICC_EMC);
+ if (err)
+ goto remove_nodes;
+
+ for (i = 0; i < num_nodes; i++) {
+ /* create MC client node */
+ node = icc_node_create(mc->soc->clients[i].id);
+ err = PTR_ERR_OR_ZERO(node);
+ if (err)
+ goto remove_nodes;
+
+ node->name = mc->soc->clients[i].name;
+ icc_node_add(node, &mc->provider);
+
+ /* link Memory Client to Memory Controller */
+ err = icc_link_create(node, TEGRA_ICC_MC);
+ if (err)
+ goto remove_nodes;
+
+ data->nodes[i] = node;
+ }
+ data->num_nodes = num_nodes;
+
+ return 0;
+
+remove_nodes:
+ icc_nodes_remove(&mc->provider);
+del_provider:
+ icc_provider_del(&mc->provider);
+err_msg:
+ dev_err(mc->dev, "failed to initialize ICC: %d\n", err);
+
+ return err;
+}
+
static int tegra_mc_probe(struct platform_device *pdev)
{
struct resource *res;
@@ -702,6 +814,8 @@ static int tegra_mc_probe(struct platform_device *pdev)
}
}

+ tegra_mc_interconnect_setup(mc);
+
return 0;
}

diff --git a/drivers/memory/tegra/mc.h b/drivers/memory/tegra/mc.h
index afa3ba45c9e6..abeb6a2cc36a 100644
--- a/drivers/memory/tegra/mc.h
+++ b/drivers/memory/tegra/mc.h
@@ -115,4 +115,12 @@ extern const struct tegra_mc_soc tegra132_mc_soc;
extern const struct tegra_mc_soc tegra210_mc_soc;
#endif

+/*
+ * These IDs are for internal use of Tegra's ICC, the values are chosen
+ * such that they don't conflict with the device-tree ICC node IDs.
+ */
+#define TEGRA_ICC_EMC 1000
+#define TEGRA_ICC_EMEM 2000
+#define TEGRA_ICC_MC 3000
+
#endif /* MEMORY_TEGRA_MC_H */
diff --git a/include/soc/tegra/mc.h b/include/soc/tegra/mc.h
index 1238e35653d1..71de023f9f47 100644
--- a/include/soc/tegra/mc.h
+++ b/include/soc/tegra/mc.h
@@ -7,6 +7,7 @@
#define __SOC_TEGRA_MC_H__

#include <linux/err.h>
+#include <linux/interconnect-provider.h>
#include <linux/reset-controller.h>
#include <linux/types.h>

@@ -178,6 +179,8 @@ struct tegra_mc {

struct reset_controller_dev reset;

+ struct icc_provider provider;
+
spinlock_t lock;
};

--
2.26.0

2020-06-09 13:19:58

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 25/37] ARM: tegra: Add interconnect properties to Tegra20 device-tree

Add interconnect properties to the memory controller, external memory
controller and the display controller nodes in order to describe hardware
interconnection.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
arch/arm/boot/dts/tegra20.dtsi | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index c3b8ad53b967..974048e83541 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -109,6 +109,15 @@ dc@54200000 {

nvidia,head = <0>;

+ interconnects = <&mc TEGRA20_MC_DISPLAY0A &emc>,
+ <&mc TEGRA20_MC_DISPLAY0B &emc>,
+ <&mc TEGRA20_MC_DISPLAY0C &emc>,
+ <&mc TEGRA20_MC_DISPLAY1B &emc>;
+ interconnect-names = "display0a",
+ "display0b",
+ "display0c",
+ "display1b";
+
rgb {
status = "disabled";
};
@@ -126,6 +135,15 @@ dc@54240000 {

nvidia,head = <1>;

+ interconnects = <&mc TEGRA20_MC_DISPLAY0AB &emc>,
+ <&mc TEGRA20_MC_DISPLAY0BB &emc>,
+ <&mc TEGRA20_MC_DISPLAY0CB &emc>,
+ <&mc TEGRA20_MC_DISPLAY1BB &emc>;
+ interconnect-names = "display0a",
+ "display0b",
+ "display0c",
+ "display1b";
+
rgb {
status = "disabled";
};
@@ -626,15 +644,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 0x200>;
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 {
--
2.26.0

2020-06-09 13:20:18

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 22/37] dt-bindings: host1x: Document new interconnect properties

Most of Host1x devices have at least one memory client. These clients
are directly connected to the memory controller. The new interconnect
properties represent the memory client's connection to the memory
controller.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
.../display/tegra/nvidia,tegra20-host1x.txt | 68 +++++++++++++++++++
1 file changed, 68 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
index 47319214b5f6..ab4fbee7bccf 100644
--- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
+++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
@@ -20,6 +20,10 @@ Required properties:
- reset-names: Must include the following entries:
- host1x

+Each host1x client module having to perform DMA through the Memory Controller
+should have the interconnect endpoints set to the Memory Client and External
+Memory respectively.
+
The host1x top-level node defines a number of children, each representing one
of the following host1x client modules:

@@ -36,6 +40,12 @@ of the following host1x client modules:
- reset-names: Must include the following entries:
- mpe

+ Optional properties:
+ - interconnects: Must contain entry for the MPE memory clients.
+ - interconnect-names: Must include name of the interconnect path for each
+ interconnect entry. Consult TRM documentation for information about
+ available memory clients, see MEMORY CONTROLLER section.
+
- vi: video input

Required properties:
@@ -65,6 +75,12 @@ of the following host1x client modules:
- power-domains: Must include sor powergate node as csicil is in
SOR partition.

+ Optional properties:
+ - interconnects: Must contain entry for the VI memory clients.
+ - interconnect-names: Must include name of the interconnect path for each
+ interconnect entry. Consult TRM documentation for information about
+ available memory clients, see MEMORY CONTROLLER section.
+
- epp: encoder pre-processor

Required properties:
@@ -78,6 +94,12 @@ of the following host1x client modules:
- reset-names: Must include the following entries:
- epp

+ Optional properties:
+ - interconnects: Must contain entry for the EPP memory clients.
+ - interconnect-names: Must include name of the interconnect path for each
+ interconnect entry. Consult TRM documentation for information about
+ available memory clients, see MEMORY CONTROLLER section.
+
- isp: image signal processor

Required properties:
@@ -91,6 +113,12 @@ of the following host1x client modules:
- reset-names: Must include the following entries:
- isp

+ Optional properties:
+ - interconnects: Must contain entry for the ISP memory clients.
+ - interconnect-names: Must include name of the interconnect path for each
+ interconnect entry. Consult TRM documentation for information about
+ available memory clients, see MEMORY CONTROLLER section.
+
- gr2d: 2D graphics engine

Required properties:
@@ -104,6 +132,12 @@ of the following host1x client modules:
- reset-names: Must include the following entries:
- 2d

+ Optional properties:
+ - interconnects: Must contain entry for the GR2D memory clients.
+ - interconnect-names: Must include name of the interconnect path for each
+ interconnect entry. Consult TRM documentation for information about
+ available memory clients, see MEMORY CONTROLLER section.
+
- gr3d: 3D graphics engine

Required properties:
@@ -122,6 +156,12 @@ of the following host1x client modules:
- 3d
- 3d2 (Only required on SoCs with two 3D clocks)

+ Optional properties:
+ - interconnects: Must contain entry for the GR3D memory clients.
+ - interconnect-names: Must include name of the interconnect path for each
+ interconnect entry. Consult TRM documentation for information about
+ available memory clients, see MEMORY CONTROLLER section.
+
- dc: display controller

Required properties:
@@ -149,6 +189,10 @@ of the following host1x client modules:
- nvidia,hpd-gpio: specifies a GPIO used for hotplug detection
- nvidia,edid: supplies a binary EDID blob
- nvidia,panel: phandle of a display panel
+ - interconnects: Must contain entry for the DC memory clients.
+ - interconnect-names: Must include name of the interconnect path for each
+ interconnect entry. Consult TRM documentation for information about
+ available memory clients, see MEMORY CONTROLLER section.

- hdmi: High Definition Multimedia Interface

@@ -297,6 +341,12 @@ of the following host1x client modules:
- reset-names: Must include the following entries:
- vic

+ Optional properties:
+ - interconnects: Must contain entry for the VIC memory clients.
+ - interconnect-names: Must include name of the interconnect path for each
+ interconnect entry. Consult TRM documentation for information about
+ available memory clients, see MEMORY CONTROLLER section.
+
Example:

/ {
@@ -410,6 +460,15 @@ Example:
resets = <&tegra_car 27>;
reset-names = "dc";

+ interconnects = <&mc TEGRA20_MC_DISPLAY0A &emc>,
+ <&mc TEGRA20_MC_DISPLAY0B &emc>,
+ <&mc TEGRA20_MC_DISPLAY0C &emc>,
+ <&mc TEGRA20_MC_DISPLAY1B &emc>;
+ interconnect-names = "display0a",
+ "display0b",
+ "display0c",
+ "display1b";
+
rgb {
status = "disabled";
};
@@ -425,6 +484,15 @@ Example:
resets = <&tegra_car 26>;
reset-names = "dc";

+ interconnects = <&mc TEGRA20_MC_DISPLAY0AB &emc>,
+ <&mc TEGRA20_MC_DISPLAY0BB &emc>,
+ <&mc TEGRA20_MC_DISPLAY0CB &emc>,
+ <&mc TEGRA20_MC_DISPLAY1BB &emc>;
+ interconnect-names = "display0a",
+ "display0b",
+ "display0c",
+ "display1b";
+
rgb {
status = "disabled";
};
--
2.26.0

2020-06-09 13:20:41

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 20/37] dt-bindings: memory: tegra30: mc: Document new interconnect property

Memory controller is interconnected with memory clients and with the
external memory controller. Document new interconnect property which
turns memory controller into interconnect provider.

Acked-by: Rob Herring <[email protected]>
Signed-off-by: Dmitry Osipenko <[email protected]>
---
.../bindings/memory-controllers/nvidia,tegra30-mc.yaml | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
index 84fd57bcf0dc..5436e6d420bc 100644
--- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-mc.yaml
@@ -57,6 +57,9 @@ properties:
"#iommu-cells":
const: 1

+ "#interconnect-cells":
+ const: 1
+
patternProperties:
"^emc-timings-[0-9]+$":
type: object
@@ -120,6 +123,7 @@ required:
- clock-names
- "#reset-cells"
- "#iommu-cells"
+ - "#interconnect-cells"

additionalProperties: false

@@ -135,6 +139,7 @@ examples:

#iommu-cells = <1>;
#reset-cells = <1>;
+ #interconnect-cells = <1>;

emc-timings-1 {
nvidia,ram-code = <1>;
--
2.26.0

2020-06-09 13:21:14

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 21/37] dt-bindings: memory: tegra30: emc: Document new interconnect property

External memory controller is interconnected with memory controller and
with external memory. Document new interconnect property which turns
external memory controller into interconnect provider.

Acked-by: Rob Herring <[email protected]>
Signed-off-by: Dmitry Osipenko <[email protected]>
---
.../bindings/memory-controllers/nvidia,tegra30-emc.yaml | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml
index 112bae2fcbbd..c243986db420 100644
--- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.yaml
@@ -31,6 +31,9 @@ properties:
interrupts:
maxItems: 1

+ "#interconnect-cells":
+ const: 0
+
nvidia,memory-controller:
$ref: /schemas/types.yaml#/definitions/phandle
description:
@@ -214,6 +217,7 @@ required:
- interrupts
- clocks
- nvidia,memory-controller
+ - "#interconnect-cells"

additionalProperties: false

@@ -227,6 +231,8 @@ examples:

nvidia,memory-controller = <&mc>;

+ #interconnect-cells = <0>;
+
emc-timings-1 {
nvidia,ram-code = <1>;

--
2.26.0

2020-06-09 13:22:06

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 04/37] memory: tegra20-emc: Make driver modular

This patch adds modularization support to the Tegra20 EMC driver. Driver
now can be compiled as a loadable kernel module.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/memory/tegra/Kconfig | 2 +-
drivers/memory/tegra/tegra20-emc.c | 17 ++++++++++++-----
2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index 9f0a96bf9ccc..7e0e1ef87763 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -8,7 +8,7 @@ config TEGRA_MC
NVIDIA Tegra SoCs.

config TEGRA20_EMC
- bool "NVIDIA Tegra20 External Memory Controller driver"
+ tristate "NVIDIA Tegra20 External Memory Controller driver"
default y
depends on ARCH_TEGRA_2x_SOC
help
diff --git a/drivers/memory/tegra/tegra20-emc.c b/drivers/memory/tegra/tegra20-emc.c
index 027f46287dbf..0baa6590adea 100644
--- a/drivers/memory/tegra/tegra20-emc.c
+++ b/drivers/memory/tegra/tegra20-emc.c
@@ -724,6 +724,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:
@@ -736,6 +743,7 @@ static const struct of_device_id tegra_emc_of_match[] = {
{ .compatible = "nvidia,tegra20-emc", },
{},
};
+MODULE_DEVICE_TABLE(of, tegra_emc_of_match);

static struct platform_driver tegra_emc_driver = {
.probe = tegra_emc_probe,
@@ -745,9 +753,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 Tegra20 EMC driver");
+MODULE_LICENSE("GPL v2");
--
2.26.0

2020-06-09 13:22:06

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 05/37] memory: tegra30-emc: Make driver modular

This patch adds modularization support to the Tegra30 EMC driver. Driver
now can be compiled as a loadable kernel module.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
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(-)

diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index 7e0e1ef87763..bd453de9d446 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -18,7 +18,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
diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index ec8403557ed4..772aa021b5f6 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -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>
@@ -298,6 +299,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)
{
@@ -309,6 +311,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,
diff --git a/drivers/memory/tegra/tegra30-emc.c b/drivers/memory/tegra/tegra30-emc.c
index 055af0e08a2e..85d4effb8e6f 100644
--- a/drivers/memory/tegra/tegra30-emc.c
+++ b/drivers/memory/tegra/tegra30-emc.c
@@ -1343,6 +1343,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:
@@ -1393,6 +1400,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,
@@ -1403,9 +1411,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");
--
2.26.0

2020-06-09 13:22:16

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 24/37] dt-bindings: memory: tegra30: Add memory client IDs

Each memory client have a unique hardware ID, this patch adds these IDs.

Acked-by: Rob Herring <[email protected]>
Signed-off-by: Dmitry Osipenko <[email protected]>
---
include/dt-bindings/memory/tegra30-mc.h | 67 +++++++++++++++++++++++++
1 file changed, 67 insertions(+)

diff --git a/include/dt-bindings/memory/tegra30-mc.h b/include/dt-bindings/memory/tegra30-mc.h
index 169f005fbc78..930f708aca17 100644
--- a/include/dt-bindings/memory/tegra30-mc.h
+++ b/include/dt-bindings/memory/tegra30-mc.h
@@ -41,4 +41,71 @@
#define TEGRA30_MC_RESET_VDE 16
#define TEGRA30_MC_RESET_VI 17

+#define TEGRA30_MC_PTCR 0
+#define TEGRA30_MC_DISPLAY0A 1
+#define TEGRA30_MC_DISPLAY0AB 2
+#define TEGRA30_MC_DISPLAY0B 3
+#define TEGRA30_MC_DISPLAY0BB 4
+#define TEGRA30_MC_DISPLAY0C 5
+#define TEGRA30_MC_DISPLAY0CB 6
+#define TEGRA30_MC_DISPLAY1B 7
+#define TEGRA30_MC_DISPLAY1BB 8
+#define TEGRA30_MC_EPPUP 9
+#define TEGRA30_MC_G2PR 10
+#define TEGRA30_MC_G2SR 11
+#define TEGRA30_MC_MPEUNIFBR 12
+#define TEGRA30_MC_VIRUV 13
+#define TEGRA30_MC_AFIR 14
+#define TEGRA30_MC_AVPCARM7R 15
+#define TEGRA30_MC_DISPLAYHC 16
+#define TEGRA30_MC_DISPLAYHCB 17
+#define TEGRA30_MC_FDCDRD 18
+#define TEGRA30_MC_FDCDRD2 19
+#define TEGRA30_MC_G2DR 20
+#define TEGRA30_MC_HDAR 21
+#define TEGRA30_MC_HOST1XDMAR 22
+#define TEGRA30_MC_HOST1XR 23
+#define TEGRA30_MC_IDXSRD 24
+#define TEGRA30_MC_IDXSRD2 25
+#define TEGRA30_MC_MPE_IPRED 26
+#define TEGRA30_MC_MPEAMEMRD 27
+#define TEGRA30_MC_MPECSRD 28
+#define TEGRA30_MC_PPCSAHBDMAR 29
+#define TEGRA30_MC_PPCSAHBSLVR 30
+#define TEGRA30_MC_SATAR 31
+#define TEGRA30_MC_TEXSRD 32
+#define TEGRA30_MC_TEXSRD2 33
+#define TEGRA30_MC_VDEBSEVR 34
+#define TEGRA30_MC_VDEMBER 35
+#define TEGRA30_MC_VDEMCER 36
+#define TEGRA30_MC_VDETPER 37
+#define TEGRA30_MC_MPCORELPR 38
+#define TEGRA30_MC_MPCORER 39
+#define TEGRA30_MC_EPPU 40
+#define TEGRA30_MC_EPPV 41
+#define TEGRA30_MC_EPPY 42
+#define TEGRA30_MC_MPEUNIFBW 43
+#define TEGRA30_MC_VIWSB 44
+#define TEGRA30_MC_VIWU 45
+#define TEGRA30_MC_VIWV 46
+#define TEGRA30_MC_VIWY 47
+#define TEGRA30_MC_G2DW 48
+#define TEGRA30_MC_AFIW 49
+#define TEGRA30_MC_AVPCARM7W 50
+#define TEGRA30_MC_FDCDWR 51
+#define TEGRA30_MC_FDCDWR2 52
+#define TEGRA30_MC_HDAW 53
+#define TEGRA30_MC_HOST1XW 54
+#define TEGRA30_MC_ISPW 55
+#define TEGRA30_MC_MPCORELPW 56
+#define TEGRA30_MC_MPCOREW 57
+#define TEGRA30_MC_MPECSWR 58
+#define TEGRA30_MC_PPCSAHBDMAW 59
+#define TEGRA30_MC_PPCSAHBSLVW 60
+#define TEGRA30_MC_SATAW 61
+#define TEGRA30_MC_VDEBSEVW 62
+#define TEGRA30_MC_VDEDBGW 63
+#define TEGRA30_MC_VDEMBEW 64
+#define TEGRA30_MC_VDETPMW 65
+
#endif
--
2.26.0

2020-06-09 13:22:23

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 02/37] clk: tegra: Remove Memory Controller lock

The shared Memory Controller lock isn't needed since the time when
Memory Clock was made read-only. The lock could be removed safely now.
Hence let's remove it, this will help a tad to make further patches
cleaner.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clk/tegra/clk-divider.c | 4 ++--
drivers/clk/tegra/clk-tegra114.c | 6 ++----
drivers/clk/tegra/clk-tegra124.c | 7 ++-----
drivers/clk/tegra/clk-tegra20.c | 3 +--
drivers/clk/tegra/clk-tegra30.c | 3 +--
drivers/clk/tegra/clk.h | 2 +-
6 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/drivers/clk/tegra/clk-divider.c b/drivers/clk/tegra/clk-divider.c
index 38daf483ddf1..56adb01638cc 100644
--- a/drivers/clk/tegra/clk-divider.c
+++ b/drivers/clk/tegra/clk-divider.c
@@ -177,10 +177,10 @@ static const struct clk_div_table mc_div_table[] = {
};

struct clk *tegra_clk_register_mc(const char *name, const char *parent_name,
- void __iomem *reg, spinlock_t *lock)
+ void __iomem *reg)
{
return clk_register_divider_table(NULL, name, parent_name,
CLK_IS_CRITICAL,
reg, 16, 1, CLK_DIVIDER_READ_ONLY,
- mc_div_table, lock);
+ mc_div_table, NULL);
}
diff --git a/drivers/clk/tegra/clk-tegra114.c b/drivers/clk/tegra/clk-tegra114.c
index bc9e47a4cb60..ca8d9737d301 100644
--- a/drivers/clk/tegra/clk-tegra114.c
+++ b/drivers/clk/tegra/clk-tegra114.c
@@ -134,7 +134,6 @@ static DEFINE_SPINLOCK(pll_d_lock);
static DEFINE_SPINLOCK(pll_d2_lock);
static DEFINE_SPINLOCK(pll_u_lock);
static DEFINE_SPINLOCK(pll_re_lock);
-static DEFINE_SPINLOCK(emc_lock);

static struct div_nmp pllxc_nmp = {
.divm_shift = 0,
@@ -1050,10 +1049,9 @@ static __init void tegra114_periph_clk_init(void __iomem *clk_base,
ARRAY_SIZE(mux_pllmcp_clkm),
CLK_SET_RATE_NO_REPARENT,
clk_base + CLK_SOURCE_EMC,
- 29, 3, 0, &emc_lock);
+ 29, 3, 0, NULL);

- clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC,
- &emc_lock);
+ clk = tegra_clk_register_mc("mc", "emc_mux", clk_base + CLK_SOURCE_EMC);
clks[TEGRA114_CLK_MC] = clk;

clk = tegra_clk_register_periph_gate("mipi-cal", "clk_m", 0, clk_base,
diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
index e931319dcc9d..0c956e14b9ca 100644
--- a/drivers/clk/tegra/clk-tegra124.c
+++ b/drivers/clk/tegra/clk-tegra124.c
@@ -126,7 +126,6 @@ static DEFINE_SPINLOCK(pll_d_lock);
static DEFINE_SPINLOCK(pll_e_lock);
static DEFINE_SPINLOCK(pll_re_lock);
static DEFINE_SPINLOCK(pll_u_lock);
-static DEFINE_SPINLOCK(emc_lock);
static DEFINE_SPINLOCK(sor0_lock);

/* possible OSC frequencies in Hz */
@@ -1050,8 +1049,7 @@ static __init void tegra124_periph_clk_init(void __iomem *clk_base,
periph_clk_enb_refcnt);
clks[TEGRA124_CLK_DSIB] = clk;

- clk = tegra_clk_register_mc("mc", "emc", clk_base + CLK_SOURCE_EMC,
- &emc_lock);
+ clk = tegra_clk_register_mc("mc", "emc", clk_base + CLK_SOURCE_EMC);
clks[TEGRA124_CLK_MC] = clk;

/* cml0 */
@@ -1518,8 +1516,7 @@ static void __init tegra124_132_clock_init_post(struct device_node *np)
tegra124_reset_deassert);
tegra_add_of_provider(np, of_clk_src_onecell_get);

- clks[TEGRA124_CLK_EMC] = tegra_clk_register_emc(clk_base, np,
- &emc_lock);
+ clks[TEGRA124_CLK_EMC] = tegra_clk_register_emc(clk_base, np, NULL);

tegra_register_devclks(devclks, ARRAY_SIZE(devclks));

diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 3efc651b42e3..2f8b6de4198f 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -802,8 +802,7 @@ static void __init tegra20_periph_clk_init(void)

clks[TEGRA20_CLK_EMC] = clk;

- clk = tegra_clk_register_mc("mc", "emc", clk_base + CLK_SOURCE_EMC,
- NULL);
+ clk = tegra_clk_register_mc("mc", "emc", clk_base + CLK_SOURCE_EMC);
clks[TEGRA20_CLK_MC] = clk;

/* dsi */
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index 37244a7e68c2..88e8c485f8ae 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1042,8 +1042,7 @@ static void __init tegra30_periph_clk_init(void)

clks[TEGRA30_CLK_EMC] = clk;

- clk = tegra_clk_register_mc("mc", "emc", clk_base + CLK_SOURCE_EMC,
- NULL);
+ clk = tegra_clk_register_mc("mc", "emc", clk_base + CLK_SOURCE_EMC);
clks[TEGRA30_CLK_MC] = clk;

/* cml0 */
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index 6b565f6b5f66..5ed8b95d331c 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -136,7 +136,7 @@ struct clk *tegra_clk_register_divider(const char *name,
unsigned long flags, u8 clk_divider_flags, u8 shift, u8 width,
u8 frac_width, spinlock_t *lock);
struct clk *tegra_clk_register_mc(const char *name, const char *parent_name,
- void __iomem *reg, spinlock_t *lock);
+ void __iomem *reg);

/*
* Tegra PLL:
--
2.26.0

2020-06-09 13:22:37

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 19/37] dt-bindings: memory: tegra20: emc: Document new interconnect property

External memory controller is interconnected with memory controller and
with external memory. Document new interconnect property which turns
external memory controller into interconnect provider.

Acked-by: Rob Herring <[email protected]>
Signed-off-by: Dmitry Osipenko <[email protected]>
---
.../bindings/memory-controllers/nvidia,tegra20-emc.txt | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
index add95367640b..f51da7662de4 100644
--- a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.txt
@@ -12,6 +12,7 @@ Properties:
irrespective of ram-code configuration.
- interrupts : Should contain EMC General interrupt.
- clocks : Should contain EMC clock.
+- #interconnect-cells : Should be 0.

Child device nodes describe the memory settings for different configurations and clock rates.

@@ -20,6 +21,7 @@ Example:
memory-controller@7000f400 {
#address-cells = < 1 >;
#size-cells = < 0 >;
+ #interconnect-cells = < 0 >;
compatible = "nvidia,tegra20-emc";
reg = <0x7000f4000 0x200>;
interrupts = <0 78 0x04>;
--
2.26.0

2020-06-09 13:22:40

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 14/37] PM / devfreq: tegra20: Add error messages to tegra_devfreq_target()

It's useful to now when something goes wrong instead of failing silently,
so let's add error messages to tegra_devfreq_target() to prevent situation
where it fails silently.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/devfreq/tegra20-devfreq.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
index bf504ca4dea2..249d0dc44f6c 100644
--- a/drivers/devfreq/tegra20-devfreq.c
+++ b/drivers/devfreq/tegra20-devfreq.c
@@ -44,19 +44,25 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
int err;

opp = devfreq_recommended_opp(dev, freq, flags);
- if (IS_ERR(opp))
+ 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);
- if (err)
+ if (err) {
+ dev_err(dev, "failed to set min rate: %d\n", err);
return err;
+ }

err = clk_set_rate(tegra->emc_clock, 0);
- if (err)
+ if (err) {
+ dev_err(dev, "failed to set rate: %d\n", err);
goto restore_min_rate;
+ }

return 0;

--
2.26.0

2020-06-09 13:23:52

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 07/37] memory: tegra124-emc: Use devm_platform_ioremap_resource

Utilize that relatively new helper which makes code a bit cleaner.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/memory/tegra/tegra124-emc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/memory/tegra/tegra124-emc.c b/drivers/memory/tegra/tegra124-emc.c
index 98d98d09b00c..6d2897d4dca9 100644
--- a/drivers/memory/tegra/tegra124-emc.c
+++ b/drivers/memory/tegra/tegra124-emc.c
@@ -1193,7 +1193,6 @@ static int tegra_emc_probe(struct platform_device *pdev)
struct platform_device *mc;
struct device_node *np;
struct tegra_emc *emc;
- struct resource *res;
u32 ram_code;
int err;

@@ -1203,8 +1202,7 @@ static int tegra_emc_probe(struct platform_device *pdev)

emc->dev = &pdev->dev;

- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- emc->regs = devm_ioremap_resource(&pdev->dev, res);
+ emc->regs = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(emc->regs))
return PTR_ERR(emc->regs);

--
2.26.0

2020-06-09 17:19:15

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 10/37] PM / devfreq: tegra20: Silence deferred probe error

Tegra EMC driver was turned into a regular kernel driver, it also could
be compiled as a loadable kernel module now. Hence EMC clock isn't
guaranteed to be available and clk_get("emc") may return -EPROBE_DEFER and
there is no good reason to spam KMSG with a error about missing EMC clock
in this case, so let's silence the deferred probe error.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/devfreq/tegra20-devfreq.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
index ff82bac9ee4e..6469dc69c5e0 100644
--- a/drivers/devfreq/tegra20-devfreq.c
+++ b/drivers/devfreq/tegra20-devfreq.c
@@ -141,9 +141,11 @@ static int tegra_devfreq_probe(struct platform_device *pdev)

/* EMC is a system-critical clock that is always enabled */
tegra->emc_clock = devm_clk_get(&pdev->dev, "emc");
- if (IS_ERR(tegra->emc_clock)) {
- err = PTR_ERR(tegra->emc_clock);
- dev_err(&pdev->dev, "failed to get emc clock: %d\n", err);
+ err = PTR_ERR_OR_ZERO(tegra->emc_clock);
+ if (err) {
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "failed to get emc clock: %d\n",
+ err);
return err;
}

--
2.26.0

2020-06-09 17:33:12

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 03/37] clk: tegra: Export Tegra20 EMC kernel symbols

We're going to modularize Tegra EMC drivers and some of the EMC clk driver
symbols need to be exported, let's export them.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clk/tegra/clk-tegra20-emc.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/clk/tegra/clk-tegra20-emc.c b/drivers/clk/tegra/clk-tegra20-emc.c
index 03bf0009a33c..dd74b8543bf1 100644
--- a/drivers/clk/tegra/clk-tegra20-emc.c
+++ b/drivers/clk/tegra/clk-tegra20-emc.c
@@ -13,6 +13,7 @@
#include <linux/clk-provider.h>
#include <linux/clk/tegra.h>
#include <linux/err.h>
+#include <linux/export.h>
#include <linux/io.h>
#include <linux/kernel.h>
#include <linux/slab.h>
@@ -235,6 +236,7 @@ void tegra20_clk_set_emc_round_callback(tegra20_clk_emc_round_cb *round_cb,
emc->cb_arg = cb_arg;
}
}
+EXPORT_SYMBOL_GPL(tegra20_clk_set_emc_round_callback);

bool tegra20_clk_emc_driver_available(struct clk_hw *emc_hw)
{
@@ -291,3 +293,4 @@ int tegra20_clk_prepare_emc_mc_same_freq(struct clk *emc_clk, bool same)

return 0;
}
+EXPORT_SYMBOL_GPL(tegra20_clk_prepare_emc_mc_same_freq);
--
2.26.0

2020-06-09 18:38:52

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 17/37] PM / devfreq: tegra20: Relax Kconfig dependency

The Tegra EMC driver now could be compiled as a loadable kernel module.
Currently devfreq driver depends on the EMC/MC drivers in Kconfig, and
thus, devfreq is forced to be a kernel module if EMC is compiled as a
module. This build dependency could be relaxed since devfreq driver
checks MC/EMC presence on probe, allowing kernel configuration where
devfreq is a built-in driver and EMC driver is a loadable module.
This change puts Tegra20 devfreq Kconfig entry on a par with the Tegra30
devfreq entry.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/devfreq/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
index 37dc40d1fcfb..0ee36ae2fa79 100644
--- a/drivers/devfreq/Kconfig
+++ b/drivers/devfreq/Kconfig
@@ -123,7 +123,7 @@ config ARM_TEGRA_DEVFREQ

config ARM_TEGRA20_DEVFREQ
tristate "NVIDIA Tegra20 DEVFREQ Driver"
- depends on (TEGRA_MC && TEGRA20_EMC) || COMPILE_TEST
+ depends on ARCH_TEGRA_2x_SOC || COMPILE_TEST
depends on COMMON_CLK
select DEVFREQ_GOV_SIMPLE_ONDEMAND
help
--
2.26.0

2020-06-09 18:39:38

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 23/37] dt-bindings: memory: tegra20: Add memory client IDs

Each memory client have a unique hardware ID, this patch adds these IDs.

Acked-by: Rob Herring <[email protected]>
Signed-off-by: Dmitry Osipenko <[email protected]>
---
include/dt-bindings/memory/tegra20-mc.h | 53 +++++++++++++++++++++++++
1 file changed, 53 insertions(+)

diff --git a/include/dt-bindings/memory/tegra20-mc.h b/include/dt-bindings/memory/tegra20-mc.h
index 35e131eee198..6f8829508ad0 100644
--- a/include/dt-bindings/memory/tegra20-mc.h
+++ b/include/dt-bindings/memory/tegra20-mc.h
@@ -18,4 +18,57 @@
#define TEGRA20_MC_RESET_VDE 13
#define TEGRA20_MC_RESET_VI 14

+#define TEGRA20_MC_DISPLAY0A 0
+#define TEGRA20_MC_DISPLAY0AB 1
+#define TEGRA20_MC_DISPLAY0B 2
+#define TEGRA20_MC_DISPLAY0BB 3
+#define TEGRA20_MC_DISPLAY0C 4
+#define TEGRA20_MC_DISPLAY0CB 5
+#define TEGRA20_MC_DISPLAY1B 6
+#define TEGRA20_MC_DISPLAY1BB 7
+#define TEGRA20_MC_EPPUP 8
+#define TEGRA20_MC_G2PR 9
+#define TEGRA20_MC_G2SR 10
+#define TEGRA20_MC_MPEUNIFBR 11
+#define TEGRA20_MC_VIRUV 12
+#define TEGRA20_MC_AVPCARM7R 13
+#define TEGRA20_MC_DISPLAYHC 14
+#define TEGRA20_MC_DISPLAYHCB 15
+#define TEGRA20_MC_FDCDRD 16
+#define TEGRA20_MC_G2DR 17
+#define TEGRA20_MC_HOST1XDMAR 18
+#define TEGRA20_MC_HOST1XR 19
+#define TEGRA20_MC_IDXSRD 20
+#define TEGRA20_MC_MPCORER 21
+#define TEGRA20_MC_MPE_IPRED 22
+#define TEGRA20_MC_MPEAMEMRD 23
+#define TEGRA20_MC_MPECSRD 24
+#define TEGRA20_MC_PPCSAHBDMAR 25
+#define TEGRA20_MC_PPCSAHBSLVR 26
+#define TEGRA20_MC_TEXSRD 27
+#define TEGRA20_MC_VDEBSEVR 28
+#define TEGRA20_MC_VDEMBER 29
+#define TEGRA20_MC_VDEMCER 30
+#define TEGRA20_MC_VDETPER 31
+#define TEGRA20_MC_EPPU 32
+#define TEGRA20_MC_EPPV 33
+#define TEGRA20_MC_EPPY 34
+#define TEGRA20_MC_MPEUNIFBW 35
+#define TEGRA20_MC_VIWSB 36
+#define TEGRA20_MC_VIWU 37
+#define TEGRA20_MC_VIWV 38
+#define TEGRA20_MC_VIWY 39
+#define TEGRA20_MC_G2DW 40
+#define TEGRA20_MC_AVPCARM7W 41
+#define TEGRA20_MC_FDCDWR 42
+#define TEGRA20_MC_HOST1XW 43
+#define TEGRA20_MC_ISPW 44
+#define TEGRA20_MC_MPCOREW 45
+#define TEGRA20_MC_MPECSWR 46
+#define TEGRA20_MC_PPCSAHBDMAW 47
+#define TEGRA20_MC_PPCSAHBSLVW 48
+#define TEGRA20_MC_VDEBSEVW 49
+#define TEGRA20_MC_VDEMBEW 50
+#define TEGRA20_MC_VDETPMW 51
+
#endif
--
2.26.0

2020-06-09 18:53:10

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 11/37] PM / devfreq: tegra30: Silence deferred probe error

Tegra EMC driver was turned into a regular kernel driver, it also could
be compiled as a loadable kernel module now. Hence EMC clock isn't
guaranteed to be available and clk_get("emc") may return -EPROBE_DEFER and
there is no good reason to spam KMSG with a error about missing EMC clock
in this case, so let's silence the deferred probe error.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/devfreq/tegra30-devfreq.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index e94a27804c20..423dd35c95b3 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -801,9 +801,12 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
}

tegra->emc_clock = devm_clk_get(&pdev->dev, "emc");
- if (IS_ERR(tegra->emc_clock)) {
- dev_err(&pdev->dev, "Failed to get emc clock\n");
- return PTR_ERR(tegra->emc_clock);
+ err = PTR_ERR_OR_ZERO(tegra->emc_clock);
+ if (err) {
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Failed to get emc clock: %d\n",
+ err);
+ return err;
}

err = platform_get_irq(pdev, 0);
--
2.26.0

2020-06-09 18:53:10

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 13/37] PM / devfreq: tegra30: Use MC timings for building OPP table

The clk_round_rate() won't be usable for building OPP table once
interconnect support will be added to the EMC driver because that CLK API
function limits the rounded rate based on the clk rate that is imposed by
active clk-users, and thus, the rounding won't work as expected if
interconnect will set the minimum EMC clock rate before devfreq driver is
loaded. The struct tegra_mc contains memory timings which could be used by
the devfreq driver for building up OPP table instead of rounding clock
rate, this patch implements this idea.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/devfreq/tegra30-devfreq.c | 98 ++++++++++++++++++++++---------
1 file changed, 70 insertions(+), 28 deletions(-)

diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 423dd35c95b3..13f93c6038ab 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -19,6 +19,8 @@
#include <linux/reset.h>
#include <linux/workqueue.h>

+#include <soc/tegra/mc.h>
+
#include "governor.h"

#define ACTMON_GLB_STATUS 0x0
@@ -153,6 +155,18 @@ struct tegra_devfreq_device {
unsigned long target_freq;
};

+struct tegra_devfreq_soc_data {
+ const char *mc_compatible;
+};
+
+static const struct tegra_devfreq_soc_data tegra30_soc = {
+ .mc_compatible = "nvidia,tegra30-mc",
+};
+
+static const struct tegra_devfreq_soc_data tegra124_soc = {
+ .mc_compatible = "nvidia,tegra124-mc",
+};
+
struct tegra_devfreq {
struct devfreq *devfreq;

@@ -771,15 +785,44 @@ static struct devfreq_governor tegra_devfreq_governor = {
.interrupt_driven = true,
};

+static struct tegra_mc *tegra_get_memory_controller(const char *compatible)
+{
+ struct platform_device *pdev;
+ struct device_node *np;
+ struct tegra_mc *mc;
+
+ np = of_find_compatible_node(NULL, NULL, compatible);
+ if (!np)
+ return ERR_PTR(-ENOENT);
+
+ pdev = of_find_device_by_node(np);
+ of_node_put(np);
+ if (!pdev)
+ return ERR_PTR(-ENODEV);
+
+ mc = platform_get_drvdata(pdev);
+ if (!mc)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ return mc;
+}
+
static int tegra_devfreq_probe(struct platform_device *pdev)
{
+ const struct tegra_devfreq_soc_data *soc_data;
struct tegra_devfreq_device *dev;
struct tegra_devfreq *tegra;
struct devfreq *devfreq;
+ struct tegra_mc *mc;
unsigned int i;
- long rate;
int err;

+ soc_data = of_device_get_match_data(&pdev->dev);
+
+ mc = tegra_get_memory_controller(soc_data->mc_compatible);
+ if (IS_ERR(mc))
+ return PTR_ERR(mc);
+
tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
if (!tegra)
return -ENOMEM;
@@ -825,6 +868,30 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
return err;
}

+ if (!mc->num_timings) {
+ tegra->max_freq = clk_get_rate(tegra->clock) / KHZ;
+
+ err = dev_pm_opp_add(&pdev->dev, tegra->max_freq, 0);
+ if (err) {
+ dev_err(&pdev->dev, "Failed to add OPP: %d\n", err);
+ return err;
+ }
+ }
+
+ for (i = 0; i < mc->num_timings; i++) {
+ /*
+ * Memory Controller timings are sorted in ascending clock
+ * rate order, so the last timing will be the max freq.
+ */
+ tegra->max_freq = mc->timings[i].rate / KHZ;
+
+ err = dev_pm_opp_add(&pdev->dev, tegra->max_freq, 0);
+ if (err) {
+ dev_err(&pdev->dev, "Failed to add OPP: %d\n", err);
+ goto remove_opps;
+ }
+ }
+
reset_control_assert(tegra->reset);

err = clk_prepare_enable(tegra->clock);
@@ -836,37 +903,12 @@ static int tegra_devfreq_probe(struct platform_device *pdev)

reset_control_deassert(tegra->reset);

- rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
- if (rate < 0) {
- dev_err(&pdev->dev, "Failed to round clock rate: %ld\n", rate);
- return rate;
- }
-
- tegra->max_freq = rate / KHZ;
-
for (i = 0; i < ARRAY_SIZE(actmon_device_configs); i++) {
dev = tegra->devices + i;
dev->config = actmon_device_configs + i;
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;
@@ -921,8 +963,8 @@ static int tegra_devfreq_remove(struct platform_device *pdev)
}

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, },
{ },
};

--
2.26.0

2020-06-09 18:53:10

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 06/37] memory: tegra124-emc: Make driver modular

This patch adds modularization support to the Tegra124 EMC driver. Driver
now can be compiled as a loadable kernel module.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/clk/tegra/clk-tegra124-emc.c | 63 +++++++++++++++++-----------
drivers/clk/tegra/clk-tegra124.c | 3 +-
drivers/clk/tegra/clk.h | 12 ------
drivers/memory/tegra/Kconfig | 2 +-
drivers/memory/tegra/tegra124-emc.c | 32 +++++++++-----
include/linux/clk/tegra.h | 11 +++++
include/soc/tegra/emc.h | 16 -------
7 files changed, 73 insertions(+), 66 deletions(-)
delete mode 100644 include/soc/tegra/emc.h

diff --git a/drivers/clk/tegra/clk-tegra124-emc.c b/drivers/clk/tegra/clk-tegra124-emc.c
index 745f9faa98d8..4d8b8f1ba7cd 100644
--- a/drivers/clk/tegra/clk-tegra124-emc.c
+++ b/drivers/clk/tegra/clk-tegra124-emc.c
@@ -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,10 +23,10 @@
#include <linux/string.h>

#include <soc/tegra/fuse.h>
-#include <soc/tegra/emc.h>

#include "clk.h"

+#define CLK_BASE 0x60006000
#define CLK_SOURCE_EMC 0x19c

#define CLK_SOURCE_EMC_EMC_2X_CLK_DIVISOR_SHIFT 0
@@ -79,7 +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 */
@@ -98,7 +102,7 @@ static unsigned long emc_recalc_rate(struct clk_hw *hw,
*/
parent_rate = clk_hw_get_rate(clk_hw_get_parent(hw));

- val = readl(tegra->clk_regs + CLK_SOURCE_EMC);
+ val = readl(tegra->clk_regs);
div = val & CLK_SOURCE_EMC_EMC_2X_CLK_DIVISOR_MASK;

return parent_rate / (div + 2) * 2;
@@ -163,7 +167,7 @@ static u8 emc_get_parent(struct clk_hw *hw)

tegra = container_of(hw, struct tegra_clk_emc, hw);

- val = readl(tegra->clk_regs + CLK_SOURCE_EMC);
+ val = readl(tegra->clk_regs);

return (val >> CLK_SOURCE_EMC_EMC_2X_CLK_SRC_SHIFT)
& CLK_SOURCE_EMC_EMC_2X_CLK_SRC_MASK;
@@ -204,7 +208,6 @@ static int emc_set_timing(struct tegra_clk_emc *tegra,
int err;
u8 div;
u32 car_value;
- unsigned long flags = 0;
struct tegra_emc *emc = emc_ensure_emc_driver(tegra);

if (!emc)
@@ -241,13 +244,11 @@ 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;

- spin_lock_irqsave(tegra->lock, flags);
-
- car_value = readl(tegra->clk_regs + CLK_SOURCE_EMC);
+ car_value = readl(tegra->clk_regs);

car_value &= ~CLK_SOURCE_EMC_EMC_2X_CLK_SRC(~0);
car_value |= CLK_SOURCE_EMC_EMC_2X_CLK_SRC(timing->parent_index);
@@ -255,11 +256,9 @@ static int emc_set_timing(struct tegra_clk_emc *tegra,
car_value &= ~CLK_SOURCE_EMC_EMC_2X_CLK_DIVISOR(~0);
car_value |= CLK_SOURCE_EMC_EMC_2X_CLK_DIVISOR(div);

- writel(car_value, tegra->clk_regs + CLK_SOURCE_EMC);
-
- spin_unlock_irqrestore(tegra->lock, flags);
+ writel(car_value, tegra->clk_regs);

- 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,12 +472,15 @@ 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(struct device_node *emc_np,
+ tegra124_emc_prepare_timing_change_cb *prep_cb,
+ tegra124_emc_complete_timing_change_cb *complete_cb)
{
struct tegra_clk_emc *tegra;
struct clk_init_data init;
struct device_node *node;
+ struct resource res;
u32 node_ram_code;
struct clk *clk;
int err;
@@ -487,12 +489,21 @@ struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np,
if (!tegra)
return ERR_PTR(-ENOMEM);

- tegra->clk_regs = base;
- tegra->lock = lock;
+ res.start = CLK_BASE + CLK_SOURCE_EMC;
+ res.end = res.start + 3;
+ res.flags = IORESOURCE_MEM;

- tegra->num_timings = 0;
+ tegra->clk_regs = ioremap(res.start, resource_size(&res));
+ if (!tegra->clk_regs) {
+ pr_err("failed to map CLK_SOURCE_EMC\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ tegra->emc_node = emc_np;
+ tegra->prepare_timing_change = prep_cb;
+ tegra->complete_timing_change = complete_cb;

- for_each_child_of_node(np, node) {
+ for_each_child_of_node(emc_np, node) {
err = of_property_read_u32(node, "nvidia,ram-code",
&node_ram_code);
if (err)
@@ -512,11 +523,6 @@ struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np,
if (tegra->num_timings == 0)
pr_warn("%s: no memory timings registered\n", __func__);

- tegra->emc_node = of_parse_phandle(np,
- "nvidia,external-memory-controller", 0);
- if (!tegra->emc_node)
- pr_warn("%s: couldn't find node for EMC driver\n", __func__);
-
init.name = "emc";
init.ops = &tegra_clk_emc_ops;
init.flags = CLK_IS_CRITICAL;
@@ -536,5 +542,12 @@ struct clk *tegra_clk_register_emc(void __iomem *base, struct device_node *np,
/* Allow debugging tools to see the EMC clock */
clk_register_clkdev(clk, "emc", "tegra-clk-debug");

+ /*
+ * Don't allow the kernel module to be unloaded, unloading is not
+ * supported by the EMC driver.
+ */
+ try_module_get(THIS_MODULE);
+
return clk;
-};
+}
+EXPORT_SYMBOL_GPL(tegra124_clk_register_emc);
diff --git a/drivers/clk/tegra/clk-tegra124.c b/drivers/clk/tegra/clk-tegra124.c
index 0c956e14b9ca..228d87367ac6 100644
--- a/drivers/clk/tegra/clk-tegra124.c
+++ b/drivers/clk/tegra/clk-tegra124.c
@@ -928,6 +928,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 = {
@@ -1516,8 +1517,6 @@ static void __init tegra124_132_clock_init_post(struct device_node *np)
tegra124_reset_deassert);
tegra_add_of_provider(np, of_clk_src_onecell_get);

- clks[TEGRA124_CLK_EMC] = tegra_clk_register_emc(clk_base, np, NULL);
-
tegra_register_devclks(devclks, ARRAY_SIZE(devclks));

tegra_cpu_car_ops = &tegra124_cpu_car_ops;
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index 5ed8b95d331c..11a8bbe650c5 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -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);
diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
index bd453de9d446..c1cad4ce6251 100644
--- a/drivers/memory/tegra/Kconfig
+++ b/drivers/memory/tegra/Kconfig
@@ -28,7 +28,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
diff --git a/drivers/memory/tegra/tegra124-emc.c b/drivers/memory/tegra/tegra124-emc.c
index d19fb7ae230d..98d98d09b00c 100644
--- a/drivers/memory/tegra/tegra124-emc.c
+++ b/drivers/memory/tegra/tegra124-emc.c
@@ -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;
@@ -986,6 +987,7 @@ static const struct of_device_id tegra_emc_of_match[] = {
{ .compatible = "nvidia,tegra124-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)
@@ -1251,9 +1253,20 @@ static int tegra_emc_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, emc);

+ tegra124_clk_register_emc(pdev->dev.of_node,
+ 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;
};

@@ -1265,9 +1278,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");
diff --git a/include/linux/clk/tegra.h b/include/linux/clk/tegra.h
index 3f01d43f0598..797b8bde18de 100644
--- a/include/linux/clk/tegra.h
+++ b/include/linux/clk/tegra.h
@@ -136,6 +136,8 @@ 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 device_node;
+struct tegra_emc;

typedef long (tegra20_clk_emc_round_cb)(unsigned long rate,
unsigned long min_rate,
@@ -146,6 +148,15 @@ 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);
+struct clk *
+tegra124_clk_register_emc(struct device_node *emc_np,
+ 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;
diff --git a/include/soc/tegra/emc.h b/include/soc/tegra/emc.h
deleted file mode 100644
index 05199a97ccf4..000000000000
--- a/include/soc/tegra/emc.h
+++ /dev/null
@@ -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__ */
--
2.26.0

2020-06-09 18:53:11

by Dmitry Osipenko

[permalink] [raw]
Subject: [PATCH v4 08/37] soc/tegra: fuse: Export tegra_read_ram_code()

The tegra_read_ram_code() is used by EMC drivers and we're going to make
these driver modular, hence this function needs to be exported.

Signed-off-by: Dmitry Osipenko <[email protected]>
---
drivers/soc/tegra/fuse/tegra-apbmisc.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/soc/tegra/fuse/tegra-apbmisc.c b/drivers/soc/tegra/fuse/tegra-apbmisc.c
index 3cdd69d1bd4d..b3c930b805c5 100644
--- a/drivers/soc/tegra/fuse/tegra-apbmisc.c
+++ b/drivers/soc/tegra/fuse/tegra-apbmisc.c
@@ -3,6 +3,7 @@
* Copyright (c) 2014, NVIDIA CORPORATION. All rights reserved.
*/

+#include <linux/export.h>
#include <linux/kernel.h>
#include <linux/of.h>
#include <linux/of_address.h>
@@ -65,6 +66,7 @@ u32 tegra_read_ram_code(void)

return straps >> PMC_STRAPPING_OPT_A_RAM_CODE_SHIFT;
}
+EXPORT_SYMBOL_GPL(tegra_read_ram_code);

static const struct of_device_id apbmisc_match[] __initconst = {
{ .compatible = "nvidia,tegra20-apbmisc", },
--
2.26.0

2020-06-17 21:40:12

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 22/37] dt-bindings: host1x: Document new interconnect properties

On Tue, Jun 09, 2020 at 04:13:49PM +0300, Dmitry Osipenko wrote:
> Most of Host1x devices have at least one memory client. These clients
> are directly connected to the memory controller. The new interconnect
> properties represent the memory client's connection to the memory
> controller.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> .../display/tegra/nvidia,tegra20-host1x.txt | 68 +++++++++++++++++++
> 1 file changed, 68 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> index 47319214b5f6..ab4fbee7bccf 100644
> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
> @@ -20,6 +20,10 @@ Required properties:
> - reset-names: Must include the following entries:
> - host1x
>
> +Each host1x client module having to perform DMA through the Memory Controller
> +should have the interconnect endpoints set to the Memory Client and External
> +Memory respectively.
> +
> The host1x top-level node defines a number of children, each representing one
> of the following host1x client modules:
>
> @@ -36,6 +40,12 @@ of the following host1x client modules:
> - reset-names: Must include the following entries:
> - mpe
>
> + Optional properties:
> + - interconnects: Must contain entry for the MPE memory clients.
> + - interconnect-names: Must include name of the interconnect path for each
> + interconnect entry. Consult TRM documentation for information about
> + available memory clients, see MEMORY CONTROLLER section.
> +
> - vi: video input
>
> Required properties:
> @@ -65,6 +75,12 @@ of the following host1x client modules:
> - power-domains: Must include sor powergate node as csicil is in
> SOR partition.
>
> + Optional properties:
> + - interconnects: Must contain entry for the VI memory clients.
> + - interconnect-names: Must include name of the interconnect path for each
> + interconnect entry. Consult TRM documentation for information about
> + available memory clients, see MEMORY CONTROLLER section.
> +
> - epp: encoder pre-processor
>
> Required properties:
> @@ -78,6 +94,12 @@ of the following host1x client modules:
> - reset-names: Must include the following entries:
> - epp
>
> + Optional properties:
> + - interconnects: Must contain entry for the EPP memory clients.
> + - interconnect-names: Must include name of the interconnect path for each
> + interconnect entry. Consult TRM documentation for information about
> + available memory clients, see MEMORY CONTROLLER section.
> +
> - isp: image signal processor
>
> Required properties:
> @@ -91,6 +113,12 @@ of the following host1x client modules:
> - reset-names: Must include the following entries:
> - isp
>
> + Optional properties:
> + - interconnects: Must contain entry for the ISP memory clients.
> + - interconnect-names: Must include name of the interconnect path for each
> + interconnect entry. Consult TRM documentation for information about
> + available memory clients, see MEMORY CONTROLLER section.
> +
> - gr2d: 2D graphics engine
>
> Required properties:
> @@ -104,6 +132,12 @@ of the following host1x client modules:
> - reset-names: Must include the following entries:
> - 2d
>
> + Optional properties:
> + - interconnects: Must contain entry for the GR2D memory clients.
> + - interconnect-names: Must include name of the interconnect path for each
> + interconnect entry. Consult TRM documentation for information about
> + available memory clients, see MEMORY CONTROLLER section.
> +
> - gr3d: 3D graphics engine
>
> Required properties:
> @@ -122,6 +156,12 @@ of the following host1x client modules:
> - 3d
> - 3d2 (Only required on SoCs with two 3D clocks)
>
> + Optional properties:
> + - interconnects: Must contain entry for the GR3D memory clients.
> + - interconnect-names: Must include name of the interconnect path for each
> + interconnect entry. Consult TRM documentation for information about
> + available memory clients, see MEMORY CONTROLLER section.
> +
> - dc: display controller
>
> Required properties:
> @@ -149,6 +189,10 @@ of the following host1x client modules:
> - nvidia,hpd-gpio: specifies a GPIO used for hotplug detection
> - nvidia,edid: supplies a binary EDID blob
> - nvidia,panel: phandle of a display panel
> + - interconnects: Must contain entry for the DC memory clients.
> + - interconnect-names: Must include name of the interconnect path for each
> + interconnect entry. Consult TRM documentation for information about
> + available memory clients, see MEMORY CONTROLLER section.
>
> - hdmi: High Definition Multimedia Interface
>
> @@ -297,6 +341,12 @@ of the following host1x client modules:
> - reset-names: Must include the following entries:
> - vic
>
> + Optional properties:
> + - interconnects: Must contain entry for the VIC memory clients.
> + - interconnect-names: Must include name of the interconnect path for each
> + interconnect entry. Consult TRM documentation for information about
> + available memory clients, see MEMORY CONTROLLER section.
> +
> Example:
>
> / {
> @@ -410,6 +460,15 @@ Example:
> resets = <&tegra_car 27>;
> reset-names = "dc";
>
> + interconnects = <&mc TEGRA20_MC_DISPLAY0A &emc>,
> + <&mc TEGRA20_MC_DISPLAY0B &emc>,
> + <&mc TEGRA20_MC_DISPLAY0C &emc>,
> + <&mc TEGRA20_MC_DISPLAY1B &emc>;

This looks odd or wrong. Each entry has 2 phandles?

> + interconnect-names = "display0a",
> + "display0b",
> + "display0c",
> + "display1b";
> +
> rgb {
> status = "disabled";
> };
> @@ -425,6 +484,15 @@ Example:
> resets = <&tegra_car 26>;
> reset-names = "dc";
>
> + interconnects = <&mc TEGRA20_MC_DISPLAY0AB &emc>,
> + <&mc TEGRA20_MC_DISPLAY0BB &emc>,
> + <&mc TEGRA20_MC_DISPLAY0CB &emc>,
> + <&mc TEGRA20_MC_DISPLAY1BB &emc>;
> + interconnect-names = "display0a",
> + "display0b",
> + "display0c",
> + "display1b";
> +
> rgb {
> status = "disabled";
> };
> --
> 2.26.0
>

2020-06-17 21:46:55

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v4 22/37] dt-bindings: host1x: Document new interconnect properties

18.06.2020 00:37, Rob Herring пишет:
> On Tue, Jun 09, 2020 at 04:13:49PM +0300, Dmitry Osipenko wrote:
>> Most of Host1x devices have at least one memory client. These clients
>> are directly connected to the memory controller. The new interconnect
>> properties represent the memory client's connection to the memory
>> controller.
>>
>> Signed-off-by: Dmitry Osipenko <[email protected]>
>> ---
>> .../display/tegra/nvidia,tegra20-host1x.txt | 68 +++++++++++++++++++
>> 1 file changed, 68 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> index 47319214b5f6..ab4fbee7bccf 100644
>> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>> @@ -20,6 +20,10 @@ Required properties:
>> - reset-names: Must include the following entries:
>> - host1x
>>
>> +Each host1x client module having to perform DMA through the Memory Controller
>> +should have the interconnect endpoints set to the Memory Client and External
>> +Memory respectively.
>> +
>> The host1x top-level node defines a number of children, each representing one
>> of the following host1x client modules:
>>
>> @@ -36,6 +40,12 @@ of the following host1x client modules:
>> - reset-names: Must include the following entries:
>> - mpe
>>
>> + Optional properties:
>> + - interconnects: Must contain entry for the MPE memory clients.
>> + - interconnect-names: Must include name of the interconnect path for each
>> + interconnect entry. Consult TRM documentation for information about
>> + available memory clients, see MEMORY CONTROLLER section.
>> +
>> - vi: video input
>>
>> Required properties:
>> @@ -65,6 +75,12 @@ of the following host1x client modules:
>> - power-domains: Must include sor powergate node as csicil is in
>> SOR partition.
>>
>> + Optional properties:
>> + - interconnects: Must contain entry for the VI memory clients.
>> + - interconnect-names: Must include name of the interconnect path for each
>> + interconnect entry. Consult TRM documentation for information about
>> + available memory clients, see MEMORY CONTROLLER section.
>> +
>> - epp: encoder pre-processor
>>
>> Required properties:
>> @@ -78,6 +94,12 @@ of the following host1x client modules:
>> - reset-names: Must include the following entries:
>> - epp
>>
>> + Optional properties:
>> + - interconnects: Must contain entry for the EPP memory clients.
>> + - interconnect-names: Must include name of the interconnect path for each
>> + interconnect entry. Consult TRM documentation for information about
>> + available memory clients, see MEMORY CONTROLLER section.
>> +
>> - isp: image signal processor
>>
>> Required properties:
>> @@ -91,6 +113,12 @@ of the following host1x client modules:
>> - reset-names: Must include the following entries:
>> - isp
>>
>> + Optional properties:
>> + - interconnects: Must contain entry for the ISP memory clients.
>> + - interconnect-names: Must include name of the interconnect path for each
>> + interconnect entry. Consult TRM documentation for information about
>> + available memory clients, see MEMORY CONTROLLER section.
>> +
>> - gr2d: 2D graphics engine
>>
>> Required properties:
>> @@ -104,6 +132,12 @@ of the following host1x client modules:
>> - reset-names: Must include the following entries:
>> - 2d
>>
>> + Optional properties:
>> + - interconnects: Must contain entry for the GR2D memory clients.
>> + - interconnect-names: Must include name of the interconnect path for each
>> + interconnect entry. Consult TRM documentation for information about
>> + available memory clients, see MEMORY CONTROLLER section.
>> +
>> - gr3d: 3D graphics engine
>>
>> Required properties:
>> @@ -122,6 +156,12 @@ of the following host1x client modules:
>> - 3d
>> - 3d2 (Only required on SoCs with two 3D clocks)
>>
>> + Optional properties:
>> + - interconnects: Must contain entry for the GR3D memory clients.
>> + - interconnect-names: Must include name of the interconnect path for each
>> + interconnect entry. Consult TRM documentation for information about
>> + available memory clients, see MEMORY CONTROLLER section.
>> +
>> - dc: display controller
>>
>> Required properties:
>> @@ -149,6 +189,10 @@ of the following host1x client modules:
>> - nvidia,hpd-gpio: specifies a GPIO used for hotplug detection
>> - nvidia,edid: supplies a binary EDID blob
>> - nvidia,panel: phandle of a display panel
>> + - interconnects: Must contain entry for the DC memory clients.
>> + - interconnect-names: Must include name of the interconnect path for each
>> + interconnect entry. Consult TRM documentation for information about
>> + available memory clients, see MEMORY CONTROLLER section.
>>
>> - hdmi: High Definition Multimedia Interface
>>
>> @@ -297,6 +341,12 @@ of the following host1x client modules:
>> - reset-names: Must include the following entries:
>> - vic
>>
>> + Optional properties:
>> + - interconnects: Must contain entry for the VIC memory clients.
>> + - interconnect-names: Must include name of the interconnect path for each
>> + interconnect entry. Consult TRM documentation for information about
>> + available memory clients, see MEMORY CONTROLLER section.
>> +
>> Example:
>>
>> / {
>> @@ -410,6 +460,15 @@ Example:
>> resets = <&tegra_car 27>;
>> reset-names = "dc";
>>
>> + interconnects = <&mc TEGRA20_MC_DISPLAY0A &emc>,
>> + <&mc TEGRA20_MC_DISPLAY0B &emc>,
>> + <&mc TEGRA20_MC_DISPLAY0C &emc>,
>> + <&mc TEGRA20_MC_DISPLAY1B &emc>;
>
> This looks odd or wrong. Each entry has 2 phandles?

Each entry defines interconnect path, where MC is the start of the path
and EMC is the end. So yes, 2 phandles for each path.

Please see arm/boot/dts/qcom-msm8974.dtsi for another example [1].

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/qcom-msm8974.dtsi?h=v5.8-rc1#n1448

2020-06-17 21:50:58

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v4 22/37] dt-bindings: host1x: Document new interconnect properties

18.06.2020 00:44, Dmitry Osipenko пишет:
> 18.06.2020 00:37, Rob Herring пишет:
>> On Tue, Jun 09, 2020 at 04:13:49PM +0300, Dmitry Osipenko wrote:
>>> Most of Host1x devices have at least one memory client. These clients
>>> are directly connected to the memory controller. The new interconnect
>>> properties represent the memory client's connection to the memory
>>> controller.
>>>
>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>> ---
>>> .../display/tegra/nvidia,tegra20-host1x.txt | 68 +++++++++++++++++++
>>> 1 file changed, 68 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>>> index 47319214b5f6..ab4fbee7bccf 100644
>>> --- a/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>>> +++ b/Documentation/devicetree/bindings/display/tegra/nvidia,tegra20-host1x.txt
>>> @@ -20,6 +20,10 @@ Required properties:
>>> - reset-names: Must include the following entries:
>>> - host1x
>>>
>>> +Each host1x client module having to perform DMA through the Memory Controller
>>> +should have the interconnect endpoints set to the Memory Client and External
>>> +Memory respectively.
>>> +
>>> The host1x top-level node defines a number of children, each representing one
>>> of the following host1x client modules:
>>>
>>> @@ -36,6 +40,12 @@ of the following host1x client modules:
>>> - reset-names: Must include the following entries:
>>> - mpe
>>>
>>> + Optional properties:
>>> + - interconnects: Must contain entry for the MPE memory clients.
>>> + - interconnect-names: Must include name of the interconnect path for each
>>> + interconnect entry. Consult TRM documentation for information about
>>> + available memory clients, see MEMORY CONTROLLER section.
>>> +
>>> - vi: video input
>>>
>>> Required properties:
>>> @@ -65,6 +75,12 @@ of the following host1x client modules:
>>> - power-domains: Must include sor powergate node as csicil is in
>>> SOR partition.
>>>
>>> + Optional properties:
>>> + - interconnects: Must contain entry for the VI memory clients.
>>> + - interconnect-names: Must include name of the interconnect path for each
>>> + interconnect entry. Consult TRM documentation for information about
>>> + available memory clients, see MEMORY CONTROLLER section.
>>> +
>>> - epp: encoder pre-processor
>>>
>>> Required properties:
>>> @@ -78,6 +94,12 @@ of the following host1x client modules:
>>> - reset-names: Must include the following entries:
>>> - epp
>>>
>>> + Optional properties:
>>> + - interconnects: Must contain entry for the EPP memory clients.
>>> + - interconnect-names: Must include name of the interconnect path for each
>>> + interconnect entry. Consult TRM documentation for information about
>>> + available memory clients, see MEMORY CONTROLLER section.
>>> +
>>> - isp: image signal processor
>>>
>>> Required properties:
>>> @@ -91,6 +113,12 @@ of the following host1x client modules:
>>> - reset-names: Must include the following entries:
>>> - isp
>>>
>>> + Optional properties:
>>> + - interconnects: Must contain entry for the ISP memory clients.
>>> + - interconnect-names: Must include name of the interconnect path for each
>>> + interconnect entry. Consult TRM documentation for information about
>>> + available memory clients, see MEMORY CONTROLLER section.
>>> +
>>> - gr2d: 2D graphics engine
>>>
>>> Required properties:
>>> @@ -104,6 +132,12 @@ of the following host1x client modules:
>>> - reset-names: Must include the following entries:
>>> - 2d
>>>
>>> + Optional properties:
>>> + - interconnects: Must contain entry for the GR2D memory clients.
>>> + - interconnect-names: Must include name of the interconnect path for each
>>> + interconnect entry. Consult TRM documentation for information about
>>> + available memory clients, see MEMORY CONTROLLER section.
>>> +
>>> - gr3d: 3D graphics engine
>>>
>>> Required properties:
>>> @@ -122,6 +156,12 @@ of the following host1x client modules:
>>> - 3d
>>> - 3d2 (Only required on SoCs with two 3D clocks)
>>>
>>> + Optional properties:
>>> + - interconnects: Must contain entry for the GR3D memory clients.
>>> + - interconnect-names: Must include name of the interconnect path for each
>>> + interconnect entry. Consult TRM documentation for information about
>>> + available memory clients, see MEMORY CONTROLLER section.
>>> +
>>> - dc: display controller
>>>
>>> Required properties:
>>> @@ -149,6 +189,10 @@ of the following host1x client modules:
>>> - nvidia,hpd-gpio: specifies a GPIO used for hotplug detection
>>> - nvidia,edid: supplies a binary EDID blob
>>> - nvidia,panel: phandle of a display panel
>>> + - interconnects: Must contain entry for the DC memory clients.
>>> + - interconnect-names: Must include name of the interconnect path for each
>>> + interconnect entry. Consult TRM documentation for information about
>>> + available memory clients, see MEMORY CONTROLLER section.
>>>
>>> - hdmi: High Definition Multimedia Interface
>>>
>>> @@ -297,6 +341,12 @@ of the following host1x client modules:
>>> - reset-names: Must include the following entries:
>>> - vic
>>>
>>> + Optional properties:
>>> + - interconnects: Must contain entry for the VIC memory clients.
>>> + - interconnect-names: Must include name of the interconnect path for each
>>> + interconnect entry. Consult TRM documentation for information about
>>> + available memory clients, see MEMORY CONTROLLER section.
>>> +
>>> Example:
>>>
>>> / {
>>> @@ -410,6 +460,15 @@ Example:
>>> resets = <&tegra_car 27>;
>>> reset-names = "dc";
>>>
>>> + interconnects = <&mc TEGRA20_MC_DISPLAY0A &emc>,
>>> + <&mc TEGRA20_MC_DISPLAY0B &emc>,
>>> + <&mc TEGRA20_MC_DISPLAY0C &emc>,
>>> + <&mc TEGRA20_MC_DISPLAY1B &emc>;
>>
>> This looks odd or wrong. Each entry has 2 phandles?
>
> Each entry defines interconnect path, where MC is the start of the path
> and EMC is the end. So yes, 2 phandles for each path.
>
> Please see arm/boot/dts/qcom-msm8974.dtsi for another example [1].
>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/qcom-msm8974.dtsi?h=v5.8-rc1#n1448
>

Actually, there are even better examples:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sc7180.dtsi?h=v5.8-rc1#n1044

2020-07-01 17:11:05

by Georgi Djakov

[permalink] [raw]
Subject: Re: [PATCH v4 27/37] interconnect: Relax requirement in of_icc_get_from_provider()

Hi Dmitry,

On 6/9/20 16:13, Dmitry Osipenko wrote:
> From: Artur Świgoń <[email protected]>
>
> This patch relaxes the condition in of_icc_get_from_provider() so that it
> is no longer required to set #interconnect-cells = <1> in the DT. In case
> of the devfreq driver for exynos-bus, #interconnect-cells is always zero.
>
> Signed-off-by: Artur Świgoń <[email protected]>
> [[email protected]: added cells_num checking for of_icc_xlate_onecell()]
> Signed-off-by: Dmitry Osipenko <[email protected]>

I have already applied the original patch by Artur, so please make the cells_num
check a separate patch.

Thanks,
Georgi

> ---
> drivers/interconnect/core.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index e5f998744501..cb143421ca67 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -339,7 +339,7 @@ static struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec)
> struct icc_node *node = ERR_PTR(-EPROBE_DEFER);
> struct icc_provider *provider;
>
> - if (!spec || spec->args_count != 1)
> + if (!spec)
> return ERR_PTR(-EINVAL);
>
> mutex_lock(&icc_lock);
> @@ -967,6 +967,15 @@ EXPORT_SYMBOL_GPL(icc_nodes_remove);
> */
> int icc_provider_add(struct icc_provider *provider)
> {
> + struct device_node *np = provider->dev->of_node;
> + u32 cells_num;
> + int err;
> +
> + err = of_property_read_u32(np, "#interconnect-cells", &cells_num);
> + if (WARN_ON(err))
> + return err;
> + if (WARN_ON(provider->xlate == of_icc_xlate_onecell && cells_num != 1))
> + return -EINVAL;
> if (WARN_ON(!provider->set))
> return -EINVAL;
> if (WARN_ON(!provider->xlate))
>

2020-07-01 17:15:32

by Georgi Djakov

[permalink] [raw]
Subject: Re: [PATCH v4 28/37] memory: tegra: Register as interconnect provider

Hi Dmitry,

Thank you for updating the patches!

On 6/9/20 16:13, Dmitry Osipenko wrote:
> Now memory controller is a memory interconnection provider. This allows us
> to use interconnect API in order to change memory configuration.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/memory/tegra/Kconfig | 1 +
> drivers/memory/tegra/mc.c | 114 +++++++++++++++++++++++++++++++++++
> drivers/memory/tegra/mc.h | 8 +++
> include/soc/tegra/mc.h | 3 +
> 4 files changed, 126 insertions(+)
>
> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
> index 5bf75b316a2f..7055fdef2c32 100644
> --- a/drivers/memory/tegra/Kconfig
> +++ b/drivers/memory/tegra/Kconfig
> @@ -3,6 +3,7 @@ config TEGRA_MC
> bool "NVIDIA Tegra Memory Controller support"
> default y
> depends on ARCH_TEGRA
> + select INTERCONNECT
> help
> This driver supports the Memory Controller (MC) hardware found on
> NVIDIA Tegra SoCs.
> diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
> index 772aa021b5f6..7ef7ac9e103e 100644
> --- a/drivers/memory/tegra/mc.c
> +++ b/drivers/memory/tegra/mc.c
> @@ -594,6 +594,118 @@ static __maybe_unused irqreturn_t tegra20_mc_irq(int irq, void *data)
> return IRQ_HANDLED;
> }
>
> +static int tegra_mc_icc_set(struct icc_node *src, struct icc_node *dst)
> +{
> + return 0;
> +}
> +
> +static int tegra_mc_icc_aggregate(struct icc_node *node,
> + u32 tag, u32 avg_bw, u32 peak_bw,
> + u32 *agg_avg, u32 *agg_peak)
> +{
> + *agg_avg = min((u64)avg_bw + (*agg_avg), (u64)U32_MAX);
> + *agg_peak = max(*agg_peak, peak_bw);
> +
> + return 0;
> +}
> +
> +/*
> + * Memory Controller (MC) has few Memory Clients that are issuing memory
> + * bandwidth allocation requests to the MC interconnect provider. The MC
> + * provider aggregates the requests and then sends the aggregated request
> + * up to the External Memory Controller (EMC) interconnect provider which
> + * re-configures hardware interface to External Memory (EMEM) in accordance
> + * to the required bandwidth. Each MC interconnect node represents an
> + * individual Memory Client.
> + *
> + * Memory interconnect topology:
> + *
> + * +----+
> + * +--------+ | |
> + * | TEXSRD +--->+ |
> + * +--------+ | |
> + * | | +-----+ +------+
> + * ... | MC +--->+ EMC +--->+ EMEM |
> + * | | +-----+ +------+
> + * +--------+ | |
> + * | DISP.. +--->+ |
> + * +--------+ | |
> + * +----+
> + */
> +static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
> +{
> + struct icc_onecell_data *data;
> + struct icc_node *node;
> + unsigned int num_nodes;
> + unsigned int i;
> + int err;
> +
> + /* older device-trees don't have interconnect properties */
> + if (!of_find_property(mc->dev->of_node, "#interconnect-cells", NULL))
> + return 0;
> +
> + num_nodes = mc->soc->num_clients;
> +
> + data = devm_kzalloc(mc->dev, struct_size(data, nodes, num_nodes),
> + GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + mc->provider.dev = mc->dev;
> + mc->provider.set = tegra_mc_icc_set;

Hmm, maybe the core should not require a set() implementation and we can
just make it optional instead. Then the dummy function would not be needed.

> + mc->provider.data = data;
> + mc->provider.xlate = of_icc_xlate_onecell;
> + mc->provider.aggregate = tegra_mc_icc_aggregate;
> +
> + err = icc_provider_add(&mc->provider);
> + if (err)
> + goto err_msg;

Nit: I am planning to re-organize some of the existing drivers to call
icc_provider_add() after the topology is populated. Could you please move
this after the nodes are created and linked.

> +
> + /* create Memory Controller node */
> + node = icc_node_create(TEGRA_ICC_MC);
> + err = PTR_ERR_OR_ZERO(node);
> + if (err)
> + goto del_provider;
> +
> + node->name = "Memory Controller";
> + icc_node_add(node, &mc->provider);
> +
> + /* link Memory Controller to External Memory Controller */
> + err = icc_link_create(node, TEGRA_ICC_EMC);
> + if (err)
> + goto remove_nodes;
> +
> + for (i = 0; i < num_nodes; i++) {
> + /* create MC client node */
> + node = icc_node_create(mc->soc->clients[i].id);
> + err = PTR_ERR_OR_ZERO(node);
> + if (err)
> + goto remove_nodes;
> +
> + node->name = mc->soc->clients[i].name;
> + icc_node_add(node, &mc->provider);
> +
> + /* link Memory Client to Memory Controller */
> + err = icc_link_create(node, TEGRA_ICC_MC);
> + if (err)
> + goto remove_nodes;
> +
> + data->nodes[i] = node;
> + }
> + data->num_nodes = num_nodes;
> +
> + return 0;
> +
> +remove_nodes:
> + icc_nodes_remove(&mc->provider);
> +del_provider:
> + icc_provider_del(&mc->provider);
> +err_msg:
> + dev_err(mc->dev, "failed to initialize ICC: %d\n", err);
> +
> + return err;
> +}
> +
> static int tegra_mc_probe(struct platform_device *pdev)
> {
> struct resource *res;
> @@ -702,6 +814,8 @@ static int tegra_mc_probe(struct platform_device *pdev)
> }
> }
>
> + tegra_mc_interconnect_setup(mc);
> +
> return 0;
> }
>
> diff --git a/drivers/memory/tegra/mc.h b/drivers/memory/tegra/mc.h
> index afa3ba45c9e6..abeb6a2cc36a 100644
> --- a/drivers/memory/tegra/mc.h
> +++ b/drivers/memory/tegra/mc.h
> @@ -115,4 +115,12 @@ extern const struct tegra_mc_soc tegra132_mc_soc;
> extern const struct tegra_mc_soc tegra210_mc_soc;
> #endif
>
> +/*
> + * These IDs are for internal use of Tegra's ICC, the values are chosen
> + * such that they don't conflict with the device-tree ICC node IDs.
> + */
> +#define TEGRA_ICC_EMC 1000
> +#define TEGRA_ICC_EMEM 2000
> +#define TEGRA_ICC_MC 3000
> +
> #endif /* MEMORY_TEGRA_MC_H */
> diff --git a/include/soc/tegra/mc.h b/include/soc/tegra/mc.h
> index 1238e35653d1..71de023f9f47 100644
> --- a/include/soc/tegra/mc.h
> +++ b/include/soc/tegra/mc.h
> @@ -7,6 +7,7 @@
> #define __SOC_TEGRA_MC_H__
>
> #include <linux/err.h>
> +#include <linux/interconnect-provider.h>
> #include <linux/reset-controller.h>
> #include <linux/types.h>
>
> @@ -178,6 +179,8 @@ struct tegra_mc {
>
> struct reset_controller_dev reset;
>
> + struct icc_provider provider;
> +
> spinlock_t lock;
> };

The rest looks good to me!

Thanks,
Georgi

2020-07-01 23:37:13

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v4 28/37] memory: tegra: Register as interconnect provider

01.07.2020 20:12, Georgi Djakov пишет:
> Hi Dmitry,
>
> Thank you for updating the patches!

Hello, Georgi!

Thank you for the review!

> On 6/9/20 16:13, Dmitry Osipenko wrote:
>> Now memory controller is a memory interconnection provider. This allows us
>> to use interconnect API in order to change memory configuration.
>>
>> Signed-off-by: Dmitry Osipenko <[email protected]>
>> ---
>> drivers/memory/tegra/Kconfig | 1 +
>> drivers/memory/tegra/mc.c | 114 +++++++++++++++++++++++++++++++++++
>> drivers/memory/tegra/mc.h | 8 +++
>> include/soc/tegra/mc.h | 3 +
>> 4 files changed, 126 insertions(+)
>>
>> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
>> index 5bf75b316a2f..7055fdef2c32 100644
>> --- a/drivers/memory/tegra/Kconfig
>> +++ b/drivers/memory/tegra/Kconfig
>> @@ -3,6 +3,7 @@ config TEGRA_MC
>> bool "NVIDIA Tegra Memory Controller support"
>> default y
>> depends on ARCH_TEGRA
>> + select INTERCONNECT
>> help
>> This driver supports the Memory Controller (MC) hardware found on
>> NVIDIA Tegra SoCs.
>> diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
>> index 772aa021b5f6..7ef7ac9e103e 100644
>> --- a/drivers/memory/tegra/mc.c
>> +++ b/drivers/memory/tegra/mc.c
>> @@ -594,6 +594,118 @@ static __maybe_unused irqreturn_t tegra20_mc_irq(int irq, void *data)
>> return IRQ_HANDLED;
>> }
>>
>> +static int tegra_mc_icc_set(struct icc_node *src, struct icc_node *dst)
>> +{
>> + return 0;
>> +}
>> +
>> +static int tegra_mc_icc_aggregate(struct icc_node *node,
>> + u32 tag, u32 avg_bw, u32 peak_bw,
>> + u32 *agg_avg, u32 *agg_peak)
>> +{
>> + *agg_avg = min((u64)avg_bw + (*agg_avg), (u64)U32_MAX);
>> + *agg_peak = max(*agg_peak, peak_bw);
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Memory Controller (MC) has few Memory Clients that are issuing memory
>> + * bandwidth allocation requests to the MC interconnect provider. The MC
>> + * provider aggregates the requests and then sends the aggregated request
>> + * up to the External Memory Controller (EMC) interconnect provider which
>> + * re-configures hardware interface to External Memory (EMEM) in accordance
>> + * to the required bandwidth. Each MC interconnect node represents an
>> + * individual Memory Client.
>> + *
>> + * Memory interconnect topology:
>> + *
>> + * +----+
>> + * +--------+ | |
>> + * | TEXSRD +--->+ |
>> + * +--------+ | |
>> + * | | +-----+ +------+
>> + * ... | MC +--->+ EMC +--->+ EMEM |
>> + * | | +-----+ +------+
>> + * +--------+ | |
>> + * | DISP.. +--->+ |
>> + * +--------+ | |
>> + * +----+
>> + */
>> +static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
>> +{
>> + struct icc_onecell_data *data;
>> + struct icc_node *node;
>> + unsigned int num_nodes;
>> + unsigned int i;
>> + int err;
>> +
>> + /* older device-trees don't have interconnect properties */
>> + if (!of_find_property(mc->dev->of_node, "#interconnect-cells", NULL))
>> + return 0;
>> +
>> + num_nodes = mc->soc->num_clients;
>> +
>> + data = devm_kzalloc(mc->dev, struct_size(data, nodes, num_nodes),
>> + GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + mc->provider.dev = mc->dev;
>> + mc->provider.set = tegra_mc_icc_set;
>
> Hmm, maybe the core should not require a set() implementation and we can
> just make it optional instead. Then the dummy function would not be needed.

Eventually this dummy function might become populated with a memory
latency allowness programming. I could add a comment into that function
in the next version, saying that it's to-be-done for now.

>> + mc->provider.data = data;
>> + mc->provider.xlate = of_icc_xlate_onecell;
>> + mc->provider.aggregate = tegra_mc_icc_aggregate;
>> +
>> + err = icc_provider_add(&mc->provider);
>> + if (err)
>> + goto err_msg;
>
> Nit: I am planning to re-organize some of the existing drivers to call
> icc_provider_add() after the topology is populated. Could you please move
> this after the nodes are created and linked.

Are you planning to remove the provider's list-head initialization from
the icc_provider_add() [1] and move it to the individual provider
drivers, correct?

[1]
https://elixir.bootlin.com/linux/v5.8-rc3/source/drivers/interconnect/core.c#L977

If yes, then it should be easy to move the icc_provider_add() in the
case of this driver. Otherwise, please give some more clarification.

Please notice that the same "node" variable is used for both creation
and linking of the nodes to the provider here, making code nice and
clean. And thus, the provider's list-head should be initialized before
the linking.

...
> The rest looks good to me!

Thanks!

2020-07-01 23:41:38

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v4 27/37] interconnect: Relax requirement in of_icc_get_from_provider()

01.07.2020 20:10, Georgi Djakov пишет:
> Hi Dmitry,
>
> On 6/9/20 16:13, Dmitry Osipenko wrote:
>> From: Artur Świgoń <[email protected]>
>>
>> This patch relaxes the condition in of_icc_get_from_provider() so that it
>> is no longer required to set #interconnect-cells = <1> in the DT. In case
>> of the devfreq driver for exynos-bus, #interconnect-cells is always zero.
>>
>> Signed-off-by: Artur Świgoń <[email protected]>
>> [[email protected]: added cells_num checking for of_icc_xlate_onecell()]
>> Signed-off-by: Dmitry Osipenko <[email protected]>
>
> I have already applied the original patch by Artur, so please make the cells_num
> check a separate patch.

Okay, I'll update this patch! Thank you!

2020-07-02 00:47:59

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v4 10/37] PM / devfreq: tegra20: Silence deferred probe error

Hi Dmitry,

On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
> Tegra EMC driver was turned into a regular kernel driver, it also could
> be compiled as a loadable kernel module now. Hence EMC clock isn't

Looks good to me. But, you better to add the commit information
about Tegra EMC driver with commit-id ("patch title") format
into patch descritpion.

> guaranteed to be available and clk_get("emc") may return -EPROBE_DEFER and
> there is no good reason to spam KMSG with a error about missing EMC clock
> in this case, so let's silence the deferred probe error.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/devfreq/tegra20-devfreq.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
> index ff82bac9ee4e..6469dc69c5e0 100644
> --- a/drivers/devfreq/tegra20-devfreq.c
> +++ b/drivers/devfreq/tegra20-devfreq.c
> @@ -141,9 +141,11 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>
> /* EMC is a system-critical clock that is always enabled */
> tegra->emc_clock = devm_clk_get(&pdev->dev, "emc");
> - if (IS_ERR(tegra->emc_clock)) {
> - err = PTR_ERR(tegra->emc_clock);
> - dev_err(&pdev->dev, "failed to get emc clock: %d\n", err);
> + err = PTR_ERR_OR_ZERO(tegra->emc_clock);
> + if (err) {
> + if (err != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "failed to get emc clock: %d\n",
> + err);
> return err;
> }
>
>


--
Best Regards,
Chanwoo Choi
Samsung Electronics

2020-07-02 00:51:14

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v4 11/37] PM / devfreq: tegra30: Silence deferred probe error

Hi,

On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
> Tegra EMC driver was turned into a regular kernel driver, it also could
> be compiled as a loadable kernel module now. Hence EMC clock isn't
> guaranteed to be available and clk_get("emc") may return -EPROBE_DEFER and
> there is no good reason to spam KMSG with a error about missing EMC clock
> in this case, so let's silence the deferred probe error.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/devfreq/tegra30-devfreq.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
> index e94a27804c20..423dd35c95b3 100644
> --- a/drivers/devfreq/tegra30-devfreq.c
> +++ b/drivers/devfreq/tegra30-devfreq.c
> @@ -801,9 +801,12 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
> }
>
> tegra->emc_clock = devm_clk_get(&pdev->dev, "emc");
> - if (IS_ERR(tegra->emc_clock)) {
> - dev_err(&pdev->dev, "Failed to get emc clock\n");
> - return PTR_ERR(tegra->emc_clock);
> + err = PTR_ERR_OR_ZERO(tegra->emc_clock);
> + if (err) {
> + if (err != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "Failed to get emc clock: %d\n",
> + err);
> + return err;
> }
>
> err = platform_get_irq(pdev, 0);
>

As I commented on patch10, I recommend that you add the Tegra EMC driver
commit information into patch description and Looks good to me.

--
Best Regards,
Chanwoo Choi
Samsung Electronics

2020-07-02 01:02:02

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v4 14/37] PM / devfreq: tegra20: Add error messages to tegra_devfreq_target()

On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
> It's useful to now when something goes wrong instead of failing silently,
> so let's add error messages to tegra_devfreq_target() to prevent situation
> where it fails silently.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/devfreq/tegra20-devfreq.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
> index bf504ca4dea2..249d0dc44f6c 100644
> --- a/drivers/devfreq/tegra20-devfreq.c
> +++ b/drivers/devfreq/tegra20-devfreq.c
> @@ -44,19 +44,25 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
> int err;
>
> opp = devfreq_recommended_opp(dev, freq, flags);
> - if (IS_ERR(opp))
> + 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);
> - if (err)
> + if (err) {
> + dev_err(dev, "failed to set min rate: %d\n", err);
> return err;
> + }
>
> err = clk_set_rate(tegra->emc_clock, 0);
> - if (err)
> + if (err) {
> + dev_err(dev, "failed to set rate: %d\n", err);
> goto restore_min_rate;
> + }
>
> return 0;
>
>

Acked-by: Chanwoo Choi <[email protected]>

--
Best Regards,
Chanwoo Choi
Samsung Electronics

2020-07-02 01:03:54

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v4 15/37] PM / devfreq: tegra30: Add error messages to tegra_devfreq_target()

On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
> It's useful to now when something goes wrong instead of failing silently,
> so let's add error messages to tegra_devfreq_target() to prevent situation
> where it fails silently.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/devfreq/tegra30-devfreq.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
> index 13f93c6038ab..a03fb16c5c4c 100644
> --- a/drivers/devfreq/tegra30-devfreq.c
> +++ b/drivers/devfreq/tegra30-devfreq.c
> @@ -641,12 +641,16 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
> dev_pm_opp_put(opp);
>
> err = clk_set_min_rate(tegra->emc_clock, rate * KHZ);
> - if (err)
> + if (err) {
> + dev_err(dev, "Failed to set min rate: %d\n", err);
> return err;
> + }
>
> err = clk_set_rate(tegra->emc_clock, 0);
> - if (err)
> + if (err) {
> + dev_err(dev, "Failed to set rate: %d\n", err);
> goto restore_min_rate;
> + }
>
> return 0;
>
>

Acked-by: Chanwoo Choi <[email protected]>

--
Best Regards,
Chanwoo Choi
Samsung Electronics

2020-07-02 01:21:46

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v4 11/37] PM / devfreq: tegra30: Silence deferred probe error

02.07.2020 03:59, Chanwoo Choi пишет:
> Hi,
>
> On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
>> Tegra EMC driver was turned into a regular kernel driver, it also could
>> be compiled as a loadable kernel module now. Hence EMC clock isn't
>> guaranteed to be available and clk_get("emc") may return -EPROBE_DEFER and
>> there is no good reason to spam KMSG with a error about missing EMC clock
>> in this case, so let's silence the deferred probe error.
>>
>> Signed-off-by: Dmitry Osipenko <[email protected]>
>> ---
>> drivers/devfreq/tegra30-devfreq.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
>> index e94a27804c20..423dd35c95b3 100644
>> --- a/drivers/devfreq/tegra30-devfreq.c
>> +++ b/drivers/devfreq/tegra30-devfreq.c
>> @@ -801,9 +801,12 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>> }
>>
>> tegra->emc_clock = devm_clk_get(&pdev->dev, "emc");
>> - if (IS_ERR(tegra->emc_clock)) {
>> - dev_err(&pdev->dev, "Failed to get emc clock\n");
>> - return PTR_ERR(tegra->emc_clock);
>> + err = PTR_ERR_OR_ZERO(tegra->emc_clock);
>> + if (err) {
>> + if (err != -EPROBE_DEFER)
>> + dev_err(&pdev->dev, "Failed to get emc clock: %d\n",
>> + err);
>> + return err;
>> }
>>
>> err = platform_get_irq(pdev, 0);
>>
>
> As I commented on patch10, I recommend that you add the Tegra EMC driver
> commit information into patch description and Looks good to me.
>

Hello, Chanwoo!

This patch11 and patch10 are depending on the patches 4/5 (the Tegra EMC
driver patches) of *this* series, hence there is no commit information.
I'm expecting that this whole series will go via tegra tree once all the
patches will be reviewed and collect all the necessary acks from you,
ICC and CLK subsystem maintainers.

Please feel free to give yours ack to the patches 10/11 if they are good
to you :)

2020-07-02 01:25:16

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v4 11/37] PM / devfreq: tegra30: Silence deferred probe error

On 7/2/20 10:20 AM, Dmitry Osipenko wrote:
> 02.07.2020 03:59, Chanwoo Choi пишет:
>> Hi,
>>
>> On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
>>> Tegra EMC driver was turned into a regular kernel driver, it also could
>>> be compiled as a loadable kernel module now. Hence EMC clock isn't
>>> guaranteed to be available and clk_get("emc") may return -EPROBE_DEFER and
>>> there is no good reason to spam KMSG with a error about missing EMC clock
>>> in this case, so let's silence the deferred probe error.
>>>
>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>> ---
>>> drivers/devfreq/tegra30-devfreq.c | 9 ++++++---
>>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
>>> index e94a27804c20..423dd35c95b3 100644
>>> --- a/drivers/devfreq/tegra30-devfreq.c
>>> +++ b/drivers/devfreq/tegra30-devfreq.c
>>> @@ -801,9 +801,12 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>> }
>>>
>>> tegra->emc_clock = devm_clk_get(&pdev->dev, "emc");
>>> - if (IS_ERR(tegra->emc_clock)) {
>>> - dev_err(&pdev->dev, "Failed to get emc clock\n");
>>> - return PTR_ERR(tegra->emc_clock);
>>> + err = PTR_ERR_OR_ZERO(tegra->emc_clock);
>>> + if (err) {
>>> + if (err != -EPROBE_DEFER)
>>> + dev_err(&pdev->dev, "Failed to get emc clock: %d\n",
>>> + err);
>>> + return err;
>>> }
>>>
>>> err = platform_get_irq(pdev, 0);
>>>
>>
>> As I commented on patch10, I recommend that you add the Tegra EMC driver
>> commit information into patch description and Looks good to me.
>>
>
> Hello, Chanwoo!
>
> This patch11 and patch10 are depending on the patches 4/5 (the Tegra EMC
> driver patches) of *this* series, hence there is no commit information.
> I'm expecting that this whole series will go via tegra tree once all the
> patches will be reviewed and collect all the necessary acks from you,
> ICC and CLK subsystem maintainers.
>
> Please feel free to give yours ack to the patches 10/11 if they are good
> to you :)
>
>

OK. Looks good to me
Acked-by: Chanwoo Choi <[email protected]>

--
Best Regards,
Chanwoo Choi
Samsung Electronics

2020-07-02 01:27:55

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v4 10/37] PM / devfreq: tegra20: Silence deferred probe error

On 7/2/20 9:56 AM, Chanwoo Choi wrote:
> Hi Dmitry,
>
> On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
>> Tegra EMC driver was turned into a regular kernel driver, it also could
>> be compiled as a loadable kernel module now. Hence EMC clock isn't
>
> Looks good to me. But, you better to add the commit information
> about Tegra EMC driver with commit-id ("patch title") format
> into patch descritpion.
>
>> guaranteed to be available and clk_get("emc") may return -EPROBE_DEFER and
>> there is no good reason to spam KMSG with a error about missing EMC clock
>> in this case, so let's silence the deferred probe error.
>>
>> Signed-off-by: Dmitry Osipenko <[email protected]>
>> ---
>> drivers/devfreq/tegra20-devfreq.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
>> index ff82bac9ee4e..6469dc69c5e0 100644
>> --- a/drivers/devfreq/tegra20-devfreq.c
>> +++ b/drivers/devfreq/tegra20-devfreq.c
>> @@ -141,9 +141,11 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>
>> /* EMC is a system-critical clock that is always enabled */
>> tegra->emc_clock = devm_clk_get(&pdev->dev, "emc");
>> - if (IS_ERR(tegra->emc_clock)) {
>> - err = PTR_ERR(tegra->emc_clock);
>> - dev_err(&pdev->dev, "failed to get emc clock: %d\n", err);
>> + err = PTR_ERR_OR_ZERO(tegra->emc_clock);
>> + if (err) {
>> + if (err != -EPROBE_DEFER)
>> + dev_err(&pdev->dev, "failed to get emc clock: %d\n",
>> + err);
>> return err;
>> }
>>
>>
>
>

The Tegra EMC drive is included in this patchset.
So, don't need to mention the commit info.

Looks good to me.
Acked-by: Chanwoo Choi <[email protected]>


--
Best Regards,
Chanwoo Choi
Samsung Electronics

2020-07-02 01:28:40

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v4 11/37] PM / devfreq: tegra30: Silence deferred probe error

02.07.2020 04:34, Chanwoo Choi пишет:
> On 7/2/20 10:20 AM, Dmitry Osipenko wrote:
>> 02.07.2020 03:59, Chanwoo Choi пишет:
>>> Hi,
>>>
>>> On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
>>>> Tegra EMC driver was turned into a regular kernel driver, it also could
>>>> be compiled as a loadable kernel module now. Hence EMC clock isn't
>>>> guaranteed to be available and clk_get("emc") may return -EPROBE_DEFER and
>>>> there is no good reason to spam KMSG with a error about missing EMC clock
>>>> in this case, so let's silence the deferred probe error.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>>> ---
>>>> drivers/devfreq/tegra30-devfreq.c | 9 ++++++---
>>>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
>>>> index e94a27804c20..423dd35c95b3 100644
>>>> --- a/drivers/devfreq/tegra30-devfreq.c
>>>> +++ b/drivers/devfreq/tegra30-devfreq.c
>>>> @@ -801,9 +801,12 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>>> }
>>>>
>>>> tegra->emc_clock = devm_clk_get(&pdev->dev, "emc");
>>>> - if (IS_ERR(tegra->emc_clock)) {
>>>> - dev_err(&pdev->dev, "Failed to get emc clock\n");
>>>> - return PTR_ERR(tegra->emc_clock);
>>>> + err = PTR_ERR_OR_ZERO(tegra->emc_clock);
>>>> + if (err) {
>>>> + if (err != -EPROBE_DEFER)
>>>> + dev_err(&pdev->dev, "Failed to get emc clock: %d\n",
>>>> + err);
>>>> + return err;
>>>> }
>>>>
>>>> err = platform_get_irq(pdev, 0);
>>>>
>>>
>>> As I commented on patch10, I recommend that you add the Tegra EMC driver
>>> commit information into patch description and Looks good to me.
>>>
>>
>> Hello, Chanwoo!
>>
>> This patch11 and patch10 are depending on the patches 4/5 (the Tegra EMC
>> driver patches) of *this* series, hence there is no commit information.
>> I'm expecting that this whole series will go via tegra tree once all the
>> patches will be reviewed and collect all the necessary acks from you,
>> ICC and CLK subsystem maintainers.
>>
>> Please feel free to give yours ack to the patches 10/11 if they are good
>> to you :)
>>
>>
>
> OK. Looks good to me
> Acked-by: Chanwoo Choi <[email protected]>
>

Thank you! :)

2020-07-02 01:34:49

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v4 16/37] PM / devfreq: tegra20: Adjust clocks conversion ratio and polling interval

On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
> The current conversion ratio results in a higher frequency than needed,
> that is not very actual now since the Display Controller driver got
> support for memory bandwidth management and hence memory frequency can
> go lower now without bad consequences. Since memory freq now goes to a
> lower rates, the responsiveness of interactive applications become worse
> due to a quite high polling interval value that is currently set to 500ms.
> Changing polling interval to 30ms results in a good responsiveness of the
> system.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/devfreq/tegra20-devfreq.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
> index 249d0dc44f6c..7cdea4ba38f7 100644
> --- a/drivers/devfreq/tegra20-devfreq.c
> +++ b/drivers/devfreq/tegra20-devfreq.c
> @@ -79,16 +79,12 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
>
> /*
> * EMC_COUNT returns number of memory events, that number is lower
> - * than the number of clocks. Conversion ratio of 1/8 results in a
> - * bit higher bandwidth than actually needed, it is good enough for
> - * the time being because drivers don't support requesting minimum
> - * needed memory bandwidth yet.
> - *
> - * TODO: adjust the ratio value once relevant drivers will support
> - * memory bandwidth management.
> + * than the number of total EMC clocks over the sampling period.
> + * The clocks number is converted to maximum possible number of
> + * memory events using the ratio of 1/4.
> */
> stat->busy_time = readl_relaxed(tegra->regs + MC_STAT_EMC_COUNT);
> - stat->total_time = readl_relaxed(tegra->regs + MC_STAT_EMC_CLOCKS) / 8;
> + stat->total_time = readl_relaxed(tegra->regs + MC_STAT_EMC_CLOCKS) / 4;
> stat->current_frequency = clk_get_rate(tegra->emc_clock);
>
> writel_relaxed(EMC_GATHER_CLEAR, tegra->regs + MC_STAT_CONTROL);
> @@ -98,7 +94,7 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
> }
>
> static struct devfreq_dev_profile tegra_devfreq_profile = {
> - .polling_ms = 500,
> + .polling_ms = 30,
> .target = tegra_devfreq_target,
> .get_dev_status = tegra_devfreq_get_dev_status,
> };
>

Ackded-by: Chanwoo Choi <[email protected]>

--
Best Regards,
Chanwoo Choi
Samsung Electronics

2020-07-02 02:00:28

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v4 17/37] PM / devfreq: tegra20: Relax Kconfig dependency

On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
> The Tegra EMC driver now could be compiled as a loadable kernel module.
> Currently devfreq driver depends on the EMC/MC drivers in Kconfig, and
> thus, devfreq is forced to be a kernel module if EMC is compiled as a
> module. This build dependency could be relaxed since devfreq driver
> checks MC/EMC presence on probe, allowing kernel configuration where
> devfreq is a built-in driver and EMC driver is a loadable module.
> This change puts Tegra20 devfreq Kconfig entry on a par with the Tegra30
> devfreq entry.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/devfreq/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
> index 37dc40d1fcfb..0ee36ae2fa79 100644
> --- a/drivers/devfreq/Kconfig
> +++ b/drivers/devfreq/Kconfig
> @@ -123,7 +123,7 @@ config ARM_TEGRA_DEVFREQ
>
> config ARM_TEGRA20_DEVFREQ
> tristate "NVIDIA Tegra20 DEVFREQ Driver"
> - depends on (TEGRA_MC && TEGRA20_EMC) || COMPILE_TEST
> + depends on ARCH_TEGRA_2x_SOC || COMPILE_TEST
> depends on COMMON_CLK
> select DEVFREQ_GOV_SIMPLE_ONDEMAND
> help
>

Acked-by: Chanwoo Choi <[email protected]>

--
Best Regards,
Chanwoo Choi
Samsung Electronics

2020-07-02 04:07:52

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v4 12/37] PM / devfreq: tegra20: Use MC timings for building OPP table

Hi Dmitry,

On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
> The clk_round_rate() won't be usable for building OPP table once
> interconnect support will be added to the EMC driver because that CLK API
> function limits the rounded rate based on the clk rate that is imposed by
> active clk-users, and thus, the rounding won't work as expected if
> interconnect will set the minimum EMC clock rate before devfreq driver is
> loaded. The struct tegra_mc contains memory timings which could be used by
> the devfreq driver for building up OPP table instead of rounding clock
> rate, this patch implements this idea.
>
> Signed-off-by: Dmitry Osipenko <[email protected]>
> ---
> drivers/devfreq/tegra20-devfreq.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
> index 6469dc69c5e0..bf504ca4dea2 100644
> --- a/drivers/devfreq/tegra20-devfreq.c
> +++ b/drivers/devfreq/tegra20-devfreq.c
> @@ -123,8 +123,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
> {
> struct tegra_devfreq *tegra;
> struct tegra_mc *mc;
> - unsigned long max_rate;
> - unsigned long rate;
> + unsigned int i;
> int err;
>
> mc = tegra_get_memory_controller();
> @@ -151,12 +150,17 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>
> tegra->regs = mc->regs;
>
> - max_rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
> -
> - for (rate = 0; rate <= max_rate; rate++) {
> - rate = clk_round_rate(tegra->emc_clock, rate);
> + if (!mc->num_timings) {

Could you explain what is meaning of 'num_timing?
Also, why add the opp entry in case of mc->num_timings is zero?

> + err = dev_pm_opp_add(&pdev->dev,
> + clk_get_rate(tegra->emc_clock), 0);
> + if (err) {
> + dev_err(&pdev->dev, "failed to add OPP: %d\n", err);
> + return err;
> + }
> + }
>
> - err = dev_pm_opp_add(&pdev->dev, rate, 0);
> + for (i = 0; i < mc->num_timings; i++) {
> + err = dev_pm_opp_add(&pdev->dev, mc->timings[i].rate, 0);
> if (err) {
> dev_err(&pdev->dev, "failed to add opp: %d\n", err);
> goto remove_opps;
>


--
Best Regards,
Chanwoo Choi
Samsung Electronics

2020-07-02 05:08:27

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v4 12/37] PM / devfreq: tegra20: Use MC timings for building OPP table

02.07.2020 07:18, Chanwoo Choi пишет:
> Hi Dmitry,
>
> On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
>> The clk_round_rate() won't be usable for building OPP table once
>> interconnect support will be added to the EMC driver because that CLK API
>> function limits the rounded rate based on the clk rate that is imposed by
>> active clk-users, and thus, the rounding won't work as expected if
>> interconnect will set the minimum EMC clock rate before devfreq driver is
>> loaded. The struct tegra_mc contains memory timings which could be used by
>> the devfreq driver for building up OPP table instead of rounding clock
>> rate, this patch implements this idea.
>>
>> Signed-off-by: Dmitry Osipenko <[email protected]>
>> ---
>> drivers/devfreq/tegra20-devfreq.c | 18 +++++++++++-------
>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
>> index 6469dc69c5e0..bf504ca4dea2 100644
>> --- a/drivers/devfreq/tegra20-devfreq.c
>> +++ b/drivers/devfreq/tegra20-devfreq.c
>> @@ -123,8 +123,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>> {
>> struct tegra_devfreq *tegra;
>> struct tegra_mc *mc;
>> - unsigned long max_rate;
>> - unsigned long rate;
>> + unsigned int i;
>> int err;
>>
>> mc = tegra_get_memory_controller();
>> @@ -151,12 +150,17 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>
>> tegra->regs = mc->regs;
>>
>> - max_rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
>> -
>> - for (rate = 0; rate <= max_rate; rate++) {
>> - rate = clk_round_rate(tegra->emc_clock, rate);
>> + if (!mc->num_timings) {
>
> Could you explain what is meaning of 'num_timing?

The num_timings is the number of memory timings defined in a
device-tree. One timing configuration per memory clock rate.

> Also, why add the opp entry in case of mc->num_timings is zero?

Timings may be not defined in some device-trees at all and in this case
memory always running on a fixed clock rate.

The devfreq driver won't be practically useful if mc->num_timings is
zero since memory frequency can't be changed, but anyways we'd want to
load the devfreq driver in order to prevent confusion about why it's not
loaded.

For example, you may ask somebody to show contents of
/sys/class/devfreq/tegra20-devfreq/trans_stat and the person says to you
that this file doesn't exist, now you'll have to figure out what
happened to the devfreq driver.

2020-07-02 05:20:25

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v4 12/37] PM / devfreq: tegra20: Use MC timings for building OPP table

On 7/2/20 2:07 PM, Dmitry Osipenko wrote:
> 02.07.2020 07:18, Chanwoo Choi пишет:
>> Hi Dmitry,
>>
>> On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
>>> The clk_round_rate() won't be usable for building OPP table once
>>> interconnect support will be added to the EMC driver because that CLK API
>>> function limits the rounded rate based on the clk rate that is imposed by
>>> active clk-users, and thus, the rounding won't work as expected if
>>> interconnect will set the minimum EMC clock rate before devfreq driver is
>>> loaded. The struct tegra_mc contains memory timings which could be used by
>>> the devfreq driver for building up OPP table instead of rounding clock
>>> rate, this patch implements this idea.
>>>
>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>> ---
>>> drivers/devfreq/tegra20-devfreq.c | 18 +++++++++++-------
>>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
>>> index 6469dc69c5e0..bf504ca4dea2 100644
>>> --- a/drivers/devfreq/tegra20-devfreq.c
>>> +++ b/drivers/devfreq/tegra20-devfreq.c
>>> @@ -123,8 +123,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>> {
>>> struct tegra_devfreq *tegra;
>>> struct tegra_mc *mc;
>>> - unsigned long max_rate;
>>> - unsigned long rate;
>>> + unsigned int i;
>>> int err;
>>>
>>> mc = tegra_get_memory_controller();
>>> @@ -151,12 +150,17 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>>
>>> tegra->regs = mc->regs;
>>>
>>> - max_rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
>>> -
>>> - for (rate = 0; rate <= max_rate; rate++) {
>>> - rate = clk_round_rate(tegra->emc_clock, rate);
>>> + if (!mc->num_timings) {
>>
>> Could you explain what is meaning of 'num_timing?
>
> The num_timings is the number of memory timings defined in a
> device-tree. One timing configuration per memory clock rate.

OK. I understand.

>
>> Also, why add the opp entry in case of mc->num_timings is zero?
>
> Timings may be not defined in some device-trees at all and in this case
> memory always running on a fixed clock rate.

You mean that 'timings' information is optional?

>
> The devfreq driver won't be practically useful if mc->num_timings is
> zero since memory frequency can't be changed, but anyways we'd want to
> load the devfreq driver in order to prevent confusion about why it's not
> loaded.
>
> For example, you may ask somebody to show contents of
> /sys/class/devfreq/tegra20-devfreq/trans_stat and the person says to you
> that this file doesn't exist, now you'll have to figure out what
> happened to the devfreq driver.

I understand why add OPP entry point when timing is not defined on DT.
But, actually, I think that you better to change 'timings' info is mandatory
instead of optional. Because the devfreq driver is for DVFS
and the driver supporting DVFS have to have the frequency information
like OPP.

Or,
If you want to keep 'timing' is optional on DT,
I recommend that you add one timing data to tegra mc driver
when DT doesn't include the any timing information
I think that is it more clear.

--
Best Regards,
Chanwoo Choi
Samsung Electronics

2020-07-02 05:44:07

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v4 12/37] PM / devfreq: tegra20: Use MC timings for building OPP table

02.07.2020 08:30, Chanwoo Choi пишет:
> On 7/2/20 2:07 PM, Dmitry Osipenko wrote:
>> 02.07.2020 07:18, Chanwoo Choi пишет:
>>> Hi Dmitry,
>>>
>>> On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
>>>> The clk_round_rate() won't be usable for building OPP table once
>>>> interconnect support will be added to the EMC driver because that CLK API
>>>> function limits the rounded rate based on the clk rate that is imposed by
>>>> active clk-users, and thus, the rounding won't work as expected if
>>>> interconnect will set the minimum EMC clock rate before devfreq driver is
>>>> loaded. The struct tegra_mc contains memory timings which could be used by
>>>> the devfreq driver for building up OPP table instead of rounding clock
>>>> rate, this patch implements this idea.
>>>>
>>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>>> ---
>>>> drivers/devfreq/tegra20-devfreq.c | 18 +++++++++++-------
>>>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
>>>> index 6469dc69c5e0..bf504ca4dea2 100644
>>>> --- a/drivers/devfreq/tegra20-devfreq.c
>>>> +++ b/drivers/devfreq/tegra20-devfreq.c
>>>> @@ -123,8 +123,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>>> {
>>>> struct tegra_devfreq *tegra;
>>>> struct tegra_mc *mc;
>>>> - unsigned long max_rate;
>>>> - unsigned long rate;
>>>> + unsigned int i;
>>>> int err;
>>>>
>>>> mc = tegra_get_memory_controller();
>>>> @@ -151,12 +150,17 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>>>
>>>> tegra->regs = mc->regs;
>>>>
>>>> - max_rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
>>>> -
>>>> - for (rate = 0; rate <= max_rate; rate++) {
>>>> - rate = clk_round_rate(tegra->emc_clock, rate);
>>>> + if (!mc->num_timings) {
>>>
>>> Could you explain what is meaning of 'num_timing?
>>
>> The num_timings is the number of memory timings defined in a
>> device-tree. One timing configuration per memory clock rate.
>
> OK. I understand.
>
>>
>>> Also, why add the opp entry in case of mc->num_timings is zero?
>>
>> Timings may be not defined in some device-trees at all and in this case
>> memory always running on a fixed clock rate.
>
> You mean that 'timings' information is optional?

Yes

Actually, looks like I missed to properly test this case where timings
are missing in DT and it shouldn't work with the current code. I'll fix
it in the next version.

>>
>> The devfreq driver won't be practically useful if mc->num_timings is
>> zero since memory frequency can't be changed, but anyways we'd want to
>> load the devfreq driver in order to prevent confusion about why it's not
>> loaded.
>>
>> For example, you may ask somebody to show contents of
>> /sys/class/devfreq/tegra20-devfreq/trans_stat and the person says to you
>> that this file doesn't exist, now you'll have to figure out what
>> happened to the devfreq driver.
>
> I understand why add OPP entry point when timing is not defined on DT.
> But, actually, I think that you better to change 'timings' info is mandatory
> instead of optional. Because the devfreq driver is for DVFS
> and the driver supporting DVFS have to have the frequency information
> like OPP.
>
> Or,
> If you want to keep 'timing' is optional on DT,
> I recommend that you add one timing data to tegra mc driver
> when DT doesn't include the any timing information
> I think that is it more clear.

Okay, I'll move it into the MC driver in the next version.

Thank you for the review!

2020-07-02 05:53:52

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v4 12/37] PM / devfreq: tegra20: Use MC timings for building OPP table

02.07.2020 08:43, Dmitry Osipenko пишет:
> 02.07.2020 08:30, Chanwoo Choi пишет:
>> On 7/2/20 2:07 PM, Dmitry Osipenko wrote:
>>> 02.07.2020 07:18, Chanwoo Choi пишет:
>>>> Hi Dmitry,
>>>>
>>>> On 6/9/20 10:13 PM, Dmitry Osipenko wrote:
>>>>> The clk_round_rate() won't be usable for building OPP table once
>>>>> interconnect support will be added to the EMC driver because that CLK API
>>>>> function limits the rounded rate based on the clk rate that is imposed by
>>>>> active clk-users, and thus, the rounding won't work as expected if
>>>>> interconnect will set the minimum EMC clock rate before devfreq driver is
>>>>> loaded. The struct tegra_mc contains memory timings which could be used by
>>>>> the devfreq driver for building up OPP table instead of rounding clock
>>>>> rate, this patch implements this idea.
>>>>>
>>>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>>>> ---
>>>>> drivers/devfreq/tegra20-devfreq.c | 18 +++++++++++-------
>>>>> 1 file changed, 11 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/drivers/devfreq/tegra20-devfreq.c b/drivers/devfreq/tegra20-devfreq.c
>>>>> index 6469dc69c5e0..bf504ca4dea2 100644
>>>>> --- a/drivers/devfreq/tegra20-devfreq.c
>>>>> +++ b/drivers/devfreq/tegra20-devfreq.c
>>>>> @@ -123,8 +123,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>>>> {
>>>>> struct tegra_devfreq *tegra;
>>>>> struct tegra_mc *mc;
>>>>> - unsigned long max_rate;
>>>>> - unsigned long rate;
>>>>> + unsigned int i;
>>>>> int err;
>>>>>
>>>>> mc = tegra_get_memory_controller();
>>>>> @@ -151,12 +150,17 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
>>>>>
>>>>> tegra->regs = mc->regs;
>>>>>
>>>>> - max_rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
>>>>> -
>>>>> - for (rate = 0; rate <= max_rate; rate++) {
>>>>> - rate = clk_round_rate(tegra->emc_clock, rate);
>>>>> + if (!mc->num_timings) {
>>>>
>>>> Could you explain what is meaning of 'num_timing?
>>>
>>> The num_timings is the number of memory timings defined in a
>>> device-tree. One timing configuration per memory clock rate.
>>
>> OK. I understand.
>>
>>>
>>>> Also, why add the opp entry in case of mc->num_timings is zero?
>>>
>>> Timings may be not defined in some device-trees at all and in this case
>>> memory always running on a fixed clock rate.
>>
>> You mean that 'timings' information is optional?
>
> Yes
>
> Actually, looks like I missed to properly test this case where timings
> are missing in DT and it shouldn't work with the current code. I'll fix
> it in the next version.
>
>>>
>>> The devfreq driver won't be practically useful if mc->num_timings is
>>> zero since memory frequency can't be changed, but anyways we'd want to
>>> load the devfreq driver in order to prevent confusion about why it's not
>>> loaded.
>>>
>>> For example, you may ask somebody to show contents of
>>> /sys/class/devfreq/tegra20-devfreq/trans_stat and the person says to you
>>> that this file doesn't exist, now you'll have to figure out what
>>> happened to the devfreq driver.
>>
>> I understand why add OPP entry point when timing is not defined on DT.
>> But, actually, I think that you better to change 'timings' info is mandatory
>> instead of optional. Because the devfreq driver is for DVFS
>> and the driver supporting DVFS have to have the frequency information
>> like OPP.

That's what I initially did by bailing out from driver's probe if
timings info is missing, until ran into a situation described above :)

>> Or,
>> If you want to keep 'timing' is optional on DT,
>> I recommend that you add one timing data to tegra mc driver
>> when DT doesn't include the any timing information
>> I think that is it more clear.
>
> Okay, I'll move it into the MC driver in the next version.
>
> Thank you for the review!
>

2020-07-02 12:37:36

by Georgi Djakov

[permalink] [raw]
Subject: Re: [PATCH v4 28/37] memory: tegra: Register as interconnect provider

Hi Dmitry,

On 7/2/20 02:36, Dmitry Osipenko wrote:
> 01.07.2020 20:12, Georgi Djakov пишет:
>> Hi Dmitry,
>>
>> Thank you for updating the patches!
>
> Hello, Georgi!
>
> Thank you for the review!
>
>> On 6/9/20 16:13, Dmitry Osipenko wrote:
>>> Now memory controller is a memory interconnection provider. This allows us
>>> to use interconnect API in order to change memory configuration.
>>>
>>> Signed-off-by: Dmitry Osipenko <[email protected]>
>>> ---
>>> drivers/memory/tegra/Kconfig | 1 +
>>> drivers/memory/tegra/mc.c | 114 +++++++++++++++++++++++++++++++++++
>>> drivers/memory/tegra/mc.h | 8 +++
>>> include/soc/tegra/mc.h | 3 +
>>> 4 files changed, 126 insertions(+)
>>>
>>> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
>>> index 5bf75b316a2f..7055fdef2c32 100644
>>> --- a/drivers/memory/tegra/Kconfig
>>> +++ b/drivers/memory/tegra/Kconfig
>>> @@ -3,6 +3,7 @@ config TEGRA_MC
>>> bool "NVIDIA Tegra Memory Controller support"
>>> default y
>>> depends on ARCH_TEGRA
>>> + select INTERCONNECT
>>> help
>>> This driver supports the Memory Controller (MC) hardware found on
>>> NVIDIA Tegra SoCs.
>>> diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
>>> index 772aa021b5f6..7ef7ac9e103e 100644
>>> --- a/drivers/memory/tegra/mc.c
>>> +++ b/drivers/memory/tegra/mc.c
>>> @@ -594,6 +594,118 @@ static __maybe_unused irqreturn_t tegra20_mc_irq(int irq, void *data)
>>> return IRQ_HANDLED;
>>> }
>>>
>>> +static int tegra_mc_icc_set(struct icc_node *src, struct icc_node *dst)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static int tegra_mc_icc_aggregate(struct icc_node *node,
>>> + u32 tag, u32 avg_bw, u32 peak_bw,
>>> + u32 *agg_avg, u32 *agg_peak)
>>> +{
>>> + *agg_avg = min((u64)avg_bw + (*agg_avg), (u64)U32_MAX);
>>> + *agg_peak = max(*agg_peak, peak_bw);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/*
>>> + * Memory Controller (MC) has few Memory Clients that are issuing memory
>>> + * bandwidth allocation requests to the MC interconnect provider. The MC
>>> + * provider aggregates the requests and then sends the aggregated request
>>> + * up to the External Memory Controller (EMC) interconnect provider which
>>> + * re-configures hardware interface to External Memory (EMEM) in accordance
>>> + * to the required bandwidth. Each MC interconnect node represents an
>>> + * individual Memory Client.
>>> + *
>>> + * Memory interconnect topology:
>>> + *
>>> + * +----+
>>> + * +--------+ | |
>>> + * | TEXSRD +--->+ |
>>> + * +--------+ | |
>>> + * | | +-----+ +------+
>>> + * ... | MC +--->+ EMC +--->+ EMEM |
>>> + * | | +-----+ +------+
>>> + * +--------+ | |
>>> + * | DISP.. +--->+ |
>>> + * +--------+ | |
>>> + * +----+
>>> + */
>>> +static int tegra_mc_interconnect_setup(struct tegra_mc *mc)
>>> +{
>>> + struct icc_onecell_data *data;
>>> + struct icc_node *node;
>>> + unsigned int num_nodes;
>>> + unsigned int i;
>>> + int err;
>>> +
>>> + /* older device-trees don't have interconnect properties */
>>> + if (!of_find_property(mc->dev->of_node, "#interconnect-cells", NULL))
>>> + return 0;
>>> +
>>> + num_nodes = mc->soc->num_clients;
>>> +
>>> + data = devm_kzalloc(mc->dev, struct_size(data, nodes, num_nodes),
>>> + GFP_KERNEL);
>>> + if (!data)
>>> + return -ENOMEM;
>>> +
>>> + mc->provider.dev = mc->dev;
>>> + mc->provider.set = tegra_mc_icc_set;
>>
>> Hmm, maybe the core should not require a set() implementation and we can
>> just make it optional instead. Then the dummy function would not be needed.
>
> Eventually this dummy function might become populated with a memory
> latency allowness programming. I could add a comment into that function
> in the next version, saying that it's to-be-done for now.

Ah ok! Sounds good, thanks for clarifying!

>>> + mc->provider.data = data;
>>> + mc->provider.xlate = of_icc_xlate_onecell;
>>> + mc->provider.aggregate = tegra_mc_icc_aggregate;
>>> +
>>> + err = icc_provider_add(&mc->provider);
>>> + if (err)
>>> + goto err_msg;
>>
>> Nit: I am planning to re-organize some of the existing drivers to call
>> icc_provider_add() after the topology is populated. Could you please move
>> this after the nodes are created and linked.
>
> Are you planning to remove the provider's list-head initialization from
> the icc_provider_add() [1] and move it to the individual provider
> drivers, correct?

Yes, that would be the first step, but i need to post some patches first,
so let's keep it as-is for now. Sorry for the confusion.

Thanks,
Georgi

2020-07-03 08:41:49

by Dmitry Osipenko

[permalink] [raw]
Subject: Re: [PATCH v4 28/37] memory: tegra: Register as interconnect provider

02.07.2020 15:36, Georgi Djakov пишет:
...
>>>> + mc->provider.data = data;
>>>> + mc->provider.xlate = of_icc_xlate_onecell;
>>>> + mc->provider.aggregate = tegra_mc_icc_aggregate;
>>>> +
>>>> + err = icc_provider_add(&mc->provider);
>>>> + if (err)
>>>> + goto err_msg;
>>>
>>> Nit: I am planning to re-organize some of the existing drivers to call
>>> icc_provider_add() after the topology is populated. Could you please move
>>> this after the nodes are created and linked.
>>
>> Are you planning to remove the provider's list-head initialization from
>> the icc_provider_add() [1] and move it to the individual provider
>> drivers, correct?
>
> Yes, that would be the first step, but i need to post some patches first,
> so let's keep it as-is for now. Sorry for the confusion.

No problems, I'll keep an eye on the ICC core changes!
Please feel free to the CC me on the patches, I may give them a whirl.