sof icon indicating copy to clipboard operation
sof copied to clipboard

[DO NOT MERGE] EQ IIR conversion Take 2

Open ranj063 opened this issue 2 years ago • 8 comments

ranj063 avatar Jul 19 '22 20:07 ranj063

This is passed with check-signal-stop-start.sh on ADLP_RVP_NOCODEC. run-test-all.sh -l 1 is running now, looking good so far.

fredoh9 avatar Jul 19 '22 20:07 fredoh9

Failed in multiple-pipeline-capture.sh, DSP panic. We can wait and see official PR testing result as well.

+ /home/ubuntu/sof-test/test-case/multiple-pipeline-capture.sh -l 1
2022-07-19 20:47:24 UTC Sub-Test: [INFO] DUT is running IPC3 mode
...
===========================>>
[  974.550619] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: ------------[ DSP dump start ]------------
[  974.550620] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: DSP panic!
[  974.550620] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: fw_state: SOF_FW_BOOT_COMPLETE (6)
[  974.550625] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: 0x00000005: module: ROM, state: FW_ENTERED, running
[  974.550627] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: status code: 0xdead006 (unknown)
[  974.550713] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: reason: runtime exception (0x6)
[  974.550714] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: trace point: 0x00004000
[  974.550715] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: panic at :0
[  974.550715] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: error: DSP Firmware Oops
[  974.550716] kernel: sof-audio-pci-intel-tgl 0000:00:1f.3: error: Exception Cause: LoadStorePIFDataErrorCause, Synchronous PIF data error during LoadStore access

dmesg.txt etrace.txt slogger.txt

fredoh9 avatar Jul 19 '22 20:07 fredoh9

part of this pr is already merged,only two commits valid

  • module_adapter: set state to INITIALIZED after reset
  • data_blob: modify comp_data_blob_handler_free()

aiChaoSONG avatar Jul 20 '22 04:07 aiChaoSONG

part of this pr is already merged,only two commits valid

  • module_adapter: set state to INITIALIZED after reset
  • data_blob: modify comp_data_blob_handler_free()

@aiChaoSONG i'm aware of that and thats why it is still a draft. I will make it ready for review when I have clarity

ranj063 avatar Jul 20 '22 06:07 ranj063

Instead of https://github.com/thesofproject/sof/pull/6030/commits/c9dccff195c16f533b481f25c7d2d397bbbba253 the following diff:

diff --git a/src/arch/xtensa/lib/cpu.c b/src/arch/xtensa/lib/cpu.c
index 1802384fb..fa9bce42e 100644
--- a/src/arch/xtensa/lib/cpu.c
+++ b/src/arch/xtensa/lib/cpu.c
@@ -70,12 +70,16 @@ static void unpack_dynamic_vectors(void)
 }
 #endif
 
+bool cpu_power_down[CONFIG_CORE_COUNT];
+
 int arch_cpu_enable_core(int id)
 {
 	struct idc_msg power_up = {
 		IDC_MSG_POWER_UP, IDC_MSG_POWER_UP_EXT, id };
 	int ret;
 
+	cpu_power_down[id] = false;
+
 	if (!arch_cpu_is_core_enabled(id)) {
 		/* Turn on stack memory for core */
 		pm_runtime_get(CORE_MEMORY_POW, id);
@@ -182,6 +186,8 @@ void cpu_power_down_core(uint32_t flags)
 		 */
 		platform_pm_runtime_prepare_d0ix_dis(cpu_get_id());
 	} else {
+		cpu_power_down[cpu_get_id()] = true;
+
 		idc_free(0);
 
 		schedule_free(0);
diff --git a/src/schedule/edf_schedule.c b/src/schedule/edf_schedule.c
index 4e03dea64..27bcc5ba8 100644
--- a/src/schedule/edf_schedule.c
+++ b/src/schedule/edf_schedule.c
@@ -321,9 +321,12 @@ static int scheduler_restore_edf(void *data)
 	return 0;
 }
 
+extern bool cpu_power_down[CONFIG_CORE_COUNT];
+
 static void schedule_edf(struct edf_schedule_data *edf_sch)
 {
-	interrupt_set(edf_sch->irq);
+	if (!cpu_power_down[cpu_get_id()])
+		interrupt_set(edf_sch->irq);
 }
 
 static const struct scheduler_ops schedule_edf_ops = {

also fixes the problem, and since it also only affects XTOS I'd rather use a slightly cleaned up version of this

lyakh avatar Jul 21 '22 12:07 lyakh

+bool cpu_power_down[CONFIG_CORE_COUNT]; +

Not cache coherent, Any array of elems where the elems are not multiples of cache line size are incoherent.

lgirdwood avatar Jul 21 '22 14:07 lgirdwood

@lgirdwood @lyakh I've updated the PR with Guennadi's suggested patch. Please let me know if it looks ok

ranj063 avatar Jul 21 '22 14:07 ranj063

+bool cpu_power_down[CONFIG_CORE_COUNT]; +

Not cache coherent, Any array of elems where the elems are not multiples of cache line size are incoherent.

@lgirdwood we added SHARED_DATA to the array definition, it should be cache-coherent now. Also notice, that this change is XTOS-only, these files aren't used with Zephyr, and we already use static shared arrays of per-core data in other locations, e.g.

src/lib/notifier.c:static SHARED_DATA struct notify_data notify_data_shared[CONFIG_CORE_COUNT];
src/idc/idc.c:static SHARED_DATA struct idc_payload static_payload[CONFIG_CORE_COUNT];

lyakh avatar Jul 21 '22 16:07 lyakh

@ranj063 This seems to be an abandoned PR. The other PR that I used for new #6992 was earlier with name "Take 3" before rename to current.

singalsu avatar Jan 25 '23 09:01 singalsu

Can one of the admins verify this patch?

sys-pt1s avatar Apr 06 '23 10:04 sys-pt1s