-
Notifications
You must be signed in to change notification settings - Fork 140
Modified stream interrupt handler. Mimics the legacy interrupt handler #1013
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.
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
Modified stream interrupt handler. Mimics the legacy interrupt handler #1013
Conversation
|
@plbossart @kv2019i this is a extended/modifed version of #1011 and more in line with what the legacy driver does. In the few tests that I have run so far I have not met an IPC timeout yet. |
lyakh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly nitpicking
|
@ranj063 I first tested on UP2 with CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC enabled.(Actually, you have to enable this config to enable hda_generic_machine driver on Up2). Now I test your PR with WAKEEN feature, there are still many error messages. But I think your PR is not for this issue ? [ 36.441640] sof-audio-pci 0000:00:0e.0: Debug PCIR: 00000000 at 00000044 |
|
@ranj063 to reproduce it on UP2, just play a music after boot-up. |
|
|
||
| if (!pm_runtime_active(bus->dev)) | ||
| return IRQ_NONE; | ||
| int ret = IRQ_WAKE_THREAD; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ranj063 Waking the thread is a big change. I had a draft addition to PR1011 that only wakes stream thread if CIS or SIS bits are set, but then one needs to use a mask. Maybe this is better, I'll give it a test.
kv2019i
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to work very well, no IPC timeouts when testing with WHL and CFL, and sanity check with UP2 works as well.
Waking up the stream threaded handled always is a bit fishy, but it does seem to do the trick.
35feec9 to
b557c53
Compare
Modify the stream interrupt handler to always wake up the IRQ thread if the status register is valid. The IRQ thread performs the check for stream interrupts and RIRB interrupts in a loop to handle the case of missed interrupts when an unsolicited response from the codec is received just before the stream interrupt handler is completed. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
b557c53 to
408b7ab
Compare
kv2019i
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this look good. We can do the thread-wake-up optimization later if masking proves a stable solution. Plus the locking needs some work still, but this patch is not adding anything new and e.g. reg_lock usage looks good here.
@RanderWang Sorry, I still cannot reproduce this problem. Does it occur for you every time or only sometimes? On analog or HDMI or both? PCM512X? Any other special conditions? |
I managed to reproduce it. The reason I wasn't getting it before is, that I also had #977 in my tree, which also happens to fix or at least to hide this problem. |
|
(posting in correct PR this time) |
|
let's merge. |
timerlat top does some background/font color formatting. While useful
on terminal, it breaks the output on other formats. For example, when
piping the output for pastebin tools, the format strings are printed
as characters. For instance:
[2;37;40m Timer Latency [0;0;0m
0 00:00:01 | IRQ Timer Latency (us) | Thread Timer Latency (us)
[2;30;47mCPU COUNT | cur min avg max | cur min avg max[0;0;0m
0 #1013 | 1 0 1 54 | 5 2 4 57
1 #1013 | 3 0 1 10 | 6 2 4 15
To avoid this problem, do the formatting only if running on a tty,
and in !quiet mode.
Link: https://lkml.kernel.org/r/8288e1544ceab21557d5dda93a0f00339497c649.1713968967.git.bristot@kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Juri Lelli <juri.lelli@redhat.com>
Signed-off-by: Daniel Bristot de Oliveira <bristot@kernel.org>
Modify the stream interrupt handler to always wake up the IRQ thread if the status register is valid. The IRQ thread performs the check for stream interrupts and RIRB interrupts in a loop to handle the case of missed interrupts when an unsolicited response from the codec is received just before the stream interrupt handler is completed.