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

[Marek Vasut]

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 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
>>>> ?
>>> 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 ?

-- 
Best regards,
Marek Vasut