Use a separate hwcomposer hidl instance for vr flinger
Improve robustness of vr flinger <--> surface flinger switching by
having vr flinger use a separate hardware composer hidl instance instead
of sharing the instance with surface flinger. Sharing the hardware
composer instance has proven to be error prone, with situations where
both the vr flinger thread and surface flinger main thread would write
to the composer at the same time, causing hard to diagnose
crashes (b/62925812).
Instead of sharing the hardware composer instance, when switching to vr
flinger we now delete the existing instance, create a new instance
directed to the vr hardware composer shim, and vr flinger creates its
own composer instance connected to the real hardware composer. By
creating a separate composer instance for vr flinger, crashes like the
ones found in b/62925812 are no longer impossible.
Most of the changes in this commit are related to enabling surface
flinger to delete HWComposer instances cleanly. In particular:
- Previously the hardware composer callbacks (which come in on a
hwbinder thread) would land in HWC2::Device and bubble up to the
SurfaceFlinger object. But with the new behavior the HWC2::Device
might be dead or in the process of being destroyed, so instead we have
SurfaceFlinger receive the composer callbacks directly, and forward
them to HWComposer and HWC2::Device. We include a composer id field in
the callbacks so surface flinger can ignore stale callbacks from dead
composer instances.
- Object ownership for HWC2::Display and HWC2::Layer was shared by
passing around shared_ptrs to these objects. This was problematic
because they referenced and used the HWC2::Device, which can now be
destroyed when switching to vr flinger. Simplify the ownership model
by having HWC2::Device own (via unique_ptr<>) instances of
HWC2::Display, which owns (again via unique_ptr<>) instances of
HWC2::Layer. In cases where we previously passed std::shared_ptr<> to
HWC2::Display or HWC2::Layer, instead pass non-owning HWC2::Display*
and HWC2::Layer* pointers. This ensures clean composer instance
teardown with no stale references to the deleted HWC2::Device.
- When the hardware composer instance is destroyed and the HWC2::Layers
are removed, notify the android::Layer via a callback, so it can
remove the HWC2::Layer from its internal table of hardware composer
layers. This removes the burden to explicitly clear out all hardware
composer layers when switching to vr flinger, which has been a source
of bugs.
- We were missing an mStateLock lock in
SurfaceFlinger::setVsyncEnabled(), which was necessary to ensure we
were setting vsync on the correct hardware composer instance. Once
that lock was added, surface flinger would sometimes deadlock when
transitioning to vr flinger, because the surface flinger main thread
would acquire mStateLock and then EventControlThread::mMutex, whereas
the event control thread would acquire the locks in the opposite
order. The changes in EventControlThread.cpp are to ensure it doesn't
hold a lock on EventControlThread::mMutex while calling
setVsyncEnabled(), to avoid the deadlock.
I found that without a composer callback registered in vr flinger the
vsync_event file wasn't getting vsync timestamps written, so vr flinger
would get stuck in an infinite loop trying to parse a vsync
timestamp. Since we need to have a callback anyway I changed the code in
hardware_composer.cpp to get the vsync timestamp from the callback, as
surface flinger does. I confirmed the timestamps are the same with
either method, and this lets us remove some extra code for extracting
the vsync timestamp that (probably) wasn't compatible with all devices
we want to run on anyway. I also added a timeout to the vysnc wait so
we'll see an error message in the log if we fail to wait for vsync,
instead of looping forever.
Bug: 62925812
Test: - Confirmed surface flinger <--> vr flinger switching is robust by
switching devices on and off hundreds of times and observing no
hardware composer related issues, surface flinger crashes, or
hardware composer service crashes.
- Confirmed 2d in vr works as before by going through the OOBE flow on a
standalone. This also exercises virtual display creation and usage
through surface flinger.
- Added logs to confirm perfect layer/display cleanup when destroying
hardware composer instances.
- Tested normal 2d phone usage to confirm basic layer create/destroy
functionality works as before.
- Monitored surface flinger file descriptor usage across dozens of
surface flinger <--> vr flinger transitions and observed no file
descriptor leaks.
- Confirmed the HWC1 code path still compiles.
- Ran the surface flinger tests and confirmed there are no new test
failures.
- Ran the hardware composer hidl in passthrough mode on a Marlin and
confirmed it works.
- Ran CTS tests for virtual displays and confirmed they all pass.
- Tested Android Auto and confirmed basic graphics functionality still
works.
Change-Id: I17dc0e060bfb5cb447ffbaa573b279fc6d2d8bd1
diff --git a/services/surfaceflinger/DisplayHardware/HWC2.h b/services/surfaceflinger/DisplayHardware/HWC2.h
index 9bcda1e..fbe4c7e 100644
--- a/services/surfaceflinger/DisplayHardware/HWC2.h
+++ b/services/surfaceflinger/DisplayHardware/HWC2.h
@@ -53,10 +53,26 @@
class Display;
class Layer;
-typedef std::function<void(std::shared_ptr<Display>, Connection)>
- HotplugCallback;
-typedef std::function<void(std::shared_ptr<Display>)> RefreshCallback;
-typedef std::function<void(std::shared_ptr<Display>, nsecs_t)> VsyncCallback;
+// Implement this interface to receive hardware composer events.
+//
+// These callback functions will generally be called on a hwbinder thread, but
+// when first registering the callback the onHotplugReceived() function will
+// immediately be called on the thread calling registerCallback().
+//
+// All calls receive a sequenceId, which will be the value that was supplied to
+// HWC2::Device::registerCallback(). It's used to help differentiate callbacks
+// from different hardware composer instances.
+class ComposerCallback {
+ public:
+ virtual void onHotplugReceived(int32_t sequenceId, hwc2_display_t display,
+ Connection connection,
+ bool primaryDisplay) = 0;
+ virtual void onRefreshReceived(int32_t sequenceId,
+ hwc2_display_t display) = 0;
+ virtual void onVsyncReceived(int32_t sequenceId, hwc2_display_t display,
+ int64_t timestamp) = 0;
+ virtual ~ComposerCallback() = default;
+};
// C++ Wrapper around hwc2_device_t. Load all functions pointers
// and handle callback registration.
@@ -66,10 +82,8 @@
// Service name is expected to be 'default' or 'vr' for normal use.
// 'vr' will slightly modify the behavior of the mComposer.
Device(const std::string& serviceName);
- ~Device();
- friend class HWC2::Display;
- friend class HWC2::Layer;
+ void registerCallback(ComposerCallback* callback, int32_t sequenceId);
// Required by HWC2
@@ -81,27 +95,14 @@
uint32_t getMaxVirtualDisplayCount() const;
Error createVirtualDisplay(uint32_t width, uint32_t height,
- android_pixel_format_t* format,
- std::shared_ptr<Display>* outDisplay);
+ android_pixel_format_t* format, Display** outDisplay);
+ void destroyDisplay(hwc2_display_t displayId);
- void registerHotplugCallback(HotplugCallback hotplug);
- void registerRefreshCallback(RefreshCallback refresh);
- void registerVsyncCallback(VsyncCallback vsync);
-
- // For use by callbacks
-
- void callHotplug(std::shared_ptr<Display> display, Connection connected);
- void callRefresh(std::shared_ptr<Display> display);
- void callVsync(std::shared_ptr<Display> display, nsecs_t timestamp);
+ void onHotplug(hwc2_display_t displayId, Connection connection);
// Other Device methods
- // This will create a Display if one is not found, but it will not be marked
- // as connected. This Display may be null if the display has been torn down
- // but has not been removed from the map yet.
- std::shared_ptr<Display> getDisplayById(hwc2_display_t id);
-
- bool hasCapability(HWC2::Capability capability) const;
+ Display* getDisplayById(hwc2_display_t id);
android::Hwc2::Composer* getComposer() { return mComposer.get(); }
@@ -109,37 +110,23 @@
// Initialization methods
void loadCapabilities();
- void registerCallbacks();
-
- // For use by Display
-
- void destroyVirtualDisplay(hwc2_display_t display);
// Member variables
std::unique_ptr<android::Hwc2::Composer> mComposer;
-
std::unordered_set<Capability> mCapabilities;
- std::unordered_map<hwc2_display_t, std::weak_ptr<Display>> mDisplays;
-
- HotplugCallback mHotplug;
- std::vector<std::pair<std::shared_ptr<Display>, Connection>>
- mPendingHotplugs;
- RefreshCallback mRefresh;
- std::vector<std::shared_ptr<Display>> mPendingRefreshes;
- VsyncCallback mVsync;
- std::vector<std::pair<std::shared_ptr<Display>, nsecs_t>> mPendingVsyncs;
+ std::unordered_map<hwc2_display_t, std::unique_ptr<Display>> mDisplays;
+ bool mRegisteredCallback;
};
// Convenience C++ class to access hwc2_device_t Display functions directly.
-class Display : public std::enable_shared_from_this<Display>
+class Display
{
public:
- Display(Device& device, hwc2_display_t id);
+ Display(android::Hwc2::Composer& composer,
+ const std::unordered_set<Capability>& capabilities,
+ hwc2_display_t id, DisplayType type);
~Display();
- friend class HWC2::Device;
- friend class HWC2::Layer;
-
class Config
{
public:
@@ -212,12 +199,12 @@
// Required by HWC2
[[clang::warn_unused_result]] Error acceptChanges();
- [[clang::warn_unused_result]] Error createLayer(
- std::shared_ptr<Layer>* outLayer);
+ [[clang::warn_unused_result]] Error createLayer(Layer** outLayer);
+ [[clang::warn_unused_result]] Error destroyLayer(Layer* layer);
[[clang::warn_unused_result]] Error getActiveConfig(
std::shared_ptr<const Config>* outConfig) const;
[[clang::warn_unused_result]] Error getChangedCompositionTypes(
- std::unordered_map<std::shared_ptr<Layer>, Composition>* outTypes);
+ std::unordered_map<Layer*, Composition>* outTypes);
[[clang::warn_unused_result]] Error getColorModes(
std::vector<android_color_mode_t>* outModes) const;
@@ -227,14 +214,13 @@
[[clang::warn_unused_result]] Error getName(std::string* outName) const;
[[clang::warn_unused_result]] Error getRequests(
DisplayRequest* outDisplayRequests,
- std::unordered_map<std::shared_ptr<Layer>, LayerRequest>*
- outLayerRequests);
+ std::unordered_map<Layer*, LayerRequest>* outLayerRequests);
[[clang::warn_unused_result]] Error getType(DisplayType* outType) const;
[[clang::warn_unused_result]] Error supportsDoze(bool* outSupport) const;
[[clang::warn_unused_result]] Error getHdrCapabilities(
std::unique_ptr<android::HdrCapabilities>* outCapabilities) const;
[[clang::warn_unused_result]] Error getReleaseFences(
- std::unordered_map<std::shared_ptr<Layer>,
+ std::unordered_map<Layer*,
android::sp<android::Fence>>* outFences) const;
[[clang::warn_unused_result]] Error present(
android::sp<android::Fence>* outPresentFence);
@@ -266,32 +252,31 @@
// Other Display methods
- Device& getDevice() const { return mDevice; }
hwc2_display_t getId() const { return mId; }
bool isConnected() const { return mIsConnected; }
+ void setConnected(bool connected); // For use by Device only
private:
- // For use by Device
-
- void setConnected(bool connected) { mIsConnected = connected; }
int32_t getAttribute(hwc2_config_t configId, Attribute attribute);
void loadConfig(hwc2_config_t configId);
void loadConfigs();
- // For use by Layer
- void destroyLayer(hwc2_layer_t layerId);
-
// This may fail (and return a null pointer) if no layer with this ID exists
// on this display
- std::shared_ptr<Layer> getLayerById(hwc2_layer_t id) const;
+ Layer* getLayerById(hwc2_layer_t id) const;
// Member variables
- Device& mDevice;
+ // These are references to data owned by HWC2::Device, which will outlive
+ // this HWC2::Display, so these references are guaranteed to be valid for
+ // the lifetime of this object.
+ android::Hwc2::Composer& mComposer;
+ const std::unordered_set<Capability>& mCapabilities;
+
hwc2_display_t mId;
bool mIsConnected;
DisplayType mType;
- std::unordered_map<hwc2_layer_t, std::weak_ptr<Layer>> mLayers;
+ std::unordered_map<hwc2_layer_t, std::unique_ptr<Layer>> mLayers;
// The ordering in this map matters, for getConfigs(), when it is
// converted to a vector
std::map<hwc2_config_t, std::shared_ptr<const Config>> mConfigs;
@@ -301,12 +286,18 @@
class Layer
{
public:
- Layer(const std::shared_ptr<Display>& display, hwc2_layer_t id);
+ Layer(android::Hwc2::Composer& composer,
+ const std::unordered_set<Capability>& capabilities,
+ hwc2_display_t displayId, hwc2_layer_t layerId);
~Layer();
- bool isAbandoned() const { return mDisplay.expired(); }
hwc2_layer_t getId() const { return mId; }
+ // Register a listener to be notified when the layer is destroyed. When the
+ // listener function is called, the Layer will be in the process of being
+ // destroyed, so it's not safe to call methods on it.
+ void setLayerDestroyedListener(std::function<void(Layer*)> listener);
+
[[clang::warn_unused_result]] Error setCursorPosition(int32_t x, int32_t y);
[[clang::warn_unused_result]] Error setBuffer(uint32_t slot,
const android::sp<android::GraphicBuffer>& buffer,
@@ -333,11 +324,16 @@
[[clang::warn_unused_result]] Error setInfo(uint32_t type, uint32_t appId);
private:
- std::weak_ptr<Display> mDisplay;
+ // These are references to data owned by HWC2::Device, which will outlive
+ // this HWC2::Layer, so these references are guaranteed to be valid for
+ // the lifetime of this object.
+ android::Hwc2::Composer& mComposer;
+ const std::unordered_set<Capability>& mCapabilities;
+
hwc2_display_t mDisplayId;
- Device& mDevice;
hwc2_layer_t mId;
android_dataspace mDataSpace = HAL_DATASPACE_UNKNOWN;
+ std::function<void(Layer*)> mLayerDestroyedListener;
};
} // namespace HWC2