[RFC,2/3] arm64: dts: qcom: sdm845-cheza: Re-add reserved memory

Commit Message

Rob Clark May 9, 2019, 6:44 p.m. UTC

From: Douglas Anderson <[e-mail]>

Let's fixup the reserved memory to re-add the things we deleted in
("CHROMIUM: arm64: dts: qcom: sdm845-cheza: Temporarily delete
reserved-mem changes") in a way that plays nicely with the new
upstream definitions.

Signed-off-by: Douglas Anderson <[e-mail]>
Reviewed-by: Stephen Boyd <[e-mail]>
Signed-off-by: Rob Clark <[e-mail]>
---
 arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)

Comments

Doug Anderson May 13, 2019, 10:48 p.m. UTC | #1

Hi,

On Thu, May 9, 2019 at 11:44 AM Rob Clark <[e-mail]> wrote:

> From: Douglas Anderson <[e-mail]>
>
> Let's fixup the reserved memory to re-add the things we deleted in
> ("CHROMIUM: arm64: dts: qcom: sdm845-cheza: Temporarily delete
> reserved-mem changes") in a way that plays nicely with the new
> upstream definitions.

The message above makes no sense since that commit you reference isn't
in upstream.

...but in any case, why not squash this in with the previous commit?


> Signed-off-by: Douglas Anderson <[e-mail]>
> Reviewed-by: Stephen Boyd <[e-mail]>
> Signed-off-by: Rob Clark <[e-mail]>

Remove Stephen's Reviewed-by.  In general reviews that happen in the
Chrome OS gerrit shouldn't be carried over when things are posted
upstream.


> +/* Increase the size from 2MB to 8MB */
> +&rmtfs_mem {
> +       reg = <0 0x88f00000 0 0x800000>;
> +};
> +
> +/ {
> +       reserved-memory {
> +               venus_mem: memory@96000000 {
> +                       reg = <0 0x96000000 0 0x500000>;
> +                       no-map;
> +               };
> +       };
> +};

nit: blank line?

-Doug

Rob Clark May 15, 2019, 4:09 a.m. UTC | #2

On Mon, May 13, 2019 at 3:48 PM Doug Anderson <[e-mail]> wrote:
>
> Hi,
>
> On Thu, May 9, 2019 at 11:44 AM Rob Clark <[e-mail]> wrote:
>
> > From: Douglas Anderson <[e-mail]>
> >
> > Let's fixup the reserved memory to re-add the things we deleted in
> > ("CHROMIUM: arm64: dts: qcom: sdm845-cheza: Temporarily delete
> > reserved-mem changes") in a way that plays nicely with the new
> > upstream definitions.
>
> The message above makes no sense since that commit you reference isn't
> in upstream.
>
> ...but in any case, why not squash this in with the previous commit?

Yeah, I should have mentioned this was my intention, I just left it
unsquashed since (at the time) it was something I had cherry-picked on
top of current 4.19 cros kernel..

anyways, I pushed an (unsquashed, converted to fixup!'s) update to:

https://github.com/freedreno/kernel-msm/commits/wip/cheza-dtb-upstreaming

which has updates based on you're review comments (at least assuming I
understood them correctly).. plus some unrelated to cheza-dt patches
on top to get things actually working (ie. ignore everything on top of
the fixup!'s)

I didn't see any comments on the 'delete zap-shader' patch, so
hopefully that means what I did there was a sane (or at least not
insane) way to handle android/linux tz vs what we have on cheza?

BR,
-R


>
>
> > Signed-off-by: Douglas Anderson <[e-mail]>
> > Reviewed-by: Stephen Boyd <[e-mail]>
> > Signed-off-by: Rob Clark <[e-mail]>
>
> Remove Stephen's Reviewed-by.  In general reviews that happen in the
> Chrome OS gerrit shouldn't be carried over when things are posted
> upstream.
>
>
> > +/* Increase the size from 2MB to 8MB */
> > +&rmtfs_mem {
> > +       reg = <0 0x88f00000 0 0x800000>;
> > +};
> > +
> > +/ {
> > +       reserved-memory {
> > +               venus_mem: memory@96000000 {
> > +                       reg = <0 0x96000000 0 0x500000>;
> > +                       no-map;
> > +               };
> > +       };
> > +};
>
> nit: blank line?
>
> -Doug

Jordan Crouse May 15, 2019, 7:24 p.m. UTC | #3

On Tue, May 14, 2019 at 09:09:55PM -0700, Rob Clark wrote:
> On Mon, May 13, 2019 at 3:48 PM Doug Anderson <[e-mail]> wrote:
> >
> > Hi,
> >
> > On Thu, May 9, 2019 at 11:44 AM Rob Clark <[e-mail]> wrote:
> >
> > > From: Douglas Anderson <[e-mail]>
> > >
> > > Let's fixup the reserved memory to re-add the things we deleted in
> > > ("CHROMIUM: arm64: dts: qcom: sdm845-cheza: Temporarily delete
> > > reserved-mem changes") in a way that plays nicely with the new
> > > upstream definitions.
> >
> > The message above makes no sense since that commit you reference isn't
> > in upstream.
> >
> > ...but in any case, why not squash this in with the previous commit?
> 
> Yeah, I should have mentioned this was my intention, I just left it
> unsquashed since (at the time) it was something I had cherry-picked on
> top of current 4.19 cros kernel..
> 
> anyways, I pushed an (unsquashed, converted to fixup!'s) update to:
> 
> https://github.com/freedreno/kernel-msm/commits/wip/cheza-dtb-upstreaming
> 
> which has updates based on you're review comments (at least assuming I
> understood them correctly).. plus some unrelated to cheza-dt patches
> on top to get things actually working (ie. ignore everything on top of
> the fixup!'s)
> 
> I didn't see any comments on the 'delete zap-shader' patch, so
> hopefully that means what I did there was a sane (or at least not
> insane) way to handle android/linux tz vs what we have on cheza?

Yeah. In a world where all the 845 firmware is in linux-firmware the best
differentiating factor would be the absence of the reserved memory or the
zap-shader node in the device tree. Otherwise we would have to try and fail to
execute the scm call and then make some sort of educated guess as to if it
failed for the "right" reasons.

Jordan

> > > Signed-off-by: Douglas Anderson <[e-mail]>
> > > Reviewed-by: Stephen Boyd <[e-mail]>
> > > Signed-off-by: Rob Clark <[e-mail]>
> >
> > Remove Stephen's Reviewed-by.  In general reviews that happen in the
> > Chrome OS gerrit shouldn't be carried over when things are posted
> > upstream.
> >
> >
> > > +/* Increase the size from 2MB to 8MB */
> > > +&rmtfs_mem {
> > > +       reg = <0 0x88f00000 0 0x800000>;
> > > +};
> > > +
> > > +/ {
> > > +       reserved-memory {
> > > +               venus_mem: memory@96000000 {
> > > +                       reg = <0 0x96000000 0 0x500000>;
> > > +                       no-map;
> > > +               };
> > > +       };
> > > +};
> >
> > nit: blank line?
> >
> > -Doug

Doug Anderson May 15, 2019, 9:50 p.m. UTC | #4

Hi,

On Tue, May 14, 2019 at 9:10 PM Rob Clark <[e-mail]> wrote:

> On Mon, May 13, 2019 at 3:48 PM Doug Anderson <[e-mail]> wrote:
> >
> > Hi,
> >
> > On Thu, May 9, 2019 at 11:44 AM Rob Clark <[e-mail]> wrote:
> >
> > > From: Douglas Anderson <[e-mail]>
> > >
> > > Let's fixup the reserved memory to re-add the things we deleted in
> > > ("CHROMIUM: arm64: dts: qcom: sdm845-cheza: Temporarily delete
> > > reserved-mem changes") in a way that plays nicely with the new
> > > upstream definitions.
> >
> > The message above makes no sense since that commit you reference isn't
> > in upstream.
> >
> > ...but in any case, why not squash this in with the previous commit?
>
> Yeah, I should have mentioned this was my intention, I just left it
> unsquashed since (at the time) it was something I had cherry-picked on
> top of current 4.19 cros kernel..
>
> anyways, I pushed an (unsquashed, converted to fixup!'s) update to:
>
> https://github.com/freedreno/kernel-msm/commits/wip/cheza-dtb-upstreaming
>
> which has updates based on you're review comments (at least assuming I
> understood them correctly).. plus some unrelated to cheza-dt patches
> on top to get things actually working (ie. ignore everything on top of
> the fixup!'s)

Looks OK to me.  Are you going to post this?  Bjorn / Andy: do you
guys have opinions here?


> I didn't see any comments on the 'delete zap-shader' patch, so
> hopefully that means what I did there was a sane (or at least not
> insane) way to handle android/linux tz vs what we have on cheza?

I wasn't CCed, so I assumed you were looking for feedback from others
on that one.  ;-)  Oh, but I guess Jordan and Bjorn also weren't
CCed...  In any case, I replied now.

Bjorn Andersson May 15, 2019, 11:41 p.m. UTC | #5

On Wed 15 May 14:50 PDT 2019, Doug Anderson wrote:

> Hi,
> 
> On Tue, May 14, 2019 at 9:10 PM Rob Clark <[e-mail]> wrote:
> 
> > On Mon, May 13, 2019 at 3:48 PM Doug Anderson <[e-mail]> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, May 9, 2019 at 11:44 AM Rob Clark <[e-mail]> wrote:
> > >
> > > > From: Douglas Anderson <[e-mail]>
> > > >
> > > > Let's fixup the reserved memory to re-add the things we deleted in
> > > > ("CHROMIUM: arm64: dts: qcom: sdm845-cheza: Temporarily delete
> > > > reserved-mem changes") in a way that plays nicely with the new
> > > > upstream definitions.
> > >
> > > The message above makes no sense since that commit you reference isn't
> > > in upstream.
> > >
> > > ...but in any case, why not squash this in with the previous commit?
> >
> > Yeah, I should have mentioned this was my intention, I just left it
> > unsquashed since (at the time) it was something I had cherry-picked on
> > top of current 4.19 cros kernel..
> >
> > anyways, I pushed an (unsquashed, converted to fixup!'s) update to:
> >
> > https://github.com/freedreno/kernel-msm/commits/wip/cheza-dtb-upstreaming
> >
> > which has updates based on you're review comments (at least assuming I
> > understood them correctly).. plus some unrelated to cheza-dt patches
> > on top to get things actually working (ie. ignore everything on top of
> > the fixup!'s)
> 
> Looks OK to me.  Are you going to post this?  Bjorn / Andy: do you
> guys have opinions here?
> 

I think this is a good opportunity to get people with Cheza off 4.19, so
I'm all for landing this upstream.

Regards,
Bjorn

> 
> > I didn't see any comments on the 'delete zap-shader' patch, so
> > hopefully that means what I did there was a sane (or at least not
> > insane) way to handle android/linux tz vs what we have on cheza?
> 
> I wasn't CCed, so I assumed you were looking for feedback from others
> on that one.  ;-)  Oh, but I guess Jordan and Bjorn also weren't
> CCed...  In any case, I replied now.

Patch

diff --git a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
index 338c92ddd1c3..8ccbe246dff4 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi
@@ -156,10 +156,44 @@ 
 	};
 };
 
+/*
+ * Reserved memory changes
+ *
+ * Putting this all together (out of order with the rest of the file) to keep
+ * 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/ &venus_mem;
+/delete-node/ &cdsp_mem;
+/delete-node/ &cdsp_pas;
+
+/* Increase the size from 120 MB to 128 MB */
 &mpss_region {
 	reg = <0 0x8e000000 0 0x8000000>;
 };
 
+/* Increase the size from 2MB to 8MB */
+&rmtfs_mem {
+	reg = <0 0x88f00000 0 0x800000>;
+};
+
+/ {
+	reserved-memory {
+		venus_mem: memory@96000000 {
+			reg = <0 0x96000000 0 0x500000>;
+			no-map;
+		};
+	};
+};
 &qspi {
 	status = "okay";
 	pinctrl-names = "default";