[RFC]ASoC: SOF: acpi: enable PM for ACPI platforms #1295

ranj063 · 2019-10-09T17:56:55Z

This patch adds the platform-specific PM callbacks for
the BYT/CHT ACPI platforms. Since the Minnowboards
do not support PM, the SOF device is prevented from
entering runtime suspend by incrementing the ref count
after the topology is loaded.

This doesnt compile yet. Looking for comments.

sound/soc/sof/core.c

plbossart

see comments below.

sound/soc/sof/sof-acpi-dev.c

plbossart · 2019-10-09T19:27:07Z

sound/soc/sof/pcm.c

-	 * prevent the device from entering runtime suspend.
+	 * Some boards such as the Minnowboard do not support PM.
+	 * Increment the usage count so as to prevent the device
+	 * from entering runtime suspend.


For MinnowBoards, isn't it suspend-resume that's broken for audio?
if you use rtcwake -m mem -s 3, would audio work when you resume?

If not, then pm_runtime doesn't help.

@plbossart if I remember correctly, on the minnowboards, not holding the ref count to prevent runtime PM somehow ends up in the DSP getting powered off. I might have to test this again though.

sound/soc/sof/sof-acpi-dev.c

sound/soc/sof/intel/byt.c

cujomalainey · 2019-10-09T21:45:42Z

Looks like firmware was not loaded again after suspend/resume cycle

[  102.344607] sof-audio-acpi 808622A8:00: generating page table for 0000000000b3ed3a size 0xffc0 pages 16
[  102.344611] sof-audio-acpi 808622A8:00: stream_tag 0
[  102.344621] sof-audio-acpi 808622A8:00: ipc tx: 0x60010000
[  102.845378] sof-audio-acpi 808622A8:00: error: ipc timed out for 0x60010000 size 108
[  102.845455] sof-audio-acpi 808622A8:00: invalid header size 0x1120c9c. FW oops is bogus
[  102.845475] sof-audio-acpi 808622A8:00: error: unexpected fault 0x00000000 trace 0x00000000
[  102.845495] sof-audio-acpi 808622A8:00: error: ipc host -> DSP: pending no complete no raw 0x00000000
[  102.845513] sof-audio-acpi 808622A8:00: error: mask host: pending no complete no raw 0x00000000
[  102.845529] sof-audio-acpi 808622A8:00: error: ipc DSP -> host: pending no complete no raw 0x00000000
[  102.845546] sof-audio-acpi 808622A8:00: error: mask DSP: pending no complete no raw 0x00000000
[  102.845564] sof-audio-acpi 808622A8:00: error: hw params ipc failed for stream 0
[  102.845586] sof-audio-acpi 808622A8:00: ASoC: 808622A8:00 hw params failed: -110
[  102.845608]  Low Latency: ASoC: hw_params FE failed -110

cujomalainey · 2019-10-10T00:26:17Z

Changes pass suspend/resume on cht (Cyan) :)

cujomalainey · 2019-10-10T00:30:02Z

for github
fixes https://github.com/thesofproject/sof/issues/1898

This patch adds the platform-specific PM callbacks for the BYT/CHT ACPI platforms. Since the Minnowboards do not support PM, the SOF device is prevented from entering runtime suspend by incrementing the ref count after the topology is loaded. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

plbossart

suggested changes below to make the code even better

plbossart · 2019-10-10T02:25:03Z

sound/soc/sof/pm.c

+	if (!runtime_suspend && !sof_ops(sdev)->suspend)
+		return 0;
+
+	if (runtime_suspend && !sof_ops(sdev)->runtime_suspend)


should be a separate patch, no?

plbossart · 2019-10-10T02:26:49Z

sound/soc/sof/sof-acpi-dev.c

+#if IS_ENABLED(CONFIG_SND_SOC_SOF_BAYTRAIL)
+	/* set driver data in the quirk table */
+	for (i = 0; i < ARRAY_SIZE(minnowbard_quirk_table); i++)
+		minnowbard_quirk_table[i].driver_data = dev;


this is a bit ugly, no? You could just check the quirk, which sets a variable and then you test said variable.

