ASoC: SOF: ipc4-pcm: do not report invalid delay values #5513
Conversation
|
Marking as draft until #5499 is merged. |
| #define DELAY_BOUNDARY U32_MAX | ||
|
|
||
| #define DELAY_MAX (DELAY_BOUNDARY >> 1) | ||
|
|
There was a problem hiding this comment.
I did consider multiple options:
- use runtime->buffer_size as reference for maximum sensible/valid delay. work in most cases, but in theory DSP could have bigger internal delays, so dropped this
- define DELAY_MAX in wallclock time (e.g. 10 seconds of audio) and calculate a frame limit by multiplying with sampling rate -> in the end, this would require some overhead to calculate based on the stream or assume some maximum realistic sampling rate... and in the end, this is not any less adhoc than just defining BOUNDARY/2
... so went with BOUNDARY/2 in the end. This will primarily cover cases where the DSP pointers have gone out-of-sync (e.g. we had bugs with handling xruns and pauses in the past) and continuously report delay values close to DELAY_BOUNDARY
kv2019i
force-pushed
the
202508-filter-invalid-delay-values
branch
from
938fc33 to
37a9ec5
Compare
|
V2:
|
Add a sanity check for the calculated delay value before reporting it to the application. If the value is clearly invalid, emit a rate limited warning to kernel log and return a zero delay. This can occur e.g if the host or link DMA hits a buffer over/underrun condition. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
kv2019i
force-pushed
the
202508-filter-invalid-delay-values
branch
from
37a9ec5 to
81beb31
Compare
kv2019i
changed the title
|
V3:
|
kv2019i
requested review from
bardliao,
dbaluta,
lgirdwood,
plbossart,
ranj063 and
ujfalusi
as code owners
| else | ||
| time_info->delay = head_cnt - tail_cnt; | ||
|
|
||
| if (time_info->delay > DELAY_MAX) { |
There was a problem hiding this comment.
when this happens it is because the dai ptr counter overtook the host pointer, which indicates incorrect start offset and a rare race when the dai / host DMA burst happens just a bit out of sync (or the pointer report) and things flop.
I think I'm fine with this filtering at the end.
There was a problem hiding this comment.
Ack @ujfalusi . I think this is just a safety-valve against trusting the FW too much. If this is hit, it's an issue we would have to go and hunt for these cases and fix them one by one, but without an error printed, these are difficult to catch.
One option is to just leave this out-of-tree as a debugging patch. But if these do happen on end-user systems, letting clearly invalid delay values through will cause pretty major issues (e.g. video playback not working), so I'm leaning towards having a safety check built in.
|
@bardliao @lgirdwood if you have time to review, this one would be waiting |
3 participants
[PATCH] ASoC: SOF: ipc4-pcm: do not report invalid delay values
Add a sanity check for the calculated delay value before reporting it to
the application. If the value is clearly invalid, emit a rate limited
warning to kernel log and return a zero delay. This can occur e.g if the
host or link DMA hits a buffer over/underrun condition.