Abailon/i500 upstream v1 #139

anobli · 2020-11-27T10:55:04Z

Hello,

This pull request intends to add support of the AI Processing Unit (APU), a dual core VP6 (Xtensa LX7 ISA).
This just adds the minimum required for Rpmsg to work on the Mediatek i500 SoC.
Because there many other SoC or microcontroller using the Xtensa architecture, the support of xtensa and APU have been separated.

To build libmetal, gcc 10 could be used. Still this requires few additional low level libraries so I could provided a prebuilt toolchain for people interested (until I could fully document how to rebuild one).
XCC (Cadence complier) could also be used but this requires more cmake options to be configured.

For testing libmetal, we are building and running test-metal-static.
Currently, we can only load it on a real target, using remoteproc which add some constraints (e.g a resource table is required).
In addition, there are no uart so all the log must go to a log buffer.
I may try to use qemu or Candence ISS instead of real target as this would be more convenient for CI.

Thanks,
Alexandre

arnopo

Hello,

Few comments , i think the main point is the logs management.
Concerning CI I let @edmooring responds, he is working on CI relying on QEMU for Xilinx platform.
Then we can also imagine a CI build test based on a github action using XCC if your build environment is opensource. Something similar to existing github actions: https://github.com/OpenAMP/libmetal/tree/master/.github/actions/build_ci.

lib/processor/xtensa/CMakeLists.txt

lib/processor/xtensa/atomic.h

lib/system/generic/xtensa_common/CMakeLists.txt

lib/io.h

cmake/platforms/i500-apu-generic.cmake

lib/system/generic/i500_apu/sys.h

lib/system/generic/i500_apu/log.c

lib/system/generic/i500_apu/sys.h

test/system/generic/irq.c

This adds support of xtensa processor. Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

This adds support of xtensa architecture. This dependens on xtos, a low level library from Cadence. Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

anobli · 2020-12-03T16:28:48Z

I pushed a new branch. Thanks for you feed back.

The i500 has a dual core VP6 (Xtensa LX7 ISA), also know as APU. This adds a basic support of i500 Mediatek APU to libmetal. Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

To load the firmware on i500 APU, we need to use remoteproc. This implies to have a resource table. This adds the resource table and the linker script required to build a firmware for i500 apu. Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

arnopo

A few additional comments and questions, no major points.

arnopo · 2020-12-10T13:30:51Z

lib/system/generic/i500_apu/CMakeLists.txt

+collect (PROJECT_LIB_SOURCES irq.c)
+collect (PROJECT_LIB_SOURCES sys.c)
+
+add_subdirectory(../xtensa_common ${CMAKE_CURRENT_BINARY_DIR}/../xtensa_common)


I wonder if we should have following hierarchy instead:
/lib/ssytem/generic/xtensa/i500_apu/
means /familly/product/ hierarchy
@edmooring : any opinion on that?

Hum, this is going to imply some changes in the libmetal code.
Currently, the cmake files and some headers expect the following hierarchy:
/lib/ssytem/generic/machine/

So to have a hierarchy like /familly/product/ I will have to change many files if I want to do it correctly (e.g do the same for all the supported machines).

Another intermediate options would be to replace PROJECT_MACHINE in CMakeLists.txt and some headers by another variable that would contains the good path, depending if we using the current hierarchy or the family/product hierarchy.

Your point is valid. As the hierarchy would have to be reworked. if @edmooring is Ok i propose to not update this ( so similar to xlnx_common) and treat it is a separate point. The include hierarchy is something that as to be improved in libmetal

arnopo · 2020-12-10T13:33:22Z

lib/system/generic/i500_apu/irq.c

+	sys_irq_enable(number);
+	value = i500_read_reg(VPU_INT_MASK_REG);
+	value &= ~(1 << number);
+	i500_write_reg(VPU_INT_MASK_REG, value);


No need to protect agains concurrent access, the read, update, write action? or an atomic way to do it?

arnopo · 2020-12-10T13:50:02Z

test/system/generic/i500_apu/specs

+
+*lib:
+-lc -lgloss -lminrt -lc -lhandler-reset -lhandlers-board -lminrt -lhal -lc
+


