Skip to content

Conversation

@ujfalusi
Copy link
Contributor

@ujfalusi ujfalusi commented Oct 6, 2021

The host will receive an error if it tries to re-configure the dtrace:

[ 22736132.117381] ( 28.645832) c0 hda-dma ..../intel/hda/hda-dma.c:489 ERROR hda-dmac: 4 no free channel 0
[ 22736147.377797] ( 15.260416) c0 dma-copy src/ipc/dma-copy.c:163 ERROR dma_copy_set_stream_tag(): dc->chan is NULL
[ 22736180.346545] ( 32.968750) c0 ipc src/ipc/ipc3/handler.c:805 ERROR ipc: failed to enable trace -22

Since we try to request an already used channel. If we skip the requesting
then we will fail with the configuration:

[ 31707957.542122] ( 27.447916) c0 dma-copy src/ipc/dma-copy.c:162 ERROR dma_copy_set_stream_tag(): already have chan for stream_tag 1
[ 31708021.917120] ( 64.375000) c0 hda-dma ..../intel/hda/hda-dma.c:539 ERROR hda-dmac: 4 channel 0 busy. dgcs 0x800000 status 5
[ 31708056.083785] ( 34.166664) c0 ipc src/ipc/ipc3/handler.c:805 ERROR ipc: failed to enable trace -16

To support the reconfiguration:
if we have DMA channel for dtrace, stop it to allow the reconfiguration.
In the unlikely case when the stream_id has changed, release the channel
as well and request a new one.

Tested with the SOF client kernel by rmmod and modprobe the dtrace client
during active audio playback to prevent the DSP to be turned off.

Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com

@marc-hb
Copy link
Collaborator

marc-hb commented Oct 6, 2021

What is the simplest way to test this?

@ujfalusi
Copy link
Contributor Author

ujfalusi commented Oct 6, 2021

What is the simplest way to test this?

my wip branch is kept up to date with sof-dev:
https://github.com/ujfalusi/sof-linux/tree/peter/sof%2Fauxiliary_bus_wip_take3

Build it, reboot if needed.
start audio playback (aplay -Dhw:0,0 -fdat /dev/urandom)
while it is running:
rmmod snd_sof_dma_trace
modprobe snd_sof_dma_trace

And repeat the module removal/insert

@ujfalusi ujfalusi force-pushed the peter/pr/dtrace_reconfig branch from a58736b to 1074b03 Compare October 7, 2021 07:39
After the dma_copy_set_stream_tag() a DMA channel is requested, release it
in any of the error cases later.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
@ujfalusi ujfalusi force-pushed the peter/pr/dtrace_reconfig branch from 1074b03 to ac94e38 Compare October 7, 2021 07:46
@ujfalusi
Copy link
Contributor Author

ujfalusi commented Oct 7, 2021

Changes since v1:

  • Add a patch to implement cleanup on error in dma_trace_start()
  • Use mtrace_printf() to print a warning in case of DMA channel reconfiguration

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

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

Hapy to merge alongside #4849 or this one first. Let me know.

@lgirdwood
Copy link
Member

Seems CI is stuck. Restart it.

@lgirdwood
Copy link
Member

SOFCI TEST

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could adding some reference to the channel in the log be helpful?

@ujfalusi ujfalusi force-pushed the peter/pr/dtrace_reconfig branch from ac94e38 to c30112b Compare October 8, 2021 07:12
@ujfalusi
Copy link
Contributor Author

ujfalusi commented Oct 8, 2021

Changes since v2:

  • print the stream_tag if the channel is used, also print the change in stream_tag (unlikely)

Copy link
Collaborator

Choose a reason for hiding this comment

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

just use else if to avoid repeating err == 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is a cleaner way.

The host will receive an error if it tries to re-configure the dtrace:

[    22736132.117381] (          28.645832) c0 hda-dma            ..../intel/hda/hda-dma.c:489  ERROR hda-dmac: 4 no free channel 0
[    22736147.377797] (          15.260416) c0 dma-copy                 src/ipc/dma-copy.c:163  ERROR dma_copy_set_stream_tag(): dc->chan is NULL
[    22736180.346545] (          32.968750) c0 ipc                  src/ipc/ipc3/handler.c:805  ERROR ipc: failed to enable trace -22

Since we try to request an already used channel. If we skip the requesting
then we will fail with the configuration:

[    31707957.542122] (          27.447916) c0 dma-copy                 src/ipc/dma-copy.c:162  ERROR dma_copy_set_stream_tag(): already have chan for stream_tag 1
[    31708021.917120] (          64.375000) c0 hda-dma            ..../intel/hda/hda-dma.c:539  ERROR hda-dmac: 4 channel 0 busy. dgcs 0x800000 status 5
[    31708056.083785] (          34.166664) c0 ipc                  src/ipc/ipc3/handler.c:805  ERROR ipc: failed to enable trace -16

To support the reconfiguration:
if we have DMA channel for dtrace, stop it to allow the reconfiguration.
In the unlikely case when the stream_id has changed, release the channel
as well and request a new one.

Tested with the SOF client kernel by rmmod and modprobe the dtrace client
during active audio playback to prevent the DSP to be turned off.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
@ujfalusi ujfalusi force-pushed the peter/pr/dtrace_reconfig branch from c30112b to 69f2d54 Compare October 8, 2021 10:21
@ujfalusi
Copy link
Contributor Author

ujfalusi commented Oct 8, 2021

Changes since v3:

  • use else if to avoid re-testing for err == 0 when verifying the current and new stream_tag.

@lgirdwood lgirdwood merged commit 9823983 into thesofproject:main Oct 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants