ipc4: notification: Add support for xruns notifications #10002
Conversation
softwarecki
requested review from
abonislawski,
gbernatxintel,
jxstelter,
kv2019i,
lgirdwood,
lyakh,
serhiy-katsyuba-intel and
tmleman
softwarecki
force-pushed
the
notify-xrun
branch
2 times, most recently
from
c489bec to
749ae38
Compare
lyakh
left a comment
lyakh
left a comment
There was a problem hiding this comment.
a general question: can we not check under- and overruns at a central location, e.g. at the pipeline level, when calling modules' .process() methods? Or maybe when checking available data / bytes, e.g. in source_get_data_frames_available()?
softwarecki
requested review from
RanderWang,
dbaluta,
lbetlej,
marcinszkudlinski,
mmaka1,
pblaszko,
plbossart and
ranj063
as code owners
lgirdwood
left a comment
lgirdwood
left a comment
There was a problem hiding this comment.
I think this is useful feature, will be helpful in debugging and reporting to users.
@softwarecki What's the granularity of the xrun reporting size ? Will it flood any IPCs to the host under any condition ?
@ujfalusi @ranj063 @bardliao pls see wrt kernel, this will need some code for debug and also for passing up the ALSA stack when severe xruns are detected.
|
why these changes? Doesn't look like an improvement to me... |
I agree, but checkpatch... |
@softwarecki exactly, as you've done in the latest version - just adding braces after |
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
This is starting to have a lot of stuff in same PR, but overall looks good. Please adjust the commit for the last message.
We'd need some basic smoketest for the case where notifications are actually sent. Let's make sure the Linux driver can handle this in a sane way. If not, this cannot be made default until driver has had time to prepare. In theory should be good though, the notification should be just ignored.
@lgirdwood The xrun notification will be sent only once when it is detected. No further notifications will be sent until xrun is gone. In the worst case, the notification will be sent every two periods. |
Add functions to prepare ipc4 notifications about underrun/overrun detected by a dma. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Change xrun notifications to use the notification pool. Improve the logic for clearing the flag that informs the notification was sent. Previously, it was cleared when xrun occurred and the notification was sent, which meant that a notification was sent every second time xrun was detected, which could cause a flood of notifications. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Add support to the host for sending ipc4 notifications when dma reports an underrun/overrun occurrence. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Add support to dai for sending ipc4 notifications when dma reports an underrun/overrun occurrence. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Using Module Config Get / Set command, host driver may send a parameter that fits into the header (a very short one), packed along with parameter id. Larger parameters require fragmentation and a series of Large Config Set commands. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
…et ipc Add support for Config Get and Config Set ipc commands. Handle them through the get_attribute and set_attribute methods of the component with the COMP_ATTR_IPC4_CONFIG parameter. Add the set_config_param and get_config_param functions to the module interface. Extend the functionality of module adapter to call the above functions to handle ipc request. Increase the version of module api due to modification of module_interface structure. Using Module Config Get / Set command, host driver may send a parameter that fits into the header (a very short one), packed along with parameter id. Larger parameters require fragmentation and a series of Large Config Set commands. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Add function preparing ipc4 notification about underrun detected by mixer. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Add to the mixin the capability to send notifications when an underrun occurs, leading to the generation of silence in the output. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
Change the configuration so that ipc underrun notifications are sent by default. Signed-off-by: Adrian Warecki <adrian.warecki@intel.com>
kv2019i
left a comment
kv2019i
left a comment
There was a problem hiding this comment.
I did a quick test with SOF Linux driver and slightly modified PR to inject a few xruns on FW side. Seems the Linux side does not at least crash and burn, so I see no blockers to merge:
Linux side log:
[ 161.213809] snd_sof:sof_ipc4_log_header: sof-audio-pci-intel-ptl 0000:02:1f.3: ipc rx : 0x1b050000|0x0: GLB_NOTIFICATION|RESOURCE_EVENT
[ 161.213901] snd_sof:sof_ipc4_log_header: sof-audio-pci-intel-ptl 0000:02:1f.3: ipc rx done : 0x1b050000|0x0: GLB_NOTIFICATION|RESOURCE_EVENT [data size: 40]
[ 161.228820] snd_sof:sof_ipc4_log_header: sof-audio-pci-intel-ptl 0000:02:1f.3: ipc rx : 0x1b050000|0x0: GLB_NOTIFICATION|RESOURCE_EVENT
[ 161.228911] snd_sof:sof_ipc4_log_header: sof-audio-pci-intel-ptl 0000:02:1f.3: ipc rx done : 0x1b050000|0x0: GLB_NOTIFICATION|RESOURCE_EVENT [data size: 40]
[ 174.472906] snd_sof:sof_ipc4_log_header: sof-audio-pci-intel-ptl 0000:02:1f.3: ipc rx : 0x1b050000|0x0: GLB_NOTIFICATION|RESOURCE_EVENT
[ 174.473020] snd_sof:sof_ipc4_log_header: sof-audio-pci-intel-ptl 0000:02:1f.3: ipc rx done : 0x1b050000|0x0: GLB_NOTIFICATION|RESOURCE_EVENT [data size: 40]
[ 174.509949] snd_sof:sof_ipc4_log_header: sof-audio-pci-intel-ptl 0000:02:1f.3: ipc rx : 0x1b050000|0x0: GLB_NOTIFICATION|RESOURCE_EVENT
[ 174.510044] snd_sof:sof_ipc4_log_header: sof-audio-pci-intel-ptl 0000:02:1f.3: ipc rx done : 0x1b050000|0x0: GLB_NOTIFICATION|RESOURCE_EVENT [data size: 40]
Two FW periods (2ms in most cases) is too fast as ALSA xrun recovery works at the level of host period sizes. Can we make this a Kconfig value. |
@lgirdwood: Two fw periods assuming that xrun will alternately appear and disappear. If it does not magically disappear, then the next notification will not be sent until alsa fixes the situation. This is how the reference firmware works. Of course we can complicate this whole mechanism, add a value in Kconfig and count the cycles between notifications. However, I'm not convinced that this is necessary at the moment. |
|
SOFCI TEST |
ok, we can circle back if this is a problem for the kernel. |
|
@lgirdwood Internal Intel CI build and tests seems fine too. |
|
Out of curiosity: what the kernel should be doing if receives this? |
Nothing yet. Lets first see if the IPC frequency is right for ALSA and whether any new data is needed for ALSA. Today we will pickup xruns using host ptr position. |
7 participants
Add functions to prepare ipc4 notifications about underrun/overrun detected by a dma.
In chain_dma change xrun notifications to use the notification pool. Improve the logic for clearing the flag that informs the notification was sent. Previously, it was cleared when xrun occurred and the notification was sent, which meant that a notification was sent every second time xrun was detected, which could cause a flood of notifications.
Add definitions for Config Get / Set ipc. Using Module Config Get / Set command, host driver may send a parameter that fits into the header (a very short one), packed along with parameter id. Larger parameters require fragmentation and a series of Large Config Set commands.
Add support for Config Get and Config Set ipc commands. Handle them through the
get_attributeandset_attributemethods of the component with theCOMP_ATTR_IPC4_CONFIGparameter. Add theset_config_paramandget_config_paramfunctions to the module interface. Extend the functionality of module adapter to call the above functions to handle ipc request. Increase the version of module api due to modification ofmodule_interfacestructure.Add support to the dai, host and mixer for sending ipc4 notifications when dma reports an underrun/overrun occurrence.
Change the configuration so that ipc underrun notifications are sent by default.