ASoC: SOF: Intel: hda: Disable L1 support during capture #1070

ranj063 · 2019-07-02T18:36:55Z

Disable L1 Support to address a known issue with Host DMA
that causes xruns during pause/release on certain plaforms.

ranj063 · 2019-07-02T18:38:03Z

@fredoh9 @keqiaozhang can I please request a test with this PR on GLK/WHL codec and nocodec modes to see if it addresses the pause/release xruns?

fredoh9 · 2019-07-03T00:03:00Z

CTI server is down, hence there is no devicetest result. I need to take a look why test result is positive. " On-device Test Completed"
I will check on GLK Ubuntu manually.

fredoh9 · 2019-07-03T00:59:12Z

I tested on GLK RT7219. pause/resume is working.
And I ran base-test suite manually on GLK, all test are passed.

Update:
I tested on GLK nocodec mode also. Same test cases are all passed.

ranj063 · 2019-07-03T01:05:53Z

thanks @fredoh9. did you verify both codec and nocodec modes?

fredoh9

bus is not used anywhere else. You should remove this together.

ranj063 · 2019-07-03T01:19:30Z

bus is not used anywhere else. You should remove this together.

good point. fixed now. Thanks!

keqiaozhang · 2019-07-03T03:11:58Z

@ranj063
Confirmed that this PR can fix thesofproject/sof#1578.
I tested it on WHL-I2S(both codec and nocodec mode), pause/release works well, I did 30 mins stress test(pause/release per 0.5 sec), no errors.

keyonjie

Is this a recent regression issue? Some doubt about having to disable L1 support.

sound/soc/sof/intel/hda-ctrl.c

ranj063 · 2019-07-03T03:20:04Z

Thanks @keqiaozhang

keyonjie

Thanks, looks good.

kv2019i

Are we sure we want to just disable this for for all platforms (how about a whitelist of platforms known to be not affected)? And have we tried adjusting the L1 exit threshold insteads (RL1ETT for RIRB and ISL1EXT for input, OSL1EXT for output)?

ranj063 · 2019-07-03T23:44:23Z

@keyonjie @kv2019i i've changed the logic to disable/enable L1 support when a capture stream is opened/closed now.

plbossart · 2019-08-23T20:34:35Z

I'm not sure those "input/output error" are all L1 related though. In fact, let me cast the spell on this PR to see if it makes difference...

Please check the check-pause-resume-capture-10 in latest CI test result of this PR, seems it does make difference on nearly all platforms. @plbossart @ranj063

Sorry, I am not trying to be disrespectful but there is no possible impact of an HDAudio configuration change on Baytrail. It's like reporting a SoundWire issue on Baytrail, it's not possible.
If the test results tell you so, then the tests are not quite right. it's a CI issue or something else that existed prior to this patch.
Besides the code has not changed in weeks, so the only change is at the firmware level, so there is another factor involved.

plbossart · 2019-08-23T20:35:40Z

I'm not sure those "input/output error" are all L1 related though. In fact, let me cast the spell on this PR to see if it makes difference...

Please check the check-pause-resume-capture-10 in latest CI test result of this PR, seems it does make difference on nearly all platforms. @plbossart @ranj063

Sorry, I am not trying to be disrespectful but there is no possible impact of an HDAudio configuration change on Baytrail. It's like reporting a SoundWire issue on Baytrail, it's not possible.
If the test results tell you so, then the tests are not quite right. it's a CI issue or something else that existed prior to this patch.
Besides the code has not changed in weeks, so the only change is at the firmware level, so there is another factor involved.

And a simple way to prove my point would be to compile SOF without HDaudio, you would still see the issues on Baytrail with this patch completely ignored.

wenqingfu · 2019-08-23T21:57:12Z

thanks for pointing out there is no HDA in BYT @plbossart . I'm confused with Up2 and assume MB has HDMI, too... It may be other issues that are causing the same "input/output error" message in aplay/arecord for BYT. In fact, in the last CI testing result, it did show BYT_5682 still fail with the same error message, but error disappear on other platforms.

plbossart · 2019-09-26T18:48:13Z

@ranj063 can you take a look at commit 49d28c3 and let me know if the rebase looks correct. if yes, can you update this PR?

