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

USB MSC: speed optimization #182

Merged
merged 7 commits into from
Jun 29, 2023
Merged

Conversation

tore-espressif
Copy link
Collaborator

@tore-espressif tore-espressif commented May 22, 2023

Checklist

  • Component contains License
  • Component contains README.md
  • Component contains idf_component.yml file with url field defined
  • Component was added to upload job
  • Component was added to build job
  • Optional: Component contains unit tests
  • CI passing

Change description

  • Significantly increase performance with Virtual File System by allowing longer transfers
  • Optimize used heap memory by reusing the Virtual File System buffer
  • Optimize CPU usage by putting the background MSC task to 'Blocked' state indefinitely when there is nothing to do
  • Fix MSC commands for devices on interface numbers other than zero
  • Replace unsafe debug functions for direct access of MSC sectors with private SCSI commands

After the application starts new USB transfer, the USB-OTG peripheral will schedule it for next frame (which can be as delayed as 1ms). Therefore, the application must submit large-enough (>1kB) transfers to fully leverage USB FullSpeed bandwidth. For larger USB transfers we need more DMA capable memory, plus we need the same amount for memory for buffer in Virtual File System.

This change reuses the VFS buffer for USB transfers, which saves memory and does not require any copying between buffers. Plus, the user gets a convenient API to change the buffer size per file with setvbuf(), without any configuration of the MSC driver

@tore-espressif tore-espressif self-assigned this May 22, 2023
@CLAassistant
Copy link

CLAassistant commented May 22, 2023

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link

github-actions bot commented May 22, 2023

Unit Test Results

  11 files    11 suites   15m 15s ⏱️
  26 tests   26 ✔️ 0 💤 0
243 runs  243 ✔️ 0 💤 0

Results for commit f83def3.

♻️ This comment has been updated with latest results.

@tore-espressif tore-espressif force-pushed the usb/msc/speed_optimization branch 2 times, most recently from 089fb6f to a466843 Compare May 22, 2023 20:03
Copy link
Contributor

@Dazza0 Dazza0 left a comment

Choose a reason for hiding this comment

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

@tore-espressif Left some comments.

usb/usb_host_msc/test/test_msc.c Show resolved Hide resolved
usb/usb_host_msc/src/msc_host.c Outdated Show resolved Hide resolved
usb/usb_host_msc/src/msc_host.c Outdated Show resolved Hide resolved
usb/usb_host_msc/include/usb/msc_host.h Outdated Show resolved Hide resolved
usb/usb_host_msc/private_include/msc_common.h Outdated Show resolved Hide resolved
usb/usb_host_msc/src/msc_host.c Outdated Show resolved Hide resolved
usb/usb_host_msc/src/msc_scsi_bot.c Outdated Show resolved Hide resolved
@tore-espressif
Copy link
Collaborator Author

tore-espressif commented Jun 1, 2023

Hello @Dazza0 @roma-jam thank you for you reviews.

Here are my fixes (I'll clean up the git history before merging) 5388f6f

@roma-jam roma-jam self-requested a review June 1, 2023 15:53
@tore-espressif tore-espressif force-pushed the usb/msc/speed_optimization branch 2 times, most recently from dcdbe29 to da0e56a Compare June 5, 2023 11:31
Copy link
Collaborator

@roma-jam roma-jam left a comment

Choose a reason for hiding this comment

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

Hey @tore-espressif ,
I have checked the code again and run error_recovery_1 and _2 locally.
Small comments only, otherwise LGTM.

usb/usb_host_msc/test/test_msc.c Show resolved Hide resolved
usb/usb_host_msc/src/msc_host.c Show resolved Hide resolved
Copy link
Contributor

@esp-saurabhbansal esp-saurabhbansal left a comment

Choose a reason for hiding this comment

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

@tore-espressif
Please check the review comments

usb/test_app/main/usb_test_main.c Show resolved Hide resolved
usb/usb_host_msc/CMakeLists.txt Show resolved Hide resolved
usb/usb_host_msc/include/usb/msc_host.h Outdated Show resolved Hide resolved
@Dazza0
Copy link
Contributor

Dazza0 commented Jun 13, 2023

@tore-espressif Changes LGTM

@tore-espressif tore-espressif force-pushed the usb/msc/speed_optimization branch 2 times, most recently from ef0257d to 6928417 Compare June 28, 2023 11:30
This will save CPU time when MSC driver has nothing to do
msc driver offered debug functions that accessed an array outside of its boundaries
The MSC driver can now only be used with the VFS component. The MSC driver will reuse an intermediate VFS buffer to send larger transfers to MSC device, dramatically increasing transfer speeds

Expose SCSI commands as esp_private include"
SCSI commands can be used for direct access of memory blocks
@tore-espressif tore-espressif merged commit 1ad789e into master Jun 29, 2023
@mahavirj mahavirj deleted the usb/msc/speed_optimization branch July 11, 2023 09:14
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.

5 participants