and you can also use the sof-acpi-debug kernel parameter to override the DMI and also disable PM!

plbossart · 2019-10-10T02:27:58Z

sound/soc/sof/intel/byt.c


+	/* PM */
+	.suspend = byt_suspend,
+	.resume = byt_resume,


is this the same routine for basic PM and runtime PM?

ranj063 · 2019-10-10T16:20:44Z

@plbossart @cujomalainey im ooo today due to the power cut. Will address comments tomorrow

plbossart · 2019-10-10T19:14:28Z

sound/soc/sof/sof-acpi-dev.c

+	 * Increment the usage count so as to prevent the device
+	 * from entering runtime suspend.
+	 */
+	pm_runtime_get_noresume(dev);


btw what happens if instead of taking a fake reference you just don't enable pm_runtime?

@plbossart this is what I want to do ideally. But the one test that fails with this change is the ipc_flood_test(). When this test runs, we call pm_runtime_get_sync() to resume the DSP if it is suspended and this throws an error is runtime_pm is not enabled.
So if I add a check to see if runtime PM is enabled before calling get_sync(), then it would solve the problem. And of course, every other place we add a pm_runtime_get_sync() would need to the same too. What do you think?

plbossart · 2019-10-10T20:04:05Z

The quirks crap out on my Asus T100 TA

[   52.622026] general protection fault: 0000 [#1] SMP PTI
[   52.622034] CPU: 1 PID: 157 Comm: kworker/1:2 Not tainted 5.4.0-rc1-test-test+ #142
[   52.622037] Hardware name: ASUSTeK COMPUTER INC. T100TA/T100TA, BIOS T100TA.304 03/14/2014
[   52.622047] Workqueue: events sof_probe_work [snd_sof]
[   52.622057] RIP: 0010:strstr+0x19/0x70
[   52.622061] Code: f8 75 f0 c3 48 89 f8 c3 66 0f 1f 84 00 00 00 00 00 80 3e 00 74 5d 48 89 f1 48 83 c1 01 80 39 00 75 f7 48 89 f8 48 29 f1 74 48 <80> 3f 00 74 41 48 83 c0 01 80 38 00 75 f7 48 29 f8 48 89 c2 48 39
[   52.622064] RSP: 0018:ffffb641802afdc8 EFLAGS: 00010202
[   52.622068] RAX: 00270013795f1010 RBX: ffffffffc05a1370 RCX: 0000000000000010
[   52.622071] RDX: 0000000000000002 RSI: ffffffffc05a1381 RDI: 00270013795f1010
[   52.622073] RBP: 00000000fffffffe R08: 0000000000000041 R09: 0000000000000004
[   52.622076] R10: 00000012245d6a52 R11: 0000000000000001 R12: 0000000000000000
[   52.622079] R13: 0000000000000000 R14: ffffffffc05a1381 R15: 0ffff9864f66ab10
[   52.622082] FS:  0000000000000000(0000) GS:ffff9864f6680000(0000) knlGS:0000000000000000
[   52.622085] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   52.622088] CR2: 00007f90a84a0088 CR3: 0000000036fa2000 CR4: 00000000001006e0
[   52.622090] Call Trace:
[   52.622101]  dmi_matches+0x74/0xd0
[   52.622108]  dmi_check_system+0x15/0x50
[   52.622114]  sof_probe_work+0x18b/0x1d0 [snd_sof]
[   52.622123]  process_one_work+0x1f5/0x3b0
[   52.622128]  worker_thread+0x28/0x3c0
[   52.622133]  ? process_one_work+0x3b0/0x3b0
[   52.622137]  kthread+0x10d/0x130
[   52.622142]  ? kthread_park+0x80/0x80
[   52.622148]  ret_from_fork+0x35/0x40
[   52.622152] Modules linked in: snd_sof_acpi snd_sof_intel_byt snd_soc_acpi_intel_match snd_sof_intel_bdw snd_sof_intel_ipc snd_sof snd_sof_xtensa_dsp snd_soc_acpi ax88179_178a usbnet snd_soc_rt5670 snd_soc_rt5651 snd_soc_rt5645 snd_soc_rt5640 intel_soc_dts_thermal snd_seq_midi snd_seq_midi_event snd_soc_rl6231 intel_powerclamp snd_rawmidi snd_soc_core snd_pcm snd_seq mei_txe mei snd_seq_device snd_timer snd int3400_thermal processor_thermal_device int3403_thermal intel_soc_dts_iosf int340x_thermal_zone acpi_thermal_rel int3406_thermal soundcore efivarfs mmc_block i915 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops xhci_pci drm xhci_hcd sdhci_acpi sdhci
[   52.622217] ---[ end trace e985fde876d178dc ]---

