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

[Marek Vasut]

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 at intel.com>
>>>>>>>
>>>>>>> Add default fitImage file bundling FPGA bitstreams for
>>>>>>> Arria10.
>>>>>>>
>>>>>>> Signed-off-by: Tien Fong Chee <tien.fong.chee at intel.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.intel.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 ?

>>>>>>>
>>>>>>> +		};
>>>>>>> +
>>>>>>> +		fpga-periph at 2 {
>>>>>>> +			description = "FPGA peripheral
>>>>>>> bitstream";
>>>>>>> +			data =
>>>>>>> /incbin/("../../../ghrd_10as066n2.periph.rbf");
>>>>>>> +			type = "fpga";
>>>>>>> +			arch = "arm";
>>>>>>> +			compression = "none";
>>>>>>> +		};
>>>>>>> +	};
>>>>>>> +
>>>>>>> +	configurations {
>>>>>>> +		default = "config-1";
>>>>>>> +		config-1 {
>>>>>>> +			description = "Boot with FPGA
>>>>>>> early IO
>>>>>>> release config";
>>>>>>> +			fpga = "fpga-periph at 2", "fpga-core
>>>>>>> @1";
>>>>>> Don't you need to load the core first ?
>>>>> No, the periphery is first.  This brings up the dram and i/o.
>>>> Then why do we have periph at 2 above ? Shouldn't those two images
>>>> be
>>>> swapped to make this look less confusing ?
>>> The ordering in configuration fpga property doesn't matter, the
>>> driver
>>> smart enough to determine what bitstream need to be programmed at
>>> what
>>> FPGA mode.
>> Good, then please order it naturally, @1 then @2 etc .
> Okay.
>>
>>>
>>> The image fpga-core at 1 is alligned at 1st just for avoiding 
>>> the performance penalty.
>> I thought we concluded this should be fixed elsewhere ?
> May be we can wait until there is a solution? So user can benefit the
> good performance with this default fitIMage?

This will only work for specific cases of specific storage devices and
filesystems .

-- 
Best regards,
Marek Vasut