Re: [PATCH v2] dt-bindings: memory: tegra: Add external memory controller binding for Tegra210

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/14/19 10:11 PM, Dmitry Osipenko wrote:
12.04.2019 11:08, Joseph Lo пишет:
Add the binding document for the external memory controller (EMC) which
communicates with external LPDDR4 devices. It includes the bindings of
the EMC node and the EMC table of different rates.

To support high rates for LPDDR4, the EMC table must be trained before
it can be used for runtime clock switching. It has been done by firmware
and merged the training data to the table that the kernel can share the
result. So the bindings are used for both kernel and firmware.

Based on the work of Peter De Schrijver <pdeschrijver@xxxxxxxxxx>.

Signed-off-by: Joseph Lo <josephl@xxxxxxxxxx>
---
This patch splits from the original patch set that supports EMC scaling
with binding document and drivers. Because the binding would be shared
by both firmware and kernel. We want to settle this first. Then we can
fix the kernel and firmware to support the same.

Hello Joseph,

Very nice to see that T210 will be able to get a non-alienated binding! I have some comments, please see them below.

Thanks for your review.


Changes in v2:
* only use "tegra210" string in compatible string and remove the legacy
   "tegra21" string.
* clock-frequency -> fix the unit from kilohertz to hertz
* add "interrupts" property
* s/nvidia,emc-min-mv/nvidia,emc-min-millivolt/
* s/nvidia,gk20a-min-mv/nvidia,gk20a-min-millivolt/
* s/nvidia,source/clock-names/
* fix lots of properties that use underline to hyphen
* s/nvidia,emc-clock-latency-change/nvidia,emc-clock-latency-microsecond/
* add more information in the property descriptions
---
  .../nvidia,tegra210-emc.txt                   | 614 ++++++++++++++++++
  1 file changed, 614 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.txt

diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.txt b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.txt
new file mode 100644
index 000000000000..318239c3c295
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.txt
@@ -0,0 +1,614 @@
+NVIDIA Tegra210 SoC EMC (external memory controller)
+====================================================
+
+Required properties :
+- compatible : should be "nvidia,tegra210-emc".
+- reg : physical base address and length of the controller's registers.
+- clocks : phandles of the possible source clocks
+- clock-names : names of the possible source clocks
+- interrupts : Should contain the EMC general interrupt
+- #address-cells : should be 1
+- #size-cells : should be 0
+- nvidia,memory-controller : phandle of the memory controller.
+- nvidia,use-ram-code : boolean, indicates whether we should use RAM_CODE in
+		        the register to find matching emc-table nodes
+
+The node should contain a "emc-table" subnode for each supported RAM type
+(see field RAM_CODE in register APB_MISC_PP_STRAPPING_OPT_A), with its unit
+address being its RAM_CODE.
+
+Required properties for "emc-tables" nodes :
+- nvidia,ram-code : Should contain the value of RAM_CODE this timing set is
+		    used for.
+
+Each "emc-tables" node should contain a "emc-table" subnode for every supported
+EMC clock rate. The "emc-table" subnodes should have the clock rate in hertz as
+their unit address.
+
+Required properties for "emc-table" nodes :
+- compatible :  "nvidia,tegra210-emc-table"
+- nvidia,revision : revision of the parameter set used for this node. All
+                    nodes in the same "emc-table" should have the same revision
+- nvidia,dvfs-version : string for the DVFS version of this table

What's the "DVFS version"? Sounds like some kind of software description to me. This is probably irrelevant for upstream.
Yes, you are right. Will remove this.


+- clock-frequency : frequency in hertz
+- clock-names : name of clock source to be used for this rate

Please follow the T124 and move out everything clock-related to the clock-and-reset binding.
Will remove this property.


+- nvidia,emc-min-millivolt : minimum voltage in millivolt for this rate

What about to move out the voltages description into OPP table?

+- nvidia,gk20a-min-millivolt : minimum GPU voltage in millivolt for this rate

Originally, the two properties above were designed for DVFS, which can evaluate if the rate is available for the minimal voltage supplied.

Because we haven't support DVFS in upstream kernel yet, I will remove both of the properties.

Could you please explain why memory voltage depends on the GPU's? Is there any kind of electrical coupling in this case and thus this is a voltage regulators coupling? >
Is GPU-memory voltage dependency really could vary from board to board? It looks to me that at maximum the dependency could vary between SoC SKU variations and then this probably should be internal to the kernel.


For the GPU DVFS on Tegra210, it's controlled itself in the GPU driver, like the DFLL support for the CPU. So fine to remove that.