This PR does solve xruns reported on ICL-U with SoundWire, at least on my board.

ranj063 · 2019-09-26T19:12:21Z

@ranj063 can you take a look at commit 49d28c3 and let me know if the rebase looks correct. if yes, can you update this PR?

@plbossart yes, this looks good. I will update the PR to add a kernel module param to enable/disable thes feature and update the PR today. Thanks!

plbossart · 2019-09-27T13:19:08Z

Fixes #1222

ranj063 · 2019-09-27T16:43:50Z

@plbossart updated. I've also added the config option to disable DMI in the sof-defconfig fragment.

plbossart

see below

plbossart · 2019-09-27T16:50:51Z

sound/soc/sof/intel/hda-stream.c

+	 * Workaround to address a known issue with host DMA that results
+	 * in xruns during pause/release in capture scenarios.
+	 */
+	if (IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_DISABLE_DMI_L1))


that doesn't seem quite right to me.

This is not a debug option. You want DMI_L1 disabled by default, UNLESS the user chooses to enable DMI_L1 to e.g. look into power savings.

And this should be an Intel-specific Kconfig.

@plbossart do you mean to say that we disable DMI L1 altogether by default irrespective of whether there is capture or not?

@plbossart I've moved the config to intel Kconfig now. But Im not sure if we should disable DMI L1 altogether. The recommendation from Lech is to only disable it during capture.

plbossart

Comments below. The point is to avoid confusions whether the Kconfig is needed or not.

'Disable L1 DMI' with a default no is a double negative.

plbossart · 2019-09-27T17:54:15Z

sound/soc/sof/intel/Kconfig


+config SND_SOC_SOF_HDA_DISABLE_DMI_L1
+	bool "SOF disable DMI Link L1"
+	depends on SND_SOC_SOF_HDA_COMMON


not needed, you are already in the if SND_SOC_SOF_HDA_COMMON

and when you make it a positive statement, you can do

config SND_SOC_SOF_HDA_ALWAYS_ENABLE_DMI_L1
bool "SOF HDAudio always enable DMI Link L1"
help
this option will enable DMI L1 for both playback and capture, and disable known workarounds for specific HDaudio platforms. Only use to look into power optimizations on platforms not affected by DMI L1 issues. This option is not recommended.

plbossart · 2019-09-27T18:00:37Z

sound/soc/sof/intel/hda-stream.c

+	 * Workaround to address a known issue with host DMA that results
+	 * in xruns during pause/release in capture scenarios.
+	 */
+	if (IS_ENABLED(SND_SOC_SOF_HDA_DISABLE_DMI_L1))


can we make this if (!IS_ENABLED(SND_SOC_SOF_HDA_ALWAYS_ENABLE_DMI_L1)) ?

so that if the option is not selected by default we get the workaround. You have to explicit select the option to remove the intelligence and enable DM1_L1 for both playback and capture.

There is a known issue on some Intel platforms which causes pause/release to run into xrun's during capture usecases. The suggested workaround to address the issue is to disable the entry of lower power L1 state in the physical DMI link when there is a capture stream open. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

plbossart

thanks @ranj063

ranj063 requested review from lgirdwood and plbossart as code owners July 2, 2019 18:36

ranj063 force-pushed the fix/pause-xrun branch from 6886ceb to 8d39da9 Compare July 2, 2019 18:39

ranj063 requested review from keyonjie, kv2019i and lyakh July 2, 2019 18:57

fredoh9 reviewed Jul 3, 2019

View reviewed changes

ranj063 force-pushed the fix/pause-xrun branch from 8d39da9 to 612ef5a Compare July 3, 2019 01:19

fredoh9 previously approved these changes Jul 3, 2019

View reviewed changes

keyonjie reviewed Jul 3, 2019

View reviewed changes

sound/soc/sof/intel/hda-ctrl.c Show resolved Hide resolved

sound/soc/sof/intel/hda-ctrl.c Outdated Show resolved Hide resolved

ranj063 dismissed fredoh9’s stale review via a0746eb July 3, 2019 03:26

ranj063 force-pushed the fix/pause-xrun branch 2 times, most recently from a0746eb to 4a35044 Compare July 3, 2019 03:32

keyonjie self-requested a review July 3, 2019 04:29

keyonjie previously approved these changes Jul 3, 2019

