All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: dts: aspeed: Update reserved memory nodes for zaius and witherspoon
@ 2017-02-16  5:49 Cyril Bur
  2017-02-16  6:14 ` Mine
  2017-02-16 14:05 ` Patrick Williams
  0 siblings, 2 replies; 9+ messages in thread
From: Cyril Bur @ 2017-02-16  5:49 UTC (permalink / raw)
  To: joel, openbmc

Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
---
 arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 9 +++++++--
 arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts       | 9 +++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
index 4b2d8aa8abb6..2d76bfee1262 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
@@ -25,9 +25,14 @@
 		#size-cells = <1>;
 		ranges;
 
-		flash_memory: region@94000000 {
+		flash_memory: region@98000000 {
 			no-map;
-			reg = <0x94000000 0x04000000>; /* 64M */
+			reg = <0x98000000 0x04000000>; /* 64M */
+		};
+
+		vga_memory: framebuffer@9c000000 {
+			no-map;
+			reg = <0x9c000000 0x04000000>; /* 64MB */
 		};
 	};
 
diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
index e1c9b3f4fe44..f30288f735bc 100644
--- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
+++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
@@ -25,9 +25,14 @@
 		#size-cells = <1>;
 		ranges;
 
-		flash_memory: region@94000000 {
+		flash_memory: region@98000000 {
 			no-map;
-			reg = <0x94000000 0x04000000>; /* 64M */
+			reg = <0x98000000 0x04000000>; /* 64M */
+		};
+
+		vga_memory: framebuffer@9c000000 {
+			no-map;
+			reg = <0x9c000000 0x04000000>; /* 64MB */
 		};
 	};
 
-- 
2.11.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: aspeed: Update reserved memory nodes for zaius and witherspoon
  2017-02-16  5:49 [PATCH] ARM: dts: aspeed: Update reserved memory nodes for zaius and witherspoon Cyril Bur
@ 2017-02-16  6:14 ` Mine
  2017-02-17  2:51   ` Cyril Bur
  2017-02-16 14:05 ` Patrick Williams
  1 sibling, 1 reply; 9+ messages in thread
From: Mine @ 2017-02-16  6:14 UTC (permalink / raw)
  To: Cyril Bur; +Cc: Joel Stanley, OpenBMC Maillist

Hi Cyril,

On Thu, Feb 16, 2017 at 1:49 PM, Cyril Bur <cyrilbur@gmail.com> wrote:
> Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
> ---
>  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 9 +++++++--
>  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts       | 9 +++++++--
>  2 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> index 4b2d8aa8abb6..2d76bfee1262 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> @@ -25,9 +25,14 @@
>                 #size-cells = <1>;
>                 ranges;
>
> -               flash_memory: region@94000000 {
> +               flash_memory: region@98000000 {
>                         no-map;
> -                       reg = <0x94000000 0x04000000>; /* 64M */
> +                       reg = <0x98000000 0x04000000>; /* 64M */
> +               };
> +
> +               vga_memory: framebuffer@9c000000 {
> +                       no-map;
> +                       reg = <0x9c000000 0x04000000>; /* 64MB */

This assumes that Witherspoon has 512MiB RAM, then why the memory part
indicates it has 1GiB?
```
        memory {
                reg = <0x80000000 0x40000000>;
        };
```

>                 };
>         };
>
> diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> index e1c9b3f4fe44..f30288f735bc 100644
> --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> @@ -25,9 +25,14 @@
>                 #size-cells = <1>;
>                 ranges;
>
> -               flash_memory: region@94000000 {
> +               flash_memory: region@98000000 {
>                         no-map;
> -                       reg = <0x94000000 0x04000000>; /* 64M */
> +                       reg = <0x98000000 0x04000000>; /* 64M */
> +               };
> +
> +               vga_memory: framebuffer@9c000000 {
> +                       no-map;
> +                       reg = <0x9c000000 0x04000000>; /* 64MB */

As I know Google decides to not support VGA on Zaius, so maybe
vga_memory is not needed to Zaius at all.

>                 };
>         };
>
> --
> 2.11.1
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: aspeed: Update reserved memory nodes for zaius and witherspoon
  2017-02-16  5:49 [PATCH] ARM: dts: aspeed: Update reserved memory nodes for zaius and witherspoon Cyril Bur
  2017-02-16  6:14 ` Mine
