| Message ID | 20230124182857.1524912-1-amit.pundir@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | arm64: dts: qcom: sdm845-db845c: Mark cont splash memory region as reserved | expand |
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
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";
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(+)