Re: [PATCH 1/2] arm64: dts: qcom: sdm845: Move reserved-memory to devices
On Tue 09 Feb 17:25 CST 2021, Doug Anderson wrote:
> Hi,
>
> On Tue, Feb 9, 2021 at 8:09 AM Bjorn Andersson
> wrote:
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> > b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> > index 216a74f0057c..2f44785d1af0 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> > @@ -153,36 +153,66 @@ panel_in_edp: endpoint {
> > * all modifications to the memory map (from sdm845.dtsi) in one place.
> > */
> >
> > -/*
> > - * Our mpss_region is 8MB bigger than the default one and that conflicts
> > - * with venus_mem and cdsp_mem.
> > - *
> > - * For venus_mem we'll delete and re-create at a different address.
> > - *
> > - * cdsp_mem isn't used on cheza right now so we won't bother re-creating
> > it; but
> > - * that also means we need to delete cdsp_pas.
> > - */
> > -/delete-node/ _mem;
> > -/delete-node/ _mem;
> > -/delete-node/ _pas;
>
> Note to self: on cheza we'll end up with "cdsp_pas" existing now, but
> that _should_ be OK since it's disabled
>
That was not intentional, but as you say it shouldn't make a difference.
>
> > @@ -1321,6 +1359,7 @@ config {
> > };
> >
> > {
> > + memory-region = <_mem>;
> > video-firmware {
> > iommus = <_smmu 0x10b2 0x0>;
> > };
>
> slight nit: I think it looks ugly not to have a blank line between the
> property and the sub-node. ;-)
>
I agree, will go over and adjust this in v2.
>
> > @@ -766,8 +697,6 @@ adsp_pas: remoteproc-adsp {
> > clocks = < RPMH_CXO_CLK>;
> > clock-names = "xo";
> >
> > - memory-region = <_mem>;
> > -
>
> Note to self: we're losing the above on cheza, but that _should_ be OK
> since the node is disabled anyway.
>
> Probably not critical at this point, but it makes me wonder whether we
> could remove adsp_mem on cheza...
>
I noticed this too, but figured that this is an actual change, so it
would be better to do in a separate commit - if that's desired.
> ---
>
> So I only looked at the cheza and sdm845 changes and they look fine to
> me and this seems like a good change overall. I'm assuming that folks
> who focus on the other boards will double-check your work there if
> they care or just trust that everything is great. ;-)
>
> OK, I lied. I took a quick glance. In "sdm845-db845c" you maybe miss
> adding the "memory-region" back to the WiFi? Have you tried running
> something like this before/after:
>
> for f in sdm845*.dtb; do dtc -I dtb -O dts --sort $f > $f.dts; done
>
I was not aware of the --sort, so I did this by chaining together some
more things in the shell to confirm that I didn't actually change any
reserved-memory regions...
> You've gotta ignore phandle ID differences but otherwise it'll point
> out things like this.
>
But as the phandles changed all over the place I looked only at the
reserved-memory, will fix up the db845c wifi node and double check the
rest of the nodes. Thanks for spotting this!
Regards,
Bjorn
Re: [PATCH 1/2] arm64: dts: qcom: sdm845: Move reserved-memory to devices
Hi,
On Tue, Feb 9, 2021 at 8:09 AM Bjorn Andersson
wrote:
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> index 216a74f0057c..2f44785d1af0 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
> @@ -153,36 +153,66 @@ panel_in_edp: endpoint {
> * all modifications to the memory map (from sdm845.dtsi) in one place.
> */
>
> -/*
> - * Our mpss_region is 8MB bigger than the default one and that conflicts
> - * with venus_mem and cdsp_mem.
> - *
> - * For venus_mem we'll delete and re-create at a different address.
> - *
> - * cdsp_mem isn't used on cheza right now so we won't bother re-creating it;
> but
> - * that also means we need to delete cdsp_pas.
> - */
> -/delete-node/ _mem;
> -/delete-node/ _mem;
> -/delete-node/ _pas;
Note to self: on cheza we'll end up with "cdsp_pas" existing now, but
that _should_ be OK since it's disabled
> @@ -1321,6 +1359,7 @@ config {
> };
>
> {
> + memory-region = <_mem>;
> video-firmware {
> iommus = <_smmu 0x10b2 0x0>;
> };
slight nit: I think it looks ugly not to have a blank line between the
property and the sub-node. ;-)
> @@ -766,8 +697,6 @@ adsp_pas: remoteproc-adsp {
> clocks = < RPMH_CXO_CLK>;
> clock-names = "xo";
>
> - memory-region = <_mem>;
> -
Note to self: we're losing the above on cheza, but that _should_ be OK
since the node is disabled anyway.
Probably not critical at this point, but it makes me wonder whether we
could remove adsp_mem on cheza...
---
So I only looked at the cheza and sdm845 changes and they look fine to
me and this seems like a good change overall. I'm assuming that folks
who focus on the other boards will double-check your work there if
they care or just trust that everything is great. ;-)
OK, I lied. I took a quick glance. In "sdm845-db845c" you maybe miss
adding the "memory-region" back to the WiFi? Have you tried running
something like this before/after:
for f in sdm845*.dtb; do dtc -I dtb -O dts --sort $f > $f.dts; done
You've gotta ignore phandle ID differences but otherwise it'll point
out things like this.
-Doug
[PATCH 1/2] arm64: dts: qcom: sdm845: Move reserved-memory to devices
The reserved-memory regions used for carrying firmware to be run on the
various cores and co-processors in a Qualcomm platform differs in size,
placement and presence based on each device's feature set and security
configuration.
Rather than providing some basic set that works on the MTP and then
piecemeal patch this up on the various devices, push the configuration
of these regions out to the individual device dts files.
Signed-off-by: Bjorn Andersson
---
arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi| 89 +--
arch/arm64/boot/dts/qcom/sdm845-db845c.dts| 85 ++
arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 87 ++
.../boot/dts/qcom/sdm845-oneplus-common.dtsi | 78 +++-
.../boot/dts/qcom/sdm845-xiaomi-beryllium.dts | 45 ++
arch/arm64/boot/dts/qcom/sdm845.dtsi | 83 -
.../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 86 ++
7 files changed, 428 insertions(+), 125 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
index 216a74f0057c..2f44785d1af0 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
@@ -153,36 +153,66 @@ panel_in_edp: endpoint {
* all modifications to the memory map (from sdm845.dtsi) in one place.
*/
-/*
- * Our mpss_region is 8MB bigger than the default one and that conflicts
- * with venus_mem and cdsp_mem.
- *
- * For venus_mem we'll delete and re-create at a different address.
- *
- * cdsp_mem isn't used on cheza right now so we won't bother re-creating it;
but
- * that also means we need to delete cdsp_pas.
- */
-/delete-node/ _mem;
-/delete-node/ _mem;
-/delete-node/ _pas;
-/delete-node/ _mem;
-
-/* Increase the size from 120 MB to 128 MB */
-_region {
- reg = <0 0x8e00 0 0x800>;
-};
-
-/* Increase the size from 2MB to 8MB */
-_mem {
- reg = <0 0x88f0 0 0x80>;
-};
-
/ {
reserved-memory {
+ tz_mem: memory@8620 {
+ reg = <0 0x8620 0 0x2d0>;
+ no-map;
+ };
+
+ rmtfs_mem: memory@88f0 {
+ compatible = "qcom,rmtfs-mem";
+ reg = <0 0x88f0 0 0x80>;
+ no-map;
+
+ qcom,client-id = <1>;
+ qcom,vmid = <15>;
+ };
+
+ ipa_fw_mem: memory@8c40 {
+ reg = <0 0x8c40 0 0x1>;
+ no-map;
+ };
+
+ ipa_gsi_mem: memory@8c41 {
+ reg = <0 0x8c41 0 0x5000>;
+ no-map;
+ };
+
+ adsp_mem: memory@8c50 {
+ reg = <0 0x8c50 0 0x1a0>;
+ no-map;
+ };
+
+ wlan_msa_mem: memory@8df0 {
+ reg = <0 0x8df0 0 0x10>;
+ no-map;
+ };
+
+ mpss_region: memory@8e00 {
+ reg = <0 0x8e00 0 0x800>;
+ no-map;
+ };
+
venus_mem: memory@9600 {
reg = <0 0x9600 0 0x50>;
no-map;
};
+
+ mba_region: memory@9650 {
+ reg = <0 0x9650 0 0x20>;
+ no-map;
+ };
+
+ slpi_mem: memory@9670 {
+ reg = <0 0x9670 0 0x140>;
+ no-map;
+ };
+
+ spss_mem: memory@97b0 {
+ reg = <0 0x97b0 0 0x10>;
+ no-map;
+ };
};
};
@@ -206,7 +236,6 @@ flash@0 {
};
};
-
_rsc {
pm8998-rpmh-regulators {
compatible = "qcom,pm8998-rpmh-regulators";
@@ -645,6 +674,14 @@ _smmu {
_pil {
iommus = <_smmu 0x781 0x0>,
<_smmu 0x724 0x3>;
+
+ mba {
+ memory-region = <_region>;
+ };
+
+ mpss {
+ memory-region = <_region>;
+ };
};
_pwrkey {
@@ -850,6 +887,7 @@ _2_qmpphy {
{
status = "okay";
+ memory-region = <_msa_mem>;
vdd-0.8-cx-mx-supply = <_pp800_l5a >;
vdd-1.8-xo-supply = <_l7a_wcn3990>;
@@ -1321,6 +1359,7 @@ config {
};
{
+ memory-region = <_mem>;
video-firmware {
iommus = <_smmu 0x10b2 0x0>;
};
diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
index c4ac6f5dc008..0c175685c135 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
@@ -157,6 +157,77 @@ cam3_avdd_2v8: reg_cam3_avdd_2v8 {
vin-supply = <>;
};
+
