From 5d995eec811cf7daf9bb5373e4f1ce8c332a2cde Mon Sep 17 00:00:00 2001 From: ohadmeir Date: Thu, 19 Dec 2024 12:19:41 +0200 Subject: [PATCH 1/3] Power uvc_device for 1 second after creation --- src/ds/d400/d400-color.cpp | 3 +++ src/ds/d400/d400-device.cpp | 3 +++ src/ds/d500/d500-color.cpp | 3 +++ src/ds/d500/d500-device.cpp | 3 +++ 4 files changed, 12 insertions(+) diff --git a/src/ds/d400/d400-color.cpp b/src/ds/d400/d400-color.cpp index eb0ed6dffd..8126751726 100644 --- a/src/ds/d400/d400-color.cpp +++ b/src/ds/d400/d400-color.cpp @@ -93,6 +93,9 @@ namespace librealsense std::unique_ptr(new global_timestamp_reader(std::move(ds_timestamp_reader_metadata), _tf_keeper, enable_global_time_option)), this); + // Many commands need power during initialization phase, no point turning it on and off again for each. + raw_color_ep->power_for_duration( std::chrono::milliseconds( 1000 ) ); + auto color_ep = std::make_shared(this, raw_color_ep, d400_color_fourcc_to_rs2_format, diff --git a/src/ds/d400/d400-device.cpp b/src/ds/d400/d400-device.cpp index b390e9aac8..e81238e5cc 100644 --- a/src/ds/d400/d400-device.cpp +++ b/src/ds/d400/d400-device.cpp @@ -509,6 +509,9 @@ namespace librealsense raw_depth_ep->register_xu(depth_xu); // make sure the XU is initialized every time we power the camera + // Many commands need power during initialization phase, no point turning it on and off again for each. + raw_depth_ep->power_for_duration( std::chrono::milliseconds( 1000 ) ); + auto depth_ep = std::make_shared(this, raw_depth_ep); depth_ep->register_info(RS2_CAMERA_INFO_PHYSICAL_PORT, filter_by_mi(all_device_infos, 0).front().device_path); diff --git a/src/ds/d500/d500-color.cpp b/src/ds/d500/d500-color.cpp index db220350c2..24c7c7958e 100644 --- a/src/ds/d500/d500-color.cpp +++ b/src/ds/d500/d500-color.cpp @@ -83,6 +83,9 @@ namespace librealsense enable_global_time_option ) ), this ); + // Many commands need power during initialization phase, no point turning it on and off again for each. + raw_color_ep->power_for_duration( std::chrono::milliseconds( 1000 ) ); + auto color_ep = std::make_shared(this, raw_color_ep, d500_color_fourcc_to_rs2_format, diff --git a/src/ds/d500/d500-device.cpp b/src/ds/d500/d500-device.cpp index e33c9b144c..301933ed50 100644 --- a/src/ds/d500/d500-device.cpp +++ b/src/ds/d500/d500-device.cpp @@ -375,6 +375,9 @@ namespace librealsense raw_depth_ep->register_xu(depth_xu); // make sure the XU is initialized every time we power the camera + // Many commands need power during initialization phase, no point turning it on and off again for each. + raw_depth_ep->power_for_duration( std::chrono::milliseconds( 1000 ) ); + auto depth_ep = std::make_shared(this, raw_depth_ep); depth_ep->register_info(RS2_CAMERA_INFO_PHYSICAL_PORT, filter_by_mi(all_device_infos, 0).front().device_path); From cf7ed8ca002c0db6629f182b3e13b1ff6e57ede2 Mon Sep 17 00:00:00 2001 From: ohadmeir Date: Tue, 24 Dec 2024 21:37:48 +0200 Subject: [PATCH 2/3] XU list saved in uvc_device not uvc_sensor --- src/linux/backend-v4l2.h | 2 +- src/mf/mf-uvc.cpp | 3 +++ src/mf/mf-uvc.h | 4 +++- src/platform/uvc-device.h | 6 +++--- src/uvc-sensor.cpp | 4 +--- src/uvc-sensor.h | 1 - src/uvc/uvc-device.cpp | 6 ------ src/uvc/uvc-device.h | 2 +- 8 files changed, 12 insertions(+), 16 deletions(-) diff --git a/src/linux/backend-v4l2.h b/src/linux/backend-v4l2.h index bc24df144e..a4bfd8749e 100644 --- a/src/linux/backend-v4l2.h +++ b/src/linux/backend-v4l2.h @@ -355,7 +355,7 @@ namespace librealsense void set_power_state(power_state state) override; power_state get_power_state() const override { return _state; } - void init_xu(const extension_unit&) override {} + void register_xu( platform::extension_unit && xu ) override {} bool set_xu(const extension_unit& xu, uint8_t control, const uint8_t* data, int size) override; bool get_xu(const extension_unit& xu, uint8_t control, uint8_t* data, int size) const override; control_range get_xu_range(const extension_unit& xu, uint8_t control, int len) const override; diff --git a/src/mf/mf-uvc.cpp b/src/mf/mf-uvc.cpp index 305cfab412..6e985c462e 100644 --- a/src/mf/mf-uvc.cpp +++ b/src/mf/mf-uvc.cpp @@ -885,6 +885,9 @@ namespace librealsense CHECK_HR(MFCreateSourceReaderFromMediaSource(_source, _reader_attrs, &_reader)); CHECK_HR(_reader->SetStreamSelection(static_cast(MF_SOURCE_READER_ALL_STREAMS), TRUE)); _power_state = D0; + + for( auto && xu : _xus ) + init_xu( xu ); } void wmf_uvc_device::set_d3() diff --git a/src/mf/mf-uvc.h b/src/mf/mf-uvc.h index 1ae61ab5a0..ee9a04ef5b 100644 --- a/src/mf/mf-uvc.h +++ b/src/mf/mf-uvc.h @@ -79,7 +79,7 @@ namespace librealsense static bool is_connected(const uvc_device_info& info); static void foreach_uvc_device(enumeration_callback action); - void init_xu(const extension_unit& xu) override; + void register_xu( platform::extension_unit && xu ) override { _xus.push_back( std::move( xu ) ); } bool set_xu(const extension_unit& xu, uint8_t ctrl, const uint8_t* data, int len) override; bool get_xu(const extension_unit& xu, uint8_t ctrl, uint8_t* data, int len) const override; control_range get_xu_range(const extension_unit& xu, uint8_t ctrl, int len) const override; @@ -101,6 +101,7 @@ namespace librealsense private: friend class source_reader_callback; + void init_xu(const extension_unit& xu); void play_profile(stream_profile profile, frame_callback callback); void stop_stream_cleanup(const stream_profile& profile, std::vector::iterator& elem); void flush(int sIndex); @@ -146,6 +147,7 @@ namespace librealsense bool _streaming = false; std::atomic _is_started = false; std::wstring _device_id; + std::vector< platform::extension_unit > _xus; }; class source_reader_callback : public IMFSourceReaderCallback diff --git a/src/platform/uvc-device.h b/src/platform/uvc-device.h index b6267990f0..7c37561176 100644 --- a/src/platform/uvc-device.h +++ b/src/platform/uvc-device.h @@ -128,7 +128,7 @@ class uvc_device virtual void set_power_state( power_state state ) = 0; virtual power_state get_power_state() const = 0; - virtual void init_xu( const extension_unit & xu ) = 0; + virtual void register_xu( platform::extension_unit && xu ) = 0; virtual bool set_xu( const extension_unit & xu, uint8_t ctrl, const uint8_t * data, int len ) = 0; virtual bool get_xu( const extension_unit & xu, uint8_t ctrl, uint8_t * data, int len ) const = 0; virtual control_range get_xu_range( const extension_unit & xu, uint8_t ctrl, int len ) const = 0; @@ -183,7 +183,7 @@ class retry_controls_work_around : public uvc_device power_state get_power_state() const override { return _dev->get_power_state(); } - void init_xu( const extension_unit & xu ) override { _dev->init_xu( xu ); } + void register_xu( platform::extension_unit && xu ) override { _dev->register_xu( std::move( xu ) ); } bool set_xu( const extension_unit & xu, uint8_t ctrl, const uint8_t * data, int len ) override { @@ -313,7 +313,7 @@ class multi_pins_uvc_device : public uvc_device power_state get_power_state() const override { return _dev.front()->get_power_state(); } - void init_xu( const extension_unit & xu ) override { _dev.front()->init_xu( xu ); } + void register_xu( platform::extension_unit && xu ) override { _dev.front()->register_xu( std::move( xu ) ); } bool set_xu( const extension_unit & xu, uint8_t ctrl, const uint8_t * data, int len ) override { diff --git a/src/uvc-sensor.cpp b/src/uvc-sensor.cpp index fab565af0f..c617c165ca 100644 --- a/src/uvc-sensor.cpp +++ b/src/uvc-sensor.cpp @@ -372,7 +372,7 @@ void uvc_sensor::finished_bulk_operation() void uvc_sensor::register_xu( platform::extension_unit xu ) { - _xus.push_back( std::move( xu ) ); + _device->register_xu( std::move( xu ) ); } @@ -419,8 +419,6 @@ void uvc_sensor::acquire_power() try { _device->set_power_state( platform::D0 ); - for( auto && xu : _xus ) - _device->init_xu( xu ); } catch( std::exception const & e ) { diff --git a/src/uvc-sensor.h b/src/uvc-sensor.h index f2370d56b0..ffc63dc3b7 100644 --- a/src/uvc-sensor.h +++ b/src/uvc-sensor.h @@ -104,7 +104,6 @@ class uvc_sensor : public raw_sensor_base std::atomic< int > _user_count; std::mutex _power_lock; std::mutex _configure_lock; - std::vector< platform::extension_unit > _xus; std::unique_ptr< power > _power; std::unique_ptr< frame_timestamp_reader > _timestamp_reader; }; diff --git a/src/uvc/uvc-device.cpp b/src/uvc/uvc-device.cpp index a23615ae74..d144ed81b2 100644 --- a/src/uvc/uvc-device.cpp +++ b/src/uvc/uvc-device.cpp @@ -210,12 +210,6 @@ namespace librealsense return _power_state; } - void rs_uvc_device::init_xu(const extension_unit& xu) - { - // not supported - return; - } - bool rs_uvc_device::set_xu(const extension_unit& xu, uint8_t ctrl, const uint8_t* data, int len) { return uvc_set_ctrl(xu.unit, ctrl, (void *) data, len); diff --git a/src/uvc/uvc-device.h b/src/uvc/uvc-device.h index aa9b6c93d1..6ca8516626 100644 --- a/src/uvc/uvc-device.h +++ b/src/uvc/uvc-device.h @@ -51,7 +51,7 @@ namespace librealsense virtual void set_power_state(power_state state) override; virtual power_state get_power_state() const override; - virtual void init_xu(const extension_unit& xu) override; + void register_xu( platform::extension_unit && xu ) override {} // Not supported virtual bool set_xu(const extension_unit& xu, uint8_t ctrl, const uint8_t* data, int len) override; virtual bool get_xu(const extension_unit& xu, uint8_t ctrl, uint8_t* data, int len) const override; virtual control_range get_xu_range(const extension_unit& xu, uint8_t ctrl, int len) const override; From 20f49f7133795c4db245c8b701bca429957e78ee Mon Sep 17 00:00:00 2001 From: ohadmeir Date: Wed, 25 Dec 2024 15:38:34 +0200 Subject: [PATCH 3/3] Count power state requests in device, not sensor --- src/linux/backend-v4l2.cpp | 42 ++++++++++++++++++++---- src/linux/backend-v4l2.h | 2 ++ src/mf/mf-uvc.cpp | 65 ++++++++++++++++++++++++++++++-------- src/mf/mf-uvc.h | 5 +-- src/uvc-sensor.cpp | 57 +++++++++++++-------------------- src/uvc-sensor.h | 2 -- src/uvc/uvc-device.cpp | 60 +++++++++++++++++++++++++---------- src/uvc/uvc-device.h | 3 +- 8 files changed, 160 insertions(+), 76 deletions(-) diff --git a/src/linux/backend-v4l2.cpp b/src/linux/backend-v4l2.cpp index 49c2686248..a88cf8358c 100644 --- a/src/linux/backend-v4l2.cpp +++ b/src/linux/backend-v4l2.cpp @@ -1205,7 +1205,8 @@ namespace librealsense } v4l_uvc_device::v4l_uvc_device(const uvc_device_info& info, bool use_memory_map) - : _name(info.id), + : _power_counter( 0 ), + _name(info.id), _device_path(info.device_path), _device_usb_spec(info.conn_spec), _info(info), @@ -1814,15 +1815,44 @@ namespace librealsense void v4l_uvc_device::set_power_state(power_state state) { - if (state == D0 && _state == D3) + std::lock_guard< std::recursive_mutex > lock( _power_lock ); + switch (state) { - map_device_descriptor(); + case D0: + { + if( _power_counter.fetch_add( 1 ) == 0 ) + { + try + { + map_device_descriptor(); + } + //In case of failure need to decrease use counter + catch( std::exception const & e ) + { + _power_counter.fetch_add( -1 ); + throw e; + } + catch( ... ) + { + _power_counter.fetch_add( -1 ); + throw; + } + } + break; } - if (state == D3 && _state == D0) + case D3: { - close(_profile); - unmap_device_descriptor(); + if( _power_counter.fetch_add( -1 ) == 1 ) + { + close(_profile); + unmap_device_descriptor(); + } + break; + } + default: + throw std::runtime_error("illegal power state request"); } + _state = state; } diff --git a/src/linux/backend-v4l2.h b/src/linux/backend-v4l2.h index a4bfd8749e..193d5377cd 100644 --- a/src/linux/backend-v4l2.h +++ b/src/linux/backend-v4l2.h @@ -407,6 +407,8 @@ namespace librealsense static bool get_devname_from_video_path(const std::string& real_path, std::string& devname); + std::recursive_mutex _power_lock; + std::atomic< int > _power_counter; power_state _state = D3; std::string _name = ""; std::string _device_path = ""; diff --git a/src/mf/mf-uvc.cpp b/src/mf/mf-uvc.cpp index 6e985c462e..6086f59176 100644 --- a/src/mf/mf-uvc.cpp +++ b/src/mf/mf-uvc.cpp @@ -773,23 +773,55 @@ namespace librealsense void wmf_uvc_device::set_power_state(power_state state) { - if (state == _power_state) - return; + std::lock_guard< std::recursive_mutex > lock( _source_lock ); switch (state) { - case D0: set_d0(); break; - case D3: set_d3(); break; + case D0: + { + if( _power_counter.fetch_add( 1 ) == 0 ) + { + try + { + set_d0(); + } + //In case of failure need to decrease use counter + catch( std::exception const & e ) + { + _power_counter.fetch_add( -1 ); + throw e; + } + catch( ... ) + { + _power_counter.fetch_add( -1 ); + throw; + } + } + break; + } + case D3: + { + if( _power_counter.fetch_add( -1 ) == 1 ) + { + set_d3(); + } + break; + } default: throw std::runtime_error("illegal power state request"); } } - wmf_uvc_device::wmf_uvc_device(const uvc_device_info& info, - std::shared_ptr backend) - : _streamIndex(MAX_PINS), _info(info), _is_flushed(), _has_started(), _backend(std::move(backend)), - _systemwide_lock(info.unique_id.c_str(), WAIT_FOR_MUTEX_TIME_OUT), - _location(""), _device_usb_spec(usb3_type) + wmf_uvc_device::wmf_uvc_device( const uvc_device_info & info, std::shared_ptr< const wmf_backend > backend ) + : _streamIndex( MAX_PINS ) + , _info( info ) + , _is_flushed() + , _has_started() + , _backend( std::move( backend ) ) + , _systemwide_lock( info.unique_id.c_str(), WAIT_FOR_MUTEX_TIME_OUT ) + , _location( "" ) + , _device_usb_spec( usb3_type ) + , _power_counter( 0 ) { if (!is_connected(info)) { @@ -884,7 +916,6 @@ namespace librealsense //enable reader CHECK_HR(MFCreateSourceReaderFromMediaSource(_source, _reader_attrs, &_reader)); CHECK_HR(_reader->SetStreamSelection(static_cast(MF_SOURCE_READER_ALL_STREAMS), TRUE)); - _power_state = D0; for( auto && xu : _xus ) init_xu( xu ); @@ -899,7 +930,6 @@ namespace librealsense safe_release(_source); for (auto& elem : _streams) elem.callback = nullptr; - _power_state = D3; } void wmf_uvc_device::foreach_profile(std::function media_type, bool& quit)> action) const @@ -1210,5 +1240,14 @@ namespace librealsense _profiles.clear(); _frame_callbacks.clear(); } - } -} + + power_state wmf_uvc_device::get_power_state() const + { + LOG_ERROR( "wmf_uvc_device::get_power_state start. this = " << this ); + std::lock_guard< std::recursive_mutex > lock( _source_lock ); + std::string tmp = _source ? "D0" : "D3"; + LOG_ERROR( "wmf_uvc_device::get_power_state got lock. power state is " << tmp ); + return _source ? D0 : D3; + } + } //namespace platform +} //namespace librealsense diff --git a/src/mf/mf-uvc.h b/src/mf/mf-uvc.h index ee9a04ef5b..108fec5cc0 100644 --- a/src/mf/mf-uvc.h +++ b/src/mf/mf-uvc.h @@ -73,7 +73,7 @@ namespace librealsense void stop_callbacks() override; void close(stream_profile profile) override; void set_power_state(power_state state) override; - power_state get_power_state() const override { return _power_state; } + power_state get_power_state() const override; std::vector get_profiles() const override; static bool is_connected(const uvc_device_info& info); @@ -119,7 +119,6 @@ namespace librealsense std::shared_ptr _backend; const uvc_device_info _info; - power_state _power_state = D3; CComPtr _reader = nullptr; CComPtr _source = nullptr; @@ -138,6 +137,7 @@ namespace librealsense std::vector _streams; std::mutex _streams_mutex; + mutable std::recursive_mutex _source_lock; // Guarding access to _source named_mutex _systemwide_lock; std::string _location; usb_spec _device_usb_spec; @@ -147,6 +147,7 @@ namespace librealsense bool _streaming = false; std::atomic _is_started = false; std::wstring _device_id; + std::atomic< int > _power_counter; std::vector< platform::extension_unit > _xus; }; diff --git a/src/uvc-sensor.cpp b/src/uvc-sensor.cpp index c617c165ca..8a2e874d3b 100644 --- a/src/uvc-sensor.cpp +++ b/src/uvc-sensor.cpp @@ -31,7 +31,6 @@ uvc_sensor::uvc_sensor( std::string const & name, device * dev ) : super( name, dev ) , _device( std::move( uvc_device ) ) - , _user_count( 0 ) , _timestamp_reader( std::move( timestamp_reader ) ) , _gyro_counter(0) , _accel_counter(0) @@ -413,47 +412,35 @@ void uvc_sensor::reset_streaming() void uvc_sensor::acquire_power() { - std::lock_guard< std::mutex > lock( _power_lock ); - if( _user_count.fetch_add( 1 ) == 0 ) + try { - try - { - _device->set_power_state( platform::D0 ); - } - catch( std::exception const & e ) - { - _user_count.fetch_add( -1 ); - LOG_ERROR( "acquire_power failed: " << e.what() ); - throw; - } - catch( ... ) - { - _user_count.fetch_add( -1 ); - LOG_ERROR( "acquire_power failed" ); - throw; - } + _device->set_power_state( platform::D0 ); + } + catch( std::exception const & e ) + { + LOG_ERROR( "acquire_power failed: " << e.what() ); + throw; + } + catch( ... ) + { + LOG_ERROR( "acquire_power failed" ); + throw; } } void uvc_sensor::release_power() { - std::lock_guard< std::mutex > lock( _power_lock ); - if( _user_count.fetch_add( -1 ) == 1 ) + try { - try - { - _device->set_power_state( platform::D3 ); - } - catch( std::exception const & e ) - { - // TODO may need to change the user-count? - LOG_ERROR( "release_power failed: " << e.what() ); - } - catch( ... ) - { - // TODO may need to change the user-count? - LOG_ERROR( "release_power failed" ); - } + _device->set_power_state( platform::D3 ); + } + catch( std::exception const & e ) + { + LOG_ERROR( "release_power failed: " << e.what() ); + } + catch( ... ) + { + LOG_ERROR( "release_power failed" ); } } diff --git a/src/uvc-sensor.h b/src/uvc-sensor.h index ffc63dc3b7..25600f409f 100644 --- a/src/uvc-sensor.h +++ b/src/uvc-sensor.h @@ -101,8 +101,6 @@ class uvc_sensor : public raw_sensor_base std::shared_ptr< platform::uvc_device > _device; std::vector< platform::stream_profile > _internal_config; - std::atomic< int > _user_count; - std::mutex _power_lock; std::mutex _configure_lock; std::unique_ptr< power > _power; std::unique_ptr< frame_timestamp_reader > _timestamp_reader; diff --git a/src/uvc/uvc-device.cpp b/src/uvc/uvc-device.cpp index d144ed81b2..6ab0bff3ec 100644 --- a/src/uvc/uvc-device.cpp +++ b/src/uvc/uvc-device.cpp @@ -79,11 +79,12 @@ namespace librealsense return nullptr; } - rs_uvc_device::rs_uvc_device(const rs_usb_device& usb_device, const uvc_device_info &info, uint8_t usb_request_count) : - _usb_device(usb_device), - _info(info), - _action_dispatcher(10), - _usb_request_count(usb_request_count) + rs_uvc_device::rs_uvc_device( const rs_usb_device & usb_device, const uvc_device_info & info, uint8_t usb_request_count ) + : _info( info ) + , _power_counter( 0 ) + , _usb_device( usb_device ) + , _usb_request_count( usb_request_count ) + , _action_dispatcher( 10 ) { _parser = std::make_shared(usb_device, info); _action_dispatcher.start(); @@ -174,28 +175,53 @@ namespace librealsense { if(state != _power_state) { + std::lock_guard< std::recursive_mutex > lock( _power_lock ); switch(state) { case D0: - _messenger = _usb_device->open(static_cast(_info.mi)); - if (_messenger) + if( _power_counter.fetch_add( 1 ) == 0 ) { - try{ - listen_to_interrupts(); - } catch(const std::exception& exception) { - // this exception catching avoids crash when disconnecting 2 devices at once - bug seen in android os - LOG_WARNING("rs_uvc_device exception in listen_to_interrupts method: " << exception.what()); + try + { + _messenger = _usb_device->open(static_cast(_info.mi)); + if (_messenger) + { + try + { + listen_to_interrupts(); + } + catch(const std::exception& exception) + { + // this exception catching avoids crash when disconnecting 2 devices at once - bug seen in android os + LOG_WARNING("rs_uvc_device exception in listen_to_interrupts method: " << exception.what()); + } + _power_state = D0; + } + } + //In case of failure need to decrease use counter + catch( std::exception const & e ) + { + _power_counter.fetch_add( -1 ); + throw e; + } + catch( ... ) + { + _power_counter.fetch_add( -1 ); + throw; } - _power_state = D0; } + break; case D3: - if(_messenger) + if( _power_counter.fetch_add( -1 ) == 1 ) { - close_uvc_device(); - _messenger.reset(); + if( _messenger ) + { + close_uvc_device(); + _messenger.reset(); + } + _power_state = D3; } - _power_state = D3; break; } } diff --git a/src/uvc/uvc-device.h b/src/uvc/uvc-device.h index 6ca8516626..a070b6343b 100644 --- a/src/uvc/uvc-device.h +++ b/src/uvc/uvc-device.h @@ -96,7 +96,8 @@ namespace librealsense const uvc_device_info _info; power_state _power_state = D3; // power state change is unsupported - + std::recursive_mutex _power_lock; + std::atomic< int > _power_counter; std::vector _streams; std::string _location;