thesofproject
[DNM] nxp: imx8/imx8x: switch to Zephyr native drivers
Hi there, I thought of checking what's happening around here. I noticed that there may be an opportunity to break bisection (though I may misunderstand something) between the last two commits (a place where you no longer initialise a DMA domain, yet the topology still specifies one). Did you consider swapping the last two commits to avoid this?
Other than that I like how this looks! (well except the build errors from the DT, but I guess you're already working on those)
I noticed that there may be an opportunity to break bisection (though I may misunderstand something) between the last two commits (a place where you no longer initialise a DMA domain, yet the topology still specifies one)
yep, you're right
Did you consider swapping the last two commits to avoid this?
that will also break git bisect since the timer domain doesn't work right without the native switch. I think the solution would be to just squash the last 2 commits.
I noticed that there may be an opportunity to break bisection (though I may misunderstand something) between the last two commits (a place where you no longer initialise a DMA domain, yet the topology still specifies one)
yep, you're right
Did you consider swapping the last two commits to avoid this?
that will also break git bisect since the timer domain doesn't work right without the native switch. I think the solution would be to just squash the last 2 commits.
And use in commit message "switch to native drivers and timer domain", since a lot of changes are also related to timer domain. IMO you should do the same in PR https://github.com/thesofproject/sof/pull/8859
V2 changes
- Commit that changes the topologies to timer domain and commit that does transition to native drivers have been squashed into a single one.
- Adapted to HWMv2.
V3 changes
- Rebased
- All Zephyr dependencies merged. Added zephyr hash bump.
Opening this up for reviews. Will remove the [DNM] tag when testing is done.
This PR introduces a small and surprising, imx8/zephyr.lst disassembly difference between the Linux and Windows build:
https://github.com/thesofproject/sof/actions/runs/8552063499?pr=8863
--- win/build-sof-staging/sof-info/imx8x/zephyr.lst 2024-04-04 09:13:44
+++ lin/build-sof-staging/sof-info/imx8x/zephyr.lst 2024-04-04 09:12:34
@@ -81127,20 +81127,26 @@
92433b0e: 430c movi.n a3, 4
#if !(defined(FSL_SDK_DISABLE_DRIVER_CLOCK_CONTROL) && FSL_SDK_DISABLE_DRIVER_CLOCK_CONTROL)
uint32_t instance = LPUART_GetInstance(base);
/* Enable lpuart clock */
(void)CLOCK_EnableClock(s_lpuartClock[instance]);
92433b10: 833441 l32r a4, 924147e0 <_stext+0x1598> (92413034 <s_lpuartClock>)
92433b13: a03340 addx4 a3, a3, a4
92433b16: 03a8 l32i.n a10, a3, 0
+ * @param name Which clock to enable, see \ref clock_ip_name_t.
+ * @return true for success, false for failure.
+ */
+static inline bool CLOCK_EnableClock(clock_ip_name_t name)
+{
+ return CLOCK_EnableClockExt(name, 0);
92433b18: 31e9 s32i.n a14, a1, 12
92433b1a: 00a0b2 movi a11, 0
92433b1d: ffcfa5 call8 92433818 <CLOCK_EnableClockExt>
base->GLOBAL |= LPUART_GLOBAL_RST_MASK;
92433b20: 0020c0 memw
92433b23: 2238 l32i.n a3, a2, 8
92433b25: 240c movi.n a4, 2
92433b27: 203340 or a3, a3, a4
92433b2a: 0020c0 memw
92433b2d: 2239 s32i.n a3, a2, 8
It is triggered by this PR because daily builds are ok: https://github.com/thesofproject/sof/actions/runs/8563182403
In light of recent events I hope everything understands the importance of reproducible builds.
The first thing is to submit the west.yml separately:
https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#defining-smaller-prs
Not just for reviewers but also for Continous integration.
Then, if you don't have BOTH one Linux and one Windows build system, could you please submit draft/throw-away PRs with subsets of this (pretty big) PR to isolate the trigger?
The first thing is to submit the west.yml separately:
... unless there is a two-way dependency between this west.yml bump and some of the code changes in this PR. In that case they must be part of the same commit so every commit works.
This PR introduces a small and surprising,
imx8/zephyr.lstdisassembly difference between the Linux and Windows build:https://github.com/thesofproject/sof/actions/runs/8552063499?pr=8863
--- win/build-sof-staging/sof-info/imx8x/zephyr.lst 2024-04-04 09:13:44 +++ lin/build-sof-staging/sof-info/imx8x/zephyr.lst 2024-04-04 09:12:34 @@ -81127,20 +81127,26 @@ 92433b0e: 430c movi.n a3, 4 #if !(defined(FSL_SDK_DISABLE_DRIVER_CLOCK_CONTROL) && FSL_SDK_DISABLE_DRIVER_CLOCK_CONTROL) uint32_t instance = LPUART_GetInstance(base); /* Enable lpuart clock */ (void)CLOCK_EnableClock(s_lpuartClock[instance]); 92433b10: 833441 l32r a4, 924147e0 <_stext+0x1598> (92413034 <s_lpuartClock>) 92433b13: a03340 addx4 a3, a3, a4 92433b16: 03a8 l32i.n a10, a3, 0 + * @param name Which clock to enable, see \ref clock_ip_name_t. + * @return true for success, false for failure. + */ +static inline bool CLOCK_EnableClock(clock_ip_name_t name) +{ + return CLOCK_EnableClockExt(name, 0); 92433b18: 31e9 s32i.n a14, a1, 12 92433b1a: 00a0b2 movi a11, 0 92433b1d: ffcfa5 call8 92433818 <CLOCK_EnableClockExt> base->GLOBAL |= LPUART_GLOBAL_RST_MASK; 92433b20: 0020c0 memw 92433b23: 2238 l32i.n a3, a2, 8 92433b25: 240c movi.n a4, 2 92433b27: 203340 or a3, a3, a4 92433b2a: 0020c0 memw 92433b2d: 2239 s32i.n a3, a2, 8It is triggered by this PR because daily builds are ok: https://github.com/thesofproject/sof/actions/runs/8563182403
In light of recent events I hope everything understands the importance of reproducible builds.
The first thing is to submit the
west.ymlseparately: https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#defining-smaller-prs Not just for reviewers but also for Continous integration.Then, if you don't have BOTH one Linux and one Windows build system, could you please submit draft/throw-away PRs with subsets of this (pretty big) PR to isolate the trigger?
ACK, started with the west.yml change: https://github.com/thesofproject/sof/pull/9010, which seems to be ok w.r.t the reproducible builds. Anyways, if I were to guess based on the log you've provided, I'd say the change that enables the serial interface causes this issue.
@LaurentiuM1234 is building on Windows priority for you, it seems the compiler has different behavior here for this platform only. I would say here that this is a second order test vs the 1st order runtime functional test for this platform.
I would suggest we comment this platform out from the OS reproducability test and comment in the test that the compiler is generating different code here on Windows vs Linux. You can reference the GH PR and BUG in the comment. We can then move on. @dbaluta this also good with you ?
Comparing Windows and Linux is the only way the reproducibility test work.
That fine when the tools are deterministsic and align when used on different OSes, in this case effort is being spent to workaround a compiler bug for a second order test. Today we need to comment out this platform in the test (or report and ignore result for this platform). The compiler will update over time and may align in a future version (when we can re-enable).
The fix is tested and ready:
- https://github.com/zephyrproject-rtos/zephyr/pull/71535
Rebased. Let's see the repro builds results after https://github.com/thesofproject/sof/pull/9071.