@ 2017-02-16 14:05 ` Patrick Williams
  2017-02-17  2:48   ` Cyril Bur
  1 sibling, 1 reply; 9+ messages in thread
From: Patrick Williams @ 2017-02-16 14:05 UTC (permalink / raw)
  To: Cyril Bur; +Cc: joel, openbmc

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

On Thu, Feb 16, 2017 at 04:49:22PM +1100, Cyril Bur wrote:
> -		flash_memory: region@94000000 {
> +		flash_memory: region@98000000 {
>  			no-map;
> -			reg = <0x94000000 0x04000000>; /* 64M */
> +			reg = <0x98000000 0x04000000>; /* 64M */
> +		};
> +
> +		vga_memory: framebuffer@9c000000 {
> +			no-map;
> +			reg = <0x9c000000 0x04000000>; /* 64MB */

Do we really have to allocate 64MB to a VGA framebuffer?  We can store a
4K resolution monitor with 32bit color in 32MB, so why is it required to
reserve this much memory?  Between this and the PNOR memory region, now
1/4th of the BMCs memory is reserved.

-- 
Patrick Williams

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: aspeed: Update reserved memory nodes for zaius and witherspoon
  2017-02-16 14:05 ` Patrick Williams
@ 2017-02-17  2:48   ` Cyril Bur
  2017-02-21  4:16     ` Patrick Williams
  0 siblings, 1 reply; 9+ messages in thread
From: Cyril Bur @ 2017-02-17  2:48 UTC (permalink / raw)
  To: Patrick Williams; +Cc: joel, openbmc

On Thu, 2017-02-16 at 08:05 -0600, Patrick Williams wrote:
> On Thu, Feb 16, 2017 at 04:49:22PM +1100, Cyril Bur wrote:
> > -		flash_memory: region@94000000 {
> > +		flash_memory: region@98000000 {
> >  			no-map;
> > -			reg = <0x94000000 0x04000000>; /* 64M */
> > +			reg = <0x98000000 0x04000000>; /* 64M */
> > +		};
> > +
> > +		vga_memory: framebuffer@9c000000 {
> > +			no-map;
> > +			reg = <0x9c000000 0x04000000>; /* 64MB */
> 
> Do we really have to allocate 64MB to a VGA framebuffer?  We can store a
> 4K resolution monitor with 32bit color in 32MB, so why is it required to
> reserve this much memory?  Between this and the PNOR memory region, now
> 1/4th of the BMCs memory is reserved.

The reservation of 64MB is because the hardware is capable of using
64MB, therefore the host can use that region of memory. We're playing
it safe here by ensuring the BMC doesn't use any of it to avoid any
possibility of the host corrupting BMC memory. This is the most generic
dts for the platform so we've gone with stability over performance.


Cyril

> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: aspeed: Update reserved memory nodes for zaius and witherspoon
  2017-02-16  6:14 ` Mine
@ 2017-02-17  2:51   ` Cyril Bur
  2017-02-17  2:56     ` Cyril Bur
  0 siblings, 1 reply; 9+ messages in thread
From: Cyril Bur @ 2017-02-17  2:51 UTC (permalink / raw)
  To: Mine; +Cc: Joel Stanley, OpenBMC Maillist

On Thu, 2017-02-16 at 14:14 +0800, Mine wrote:
> Hi Cyril,
> 
> On Thu, Feb 16, 2017 at 1:49 PM, Cyril Bur <cyrilbur@gmail.com> wrote:
> > Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
> > ---
> >  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 9 +++++++--
> >  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts       | 9 +++++++--
> >  2 files changed, 14 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> > index 4b2d8aa8abb6..2d76bfee1262 100644
> > --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> > +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> > @@ -25,9 +25,14 @@
> >                 #size-cells = <1>;
> >                 ranges;
> > 
> > -               flash_memory: region@94000000 {
> > +               flash_memory: region@98000000 {
> >                         no-map;
> > -                       reg = <0x94000000 0x04000000>; /* 64M */
> > +                       reg = <0x98000000 0x04000000>; /* 64M */
> > +               };
> > +
> > +               vga_memory: framebuffer@9c000000 {
> > +                       no-map;
> > +                       reg = <0x9c000000 0x04000000>; /* 64MB */
> 
> This assumes that Witherspoon has 512MiB RAM, then why the memory part
> indicates it has 1GiB?
> ```
>         memory {
>                 reg = <0x80000000 0x40000000>;
>         };
> ```

Oops, you're correct. I'll adjust the numbers.

> 
> >                 };
> >         };
> > 
> > diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> > index e1c9b3f4fe44..f30288f735bc 100644
> > --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> > +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> > @@ -25,9 +25,14 @@
> >                 #size-cells = <1>;
> >                 ranges;
> > 
> > -               flash_memory: region@94000000 {
> > +               flash_memory: region@98000000 {
> >                         no-map;
> > -                       reg = <0x94000000 0x04000000>; /* 64M */
> > +                       reg = <0x98000000 0x04000000>; /* 64M */
> > +               };
> > +
> > +               vga_memory: framebuffer@9c000000 {
> > +                       no-map;
> > +                       reg = <0x9c000000 0x04000000>; /* 64MB */
> 
> As I know Google decides to not support VGA on Zaius, so maybe
> vga_memory is not needed to Zaius at all.
> 

Happy to drop that if there won't be any VGA on the platform.

Thanks,

Cyril

> >                 };
> >         };
> > 
> > --
> > 2.11.1
> > 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: aspeed: Update reserved memory nodes for zaius and witherspoon
  2017-02-17  2:51   ` Cyril Bur
@ 2017-02-17  2:56     ` Cyril Bur
  0 siblings, 0 replies; 9+ messages in thread
From: Cyril Bur @ 2017-02-17  2:56 UTC (permalink / raw)
  To: Mine; +Cc: Joel Stanley, OpenBMC Maillist

On Fri, 2017-02-17 at 13:51 +1100, Cyril Bur wrote:
> On Thu, 2017-02-16 at 14:14 +0800, Mine wrote:
> > Hi Cyril,
> > 
> > On Thu, Feb 16, 2017 at 1:49 PM, Cyril Bur <cyrilbur@gmail.com> wrote:
> > > Signed-off-by: Cyril Bur <cyrilbur@gmail.com>
> > > ---
> > >  arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts | 9 +++++++--
> > >  arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts       | 9 +++++++--
> > >  2 files changed, 14 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> > > index 4b2d8aa8abb6..2d76bfee1262 100644
> > > --- a/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> > > +++ b/arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts
> > > @@ -25,9 +25,14 @@
> > >                 #size-cells = <1>;
> > >                 ranges;
> > > 
> > > -               flash_memory: region@94000000 {
> > > +               flash_memory: region@98000000 {
> > >                         no-map;
> > > -                       reg = <0x94000000 0x04000000>; /* 64M */
> > > +                       reg = <0x98000000 0x04000000>; /* 64M */
> > > +               };
> > > +
> > > +               vga_memory: framebuffer@9c000000 {
> > > +                       no-map;
> > > +                       reg = <0x9c000000 0x04000000>; /* 64MB */
> > 
> > This assumes that Witherspoon has 512MiB RAM, then why the memory part
> > indicates it has 1GiB?
> > ```
> >         memory {
> >                 reg = <0x80000000 0x40000000>;
> >         };
> > ```
> 
> Oops, you're correct. I'll adjust the numbers.
> 
> > 
> > >                 };
> > >         };
> > > 
> > > diff --git a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> > > index e1c9b3f4fe44..f30288f735bc 100644
> > > --- a/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> > > +++ b/arch/arm/boot/dts/aspeed-bmc-opp-zaius.dts
> > > @@ -25,9 +25,14 @@
> > >                 #size-cells = <1>;
> > >                 ranges;
> > > 
> > > -               flash_memory: region@94000000 {
> > > +               flash_memory: region@98000000 {
> > >                         no-map;
> > > -                       reg = <0x94000000 0x04000000>; /* 64M */
> > > +                       reg = <0x98000000 0x04000000>; /* 64M */
> > > +               };
> > > +
> > > +               vga_memory: framebuffer@9c000000 {
> > > +                       no-map;
> > > +                       reg = <0x9c000000 0x04000000>; /* 64MB */
> > 
> > As I know Google decides to not support VGA on Zaius, so maybe
> > vga_memory is not needed to Zaius at all.
> > 
> 
> Happy to drop that if there won't be any VGA on the platform.

Sorry for the followup, I assume it would then make sense to put the
flash_memory at the top of RAM?

> 
> Thanks,
> 
> Cyril
> 
> > >                 };
> > >         };
> > > 
> > > --
> > > 2.11.1
> > > 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: aspeed: Update reserved memory nodes for zaius and witherspoon
  2017-02-17  2:48   ` Cyril Bur
@ 2017-02-21  4:16     ` Patrick Williams
  2017-02-21  4:45       ` Joel Stanley
  0 siblings, 1 reply; 9+ messages in thread
From: Patrick Williams @ 2017-02-21  4:16 UTC (permalink / raw)
  To: Cyril Bur; +Cc: joel, openbmc

[-- Attachment #1: Type: text/plain, Size: 1435 bytes --]

On Fri, Feb 17, 2017 at 01:48:41PM +1100, Cyril Bur wrote:
> On Thu, 2017-02-16 at 08:05 -0600, Patrick Williams wrote:
> > On Thu, Feb 16, 2017 at 04:49:22PM +1100, Cyril Bur wrote:
> > > -		flash_memory: region@94000000 {
> > > +		flash_memory: region@98000000 {
> > >  			no-map;
> > > -			reg = <0x94000000 0x04000000>; /* 64M */
> > > +			reg = <0x98000000 0x04000000>; /* 64M */
> > > +		};
> > > +
> > > +		vga_memory: framebuffer@9c000000 {
> > > +			no-map;
> > > +			reg = <0x9c000000 0x04000000>; /* 64MB */
> > 
> > Do we really have to allocate 64MB to a VGA framebuffer?  We can store a
> > 4K resolution monitor with 32bit color in 32MB, so why is it required to
> > reserve this much memory?  Between this and the PNOR memory region, now
> > 1/4th of the BMCs memory is reserved.
> 
> The reservation of 64MB is because the hardware is capable of using
> 64MB, therefore the host can use that region of memory. We're playing
> it safe here by ensuring the BMC doesn't use any of it to avoid any
> possibility of the host corrupting BMC memory. This is the most generic
> dts for the platform so we've gone with stability over performance.

Is there anything the BMC can do to restrict how big of a memory map the
host side of the VGA can open up?  If there isn't we're going to have to
allocate this much on all systems because we can't trust the host to
"behave".

-- 
Patrick Williams

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: aspeed: Update reserved memory nodes for zaius and witherspoon
  2017-02-21  4:16     ` Patrick Williams
@ 2017-02-21  4:45       ` Joel Stanley
  2017-02-21  5:13         ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 9+ messages in thread
From: Joel Stanley @ 2017-02-21  4:45 UTC (permalink / raw)
  To: Patrick Williams, Benjamin Herrenschmidt, Jeremy Kerr
  Cc: Cyril Bur, OpenBMC Maillist

On Tue, Feb 21, 2017 at 2:46 PM, Patrick Williams <patrick@stwcx.xyz> wrote:
> On Fri, Feb 17, 2017 at 01:48:41PM +1100, Cyril Bur wrote:
>> On Thu, 2017-02-16 at 08:05 -0600, Patrick Williams wrote:
>> > On Thu, Feb 16, 2017 at 04:49:22PM +1100, Cyril Bur wrote:
>> > > -         flash_memory: region@94000000 {
>> > > +         flash_memory: region@98000000 {
>> > >                   no-map;
>> > > -                 reg = <0x94000000 0x04000000>; /* 64M */
>> > > +                 reg = <0x98000000 0x04000000>; /* 64M */
>> > > +         };
>> > > +
>> > > +         vga_memory: framebuffer@9c000000 {
>> > > +                 no-map;
>> > > +                 reg = <0x9c000000 0x04000000>; /* 64MB */
>> >
>> > Do we really have to allocate 64MB to a VGA framebuffer?  We can store a
>> > 4K resolution monitor with 32bit color in 32MB, so why is it required to
>> > reserve this much memory?  Between this and the PNOR memory region, now
>> > 1/4th of the BMCs memory is reserved.
>>
>> The reservation of 64MB is because the hardware is capable of using
>> 64MB, therefore the host can use that region of memory. We're playing
>> it safe here by ensuring the BMC doesn't use any of it to avoid any
>> possibility of the host corrupting BMC memory. This is the most generic
>> dts for the platform so we've gone with stability over performance.
>
> Is there anything the BMC can do to restrict how big of a memory map the
> host side of the VGA can open up?  If there isn't we're going to have to
> allocate this much on all systems because we can't trust the host to
> "behave".

Ben and Jeremy have been enhancing the host driver to be smarter. Ben,
what's the state of play?

Cheers,

Joel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] ARM: dts: aspeed: Update reserved memory nodes for zaius and witherspoon
  2017-02-21  4:45       ` Joel Stanley
@ 2017-02-21  5:13         ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 9+ messages in thread
From: Benjamin Herrenschmidt @ 2017-02-21  5:13 UTC (permalink / raw)
  To: Joel Stanley, Patrick Williams, Jeremy Kerr; +Cc: Cyril Bur, OpenBMC Maillist

On Tue, 2017-02-21 at 15:15 +1030, Joel Stanley wrote:
> > > The reservation of 64MB is because the hardware is capable of using
> > > 64MB, therefore the host can use that region of memory. We're playing
> > > it safe here by ensuring the BMC doesn't use any of it to avoid any
> > > possibility of the host corrupting BMC memory. This is the most generic
> > > dts for the platform so we've gone with stability over performance.
> > 
> > Is there anything the BMC can do to restrict how big of a memory map the
> > host side of the VGA can open up?  If there isn't we're going to have to
> > allocate this much on all systems because we can't trust the host to
> > "behave".
> 
> Ben and Jeremy have been enhancing the host driver to be smarter. Ben,
> what's the state of play?

That's actually an interesting one ... there is a different mem region
for host VGA and BMC graphics ...

The size it can use as FB (and exposes through PCI) can be selected via
a strapping of the chip.

Ideally, when we have a DT aware uBoot, we could have it adjust that
property based on the value of the strap. I think we can also change it
though, thus we could probably make the SCU code in Linux adjust the
value based on the DT and thus have platform specific DTs reducing the
amount of VRAM exposed to the host.

Cheers,
Ben.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-02-21  5:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-16  5:49 [PATCH] ARM: dts: aspeed: Update reserved memory nodes for zaius and witherspoon Cyril Bur
2017-02-16  6:14 ` Mine
2017-02-17  2:51   ` Cyril Bur
2017-02-17  2:56     ` Cyril Bur
2017-02-16 14:05 ` Patrick Williams
2017-02-17  2:48   ` Cyril Bur
2017-02-21  4:16     ` Patrick Williams
2017-02-21  4:45       ` Joel Stanley
2017-02-21  5:13         ` Benjamin Herrenschmidt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.