drm_hwcomposer: Process DPMS requests through compositor

This patch changes the behavior of DPMS in hwcomposer from
applying asynchronously/immediately, to queuing in the
compositor and being processed in order. This is desirable
for a couple of reasons:
  1- It ensures all frames set before set_power_mode are
     shown on the screen before it turns off
  2- We make sure we don't rmfb a framebuffer that is
     currently applied to a disabled crtc.

The second reason above can cause the display to turn back
off once it's on since the fb will dereference to zero in
the kernel and it will disable the pipe without notifying
us.

Change-Id: I2aab9ee0353b12fecced46766ed2dbb64f0aef4b
Signed-off-by: Sean Paul <seanpaul@chromium.org>

diff --git a/drmcomposition.cpp b/drmcomposition.cpp
index 37e3bad..7a50601 100644 (file)
--- a/drmcomposition.cpp
+++ b/drmcomposition.cpp
@@ -118,6 +118,10 @@ int DrmComposition::AddLayer(int display, hwc_layer_1_t *layer,
   return composition_map_[display]->AddLayer(layer, bo, crtc, plane);
 }
 
+int DrmComposition::AddDpmsMode(int display, uint32_t dpms_mode) {
+  return composition_map_[display]->AddDpmsMode(dpms_mode);
+}
+
 std::unique_ptr<DrmDisplayComposition> DrmComposition::TakeDisplayComposition(
     int display) {
   return std::move(composition_map_[display]);

diff --git a/drmcomposition.h b/drmcomposition.h
index a8f9836..e490811 100644 (file)
--- a/drmcomposition.h
+++ b/drmcomposition.h
@@ -41,6 +41,7 @@ class DrmComposition : public Composition {
 
   virtual unsigned GetRemainingLayers(int display, unsigned num_needed) const;
   virtual int AddLayer(int display, hwc_layer_1_t *layer, hwc_drm_bo_t *bo);
+  int AddDpmsMode(int display, uint32_t dpms_mode);
 
   std::unique_ptr<DrmDisplayComposition> TakeDisplayComposition(int display);
 

diff --git a/drmdisplaycomposition.cpp b/drmdisplaycomposition.cpp
index f049f2d..2f8d651 100644 (file)
--- a/drmdisplaycomposition.cpp
+++ b/drmdisplaycomposition.cpp
@@ -26,6 +26,7 @@
 #include <cutils/log.h>
 #include <sw_sync.h>
 #include <sync/sync.h>
+#include <xf86drmMode.h>
 
 namespace android {
 
@@ -43,7 +44,8 @@ DrmDisplayComposition::DrmDisplayComposition()
       importer_(NULL),
       type_(DRM_COMPOSITION_TYPE_EMPTY),
       timeline_fd_(-1),
-      timeline_(0) {
+      timeline_(0),
+      dpms_mode_(DRM_MODE_DPMS_ON) {
 }
 
 DrmDisplayComposition::~DrmDisplayComposition() {
@@ -109,6 +111,14 @@ int DrmDisplayComposition::AddLayer(hwc_layer_1_t *layer, hwc_drm_bo_t *bo,
   return 0;
 }
 
+int DrmDisplayComposition::AddDpmsMode(uint32_t dpms_mode) {
+  if (!validate_composition_type(DRM_COMPOSITION_TYPE_DPMS))
+    return -EINVAL;
+  dpms_mode_ = dpms_mode;
+  type_ = DRM_COMPOSITION_TYPE_DPMS;
+  return 0;
+}
+
 int DrmDisplayComposition::FinishComposition() {
   int ret = sw_sync_timeline_inc(timeline_fd_, timeline_);
   if (ret)
@@ -120,4 +130,8 @@ int DrmDisplayComposition::FinishComposition() {
 DrmCompositionLayerVector_t *DrmDisplayComposition::GetCompositionLayers() {
   return &layers_;
 }
+
+uint32_t DrmDisplayComposition::dpms_mode() const {
+  return dpms_mode_;
+}
 }

diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h
index bc8cf2e..2ae696f 100644 (file)
--- a/drmdisplaycomposition.h
+++ b/drmdisplaycomposition.h
@@ -31,6 +31,7 @@ namespace android {
 enum DrmCompositionType {
   DRM_COMPOSITION_TYPE_EMPTY,
   DRM_COMPOSITION_TYPE_FRAME,
+  DRM_COMPOSITION_TYPE_DPMS,
 };
 
 typedef struct DrmCompositionLayer {
@@ -55,10 +56,12 @@ class DrmDisplayComposition {
 
   int AddLayer(hwc_layer_1_t *layer, hwc_drm_bo_t *bo, DrmCrtc *crtc,
                DrmPlane *plane);
+  int AddDpmsMode(uint32_t dpms_mode);
 
   int FinishComposition();
 
   DrmCompositionLayerVector_t *GetCompositionLayers();
+  uint32_t dpms_mode() const;
 
  private:
   DrmDisplayComposition(const DrmDisplayComposition &) = delete;
@@ -74,6 +77,7 @@ class DrmDisplayComposition {
   int timeline_;
 
   DrmCompositionLayerVector_t layers_;
+  uint32_t dpms_mode_;
 };
 }
 

diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
index 2a11ee7..80b71f5 100644 (file)
--- a/drmdisplaycompositor.cpp
+++ b/drmdisplaycompositor.cpp
@@ -40,6 +40,7 @@ DrmDisplayCompositor::DrmDisplayCompositor()
       worker_(this),
       frame_no_(0),
       initialized_(false),
+      active_(false),
       dump_frames_composited_(0),
       dump_last_timestamp_ns_(0) {
   struct timespec ts;
@@ -95,6 +96,15 @@ int DrmDisplayCompositor::QueueComposition(
     std::unique_ptr<DrmDisplayComposition> composition) {
   switch (composition->type()) {
   case DRM_COMPOSITION_TYPE_FRAME:
+    if (!active_)
+      return -ENODEV;
+    break;
+  case DRM_COMPOSITION_TYPE_DPMS:
+    /*
+     * Update the state as soon as we get it so we can start/stop queuing
+     * frames asap.
+     */
+    active_ = (composition->dpms_mode() == DRM_MODE_DPMS_ON);
     break;
   case DRM_COMPOSITION_TYPE_EMPTY:
     return 0;
@@ -189,6 +199,23 @@ int DrmDisplayCompositor::ApplyFrame(DrmDisplayComposition *display_comp) {
   return ret;
 }
 
+int DrmDisplayCompositor::ApplyDpms(DrmDisplayComposition *display_comp) {
+  DrmConnector *conn = drm_->GetConnectorForDisplay(display_);
+  if (!conn) {
+    ALOGE("Failed to get DrmConnector for display %d", display_);
+    return -ENODEV;
+  }
+
+  const DrmProperty &prop = conn->dpms_property();
+  int ret = drmModeConnectorSetProperty(drm_->fd(), conn->id(), prop.id(),
+                                        display_comp->dpms_mode());
+  if (ret) {
+    ALOGE("Failed to set DPMS property for connector %d", conn->id());
+    return ret;
+  }
+  return 0;
+}
+
 int DrmDisplayCompositor::Composite() {
   ATRACE_CALL();
   int ret = pthread_mutex_lock(&lock_);
@@ -222,6 +249,11 @@ int DrmDisplayCompositor::Composite() {
     }
     ++dump_frames_composited_;
     break;
+  case DRM_COMPOSITION_TYPE_DPMS:
+    ret = ApplyDpms(composition.get());
+    if (ret)
+      ALOGE("Failed to apply dpms for display %d", display_);
+    return ret;
   default:
     ALOGE("Unknown composition type %d", composition->type());
     return -EINVAL;

diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
index 928ce46..9f50664 100644 (file)
--- a/drmdisplaycompositor.h
+++ b/drmdisplaycompositor.h
@@ -47,6 +47,7 @@ class DrmDisplayCompositor {
   DrmDisplayCompositor(const DrmDisplayCompositor &) = delete;
 
   int ApplyFrame(DrmDisplayComposition *display_comp);
+  int ApplyDpms(DrmDisplayComposition *display_comp);
 
   DrmResources *drm_;
   int display_;
@@ -59,6 +60,7 @@ class DrmDisplayCompositor {
   uint64_t frame_no_;
 
   bool initialized_;
+  bool active_;
 
   // mutable since we need to acquire in HaveQueuedComposites
   mutable pthread_mutex_t lock_;

diff --git a/drmresources.cpp b/drmresources.cpp
index 2cda217..9be990f 100644 (file)
--- a/drmresources.cpp
+++ b/drmresources.cpp
@@ -467,19 +467,22 @@ int DrmResources::SetDpmsMode(int display, uint64_t mode) {
     return -EINVAL;
   }
 
-  DrmConnector *c = GetConnectorForDisplay(display);
-  if (!c) {
-    ALOGE("Failed to get DrmConnector for display %d", display);
-    return -ENODEV;
+  DrmComposition *comp = (DrmComposition *)compositor_.CreateComposition(NULL);
+  if (!comp) {
+    ALOGE("Failed to create composition for dpms on %d", display);
+    return -ENOMEM;
   }
-
-  const DrmProperty &prop = c->dpms_property();
-  int ret = drmModeConnectorSetProperty(fd_, c->id(), prop.id(), mode);
+  int ret = comp->AddDpmsMode(display, mode);
   if (ret) {
-    ALOGE("Failed to set DPMS property for connector %d", c->id());
+    ALOGE("Failed to add dpms %ld to composition on %d %d", mode, display, ret);
+    delete comp;
+    return ret;
+  }
+  ret = compositor_.QueueComposition((Composition *)comp);
+  if (ret) {
+    ALOGE("Failed to queue dpms composition on %d %d", display, ret);
     return ret;
   }
-
   return 0;
 }