-
Notifications
You must be signed in to change notification settings - Fork 4.9k
enable usb weak host workaround to improve android stability #7205
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
enable usb weak host workaround to improve android stability #7205
Conversation
@maloel thanks for review feedback. hope the changes addressed concerns. |
Hi @gwen2018 |
Thanks. I will check with firmware team. |
include/librealsense2/h/rs_option.h
Outdated
@@ -96,6 +96,7 @@ extern "C" { | |||
RS2_OPTION_THERMAL_COMPENSATION, /**< Depth Thermal Compensation for selected D400 SKUs */ | |||
RS2_OPTION_TRIGGER_CAMERA_ACCURACY_HEALTH, /**< Enable depth & color frame sync with periodic calibration for proper alignment */ | |||
RS2_OPTION_RESET_CAMERA_ACCURACY_HEALTH, | |||
RS2_OPTION_ENABLE_WEAK_USB_HOST_WA, /**< Enable workaround for weak USB hosts to improve performance and stability */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls rename and refactor the option to be rs2_option_trb_size so that it will read/set the TRB size (units TBD) - it is not platform specfic and therefore may be utilized in additional scenarios.
You can set an enum for the supported RGB sizes in order to enforce strict (and validated) values
src/l500/l500-depth.cpp
Outdated
|
||
auto usb_perf_enabled = get_option(RS2_OPTION_ENABLE_WEAK_USB_HOST_WA).query(); | ||
|
||
if (usb_perf_enabled) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be encapsulated into opt->set
…t on android and default settings for other platforms, user has the option to change
@ev-mp thanks for the review. refactored the option, included your suggestion, name is host performance mode, because it will be a generic option for future use as well, this time, only include the TRB settings. at launch, on android platforms, low performance mode is turned on and larger TRB settings, on other platforms (windows, linux, etc.), device default setting is used at launch. user has the option to change the option later after viewer launch. The option is only for L515, the conditional compilation is minimized (only the launch performance mode). also added settings for the 4th endpoint (confidence stream). now the settings are complete. |
@@ -374,6 +392,67 @@ namespace librealsense | |||
|
|||
void l500_depth_sensor::start(frame_callback_ptr callback) | |||
{ | |||
if (supports_option(RS2_OPTION_HOST_PERFORMANCE)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about encapsulating the below code into opt->set(..) ?
The TRB settings are customized per sensor and can be passed into the option at ctor level (where sensor->register_option( occrs), so this could can become:
if supports(TRB)
trb_opt-set(TRB_MOD)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the suggestion, looked into it, will implement tomorrow. if need to make the build, please go ahead and take what current implementation. can do another PR for this part of the change once it's tested.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, we'll merge the current implementation so that refactoring will be moved to the follow-on task.
@gwen2018 , please open a new PR for the remaining work.
@@ -96,6 +96,7 @@ extern "C" { | |||
RS2_OPTION_THERMAL_COMPENSATION, /**< Depth Thermal Compensation for selected D400 SKUs */ | |||
RS2_OPTION_TRIGGER_CAMERA_ACCURACY_HEALTH, /**< Enable depth & color frame sync with periodic calibration for proper alignment */ | |||
RS2_OPTION_RESET_CAMERA_ACCURACY_HEALTH, | |||
RS2_OPTION_HOST_PERFORMANCE, /**< Set host performance mode to optimize device settings so host can keep up with workload, for example, USB transaction granularity, setting option to low performance host leads to larger USB transaction size and reduced number of transactions which improves performance and stability if host is relatively weak as compared to workload */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure the option and enum are propagated into python (+pybackend), C#, Java and Matlab.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added to python and java for now, but not c# and matlab yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you for the in-depth analysis!
Enable L515 android stability workaround. Current usb buffer settings in device require large number of small usb transactions from device to usb host controller, some hosts with relatively weak host cannot handle this many transactions quickly, results in stability issues, camera disappear from os enumeration or one of the streams dies during stream, etc. This workaround uses larger buffer settings to reduce the number of transaction and improve performance and stability.
Workaround is enabled by default for all android platforms. Requires additional firmware command support in new firmware release.
Tracked on: RS5-8011