samd51: Work around DMA hang

[jepler]

There's a hang when using multiple DMA channels on SAMD51 that has affected several projects based on CircuitPython and on Arduino (nofrendo_arcada).

This patch attempts to address the DMA hang for Circuitpython, in the specific case where audio hangs while doing DMA for display. It should also help if there are DMA hangs while doing other SPI or QSPI activity, but I never personally observed a hang other than from display-during-audio.

There's no particular reason I chose 10 iterations for the loop; in CircuitPython, we know that there are at least 16 bytes of transfer to do, so probably checking 10 times in a row wouldn't lengthen a short transaction.

Testing performed: On a pygamer, with an additional local patch that tracked the number of times the (!is_okay) branch was entered, I found that it was entered around 2-3 times per hour during my audio playback with display test, and zero hangs were observed.

Because I was not in the room when it was playing, I didn't hear whether audible glitches occur.

[jepler]

@tannewt I can't request review in here, but it would be great if you could take a look at this or send it on to the right person!

[dhalbert]

I can't say I see any issues with the current code, but there's a lot of confusing dead code and comments.

I think sercom index is incorrect for SAMD51 below may no longer be true, because it looks it up. But need to check this.

samd-peripherals/samd/dma.c

Lines 113 to 114 in 28ad937

	// sercom index is incorrect for SAMD51
	dma_configure(SHARED_TX_CHANNEL, sercom_index(peripheral) * 2 + FIRST_SERCOM_TX_TRIGSRC, false);

Not sure what is going on below with the commented-out stuff:

samd-peripherals/samd/dma.c

Lines 155 to 160 in 28ad937

	if (sercom) {
	SercomSpi *s = &((Sercom*) peripheral)->SPI;
	s->INTFLAG.reg = SERCOM_SPI_INTFLAG_RXC | SERCOM_SPI_INTFLAG_DRE;
	} else {
	//QSPI->INTFLAG.reg = QSPI_INTFLAG_RXC | QSPI_INTFLAG_DRE;
	}

The comments below seem completely outdated and, like above, not sure what is going on with the if. We did use to trigger DMA by doing direct writes (I looked at or worked on that code a long time ago). I think the pragma stuff could go away.

samd-peripherals/samd/dma.c

Lines 171 to 184 in 28ad937

	if (sercom) {
	//DMAC->SWTRIGCTRL.reg |= (1 << SHARED_TX_CHANNEL);
	} else {
	// Do a manual copy to trigger then DMA. We do 32-bit accesses to match the DMA.
	#pragma GCC diagnostic push
	#pragma GCC diagnostic ignored "-Wcast-align"
	if (rx_active) {
	//buffer_in[0] = *src;
	DMAC->SWTRIGCTRL.reg |= (1 << SHARED_RX_CHANNEL);
	} else {
	//*(uint32_t*)dest = ((uint32_t*) buffer_out)[0];
	}
	#pragma GCC diagnostic pop
	}

The SWTRIGCTRL part of the datasheet says:

Bits 31:0 – SWTRIGn[31:0] Channel n Software Trigger [n = 31..0]
This bit is cleared when the Channel Pending bit in the Channel Status register (CHSTATUS.PEND) for
the corresponding channel is either set, or by writing a '1' to it.
This bit is set if CHSTATUS.PEND is already '1' when writing a '1' to that bit.
Writing a '0' to this bit will clear the bit.
Writing a '1' to this bit will generate a DMA software trigger on channel x, if CHSTATUS.PEND=0 for
channel x. CHSTATUS.PEND will be set and SWTRIGn will remain cleared.

If CHSTATUS.PEND is set, then the DMA won't be triggered, it sounds like. In your debugging, did you look for this. Is that something to busy-wait on?

[jepler]

Fwiw the change is intended to fix adafruit/circuitpython#2005 rather than generally address problems like @dhalbert mentions.

[dhalbert]

@jepler Sorry for introducing general code cleanup, but I was trying to see if some previous code change introduced a problem, and the current state of the code is confusing. I do wonder about the CHSTATUS.PEND thing which I mentioned last. Also , is there any timing issue between enabling the channels and doing the SWTRIG? If you inserted a delay there of 10 or 100 usecs, does the problem go away? That is not necessarily a fix: maybe instead there's a status bit that could be checked before triggering.

[dhalbert]

@tannewt I can't request review in here, but it would be great if you could take a look at this or send it on to the right person!

@jepler I made you a collaborator on this repo.

[jepler]

In a fresh commit, I updated the comments based on @dhalbert 's comments and my own understanding of the code, and removed dead code.

I don't think SWTRIGCTRL figures into this, because that's for the !sercom case (i.e., qspi) and we're observing the hang in the case of SD card access.

[tannewt]

Looks good to me! Sorry for the dead code.

[tannewt]

@dhalbert Feel free to merge if you are happy.

[dhalbert]

Thank you for the cleanup! This is OK by me now.