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

dai_zephyr: Silence benign warnings #8621

Closed
wants to merge 1 commit into from
Closed

dai_zephyr: Silence benign warnings #8621

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 0 additions & 13 deletions src/audio/dai-zephyr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1286,8 +1286,6 @@ int dai_zephyr_multi_endpoint_copy(struct dai_data **dd, struct comp_dev *dev,

/* return if nothing to copy */
if (!frames) {
comp_warn(dev, "dai_zephyr_multi_endpoint_copy(): nothing to copy");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like @marcinszkudlinski weigh in from DP perspective.

These really are useful warnings as the DAI interface should in fact see a steady stream of data in LL ticks. DP modules may process in larger chunks and this is perfectly fine, but
these should be hidden by the DP queues that sit between DP and LL domains. The audio interface (e.g. DAI) runs at fixed rate and if we see these warnings in volume, something is usually wrong. If e.g. peer pipeline is paused, the copiers should handle these situations.

Granted, we do have a minimal ping-pong buffer towards the DAI, so it is possible to have a stable buffering scheme where this warning will come continuously, but so far this has not been the case (given the warnings are still there).

With DP and AEC this might indeed happen, so I'm ok if @marcinszkudlinski et al sign this off.

I would prefer to just downgrade to comp_dbg() first, to make this is a little easier to bring back.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kv2019i @marcinszkudlinski , as you pointed out, when DP handling large chunks, delay is expected ( > large trunk period), however, it is not error, it should not be print out.

We may need find a way to make it only report real under-run, or overrun.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

split the log, once you hit a period threshold report an error? Otherwise this can just be a debug log.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@btian1 Right but this delay should be handled within DP domain and DP elements. These prints are from the DAI interface code. If we don't have data when DAI copy is called, we are at the mercy of the per-DAI buffers (depends on DAI type, but typically 2-4 1ms chunks of DAI facing buffers). We should not rely on this buffering for data flow, but instead should always have data to read/write when DAI needs it. We have had separate discussions about minizing the latency and if we have a DAI that can operate with the 2x1ms ping-pong buffer, that does not leave any margin for errors from audio pipeline code. The LL scheduler is run every 1ms tick, and it should be able to provide/consume data without exceptions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From @marcinszkudlinski stuck in older #8571

@andyross this message must not appear more than 2xDP processing period - that means 20 times in case of AEC during pipeline startup.
500 lines means - processing is not working properly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andyross I would strongly recommend to change the message level to debug. It is a potential risk of message flood that can affect FW operability. It might be tricky to determine at DAI level if no data to copy was intended in given situation.


for (i = 0; i < num_endpoints; i++) {
ret = dma_reload(dd[i]->chan->dma->z_dev, dd[i]->chan->index, 0, 0, 0);
if (ret < 0) {
Expand Down Expand Up @@ -1460,19 +1458,8 @@ int dai_common_copy(struct dai_data *dd, struct comp_dev *dev, pcm_converter_fun
comp_dbg(dev, "dai_common_copy(), dir: %d copy_bytes= 0x%x",
dev->direction, copy_bytes);

/* Check possibility of glitch occurrence */
if (dev->direction == SOF_IPC_STREAM_PLAYBACK &&
copy_bytes + avail_bytes < dd->period_bytes)
comp_warn(dev, "dai_common_copy(): Copy_bytes %d + avail bytes %d < period bytes %d, possible glitch",
copy_bytes, avail_bytes, dd->period_bytes);
else if (dev->direction == SOF_IPC_STREAM_CAPTURE &&
copy_bytes + free_bytes < dd->period_bytes)
comp_warn(dev, "dai_common_copy(): Copy_bytes %d + free bytes %d < period bytes %d, possible glitch",
copy_bytes, free_bytes, dd->period_bytes);

/* return if nothing to copy */
if (!copy_bytes) {
comp_warn(dev, "dai_zephyr_copy(): nothing to copy");
dma_reload(dd->chan->dma->z_dev, dd->chan->index, 0, 0, 0);
return 0;
}
Expand Down
Loading