[U-Boot] [PATCH v8 2/8] ARM: socfpga: Add default FPGA bitstream fitImage for Arria10 SoCDK

[Chee, Tien Fong <tien.fong.chee@intel.com>]

On Thu, 2019-02-14 at 17:26 +0100, Marek Vasut wrote:
> On 2/14/19 4:47 PM, Chee, Tien Fong wrote:
> > 
> > On Thu, 2019-02-14 at 16:13 +0100, Marek Vasut wrote:
> > > 
> > > On 2/14/19 4:11 PM, Chee, Tien Fong wrote:
> > > > 
> > > > 
> > > > On Thu, 2019-02-14 at 13:24 +0100, Marek Vasut wrote:
> > > > > 
> > > > > 
> > > > > On 2/14/19 12:23 PM, Chee, Tien Fong wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Thu, 2019-02-14 at 11:35 +0100, Marek Vasut wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > On 2/14/19 7:04 AM, Chee, Tien Fong wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On Thu, 2019-02-14 at 00:04 +0100, Marek Vasut wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On 2/13/19 11:45 PM, Dalon L Westergreen wrote:
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > On Wed, 2019-02-13 at 17:10 +0100, Marek Vasut
> > > > > > > > > > wrote:
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > On 2/13/19 3:18 PM, tien.fong.chee at intel.com
> > > > > > > > > > > wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > > > > > > > > > 
> > > > > > > > > > > > Add default fitImage file bundling FPGA
> > > > > > > > > > > > bitstreams
> > > > > > > > > > > > for
> > > > > > > > > > > > Arria10.
> > > > > > > > > > > > 
> > > > > > > > > > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@i
> > > > > > > > > > > > ntel
> > > > > > > > > > > > .com
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > ---
> > > > > > > > > > > > 
> > > > > > > > > > > > changes for v8
> > > > > > > > > > > > - Changed the FPGA node name to fpga-core and
> > > > > > > > > > > > fpga-
> > > > > > > > > > > > periph
> > > > > > > > > > > > for
> > > > > > > > > > > > both core and
> > > > > > > > > > > >   periph bitstreams respectively.
> > > > > > > > > > > > ---
> > > > > > > > > > > >  board/altera/arria10-socdk/fit_spl_fpga.its |
> > > > > > > > > > > > 39
> > > > > > > > > > > > +++++++++++++++++++++++++++++
> > > > > > > > > > > >  1 file changed, 39 insertions(+)
> > > > > > > > > > > >  create mode 100644 board/altera/arria10-
> > > > > > > > > > > > socdk/fit_spl_fpga.its
> > > > > > > > > > > > 
> > > > > > > > > > > > diff --git a/board/altera/arria10-
> > > > > > > > > > > > socdk/fit_spl_fpga.its
> > > > > > > > > > > > b/board/altera/arria10-socdk/fit_spl_fpga.its
> > > > > > > > > > > > new file mode 100644
> > > > > > > > > > > > index 0000000..8ce175b
> > > > > > > > > > > > --- /dev/null
> > > > > > > > > > > > +++ b/board/altera/arria10-
> > > > > > > > > > > > socdk/fit_spl_fpga.its
> > > > > > > > > > > > @@ -0,0 +1,39 @@
> > > > > > > > > > > > +// SPDX-License-Identifier: GPL-2.0
> > > > > > > > > > > > + /*
> > > > > > > > > > > > + * Copyright (C) 2019 Intel Corporation <www.i
> > > > > > > > > > > > ntel
> > > > > > > > > > > > .com
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > + *
> > > > > > > > > > > > + */
> > > > > > > > > > > > +
> > > > > > > > > > > > +/dts-v1/;
> > > > > > > > > > > > +
> > > > > > > > > > > > +/ {
> > > > > > > > > > > > +	description = "FIT image with FPGA
> > > > > > > > > > > > bistream";
> > > > > > > > > > > > +	#address-cells = <1>;
> > > > > > > > > > > > +
> > > > > > > > > > > > +	images {
> > > > > > > > > > > > +		fpga-core at 1 {
> > > > > > > > > > > > +			description = "FPGA
> > > > > > > > > > > > core
> > > > > > > > > > > > bitstream";
> > > > > > > > > > > > +			data =
> > > > > > > > > > > > /incbin/("../../../ghrd_10as066n2.core.rbf");
> > > > > > > > > > > > +			type = "fpga";
> > > > > > > > > > > > +			arch = "arm";
> > > > > > > > > > > > +			compression = "none";
> > > > > > > > > > > > +			load = <0x400>;
> > > > > > > > > > > Is the load address required ?
> > > > > > > > It is optional for telling destination address of DDR
> > > > > > > > where
> > > > > > > > this
> > > > > > > > core.rbf going to be loaded. If load property, the
> > > > > > > > default
> > > > > > > > OCRAM
> > > > > > > > buffer
> > > > > > > > would be used, bad for performance when loading chunk
> > > > > > > > by
> > > > > > > > chunk.
> > > > > > > So if I have something at 0x400 in DRAM and use this
> > > > > > > example
> > > > > > > in
> > > > > > > U-
> > > > > > > Boot,
> > > > > > > it will be overwritten ?
> > > > > > This is used for SPL only, at least for now, so that is
> > > > > > before
> > > > > > loading
> > > > > > the U-Boot, DDR is blank.
> > > > > > But, you can define the blank location.
> > > > > > If load property is not defined, the driver would use small
> > > > > > buffer
> > > > > > from
> > > > > > OCRAM.
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > How is OCRAM bad for performance ?
> > > > > > With DDR, the performance can up to 85-90%.
> > > > > > It is because very limited space in OCRAM, so very small
> > > > > > buffer
> > > > > > is
> > > > > > used
> > > > > > for loading bitstream, so the bitstream has to be loaded
> > > > > > chunk
> > > > > > by
> > > > > > chunk.
> > > > > > For DDR, where whole bitstream can be loaded.
> > > > > Shouldn't the logic to determine where the buffer is be in
> > > > > the
> > > > > code ?
> > > > > I mean, once you have DRAM available, you have all the malloc
> > > > > space,
> > > > > so
> > > > > you can use larger buffer.Why encode that knowledge into the
> > > > > fitImage
> > > > > ?
> > > > In previosly, we hard code the dest location for loading the
> > > > core
> > > > bitstream in SPL, because by that time DDR up running, but
> > > > malloc
> > > > is
> > > > still pointed to OCRAM.
> > > > Now, we let user to define the dest location through the load
> > > > property.
> > > But the user shouldn't care where the buffer is, the buffer
> > > should be
> > > in
> > > the best possible location and the SPL can determine that (if
> > > DRAM is
> > > running, in DRAM, otherwise in OCRAM), no ?
> > Yes, most of the time user don't need to change the dest value at
> > load
> > property, unless they have very good reason to do so.
> > SPL would smart enough to read the load property after the DDR is
> > up
> > running, and loading the bitstream to dest location, and
> > programming
> > FPGA from there.
> Then we don't need the 'load' property by default ?
I will add a default ddr load address in the code, and enable the load
property to overwrite it if it is defined.

So, that means the load property is optional.
>