Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SoundWire: rt5682: mutex problem #4371

Closed
plbossart opened this issue May 23, 2023 · 7 comments · Fixed by #4372
Closed

SoundWire: rt5682: mutex problem #4371

plbossart opened this issue May 23, 2023 · 7 comments · Fixed by #4372

Comments

@plbossart
Copy link
Member

Intel daily test: result/planresultdetail/26265?model=ADLP_BRYA_SDW&testcase=check-playback-all-formats

[  281.494003] kernel: soundwire_bus:sdw_modify_slave_status: rt5682 sdw:0:025d:5682:00: signaling enumeration completion for Slave 6
[  281.787915] kernel: BUG: sleeping function called from invalid context at kernel/locking/mutex.c:580
[  281.787922] kernel: in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 7, name: kworker/0:0
[  281.787923] kernel: preempt_count: 0, expected: 0
[  281.787924] kernel: RCU nest depth: 1, expected: 0
[  281.787925] kernel: 5 locks held by kworker/0:0/7:
[  281.787927] kernel:  #0: ffff8e3c00068748 ((wq_completion)events){....}-{0:0}, at: process_one_work+0x206/0x550
[  281.787934] kernel:  #1: ffffb23400053e70 ((work_completion)(&cdns->work)){....}-{0:0}, at: process_one_work+0x206/0x550
[  281.787938] kernel:  #2: ffff8e3c01155c48 (&slave->sdw_dev_lock){....}-{3:3}, at: sdw_update_slave_status+0x26/0x70 [soundwire_bus]
[  281.787949] kernel:  #3: ffff8e3c4b509870 (rt5682_sdw:316:(&rt5682_sdw_indirect_regmap)->lock){....}-{3:3}, at: regcache_sync+0x42/0x2c0
[  281.787956] kernel:  #4: ffffffffa1366880 (rcu_read_lock){....}-{1:2}, at: regcache_maple_sync+0x9/0x170
[  281.787960] kernel: CPU: 0 PID: 7 Comm: kworker/0:0 Tainted: G     U             6.4.0-rc1-daily-default-20230523-g91774c9b5d56 #dev
[  281.787962] kernel: Hardware name: Google Brya/Brya, BIOS  12/24/2021
[  281.787964] kernel: Workqueue: events cdns_update_slave_status_work [soundwire_cadence]
[  281.787971] kernel: Call Trace:
[  281.787974] kernel:  
[  281.787976] kernel:  dump_stack_lvl+0x37/0x50
[  281.787980] kernel:  __might_resched+0x12f/0x180
[  281.787984] kernel:  __mutex_lock+0x55/0xd10
[  281.787987] kernel:  ? __mutex_unlock_slowpath+0x45/0x280
[  281.787990] kernel:  ? regmap_write+0x32/0x70
[  281.787993] kernel:  ? sdw_ntransfer_no_pm+0xc8/0x110 [soundwire_bus]
[  281.787999] kernel:  ? local_clock+0xd/0xd0
[  281.788002] kernel:  ? regmap_write+0x32/0x70
[  281.788004] kernel:  ? __lock_acquire.constprop.0+0x135/0x6b0
[  281.788006] kernel:  regmap_write+0x32/0x70
[  281.788010] kernel:  rt5682_sdw_write+0x29/0x80 [snd_soc_rt5682_sdw]
[  281.788015] kernel:  _regmap_write+0x56/0x140
[  281.788018] kernel:  regcache_sync_val+0x48/0x130
[  281.788021] kernel:  regcache_maple_sync+0x10a/0x170
[  281.788025] kernel:  regcache_sync+0x273/0x2c0
[  281.788028] kernel:  rt5682_update_status+0x43f/0x4d0 [snd_soc_rt5682_sdw]
[  281.788033] kernel:  sdw_update_slave_status+0x4d/0x70 [soundwire_bus]
[  281.788039] kernel:  sdw_handle_slave_status+0xdb3/0x13d0 [soundwire_bus]
[  281.788047] kernel:  ? _raw_spin_unlock_irqrestore+0x27/0x50
[  281.788051] kernel:  cdns_update_slave_status_work+0x1b3/0x270 [soundwire_cadence]
[  281.788057] kernel:  process_one_work+0x283/0x550
[  281.788061] kernel:  worker_thread+0x53/0x3e0
[  281.788063] kernel:  ? __pfx_worker_thread+0x10/0x10
[  281.788064] kernel:  kthread+0xf1/0x130
[  281.788066] kernel:  ? __pfx_kthread+0x10/0x10
[  281.788069] kernel:  ret_from_fork+0x29/0x50
[  281.788074] kernel:  
[  281.788076] kernel: ------------[ cut here ]------------

