[PATCH 1/2] dt-bindings: display/msm: qcom,sdm845-mdss: add memory-region property

[PATCH 1/2] dt-bindings: display/msm: qcom,sdm845-mdss: add memory-region property

[Amit Pundir]

Add and document the reserved memory region property
in the qcom,sdm845-mdss schema.

Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
---
 .../devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml    | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
index 6ecb00920d7f..3ea1dbd7e317 100644
--- a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
@@ -39,6 +39,11 @@ properties:
   interconnect-names:
     maxItems: 2
 
+  memory-region:
+    maxItems: 1
+    description:
+      Phandle to a node describing a reserved memory region.
+
 patternProperties:
   "^display-controller@[0-9a-f]+$":
     type: object
-- 
2.25.1

[PATCH 2/2][v4] arm64: dts: qcom: sdm845-db845c: Mark cont splash memory region as reserved

[Amit Pundir]

Adding a reserved memory region for the framebuffer memory
(the splash memory region set up by the bootloader).

Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
---
v4: Re-sending this along with a new dt-bindings patch to
    document memory-region property in qcom,sdm845-mdss
    schema and keep dtbs_check happy.

v3: Point this reserved region to MDSS.

v2: Updated commit message.

There was some dicussion on v1 but it didn't go anywhere,
https://lore.kernel.org/linux-kernel/20230124182857.1524912-1-amit.pundir@linaro.org/T/#u.
The general consensus is that this memory should be freed and be
made resuable but that (releasing this piece of memory) has been
tried before and it is not trivial to return the reserved memory
node to the system RAM pool in this case.

 arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
index d6b464cb61d6..f546f6f57c1e 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
@@ -101,6 +101,14 @@ hdmi_con: endpoint {
 		};
 	};
 
+	reserved-memory {
+		/* Cont splash region set up by the bootloader */
+		cont_splash_mem: framebuffer@9d400000 {
+			reg = <0x0 0x9d400000 0x0 0x2400000>;
+			no-map;
+		};
+	};
+
 	lt9611_1v8: lt9611-vdd18-regulator {
 		compatible = "regulator-fixed";
 		regulator-name = "LT9611_1V8";
@@ -506,6 +514,7 @@ &i2c14 {
 };
 
 &mdss {
+	memory-region = <&cont_splash_mem>;
 	status = "okay";
 };
 
-- 
2.25.1

Re: [PATCH 1/2] dt-bindings: display/msm: qcom,sdm845-mdss: add memory-region property

[Dmitry Baryshkov]

On 12/07/2023 16:02, Amit Pundir wrote:
> Add and document the reserved memory region property
> in the qcom,sdm845-mdss schema.
> 
> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> ---
>   .../devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml    | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> index 6ecb00920d7f..3ea1dbd7e317 100644
> --- a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> @@ -39,6 +39,11 @@ properties:
>     interconnect-names:
>       maxItems: 2
>   
> +  memory-region:
> +    maxItems: 1
> +    description:
> +      Phandle to a node describing a reserved memory region.
> +

Please add it to mdss-common.yaml instead

>   patternProperties:
>     "^display-controller@[0-9a-f]+$":
>       type: object

-- 
With best wishes
Dmitry

Re: [PATCH 1/2] dt-bindings: display/msm: qcom,sdm845-mdss: add memory-region property

[Amit Pundir]

On Wed, 12 Jul 2023 at 18:45, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 12/07/2023 16:02, Amit Pundir wrote:
> > Add and document the reserved memory region property
> > in the qcom,sdm845-mdss schema.
> >
> > Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> > ---
> >   .../devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml    | 5 +++++
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> > index 6ecb00920d7f..3ea1dbd7e317 100644
> > --- a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> > @@ -39,6 +39,11 @@ properties:
> >     interconnect-names:
> >       maxItems: 2
> >
> > +  memory-region:
> > +    maxItems: 1
> > +    description:
> > +      Phandle to a node describing a reserved memory region.
> > +
>
> Please add it to mdss-common.yaml instead

mdss-common.yaml didn't like this property at all and
I ran into a lot of new dtbs_check warnings:
https://www.irccloud.com/pastebin/raw/pEYAeaB1

I need some help in decoding these please.

Regards,
Amit Pundir

>
> >   patternProperties:
> >     "^display-controller@[0-9a-f]+$":
> >       type: object
>
> --
> With best wishes
> Dmitry
>

Re: [PATCH 1/2] dt-bindings: display/msm: qcom,sdm845-mdss: add memory-region property

[Dmitry Baryshkov]

On Thu, 13 Jul 2023 at 18:34, Amit Pundir <amit.pundir@linaro.org> wrote:
>
> On Wed, 12 Jul 2023 at 18:45, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On 12/07/2023 16:02, Amit Pundir wrote:
> > > Add and document the reserved memory region property
> > > in the qcom,sdm845-mdss schema.
> > >
> > > Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> > > ---
> > >   .../devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml    | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> > > index 6ecb00920d7f..3ea1dbd7e317 100644
> > > --- a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> > > +++ b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> > > @@ -39,6 +39,11 @@ properties:
> > >     interconnect-names:
> > >       maxItems: 2
> > >
> > > +  memory-region:
> > > +    maxItems: 1
> > > +    description:
> > > +      Phandle to a node describing a reserved memory region.
> > > +
> >
> > Please add it to mdss-common.yaml instead
>
> mdss-common.yaml didn't like this property at all and
> I ran into a lot of new dtbs_check warnings:
> https://www.irccloud.com/pastebin/raw/pEYAeaB1
>
> I need some help in decoding these please.

I'm not sure what happened there (and it's hard to understand without
seeing your patch). But after applying your patch to mdss-common.yaml,
`make dt_binding_check' passes:

diff --git a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
index ccd7d6417523..924fe383e4a1 100644
--- a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
+++ b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
@@ -77,6 +77,11 @@ properties:
     items:
       - description: MDSS_CORE reset

+  memory-region:
+    maxItems: 1
+    description:
+      Phandle to a node describing a reserved memory region.
+
 required:
   - reg
   - reg-names


>
> Regards,
> Amit Pundir
>
> >
> > >   patternProperties:
> > >     "^display-controller@[0-9a-f]+$":
> > >       type: object
> >
> > --
> > With best wishes
> > Dmitry
> >



-- 
With best wishes
Dmitry

Re: [PATCH 1/2] dt-bindings: display/msm: qcom,sdm845-mdss: add memory-region property

[Amit Pundir]

On Thu, 13 Jul 2023 at 21:13, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Thu, 13 Jul 2023 at 18:34, Amit Pundir <amit.pundir@linaro.org> wrote:
> >
> > On Wed, 12 Jul 2023 at 18:45, Dmitry Baryshkov
> > <dmitry.baryshkov@linaro.org> wrote:
> > >
> > > On 12/07/2023 16:02, Amit Pundir wrote:
> > > > Add and document the reserved memory region property
> > > > in the qcom,sdm845-mdss schema.
> > > >
> > > > Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> > > > ---
> > > >   .../devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml    | 5 +++++
> > > >   1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> > > > index 6ecb00920d7f..3ea1dbd7e317 100644
> > > > --- a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> > > > @@ -39,6 +39,11 @@ properties:
> > > >     interconnect-names:
> > > >       maxItems: 2
> > > >
> > > > +  memory-region:
> > > > +    maxItems: 1
> > > > +    description:
> > > > +      Phandle to a node describing a reserved memory region.
> > > > +
> > >
> > > Please add it to mdss-common.yaml instead
> >
> > mdss-common.yaml didn't like this property at all and
> > I ran into a lot of new dtbs_check warnings:
> > https://www.irccloud.com/pastebin/raw/pEYAeaB1
> >
> > I need some help in decoding these please.
>
> I'm not sure what happened there (and it's hard to understand without
> seeing your patch).

Yup.. It was my broken patch. I used "For example:" in the property
description and it tripped off the checks. Didn't realise that
casually used ":" can break yaml parsing until now. Sorry for all the
noise.

Regards,
Amit Pundir

> But after applying your patch to mdss-common.yaml,
> `make dt_binding_check' passes:
>
> diff --git a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
> b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
> index ccd7d6417523..924fe383e4a1 100644
> --- a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml
> @@ -77,6 +77,11 @@ properties:
>      items:
>        - description: MDSS_CORE reset
>
> +  memory-region:
> +    maxItems: 1
> +    description:
> +      Phandle to a node describing a reserved memory region.
> +
>  required:
>    - reg
>    - reg-names
>
>
> >
> > Regards,
> > Amit Pundir
> >
> > >
> > > >   patternProperties:
> > > >     "^display-controller@[0-9a-f]+$":
> > > >       type: object
> > >
> > > --
> > > With best wishes
> > > Dmitry
> > >
>
>
>
> --
> With best wishes
> Dmitry

