Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HAL_ChibiOS: add fair scheduling for UARTs #14604

Merged
merged 1 commit into from
Apr 7, 2021

Conversation

tridge
Copy link
Contributor

@tridge tridge commented Jun 15, 2020

if two UARTs have DMA contention then we can get 2nd UART always
failing to send as as always do sends in the same order per 1kHz
loop. This ensures we give equal priority to all UARTs

see issue #14581

@bugobliterator
Copy link
Member

Looks good to me.

@bugobliterator bugobliterator self-requested a review June 15, 2020 11:11
@peterbarker
Copy link
Contributor

So I think this patch is reasonable, but it doesn't solve the problem it was intended to. So probably a merge but no backport.

@jschall
Copy link
Contributor

jschall commented Jun 15, 2020

If the DMA is busy when there are bytes to send or receive, the CPU should do it.

@tridge
Copy link
Contributor Author

tridge commented Sep 24, 2020

If the DMA is busy when there are bytes to send or receive, the CPU should do it.

for recv that can't happen as we don't have receive DMA. For send I agree that it would be nice to switch to interrupt driven when busy

@tridge
Copy link
Contributor Author

tridge commented Oct 13, 2020

rebased

@yaapu
Copy link
Contributor

yaapu commented Oct 14, 2020

@tridge I did some testing on a MatekF405-Wing using UART5 (RX5/TX5) and UART1 (RX1/TX1), I'm logging CRSF telemetry rates every 5 seconds, should be around 150Hz

It looks like the patch does not improve UART5 peformance.

Note: CRSF protocol is hardcoded @416666 baud

    _uart->set_flow_control(AP_HAL::UARTDriver::FLOW_CONTROL_DISABLE);
    _uart->set_unbuffered_writes(true);
    _uart->set_blocking_writes(false);

UART5 with this patch:

APM: RCInput: decoding CRSF
APM: CRSF packet rate 6
APM: CRSF packet rate 12
APM: CRSF packet rate 13
APM: CRSF packet rate 12
APM: CRSF packet rate 10
APM: CRSF packet rate 10
APM: CRSF packet rate 13
APM: CRSF packet rate 14
APM: CRSF packet rate 16
APM: CRSF packet rate 16
APM: CRSF packet rate 11

UART5 without this patch:

APM: RCInput: decoding CRSF
APM: CRSF packet rate 13
APM: CRSF packet rate 23
APM: CRSF packet rate 21
APM: CRSF packet rate 17
APM: CRSF packet rate 22
APM: CRSF packet rate 23
APM: CRSF packet rate 21
APM: CRSF packet rate 22
APM: CRSF packet rate 25
APM: CRSF packet rate 13
APM: CRSF packet rate 27

UART1 with this patch

APM: RCInput: decoding CRSF
APM: CRSF packet rate 125
APM: CRSF packet rate 131
APM: CRSF packet rate 142
APM: CRSF packet rate 144
APM: CRSF packet rate 145
APM: CRSF packet rate 150
APM: CRSF packet rate 133
APM: CRSF packet rate 137

UART1 without this patch

APM: RCInput: decoding CRSF
APM: CRSF packet rate 107
APM: CRSF packet rate 127
APM: CRSF packet rate 143
APM: CRSF packet rate 135
APM: CRSF packet rate 137
APM: CRSF packet rate 137
APM: CRSF packet rate 143

@davidbuzz
Copy link
Collaborator

is there any reason this wasnt merged?

@amilcarlucas
Copy link
Contributor

How does this relate to #15984 ?

@andyp1per
Copy link
Collaborator

@amilcarlucas this should be fixed by #15984 as I believe we will then rely on the thread scheduling algorithm which tries to be fair.

@IamPete1
Copy link
Member

This issue seems to be related to this, #16587

@tridge
Copy link
Contributor Author

tridge commented Feb 11, 2021

This issue seems to be related to this, #16587

no, I don't think it is related. See fix for that in #16590

switch to interrupt driven when in high contention
@tridge
Copy link
Contributor Author

tridge commented Apr 2, 2021

rebased, now includes only the dma disable change

@tridge tridge merged commit 47a5d78 into ArduPilot:master Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Copter 4.0 backports
Awaiting triage
Plane 4.0 Backports
  
Awaiting triage
Rover 4.0 backports
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

10 participants