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

dw-dma: fix the lli corruption issue #4640

Merged
merged 3 commits into from
Sep 2, 2021

Conversation

keyonjie
Copy link
Contributor

@keyonjie keyonjie commented Aug 16, 2021

dw-dma: change to use runtime_shared heap for lli buffer

The lli structs are used by both DSP and DMAC, allocate them from the
runtime_shared heap to avoid being corrupted by cache writing back.

This helps to fix a lot of DMA xrun issue according to the validation.

Signed-off-by: Keyon Jie yang.jie@linux.intel.com

@@ -439,7 +439,7 @@ static int dw_dma_stop(struct dma_chan_data *channel)
}

dcache_writeback_region(dw_chan->lli,
sizeof(struct dw_lli) * channel->desc_count);
sizeof(struct dw_lli) * (channel->desc_count + 2));
Copy link
Member

Choose a reason for hiding this comment

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

@keyonjie looks like we need to make this a CONFIG option. CONFIG_DMA_DW_LLI_CACHE_LINE_SIZE(force LLI to be cache line size) and CONFIG_DMA_DW_LLI_PADDING (force LLI to be padded in LLI blocks). This way we can disable on legacy platforms and use only on Intel CAVS HW.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lgirdwood I didn't have intent to make this get merged to any branch, I will go forward to pursue explicit root cause of it.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @keyonjie , we will also need a version for main branch.

@keyonjie keyonjie force-pushed the tgl-dev branch 2 times, most recently from c9b977e to 28b3068 Compare August 17, 2021 12:14
@fuyuntsuo
Copy link
Contributor

Just wondering why only LLI struct needs to be aligned with the cache line size

@lgirdwood
Copy link
Member

Just wondering why only LLI struct needs to be aligned with the cache line size

The IO buses that connect all the IP will have constraints around bus arbitration and data transfers and the DW DMAC IP config may be configured with a min transfer size R and W of 128 bytes on TGL. No impact on audio transfers are buffers/periods are all > 128 bytes.

@keyonjie this also makes me think we may need a constraint on the audio buffer sizes on TGL for DW DMA so period sizes and addresses are aligned to 128 bytes (which I think is true today in all topologies but not enforced).

@plbossart
Copy link
Member

plbossart commented Aug 17, 2021

Just wondering why only LLI struct needs to be aligned with the cache line size

The IO buses that connect all the IP will have constraints around bus arbitration and data transfers and the DW DMAC IP config may be configured with a min transfer size R and W of 128 bytes on TGL. No impact on audio transfers are buffers/periods are all > 128 bytes.

@keyonjie this also makes me think we may need a constraint on the audio buffer sizes on TGL for DW DMA so period sizes and addresses are aligned to 128 bytes (which I think is true today in all topologies but not enforced).

It would be surprising, with our use of a 1ms tick it's not clear how the DMA sizes might be aligned to 128 bytes. Edit: the address could be fine, it's the size that would seem problematic.

@lgirdwood
Copy link
Member

It would be surprising, with our use of a 1ms tick it's not clear how the DMA sizes might be aligned to 128 bytes. Edit: the address could be fine, it's the size that would seem problematic.

Just checked topology, the S32_LE stereo comply with the size, but S16_LE don't (and work). My thinking now is that the LLI writeback uses a HW defined DMAC channel, wordsize, burstsize, and block size (meaning we only see this for LLI copies).

@fuyuntsuo
Copy link
Contributor

Just wondering why only LLI struct needs to be aligned with the cache line size

The IO buses that connect all the IP will have constraints around bus arbitration and data transfers and the DW DMAC IP config may be configured with a min transfer size R and W of 128 bytes on TGL. No impact on audio transfers are buffers/periods are all > 128 bytes.

@keyonjie this also makes me think we may need a constraint on the audio buffer sizes on TGL for DW DMA so period sizes and addresses are aligned to 128 bytes (which I think is true today in all topologies but not enforced).

Thanks. Just find a related PR #3645 and learn that the lli buffer is shared by both DSP and DMA controller.

This reverts commit f51d4ab.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Change runtime_shared heap to be dma-able, which could be use for DMA
usage.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
The lli structs are used by both DSP and DMAC, allocate them from the
runtime_shared heap to avoid being corrupted by cache writing back.

This helps to fix a lot of DMA xrun issue according to the validation.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
@keyonjie keyonjie changed the title dma: workaround to fix the lli corruption issue dw-dma: fix the lli corruption issue Aug 20, 2021
@keyonjie keyonjie marked this pull request as ready for review August 20, 2021 10:54
@mwasko mwasko merged commit 606c636 into thesofproject:tgl-013-drop-stable Sep 2, 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