plbossart · 2019-10-10T20:14:21Z

@ranj063 you need to apply this to your quirk table

diff --git a/sound/soc/sof/sof-acpi-dev.c b/sound/soc/sof/sof-acpi-dev.c
index 9eae05aebeca..ab948c1b12b6 100644
--- a/sound/soc/sof/sof-acpi-dev.c
+++ b/sound/soc/sof/sof-acpi-dev.c
@@ -149,6 +149,7 @@ static struct dmi_system_id minnowbard_quirk_table[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "Minnowboard Turbot"),
 		},
 	},
+	{}
 };
 
 #endif

plbossart · 2019-10-10T20:43:23Z

It's a complete fail on Asus T100 TA. with this one patch, I get an IPC error

speaker-test -Dhw:0,0 -c2 -r48000 -t sine

speaker-test 1.1.0

Playback device is hw:0,0
Stream parameters are 48000Hz, S16_LE, 2 channels
Sine wave rate is 440.0000Hz
Rate set to 48000Hz (requested 48000Hz)
Buffer size range from 96 to 16368
Period size range from 48 to 4080
Using max buffer size 16368
Periods = 4
Unable to set hw params for playback: Connection timed out
Setting of hwparams failed: Connection timed out

[   37.948073] sof-audio-acpi 80860F28:00: ipc tx: 0x60010000: GLB_STREAM_MSG: PCM_PARAMS
[   38.448962] sof-audio-acpi 80860F28:00: error: ipc timed out for 0x60010000 size 108
[   38.448984] sof-audio-acpi 80860F28:00: info: preventing DSP entering D3 state to preserve context
[   38.449022] sof-audio-acpi 80860F28:00: invalid header size 0x7f6ffdef. FW oops is bogus
[   38.449038] sof-audio-acpi 80860F28:00: error: unexpected fault 0x00000000 trace 0x00000000
[   38.449055] sof-audio-acpi 80860F28:00: error: ipc host -> DSP: pending no complete no raw 0x00000000
[   38.449069] sof-audio-acpi 80860F28:00: error: mask host: pending no complete no raw 0x00000000
[   38.449082] sof-audio-acpi 80860F28:00: error: ipc DSP -> host: pending no complete no raw 0x00000000
[   38.449096] sof-audio-acpi 80860F28:00: error: mask DSP: pending no complete no raw 0x00000000
[   38.449108] sof-audio-acpi 80860F28:00: error: waking up any trace sleepers
[   38.449124] sof-audio-acpi 80860F28:00: error: hw params ipc failed for stream 0
[   38.449139] sof-audio-acpi 80860F28:00: ASoC: 80860F28:00 hw params failed: -110
[   38.449157]  Low Latency: ASoC: hw_params FE failed -110
[   38.449187] sof-audio-acpi 80860F28:00: pcm: free stream 0 dir 0
[   38.449590] sof-audio-acpi 80860F28:00: pcm: close stream 0 dir 0

So my guess is that for an ACPI device, you must be using some ACPI power helpers when entering D3, or the setup is not correct.
The ACPI device still shows with an 'unsupported' state even though you've enabled pm_runtime. Something's wrong really.

root@plb-T00TA:/sys/bus/acpi/devices/80860F28:00# more power/runtime_status 
unsupported

plbossart · 2019-10-10T20:55:08Z

on the positive side of things, the legacy baytrail driver (byt-rt5640), not even the Atom/SST stuff, seems to support S3 suspend/resume while doing audio playback, so there's light at the end of the tunnel.

