TC / TCC All timers in use SAMD51

wallarug commented 5 years ago

Hi !

I am having more issues with timers and CircuitPython using a SAMD51G19A this time.

I have been doing some testing and discovered that the CircuitPython is reporting that all timers are being used. I have done a table check and think this might be wrong or not using all the available channels. - happy to be told otherwise.

From the Datasheet

Servo	Pin	TC	TCC	TCC
8	PA08	0.0	0.0	1.4
7	PA09	0.1	0.1	1.5
6	PA10	1.0	0.2	1.6
5	PA11	1.1	0.3	1.7
1	PA18	3.0	1.2	0.6
2	PA19	3.1	1.3	0.7
3	PA20	7.0*	1.4	0.0
4	PA21	7.1*	1.5	0.1

not available

I am aware of:

TC channel 0 cannot be used as per #1766 #1776
TC7 does not exist
TCC0 and TCC1 should both have 6 Compare/Capture channels (12 total)

Test One:

[INFO] START - SERVO Test
Initialising all servo Pins.  There could be errors at this stage relating to timers.  Swap pins to remove errors and note in documentation.
[LOOP] Initialising...microcontroller.pin.SERVO1
[LOOP] Initialising...microcontroller.pin.SERVO2
[LOOP] Initialising...microcontroller.pin.SERVO3
[LOOP] Initialising...microcontroller.pin.SERVO4
[LOOP] Initialising...microcontroller.pin.SERVO5
[LOOP] Initialising...microcontroller.pin.SERVO6
[LOOP] Initialising...microcontroller.pin.SERVO7
[LOOP] Initialising...microcontroller.pin.SERVO8
Traceback (most recent call last):
  File "code.py", line 106, in <module>
ValueError: All timers for this pin are in use

Looks like a "not enough timers" failure on SERVO8 (PA08). However I believe the first TCC timer should be selected TCC0[0] because it is not used. Servo 1 to 4 should choose the TCC1 and Servo 5 to 7 are exclusive TCC0 channels.

Test Two:

[INFO] START - SERVO Test
Initialising all servo Pins.  There could be errors at this stage relating to timers.  Swap pins to remove errors and note in documentation.
[LOOP] Initialising...microcontroller.pin.SERVO5
[LOOP] Initialising...microcontroller.pin.SERVO6
[LOOP] Initialising...microcontroller.pin.SERVO7
[LOOP] Initialising...microcontroller.pin.SERVO8
[LOOP] Initialising...microcontroller.pin.SERVO1
[LOOP] Initialising...microcontroller.pin.SERVO2
[LOOP] Initialising...microcontroller.pin.SERVO3
Traceback (most recent call last):
  File "code.py", line 105, in <module>
RuntimeError: All timers in use

This looks like a different error telling me that CircuitPython cannot function because I used all the timers before I got to SERVO4.

I am at a bit of a loss of what is happening here - I would have thought having up to 3 timers to choose from would result in no issues.

Is there anyway to get debug output of which timers CircuitPython is selecting?

Thanks in advance for any assistance.

dhalbert commented 5 years ago

Could you post your entire test program?

wallarug commented 5 years ago

No problemo!

It is here (posted link to save on comment space): https://github.com/robotics-masters/mm1-hat-cpy-native/blob/master/experiments/backup-v1.5/tests.py#L97

dhalbert commented 5 years ago

I am looking at https://github.com/adafruit/circuitpython/blob/master/ports/atmel-samd/boards/robohatmm1/pins.c, and I see these pin definitions, which are somewhat different that you posted in the first post:

    // SERVO Pins
    { MP_ROM_QSTR(MP_QSTR_SERVO1), MP_ROM_PTR(&pin_PA16) },
    { MP_ROM_QSTR(MP_QSTR_SERVO2), MP_ROM_PTR(&pin_PA17) },
    { MP_ROM_QSTR(MP_QSTR_SERVO3), MP_ROM_PTR(&pin_PA18) },
    { MP_ROM_QSTR(MP_QSTR_SERVO4), MP_ROM_PTR(&pin_PA19) },
    { MP_ROM_QSTR(MP_QSTR_SERVO5), MP_ROM_PTR(&pin_PA11) },
    { MP_ROM_QSTR(MP_QSTR_SERVO6), MP_ROM_PTR(&pin_PA10) },
    { MP_ROM_QSTR(MP_QSTR_SERVO7), MP_ROM_PTR(&pin_PA09) },
    { MP_ROM_QSTR(MP_QSTR_SERVO8), MP_ROM_PTR(&pin_PA08) },

dhalbert commented 5 years ago

OK, never mind, that's the SAMD21 board. But where is the SAMD51 board definition?

dhalbert commented 5 years ago

You can add some print statements in common-hal/pulseio/PWMOut.c to print out the timers and channels. I started to do this but it would be more direct if you did. To print, do this (adjust args as necessary):

mp_print(&mp_plat_print, "%d %d\n", something, something);

I did add a print statement and tried this on a Metro M4 but unfortunately not all the pins you are using are available on that board.

I am not sure, but I think the assignment logic more goes by the FUNCTION position in the pin table first than by the TCC number. The functions are E, F, G, where E is TC, and F and G are TCC's (usually). But on some pins, F may be TCC0 and G may be TCC1, whereas on others F may be TCC1 and G may be TCC0. So I don't think it tries to fill up TCC0 completely first, necessarily. In the limited testing I did it aassigned TCC0 for PA18 and PA19 (function column F), and then switched to TCC1 for PA20 and PA21 (also function column F).

I would try more variations on the ordering if you're having trouble assigning all of them.

wallarug commented 5 years ago

Hi @dhalbert ,

It has been merged in last night to the main CircuitPython repo in PR #2014 :

https://github.com/adafruit/circuitpython/blob/master/ports/atmel-samd/boards/robohatmm1_m4/pins.c

        { MP_ROM_QSTR(MP_QSTR_SERVO1), MP_ROM_PTR(&pin_PA18) },
    { MP_ROM_QSTR(MP_QSTR_SERVO2), MP_ROM_PTR(&pin_PA19) },
    { MP_ROM_QSTR(MP_QSTR_SERVO3), MP_ROM_PTR(&pin_PA20) },
    { MP_ROM_QSTR(MP_QSTR_SERVO4), MP_ROM_PTR(&pin_PA21) },
    { MP_ROM_QSTR(MP_QSTR_SERVO5), MP_ROM_PTR(&pin_PA11) },
    { MP_ROM_QSTR(MP_QSTR_SERVO6), MP_ROM_PTR(&pin_PA10) },
    { MP_ROM_QSTR(MP_QSTR_SERVO7), MP_ROM_PTR(&pin_PA09) },
    { MP_ROM_QSTR(MP_QSTR_SERVO8), MP_ROM_PTR(&pin_PA08) },

I'll give this a test tonight and see what is happening. Even if it is doing it logically by the table there should be plenty of TC and TCC channels free.

wallarug commented 5 years ago

Below are some test results:

[LOOP] Initialising...microcontroller.pin.SERVO5
use new... TCC1[7] 2
[LOOP] Initialising...microcontroller.pin.SERVO6
use existing... TCC1[6] 2 1
[LOOP] Initialising...microcontroller.pin.SERVO7
use existing... TCC1[5] 2 1
[LOOP] Initialising...microcontroller.pin.SERVO8
use existing... TCC1[4] 2 1
[LOOP] Initialising...microcontroller.pin.SERVO1
use new... TCC0[6] 2
[LOOP] Initialising...microcontroller.pin.SERVO2
use existing... TCC0[7] 2 0
[LOOP] Initialising...microcontroller.pin.SERVO3
Traceback (most recent call last):
  File "code.py", line 36, in <module>
RuntimeError: All timers in use

Question: What is wrong with TCC0[0] for Servo 3?

[LOOP] Initialising...microcontroller.pin.SERVO1
use new... TCC0[6] 2
[LOOP] Initialising...microcontroller.pin.SERVO2
use existing... TCC0[7] 2 0
[LOOP] Initialising...microcontroller.pin.SERVO3
use new... TCC1[4] 1
[LOOP] Initialising...microcontroller.pin.SERVO4
use existing... TCC1[5] 1 1
[LOOP] Initialising...microcontroller.pin.SERVO5
use existing... TCC0[3] 1 0
[LOOP] Initialising...microcontroller.pin.SERVO6
use existing... TCC0[2] 1 0
[LOOP] Initialising...microcontroller.pin.SERVO7
use new... TC0[1] 0
[LOOP] Initialising...microcontroller.pin.SERVO8
Traceback (most recent call last):
  File "code.py", line 36, in <module>
ValueError: All timers for this pin are in use

Question: What is wrong with TCC0[0] for Servo 8?