View reviewed changes

kv2019i requested changes Jul 3, 2019

View reviewed changes

ranj063 dismissed keyonjie’s stale review via a52602b July 3, 2019 23:39

ranj063 force-pushed the fix/pause-xrun branch from 4a35044 to a52602b Compare July 3, 2019 23:39

ranj063 changed the title ~~ASoC: SOF: Intel: hda: Disable L1 support~~ ASoC: SOF: Intel: hda: Disable L1 support during capture Jul 3, 2019

ranj063 force-pushed the fix/pause-xrun branch from a52602b to 310e8b0 Compare July 3, 2019 23:42

ranj063 requested review from fredoh9 and removed request for fredoh9 July 3, 2019 23:46

plbossart mentioned this pull request Sep 26, 2019

[BUG][SDW]Arecord shows I/O error after pause and resume. #1222

Closed

ranj063 force-pushed the fix/pause-xrun branch from f9b82a3 to 0ca7c94 Compare September 27, 2019 16:35

ranj063 requested a review from libinyang as a code owner September 27, 2019 16:35

plbossart requested changes Sep 27, 2019

View reviewed changes

ranj063 force-pushed the fix/pause-xrun branch 5 times, most recently from 338cfc4 to d0102c7 Compare September 27, 2019 17:42

plbossart requested changes Sep 27, 2019

View reviewed changes

ranj063 force-pushed the fix/pause-xrun branch from d0102c7 to f648e8a Compare September 27, 2019 18:15

plbossart approved these changes Sep 27, 2019

View reviewed changes

plbossart added CFL Applies to Coffee Lake platform CML Applies to Comet Lake platform CNL Applies to Cannonlake GLK Applies to Gemini Lake ICL Applies to Icelake platform and removed Unclear No agreement on problem statement and resolution labels Sep 27, 2019

plbossart merged commit c5a665d into thesofproject:topic/sof-dev Sep 27, 2019

wenqingfu mentioned this pull request Oct 2, 2019

[BUG][cAVS 1.8][Capture] Pause-release overrun thesofproject/sof#1760

Closed

ranj063 deleted the fix/pause-xrun branch October 3, 2019 19:56

aiChaoSONG mentioned this pull request Nov 29, 2019

[BUG][CML-007] ipc error in dmic pause-release test thesofproject/sof#2092

Closed

ASoC: SOF: Intel: hda: Disable L1 support during capture #1070

ASoC: SOF: Intel: hda: Disable L1 support during capture #1070

Uh oh!

Conversation

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

Uh oh!

Uh oh!

ranj063 commented Jul 2, 2019

Uh oh!

fredoh9 commented Jul 3, 2019

Uh oh!

fredoh9 commented Jul 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ranj063 commented Jul 3, 2019

Uh oh!

fredoh9 left a comment

Choose a reason for hiding this comment

Uh oh!

ranj063 commented Jul 3, 2019

Uh oh!

keqiaozhang commented Jul 3, 2019

Uh oh!

keyonjie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ranj063 commented Jul 3, 2019

Uh oh!

keyonjie left a comment

Choose a reason for hiding this comment

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

ranj063 commented Jul 3, 2019

Uh oh!

plbossart commented Aug 23, 2019

Uh oh!

plbossart commented Aug 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wenqingfu commented Aug 23, 2019

Uh oh!

plbossart commented Sep 26, 2019

Uh oh!

ranj063 commented Sep 26, 2019

Uh oh!

plbossart commented Sep 27, 2019

Uh oh!

ranj063 commented Sep 27, 2019

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart Sep 27, 2019

Choose a reason for hiding this comment

Uh oh!

ranj063 Sep 27, 2019

Choose a reason for hiding this comment

Uh oh!

ranj063 Sep 27, 2019

Choose a reason for hiding this comment

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

plbossart Sep 27, 2019

Choose a reason for hiding this comment

Uh oh!

plbossart Sep 27, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plbossart Sep 27, 2019

Choose a reason for hiding this comment

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

ranj063 commented Jul 2, 2019 •

edited

Loading

fredoh9 commented Jul 3, 2019 •

edited

Loading

plbossart commented Aug 23, 2019 •

edited

Loading

plbossart Sep 27, 2019 •

edited

Loading