lib: wait for fw loaded before starting any test #1033

fredoh9 · 2023-05-04T22:06:56Z

This is going to check whether the sof-test can be started after boot/reboot.

If FW LOADING is not a prerequisite for the test, this can be skipped, by defining NO_POLL_FW_LOADING=true

Another benefit with this change, if FW is not loaded it will be failed instead of TIMEOUT.

fredoh9 · 2023-05-04T22:08:36Z

typo in title, staring -> starting.
will fix when next push is required

marc-hb · 2023-05-04T22:21:46Z

case-lib/config.sh

+# Set MAX Polling time to check FW Loading. If FW is already loaded, it will
+# return immediately. Default value is set to 60 seconds, because i915 driver
+# timeout is 60 seconds.
+MAX_WAIT_FW_LOADING=${MAX_WAIT_FW_LOADING:-60}


If i915 driver is 60 seconds, shouldn't this be 65 seconds?

Also: can you please add at least one reference for more context? Maybe some of these:

Add test for no-i915 configurations #291

[BUG]ADLP firmware boot delayed by i915 issue (failure in check-sof-logger) linux#3042

[BUG] "Test Result: FAIL!" is missing when firmware path is not found from journalctl #979

other?

If i915 driver is 60 seconds, shouldn't this be 65 seconds?

Good point, I think about this too. But there are some other delays already applied top of this. so it should have plenty of room

But there are some other delays already applied top of this

Once we merge this then we should get rid of those.

Also: this is only a maximum. The test will start as soon as the firmware has booted.

okay, I will update default value to 70. if you plan to remove all extra delays, I need some extra margin

marc-hb

@plbossart should we add a counter and print a warning when waiting more than 10s?

Note we already discussed this in test-specific #862. There we set a 10s maximum 1 year ago and it has never spuriously failed.

plbossart

couple of comments below @fredoh9, nice work indeed.

plbossart · 2023-05-04T23:06:38Z

case-lib/config.sh

+# Set MAX Polling time to check FW Loading. If FW is already loaded, it will
+# return immediately. Default value is set to 60 seconds, because i915 driver
+# timeout is 60 seconds.
+MAX_WAIT_FW_LOADING=${MAX_WAIT_FW_LOADING:-60}


what is the relationship between DSP firmware load and i915? is this that we first request the i915 modules before downloading the firmware?

and if yes, should we use the same routine as for the kmod remove/modprobe test where we have an iteration that stops on BOOT_COMPLETE.

and if yes, should we use the same routine as for the kmod remove/modprobe test where we have an iteration that stops on BOOT_COMPLETE.

That's exactly the suggestion I made in internal bug 387 and what @fredoh9 is doing here: he's re-using the functions I wrote in #862

case-lib/lib.sh

marc-hb

@plbossart should we add a counter and print a warning when waiting more than 10s?

I think you missed this question

marc-hb · 2023-05-04T23:10:46Z

case-lib/config.sh

+# Set MAX Polling time to check FW Loading. If FW is already loaded, it will
+# return immediately. Default value is set to 60 seconds, because i915 driver
+# timeout is 60 seconds.
+MAX_WAIT_FW_LOADING=${MAX_WAIT_FW_LOADING:-60}


But there are some other delays already applied top of this

Once we merge this then we should get rid of those.

marc-hb · 2023-05-04T23:12:35Z

case-lib/config.sh

+# Set MAX Polling time to check FW Loading. If FW is already loaded, it will
+# return immediately. Default value is set to 60 seconds, because i915 driver
+# timeout is 60 seconds.
+MAX_WAIT_FW_LOADING=${MAX_WAIT_FW_LOADING:-60}


and if yes, should we use the same routine as for the kmod remove/modprobe test where we have an iteration that stops on BOOT_COMPLETE.

That's exactly the suggestion I made in internal bug 387 and what @fredoh9 is doing here: he's re-using the functions I wrote in #862

marc-hb · 2023-05-04T23:19:53Z

Another benefit with this change, if FW is not loaded it will be failed instead of TIMEOUT.

Because it's the "last chance" TIMEOUT in our CI it's very long, how long Fred?.

Moreover we power-cycle the device after such a CI TIMEOUT. When combined with another unrelated issue #979, the whole test run ends up in a very time-consuming tail spin on that device.

fredoh9 · 2023-05-05T18:02:26Z

Because it's the "last chance" TIMEOUT in our CI it's very long, how long Fred?.

TIMEOUT value is dependent on each test case. we used to set 2-4 times higher value of normal runtime. For an example, check-suspend-resume-50.sh has 6000 second. TIMEOUT has become an expensive way to waste resources.

This is going to check whether the sof-test can be started after boot/reboot. If FW LOADING is not a prerequisite for the test, this can be skipped, by defining NO_POLL_FW_LOADING=true Another benefit with this change, if FW is not loaded it will be failed instead of TIMEOUT. Signed-off-by: Fred Oh <[email protected]>

fredoh9 · 2023-05-05T18:14:44Z

@plbossart should we add a counter and print a warning when waiting more than 10s?

I think you missed this question

@plbossart @marc-hb I can print out warning every 10 sec. instead of zero output for MAX wait time

keqiaozhang

Nice solution.

paulstelian97

This feels good but incomplete. Before merging this I would like to see a good analysis -- do some tests actually need that variable right now or can it be done later?

marc-hb · 2023-05-08T13:46:56Z

@paulstelian97 this will help a lot in scenarios where the GPU driver is missing or failing and audio it timing out and where other issues make the situation much worse. I just unresolved the comment where I asked to provide some links for more context; you could start with #979.

marc-hb · 2023-05-09T18:16:09Z

@plbossart @marc-hb I can print out warning every 10 sec. instead of zero output for MAX wait time

That would be great. I think we could merge this and then add these warnings later?

fredoh9 · 2023-05-09T21:27:24Z

That would be great. I think we could merge this and then add these warnings later?

yes, let's merge this and see. If we need anything improvement, I will address them together

fredoh9 · 2023-05-10T16:21:07Z

I will monitor upcoming the test results

marc-hb · 2023-05-15T15:10:30Z

I think @lyakh found an issue with this new feature: kernel logs seem to be missing when the firmware does not boot. Example: https://sof-ci.01.org/sofpr/PR7584/build7585/devicetest/index.html

In jenkins logs generic_test/13429/

10:58:40  cmd: scp -r [email protected]:~/sof-test/test-case/../logs/verify-firmware-presence/last/* results/2023-05-12-03:55:59/sh-adlp-rvp-sdw-01/verify-firmware-presence
10:58:40  return code: 1
10:58:40  cmd output: scp: /home/ubuntu/sof-test/test-case/../logs/verify-firmware-presence/last/*: No such file or directory

This is very likely because the new code runs "too early".

fredoh9 requested a review from a team as a code owner May 4, 2023 22:06

fredoh9 changed the title ~~lib: wait for fw loaded before staring any test~~ lib: wait for fw loaded before starting any test May 4, 2023

marc-hb reviewed May 4, 2023

View reviewed changes

fredoh9 mentioned this pull request May 4, 2023

Add test for no-i915 configurations #291

Open

fredoh9 requested review from plbossart, keqiaozhang, aiChaoSONG and greg-intel May 4, 2023 22:47

plbossart reviewed May 4, 2023

View reviewed changes

marc-hb reviewed May 4, 2023

View reviewed changes

marc-hb requested review from andyross, lyakh, ranj063, paulstelian97, LaurentiuM1234 and kv2019i and removed request for greg-intel May 4, 2023 23:14

fredoh9 force-pushed the fix/wait_for_fw_load branch from 00e538a to 6a6b7a2 Compare May 5, 2023 18:10

marc-hb approved these changes May 5, 2023

View reviewed changes

keqiaozhang approved these changes May 6, 2023

View reviewed changes

paulstelian97 reviewed May 8, 2023

View reviewed changes

dbaluta approved these changes May 8, 2023

View reviewed changes

paulstelian97 approved these changes May 9, 2023

View reviewed changes

fredoh9 merged commit fceeee0 into thesofproject:main May 10, 2023

fredoh9 mentioned this pull request May 15, 2023

[BUG] No kernel boot log when SOF FW loading fails #1036

Closed

keqiaozhang mentioned this pull request May 17, 2023

[BUG] [MTL] soundwire_intel soundwire_intel.link.0: SCP Msg trf timed out thesofproject/linux#4233

Closed

marc-hb mentioned this pull request May 17, 2023

[BUG] check-kmod-load-unload fails on MTL #1037

Closed

marc-hb mentioned this pull request Apr 24, 2024

lib.sh,hijack.sh: restore ability to be sourced interactively #1071

Closed

marc-hb mentioned this pull request May 6, 2024

ALSA/ASoC: Intel: log probe details once thesofproject/linux#4977

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib: wait for fw loaded before starting any test #1033

lib: wait for fw loaded before starting any test #1033

fredoh9 commented May 4, 2023

fredoh9 commented May 4, 2023

marc-hb May 4, 2023

fredoh9 May 4, 2023

marc-hb May 4, 2023

marc-hb May 4, 2023

fredoh9 May 5, 2023

marc-hb left a comment •

edited

Loading

plbossart left a comment

plbossart May 4, 2023

marc-hb May 4, 2023

marc-hb left a comment

marc-hb May 4, 2023

marc-hb May 4, 2023

marc-hb commented May 4, 2023

fredoh9 commented May 5, 2023 •

edited

Loading

fredoh9 commented May 5, 2023

keqiaozhang left a comment

paulstelian97 left a comment

marc-hb commented May 8, 2023

marc-hb commented May 9, 2023

fredoh9 commented May 9, 2023

fredoh9 commented May 10, 2023

marc-hb commented May 15, 2023

lib: wait for fw loaded before starting any test #1033

lib: wait for fw loaded before starting any test #1033

Conversation

fredoh9 commented May 4, 2023

fredoh9 commented May 4, 2023

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

marc-hb left a comment • edited Loading

Choose a reason for hiding this comment

plbossart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marc-hb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marc-hb commented May 4, 2023

fredoh9 commented May 5, 2023 • edited Loading

fredoh9 commented May 5, 2023

keqiaozhang left a comment

Choose a reason for hiding this comment

paulstelian97 left a comment

Choose a reason for hiding this comment

marc-hb commented May 8, 2023

marc-hb commented May 9, 2023

fredoh9 commented May 9, 2023

fredoh9 commented May 10, 2023

marc-hb commented May 15, 2023

marc-hb left a comment •

edited

Loading

fredoh9 commented May 5, 2023 •

edited

Loading