ranj063 · 2019-10-10T23:17:27Z

It's a complete fail on Asus T100 TA. with this one patch, I get an IPC error

@plbossart this looks like the DSP is in D3 and is not resumed when playback is started. Could you please send me the whole dmesg log after you close and open the lid?

juimonen · 2019-10-21T10:40:47Z

@ranj063 should this work on byt-mb? what's the procedure to test it, I would try it out...

ranj063 · 2019-11-20T19:31:47Z

closing. will be replaced soon

cujomalainey · 2019-11-22T20:22:50Z

@ranj063 can you link the new PR here once it is pushed?

ranj063 · 2019-11-22T20:24:17Z

@cujomalainey this is the new PR #1523 but it is still WIP as there're some fw changes needed to go with it

ranj063 requested review from RanderWang, juimonen, keyonjie, lgirdwood, lyakh and plbossart as code owners October 9, 2019 17:56

ranj063 requested a review from cujomalainey October 9, 2019 17:58

ranj063 force-pushed the fix/byt_pm branch 2 times, most recently from 9b68959 to ee56304 Compare October 9, 2019 18:00

ranj063 changed the title ~~ASoC: SOF: acpi: enable PM for ACPI platforms~~ [RFC]ASoC: SOF: acpi: enable PM for ACPI platforms Oct 9, 2019

ranj063 force-pushed the fix/byt_pm branch from ee56304 to c371c54 Compare October 9, 2019 18:10

ranj063 closed this Oct 9, 2019

ranj063 reopened this Oct 9, 2019

ranj063 force-pushed the fix/byt_pm branch from c371c54 to f6b7cc3 Compare October 9, 2019 18:14

ranj063 requested a review from dbaluta as a code owner October 9, 2019 18:14

dbaluta reviewed Oct 9, 2019

View reviewed changes

sound/soc/sof/core.c Outdated Show resolved Hide resolved

plbossart reviewed Oct 9, 2019

View reviewed changes

cujomalainey suggested changes Oct 9, 2019

View reviewed changes

sound/soc/sof/intel/byt.c Outdated Show resolved Hide resolved

cujomalainey suggested changes Oct 9, 2019

View reviewed changes

sound/soc/sof/intel/byt.c Outdated Show resolved Hide resolved

sound/soc/sof/intel/byt.c Outdated Show resolved Hide resolved

ranj063 force-pushed the fix/byt_pm branch 2 times, most recently from 4a4c456 to ca34874 Compare October 9, 2019 20:51

ranj063 force-pushed the fix/byt_pm branch 3 times, most recently from 9b612fe to d4f71b9 Compare October 10, 2019 00:16

ranj063 force-pushed the fix/byt_pm branch from d4f71b9 to 2fef4fd Compare October 10, 2019 00:46

plbossart requested changes Oct 10, 2019

View reviewed changes

plbossart reviewed Oct 10, 2019

View reviewed changes

plbossart closed this Nov 21, 2019

ranj063 deleted the fix/byt_pm branch February 13, 2022 05:11

[RFC]ASoC: SOF: acpi: enable PM for ACPI platforms #1295

[RFC]ASoC: SOF: acpi: enable PM for ACPI platforms #1295

Uh oh!

Conversation

ranj063 commented Oct 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cujomalainey commented Oct 9, 2019

Uh oh!

cujomalainey commented Oct 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cujomalainey commented Oct 10, 2019

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ranj063 commented Oct 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plbossart commented Oct 10, 2019

Uh oh!

plbossart commented Oct 10, 2019

Uh oh!

plbossart commented Oct 10, 2019

Uh oh!

plbossart commented Oct 10, 2019

Uh oh!

ranj063 commented Oct 10, 2019

Uh oh!

juimonen commented Oct 21, 2019

Uh oh!

ranj063 commented Nov 20, 2019

Uh oh!

cujomalainey commented Nov 22, 2019

Uh oh!

ranj063 commented Nov 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ranj063 commented Oct 9, 2019 •

edited

Loading

cujomalainey commented Oct 10, 2019 •

edited

Loading