-
Notifications
You must be signed in to change notification settings - Fork 132
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.
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
ASoC: SOF: Intel: hda: improve code loader and prepare for SoundWire BPT reuse #4697
ASoC: SOF: Intel: hda: improve code loader and prepare for SoundWire BPT reuse #4697
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@plbossart, looks reasonable, only few nitpicks.
The existing code conflates capture stream and ICCMAX stream. This isn't going to be true any longer when we add support for SDW BPT RX streams. Add a boolean tag to flag ICCMAX streams. No functionality change, only future-proofing change. Signed-off-by: Pierre-Louis Bossart <[email protected]>
We need to reuse cl_prepare, cl_trigger and cl_cleanup helpers from a SoundWire context where only the device information is available. Rather than pass the 'sdev' argument, use get_drvdata() to retrieve the required structure. For consistency, rename hda_cl_stream_prepare() as hda_cl_prepare(). These three helpers are also exported so that they can be referenced from another module. Signed-off-by: Pierre-Louis Bossart <[email protected]>
6cf851d
to
a06109e
Compare
The HDaudio stream interrupts are ignored unless the stream is PCM or compressed audio. For alternate non-audio usages, such as code loader or SoundWire BPT case, the IOC interrupt on the last buffer transferred is silently ignored. This patch adds a 'struct completion' for each HDaudio stream. This capability helps detect if the non-audio data transfers completed. There is no performance impact for audio streams. In the code loader case, the code currently starts the DMA and directly checks if the firmware status changes, without checking if the DMA succeeded. With a first pass waiting for the DMA to complete, system validation engineers can gather more precise timing information on firmware boot time or root-cause boot failures more accurately. A timeout of 500ms was selected for the code loader DMA. This is an experimental value which should be more than enough - higher values would certainly be problematic from a usage/latency perspective. Signed-off-by: Pierre-Louis Bossart <[email protected]>
The Yoda grammar and multiple negatives are unclear. Signed-off-by: Pierre-Louis Bossart <[email protected]>
a06109e
to
1b67be7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good except for a minor typo
continue; | ||
if (!s->substream && !s->cstream) { | ||
/* | ||
* when no substream is found, the DMA may used for code loading |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo "may be used..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'll add a fixup
Initial patches are harmless but the last patch is quite invasive with the addition of a DMA completion timeout for the code loader.
These patches prepare the integration of the SoundWire BPT support which will rely on parts of the code loader examples.
One additional benefit is that a simple 'dmesg | grep "Code loader"' will tell us how much time is spent transferring the firmware v. waiting for the firmware to boot.
@ranj063 @ujfalusi @bardliao @RanderWang @kv2019i comments welcome.