Re-work hot plug event handling.

This patch re-works code to make sure we can handle
hot plug notifications after all related book keeping
has been done when we have hot plug notification rather
than in between. Also, adds logs to easliy check state
in future.

Jira: None.
Test: Display connect and disconnect are recieved by
      SurfaceFlinger appropriately,

Signed-off-by: Kalyan Kondapally <kalyan.kondapally@intel.com>
Signed-off-by: Harish Krupo <harish.krupo.kps@intel.com>

diff --git a/common/compositor/compositor.cpp b/common/compositor/compositor.cpp
index 83f9761..25b8d76 100644 (file)
--- a/common/compositor/compositor.cpp
+++ b/common/compositor/compositor.cpp
@@ -54,7 +54,8 @@ bool Compositor::BeginFrame(bool disable_explicit_sync) {
 }
 
 void Compositor::Reset() {
-  thread_->ExitThread();
+  if (thread_)
+    thread_->ExitThread();
 }
 
 bool Compositor::Draw(DisplayPlaneStateList &comp_planes,

diff --git a/common/compositor/compositorthread.cpp b/common/compositor/compositorthread.cpp
index 6f19266..53e520f 100644 (file)
--- a/common/compositor/compositorthread.cpp
+++ b/common/compositor/compositorthread.cpp
@@ -37,8 +37,12 @@ CompositorThread::~CompositorThread() {
 }
 
 void CompositorThread::Initialize(DisplayPlaneManager *plane_manager) {
-  gpu_resource_handler_.reset(CreateNativeGpuResourceHandler());
+  tasks_lock_.lock();
+  if (!gpu_resource_handler_)
+    gpu_resource_handler_.reset(CreateNativeGpuResourceHandler());
+
   plane_manager_ = plane_manager;
+  tasks_lock_.unlock();
   if (!InitWorker()) {
     ETRACE("Failed to initalize CompositorThread. %s", PRINTERROR());
   }
@@ -134,6 +138,7 @@ void CompositorThread::HandleReleaseRequest() {
 
   if (release_all_resources_) {
     plane_manager_->ReleaseAllOffScreenTargets();
+    gpu_resource_handler_.reset(nullptr);
   } else {
     plane_manager_->ReleaseFreeOffScreenTargets();
   }

diff --git a/common/display/displayqueue.cpp b/common/display/displayqueue.cpp
index e62e74b..c2159c6 100644 (file)
--- a/common/display/displayqueue.cpp
+++ b/common/display/displayqueue.cpp
@@ -66,6 +66,12 @@ DisplayQueue::~DisplayQueue() {
 bool DisplayQueue::Initialize(uint32_t pipe, uint32_t width, uint32_t height,
                               DisplayPlaneHandler* plane_handler) {
   frame_ = 0;
+  std::vector<OverlayLayer>().swap(in_flight_layers_);
+  DisplayPlaneStateList().swap(previous_plane_state_);
+  idle_tracker_.state_ = 0;
+  idle_tracker_.idle_frames_ = 0;
+  compositor_.Reset();
+
   display_plane_manager_.reset(
       new DisplayPlaneManager(gpu_fd_, buffer_handler_, plane_handler));
   if (!display_plane_manager_->Initialize(width, height)) {

diff --git a/common/utils/hwcthread.cpp b/common/utils/hwcthread.cpp
index d67bd13..73ebc41 100644 (file)
--- a/common/utils/hwcthread.cpp
+++ b/common/utils/hwcthread.cpp
@@ -50,20 +50,19 @@ bool HWCThread::InitWorker() {
 }
 
 void HWCThread::Resume() {
-  if (exit_)
+  if (exit_ || !initialized_)
     return;
 
   event_.Signal();
 }
 
 void HWCThread::Exit() {
-  IHOTPLUGEVENTTRACE("HWCThread::Exit recieved.");
   if (!initialized_)
     return;
 
   initialized_ = false;
   exit_ = true;
-
+  IHOTPLUGEVENTTRACE("HWCThread::Exit recieved.");
   event_.Signal();
   thread_->join();
 }

diff --git a/os/android/iahwc2.cpp b/os/android/iahwc2.cpp
index 3275ae6..d576e3d 100644 (file)
--- a/os/android/iahwc2.cpp
+++ b/os/android/iahwc2.cpp
@@ -83,6 +83,9 @@ class IAHotPlugEventCallback : public hwcomposer::HotPlugCallback {
       status = static_cast<int32_t>(HWC2::Connection::Disconnected);
     }
 
+    IHOTPLUGEVENTTRACE(
+        "IAHotPlugEventCallback called displayid: %d status: %d \n", display,
+        status);
     hook(data_, display, status);
   }
 
@@ -133,8 +136,9 @@ HWC2::Error IAHWC2::Init() {
   uint32_t external_display_id = 1;
   for (size_t i = 0; i < size; ++i) {
     hwcomposer::NativeDisplay *display = displays.at(i);
-    if (primary_display == display)
+    if (primary_display == display) {
       continue;
+    }
 
     std::unique_ptr<HwcDisplay> temp(new HwcDisplay());
     temp->Init(display, external_display_id, disable_explicit_sync_);

diff --git a/wsi/drm/drmdisplay.cpp b/wsi/drm/drmdisplay.cpp
index 26b01a3..7c44df0 100644 (file)
--- a/wsi/drm/drmdisplay.cpp
+++ b/wsi/drm/drmdisplay.cpp
@@ -66,12 +66,16 @@ bool DrmDisplay::ConnectDisplay(const drmModeModeInfo &mode_info,
   IHOTPLUGEVENTTRACE("DrmDisplay::Connect recieved.");
   // TODO(kalyan): Add support for multi monitor case.
   if (connector_ && connector->connector_id == connector_) {
-    IHOTPLUGEVENTTRACE("Display is already connected to this connector.");
+    IHOTPLUGEVENTTRACE(
+        "Display is already connected to this connector. %d %d %p \n",
+        connector->connector_id, connector_, this);
     PhysicalDisplay::Connect();
     return true;
   }
 
-  IHOTPLUGEVENTTRACE("Display is being connected to a new connector.");
+  IHOTPLUGEVENTTRACE(
+      "Display is being connected to a new connector.%d %d %p \n",
+      connector->connector_id, connector_, this);
   connector_ = connector->connector_id;
   mmWidth_ = connector->mmWidth;
   mmHeight_ = connector->mmHeight;
@@ -318,8 +322,10 @@ bool DrmDisplay::CommitFrame(
 void DrmDisplay::SetDrmModeInfo(const std::vector<drmModeModeInfo> &mode_info) {
   display_lock_.lock();
   uint32_t size = mode_info.size();
-  for (uint32_t i = 0; i < size; ++i)
+  std::vector<drmModeModeInfo>().swap(modes_);
+  for (uint32_t i = 0; i < size; ++i) {
     modes_.emplace_back(mode_info[i]);
+  }
 
   display_lock_.unlock();
 }

diff --git a/wsi/drm/drmdisplaymanager.cpp b/wsi/drm/drmdisplaymanager.cpp
index 8048a91..8c31b09 100644 (file)
--- a/wsi/drm/drmdisplaymanager.cpp
+++ b/wsi/drm/drmdisplaymanager.cpp
@@ -202,7 +202,7 @@ bool DrmDisplayManager::UpdateDisplayState() {
   spin_lock_.lock();
   // Start of assuming no displays are connected
   for (auto &display : displays_) {
-    display->DisConnect();
+    display->MarkForDisconnect();
   }
 
   std::vector<NativeDisplay *>().swap(connected_displays_);
@@ -236,13 +236,19 @@ bool DrmDisplayManager::UpdateDisplayState() {
           drmModeGetEncoder(fd_, connector->encoder_id));
       if (encoder && encoder->crtc_id) {
         for (auto &display : displays_) {
-          // Set the modes supported for each display
-          display->SetDrmModeInfo(mode);
+          IHOTPLUGEVENTTRACE(
+              "Trying to connect %d with crtc: %d is display connected: %d \n",
+              encoder->crtc_id, display->CrtcId(), display->IsConnected());
           // At initilaization  preferred mode is set!
-          if (encoder->crtc_id == display->CrtcId() &&
+          if (!display->IsConnected() &&
+              encoder->crtc_id == display->CrtcId() &&
               display->ConnectDisplay(mode.at(preferred_mode),
                                       connector.get())) {
-            connected_displays_.emplace_back(display.get());
+            IHOTPLUGEVENTTRACE("Connected %d with crtc: %d \n",
+                               encoder->crtc_id, display->CrtcId());
+            // Set the modes supported for each display
+            display->SetDrmModeInfo(mode);
+            display->NotifyClientOfConnectedState();
             break;
           }
         }
@@ -262,7 +268,7 @@ bool DrmDisplayManager::UpdateDisplayState() {
             IHOTPLUGEVENTTRACE("connected pipe:%d \n", display->Pipe());
             // Set the modes supported for each display
             display->SetDrmModeInfo(mode);
-            connected_displays_.emplace_back(display.get());
+            display->NotifyClientOfConnectedState();
             break;
           }
         }
@@ -272,7 +278,10 @@ bool DrmDisplayManager::UpdateDisplayState() {
 
   for (auto &display : displays_) {
     if (!display->IsConnected()) {
-      display->SetPowerMode(kOff);
+      display->DisConnect();
+      display->NotifyClientOfDisConnectedState();
+    } else {
+      connected_displays_.emplace_back(display.get());
     }
   }
 

diff --git a/wsi/physicaldisplay.cpp b/wsi/physicaldisplay.cpp
index 99fd32e..4fdfd2a 100644 (file)
--- a/wsi/physicaldisplay.cpp
+++ b/wsi/physicaldisplay.cpp
@@ -51,19 +51,59 @@ bool PhysicalDisplay::Initialize(NativeBufferHandler *buffer_handler) {
   return true;
 }
 
-void PhysicalDisplay::DisConnect() {
-  IHOTPLUGEVENTTRACE("PhysicalDisplay::DisConnect recieved.");
+void PhysicalDisplay::MarkForDisconnect() {
+  modeset_lock_.lock();
+  IHOTPLUGEVENTTRACE("PhysicalDisplay::MarkForDisconnect recieved.");
   display_state_ |= kDisconnectionInProgress;
   display_state_ |= kRefreshClonedDisplays;
+  modeset_lock_.unlock();
 }
 
-void PhysicalDisplay::Connect() {
-  IHOTPLUGEVENTTRACE("PhysicalDisplay::Connect recieved.");
+void PhysicalDisplay::NotifyClientOfConnectedState() {
+  modeset_lock_.lock();
+  if (hotplug_callback_ && (display_state_ & kConnected)) {
+    IHOTPLUGEVENTTRACE(
+        "PhysicalDisplay Sent Hotplug even call back with connected value set "
+        "to true. %p hotplugdisplayid: %d \n",
+        this, hot_plug_display_id_);
+    hotplug_callback_->Callback(hot_plug_display_id_, true);
+  }
+  modeset_lock_.unlock();
+}
+
+void PhysicalDisplay::NotifyClientOfDisConnectedState() {
+  modeset_lock_.lock();
+  if (hotplug_callback_ && !(display_state_ & kConnected)) {
+    IHOTPLUGEVENTTRACE(
+        "PhysicalDisplay Sent Hotplug even call back with connected value set "
+        "to false. %p hotplugdisplayid: %d \n",
+        this, hot_plug_display_id_);
+    hotplug_callback_->Callback(hot_plug_display_id_, false);
+  }
+  modeset_lock_.unlock();
+}
+
+void PhysicalDisplay::DisConnect() {
+  modeset_lock_.lock();
   display_state_ &= ~kDisconnectionInProgress;
-  if (display_state_ & kConnected)
+  if (!(display_state_ & kConnected)) {
+    modeset_lock_.unlock();
     return;
+  }
+
+  modeset_lock_.unlock();
+  SetPowerMode(kOff);
+}
 
+void PhysicalDisplay::Connect() {
   modeset_lock_.lock();
+  display_state_ &= ~kDisconnectionInProgress;
+  IHOTPLUGEVENTTRACE("PhysicalDisplay::Connect recieved. %p \n", this);
+  if (display_state_ & kConnected) {
+    modeset_lock_.unlock();
+    return;
+  }
+
   display_state_ |= kConnected;
   display_state_ &= ~kInitialized;
 
@@ -71,15 +111,20 @@ void PhysicalDisplay::Connect() {
     ETRACE("Failed to initialize Display Queue.");
   } else {
     display_state_ |= kInitialized;
-    if (hotplug_callback_) {
-      hotplug_callback_->Callback(hot_plug_display_id_, true);
-    }
   }
 
   UpdatePowerMode();
+
   modeset_lock_.unlock();
 }
 
+bool PhysicalDisplay::IsConnected() const {
+  if (display_state_ & kDisconnectionInProgress)
+    return false;
+
+  return display_state_ & kConnected;
+}
+
 uint32_t PhysicalDisplay::PowerMode() const {
   return power_mode_;
 }
@@ -109,26 +154,28 @@ bool PhysicalDisplay::GetActiveConfig(uint32_t *config) {
 }
 
 bool PhysicalDisplay::SetPowerMode(uint32_t power_mode) {
-  if (power_mode_ == power_mode)
+  ScopedSpinLock lock(modeset_lock_);
+  if (power_mode_ == power_mode) {
     return true;
-
-  modeset_lock_.lock();
-  // Don't update power mode in case disconnect is in
-  // progress.
-  if (!(display_state_ & kDisconnectionInProgress)) {
-    power_mode_ = power_mode;
-  } else if (power_mode == kOff) {
-    display_state_ &= ~kConnected;
-    if (hotplug_callback_) {
-      hotplug_callback_->Callback(hot_plug_display_id_, false);
-    }
   }
-  modeset_lock_.unlock();
 
+  power_mode_ = power_mode;
   if (!(display_state_ & kConnected)) {
+    IHOTPLUGEVENTTRACE(
+        "PhysicalDisplay is not connected, postponing power mode update.");
     display_state_ |= kPendingPowerMode;
-
     return true;
+  } else if (display_state_ & kDisconnectionInProgress) {
+    IHOTPLUGEVENTTRACE(
+        "PhysicalDisplay diconnection in progress, postponing power mode "
+        "update.");
+    // Don't update power mode in case disconnect is in
+    // progress.
+    display_state_ |= kPendingPowerMode;
+    return true;
+  } else if (power_mode_ == kOff) {
+    IHOTPLUGEVENTTRACE("PhysicalDisplay Powering off the Display. %p", this);
+    display_state_ &= ~kConnected;
   }
 
   return UpdatePowerMode();
@@ -238,14 +285,16 @@ void PhysicalDisplay::RegisterHotPlugCallback(
   modeset_lock_.lock();
   hot_plug_display_id_ = display_id;
   hotplug_callback_ = callback;
+  bool connected = display_state_ & kConnected;
+  modeset_lock_.unlock();
+
   if (hotplug_callback_) {
-    if (display_state_ & kConnected) {
+    if (connected) {
       hotplug_callback_->Callback(hot_plug_display_id_, true);
     } else {
       hotplug_callback_->Callback(hot_plug_display_id_, false);
     }
   }
-  modeset_lock_.unlock();
 }
 
 void PhysicalDisplay::VSyncControl(bool enabled) {

diff --git a/wsi/physicaldisplay.h b/wsi/physicaldisplay.h
index b5f69e9..011c3a3 100644 (file)
--- a/wsi/physicaldisplay.h
+++ b/wsi/physicaldisplay.h
@@ -92,9 +92,7 @@ class PhysicalDisplay : public NativeDisplay, public DisplayPlaneHandler {
 
   void Connect() override;
 
-  bool IsConnected() const override {
-    return !(display_state_ & kDisconnectionInProgress);
-  }
+  bool IsConnected() const override;
 
   bool TestCommit(
       const std::vector<OverlayPlane> &commit_planes) const override;
@@ -160,6 +158,24 @@ class PhysicalDisplay : public NativeDisplay, public DisplayPlaneHandler {
   */
   virtual bool InitializeDisplay() = 0;
 
+  /**
+  * API for informing the display that it might be disconnected in near
+  * future.
+  */
+  void MarkForDisconnect();
+
+  /**
+  * API for informing the clients resgistered via RegisterHotPlugCallback
+  * that this display had been disconnected.
+  */
+  void NotifyClientOfConnectedState();
+
+  /**
+  * API for informing the clients resgistered via RegisterHotPlugCallback
+  * that this display had been connected.
+  */
+  void NotifyClientOfDisConnectedState();
+
  private:
   bool UpdatePowerMode();
   void RefreshClones();
@@ -167,13 +183,14 @@ class PhysicalDisplay : public NativeDisplay, public DisplayPlaneHandler {
 
  protected:
   enum DisplayState {
-    kConnected = 1 << 0,
-    kNeedsModeset = 1 << 1,
-    kPendingPowerMode = 1 << 2,
-    kUpdateDisplay = 1 << 3,
-    kDisconnectionInProgress = 1 << 4,
-    kInitialized = 1 << 5,  // Display Queue is initialized.
-    kRefreshClonedDisplays = 1 << 6
+    kNone = 1 << 0,
+    kConnected = 1 << 1,
+    kNeedsModeset = 1 << 2,
+    kPendingPowerMode = 1 << 3,
+    kUpdateDisplay = 1 << 4,
+    kDisconnectionInProgress = 1 << 5,
+    kInitialized = 1 << 6,  // Display Queue is initialized.
+    kRefreshClonedDisplays = 1 << 7
   };
 
   uint32_t pipe_;
@@ -184,7 +201,7 @@ class PhysicalDisplay : public NativeDisplay, public DisplayPlaneHandler {
   int32_t dpiy_;
   uint32_t gpu_fd_;
   uint32_t power_mode_ = kOn;
-  uint32_t display_state_ = 0;
+  int display_state_ = kNone;
   uint32_t hot_plug_display_id_ = 0;
   SpinLock modeset_lock_;
   SpinLock cloned_displays_lock_;