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.

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

host: unify component for all DMA types #1565

Merged
merged 9 commits into from
Jun 25, 2019

Conversation

tlauda
Copy link
Contributor

@tlauda tlauda commented Jun 13, 2019

Unifies host component for all DMA types.
We don't need to branch on CONFIG_DMA_GW anymore.

Closes #1052.

Signed-off-by: Tomasz Lauda [email protected]

DW-DMA should ignore copy call if flags include
DMA_COPY_PRELOAD.

Signed-off-by: Tomasz Lauda <[email protected]>
Unifies host_copy implementation for all types
of DMA.

Signed-off-by: Tomasz Lauda <[email protected]>
Ignores request for HDA-DMA configuration if the
channel is in active state.

Signed-off-by: Tomasz Lauda <[email protected]>
Checks if dma_sg_elem_array buffer is allocated
before performing cache writeback or invalidation.

Signed-off-by: Tomasz Lauda <[email protected]>
Changes host_buffer component ops to be a subtype of
component set_attribute ops. This change allows to
unify host implementation across different types
of DMA.

Signed-off-by: Tomasz Lauda <[email protected]>
Signed-off-by: Tomasz Lauda <[email protected]>
Removes period_bytes field from host_data struct.
This field is only used in host_params, so we
don't need it globally.

Signed-off-by: Tomasz Lauda <[email protected]>
Adds support for one shot copy to DW-DMA.
One shot means that transfer isn't circular,
so every copy is basically just restart of
DMA engine.

Signed-off-by: Tomasz Lauda <[email protected]>
struct hc_buf host;
#if !CONFIG_DMA_GW
struct hc_buf local;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here we still branch on CONFIG_DMA_GW. Is this as intended? Because the PR description says we no longer need to branch on this config option.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@tlauda I now see the last patch where the config is removed. So, it looks good to me.

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.

@mengdonglin @keqiaozhang can this be tested on legacy.

Copy link
Contributor

@jajanusz jajanusz left a comment

Choose a reason for hiding this comment

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

good job

src/audio/host.c Outdated

if (local_elem->src == hd->source->current_end) {
/* end of element, so use next */
source_elem = next_buffer(hd->source);
hd->source->current_end = source_elem->src + source_elem->size;
Copy link
Contributor

Choose a reason for hiding this comment

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

Codecheck complains that source_elem can be null here.

src/audio/host.c Outdated

if (local_elem->dest == hd->sink->current_end) {
/* end of element, so use next */
sink_elem = next_buffer(hd->sink);
hd->sink->current_end = sink_elem->dest + sink_elem->size;
Copy link
Contributor

Choose a reason for hiding this comment

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

Codecheck complains that sink_elem can be null here.

@dbaluta
Copy link
Collaborator

dbaluta commented Jun 14, 2019

@tlauda We should be careful to run all regression test we have! Other than that, excellent work!

@jajanusz
Copy link
Contributor

@tlauda ping, UT and CodeCheck didn't pass

Unifies host component for all DMA types.
We don't need to branch on CONFIG_DMA_GW anymore.

Signed-off-by: Tomasz Lauda <[email protected]>
@tlauda
Copy link
Contributor Author

tlauda commented Jun 24, 2019

@mengdonglin @keqiaozhang It's been over a week. Can I assume that it's not breaking legacy platforms?

@lgirdwood
Copy link
Member

@tlauda I have a feeling there has been some national holidays for @mengdonglin and @keqiaozhang last week but some folks seem back today.

@Jiangxinx
Copy link
Contributor

I have done sanity test for this PR on apl and byt platform.

env
kernel (topic/sof-dev): 0087d4a
topology: sof-apl-pcm512x.tplg & sof-apl-nocodec.tplg & sof-byt-rt5651.tplg & sof-byt-nocodec.tplg

Test result

test case byt nocodec byt rt5651 apl up2 nocodec apl up2 nocodec(xcc) apl up2 PCM512x apl up2 PCM512x(xcc)
repeat 3 times playback over headset Passed Passed Passed Passed Passed Passed
Pause/Resume playback over headset - 10 times Failed Failed Passed Passed Passed Passed
Play an audio for 3 minutes Passed Passed Passed Passed Passed Passed
Change volume during playback. Passed Passed Passed Passed Passed Passed
aplay - playback over HDMI (hw:0, 2) N/A N/A N/A N/A Passed Passed
stop/resume playback over HDMI N/A N/A N/A N/A Passed Passed
repeat 3 times playback over HDMI N/A N/A N/A N/A Passed Passed
Pause/Resume playback over HDMI N/A N/A N/A N/A Passed Passed
Play an audio for 3 minutes over HDMI N/A N/A N/A N/A Passed Passed
Change volume during playback over HDMI. N/A N/A N/A N/A Passed Passed
arecord - capture over headset(hw:0,0) (HDA Analog) Passed Passed Passed Passed N/A N/A
Change volume during capture (hw:0,0). Passed Passed Passed Passed N/A N/A
Pause/Resume capture (hw:0,0) Failed (#1578) Failed (#1578) Passed Passed N/A N/A
Capture a audio for 5 minutes via HDA Analog Passed Passed Passed Passed N/A N/A
repeat 3 times capture Passed Passed Passed Passed N/A N/A
Stop/Resume capture Passed Passed Passed Passed N/A N/A
arecord - capture over DMIC32 (hw:0,6) N/A N/A Passed Passed N/A N/A
Change volume during capture over DMIC32 (hw:0,6) . N/A N/A Passed Passed N/A N/A
Pause/Resume capture over DMIC32 (hw:0,6) N/A N/A Passed Passed N/A N/A
Capture a audio for 5 minutes over DMIC32 (hw:0,6) N/A N/A Passed Passed N/A N/A
repeat 3 times capture over DMIC32 (hw:0,6) N/A N/A Passed Passed N/A N/A
Stop/Resume capture over DMIC32 (hw:0,6) N/A N/A Passed Passed N/A N/A
Do playback and capture at same time Passed Passed Passed Passed N/A N/A
2 aplay_Analogue + Media Playback Passed Passed Failed (#1526) Failed (#1526) Failed (#1526) Failed (#1526)
2 aplay_Analogue + HDMI N/A N/A N/A N/A Passed Passed
Check the audio channel via speaker-test Passed Passed Passed Passed Passed Passed
Check the runtime PM status N/A N/A Passed Passed Passed Passed
Modules reload Passed Passed Passed Passed Passed Passed
Modules reload - 100 times Passed Passed Passed Passed Passed Passed

@tlauda
Copy link
Contributor Author

tlauda commented Jun 25, 2019

@Jiangxinx To sum up, do we have any regression or not?

@Jiangxinx
Copy link
Contributor

Sorry to this mission: This pr have not another regression issues
Pause/Resume playback over headset - 10 times can be also reproduced on daily version.

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.

host.c: remove DMA specific logic
5 participants