+- nvidia,src-sel-reg : value of EMC CAR register to be used for this rate
+- nvidia,needs-training : 1 if needs training at boot, 0 otherwise
+- nvidia,trained : 1 if initial training has been done by firmware, 0 otherwise
+- nvidia,periodic-training : 1 if needs periodic training, 0 otherwise
+- nvidia,trained-dram-clktree-c0d0u0 : training data word
+- nvidia,trained-dram-clktree-c0d0u1 : training data word
+- nvidia,trained-dram-clktree-c0d1u0 : training data word
+- nvidia,trained-dram-clktree-c0d1u1 : training data word
+- nvidia,trained-dram-clktree-c1d0u0 : training data word
+- nvidia,trained-dram-clktree-c1d0u1 : training data word
+- nvidia,trained-dram-clktree-c1d1u0 : training data word
+- nvidia,trained-dram-clktree-c1d1u1 : training data word
+- nvidia,current-dram-clktree-c0d0u0 : training data word
+- nvidia,current-dram-clktree-c0d0u1 : training data word
+- nvidia,current-dram-clktree-c0d1u0 : training data word
+- nvidia,current-dram-clktree-c0d1u1 : training data word
+- nvidia,current-dram-clktree-c1d0u0 : training data word
+- nvidia,current-dram-clktree-c1d0u1 : training data word
+- nvidia,current-dram-clktree-c1d1u0 : training data word
+- nvidia,current-dram-clktree-c1d1u1 : training data word

So the "clktree" properties describe the clock properties, hence should be in the clock-and-reset binding (clock's emc-timings table, see T124 for the example). Same for the rest of clk-related properties below.

It's not really about the clock-and-reset. This is the timing data of the DDR4 DRAM characteristic data after training. I think the current-dram-clktree-* can be removed, only need to keep the trained one. Will check.


+- nvidia,emc-burst-mc-regs : a 33 word array of the following registers
+			     (See TRM 18.10.1 for register descriptions)
+	MC_EMEM_ARB_CFG
+	MC_EMEM_ARB_OUTSTANDING_REQ
+	MC_EMEM_ARB_REFPB_HP_CTRL
+	MC_EMEM_ARB_REFPB_BANK_CTRL
+	MC_EMEM_ARB_TIMING_RCD
+	MC_EMEM_ARB_TIMING_RP
+	MC_EMEM_ARB_TIMING_RC
+	MC_EMEM_ARB_TIMING_RAS
+	MC_EMEM_ARB_TIMING_FAW
+	MC_EMEM_ARB_TIMING_RRD
+	MC_EMEM_ARB_TIMING_RAP2PRE
+	MC_EMEM_ARB_TIMING_WAP2PRE
+	MC_EMEM_ARB_TIMING_R2R
+	MC_EMEM_ARB_TIMING_W2W
+	MC_EMEM_ARB_TIMING_R2W
+	MC_EMEM_ARB_TIMING_CCDMW
+	MC_EMEM_ARB_TIMING_W2R
+	MC_EMEM_ARB_TIMING_RFCPB
+	MC_EMEM_ARB_DA_TURNS
+	MC_EMEM_ARB_DA_COVERS
+	MC_EMEM_ARB_MISC0
+	MC_EMEM_ARB_MISC1
+	MC_EMEM_ARB_MISC2
+	MC_EMEM_ARB_RING1_THROTTLE
+	MC_EMEM_ARB_DHYST_CTRL
+	MC_EMEM_ARB_DHYST_TIMEOUT_UTIL_0
+	MC_EMEM_ARB_DHYST_TIMEOUT_UTIL_1
+	MC_EMEM_ARB_DHYST_TIMEOUT_UTIL_2
+	MC_EMEM_ARB_DHYST_TIMEOUT_UTIL_3
+	MC_EMEM_ARB_DHYST_TIMEOUT_UTIL_4
+	MC_EMEM_ARB_DHYST_TIMEOUT_UTIL_5
+	MC_EMEM_ARB_DHYST_TIMEOUT_UTIL_6
+	MC_EMEM_ARB_DHYST_TIMEOUT_UTIL_7
+- nvidia,emc-la-scale-regs : a 24 word array of the following registers
+			     (See TRM 18.10.1 for register descriptions)
+	MC_MLL_MPCORER_PTSA_RATE
+	MC_FTOP_PTSA_RATE
+	MC_PTSA_GRANT_DECREMENT
+	MC_LATENCY_ALLOWANCE_XUSB_0
+	MC_LATENCY_ALLOWANCE_XUSB_1
+	MC_LATENCY_ALLOWANCE_TSEC_0
+	MC_LATENCY_ALLOWANCE_SDMMCA_0
+	MC_LATENCY_ALLOWANCE_SDMMCAA_0
+	MC_LATENCY_ALLOWANCE_SDMMC_0
+	MC_LATENCY_ALLOWANCE_SDMMCAB_0
+	MC_LATENCY_ALLOWANCE_PPCS_0
+	MC_LATENCY_ALLOWANCE_PPCS_1
+	MC_LATENCY_ALLOWANCE_MPCORE_0
+	MC_LATENCY_ALLOWANCE_HC_0
+	MC_LATENCY_ALLOWANCE_HC_1
+	MC_LATENCY_ALLOWANCE_AVPC_0
+	MC_LATENCY_ALLOWANCE_GPU_0
+	MC_LATENCY_ALLOWANCE_GPU2_0
+	MC_LATENCY_ALLOWANCE_NVENC_0
+	MC_LATENCY_ALLOWANCE_NVDEC_0
+	MC_LATENCY_ALLOWANCE_VIC_0
+	MC_LATENCY_ALLOWANCE_VI2_0
+	MC_LATENCY_ALLOWANCE_ISP2_0
+	MC_LATENCY_ALLOWANCE_ISP2_1

Please move out everything MC-related to the MC binding.

Okay, will do.

Thanks,
Joseph




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux