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

boards: add centralized DMA map and enable SPI DMA on more boards #14207

Merged
merged 16 commits into from Mar 12, 2020

Conversation

dagar
Copy link
Member

@dagar dagar commented Feb 22, 2020

Let's give this another try. These headers seem a bit archaic compared to something with actual structure, but it seems workable for now until we're ready to jump into code generation.

Requires PX4/NuttX#85 for STM32F7.

@dagar
Copy link
Member Author

dagar commented Mar 2, 2020

@bkueng could you propose any changes you'd like to see here? Maybe mock up px4_fmu-v4 or v5.
509045b

@dagar dagar requested a review from bkueng March 2, 2020 19:15
@bkueng
Copy link
Member

bkueng commented Mar 3, 2020

Something like this (?):


|    DMA1    |      Stream 0      |        Stream 1        |     Stream 2     |        Stream 3        |      Stream 4      |  Stream 5  |      Stream 6       |       Stream 7       |
|------------|--------------------|------------------------|------------------|------------------------|--------------------|------------|---------------------|----------------------|
| Channel 0  | SPI3_RX_1          | SPDIFRX_DT             | SPI3_RX_2        | SPI2_RX                | SPI2_TX            | SPI3_TX_1  | SPDIFRX_CS          | SPI3_TX_2            |
| Channel 1  | I2C1_RX            | I2C3_RX                | TIM7_UP_1        |  -                     | TIM7_UP_2          |  I2C1_RX_1 | I2C1_TX             | I2C1_TX_1            |
| Channel 2  | TIM4_CH1           | -                      | I2C4_RX          | TIM4_CH2               | -                  | I2C4_RX    | TIM4_UP             | TIM4_CH3             |
| Channel 3  | -                  | TIM2_UP_1 TIM2_CH3     | I2C3_RX_1        | -                      | I2C3_TX            | TIM2_CH1   | TIM2_CH2 TIM2_CH4_1 | TIM2_UP_2 TIM2_CH4_2 |
| Channel 4  | UART5_RX           | USART3_RX              | UART4_RX         | USART3_TX_1            | UART4_TX           | USART2_RX  | USART2_TX           | UART5_TX             |
| Channel 5  | UART8_TX           | UART7_TX               | TIM3_CH4 TIM3_UP | UART7_RX               | TIM3_CH1 TIM3_TRIG | TIM3_CH2   | UART8_RX            | TIM3_CH3             |
| Channel 6  | TIM5_CH3 TIM5_UP_1 | TIM5_CH4_1 TIM5_TRIG_1 | TIM5_CH1         | TIM5_CH4_2 TIM5_TRIG_2 | TIM5_CH2           | -          | TIM5_UP_2           | -                    |
| Channel 7  | -                  | TIM6_UP                | I2C2_RX          | I2C2_RX_1              | USART3_TX_2        | DAC1       | DAC2                | I2C2_TX              |
|            |                    |                        |                  |                        |                    |            |                     |                      |
| Usage      | UART8_TX (IO)      | USART3_RX              | UART4_RX         | -                      | -                  | USART2_RX  | UART8_RX (IO)       | -                    |

@dagar
Copy link
Member Author

dagar commented Mar 3, 2020

That's certainly better as documentation. I was trying to find a line in between where we could handle the actual NuttX defines in order.

//  DMA2 Channel/Stream Selections
//--------------------------------------------//---------------------------//----------------
#define DMACHAN_SPI1_RX    DMAMAP_SPI1_RX_1   // DMA2, Stream 0, Channel 3    (SPI sensors RX)
#define DMAMAP_USART6_RX   DMAMAP_USART6_RX_1 // DMA2, Stream 1, Channel 4
#define DMAMAP_USART1_RX   DMAMAP_USART1_RX_1 // DMA2, Stream 2, Channel 4
#define DMACHAN_SPI1_TX    DMAMAP_SPI1_TX_1   // DMA2, Stream 3, Channel 3    (SPI sensors TX)
//      AVAILABLE                             // DMA2, Stream 4
//      DMAMAP_TIM1_UP                        // DMA2, Stream 5, Channel 6    (DSHOT)
#define DMAMAP_SDIO        DMAMAP_SDIO_2      // DMA2, Stream 6, Channel 4

I suppose we could just do both. The full table with usage followed by a simpler list of defines in stream order.

@bkueng
Copy link
Member

bkueng commented Mar 4, 2020

Let's do both then. I added the tables in the last commit, if you want I can amend it to the individual board commits.

@dagar
Copy link
Member Author

dagar commented Mar 10, 2020

Let's do both then. I added the tables in the last commit, if you want I can amend it to the individual board commits.

Thanks @bkueng. No, let's just keep going and finally get this in.

Copy link
Member

@davids5 davids5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dagar

The format of this is super for readability and DMA system usage overview!

The part I think that will need work (here in or the next PR) is the allocation sizes needed by the drivers. We may need to make this more dynamic and position RAM based on platform.

We also need to catch allocation failures.

Has this been bench tested on all HW?

@dagar
Copy link
Member Author

dagar commented Mar 12, 2020

I've backed out the change that removed the DMA capable check. Let's get this in and keep working towards an acceptable solution for stm32f7 and stm32h7.

@dagar dagar merged commit 0ce9e11 into master Mar 12, 2020
@dagar dagar deleted the pr-board_dma_maps branch March 12, 2020 14:03
@davids5
Copy link
Member

davids5 commented Mar 13, 2020

@dagar I am sorry it took so long to get this done! I am surprised by how little the DMA overhead really is!

FMUv5      
48 Bytes @20 Mhz   time uS time uS
Total   40.637  
RX Setup   1.065 1.065
TX Setup   0.688 0.688
RX Start   0.246 0.246
TX Start   0.246 0.246
spi_dmarxwait   0.625 0.625
spi_dmatxwait   34.38 0.625
DMA Cost   37.25 3.495
PROBE Cost   3.387  
SPI No DMA  
Bytes time uS
1 1.625
2 2.875
3 3.18
4 5
5 6.125
6 7.125
7 8.3
8 9.5
9 10.625
10 11.688
16 18.3
24 26.94
32 35.8
memcpy arch  
Bytes time uS
1024 5
2048 6.63
4096 9.08
8192 13.37

@davids5
Copy link
Member

davids5 commented Mar 13, 2020

@dagar also for the 20 Mhz the actual frequency is 13.5 Mhz

@mrpollo mrpollo mentioned this pull request Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants