Skip to content

Conversation

@plbossart
Copy link
Member

@plbossart plbossart commented Mar 31, 2020

This changes the SSP configuration and moves the scheduling to DMA instead of TIMER. 44.1 kHz is also supported, a first in the history of SOF I guess :-)

@singalsu
Copy link
Collaborator

@plbossart There's no ASRC in the media pipeline so I'd assume it should be DMA like it was before the topologies were switched to timer based scheduling.

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.

Both should be DMA, IIRC media is not using ASRC and codec is master.

@paulstelian97
Copy link
Collaborator

paulstelian97 commented Mar 31, 2020

If it's DMA you should also disable the agent as it only works correctly with timer driven pipelines. If not done already.

@plbossart
Copy link
Member Author

plbossart commented Mar 31, 2020

If it's DMA you should also disable the agent as it only works correctly with timer driven pipelines. If not done already.

?? Are we saying that we need a new firmware with a different config when the topology changes from SOC master to SOC slave?
@tlauda can you please comment?

@plbossart
Copy link
Member Author

plbossart commented Mar 31, 2020

If the Kconfig documentation is right, then this AGENT is really wrong:

config HAVE_AGENT
	bool "Enable system agent"
	default y
	help
	  Enables system agent. It can be disabled on systems
	  which are still unstable and cannot assure that
	  system agent will always execute on time or systems
	  with DMA based scheduling, where asynchronous interrupts
	  can potentially starve the agent.

If we provide a means to change the behavior with the topology, then somehow the agent should be disabled automatically for pipelines/DAIs that rely on DMA. It doesn't seem right to me to require a new binary be generated.

@paulstelian97
Copy link
Collaborator

If the Kconfig documentation is right, then this AGENT is really wrong:

config HAVE_AGENT
	bool "Enable system agent"
	default y
	help
	  Enables system agent. It can be disabled on systems
	  which are still unstable and cannot assure that
	  system agent will always execute on time or systems
	  with DMA based scheduling, where asynchronous interrupts
	  can potentially starve the agent.

If we provide a means to change the behavior with the topology, then somehow the agent should be disabled automatically for pipelines/DAIs that rely on DMA. It doesn't seem right to me to require a new binary be generated.

@tlauda Any comments on what should be done with this?

@tlauda
Copy link
Contributor

tlauda commented Apr 1, 2020

If the Kconfig documentation is right, then this AGENT is really wrong:

config HAVE_AGENT
	bool "Enable system agent"
	default y
	help
	  Enables system agent. It can be disabled on systems
	  which are still unstable and cannot assure that
	  system agent will always execute on time or systems
	  with DMA based scheduling, where asynchronous interrupts
	  can potentially starve the agent.

If we provide a means to change the behavior with the topology, then somehow the agent should be disabled automatically for pipelines/DAIs that rely on DMA. It doesn't seem right to me to require a new binary be generated.

@tlauda Any comments on what should be done with this?

We can easily dynamically turn off the agent as soon as there is any pipeline scheduled on DMA created. It will give us much more flexibility in terms of topology creation. Platforms which are not capable at all to schedule pipelines on timer should have agent disabled in kConfig (which I think is currently done).

@plbossart
Copy link
Member Author

If the Kconfig documentation is right, then this AGENT is really wrong:

config HAVE_AGENT
	bool "Enable system agent"
	default y
	help
	  Enables system agent. It can be disabled on systems
	  which are still unstable and cannot assure that
	  system agent will always execute on time or systems
	  with DMA based scheduling, where asynchronous interrupts
	  can potentially starve the agent.

If we provide a means to change the behavior with the topology, then somehow the agent should be disabled automatically for pipelines/DAIs that rely on DMA. It doesn't seem right to me to require a new binary be generated.

@tlauda Any comments on what should be done with this?

We can easily dynamically turn off the agent as soon as there is any pipeline scheduled on DMA created. It will give us much more flexibility in terms of topology creation. Platforms which are not capable at all to schedule pipelines on timer should have agent disabled in kConfig (which I think is currently done).

For my education, what does this 'system agent' do, and could it still be used for paths that will always be timer-based (HDA/HDMI + SoundWire + DMIC)?

@lgirdwood
Copy link
Member

For my education, what does this 'system agent' do, and could it still be used for paths that will always be timer-based (HDA/HDMI + SoundWire + DMIC)?

It's primary purpose it to detect deadlock or anything that blocks main loop re-entry. i.e. we expect to enter the main loop but FW is stuck somewhere else for too long. It will cause a panic so drivers can attempt an recovery.

It really needs a little more intelligence in programming the new timeout to deal with sync and async conditions rather than a disable for use case X.

@singalsu
Copy link
Collaborator

singalsu commented Apr 2, 2020

Cool, the 44100 Hz version works too! Though only with 16 bit content. With 24 bit and 32 bit it triggers an issue with mixer buffer sizes. This is seen in trace:

[    86533.020833] (    34716.824219) c0 IPC                       src/ipc/handler.c:239   ipc: comp 0 -> params
[    86536.770833] (        3.750000) c0 PIPE         1.9       src/audio/pipeline.c:419   pipe params dir 0 frame_fmt 2 buffer_fmt 0 rate 44100
[    86539.531250] (        2.760417) c0 PIPE         1.9       src/audio/pipeline.c:423   pipe params stream_tag 1 channels 2 sample_valid_bytes 4 sample_container_bytes 4
[    86573.020833] (       33.489582) c0 DMA                .../intel/cavs/hda-dma.c:405   hda-dmac: 5 channel 0 -> get
[    86577.552083] (        4.531250) c0 DMA                .../intel/cavs/hda-dma.c:587   hda-dmac: 5 channel 0 -> config
[    86590.937500] (       13.385417) c0 mixer        1.3          src/audio/mixer.c:204   ERROR mixer_params(): sink buffer size is insufficient
[    86595.572917] (        4.635417) c0 PIPE                   src/audio/pipeline.c:443   ERROR pipeline_params(): ret = -12, host->comp.id = 0
[    86599.739583] (        4.166667) c0 IPC                       src/ipc/handler.c:302   ERROR ipc: pipe 1 comp 0 params failed -12

The media PCM has the same issue. It can playback 16 kHz 48 kHz content via SRC so I think configuration should be OK.

The SRC likely suffers from insufficient buffers since there's audio discontinuity (few timer per second). I can fix later as well as the mixer buffer so you don't need to deep dive into them.

Copy link
Collaborator

@singalsu singalsu left a comment

Choose a reason for hiding this comment

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

Unrelated issues but I can fix them later.

@plbossart
Copy link
Member Author

please don't merge, I have a better one coming

This patch provides an optional support for codec master mode and uses
a macro to change the frame sync as needed. The scheduling is also
changes to to DMA instead of TIMER.

The user is still required to manually copy the desired topology as
sof-tplg/sof-apl-pcm512x.tplg, but a manual configuration of the ACPI
initrd overlays and BIOS settings is required as well, so there's no
turn-key solution possible anyways.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@plbossart plbossart requested review from lgirdwood and singalsu April 2, 2020 16:23
@plbossart plbossart changed the title [RFC] topology: sof-apl-pcm512x: add option for codec master mode topology: sof-apl-pcm512x: add option for codec master mode and 44.1kHz Apr 2, 2020
@lgirdwood
Copy link
Member

CI known issues

@lgirdwood lgirdwood merged commit 8b7f1fa into thesofproject:master Apr 3, 2020
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