Re: [PATCH 1/2] dt-bindings: display/msm: qcom,sdm845-mdss: add memory-region property

[Krzysztof Kozlowski]

On 12/07/2023 15:02, Amit Pundir wrote:
> Add and document the reserved memory region property
> in the qcom,sdm845-mdss schema.
> 
> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>

Please keep consistent versioning, so this is new patch in v4.

> ---
>  .../devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml    | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> index 6ecb00920d7f..3ea1dbd7e317 100644
> --- a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> @@ -39,6 +39,11 @@ properties:
>    interconnect-names:
>      maxItems: 2
>  
> +  memory-region:
> +    maxItems: 1
> +    description:
> +      Phandle to a node describing a reserved memory region.

Your description says nothing new. It's entirely redundant/obvious.
Instead please describe what reserved memory is expected to be here.


Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: display/msm: qcom,sdm845-mdss: add memory-region property

[Amit Pundir]

On Wed, 12 Jul 2023 at 19:46, Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 12/07/2023 15:02, Amit Pundir wrote:
> > Add and document the reserved memory region property
> > in the qcom,sdm845-mdss schema.
> >
> > Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
>
> Please keep consistent versioning, so this is new patch in v4.

ACK.

>
> > ---
> >  .../devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml    | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> > index 6ecb00920d7f..3ea1dbd7e317 100644
> > --- a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml
> > @@ -39,6 +39,11 @@ properties:
> >    interconnect-names:
> >      maxItems: 2
> >
> > +  memory-region:
> > +    maxItems: 1
> > +    description:
> > +      Phandle to a node describing a reserved memory region.
>
> Your description says nothing new. It's entirely redundant/obvious.
> Instead please describe what reserved memory is expected to be here.

On it. I'll update in v5. Thanks.

Regards,
Amit Pundir

>
>
> Best regards,
> Krzysztof
>