Skip to content

enable usb weak host workaround to improve android stability - #7205

Merged
ev-mp merged 7 commits into
realsenseai:developmentfrom
gwen2018:l515_android_stability_wa
Sep 13, 2020
Merged

enable usb weak host workaround to improve android stability#7205
ev-mp merged 7 commits into
realsenseai:developmentfrom
gwen2018:l515_android_stability_wa

Conversation

@gwen2018

@gwen2018 gwen2018 commented Aug 27, 2020

Copy link
Copy Markdown
Contributor

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

Comment thread src/l500/l500-color.cpp Outdated
Comment thread src/l500/l500-color.cpp Outdated
Comment thread src/l500/l500-color.cpp
Comment thread src/l500/l500-color.cpp Outdated
Comment thread src/sensor.cpp Outdated
@gwen2018

Copy link
Copy Markdown
Contributor Author

@maloel thanks for review feedback. hope the changes addressed concerns.

@maloel

maloel commented Aug 29, 2020

Copy link
Copy Markdown
Contributor

Hi @gwen2018
The changes are fine. I'm a little hesitant about the FW version... can we delay the merge of this until they give a proper version? Or do you need it in right away?

@gwen2018

Copy link
Copy Markdown
Contributor Author

Hi @gwen2018
The changes are fine. I'm a little hesitant about the FW version... can we delay the merge of this until they give a proper version? Or do you need it in right away?

Thanks. I will check with firmware team.

Comment thread include/librealsense2/h/rs_option.h Outdated
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 */

Copy link
Copy Markdown
Contributor

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

Comment thread src/l500/l500-depth.cpp Outdated

auto usb_perf_enabled = get_option(RS2_OPTION_ENABLE_WEAK_USB_HOST_WA).query();

if (usb_perf_enabled)

Copy link
Copy Markdown
Contributor

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
@gwen2018

Copy link
Copy Markdown
Contributor Author

@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.

Comment thread src/l500/l500-depth.cpp

void l500_depth_sensor::start(frame_callback_ptr callback)
{
if (supports_option(RS2_OPTION_HOST_PERFORMANCE))

Copy link
Copy Markdown
Contributor

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)

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor

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.

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 */

Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor Author

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.

Comment thread include/librealsense2/h/rs_option.h

@ev-mp ev-mp left a comment

Copy link
Copy Markdown
Contributor

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!

@ev-mp
ev-mp merged commit e229fdb into realsenseai:development Sep 13, 2020
@gwen2018
gwen2018 deleted the l515_android_stability_wa branch February 4, 2025 05:21
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