Skip to content

Fixed very slow implementation of _ff_mod#768

Closed
phdussud wants to merge 1 commit into
hathach:masterfrom
phdussud:master
Closed

Fixed very slow implementation of _ff_mod#768
phdussud wants to merge 1 commit into
hathach:masterfrom
phdussud:master

Conversation

@phdussud
Copy link
Copy Markdown

@phdussud phdussud commented Apr 3, 2021

Describe the PR
Fixed very slow implementation. Symptoms are that the USB transfers take more and more time, then become fast again as the index (uint16_t) wraps around

I appreciate the fact that some MPUs don't have a hardware divide, so I provided a fast path if the fifo depth is a power of 2.

…ake more and more time, then become fast again as the index (uint16_t) wraps around
Copy link
Copy Markdown
Owner

@hathach hathach left a comment

Choose a reason for hiding this comment

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

thank for the pr, could you provide the before and after timing measurement based on your test setup. It is not clear to me how much it is boost the performance here.

@phdussud
Copy link
Copy Markdown
Author

phdussud commented Apr 4, 2021 via email

@phdussud
Copy link
Copy Markdown
Author

phdussud commented Apr 4, 2021 via email

Comment thread src/common/tusb_fifo.c
Copy link
Copy Markdown
Owner

@hathach hathach left a comment

Choose a reason for hiding this comment

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

Could you also print out the idx value at the 1st and 9th, I am curious to see what is its value at that time.

Comment thread src/common/tusb_fifo.c
@hathach
Copy link
Copy Markdown
Owner

hathach commented Apr 6, 2021

Make sure you use this repo master for testing instead of the fork in raspberrypi repo. If not then switch branch and try again

Copy link
Copy Markdown
Owner

@hathach hathach left a comment

Choose a reason for hiding this comment

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

Thank you for you PR. This is already fixed by #655 . Thank you for @PanRe for joining the review.

@hathach hathach closed this Apr 6, 2021
@phdussud
Copy link
Copy Markdown
Author

phdussud commented Apr 6, 2021

I can confirm that everything is good when I switch to the master branch. Thank you very much for your help!!

@hathach
Copy link
Copy Markdown
Owner

hathach commented Apr 6, 2021

I can confirm that everything is good when I switch to the master branch. Thank you very much for your help!!

thanks, please test again this repo for your next issue/pr. There has been quite a bit of fixes for rp2040 since it is released in sdk.

7FM pushed a commit to 7FM/tinyusb that referenced this pull request Aug 23, 2025
7FM pushed a commit to 7FM/tinyusb that referenced this pull request Aug 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants