Re: [RFC PATCH 4/6] arm64: dts: qcom: sm8650p: introduce sm8650p dtsi

From: Aiqun Yu (Maria)
Date: Fri Feb 23 2024 - 04:10:29 EST




On 2/22/2024 9:21 PM, neil.armstrong@xxxxxxxxxx wrote:
On 05/02/2024 12:57, Jingyi Wang wrote:
Introduce sm8650p dtsi, sm8650p has same base functions
as sm8650 with different memory regions.

There are 3 types of reserved memory regions here:
1. Firmware related regions.
     This will be described as: reserved-region@address. Current
reserved-region may have reserved area which was not yet used, release
note of the firmware can have such kind of information.
2. Firmware related which shared with kernel access.
     Each region will have a specific node with specific label name for
later phandle reference from other driver dt node. May overlapping with
above type regions.
3. PIL regions.
     PIL regions are allocated by kernel and assigned to subsystem
firmware later.
Here is a map for this platform:
0x100000000 +------------------+
             |                  |
             | Firmware Related |
             |                  |
  0xd8000000 +------------------+
             |                  |
             | Kernel Available |
             |                  |
  0xA7000000 +------------------+
             |                  |
             |    PIL Region    |
             |                  |
  0x8BC00000 +------------------+
             |                  |
             | Firmware Related |
             |                  |
  0x80000000 +------------------+
Note that:
1. 0xA7000000 to 0xA8000000 was used by bootloader as well, not suggest
for other usage.
2. Kernel start address was start at 0xA8000000.

Signed-off-by: Jingyi Wang <quic_jingyw@xxxxxxxxxxx>
---
  arch/arm64/boot/dts/qcom/sm8650p.dtsi | 180 ++++++++++++++++++++++++++
  1 file changed, 180 insertions(+)
  create mode 100644 arch/arm64/boot/dts/qcom/sm8650p.dtsi

diff --git a/arch/arm64/boot/dts/qcom/sm8650p.dtsi b/arch/arm64/boot/dts/qcom/sm8650p.dtsi
new file mode 100644
index 000000000000..26dfe315b49d
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sm8650p.dtsi
@@ -0,0 +1,180 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (c) 2024, Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#include "sm8650.dtsi"
+
+/delete-node/ &reserved_memory;
+
+/ {
+    reserved_memory: reserved-memory {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        ranges;
+
+        /*
+         * There are 3 types of reserved memory regions here:
+         * 1. Firmware related regions.
+         *     This will be described as: reserved-region@address. Current
+         * reserved-region may have reserved area which was not yet used,
+         * release note of the firmware can have such kind of information.
+         * 2. Firmware related which shared with kernel access.
+         *     Each region will have a specific node with specific label
+         * name for later phandle reference from other driver dt node. May
+         * overlapping with above type regions.
+         * 3. PIL regions.
+         *     PIL regions are allocated by kernel and assigned to subsystem
+         * firmware later.
+         * Here is a map for this platform:
+         * 0x100000000 +------------------+
+         *             |                  |
+         *             | Firmware Related |
+         *             |                  |
+         *  0xd8000000 +------------------+
+         *             |                  |
+         *             | Kernel Available |
+         *             |                  |
+         *  0xA7000000 +------------------+
+         *             |                  |
+         *             |    PIL Region    |
+         *             |                  |
+         *  0x8BC00000 +------------------+
+         *             |                  |
+         *             | Firmware Related |
+         *             |                  |
+         *  0x80000000 +------------------+
+         * Note that:
+         * 1. 0xA7000000 to 0xA8000000 was used by bootloader as well, not
+         * suggest for other usage.
+         * 2. Kernel start address was start at 0xA8000000.
+         */
+
+        /* Firmware related regions */
+        reserved-region@80000000 {
+            reg = <0x0 0x80000000 0x0 0xbc00000>;
+            no-map;
+        };

Ok this region goes up to 0x8BC00000 and so overlaps with the next regions:
The idea here is to reserve more needed ddr regions for different version of firmware compatibility. While inside this region which shared device memory from firmware to kernel, it is still needed to have node information in the device tree.

More clear reference here for the firmware needed no-map reserved region diagram, take the smem_mem here to be exposed shared read to kernel:
* 0x8BC00000 +------------------+
* | |
* | reserved_region2 |
* 0x81c60000 +------------------+
* | smem_mem |
* 0x81c00000 +------------------+
* | reserved_region1|
* 0x80000000 +------------------+

what's the suggestion here for this requirement?:
option 1: have a big region_reserved, and then have smem_mem overlap reserved region node information inside the dt.
option 2: Have each separate "reserved_region1 node + smem_mem node + reserved_region2 node".
other options?


+
+        aop_image_mem: aop-image-region@81c00000 {
+            reg = <0x0 0x81c00000 0x0 0x60000>;
+            no-map;
+        };
+
+        aop_cmd_db_mem: aop-cmd-db-region@81c60000 {
+            compatible = "qcom,cmd-db";
+            reg = <0x0 0x81c60000 0x0 0x20000>;
+            no-map;
+        };
+
+        aop_config_mem: aop-config-region@81c80000 {
+            no-map;
+            reg = <0x0 0x81c80000 0x0 0x20000>;
+        };
+
+        smem_mem: smem-region@81d00000 {
+            compatible = "qcom,smem";
+            reg = <0x0 0x81d00000 0x0 0x200000>;
+            hwlocks = <&tcsr_mutex 3>;
+            no-map;
+        };
+
+        adsp_mhi_mem: adsp-mhi-region@81f00000 {
+            reg = <0x0 0x81f00000 0x0 0x20000>;
+            no-map;
+        };
+
+        global_sync_mem: global-sync@82600000 {
+            reg = <0 0x82600000 0 0x100000>;
+            no-map;
+        };
+
+        mpss_dsm_mem: mpss-dsm@86b00000 {
+            reg = <0 0x86b00000 0 0x4900000>;
+            no-map;
+        };
+
+        mpss_dsm_mem_2: mpss-dsm-2@8b400000 {
+            reg = <0 0x8b400000 0 0x800000>;
+            no-map;
+        };

up to here

Please fix this,

I just checked against plain sm8650.dtsi and actually the memory adresses are the same.

So what's the _real_ difference here ? Just drop the superfluous memory zones and redefine them if needed.
With big reserved regions agreed, I think the memory map can be modified directly in sm8650.dtsi. It will be a memory map support different derived soc firmware release as well.

Thanks,
Neil

+
+        /* PIL region */
+        mpss_mem: mpss-region@8bc00000 {
+            reg = <0x0 0x8bc00000 0x0 0xf400000>;
+            no-map;
+        };
+
+        q6_mpss_dtb_mem: q6-mpss-dtb-region@9b000000 {
+            reg = <0x0 0x9b000000 0x0 0x80000>;
+            no-map;
+        };
+
+        ipa_fw_mem: ipa-fw-region@9b080000 {
+            reg = <0x0 0x9b080000 0x0 0x10000>;
+            no-map;
+        };
+
+        ipa_gsi_mem: ipa-gsi-region@9b090000 {
+            reg = <0x0 0x9b090000 0x0 0xa000>;
+            no-map;
+        };
+
+        gpu_micro_code_mem: gpu-micro-code-region@9b09a000 {
+            reg = <0x0 0x9b09a000 0x0 0x2000>;
+            no-map;
+        };
+
+        spss_region_mem: spss-region@9b0a0000 {
+            reg = <0x0 0x9b0a0000 0x0 0x1e0000>;
+            no-map;
+        };
+
+        spu_secure_shared_memory_mem: spu-secure-shared-memory-region@9b280000 {
+            reg = <0x0 0x9b280000 0x0 0x80000>;
+            no-map;
+        };
+
+        camera_mem: camera-region@9b300000 {
+            reg = <0x0 0x9b300000 0x0 0x800000>;
+            no-map;
+        };
+
+        video_mem: video-region@9bb00000 {
+            reg = <0x0 0x9bb00000 0x0 0x800000>;
+            no-map;
+        };
+
+        cvp_mem: cvp-region@9c300000 {
+            reg = <0x0 0x9c300000 0x0 0x700000>;
+            no-map;
+        };
+
+        cdsp_mem: cdsp-region@9ca00000 {
+            reg = <0x0 0x9ca00000 0x0 0x1400000>;
+            no-map;
+        };
+
+        q6_cdsp_dtb_mem: q6-cdsp-dtb-region@9de00000 {
+            reg = <0x0 0x9de00000 0x0 0x80000>;
+            no-map;
+        };
+
+        q6_adsp_dtb_mem: q6-adsp-dtb-region@9de80000 {
+            reg = <0x0 0x9de80000 0x0 0x80000>;
+            no-map;
+        };
+
+        adspslpi_mem: adspslpi-region@9df00000 {
+            reg = <0x0 0x9df00000 0x0 0x4080000>;
+            no-map;
+        };
+
+        /* Firmware related regions */
+        reserved-region@d8000000 {
+            reg = <0x0 0xd8000000 0x0 0x28000000>;
+            no-map;
+        };
+
+    };
+};


--
Thx and BRs,
Aiqun(Maria) Yu