[SKIP SOF-TEST] .github/sparse: add imx8 and imx8m for IPC3 coverage #6879

marc-hb · 2022-12-21T19:58:28Z

This restores sparse coverage for IPC3.

Signed-off-by: Marc Herbert [email protected]

cc:

marc-hb · 2022-12-21T20:15:24Z

@lyakh , @dbaluta , any idea why imx8m shows many sparse errors that seem NOT NXP-specific? Why are these appearing neither in TGL and MTL

https://github.com/thesofproject/sof/actions/runs/3752405278/jobs/6374525487

/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:96:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:96:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:96:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:111:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:96:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:96:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:96:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:96:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:96:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:96:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:111:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/include/sof/coherent.h:346:54: warning: incorrect type in initializer (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:111:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/include/sof/coherent.h:361:39: warning: incorrect type in initializer (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:140:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/ipc/ipc-helper.c:301:52: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/ipc/ipc-helper.c:314:52: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/include/sof/coherent.h:346:54: warning: incorrect type in initializer (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:111:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/include/sof/coherent.h:361:39: warning: incorrect type in initializer (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:140:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/include/sof/coherent.h:346:54: warning: incorrect type in initializer (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:111:13: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/zephyr/../src/include/sof/coherent.h:361:39: warning: incorrect type in initializer (different address spaces)
/zep_workspace/sof/zephyr/../src/arch/xtensa/include/arch/lib/cache.h:140:13: warning: incorrect type in argument 1 (different address spaces)

and much more

lyakh · 2022-12-22T07:33:31Z

@marc-hb there are several reasons. One of them is

sof/src/platform/imx8/include/platform/lib/memory.h

Lines 195 to 198 in 8368a6c

    
           #define uncache_to_cache(address)	address 
        
           #define cache_to_uncache(address)	address 
        
           #define cache_to_uncache_init(address)	address 
        
           #define is_uncached(address)		0

Looking at that I'm actually wondering if our __sparse_cache annotations make sense for iMX8 and all other platforms with the identity cache_to_uncache() mapping. Is this PR just an experiment of yours or is there a requirement to implement this? Maybe we only need sparse checking for TGL and MTL, where cache_to_uncache() are not 1-to-1

paulstelian97 · 2022-12-22T08:37:18Z

Maybe the macros should get annotations to avoid these issues without doing anything at runtime, just for the sake of it? I wouldn't have an issue with that. Just a cast without any arithmetic done on the pointer. sparse should be satisfied if we do that?

lyakh · 2022-12-23T07:03:18Z

Maybe the macros should get annotations to avoid these issues without doing anything at runtime, just for the sake of it? I wouldn't have an issue with that. Just a cast without any arithmetic done on the pointer. sparse should be satisfied if we do that?

@paulstelian97 I'm actually unsure what's better: on the one hand if there isn't a second distinct address space, we shouldn't claim to use it. I.e. on those platforms we shouldn't have additional address spaces and therefore we shouldn't use sparse to verify them. OTOH maybe defining a second address space on all platforms and running sparse on them can help us keep the software uniform and avoid some bugs.
But I think I tend towards the first option: if there isn't a second address space we shouldn't pretend, that there is one.

marc-hb · 2022-12-23T07:17:50Z

This was an experiment following your comment in #6876 (comment)

The idea was to keep providing sparse coverage for IPC3 code if/when TGL+IPC3 stops building.

Maybe we only need sparse checking for TGL and MTL, where cache_to_uncache() are not 1-to-1

I didn't know only TGL and MTL had this.

In that case I guess we won't need sparse coverage for IPC3 if/when it does not even build at all for neither TGL nor MTL?

marc-hb · 2022-12-23T07:19:24Z

In that case I guess we won't need sparse coverage for IPC3 if/when it does not even build at all for neither TGL nor MTL?

Hopefully MTL/IPC4 will be sparse-clean before TGL/IPC3 stops building. Otherwise we'll be back to zero green sparse report for a while.

paulstelian97 · 2022-12-23T07:57:44Z

Maybe the macros should get annotations to avoid these issues without doing anything at runtime, just for the sake of it? I wouldn't have an issue with that. Just a cast without any arithmetic done on the pointer. sparse should be satisfied if we do that?

@paulstelian97 I'm actually unsure what's better: on the one hand if there isn't a second distinct address space, we shouldn't claim to use it. I.e. on those platforms we shouldn't have additional address spaces and therefore we shouldn't use sparse to verify them. OTOH maybe defining a second address space on all platforms and running sparse on them can help us keep the software uniform and avoid some bugs. But I think I tend towards the first option: if there isn't a second address space we shouldn't pretend, that there is one.

@lyakh I tend to prefer the uniformity there, and then sparse can detect issues in generic code on any platform really. Plus, even for the platform-specific code, who is to say our future hardware revisions won't have the different address spaces? For that hypothetical it might be beneficial even for platform code to have it in the end (drivers shared between the hardware revisions).

I still remember the unrelated issue of Dummy DMA when we enabled the caching (where I had to add the cache control stuff -- discarding or writing back cache lines so that the memcpy can work correctly between host and DSP memory -- later instead of when I initially wrote the driver), I would like other issues to be detected earlier when they don't actually affect us in the end, rather than slow us down when they actually do affect us.

lyakh · 2022-12-27T06:57:30Z

Maybe the macros should get annotations to avoid these issues without doing anything at runtime, just for the sake of it? I wouldn't have an issue with that. Just a cast without any arithmetic done on the pointer. sparse should be satisfied if we do that?

@paulstelian97 I'm actually unsure what's better: on the one hand if there isn't a second distinct address space, we shouldn't claim to use it. I.e. on those platforms we shouldn't have additional address spaces and therefore we shouldn't use sparse to verify them. OTOH maybe defining a second address space on all platforms and running sparse on them can help us keep the software uniform and avoid some bugs. But I think I tend towards the first option: if there isn't a second address space we shouldn't pretend, that there is one.

@lyakh I tend to prefer the uniformity there, and then sparse can detect issues in generic code on any platform really. Plus, even for the platform-specific code, who is to say our future hardware revisions won't have the different address spaces? For that hypothetical it might be beneficial even for platform code to have it in the end (drivers shared between the hardware revisions).

I still remember the unrelated issue of Dummy DMA when we enabled the caching (where I had to add the cache control stuff -- discarding or writing back cache lines so that the memcpy can work correctly between host and DSP memory -- later instead of when I initially wrote the driver), I would like other issues to be detected earlier when they don't actually affect us in the end, rather than slow us down when they actually do affect us.

@paulstelian97 ok, then we need cache_to_uncache() and similar macros supplied with __sparse_cache annotations. Basically we need to fix all failures that this PR uncovers

lyakh · 2022-12-27T07:02:56Z

In that case I guess we won't need sparse coverage for IPC3 if/when it does not even build at all for neither TGL nor MTL?

@marc-hb it looks like @paulstelian97 does want to have sparse support for their platforms, so I assume they will fix all the issues that your PR has uncovered. Otherwise - it looks like v2.2 branches don't have sparse support, so, then no, doesn't look like we're going to test IPC3 with sparse on Intel platforms.

gkbldcig · 2023-02-18T04:18:08Z

Can one of the admins verify this patch?

marc-hb · 2023-03-13T14:13:54Z

Still failing in rebased https://github.com/thesofproject/sof/actions/runs/4405751530/jobs/7717051185

Most of the warnings seem to come from a handful of .h lines : massively multiplied by the number of .c files using them.

dbaluta · 2023-03-13T14:30:24Z

@paulstelian97 I think we should go with your proposed solution:

@paulstelian97 ok, then we need cache_to_uncache() and similar macros supplied with __sparse_cache annotations. Basically we need to fix all failures that this PR uncovers

Please take care of this when you have some bandwidth.

cujomalainey · 2023-03-13T19:53:47Z

@marc-hb please let me know if you spot a CL for TGL/ADL/RPL removal. That will royally disrupt our development workflow over here as IPC4 is not even remotely available

This restores sparse coverage for IPC3. Signed-off-by: Marc Herbert <[email protected]>

marc-hb · 2024-09-26T22:12:25Z

Unrelated but interesting testbench error, @singalsu you may want to track this somewhere:

==23756== Command: ../../testbench/build_testbench/install/bin/testbench -q -r 48000 -R 48000 -c 2 -n 2 -b S16_LE -t ../../build_tools/test/topology/test-playback-ssp5-mclk-0-I2S-volume-s16le-s16le-48k-24576k-codec.tplg -i chirp_test_in_988436.raw -o chirp_test_out_988436.raw
==23756== 
1727388220.778265:(ipc-common.c:273) ipc_init()
1727388220.790481:(ll_schedule.c:114) ll_scheduler_init()
1727388220.791858:(edf_schedule.c:112) edf_scheduler_init()
1727388220.819643:(helper.c:144) get_drv():
    the provided UUID (bfc7488c4ce875aadad8be9dc[29](https://github.com/thesofproject/sof/actions/runs/11060897847/job/30732418706?pr=6879#step:6:30)8a608)
    doesn't match to any driver!
1727388220.820339:(helper.c:684) ipc_comp_new(): component cd = NULL
error: file read
error: load AIF IN failed
error: parsing topology
error: pipeline load 0 failed -22

marc-hb · 2024-09-26T22:15:43Z

This is really "wild": CMocka tests also failed?! Totally unrelated to this PR too

https://github.com/thesofproject/sof/actions/runs/11060897851/job/30732419238?pr=6879

Daily tests have been green for a very long time: https://github.com/thesofproject/sof/actions/runs/11043440815

94% tests passed, 3 tests failed out of 47

Total Test time (real) =   0.06 sec

The following tests FAILED:
Errors while running CTest
Output from these tests are in: /home/runner/work/sof/sof/build_ut_defs/acp_6_3_defconfig/Testing/Temporary/LastTest.log
Use "--rerun-failed --output-on-failure" to re-run the failed cases verbosely.
	 10 - mux_get_processing_function (Failed)
	 11 - mux_copy (Failed)
	 12 - demux_copy (Failed)
gmake: *** [Makefile:71: test] Error 8

marc-hb · 2024-09-26T22:17:10Z

IMX still has sparse errors

https://github.com/thesofproject/sof/actions/runs/11060897842/job/30732418315?pr=6879

/zep_workspace/zephyr/drivers/clock_control/clock_control_mcux_ccm.c:444:17: error: typename in expression
/zep_workspace/zephyr/drivers/clock_control/clock_control_mcux_ccm.c:444:17: error: undefined identifier '__attribute__'
/zep_workspace/zephyr/drivers/dai/nxp/sai/sai.c:935:1: error: bad integer constant expression
/zep_workspace/zephyr/drivers/dai/nxp/sai/sai.c:935:1: error: static assertion failed: "bad or unsupported SAI instance. Is the base address correct?"
/zep_workspace/zephyr/drivers/dai/nxp/sai/sai.c:935:1: error: bad integer constant expression
/zep_workspace/zephyr/drivers/dai/nxp/sai/sai.c:935:1: error: static assertion failed: "invalid TX data line index"
/zep_workspace/zephyr/drivers/dai/nxp/sai/sai.c:935:1: error: bad integer constant expression
/zep_workspace/zephyr/drivers/dai/nxp/sai/sai.c:935:1: error: static assertion failed: "invalid RX data line index"
/zep_workspace/sof/src/ipc/ipc-helper.c:313:52: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/ipc/ipc-helper.c:324:52: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/audio/pipeline/pipeline-graph.c:417:52: warning: incorrect type in argument 1 (different address spaces)

https://github.com/thesofproject/sof/actions/runs/11060897842/job/30732419025?pr=6879

/zep_workspace/zephyr/drivers/clock_control/clock_control_mcux_ccm.c:444:17: error: typename in expression
/zep_workspace/zephyr/drivers/clock_control/clock_control_mcux_ccm.c:444:17: error: undefined identifier '__attribute__'
/zep_workspace/sof/src/ipc/ipc-helper.c:313:52: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/ipc/ipc-helper.c:324:52: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/drivers/generic/dummy-dma.c:106:35: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/drivers/generic/dummy-dma.c:116:34: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/drivers/imx/sdma.c:127:33: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/drivers/imx/sdma.c:129:39: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/drivers/imx/sdma.c:252:38: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/drivers/imx/sdma.c:563:46: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/drivers/imx/sdma.c:566:45: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/drivers/imx/sdma.c:857:33: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/drivers/imx/sdma.c:858:38: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/drivers/imx/sdma.c:859:38: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/drivers/imx/sdma.c:976:34: warning: incorrect type in argument 1 (different address spaces)
/zep_workspace/sof/src/audio/pipeline/pipeline-graph.c:417:52: warning: incorrect type in argument 1 (different address spaces)

marc-hb · 2024-09-26T22:18:27Z

BUT the firmware now builds with XCC and Python 3.10: https://sof-ci.01.org/sofpr/PR6879/build8365/build/firmware_community/tgl.log thanks to some internal changes we just did (internal issue 606 + #9475 (comment))

marc-hb · 2024-09-27T14:02:18Z

This is really "wild": CMocka tests also failed?! Totally unrelated to this PR too

"Mystery" solved, #9496 (comment) was also merged broken - on the same day as #9475 (comment) was merged broken.

marc-hb requested a review from paulstelian97 December 21, 2022 20:31

marc-hb marked this pull request as ready for review December 23, 2022 15:55

marc-hb requested review from dbaluta, cujomalainey, lgirdwood and aborisovich as code owners December 23, 2022 15:55

marc-hb marked this pull request as draft December 23, 2022 15:56

marc-hb requested review from andyross, dcpleung, kv2019i and a team December 23, 2022 15:59

marc-hb force-pushed the sparse-imx8m branch from 139a45d to 4d159a0 Compare March 13, 2023 14:00

.github/sparse: add imx8 and imx8m for IPC3 coverage

517905e

This restores sparse coverage for IPC3. Signed-off-by: Marc Herbert <[email protected]>

marc-hb force-pushed the sparse-imx8m branch from 4d159a0 to 517905e Compare September 26, 2024 22:01

marc-hb changed the title ~~[SKIP CI] .github/sparse: add imx8m~~ github/sparse: add imx8m Sep 26, 2024

marc-hb changed the title ~~github/sparse: add imx8m~~ .github/sparse: add imx8 and imx8m for IPC3 coverage Sep 26, 2024

marc-hb removed request for a team and aborisovich September 26, 2024 22:18

marc-hb changed the title ~~.github/sparse: add imx8 and imx8m for IPC3 coverage~~ [SKIP SOF-TEST] .github/sparse: add imx8 and imx8m for IPC3 coverage Sep 26, 2024

marc-hb mentioned this pull request Sep 27, 2024

ipc3: override type field once comp_driver found #9496

Merged

dbaluta approved these changes Sep 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SKIP SOF-TEST] .github/sparse: add imx8 and imx8m for IPC3 coverage #6879

[SKIP SOF-TEST] .github/sparse: add imx8 and imx8m for IPC3 coverage #6879

marc-hb commented Dec 21, 2022 •

edited

Loading

marc-hb commented Dec 21, 2022 •

edited

Loading

lyakh commented Dec 22, 2022

paulstelian97 commented Dec 22, 2022 •

edited

Loading

lyakh commented Dec 23, 2022

marc-hb commented Dec 23, 2022

marc-hb commented Dec 23, 2022

paulstelian97 commented Dec 23, 2022

lyakh commented Dec 27, 2022

lyakh commented Dec 27, 2022

gkbldcig commented Feb 18, 2023

marc-hb commented Mar 13, 2023

dbaluta commented Mar 13, 2023

cujomalainey commented Mar 13, 2023

marc-hb commented Sep 26, 2024

marc-hb commented Sep 26, 2024

marc-hb commented Sep 26, 2024

marc-hb commented Sep 26, 2024 •

edited

Loading

marc-hb commented Sep 27, 2024 •

edited

Loading

[SKIP SOF-TEST] .github/sparse: add imx8 and imx8m for IPC3 coverage #6879

Are you sure you want to change the base?

[SKIP SOF-TEST] .github/sparse: add imx8 and imx8m for IPC3 coverage #6879

Conversation

marc-hb commented Dec 21, 2022 • edited Loading

marc-hb commented Dec 21, 2022 • edited Loading

lyakh commented Dec 22, 2022

paulstelian97 commented Dec 22, 2022 • edited Loading

lyakh commented Dec 23, 2022

marc-hb commented Dec 23, 2022

marc-hb commented Dec 23, 2022

paulstelian97 commented Dec 23, 2022

lyakh commented Dec 27, 2022

lyakh commented Dec 27, 2022

gkbldcig commented Feb 18, 2023

marc-hb commented Mar 13, 2023

dbaluta commented Mar 13, 2023

cujomalainey commented Mar 13, 2023

marc-hb commented Sep 26, 2024

marc-hb commented Sep 26, 2024

marc-hb commented Sep 26, 2024

marc-hb commented Sep 26, 2024 • edited Loading

marc-hb commented Sep 27, 2024 • edited Loading

marc-hb commented Dec 21, 2022 •

edited

Loading

marc-hb commented Dec 21, 2022 •

edited

Loading

paulstelian97 commented Dec 22, 2022 •

edited

Loading

marc-hb commented Sep 26, 2024 •

edited

Loading

marc-hb commented Sep 27, 2024 •

edited

Loading