[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 ?
In previosly, we hide this from user, the dest location is defined in
the code. But now, we expose this to user, so user can change that
through the load property.

If this load property is not defined, then OCRAM buffer would be used
instead.
>