On 10/6/2022 10:45 AM, Jon Hunter wrote:
On 05/10/2022 13:29, Takashi Iwai wrote:
...
HDA playback is failing on -next for various Tegra boards. Bisect is
point to this commit and reverting it fixes the problem. I was a bit
puzzled why this change is causing a problem, but looking closer there
is a difference between the previous code that was calling
snd_hdac_stream_readb() and the new code that is calling
snd_hdac_stream_readb_poll(). The function snd_hdac_stream_readb()
calls snd_hdac_aligned_mmio() is see if the device has an aligned MMIO
which Tegra does and then would call snd_hdac_aligned_read(). However,
now the code always call readb() and this is breaking Tegra.
So it is either necessary to update snd_hdac_stream_readb_poll() to
handle this or revert this change.
Does the patch below work?
thanks,
Takashi
-- 8< --
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -592,8 +592,8 @@ int snd_hdac_get_stream_stripe_ctl(struct hdac_bus
*bus,
#define snd_hdac_stream_readb(dev, reg) \
snd_hdac_reg_readb((dev)->bus, (dev)->sd_addr + AZX_REG_ ## reg)
#define snd_hdac_stream_readb_poll(dev, reg, val, cond, delay_us,
timeout_us) \
- readb_poll_timeout((dev)->sd_addr + AZX_REG_ ## reg, val, cond, \
- delay_us, timeout_us)
+ read_poll_timeout(snd_hdac_reg_readb, val, cond, delay_us,
timeout_us,\
+ false, (dev)->bus, (dev)->sd_addr + AZX_REG_ ## reg)
#define snd_hdac_stream_readl_poll(dev, reg, val, cond, delay_us,
timeout_us) \
readl_poll_timeout((dev)->sd_addr + AZX_REG_ ## reg, val, cond, \
delay_us, timeout_us)
So looking at this a bit more I see ...
[ 199.422773] bad: scheduling from the idle thread!
[ 199.427610] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G D
C 6.0.0-rc7-next-20220930-00007-gd6ae4ed0a78f-dirty #23
[ 199.438880] Hardware name: NVIDIA Jetson Nano Developer Kit (DT)
[ 199.444899] Call trace:
[ 199.447357] dump_backtrace.part.7+0xe8/0xf8
[ 199.451680] show_stack+0x14/0x38
[ 199.455024] dump_stack_lvl+0x64/0x7c
[ 199.458715] dump_stack+0x14/0x2c
[ 199.462067] dequeue_task_idle+0x2c/0x58
[ 199.466038] dequeue_task+0x38/0x98
[ 199.469565] __schedule+0x4a4/0x6d8
[ 199.473104] schedule+0x58/0xc0
[ 199.476292] schedule_hrtimeout_range_clock+0x98/0x108
[ 199.481472] schedule_hrtimeout_range+0x10/0x18
[ 199.486039] usleep_range_state+0x7c/0xb0
[ 199.490084] snd_hdac_stream_reset+0xe8/0x328 [snd_hda_core]
[ 199.495913] snd_hdac_stream_sync+0xe4/0x190 [snd_hda_core]
[ 199.501627] azx_pcm_trigger+0x1d8/0x250 [snd_hda_codec]
[ 199.507109] snd_pcm_do_stop+0x54/0x70
[ 199.510904] snd_pcm_action_single+0x44/0xa0
[ 199.515215] snd_pcm_drain_done+0x20/0x28
[ 199.519267] snd_pcm_update_state+0x114/0x128
[ 199.523670] snd_pcm_update_hw_ptr0+0x22c/0x3a0
[ 199.528246] snd_pcm_period_elapsed_under_stream_lock+0x44/0x88
[ 199.534216] snd_pcm_period_elapsed+0x24/0x48
[ 199.538617] stream_update+0x3c/0x50 [snd_hda_codec]
[ 199.543737] snd_hdac_bus_handle_stream_irq+0xe8/0x150 [snd_hda_core]
[ 199.550320] azx_interrupt+0xb4/0x1b0 [snd_hda_codec]
[ 199.555524] __handle_irq_event_percpu+0x74/0x140
[ 199.560281] handle_irq_event_percpu+0x14/0x48
[ 199.564772] handle_irq_event+0x44/0x88
[ 199.568653] handle_fasteoi_irq+0xa8/0x130
[ 199.572788] generic_handle_domain_irq+0x28/0x40
[ 199.577452] gic_handle_irq+0x9c/0xb8
[ 199.581168] call_on_irq_stack+0x2c/0x40
[ 199.585129] do_interrupt_handler+0x7c/0x80
[ 199.589355] el1_interrupt+0x34/0x68
[ 199.592969] el1h_64_irq_handler+0x14/0x20
[ 199.597107] el1h_64_irq+0x64/0x68
[ 199.600540] cpuidle_enter_state+0x130/0x2f8
[ 199.604853] cpuidle_enter+0x38/0x50
[ 199.608463] call_cpuidle+0x18/0x38
[ 199.611991] do_idle+0x1f8/0x248
[ 199.615259] cpu_startup_entry+0x20/0x28
[ 199.619224] kernel_init+0x0/0x128
[ 199.622669] arch_post_acpi_subsys_init+0x0/0x8
[ 199.627240] start_kernel+0x630/0x668
[ 199.630933] __primary_switched+0xb4/0xbc
If I change your patch to be read_poll_timeout_atomic, then it works \o/
Can we make that update?
Jon
Yes, it makes sense, as it uses udelay instead of usleep, same as
original code.
I've send patch which updates the macros. It passed validation on our side.
Thanks for report!
Amadeusz