@bardliao @fredoh9 is this a known issue? I have not seen any reports of this error before....
@plbossart
Copy link
Member Author

plbossart commented May 23, 2023 •
edited

Possible upstream issue, the last change was this commit 582ed31 ("ASoC:rt5682: Use a maple tree based register cache")

@broonie
Copy link

broonie commented May 23, 2023

It'll be the introduction of the RCU read lock.

@plbossart
Copy link
Member Author

not sure what's so special about this RCU read lock. We have mutexes in SoundWire already, they don't seem to trigger any problem.

@broonie
Copy link

broonie commented May 23, 2023

It's an atomic lock, like a spinlock.

@plbossart
Copy link
Member Author

errors reproduced with the latest topic/sof-dev branch. Reverting commit 582ed31 ("ASoC:rt5682: Use a maple tree based register cache") restores functionality in limited tests.

plbossart pushed a commit to plbossart/sound that referenced this issue May 23, 2023
@broonie @plbossart
regmap: maple: Drop the RCU read lock while syncing registers
1294073 
Unfortunately the maple tree requires us to explicitly lock it so we need
to take the RCU read lock while iterating. When syncing this means that we
end up trying to write out register values while holding the RCU read lock
which triggers lockdep issues since that is an atomic context but most
buses can't be used in atomic context. Pause the iteration and drop the
lock for each register we check to avoid this.

Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Closes: thesofproject#4371
Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
@plbossart plbossart mentioned this issue May 23, 2023
Merged
@plbossart
Copy link
Member Author

plbossart commented May 23, 2023 •
edited

limited remote tests with PR #4372 don't expose any problem with the patch suggested by @broonie

PASS: tplg-binary; pcm_list; ipc-flood; playback-d100l1r1; capture-d100l1r1; playback-d1l100r1; capture_d1l100r1; playback_d1l1r50; capture_d1l1r50; speaker; pause-resume-playback; pause-resume-capture; volume; signal-stop-start-playback; signal-stop-start-capture; multiple-pipeline-playback; multiple-pipeline-capture; multiple-pause-resume; kmod-load-unload; kmod-load-unload-after-playback; suspend-resume; suspend-resume-with-playback; suspend-resume-with-capture;
SKIP: sof-logger; xrun-injection-playback; xrun-injection-capture; simultaneous-playback-capture; volume_levels;

 test end with 0 failed tests

@keqiaozhang is there a way you can check locally if the headset functionality is fine on the BRYA devices? I can only check the results of scripts, not sure if it's actually playing/capturing any sound.

plbossart pushed a commit to plbossart/sound that referenced this issue May 23, 2023
@broonie @plbossart
regmap: maple: Drop the RCU read lock while syncing registers
0ba84c6 
Unfortunately the maple tree requires us to explicitly lock it so we need
to take the RCU read lock while iterating. When syncing this means that we
end up trying to write out register values while holding the RCU read lock
which triggers lockdep issues since that is an atomic context but most
buses can't be used in atomic context. Pause the iteration and drop the
lock for each register we check to avoid this.

Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Closes: thesofproject#4371
Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
plbossart pushed a commit that referenced this issue May 24, 2023
@broonie @plbossart
regmap: maple: Drop the RCU read lock while syncing registers
4c198c6 
Unfortunately the maple tree requires us to explicitly lock it so we need
to take the RCU read lock while iterating. When syncing this means that we
end up trying to write out register values while holding the RCU read lock
which triggers lockdep issues since that is an atomic context but most
buses can't be used in atomic context. Pause the iteration and drop the
lock for each register we check to avoid this.

Reported-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Closes: #4371
Tested-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
@keqiaozhang
Copy link
Collaborator

@keqiaozhang is there a way you can check locally if the headset functionality is fine on the BRYA devices? I can only check the results of scripts, not sure if it's actually playing/capturing any sound.

@plbossart, sorry for my late reply, I have checked the headset functionality, both playback and capture can work, audio quality is good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

regmap: maple: Drop the RCU read lock while syncing registers plbossart/sound
3 participants
@broonie @plbossart @keqiaozhang