Skip to content

ASoC:SOF:fix the NOCODEC and HDA mutually incompatible - #739

Merged
plbossart merged 1 commit into
thesofproject:topic/sof-devfrom
zhuyingjiang:topic/sof-dev-fix-hda-nocodec-incompatible
Mar 28, 2019
Merged

ASoC:SOF:fix the NOCODEC and HDA mutually incompatible#739
plbossart merged 1 commit into
thesofproject:topic/sof-devfrom
zhuyingjiang:topic/sof-dev-fix-hda-nocodec-incompatible

Conversation

@zhuyingjiang

@zhuyingjiang zhuyingjiang commented Mar 21, 2019

Copy link
Copy Markdown

for make NOCODEC and HDA mutually incompatible, the DSP
common control part is still needed for NOCODEC to set
the DSP registers.
This fix #1072 #1073
Signed-off-by: Zhu Yingjiang yingjiang.zhu@linux.intel.com

@zhuyingjiang zhuyingjiang mentioned this pull request Mar 21, 2019
Closed
plbossart
plbossart requested changes Mar 21, 2019
View reviewed changes

@plbossart plbossart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @zhuyingjiang I don't understand what you are trying to fix, there is no link to any issue and the fix is wrong anyways. We can't depend on the hdac library when the HDaudio links are not required.
NAK.

Comment thread sound/soc/sof/intel/hda-dsp.c Outdated
@wenqingfu

wenqingfu commented Mar 21, 2019

Copy link
Copy Markdown

Making hdac library dependency optional is a feature, not a bug, for this project. @zhuyingjiang It sounds twisted but you need to find another way to fix the logic why NOCODEC depends on HDA. If needed, please open a discussion Issue for idea contribution.

@ranj063

ranj063 commented Mar 21, 2019

Copy link
Copy Markdown
Collaborator

@zhuyingjiang we dont seem to have any issues at first boot without any dependency with the hdac library. So I am guessing we're probably missing programming certain registers coming back from S3.

@zhuyingjiang

zhuyingjiang commented Mar 22, 2019

Copy link
Copy Markdown
Author

@plbossart There are two issues #1072 and #1073 as described in the commit message. As I debug on Up2, they are the same root cause by commit 9a5e2e4ff4 and 7b12f064.
It means in our current code, and in NOCODEC mode, when suspend to S3, the DSP will not be powerup during resume.

[40557.134200] sof-audio-pci 0000:00:0e.0: error: timeout on core powerup

but as @ranj063 said, and I tested on Up2, there is no problem when we only sleep to S1 by rtcwake -s 2 -m freeze, and there is no problem when the DSP do the first suspend/resume during the kernel startup. so still should be a PM bug.
I guess it should be that we have done more during suspend to S3 or done less during resume from it.
In principle, we can use HOST DMA and DSP related registers in NOCODEC mode, and exclude HDA link and HDA codec related, right?
I will update the PR to do this without hda ext library.

@zhuyingjiang

zhuyingjiang commented Mar 22, 2019 •
edited by RanderWang
Loading

Copy link
Copy Markdown
Author

Making hdac library dependency optional is a feature, not a bug, for this project. @zhuyingjiang It sounds twisted but you need to find another way to fix the logic why NOCODEC depends on HDA. If needed, please open a discussion Issue for idea contribution.

@wenqingfu As I talked with @RanderWang, in NOCODEC mode, we would use the HDA Host DMA and DSP control part, but should not use HDA link DMA and HDA codec part. I will update the PR.

@RanderWang

RanderWang commented Mar 22, 2019

Copy link
Copy Markdown

@zhuyingjiang we dont seem to have any issues at first boot without any dependency with the hdac library. So I am guessing we're probably missing programming certain registers coming back from S3.

yes, I agree with your idea

@zhuyingjiang
zhuyingjiang force-pushed the topic/sof-dev-fix-hda-nocodec-incompatible branch from 67c04d8 to 6c574ef Compare March 27, 2019 08:59
@zhuyingjiang

zhuyingjiang commented Mar 27, 2019

Copy link
Copy Markdown
Author

@plbossart To make the DSP suspend/resume during the system ACPI PM flow(via state_store and pm_suspend), some DSP registers as INCTL and PPCTL is needed to be set. I re-programming-ed the snd_hdac_ext_bus_ppcap_enable, snd_hdac_ext_bus_ppcap_int_enable and azx_int_enable as these should be the common DSP control part for both HDA and NOCODEC.
Could you help check if this is the proper way to fix #1702 and #1703?

plbossart
plbossart requested changes Mar 27, 2019
View reviewed changes

@plbossart plbossart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhuyingjiang the sequences are quite different and difficult to review, also I wonder if you got the point about the hdac library, see below.

Comment thread sound/soc/sof/intel/hda-dsp.c Outdated

@plbossart plbossart Mar 27, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this removed?

@zhuyingjiang zhuyingjiang Mar 28, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, changed, thanks!

Comment thread sound/soc/sof/intel/hda-dsp.c Outdated
@zhuyingjiang
zhuyingjiang force-pushed the topic/sof-dev-fix-hda-nocodec-incompatible branch from 6c574ef to 7a14dae Compare March 28, 2019 05:29
@zhuyingjiang

zhuyingjiang commented Mar 28, 2019

Copy link
Copy Markdown
Author

@plbossart Updated, keep the HDA part with HDAC lib.

ranj063
ranj063 reviewed Mar 28, 2019
View reviewed changes
Comment thread sound/soc/sof/intel/hda-dsp.c Outdated

@ranj063 ranj063 Mar 28, 2019

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhuyingjiang can you please add a comment just like in the suspend case?

