Skip to content

Conversation

@fuyuntsuo
Copy link
Contributor

Host component stops copying bytes at element's end and resumes
copying the remaining bytes in the next round. This leads to jitter
between components of the same pipeline. Such jitter might cause
buffer overwrite for component which processes block by block.

Triggering extra DMA copy until all the copy_bytes are copied can
eliminate the jitter and hence no more buffer overwrite occurs.

Signed-off-by: fy.tsuo fy.tsuo@intelli-go.com

@fuyuntsuo
Copy link
Contributor Author

This is a fix for CONFIG_HOST_PTABLE.

It is helpful if anyone can explain this config because I couldn't find any information about it.

@keyonjie
Copy link
Contributor

@fuyuntsuo what platform are you working on? this is not enable on Intel's cAVS platforms, please see it in src/platform/Kconfig.
And, your changes breaks Intel's non-cAVS platforms (BDW, BYT) thoroughly according to the CI on device test result.

Copy link
Contributor

@keyonjie keyonjie left a comment

Choose a reason for hiding this comment

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

The change breaks Intel's non-cAVS platforms.

@fuyuntsuo
Copy link
Contributor Author

It's MT8195 DUT we use to verify the change. We know there are many platforms selecting CONFIG_HOST_PTABLE but MT8915 is the only one we have.

The change is component-level so theoretically it should be platform-independent. If possible @keyonjie can you share what error Intel non-cAVS platforms show? Then we can find out what this change is missing. Thanks!

@keyonjie
Copy link
Contributor

It's MT8195 DUT we use to verify the change. We know there are many platforms selecting CONFIG_HOST_PTABLE but MT8915 is the only one we have.

The change is component-level so theoretically it should be platform-independent. If possible @keyonjie can you share what error Intel non-cAVS platforms show? Then we can find out what this change is missing. Thanks!

you can just click "Details" of "sof-ci/jenkins/pr-device-test" to see the on device test result, or here is the link:
https://sof-ci.01.org/sofpr/PR5001/build11164/devicetest/

@keyonjie
Copy link
Contributor

@fuyuntsuo some logs quoted from the etrace:

[   364066419.127050] (      277631.093750) c0 dma-trace              src/drivers/dw/dma.c:284  ERROR dw_dma_start(): dma 1 channel 1 not ready ena 0x6 status 0x3
[   364066548.502045] (         129.375000) c0 dma-trace                  src/audio/host.c:153  ERROR host_dma_set_config_and_copy(): dma_copy() failed, ret = -16
[   364066691.366623] (         142.864578) c0 dma-trace          ....../pipeline-stream.c:134  ERROR pipeline_copy(): ret = -16, start->comp.id = 9, dir = 1
[   364066816.679118] (         125.312492) c0 dma-trace          ..../pipeline-schedule.c:160  ERROR pipeline_task(): xrun recovery failed! pipeline is stopped.

@fuyuntsuo
Copy link
Contributor Author

@keyonjie Thanks for the hint! Seems like this DMA call sequence does not apply to all the platforms selecting CONFIG_HOST_PTABLE.

Unfortunately we have only MT8195 to verify this PR. Can anyone advise how to avoid breaking Intel's non-cAVS platfroms?

@lgirdwood
Copy link
Member

I think this fix to force DMA to copy the block size is a needed feature, but we probably want to have this a Kconfig option where it's default can be set y or n depending on the platform.

@keyonjie
Copy link
Contributor

SOFCI TEST

@fuyuntsuo
Copy link
Contributor Author

fuyuntsuo commented Nov 22, 2021

SOFCI TEST

Thanks @keyonjie. Will manually trigger CI with this command by myself next time.

The PR doesn't break Intel non-cAVS platforms anymore (thanks @lyakh!). I will add a Kconfig later.

@lgirdwood
Copy link
Member

lgirdwood commented Nov 22, 2021

Lets take this for v2.0 with the Kconfig update.

@lgirdwood lgirdwood added this to the v2.0 milestone Nov 22, 2021
Host component stops copying bytes at element's end and resumes
copying the remaining bytes in the next round. This leads to jitter
between components of the same pipeline. Such jitter might cause
buffer overwrite for component which processes block by block.

Triggering extra DMA copy until all the copy_bytes are copied can
eliminate the jitter and hence no more buffer overwrite occurs.

Signed-off-by: fy.tsuo <fy.tsuo@intelli-go.com>
return ret;
}

ret = dma_copy(hd->chan, copy_bytes, DMA_COPY_ONE_SHOT);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just wondering: does this needs DMA_COPY_BLOCKING flag too?

Copy link
Member

Choose a reason for hiding this comment

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

it should block the caller until DMA copy completes.

@lgirdwood
Copy link
Member

@yaochunhung @kuanhsuncheng good for you ?

@keyonjie keyonjie self-requested a review November 26, 2021 06:21
@lgirdwood
Copy link
Member

CI failures unrelated and known.

@lgirdwood lgirdwood merged commit 3ffeb87 into thesofproject:main Nov 26, 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.

6 participants