[keyonjie]

For dw dma controllers and channels, they are shared to multiple pipelines and cores.
For dai config and ssp config, the configuration may happen on different core with the one on the active time.

Add cache invalidate and writeback to ensure the consistency, to address issue #3953

[paulstelian97]

Everything else looks good, not sure why this is going away though. What was the logic here and why is this no longer needed?

[lgirdwood]

@keyonjie this code clears all the DONE bits in the LLI that's be written by HW. If these bits are not clear then DMA will stop.

[keyonjie]

@paulstelian97 @lgirdwood this part is covered by above Zephyr lines:

		for (i = 0; i < channel->desc_count; i++)	
			dw_chan->lli[i].ctrl_hi &= ~DW_CTLH_DONE(1);

[lgirdwood]

Sorry I mean, when we dont use Zephyr ?

[keyonjie]

Sorry I mean, when we dont use Zephyr ?

before change,
Zephyr case: do invalidate -> write -> writeback
Non-Zephyr case: just write.
after my change,
unified for Zephyr and Non-Zephyr cases('#if defined ZEPHYR' is removed): do invalidate -> write -> writeback

[lgirdwood]

This is racy and there is no easy way around it. We are racing against the DMAC writing back the LLI data which could occur during this loop. This may be safer if we allocate LLI as an uncache region. i.e. both DW driver and DMAC use the uncache address for LLI.

[lgirdwood]

I assume we are good for alh and dmic ?

[lgirdwood]

@keyonjie this code clears all the DONE bits in the LLI that's be written by HW. If these bits are not clear then DMA will stop.

[keyonjie]

looks this fail on APL, mark it draft as debugging needed.

[ranj063]

spurious line

[keyonjie]

corresponding to the line 829, an empty line between variables and codes.

[keyonjie]

SOFCI TEST

[lgirdwood]

I think it probably make a lot more sense to allocate shared configuration data using the uncached alias, this way we dont have to manually remember to inv or wb and it keeps the code simpler.
Btw, there is still a CI failure in APL nocodec.

[lgirdwood]

These should be after the spin_lock (since the lock has a memory barrier built in)

[lgirdwood]

before the unlock

[lgirdwood]

This is racy and there is no easy way around it. We are racing against the DMAC writing back the LLI data which could occur during this loop. This may be safer if we allocate LLI as an uncache region. i.e. both DW driver and DMAC use the uncache address for LLI.

[paulstelian97]

I think it probably make a lot more sense to allocate shared configuration data using the uncached alias, this way we dont have to manually remember to inv or wb and it keeps the code simpler.

While this is probably fine for now, the code should be compatible with switching back to being able to correctly support either; if i.MX ever comes up with a multicore DSP, well we don't have uncached aliases so we'd have to use cached memory for that.

[lgirdwood]

I think it probably make a lot more sense to allocate shared configuration data using the uncached alias, this way we dont have to manually remember to inv or wb and it keeps the code simpler.

While this is probably fine for now, the code should be compatible with switching back to being able to correctly support either; if i.MX ever comes up with a multicore DSP, well we don't have uncached aliases so we'd have to use cached memory for that.

Agree, but the driver will be fully integrated with Zephyr at this point so that would then have to align with any Zephyr cache APIs.

[keyonjie]

@lgirdwood @paulstelian97 I just change to use small change as @lgirdwood suggested, allocated shared uncached memory where needed, now the change is very minor while addressing multi-core issue.

[keyonjie]

SOFCI TEST

[lgirdwood]

This should go for rc3

[lgirdwood]

@keyonjie CI looking clean now - we can ignore the DUT boot failure. Lets takes for rc3. @slawblauciak fyi

[keyonjie]

Thanks for reviewing, as it is minor change and fixes multi-core xrun issue, I will merge it myself.