@zhuyingjiang
ASoC:SOF:fix the NOCODEC and HDA mutually incompatible
60a9d0d 
to make NOCODEC and HDA mutually incompatible, the DSP
common control part registers are needed to be set for
NOCODEC mode. At least INTCTL and PP_PPCTL are needed
to be set, or else the DSP will not powerup during
suspend/resume.

Signed-off-by: Zhu Yingjiang <yingjiang.zhu@linux.intel.com>
@zhuyingjiang
zhuyingjiang force-pushed the topic/sof-dev-fix-hda-nocodec-incompatible branch from 7a14dae to 60a9d0d Compare March 28, 2019 06:08
plbossart
plbossart approved these changes Mar 28, 2019
View reviewed changes

@plbossart plbossart left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @zhuyingjiang. I will wait for @ranj063 to approve before merging.

@plbossart
plbossart merged commit a84e3db into thesofproject:topic/sof-dev Mar 28, 2019
oder-chiou pushed a commit to oder-chiou/linux-soundwire that referenced this pull request May 5, 2026
@qiangzh3 @akpm00
kasan: fix lockdep report invalid wait context
be41d81 
For kernels built with the following options and booting

CONFIG_SLUB=y
CONFIG_DEBUG_LOCKDEP=y
CONFIG_PROVE_LOCKING=y
CONFIG_PROVE_RAW_LOCK_NESTING=y

[    0.523115] [ BUG: Invalid wait context ]
[    0.523315] 6.3.0-rc1-yocto-standard+ thesofproject#739 Not tainted
[    0.523649] -----------------------------
[    0.523663] swapper/0/0 is trying to lock:
[    0.523663] ffff888035611360 (&c->lock){....}-{3:3}, at: put_cpu_partial+0x2e/0x1e0
[    0.523663] other info that might help us debug this:
[    0.523663] context-{2:2}
[    0.523663] no locks held by swapper/0/0.
[    0.523663] stack backtrace:
[    0.523663] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.3.0-rc1-yocto-standard+ thesofproject#739
[    0.523663] Call Trace:
[    0.523663]  <IRQ>
[    0.523663]  dump_stack_lvl+0x64/0xb0
[    0.523663]  dump_stack+0x10/0x20
[    0.523663]  __lock_acquire+0x6c4/0x3c10
[    0.523663]  lock_acquire+0x188/0x460
[    0.523663]  put_cpu_partial+0x5a/0x1e0
[    0.523663]  __slab_free+0x39a/0x520
[    0.523663]  ___cache_free+0xa9/0xc0
[    0.523663]  qlist_free_all+0x7a/0x160
[    0.523663]  per_cpu_remove_cache+0x5c/0x70
[    0.523663]  __flush_smp_call_function_queue+0xfc/0x330
[    0.523663]  generic_smp_call_function_single_interrupt+0x13/0x20
[    0.523663]  __sysvec_call_function+0x86/0x2e0
[    0.523663]  sysvec_call_function+0x73/0x90
[    0.523663]  </IRQ>
[    0.523663]  <TASK>
[    0.523663]  asm_sysvec_call_function+0x1b/0x20
[    0.523663] RIP: 0010:default_idle+0x13/0x20
[    0.523663] RSP: 0000:ffffffff83e07dc0 EFLAGS: 00000246
[    0.523663] RAX: 0000000000000000 RBX: ffffffff83e1e200 RCX: ffffffff82a83293
[    0.523663] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff8119a6b1
[    0.523663] RBP: ffffffff83e07dc8 R08: 0000000000000001 R09: ffffed1006ac0d66
[    0.523663] R10: ffff888035606b2b R11: ffffed1006ac0d65 R12: 0000000000000000
[    0.523663] R13: ffffffff83e1e200 R14: ffffffff84a7d980 R15: 0000000000000000
[    0.523663]  default_idle_call+0x6c/0xa0
[    0.523663]  do_idle+0x2e1/0x330
[    0.523663]  cpu_startup_entry+0x20/0x30
[    0.523663]  rest_init+0x152/0x240
[    0.523663]  arch_call_rest_init+0x13/0x40
[    0.523663]  start_kernel+0x331/0x470
[    0.523663]  x86_64_start_reservations+0x18/0x40
[    0.523663]  x86_64_start_kernel+0xbb/0x120
[    0.523663]  secondary_startup_64_no_verify+0xe0/0xeb
[    0.523663]  </TASK>

The local_lock_irqsave() is invoked in put_cpu_partial() and happens in
IPI context, due to the CONFIG_PROVE_RAW_LOCK_NESTING=y (the
LD_WAIT_CONFIG not equal to LD_WAIT_SPIN), so acquire local_lock in IPI
context will trigger above calltrace.

This commit therefore moves qlist_free_all() from hard-irq context to task
context.

Link: https://lkml.kernel.org/r/20230327120019.1027640-1-qiang1.zhang@intel.com
Signed-off-by: Zqiang <qiang1.zhang@intel.com>
Acked-by: Marco Elver <elver@google.com>
Cc: Alexander Potapenko <glider@google.com>
Cc: Andrey Konovalov <andreyknvl@gmail.com>
Cc: Andrey Ryabinin <ryabinin.a.a@gmail.com>
Cc: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@plbossart plbossart plbossart approved these changes
@ranj063 ranj063 ranj063 approved these changes
@lgirdwood lgirdwood Awaiting requested review from lgirdwood

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[S3] Trying resume playback failed after suspend/resume on APL-nocodec

5 participants

@zhuyingjiang @wenqingfu @ranj063 @RanderWang @plbossart