I'm not familiar with this kind of file... Is this can not be handle by cmake?

I have discovered the specs file by working on this project so I my understanding might be wrong.
I think I need to have to override some default options of the linker.
IIRC, the default options are to build a firmware that could run on an emulator.
I don't know if there is another to way to override those options but I will check.

I spent some time to figure out how to do the same thing using gcc command line and I failed.
The closer result I got was making the command line really big and complex and it was still failing at link step.
The only other option we have if we want to get ride of this file is too do some changes in the toolchain itself.
If this OK for you, I would like to keep the spec file as it makes things simpler.

Do you try to just add in cmake/platforms/i500-apu-generic.cmake?
set(CMAKE_EXE_LINKER_FLAGS "-lc -lgloss -lminrt -lhandler-reset -lhandlers-board -lhal")

Seems also that -lc and -lminrt are defined several times in the command line...

arnopo · 2020-12-10T13:57:56Z

test/system/generic/i500_apu/ldscripts/elf32xtensa.x

@@ -0,0 +1,510 @@
+/* This linker script generated from xt-genldscripts.tpp for LSP . */
+/* Linker Script for default link */


A reason to place the linker in a specific folder?
Otherwise, as for other platforms, please place it directly in test/system/generic/i500_apu/

This is legacy. Usually, the linker scripts for these platform are generated using a tool, which creates many files in ldscripts.
I kept only the linker script but I have not though to move it out from this folder.
I will do it.

edmooring

I agree with @arnopo's comments.
Can you please add appropriate information to README.md describing the supported platforms and the addition of a new architecture, with pointers to the tools needed?
My CI work is primarily focused on ARM-based platform, so I'm not sure how this would fit in.

arnopo requested review from arnopo and edmooring December 1, 2020 09:17

arnopo requested changes Dec 1, 2020

View reviewed changes

anobli added 2 commits December 3, 2020 10:55

processor: add support of xtensa processor

4de6263

This adds support of xtensa processor. Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

lib: generic: Add support of xtensa

8a5795a

This adds support of xtensa architecture. This dependens on xtos, a low level library from Cadence. Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

anobli force-pushed the abailon/i500_upstream_v1 branch 2 times, most recently from cec9bfa to 4966613 Compare December 3, 2020 16:28

anobli force-pushed the abailon/i500_upstream_v1 branch from 4966613 to 9aad850 Compare December 7, 2020 09:29

anobli added 2 commits December 7, 2020 10:29

lib: generic: Add support of Mediatek i500 APU

8e5a477

The i500 has a dual core VP6 (Xtensa LX7 ISA), also know as APU. This adds a basic support of i500 Mediatek APU to libmetal. Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

arnopo requested changes Dec 10, 2020

View reviewed changes

edmooring requested changes Dec 11, 2020

View reviewed changes

arnopo added area: platform Stale labels Aug 19, 2021

github-actions bot closed this Oct 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Abailon/i500 upstream v1 #139

Abailon/i500 upstream v1 #139

anobli commented Nov 27, 2020

arnopo left a comment

anobli commented Dec 3, 2020

arnopo left a comment

arnopo Dec 10, 2020

anobli Dec 15, 2020

arnopo Dec 15, 2020

arnopo Dec 10, 2020

arnopo Dec 10, 2020

anobli Dec 14, 2020

anobli Jan 28, 2021

arnopo Jan 29, 2021

arnopo Dec 10, 2020

anobli Dec 14, 2020

edmooring left a comment


		*lib:
		-lc -lgloss -lminrt -lc -lhandler-reset -lhandlers-board -lminrt -lhal -lc

		@@ -0,0 +1,510 @@
		/* This linker script generated from xt-genldscripts.tpp for LSP . */
		/* Linker Script for default link */

Abailon/i500 upstream v1 #139

Abailon/i500 upstream v1 #139

Conversation

anobli commented Nov 27, 2020

arnopo left a comment

Choose a reason for hiding this comment

anobli commented Dec 3, 2020

arnopo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edmooring left a comment

Choose a reason for hiding this comment