arm64: dts: qcom: sdm845-db845c: Mark cont splash memory region as reserved

  Put cont splash memory region under the reserved-memory
as confirmed by the downstream code as well.

Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
---
 arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 8 ++++++++
 1 file changed, 8 insertions(+)
  

On 1/24/23 18:28, Amit Pundir wrote:
> Put cont splash memory region under the reserved-memory
> as confirmed by the downstream code as well.

Can we put the framebuffer region in sdm845.dtsi? afaik the only device 
with a non-standard address for this is Cheza, and the SDM850 devices. 
All other devices (even the sdm845 Sony ones) have it at the same 
address and size. The other reserved-memory regions are basically just 
whatever MTP/QRD uses anyway.

shift-axolotl currently reserves the wrong size (it only reserves the 
size needed for it's resolution), the OnePlus 6 is also missing the region.
> 
> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> index f41c6d600ea8..2ae59432cbda 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> @@ -100,6 +100,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";
  

On 24/01/2023 18:28, Amit Pundir wrote:
> Put cont splash memory region under the reserved-memory
> as confirmed by the downstream code as well.
> 
> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> ---
>   arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> index f41c6d600ea8..2ae59432cbda 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> @@ -100,6 +100,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";

Doesn't this mean we loose 0x2400000 of DRAM for all rb3 platforms 
though ? About what 37 megabytes.. ?

IMO it would be better to have a bootloader that cares about continuous 
splash to apply a dtb overlay or simply to add a separate dts 
sdm845-db845c-continuous-spash.dts.

---
bod
  

On Tue, 31 Jan 2023 at 12:54, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 24/01/2023 18:28, Amit Pundir wrote:
> > Put cont splash memory region under the reserved-memory
> > as confirmed by the downstream code as well.
> >
> > Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> > ---
> >   arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> > index f41c6d600ea8..2ae59432cbda 100644
> > --- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> > +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> > @@ -100,6 +100,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";
>
> Doesn't this mean we loose 0x2400000 of DRAM for all rb3 platforms
> though ? About what 37 megabytes.. ?

I think this memory is further used for display memory allocation. So
we are not loosing it, but dedicating it to the framebuffer memory.
  

On 31.01.2023 12:06, Dmitry Baryshkov wrote:
> On Tue, 31 Jan 2023 at 12:54, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>
>> On 24/01/2023 18:28, Amit Pundir wrote:
>>> Put cont splash memory region under the reserved-memory
>>> as confirmed by the downstream code as well.
>>>
>>> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
>>> ---
>>>   arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
>>> index f41c6d600ea8..2ae59432cbda 100644
>>> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
>>> @@ -100,6 +100,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";
>>
>> Doesn't this mean we loose 0x2400000 of DRAM for all rb3 platforms
>> though ? About what 37 megabytes.. ?
> 
> I think this memory is further used for display memory allocation. So
> we are not loosing it, but dedicating it to the framebuffer memory.
Not exactly, to do so, you'd have to use the memory-region property
with mdss, which nobody does. Otherwise it's just a hole for Linux.

Konrad
> 
>
  

On 31/01/2023 14:45, Konrad Dybcio wrote:
> 
> 
> On 31.01.2023 12:06, Dmitry Baryshkov wrote:
>> On Tue, 31 Jan 2023 at 12:54, Bryan O'Donoghue
>> <bryan.odonoghue@linaro.org> wrote:
>>>
>>> On 24/01/2023 18:28, Amit Pundir wrote:
>>>> Put cont splash memory region under the reserved-memory
>>>> as confirmed by the downstream code as well.
>>>>
>>>> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
>>>> ---
>>>>    arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 8 ++++++++
>>>>    1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
>>>> index f41c6d600ea8..2ae59432cbda 100644
>>>> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
>>>> @@ -100,6 +100,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";
>>>
>>> Doesn't this mean we loose 0x2400000 of DRAM for all rb3 platforms
>>> though ? About what 37 megabytes.. ?
>>
>> I think this memory is further used for display memory allocation. So
>> we are not loosing it, but dedicating it to the framebuffer memory.
> Not exactly, to do so, you'd have to use the memory-region property
> with mdss, which nobody does. Otherwise it's just a hole for Linux.

Then maybe it's time to start using that property?

> 
> Konrad
>>
>>
  

On Tue, 31 Jan 2023 at 19:03, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 31/01/2023 14:45, Konrad Dybcio wrote:
> >
> >
> > On 31.01.2023 12:06, Dmitry Baryshkov wrote:
> >> On Tue, 31 Jan 2023 at 12:54, Bryan O'Donoghue
> >> <bryan.odonoghue@linaro.org> wrote:
> >>>
> >>> On 24/01/2023 18:28, Amit Pundir wrote:
> >>>> Put cont splash memory region under the reserved-memory
> >>>> as confirmed by the downstream code as well.
> >>>>
> >>>> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> >>>> ---
> >>>>    arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 8 ++++++++
> >>>>    1 file changed, 8 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> >>>> index f41c6d600ea8..2ae59432cbda 100644
> >>>> --- a/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> >>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-db845c.dts
> >>>> @@ -100,6 +100,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";
> >>>
> >>> Doesn't this mean we loose 0x2400000 of DRAM for all rb3 platforms
> >>> though ? About what 37 megabytes.. ?
> >>
> >> I think this memory is further used for display memory allocation. So
> >> we are not loosing it, but dedicating it to the framebuffer memory.
> > Not exactly, to do so, you'd have to use the memory-region property
> > with mdss, which nobody does. Otherwise it's just a hole for Linux.
>
> Then maybe it's time to start using that property?

Hi, So what is the verdict on this patch?

I submitted this fix to make sure UFS don't map and crash on it, which
I have seen happening occassionaly on db845c and Caleb reported
similar issues on his sdm845 device iirc. I should have probably put
that in my commit message as well.

Regards,
Amit Pundir

>
> >
> > Konrad
> >>
> >>
>
> --
> With best wishes
> Dmitry
>
  

On 09/02/2023 09:05, Amit Pundir wrote:
> Hi, So what is the verdict on this patch?
> 
> I submitted this fix to make sure UFS don't map and crash on it, which
> I have seen happening occassionaly on db845c and Caleb reported
> similar issues on his sdm845 device iirc. I should have probably put
> that in my commit message as well.
> 
> Regards,
> Amit Pundir

So the memory _is_ being used by ... continuous splash on an Android 
image, i.e. your Android ? limited to Android - image continues on with 
the splash but other blocks erroneously reuse the memory then, UFS as an 
example ?

---
bod
  

On 9.02.2023 12:03, Bryan O'Donoghue wrote:
> On 09/02/2023 09:05, Amit Pundir wrote:
>> Hi, So what is the verdict on this patch?
>>
>> I submitted this fix to make sure UFS don't map and crash on it, which
>> I have seen happening occassionaly on db845c and Caleb reported
>> similar issues on his sdm845 device iirc. I should have probably put
>> that in my commit message as well.
>>
>> Regards,
>> Amit Pundir
> 
> So the memory _is_ being used by ... continuous splash on an Android image, i.e. your Android ? limited to Android - image continues on with the splash but other blocks erroneously reuse the memory then, UFS as an example ?
If the bootloader splash is enabled then this memory is used until the
DPU driver instructs MDP5 pipes to suck data from a newly assigned address,
so there's a short window where it is.

Konrad
> 
> ---
> bod
  

On 09/02/2023 12:08, Konrad Dybcio wrote:
> If the bootloader splash is enabled then this memory is used until the
> DPU driver instructs MDP5 pipes to suck data from a newly assigned address,
> so there's a short window where it is.

It seems a shame to reserve 30 something megabytes of memory for 
continuous splash unless we are actually using it is my point.

If I'm running headless its just wasted memory.

---
bod
  

On 09/02/2023 12:11, Bryan O'Donoghue wrote:
>> If the bootloader splash is enabled then this memory is used until the
>> DPU driver instructs MDP5 pipes to suck data from a newly assigned 
>> address,
>> so there's a short window where it is.
> 
> It seems a shame to reserve 30 something megabytes of memory for 
> continuous splash unless we are actually using it is my point.
> 
> If I'm running headless its just wasted memory.

Couldn't we

1. Find reserved continuous splash memory
2. Fee it in the MDP when we make the transition

It must be possible

---
bod
  

On 9.02.2023 13:22, Bryan O'Donoghue wrote:
> On 09/02/2023 12:11, Bryan O'Donoghue wrote:
>>> If the bootloader splash is enabled then this memory is used until the
>>> DPU driver instructs MDP5 pipes to suck data from a newly assigned address,
>>> so there's a short window where it is.
>>
>> It seems a shame to reserve 30 something megabytes of memory for continuous splash unless we are actually using it is my point.
>>
>> If I'm running headless its just wasted memory.
> 
> Couldn't we
> 
> 1. Find reserved continuous splash memory
> 2. Fee it in the MDP when we make the transition
> 
> It must be possible
I suppose we could mark it as shared-dma-pool, pass it to
MDSS, reserve it from there (by occupying the whole thing)
and either use it or free it before jumping to the newly
allocated region.

The MDSS driver can already accept it through memory-region = <>
IIRC, but *nobody* uses that, so I'm not sure it even still works..

Konrad
> 
> ---
> bod
  

On 09/02/2023 14:22, Bryan O'Donoghue wrote:
> On 09/02/2023 12:11, Bryan O'Donoghue wrote:
>>> If the bootloader splash is enabled then this memory is used until the
>>> DPU driver instructs MDP5 pipes to suck data from a newly assigned 
>>> address,
>>> so there's a short window where it is.
>>
>> It seems a shame to reserve 30 something megabytes of memory for 
>> continuous splash unless we are actually using it is my point.
>>
>> If I'm running headless its just wasted memory.
> 
> Couldn't we
> 
> 1. Find reserved continuous splash memory
> 2. Fee it in the MDP when we make the transition

Qualcomm has investigated freeing the MDP/DPU cont_splash memory, but I 
fear that the end result was that it is not _that_ easy to free it. It 
is marked as reserved/no-map, so the kernel doesn't think about it as a 
memory region. Adding it back required hacking around that.
  

On Thu, 9 Feb 2023 at 19:24, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On 09/02/2023 14:22, Bryan O'Donoghue wrote:
> > On 09/02/2023 12:11, Bryan O'Donoghue wrote:
> >>> If the bootloader splash is enabled then this memory is used until the
> >>> DPU driver instructs MDP5 pipes to suck data from a newly assigned
> >>> address,
> >>> so there's a short window where it is.
> >>
> >> It seems a shame to reserve 30 something megabytes of memory for
> >> continuous splash unless we are actually using it is my point.
> >>
> >> If I'm running headless its just wasted memory.
> >
> > Couldn't we
> >
> > 1. Find reserved continuous splash memory
> > 2. Fee it in the MDP when we make the transition
>
> Qualcomm has investigated freeing the MDP/DPU cont_splash memory, but I
> fear that the end result was that it is not _that_ easy to free it. It
> is marked as reserved/no-map, so the kernel doesn't think about it as a
> memory region. Adding it back required hacking around that.

Hi Team,

This patch missed the v6.3 cut and I'm really hoping it to make it to
v6.4. Is there anything I can do it move it forward. Sorry if I missed
any action item assigned to me in the followup emails.

Regards,
Amit Pundir

>
> --
> With best wishes
> Dmitry
>
  

On Thu, 9 Feb 2023 at 16:33, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> On 09/02/2023 09:05, Amit Pundir wrote:
> > Hi, So what is the verdict on this patch?
> >
> > I submitted this fix to make sure UFS don't map and crash on it, which
> > I have seen happening occassionaly on db845c and Caleb reported
> > similar issues on his sdm845 device iirc. I should have probably put
> > that in my commit message as well.
> >
> > Regards,
> > Amit Pundir
>
> So the memory _is_ being used by ... continuous splash on an Android
> image, i.e. your Android ? limited to Android - image continues on with
> the splash but other blocks erroneously reuse the memory then, UFS as an
> example ?

Hi Bryan,

Yes UFS (reported only on v5.10) tries to map this reserved memory and
system crash and reboot. Plan is to backport this patch to v5.10.y.

Regards,
Amit Pundir

>
> ---
> bod
  

On 10/04/2023 09:54, Amit Pundir wrote:
> On Thu, 9 Feb 2023 at 16:33, Bryan O'Donoghue
> <bryan.odonoghue@linaro.org> wrote:
>>
>> On 09/02/2023 09:05, Amit Pundir wrote:
>>> Hi, So what is the verdict on this patch?
>>>
>>> I submitted this fix to make sure UFS don't map and crash on it, which
>>> I have seen happening occassionaly on db845c and Caleb reported
>>> similar issues on his sdm845 device iirc. I should have probably put
>>> that in my commit message as well.
>>>
>>> Regards,
>>> Amit Pundir
>>
>> So the memory _is_ being used by ... continuous splash on an Android
>> image, i.e. your Android ? limited to Android - image continues on with
>> the splash but other blocks erroneously reuse the memory then, UFS as an
>> example ?
> 
> Hi Bryan,
> 
> Yes UFS (reported only on v5.10) tries to map this reserved memory and
> system crash and reboot. Plan is to backport this patch to v5.10.y.
> 
> Regards,
> Amit Pundir
> 
>>
>> ---
>> bod

Personally I'm fine with this patch on the proviso we somehow associate 
it the memory MDP - even if its just a comment in the dts with the MDP.

i.e. if we run headless we want to be able to use that RAM for something.

A dts comment would do

---
bod