| Message ID | 1452602781-22424-8-git-send-email-lee.jones@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, 17 Mar 2016, Loic PALLARDY wrote: > Hi Lee, Pete, > > The coprocessor memory map defined below is for test. Addresses have > been arbitrary fixed. > The audio and video firmware you want to use are for product > configuration. > For sure memory mapping must be adapted or firmware recompiled. > > About coherency between firmware characteristics (linked address, > position independent or not, size) and DT definition, you're right, > some checks are missing in this version. It shouldn't be possible to > load/start a firmware if DT reserved memory is not aligned with > firmware resource table and firmware start address. > > Lee, I propose to have a short discussion during next ST LT weekly > meeting to list missing features and identify ST internal remoteproc > patches for upstream. Sounds good to me. FYI: This is only the first patch-set submission, which only provides basic support. There are subsequent sets, which I will start to refine after the merge-window closes. Perhaps the internal patches you speak of are already part of this set? > > -----Original Message----- > > From: Peter Griffin [mailto:peter.griffin@linaro.org] > > Sent: Wednesday, March 16, 2016 9:11 PM > > To: Lee Jones <lee.jones@linaro.org> > > Cc: ohad@wizery.com; devicetree@vger.kernel.org; f.fainelli@gmail.com; > > kernel@stlinux.com; Nathan_Lynch@mentor.com; linux- > > kernel@vger.kernel.org; s-anna@ti.com; linux-arm- > > kernel@lists.infradead.org > > Subject: Re: [STLinux Kernel] [PATCH v5 7/7] ARM: STiH407: Move over to > > using the 'reserved-memory' API for obtaining DMA memory > > > > Hi Lee, > > > > On Wed, 16 Mar 2016, Lee Jones wrote: > > > > > On Wed, 16 Mar 2016, Peter Griffin wrote: > > > > > > > Hi Lee, > > > > > > > > On Tue, 12 Jan 2016, Lee Jones wrote: > > > > > > > > > Doing so saves quite a bit of code in the driver. > > > > > > > > > > For more information on the 'reserved-memory' bindings see: > > > > > > > > > > > > > > > Documentation/devicetree/bindings/reserved-memory/reserved- > > memory. > > > > > txt > > > > > > > > > > Suggested-by: Suman Anna <s-anna@ti.com> > > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > > > > --- > > > > > arch/arm/boot/dts/stih407-family.dtsi | 46 > > > > > +++++++++++++++++++++++++++++------ > > > > > 1 file changed, 38 insertions(+), 8 deletions(-) > > > > > > > > > > diff --git a/arch/arm/boot/dts/stih407-family.dtsi > > > > > b/arch/arm/boot/dts/stih407-family.dtsi > > > > > index 15c20b6..27b8efc 100644 > > > > > --- a/arch/arm/boot/dts/stih407-family.dtsi > > > > > +++ b/arch/arm/boot/dts/stih407-family.dtsi > > > > > @@ -15,6 +15,36 @@ > > > > > #address-cells = <1>; > > > > > #size-cells = <1>; > > > > > > > > > > + reserved-memory { > > > > > + #address-cells = <1>; > > > > > + #size-cells = <1>; > > > > > + ranges; > > > > > + > > > > > + gp0_reserved: rproc@40000000 { > > > > > + compatible = "shared-dma-pool"; > > > > > + reg = <0x40000000 0x01000000>; > > > > > + no-map; > > > > > + }; > > > > > + > > > > > + gp1_reserved: rproc@41000000 { > > > > > + compatible = "shared-dma-pool"; > > > > > + reg = <0x41000000 0x01000000>; > > > > > + no-map; > > > > > + }; > > > > > + > > > > > + audio_reserved: rproc@42000000 { > > > > > + compatible = "shared-dma-pool"; > > > > > + reg = <0x42000000 0x01000000>; > > > > > + no-map; > > > > > + }; > > > > > + > > > > > + dmu_reserved: rproc@43000000 { > > > > > + compatible = "shared-dma-pool"; > > > > > + reg = <0x43000000 0x01000000>; > > > > > + no-map; > > > > > + }; > > > > > > > > I don't believe these reserved memory ranges are correct for > > audio_reserved and dmu_reserved. > > > > > > > > For example my vid_firmware-stih407.elf is linked at 0x41c00000 base > > > > address and my audio_firmware-bd-stih407.elf is linked at 0x40c00000. > > > > > > > > So with all the st231 rproc nodes enabled I guess it would still > > > > work. But currently I think st231_gp0 is reserving the memory region > > > > for st231_audio, and st231-gp1 is reserving the memory region for > > st231_dmu. > > > > > > These addresses are taken from internally tested code. > > > > Yes I did check the internal kernel, it would appear to be wrong there as well. > > One of the joys of mailing list code review I guess :-) > > > > > I don't have > > > access to the LMI layout documentation (if it even exists) so can't > > > check for myself. > > > > > Isn't this just DDR anyway? > > > > Yes it is DDR > > > > > So in theory we can > > > configure each devices' slice where ever we feel is appropriate? > > > > Nope. The st231 audio and video firmwares are provided by ST as binary > > blobs and aren't AFAIK compiled as position independent code. So the > > reserved-memory region needs to match where the firmware has been > > linked to run from. > > > > > How > > > is memory allocated to the DMU and Audio drivers? Do you have scripts > > > which link the aforementioned binaries? > > > > I don't have any scripts, firmware source code or even a st200 toolset. > > > > > > > > If you think there is an issue, I suggest the best thing to do is ping > > > Ludovic, since he is the author of the original code. > > > > Ok I will ping Ludovic and point him at this thread. > > > > I think maybe the internal kernel rproc driver was only used to reserve > > memory, manage clocks, and co-processor reset / power lines, and multicom > > actually loaded the firmware elf file. > > > > The reason for coming to that conclusion is that if rproc driver was loading the > > firmware I can't see how you would end up with a correctly booted co- > > processor with a reserved-memory node which doesn't match up with > > where the firmware is linked to run from. > > > > Did you manage to boot audio or video co-pro successfully with the dt nodes > > as they currently are in this patch? > > > > regards, > > > > Peter. > > > > _______________________________________________ > > Kernel mailing list > > Kernel@stlinux.com > > http://www.stlinux.com/mailman/listinfo/kernel -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi index 15c20b6..27b8efc 100644 --- a/arch/arm/boot/dts/stih407-family.dtsi +++ b/arch/arm/boot/dts/stih407-family.dtsi @@ -15,6 +15,36 @@ #address-cells = <1>; #size-cells = <1>; + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + gp0_reserved: rproc@40000000 { + compatible = "shared-dma-pool"; + reg = <0x40000000 0x01000000>; + no-map; + }; + + gp1_reserved: rproc@41000000 { + compatible = "shared-dma-pool"; + reg = <0x41000000 0x01000000>; + no-map; + }; + + audio_reserved: rproc@42000000 { + compatible = "shared-dma-pool"; + reg = <0x42000000 0x01000000>; + no-map; + }; + + dmu_reserved: rproc@43000000 { + compatible = "shared-dma-pool"; + reg = <0x43000000 0x01000000>; + no-map; + }; + }; + cpus { #address-cells = <1>; #size-cells = <0>; @@ -651,9 +681,9 @@ status = "okay"; }; - st231_gp0: st231-gp0@40000000 { + st231_gp0: st231-gp0 { compatible = "st,st231-rproc"; - reg = <0x40000000 0x01000000>; + memory-region = <&gp0_reserved>; resets = <&softreset STIH407_ST231_GP0_SOFTRESET>; reset-names = "sw_reset"; clocks = <&clk_s_c0_flexgen CLK_ST231_GP_0>; @@ -661,9 +691,9 @@ st,syscfg = <&syscfg_core 0x22c>; }; - st231_gp1: st231-gp1@41000000 { + st231_gp1: st231-gp1 { compatible = "st,st231-rproc"; - reg = <0x41000000 0x01000000>; + memory-region = <&gp1_reserved>; resets = <&softreset STIH407_ST231_GP1_SOFTRESET>; reset-names = "sw_reset"; clocks = <&clk_s_c0_flexgen CLK_ST231_GP_1>; @@ -671,9 +701,9 @@ st,syscfg = <&syscfg_core 0x220>; }; - st231_audio: st231-audio@42000000 { + st231_audio: st231-audio { compatible = "st,st231-rproc"; - reg = <0x42000000 0x01000000>; + memory-region = <&audio_reserved>; resets = <&softreset STIH407_ST231_AUD_SOFTRESET>; reset-names = "sw_reset"; clocks = <&clk_s_c0_flexgen CLK_ST231_AUD_0>; @@ -681,9 +711,9 @@ st,syscfg = <&syscfg_core 0x228>; }; - st231_dmu: st231-dmu@43000000 { + st231_dmu: st231-dmu { compatible = "st,st231-rproc"; - reg = <0x43000000 0x01000000>; + memory-region = <&dmu_reserved>; resets = <&softreset STIH407_ST231_DMU_SOFTRESET>; reset-names = "sw_reset"; clocks = <&clk_s_c0_flexgen CLK_ST231_DMU>;
Doing so saves quite a bit of code in the driver. For more information on the 'reserved-memory' bindings see: Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt Suggested-by: Suman Anna <s-anna@ti.com> Signed-off-by: Lee Jones <lee.jones@linaro.org> --- arch/arm/boot/dts/stih407-family.dtsi | 46 +++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 8 deletions(-) -- 1.9.1