drivers: dma: intel-adsp-gpdma: Account for LLPL wrapping #77747
Conversation
In case the LLP wraps we need to re-read the LLPU to make sure we return the correct value. Suggested-by: Pierre-Louis Bossart <[email protected]> Signed-off-by: Peter Ujfalusi <[email protected]>
| if (tmp > *llp_l) { | ||
| /* re-read the LLPU value, as LLPL just wrapped */ | ||
| *llp_u = dw_read(dev_cfg->shim, GPDMA_CHLLPU(channel)); | ||
| } |
There was a problem hiding this comment.
is this always called with interrupts blocked or can this be preempted? If the latter, at least in theory it could wrap again between lines 136 and 139? I think a traditional way to do this is to perform 3 measurements and after that to check for consistency. Potentially in a loop. Something line
for (l2 = read_low(), l1 = l2 + 1; l2 < l1; l2 = read_low()) {
h = read_high();
l1 = l2;
}
or
for (h2 = read_high(), h1 = h2 + 1; h1 != h2; h2 = read_high()) {
l = read_low();
h1 = h2;
}
There was a problem hiding this comment.
Is it reasonable to think that the DMA moves 4294967295 bytes right after it has just wrapped?
There was a problem hiding this comment.
of course not, "this error can never happen" (TM) ;-) But that's the reason why this isn't a blocker, but just a comment. Also it "this error is purely theoretical" but it costs me almost nothing to account for it, I might as well decide to do so
There was a problem hiding this comment.
I don't see the point of covering for a case which by the laws of physics and electronics cannot happen.
If the llp_l wraps again just after it has wrapped while we read llp_u (u32) register, that would mean that the DMA managed to copy 4GB. Given that we use the GPDMA for audio, that would likely take some time...
I prefer simple code compared to something that you need to spend time to figure out what is going on and I'm still not sure if I follow the two snippets you presented, but that is just me.
There was a problem hiding this comment.
@ujfalusi can this be interrupted? If this is running in a simple thread context, it can be preempted for a while
There was a problem hiding this comment.
I think it can but likely for shorter period than 1.53 hours (96KHz, S32_LE, stereo)
In case the LLP wraps we need to re-read the LLPU to make sure we return the correct value.
Suggested-by: Pierre-Louis Bossart [email protected]