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

1 files changed, 151 insertions, 143 deletions

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 4154d6a87f..9dfdaa1d99 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -77,6 +77,7 @@
 #include "MonitoredProducer.h"
 #include "SurfaceFlinger.h"
 
+#include "DisplayHardware/ComposerHal.h"
 #include "DisplayHardware/FramebufferSurface.h"
 #include "DisplayHardware/HWComposer.h"
 #include "DisplayHardware/VirtualDisplaySurface.h"
@@ -101,10 +102,24 @@ EGLAPI const char* eglQueryStringImplementationANDROID(EGLDisplay dpy, EGLint na
 
 namespace android {
 
-
 using namespace android::hardware::configstore;
 using namespace android::hardware::configstore::V1_0;
 
+namespace {
+class ConditionalLock {
+public:
+    ConditionalLock(Mutex& mutex, bool lock) : mMutex(mutex), mLocked(lock) {
+        if (lock) {
+            mMutex.lock();
+        }
+    }
+    ~ConditionalLock() { if (mLocked) mMutex.unlock(); }
+private:
+    Mutex& mMutex;
+    bool mLocked;
+};
+}  // namespace anonymous
+
 // ---------------------------------------------------------------------------
 
 const String16 sHardwareTest("android.permission.HARDWARE_TEST");
@@ -147,9 +162,6 @@ SurfaceFlinger::SurfaceFlinger()
         mLayersRemoved(false),
         mLayersAdded(false),
         mRepaintEverything(0),
-        mHwc(nullptr),
-        mRealHwc(nullptr),
-        mVrHwc(nullptr),
         mHwcServiceName(getHwcServiceName()),
         mRenderEngine(nullptr),
         mBootTime(systemTime()),
@@ -177,7 +189,9 @@ SurfaceFlinger::SurfaceFlinger()
         mTotalTime(0),
         mLastSwapTime(0),
         mNumLayers(0),
-        mVrFlingerRequestsDisplay(false)
+        mVrFlingerRequestsDisplay(false),
+        mMainThreadId(std::this_thread::get_id()),
+        mComposerSequenceId(0)
 {
     ALOGI("SurfaceFlinger is starting");
 
@@ -583,48 +597,46 @@ void SurfaceFlinger::init() {
 
     ALOGI("Phase offset NS: %" PRId64 "", vsyncPhaseOffsetNs);
 
-    { // Autolock scope
-        Mutex::Autolock _l(mStateLock);
+    Mutex::Autolock _l(mStateLock);
 
-        // initialize EGL for the default display
-        mEGLDisplay = eglGetDisplay(EGL_DEFAULT_DISPLAY);
-        eglInitialize(mEGLDisplay, NULL, NULL);
-
-        // start the EventThread
-        sp<VSyncSource> vsyncSrc = new DispSyncSource(&mPrimaryDispSync,
-                vsyncPhaseOffsetNs, true, "app");
-        mEventThread = new EventThread(vsyncSrc, *this, false);
-        sp<VSyncSource> sfVsyncSrc = new DispSyncSource(&mPrimaryDispSync,
-                sfVsyncPhaseOffsetNs, true, "sf");
-        mSFEventThread = new EventThread(sfVsyncSrc, *this, true);
-        mEventQueue.setEventThread(mSFEventThread);
+    // initialize EGL for the default display
+    mEGLDisplay = eglGetDisplay(EGL_DEFAULT_DISPLAY);
+    eglInitialize(mEGLDisplay, NULL, NULL);
 
-        // set EventThread and SFEventThread to SCHED_FIFO to minimize jitter
-        struct sched_param param = {0};
-        param.sched_priority = 2;
-        if (sched_setscheduler(mSFEventThread->getTid(), SCHED_FIFO, &param) != 0) {
-            ALOGE("Couldn't set SCHED_FIFO for SFEventThread");
-        }
-        if (sched_setscheduler(mEventThread->getTid(), SCHED_FIFO, &param) != 0) {
-            ALOGE("Couldn't set SCHED_FIFO for EventThread");
-        }
+    // start the EventThread
+    sp<VSyncSource> vsyncSrc = new DispSyncSource(&mPrimaryDispSync,
+            vsyncPhaseOffsetNs, true, "app");
+    mEventThread = new EventThread(vsyncSrc, *this, false);
+    sp<VSyncSource> sfVsyncSrc = new DispSyncSource(&mPrimaryDispSync,
+            sfVsyncPhaseOffsetNs, true, "sf");
+    mSFEventThread = new EventThread(sfVsyncSrc, *this, true);
+    mEventQueue.setEventThread(mSFEventThread);
 
-        // Get a RenderEngine for the given display / config (can't fail)
-        mRenderEngine = RenderEngine::create(mEGLDisplay,
-                HAL_PIXEL_FORMAT_RGBA_8888,
-                hasWideColorDisplay ? RenderEngine::WIDE_COLOR_SUPPORT : 0);
+    // set EventThread and SFEventThread to SCHED_FIFO to minimize jitter
+    struct sched_param param = {0};
+    param.sched_priority = 2;
+    if (sched_setscheduler(mSFEventThread->getTid(), SCHED_FIFO, &param) != 0) {
+        ALOGE("Couldn't set SCHED_FIFO for SFEventThread");
+    }
+    if (sched_setscheduler(mEventThread->getTid(), SCHED_FIFO, &param) != 0) {
+        ALOGE("Couldn't set SCHED_FIFO for EventThread");
     }
 
-    // Drop the state lock while we initialize the hardware composer. We drop
-    // the lock because on creation, it will call back into SurfaceFlinger to
-    // initialize the primary display.
-    LOG_ALWAYS_FATAL_IF(mVrFlingerRequestsDisplay,
-        "Starting with vr flinger active is not currently supported.");
-    mRealHwc = new HWComposer(mHwcServiceName);
-    mHwc = mRealHwc;
-    mHwc->setEventHandler(static_cast<HWComposer::EventHandler*>(this));
+    // Get a RenderEngine for the given display / config (can't fail)
+    mRenderEngine = RenderEngine::create(mEGLDisplay,
+            HAL_PIXEL_FORMAT_RGBA_8888,
+            hasWideColorDisplay ? RenderEngine::WIDE_COLOR_SUPPORT : 0);
 
-    Mutex::Autolock _l(mStateLock);
+    // retrieve the EGL context that was selected/created
+    mEGLContext = mRenderEngine->getEGLContext();
+
+    LOG_ALWAYS_FATAL_IF(mEGLContext == EGL_NO_CONTEXT,
+            "couldn't create EGLContext");
+
+    LOG_ALWAYS_FATAL_IF(mVrFlingerRequestsDisplay,
+            "Starting with vr flinger active is not currently supported.");
+    mHwc.reset(new HWComposer(mHwcServiceName));
+    mHwc->registerCallback(this, mComposerSequenceId);
 
     if (useVrFlinger) {
         auto vrFlingerRequestDisplayCallback = [this] (bool requestDisplay) {
@@ -639,16 +651,6 @@ void SurfaceFlinger::init() {
         }
     }
 
-    // retrieve the EGL context that was selected/created
-    mEGLContext = mRenderEngine->getEGLContext();
-
-    LOG_ALWAYS_FATAL_IF(mEGLContext == EGL_NO_CONTEXT,
-            "couldn't create EGLContext");
-
-    // make the GLContext current so that we can create textures when creating
-    // Layers (which may happens before we render something)
-    getDefaultDisplayDeviceLocked()->makeCurrent(mEGLDisplay, mEGLContext);
-
     mEventControlThread = new EventControlThread(this);
     mEventControlThread->run("EventControl", PRIORITY_URGENT_DISPLAY);
 
@@ -1218,11 +1220,16 @@ void SurfaceFlinger::resyncWithRateLimit() {
     sLastResyncAttempted = now;
 }
 
-void SurfaceFlinger::onVSyncReceived(HWComposer* composer, int32_t type,
-                                     nsecs_t timestamp) {
+void SurfaceFlinger::onVsyncReceived(int32_t sequenceId,
+        hwc2_display_t displayId, int64_t timestamp) {
     Mutex::Autolock lock(mStateLock);
-    // Ignore any vsyncs from the non-active hardware composer.
-    if (composer != mHwc) {
+    // Ignore any vsyncs from a previous hardware composer.
+    if (sequenceId != mComposerSequenceId) {
+        return;
+    }
+
+    int32_t type;
+    if (!mHwc->onVsync(displayId, timestamp, &type)) {
         return;
     }
 
@@ -1230,7 +1237,7 @@ void SurfaceFlinger::onVSyncReceived(HWComposer* composer, int32_t type,
 
     { // Scope for the lock
         Mutex::Autolock _l(mHWVsyncLock);
-        if (type == 0 && mPrimaryHWVsyncEnabled) {
+        if (type == DisplayDevice::DISPLAY_PRIMARY && mPrimaryHWVsyncEnabled) {
             needsHwVsync = mPrimaryDispSync.addResyncSample(timestamp);
         }
     }
@@ -1248,7 +1255,7 @@ void SurfaceFlinger::getCompositorTiming(CompositorTiming* compositorTiming) {
 }
 
 void SurfaceFlinger::createDefaultDisplayDevice() {
-    const int32_t type = DisplayDevice::DISPLAY_PRIMARY;
+    const DisplayDevice::DisplayType type = DisplayDevice::DISPLAY_PRIMARY;
     wp<IBinder> token = mBuiltinDisplays[type];
 
     // All non-virtual displays are currently considered secure.
@@ -1279,28 +1286,49 @@ void SurfaceFlinger::createDefaultDisplayDevice() {
     mDisplays.add(token, hw);
     setActiveColorModeInternal(hw, HAL_COLOR_MODE_NATIVE);
     hw->setCompositionDataSpace(HAL_DATASPACE_UNKNOWN);
-}
 
-void SurfaceFlinger::onHotplugReceived(HWComposer* composer, int32_t disp, bool connected) {
-    ALOGV("onHotplugReceived(%d, %s)", disp, connected ? "true" : "false");
+    // Add the primary display token to mDrawingState so we don't try to
+    // recreate the DisplayDevice for the primary display.
+    mDrawingState.displays.add(token, DisplayDeviceState(type, true));
 
-    if (composer->isUsingVrComposer()) {
-        // We handle initializing the primary display device for the VR
-        // window manager hwc explicitly at the time of transition.
-        if (disp != DisplayDevice::DISPLAY_PRIMARY) {
-            ALOGE("External displays are not supported by the vr hardware composer.");
+    // make the GLContext current so that we can create textures when creating
+    // Layers (which may happens before we render something)
+    hw->makeCurrent(mEGLDisplay, mEGLContext);
+}
+
+void SurfaceFlinger::onHotplugReceived(int32_t sequenceId,
+        hwc2_display_t display, HWC2::Connection connection,
+        bool primaryDisplay) {
+    ALOGV("onHotplugReceived(%d, %" PRIu64 ", %s, %s)",
+          sequenceId, display,
+          connection == HWC2::Connection::Connected ?
+                  "connected" : "disconnected",
+          primaryDisplay ? "primary" : "external");
+
+    // Only lock if we're not on the main thread. This function is normally
+    // called on a hwbinder thread, but for the primary display it's called on
+    // the main thread with the state lock already held, so don't attempt to
+    // acquire it here.
+    ConditionalLock lock(mStateLock,
+            std::this_thread::get_id() != mMainThreadId);
+
+    if (primaryDisplay) {
+        mHwc->onHotplug(display, connection);
+        if (!mBuiltinDisplays[DisplayDevice::DISPLAY_PRIMARY].get()) {
+            createBuiltinDisplayLocked(DisplayDevice::DISPLAY_PRIMARY);
         }
-        return;
-    }
-
-    if (disp == DisplayDevice::DISPLAY_PRIMARY) {
-        Mutex::Autolock lock(mStateLock);
-        createBuiltinDisplayLocked(DisplayDevice::DISPLAY_PRIMARY);
         createDefaultDisplayDevice();
     } else {
+        if (sequenceId != mComposerSequenceId) {
+            return;
+        }
+        if (mHwc->isUsingVrComposer()) {
+            ALOGE("External displays are not supported by the vr hardware composer.");
+            return;
+        }
+        mHwc->onHotplug(display, connection);
         auto type = DisplayDevice::DISPLAY_EXTERNAL;
-        Mutex::Autolock _l(mStateLock);
-        if (connected) {
+        if (connection == HWC2::Connection::Connected) {
             createBuiltinDisplayLocked(type);
         } else {
             mCurrentState.displays.removeItem(mBuiltinDisplays[type]);
@@ -1312,46 +1340,31 @@ void SurfaceFlinger::onHotplugReceived(HWComposer* composer, int32_t disp, bool
     }
 }
 
-void SurfaceFlinger::onInvalidateReceived(HWComposer* composer) {
+void SurfaceFlinger::onRefreshReceived(int sequenceId,
+                                       hwc2_display_t /*display*/) {
     Mutex::Autolock lock(mStateLock);
-    if (composer == mHwc) {
-        repaintEverything();
-    } else {
-        // This isn't from our current hardware composer. If it's a callback
-        // from the real composer, forward the refresh request to vr
-        // flinger. Otherwise ignore it.
-        if (!composer->isUsingVrComposer()) {
-            mVrFlinger->OnHardwareComposerRefresh();
-        }
+    if (sequenceId != mComposerSequenceId) {
+        return;
     }
+    repaintEverything();
 }
 
 void SurfaceFlinger::setVsyncEnabled(int disp, int enabled) {
     ATRACE_CALL();
+    Mutex::Autolock lock(mStateLock);
     getHwComposer().setVsyncEnabled(disp,
             enabled ? HWC2::Vsync::Enable : HWC2::Vsync::Disable);
 }
 
 // Note: it is assumed the caller holds |mStateLock| when this is called
-void SurfaceFlinger::resetHwcLocked() {
+void SurfaceFlinger::resetDisplayState() {
     disableHardwareVsync(true);
-    clearHwcLayers(mDrawingState.layersSortedByZ);
-    clearHwcLayers(mCurrentState.layersSortedByZ);
-    for (size_t disp = 0; disp < mDisplays.size(); ++disp) {
-        clearHwcLayers(mDisplays[disp]->getVisibleLayersSortedByZ());
-    }
     // Clear the drawing state so that the logic inside of
     // handleTransactionLocked will fire. It will determine the delta between
     // mCurrentState and mDrawingState and re-apply all changes when we make the
     // transition.
     mDrawingState.displays.clear();
-    // Release virtual display hwcId during vr mode transition.
-    for (size_t displayId = 0; displayId < mDisplays.size(); ++displayId) {
-        const sp<DisplayDevice>& displayDevice = mDisplays[displayId];
-        if (displayDevice->getDisplayType() == DisplayDevice::DISPLAY_VIRTUAL) {
-            displayDevice->disconnect(getHwComposer());
-        }
-    }
+    eglMakeCurrent(mEGLDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
     mDisplays.clear();
 }
 
@@ -1363,57 +1376,54 @@ void SurfaceFlinger::updateVrFlinger() {
         return;
     }
 
-    if (vrFlingerRequestsDisplay && !mVrHwc) {
-        // Construct new HWComposer without holding any locks.
-        mVrHwc = new HWComposer("vr");
-
-        // Set up the event handlers. This step is neccessary to initialize the internal state of
-        // the hardware composer object properly. Our callbacks are designed such that if they are
-        // triggered between now and the point where the display is properly re-initialized, they
-        // will not have any effect, so this is safe to do here, before the lock is aquired.
-        mVrHwc->setEventHandler(static_cast<HWComposer::EventHandler*>(this));
-        ALOGV("Vr HWC created");
+    if (vrFlingerRequestsDisplay && !mHwc->getComposer()->isRemote()) {
+        ALOGE("Vr flinger is only supported for remote hardware composer"
+              " service connections. Ignoring request to transition to vr"
+              " flinger.");
+        mVrFlingerRequestsDisplay = false;
+        return;
     }
 
     Mutex::Autolock _l(mStateLock);
 
-    if (vrFlingerRequestsDisplay) {
-        resetHwcLocked();
-
-        mHwc = mVrHwc;
-        mVrFlinger->GrantDisplayOwnership();
+    int currentDisplayPowerMode = getDisplayDeviceLocked(
+            mBuiltinDisplays[DisplayDevice::DISPLAY_PRIMARY])->getPowerMode();
 
-    } else {
+    if (!vrFlingerRequestsDisplay) {
         mVrFlinger->SeizeDisplayOwnership();
+    }
 
-        resetHwcLocked();
+    resetDisplayState();
+    mHwc.reset();  // Delete the current instance before creating the new one
+    mHwc.reset(new HWComposer(
+            vrFlingerRequestsDisplay ? "vr" : mHwcServiceName));
+    mHwc->registerCallback(this, ++mComposerSequenceId);
 
-        mHwc = mRealHwc;
+    LOG_ALWAYS_FATAL_IF(!mHwc->getComposer()->isRemote(),
+            "Switched to non-remote hardware composer");
+
+    if (vrFlingerRequestsDisplay) {
+        mVrFlinger->GrantDisplayOwnership();
+    } else {
         enableHardwareVsync();
     }
 
     mVisibleRegionsDirty = true;
     invalidateHwcGeometry();
 
-    // Explicitly re-initialize the primary display. This is because some other
-    // parts of this class rely on the primary display always being available.
-    createDefaultDisplayDevice();
-
     // Re-enable default display.
-    sp<LambdaMessage> requestMessage = new LambdaMessage([&]() {
-        sp<DisplayDevice> hw(getDisplayDevice(mBuiltinDisplays[DisplayDevice::DISPLAY_PRIMARY]));
-        setPowerModeInternal(hw, HWC_POWER_MODE_NORMAL);
+    sp<DisplayDevice> hw(getDisplayDeviceLocked(
+            mBuiltinDisplays[DisplayDevice::DISPLAY_PRIMARY]));
+    setPowerModeInternal(hw, currentDisplayPowerMode, /*stateLockHeld*/ true);
 
-        // Reset the timing values to account for the period of the swapped in HWC
-        const auto& activeConfig = mHwc->getActiveConfig(HWC_DISPLAY_PRIMARY);
-        const nsecs_t period = activeConfig->getVsyncPeriod();
-        mAnimFrameTracker.setDisplayRefreshPeriod(period);
+    // Reset the timing values to account for the period of the swapped in HWC
+    const auto& activeConfig = mHwc->getActiveConfig(HWC_DISPLAY_PRIMARY);
+    const nsecs_t period = activeConfig->getVsyncPeriod();
+    mAnimFrameTracker.setDisplayRefreshPeriod(period);
 
-        // Use phase of 0 since phase is not known.
-        // Use latency of 0, which will snap to the ideal latency.
-        setCompositorTimingSnapped(0, period, 0);
-    });
-    postMessageAsync(requestMessage);
+    // Use phase of 0 since phase is not known.
+    // Use latency of 0, which will snap to the ideal latency.
+    setCompositorTimingSnapped(0, period, 0);
 
     android_atomic_or(1, &mRepaintEverything);
     setTransactionFlags(eDisplayTransactionNeeded);
@@ -1749,15 +1759,14 @@ void SurfaceFlinger::rebuildLayerStacks() {
                         } else {
                             // Clear out the HWC layer if this layer was
                             // previously visible, but no longer is
-                            layer->setHwcLayer(displayDevice->getHwcDisplayId(),
-                                    nullptr);
+                            layer->destroyHwcLayer(
+                                    displayDevice->getHwcDisplayId());
                         }
                     } else {
                         // WM changes displayDevice->layerStack upon sleep/awake.
                         // Here we make sure we delete the HWC layers even if
                         // WM changed their layer stack.
-                        layer->setHwcLayer(displayDevice->getHwcDisplayId(),
-                                nullptr);
+                        layer->destroyHwcLayer(displayDevice->getHwcDisplayId());
                     }
                 });
             }
@@ -1872,10 +1881,7 @@ void SurfaceFlinger::setUpHWComposer() {
                 for (size_t i = 0; i < currentLayers.size(); i++) {
                     const auto& layer = currentLayers[i];
                     if (!layer->hasHwcLayer(hwcId)) {
-                        auto hwcLayer = mHwc->createLayer(hwcId);
-                        if (hwcLayer) {
-                            layer->setHwcLayer(hwcId, std::move(hwcLayer));
-                        } else {
+                        if (!layer->createHwcLayer(mHwc.get(), hwcId)) {
                             layer->forceClientComposition(hwcId);
                             continue;
                         }
@@ -2161,7 +2167,7 @@ void SurfaceFlinger::handleTransactionLocked(uint32_t transactionFlags)
                         if (state.surface != NULL) {
 
                             // Allow VR composer to use virtual displays.
-                            if (mUseHwcVirtualDisplays || mHwc == mVrHwc) {
+                            if (mUseHwcVirtualDisplays || mHwc->isUsingVrComposer()) {
                                 int width = 0;
                                 int status = state.surface->query(
                                         NATIVE_WINDOW_WIDTH, &width);
@@ -3285,7 +3291,8 @@ void SurfaceFlinger::onInitializeDisplays() {
     d.height = 0;
     displays.add(d);
     setTransactionState(state, displays, 0);
-    setPowerModeInternal(getDisplayDevice(d.token), HWC_POWER_MODE_NORMAL);
+    setPowerModeInternal(getDisplayDevice(d.token), HWC_POWER_MODE_NORMAL,
+                         /*stateLockHeld*/ false);
 
     const auto& activeConfig = mHwc->getActiveConfig(HWC_DISPLAY_PRIMARY);
     const nsecs_t period = activeConfig->getVsyncPeriod();
@@ -3311,7 +3318,7 @@ void SurfaceFlinger::initializeDisplays() {
 }
 
 void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& hw,
-        int mode) {
+             int mode, bool stateLockHeld) {
     ALOGD("Set power mode=%d, type=%d flinger=%p", mode, hw->getDisplayType(),
             this);
     int32_t type = hw->getDisplayType();
@@ -3328,7 +3335,7 @@ void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& hw,
     }
 
     if (mInterceptor.isEnabled()) {
-        Mutex::Autolock _l(mStateLock);
+        ConditionalLock lock(mStateLock, !stateLockHeld);
         ssize_t idx = mCurrentState.displays.indexOfKey(hw->getDisplayToken());
         if (idx < 0) {
             ALOGW("Surface Interceptor SavePowerMode: invalid display token");
@@ -3414,7 +3421,8 @@ void SurfaceFlinger::setPowerMode(const sp<IBinder>& display, int mode) {
                 ALOGW("Attempt to set power mode = %d for virtual display",
                         mMode);
             } else {
-                mFlinger.setPowerModeInternal(hw, mMode);
+                mFlinger.setPowerModeInternal(
+                        hw, mMode, /*stateLockHeld*/ false);
             }
             return true;
         }