atsamd5x: improve USBCDC

[sago35]

Increased speed by increasing the size of the data to be sent at a time.
I also made it so that it waits more than 1ms before timeout, so it probably won't time out.

Right now we're using the Get() process from a RingBuffer, but we can optimize it further and send the data from the RingBuffer directly.
However, the code is complicated by the fact that the ADDR register needs to be a 4 byte align.

When the PR is merged, #1175 in atsamd5x is solved.

This PR is designed to flush() every 1ms.

[sago35]

Since up to 128 bytes can be sent in a single pass, writing up to 128 bytes is very fast.
It is not very efficient when split into multiple Flush()'s, as there is a gap between Flush() and Flush().
However, it is much faster than the original implementation.

We can make it even faster by using the send completion interrupt.

• 10242 byte
  • old : 139.580 ms : 73377 B/s
  • new : 80.004 ms : 128018 B/s
• 256 byte
  • old : 3.509 ms : 72955 B/s
  • new : 1.235 ms : 207287 B/s
• 128 byte
  • old : 1.711 ms : 74810 B/s
  • new : 0.154 ms : 831168 B/s
• 14 byte
  • old : 184.75 us : 75778 B/s
  • new : 17.41 us : 804135 B/s

testcode:

testdata := [10242]byte{}

machine.LED.High()
machine.UART0.Write(testdata[:])
machine.LED.Low()

[sago35]

You can increase the size of the RingBuffer by setting the following head and tail to volatile.Register16
If the size is increased, the amount of data that can be sent in one flush() will be larger and the transfer efficiency will improve.

// src/machine/buffer.go
type RingBuffer struct {
	rxbuffer [bufferSize]volatile.Register8
	head     volatile.Register8
	tail     volatile.Register8
}

If you want to make it larger than 128, you must also change the following
Because the size of udd_ep_in_cache_buffer is 128.

usbEndpointDescriptors[usb_CDC_ENDPOINT_IN].DeviceDescBank[1].ADDR.Set(uint32(uintptr(unsafe.Pointer(&udd_ep_in_cache_buffer[usb_CDC_ENDPOINT_IN]))))

[deadprogram]

@sago35 it appears you have implemented the interrupt solution:

if i == usb_CDC_ENDPOINT_IN {
	UART0.waitTxc = false
}

However it appears that your proposed udd_ep_in_cache_buffer size increase is not in this PR.

Is that correct?

[deadprogram]

Also, it would seem that a similar change could be made to the SAMD21 support, is that true @sago35 ?

[sago35]

@deadprogram

However it appears that your proposed udd_ep_in_cache_buffer size increase is not in this PR.

I'll do a git push later.

Also, it would seem that a similar change could be made to the SAMD21 support, is that true

Yes.
SAMD21 can be modified in the same way.

but,,,
There's a race condition that needs to be resolved for now.
I'd like to use interrupt.Disable(), but it's not currently available in the machine package.

[aykevl]

I'm not sure I fully understand this code, but I don't see anything weird with it.

Sidenote: this gives me even more reason that USB should be separated out in a new package. I think the code would be easier to follow if the code that sends a buffer over USB and the code that buffers for the next transmission is cleanly separated with an API.

[deadprogram]

@sago35 what is the specific race condition you are seeing?

[sago35]

what is the specific race condition you are seeing?

Calling usbcdc.TxBuffer.Put(c) with multiple priorities will cause problems.

interrupt.Disable() can be built and used with interp-rewrite branch.
I believe #1430 will be merged with tinygo 0.17.0, so this PR will also be targeted at 0.17.0.

Note that this should probably be merged after the next release, to give it some time to find possible bugs.
#1430 (comment)

[sago35]

This process is a byte copy, but it's a waste of time.
I will create a new PR that improves on this part of the process.

[sago35]

RingBuffer.Put() does not consider calling from multiple priorities.
interrupt.Disable() is required.

[deadprogram]

Now that #1481 has been merged, I will close this PR. Thanks again for the great work @sago35