[LOOP] Initialising...microcontroller.pin.SERVO3
use new... TCC0[0] 2
[LOOP] Initialising...microcontroller.pin.SERVO8
use new... TCC1[4] 2
[LOOP] Initialising...microcontroller.pin.SERVO1
use existing... TCC1[2] 1 1
[LOOP] Initialising...microcontroller.pin.SERVO2
use existing... TCC0[7] 2 0
[LOOP] Initialising...microcontroller.pin.SERVO4
use existing... TCC1[5] 1 1
[LOOP] Initialising...microcontroller.pin.SERVO5
use existing... TCC0[3] 1 0
[LOOP] Initialising...microcontroller.pin.SERVO6
use existing... TCC0[2] 1 0
[LOOP] Initialising...microcontroller.pin.SERVO7
use new... TC0[1] 0
[INFO] Finished setting up all servo pins, without issue.

All ok in this test. Moving SERVO3 and SERVO8 to the start.

Edit: Added another test.

wallarug commented 5 years ago

I guess this sort of shows the issue.

TCC 0 is not being selected in some cases.

I have a working theory on this one. I think there could be an issue with the logic of selecting pins with the same existing TCC timer. Since this is the first part of the function - in other words: It will also check on an existing first - it might rule out some valid pins in later stages.

I am doing some more testing on this now to see if it can be worked out. I see no reason that a valid pin should not be selected based on the order the Pins are initialised.

dhalbert commented 5 years ago

Yes, it's quite possible there's a bug in the "what is free?" logic. It wasn't scanning in the way I thought it would, but I haven't had time to desk-check the code with a number of examples.

wallarug commented 5 years ago

Thanks @dhalbert for all your assistance / advise on this.

Two things at play here:

I think I just have another unfortunate pin selection.
It is going by lowest TCC timer rather than 'first function'

I have been working off this table for which pins are sharing CC Channels:

Compare Channel: 4 Wave Output: 8 Valid for: SAMD21 TCC0, TCC1 - SAMD51 TCC1

0	1	2	3
4	5	6	7

In fact for TCC0 on SAMD51 it should be:

Compare Channel: 6 Wave Output: 8

0	1	2	3	4	5
6	7

This then makes sense as to why it is upset about choosing TCC0[0] for SERVO8 since TCC0[6] was selected for SERVO1.

Here is the interesting point. Based on my desk checks... if the timers were selected as you described @dhalbert - based on the function mux table - I would not have any issues 😄 .

I guess the easy solution would be: just initialise them in a different order (which keeps getting suggested). That will be my solution to this for now.

Feature Request 1

Please complete #1766 so that TCx_0 can be used. That would be my number one suggestion.

Feature Request 2

I would prefer a more advanced feature in the future that it could dynamically check and reallocate timers. I think this becomes a question of if you can keep a track of the pins as well as the timer usages. If know how PWMOut.c does this already - let me know and I might have a stab at the below suggestion.

How I would see this working is:

if no timers are available: check if another pin is using the timer (i.e. a timer is free but used - rather than 'this pin has no timers'.
if another pin is using a timer: check what is free on that pin (basically re-run the setup checks)
either: reallocate if free or send back no pins available (even after checking reallocation).

I would set a depth search limit of maybe 1 to start and then 2 in the future. Going any deeper could cause many issues.

Example

Configure SERVO8.

Normal Execution Path:

existing: invalid pin... TC/TCC0[0] Timer: 0 Instance: 0
existing: invalid pin... TC/TCC1[4] Timer: 2 Instance: 0
existing: invalid pin... TC/TCC0[0] Timer: 0 Instance: 1
existing: invalid pin... TC/TCC0[0] Timer: 1 Instance: 1
existing: no matching frequency... instance: 2 tcc_freq: 0 freq: 50
ValueError: All timers for this pin are in use

New Path:

existing: invalid pin... TC/TCC0[0] Timer: 0 Instance: 0
existing: invalid pin... TC/TCC1[4] Timer: 2 Instance: 0
existing: invalid pin... TC/TCC0[0] Timer: 0 Instance: 1
existing: invalid pin... TC/TCC0[0] Timer: 1 Instance: 1
existing: no matching frequency... instance: 2 tcc_freq: 0 freq: 50
all timers for this pin are in use, check other pins using timers
start with tcc0[0]
tcc0[0] used by servo1 on tcc0[6]
check alternative timer tcc1[2]
use new... TCC1[2] 1
timer free for pin!
use new...TCC0[0]

I am not sure about the difficulty of this but willing to give it a red hot go 🔥 if someone can provide direction about how/where the pin assignments are currently stored.

tannewt commented 5 years ago

All of the related logic is stored in PWMOut: https://github.com/adafruit/circuitpython/blob/master/ports/atmel-samd/common-hal/pulseio/PWMOut.c#L154

There is no central list of active PWMOuts so you'll need to track it if you plan on dynamically swapping them.

Adding support for TC[0] isn'd likely what you want because it'll drastically reduce the functionality of the PWMOut to 8 bit resolution or very course frequency (since the top can't change much).

wallarug commented 5 years ago

Thanks @tannewt . I'm having a play with it now. I think it could be doable within the same logic already there but would take a bit of time.

I discovered one issue already with the errors that are sent back when no timers are free.

If you look at my first output in this comment: https://github.com/adafruit/circuitpython/issues/2020#issuecomment-516826913

It says "All timers in use" instead of "All timers for this pin are in use"

I am not sure if this is expected behaviour but the reason for this is because that pin had no timer on the first function (TC7 doesn't exist). Then it cannot use the TCC channels since they are all used. I would have expect it to say "All timers for this pin are in use" .

The found = true only appears for the TC parts and not TCC. Is this deliberate?

Take a look at the section I am talking about below...

for (int8_t i = start; i >= 0 && i < NUM_TIMERS_PER_PIN && timer == NULL; i += direction) {
            const pin_timer_t* t = &pin->timer[i];
            if ((!t->is_tc && t->index >= TCC_INST_NUM) ||
                (t->is_tc && t->index >= TC_INST_NUM)) {
                continue;
            }
            if (t->is_tc) {
                found = true;
                Tc* tc = tc_insts[t->index];
                if (tc->COUNT16.CTRLA.bit.ENABLE == 0 && t->wave_output == 1) {
                    timer = t;
                    mux_position = i;
                }
            } else {
                Tcc* tcc = tcc_insts[t->index];
                if (tcc->CTRLA.bit.ENABLE == 0 && channel_ok(t)) {
                    timer = t;
                    mux_position = i;
                }
            }
        }

       if (timer == NULL) {
            if (found) {
                return PWMOUT_ALL_TIMERS_ON_PIN_IN_USE;
            }
            return PWMOUT_ALL_TIMERS_IN_USE;
        }

I would have thought found = true would be the same for TCCs as well?

tannewt commented 5 years ago

Nope, not deliberate. Just an oversight. Please fix if you can. :-)

wallarug commented 5 years ago

@kattni @rhooper I am taking another look at something very similar to #770 which both of you were involved in.

How the current PWMOut determines if the error says "All Timers for this pin are in use" and "All timers in use" is still not clear in my view.

For some reason the TCCs are treated differently to the TCs under this arrangement. You can have a situation where all the TCC timers are used, but it won't say that. It will just say "All timers in use".

Can we change what it says completely for these errors?

If a pin has:

timers, but are used: "All timers on pin in use"
no timers: "Unsupported operation" or "No timers free" (different meaning to "All Timers in use")

Changing the behaviour so TCC and TC behave the same is easy. I just wanted to know if there was a reason for them being different.

Edit: I found a case where there is no TC but the pin has TCC capability. This triggers the "All timers in use"

So this comes down to what is meant by "All timers in use" and "All timers for this pin are in use". It should be clearly defined somewhere.

Truth Table - what pins have to trigger error

TC	TCC	Error
x	x	All timers for this pin are in use
x	-	All timers for this pin are in use
-	x	All timers in use
-	-	Invalid pin

tannewt commented 5 years ago

I don't think there is a reason. It sounds like you have an improvement of the error messages.

On Tue, Aug 6, 2019 at 3:35 AM wallarug notifications@github.com wrote:

@kattni https://github.com/kattni @rhooper https://github.com/rhooper I am taking another look at something very similar to #770 https://github.com/adafruit/circuitpython/issues/770 which both of you were involved in.

How the current PWMOut determines if the error says "All Timers for this pin are in use" and "All timers in use" is still not clear in my view.

For some reason the TCCs are treated differently to the TCs under this arrangement. You can have a situation where all the TCC timers are used, but it won't say that. It will just say "All timers in use".

Can we change what it says completely for these errors?

If a pin has:

timers, but are used: "All timers on pin in use"

no timers: "Unsupported operation" or "No timers free" (different meaning to "All Timers in use")

Changing the behaviour so TCC and TC behave the same is easy. I just wanted to know if there was a reason for them being different.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/adafruit/circuitpython/issues/2020?email_source=notifications&email_token=AAAM3KKNRVFBBBN2MKNNJJTQDFHWXA5CNFSM4IH4HYA2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3UWX2Y#issuecomment-518613995, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAM3KNEWFD5RDM7UTFQHKTQDFHWXANCNFSM4IH4HYAQ .

adafruit / circuitpython

TC / TCC All timers in use SAMD51 #2020

Feature Request 1

Feature Request 2