drm_hwcomposer: Use atomic modeset to set crtc timing

Use the new drm blob ioctl and atomic modeset properties to
do atomic modesetting, as opposed to the old setCrtc call.

This allows us to set timing as soon as the system layer requests
a new active config, as opposed to delaying it until we have
an fb. Aside from reducing complexity, this should help with
event control requests as we'll be able to service them with
hw vblanks instead of synthesized sleeps.

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

13 files changed, 236 insertions, 135 deletions

diff --git a/drmcompositor.cpp b/drmcompositor.cpp
index bf95601..dc4bfe8 100644
--- a/drmcompositor.cpp
+++ b/drmcompositor.cpp
@@ -101,36 +101,6 @@ int DrmCompositor::QueueComposition(Composition *composition) {
   return 0;
 }
 
-int DrmCompositor::PerformModeset(DrmCompositionLayerMap_t::iterator begin,
-                                  DrmCompositionLayerMap_t::iterator end) {
-  DrmCompositionLayer *layer = NULL;
-  for (DrmCompositionLayerMap_t::iterator iter = begin; iter != end; ++iter) {
-    if (iter->second.layer.compositionType == HWC_FRAMEBUFFER_TARGET) {
-      layer = &iter->second;
-      break;
-    }
-  }
-  int display = begin->first;
-  if (!layer) {
-    ALOGE("Could not find target framebuffer for display %d", display);
-    return -ENOENT;
-  }
-
-  drmModeModeInfo m;
-  DrmConnector *connector = drm_->GetConnectorForDisplay(display);
-  connector->active_mode().ToModeModeInfo(&m);
-
-  uint32_t connectors = connector->id();
-  int ret = drmModeSetCrtc(drm_->fd(), layer->crtc->id(), layer->bo.fb_id, 0, 0,
-                           &connectors, 1, &m);
-  if (ret)
-    ALOGE("Failed set crtc for disp %d/%d", display, ret);
-  else
-    layer->crtc->set_requires_modeset(false);
-
-  return ret;
-}
-
 int DrmCompositor::CompositeDisplay(DrmCompositionLayerMap_t::iterator begin,
                                     DrmCompositionLayerMap_t::iterator end) {
   int ret = 0;
@@ -150,14 +120,6 @@ int DrmCompositor::CompositeDisplay(DrmCompositionLayerMap_t::iterator begin,
     layer->acquireFenceFd = -1;
   }
 
-  DrmCrtc *crtc = begin->second.crtc;
-  if (crtc->requires_modeset()) {
-    ret = PerformModeset(begin, end);
-    if (ret)
-      ALOGE("Failed modeset on display %d", begin->first);
-    return ret;
-  }
-
   drmModePropertySetPtr pset = drmModePropertySetAlloc();
   if (!pset) {
     ALOGE("Failed to allocate property set");
@@ -171,7 +133,7 @@ int DrmCompositor::CompositeDisplay(DrmCompositionLayerMap_t::iterator begin,
 
     ret =
         drmModePropertySetAdd(pset, plane->id(), plane->crtc_property().id(),
-                              crtc->id()) ||
+                              begin->second.crtc->id()) ||
         drmModePropertySetAdd(pset, plane->id(), plane->fb_property().id(),
                               comp->bo.fb_id) ||
         drmModePropertySetAdd(pset, plane->id(), plane->crtc_x_property().id(),
diff --git a/drmcompositor.h b/drmcompositor.h
index ad1316c..2d4eb69 100644
--- a/drmcompositor.h
+++ b/drmcompositor.h
@@ -59,8 +59,6 @@ class DrmCompositor : public Compositor {
 
   int CompositeDisplay(DrmCompositionLayerMap_t::iterator begin,
                        DrmCompositionLayerMap_t::iterator end);
-  int PerformModeset(DrmCompositionLayerMap_t::iterator begin,
-                     DrmCompositionLayerMap_t::iterator end);
 
   DrmResources *drm_;
 
diff --git a/drmconnector.cpp b/drmconnector.cpp
index 44864b9..7e9b0c1 100644
--- a/drmconnector.cpp
+++ b/drmconnector.cpp
@@ -50,6 +50,11 @@ int DrmConnector::Init() {
     ALOGE("Could not get DPMS property\n");
     return ret;
   }
+  ret = drm_->GetConnectorProperty(*this, "CRTC_ID", &crtc_id_property_);
+  if (ret) {
+    ALOGE("Could not get CRTC_ID property\n");
+    return ret;
+  }
   return 0;
 }
 
@@ -105,21 +110,18 @@ const DrmMode &DrmConnector::active_mode() const {
   return active_mode_;
 }
 
-int DrmConnector::set_active_mode(uint32_t mode_id) {
-  for (std::vector<DrmMode>::const_iterator iter = modes_.begin();
-       iter != modes_.end(); ++iter) {
-    if (iter->id() == mode_id) {
-      active_mode_ = *iter;
-      return 0;
-    }
-  }
-  return -ENOENT;
+void DrmConnector::set_active_mode(const DrmMode &mode) {
+  active_mode_ = mode;
 }
 
 const DrmProperty &DrmConnector::dpms_property() const {
   return dpms_property_;
 }
 
+const DrmProperty &DrmConnector::crtc_id_property() const {
+  return crtc_id_property_;
+}
+
 DrmConnector::ModeIter DrmConnector::begin_modes() const {
   return modes_.begin();
 }
diff --git a/drmconnector.h b/drmconnector.h
index b31d9a9..0db8913 100644
--- a/drmconnector.h
+++ b/drmconnector.h
@@ -53,9 +53,10 @@ class DrmConnector {
   ModeIter begin_modes() const;
   ModeIter end_modes() const;
   const DrmMode &active_mode() const;
-  int set_active_mode(uint32_t mode_id);
+  void set_active_mode(const DrmMode &mode);
 
   const DrmProperty &dpms_property() const;
+  const DrmProperty &crtc_id_property() const;
 
   EncoderIter begin_possible_encoders() const;
   EncoderIter end_possible_encoders() const;
@@ -84,6 +85,7 @@ class DrmConnector {
   std::vector<DrmMode> modes_;
 
   DrmProperty dpms_property_;
+  DrmProperty crtc_id_property_;
 
   std::vector<DrmEncoder *> possible_encoders_;
 };
diff --git a/drmcrtc.cpp b/drmcrtc.cpp
index 91d5fd6..c2e1d6a 100644
--- a/drmcrtc.cpp
+++ b/drmcrtc.cpp
@@ -14,19 +14,23 @@
  * limitations under the License.
  */
 
+#define LOG_TAG "hwc-drm-crtc"
+
 #include "drmcrtc.h"
 #include "drmresources.h"
 
 #include <stdint.h>
 #include <xf86drmMode.h>
 
+#include <cutils/log.h>
+
 namespace android {
 
-DrmCrtc::DrmCrtc(drmModeCrtcPtr c, unsigned pipe)
-    : id_(c->crtc_id),
+DrmCrtc::DrmCrtc(DrmResources *drm, drmModeCrtcPtr c, unsigned pipe)
+    : drm_(drm),
+      id_(c->crtc_id),
       pipe_(pipe),
       display_(-1),
-      requires_modeset_(true),
       x_(c->x),
       y_(c->y),
       width_(c->width),
@@ -38,6 +42,21 @@ DrmCrtc::DrmCrtc(drmModeCrtcPtr c, unsigned pipe)
 DrmCrtc::~DrmCrtc() {
 }
 
+int DrmCrtc::Init() {
+  int ret = drm_->GetCrtcProperty(*this, "ACTIVE", &active_property_);
+  if (ret) {
+    ALOGE("Failed to get ACTIVE property");
+    return ret;
+  }
+
+  ret = drm_->GetCrtcProperty(*this, "MODE_ID", &mode_property_);
+  if (ret) {
+    ALOGE("Failed to get MODE_ID property");
+    return ret;
+  }
+  return 0;
+}
+
 uint32_t DrmCrtc::id() const {
   return id_;
 }
@@ -46,24 +65,23 @@ unsigned DrmCrtc::pipe() const {
   return pipe_;
 }
 
-bool DrmCrtc::requires_modeset() const {
-  return requires_modeset_;
-}
-
-void DrmCrtc::set_requires_modeset(bool requires_modeset) {
-  requires_modeset_ = requires_modeset;
-}
-
 int DrmCrtc::display() const {
   return display_;
 }
 
 void DrmCrtc::set_display(int display) {
   display_ = display;
-  requires_modeset_ = true;
 }
 
 bool DrmCrtc::can_bind(int display) const {
   return display_ == -1 || display_ == display;
 }
+
+const DrmProperty &DrmCrtc::active_property() const {
+  return active_property_;
+}
+
+const DrmProperty &DrmCrtc::mode_property() const {
+  return mode_property_;
+}
 }
diff --git a/drmcrtc.h b/drmcrtc.h
index db9eb76..9789f12 100644
--- a/drmcrtc.h
+++ b/drmcrtc.h
@@ -18,37 +18,42 @@
 #define ANDROID_DRM_CRTC_H_
 
 #include "drmmode.h"
+#include "drmproperty.h"
 
 #include <stdint.h>
 #include <xf86drmMode.h>
 
 namespace android {
 
+class DrmResources;
+
 class DrmCrtc {
  public:
-  DrmCrtc(drmModeCrtcPtr c, unsigned pipe);
+  DrmCrtc(DrmResources *drm, drmModeCrtcPtr c, unsigned pipe);
   ~DrmCrtc();
 
+  int Init();
+
   uint32_t id() const;
   unsigned pipe() const;
 
-  bool requires_modeset() const;
-  void set_requires_modeset(bool requires_modeset);
-
   int display() const;
   void set_display(int display);
 
   bool can_bind(int display) const;
 
+  const DrmProperty &active_property() const;
+  const DrmProperty &mode_property() const;
+
  private:
   DrmCrtc(const DrmCrtc &);
 
+  DrmResources *drm_;
+
   uint32_t id_;
   unsigned pipe_;
   int display_;
 
-  bool requires_modeset_;
-
   uint32_t x_;
   uint32_t y_;
   uint32_t width_;
@@ -56,6 +61,9 @@ class DrmCrtc {
 
   DrmMode mode_;
   bool mode_valid_;
+
+  DrmProperty active_property_;
+  DrmProperty mode_property_;
 };
 }
 
diff --git a/drmmode.cpp b/drmmode.cpp
index c2def1e..dce9803 100644
--- a/drmmode.cpp
+++ b/drmmode.cpp
@@ -19,7 +19,6 @@
 
 #include <stdint.h>
 #include <string>
-
 #include <xf86drmMode.h>
 
 namespace android {
@@ -75,7 +74,7 @@ bool DrmMode::operator==(const drmModeModeInfo &m) const {
          type_ == m.type;
 }
 
-void DrmMode::ToModeModeInfo(drmModeModeInfo *m) const {
+void DrmMode::ToDrmModeModeInfo(drm_mode_modeinfo *m) const {
   m->clock = clock_;
   m->hdisplay = h_display_;
   m->hsync_start = h_sync_start_;
diff --git a/drmmode.h b/drmmode.h
index 9e333bd..ab213ef 100644
--- a/drmmode.h
+++ b/drmmode.h
@@ -30,7 +30,7 @@ class DrmMode {
   ~DrmMode();
 
   bool operator==(const drmModeModeInfo &m) const;
-  void ToModeModeInfo(drmModeModeInfo *m) const;
+  void ToDrmModeModeInfo(drm_mode_modeinfo *m) const;
 
   uint32_t id() const;
   void set_id(uint32_t id);
diff --git a/drmproperty.cpp b/drmproperty.cpp
index d4468c7..6dca12e 100644
--- a/drmproperty.cpp
+++ b/drmproperty.cpp
@@ -67,6 +67,8 @@ void DrmProperty::Init(drmModePropertyPtr p, uint64_t value) {
     type_ = DRM_PROPERTY_TYPE_INT;
   else if (flags_ & DRM_MODE_PROP_ENUM)
     type_ = DRM_PROPERTY_TYPE_ENUM;
+  else if (flags_ & DRM_MODE_PROP_OBJECT)
+    type_ = DRM_PROPERTY_TYPE_OBJECT;
 }
 
 uint32_t DrmProperty::id() const {
@@ -93,6 +95,10 @@ int DrmProperty::value(uint64_t *value) const {
       *value = enums_[value_].value_;
       return 0;
 
+    case DRM_PROPERTY_TYPE_OBJECT:
+      *value = value_;
+      return 0;
+
     default:
       return -EINVAL;
   }
diff --git a/drmproperty.h b/drmproperty.h
index 1ebd665..a60ade3 100644
--- a/drmproperty.h
+++ b/drmproperty.h
@@ -27,6 +27,7 @@ namespace android {
 enum DrmPropertyType {
   DRM_PROPERTY_TYPE_INT,
   DRM_PROPERTY_TYPE_ENUM,
+  DRM_PROPERTY_TYPE_OBJECT,
   DRM_PROPERTY_TYPE_INVALID,
 };
 
diff --git a/drmresources.cpp b/drmresources.cpp
index 3429abc..2cda217 100644
--- a/drmresources.cpp
+++ b/drmresources.cpp
@@ -101,7 +101,7 @@ int DrmResources::Init() {
       break;
     }
 
-    DrmCrtc *crtc = new DrmCrtc(c, i);
+    DrmCrtc *crtc = new DrmCrtc(this, c, i);
 
     drmModeFreeCrtc(c);
 
@@ -110,6 +110,13 @@ int DrmResources::Init() {
       ret = -ENOMEM;
       break;
     }
+
+    ret = crtc->Init();
+    if (ret) {
+      ALOGE("Failed to initialize crtc %d", res->crtcs[i]);
+      delete crtc;
+      break;
+    }
     crtcs_.push_back(crtc);
   }
 
@@ -315,43 +322,143 @@ int DrmResources::TryEncoderForDisplay(int display, DrmEncoder *enc) {
   return -EAGAIN;
 }
 
-int DrmResources::SetDisplayActiveMode(int display, uint32_t mode_id) {
-  DrmEncoder *enc;
-  int ret;
-
-  DrmConnector *con = GetConnectorForDisplay(display);
-  if (!con) {
-    ALOGE("Could not locate connector for display %d", display);
-    return -ENODEV;
-  }
-
+int DrmResources::CreateDisplayPipe(DrmConnector *connector) {
+  int display = connector->display();
   /* Try to use current setup first */
-  enc = con->encoder();
-  if (enc) {
-    ret = TryEncoderForDisplay(display, enc);
+  if (connector->encoder()) {
+    int ret = TryEncoderForDisplay(display, connector->encoder());
     if (!ret) {
-      con->set_encoder(enc);
-      return con->set_active_mode(mode_id);
+      return 0;
     } else if (ret != -EAGAIN) {
       ALOGE("Could not set mode %d/%d", display, ret);
       return ret;
     }
   }
 
-  for (DrmConnector::EncoderIter iter = con->begin_possible_encoders();
-       iter != con->end_possible_encoders(); ++iter) {
-    ret = TryEncoderForDisplay(display, *iter);
+  for (DrmConnector::EncoderIter iter = connector->begin_possible_encoders();
+       iter != connector->end_possible_encoders(); ++iter) {
+    int ret = TryEncoderForDisplay(display, *iter);
     if (!ret) {
-      con->set_encoder(*iter);
-      return con->set_active_mode(mode_id);
+      connector->set_encoder(*iter);
+      return 0;
     } else if (ret != -EAGAIN) {
       ALOGE("Could not set mode %d/%d", display, ret);
       return ret;
     }
   }
+  ALOGE("Could not find a suitable encoder/crtc for display %d",
+        connector->display());
+  return -ENODEV;
+}
+
+int DrmResources::CreatePropertyBlob(void *data, size_t length,
+                                     uint32_t *blob_id) {
+  struct drm_mode_create_blob create_blob;
+  memset(&create_blob, 0, sizeof(create_blob));
+  create_blob.length = length;
+  create_blob.data = (__u64)data;
+
+  int ret = drmIoctl(fd_, DRM_IOCTL_MODE_CREATEPROPBLOB, &create_blob);
+  if (ret) {
+    ALOGE("Failed to create mode property blob %d", ret);
+    return ret;
+  }
+  *blob_id = create_blob.blob_id;
+  return 0;
+}
+
+int DrmResources::DestroyPropertyBlob(uint32_t blob_id) {
+  struct drm_mode_destroy_blob destroy_blob;
+  memset(&destroy_blob, 0, sizeof(destroy_blob));
+  destroy_blob.blob_id = (__u32)blob_id;
+  int ret = drmIoctl(fd_, DRM_IOCTL_MODE_DESTROYPROPBLOB, &destroy_blob);
+  if (ret) {
+    ALOGE("Failed to destroy mode property blob %d", ret);
+    return ret;
+  }
+  return 0;
+}
+
+int DrmResources::SetDisplayActiveMode(int display, const DrmMode &mode) {
+  DrmConnector *connector = GetConnectorForDisplay(display);
+  if (!connector) {
+    ALOGE("Could not locate connector for display %d", display);
+    return -ENODEV;
+  }
+
+  int ret = CreateDisplayPipe(connector);
+  if (ret) {
+    ALOGE("Failed CreateDisplayPipe with %d", ret);
+    return ret;
+  }
+
+  DrmCrtc *crtc = connector->encoder()->crtc();
+  DrmProperty old_mode;
+  ret = GetCrtcProperty(*crtc, crtc->mode_property().name().c_str(), &old_mode);
+  if (ret) {
+    ALOGE("Failed to get old mode property from crtc %d", crtc->id());
+    return ret;
+  }
+
+  struct drm_mode_modeinfo drm_mode;
+  memset(&drm_mode, 0, sizeof(drm_mode));
+  mode.ToDrmModeModeInfo(&drm_mode);
+
+  uint32_t blob_id;
+  ret =
+      CreatePropertyBlob(&drm_mode, sizeof(struct drm_mode_modeinfo), &blob_id);
+  if (ret) {
+    ALOGE("Failed to create mode property blob %d", ret);
+    return ret;
+  }
+
+  drmModePropertySetPtr pset = drmModePropertySetAlloc();
+  if (!pset) {
+    ALOGE("Failed to allocate property set");
+    DestroyPropertyBlob(blob_id);
+    return -ENOMEM;
+  }
 
-  ALOGE("Could not find a suitable encoder/crtc for display %d", display);
-  return -EINVAL;
+  ret = drmModePropertySetAdd(pset, crtc->id(), crtc->mode_property().id(),
+                              blob_id) ||
+        drmModePropertySetAdd(pset, connector->id(),
+                              connector->crtc_id_property().id(), crtc->id());
+  if (ret) {
+    ALOGE("Failed to add blob %d to pset", blob_id);
+    DestroyPropertyBlob(blob_id);
+    drmModePropertySetFree(pset);
+    return ret;
+  }
+
+  ret =
+      drmModePropertySetCommit(fd_, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL, pset);
+
+  drmModePropertySetFree(pset);
+
+  if (ret) {
+    ALOGE("Failed to commit pset ret=%d\n", ret);
+    DestroyPropertyBlob(blob_id);
+    return ret;
+  }
+
+  connector->set_active_mode(mode);
+
+  uint64_t old_blob_id;
+  ret = old_mode.value(&old_blob_id);
+  if (ret) {
+    ALOGE("Could not get old blob id value %d", ret);
+    return ret;
+  }
+  if (!old_blob_id)
+    return 0;
+
+  ret = DestroyPropertyBlob(old_blob_id);
+  if (ret) {
+    ALOGE("Failed to destroy old mode property blob", old_blob_id);
+    return ret;
+  }
+
+  return 0;
 }
 
 int DrmResources::SetDpmsMode(int display, uint64_t mode) {
@@ -360,13 +467,6 @@ int DrmResources::SetDpmsMode(int display, uint64_t mode) {
     return -EINVAL;
   }
 
-  DrmCrtc *crtc = GetCrtcForDisplay(display);
-  if (!crtc) {
-    ALOGE("Failed to get DrmCrtc for display %d", display);
-    return -ENODEV;
-  }
-  crtc->set_requires_modeset(true);
-
   DrmConnector *c = GetConnectorForDisplay(display);
   if (!c) {
     ALOGE("Failed to get DrmConnector for display %d", display);
@@ -416,6 +516,11 @@ int DrmResources::GetPlaneProperty(const DrmPlane &plane, const char *prop_name,
   return GetProperty(plane.id(), DRM_MODE_OBJECT_PLANE, prop_name, property);
 }
 
+int DrmResources::GetCrtcProperty(const DrmCrtc &crtc, const char *prop_name,
+                                  DrmProperty *property) {
+  return GetProperty(crtc.id(), DRM_MODE_OBJECT_CRTC, prop_name, property);
+}
+
 int DrmResources::GetConnectorProperty(const DrmConnector &connector,
                                        const char *prop_name,
                                        DrmProperty *property) {
diff --git a/drmresources.h b/drmresources.h
index 1261956..1b40d0e 100644
--- a/drmresources.h
+++ b/drmresources.h
@@ -51,11 +51,13 @@ class DrmResources {
 
   int GetPlaneProperty(const DrmPlane &plane, const char *prop_name,
                        DrmProperty *property);
+  int GetCrtcProperty(const DrmCrtc &crtc, const char *prop_name,
+                      DrmProperty *property);
   int GetConnectorProperty(const DrmConnector &connector, const char *prop_name,
                            DrmProperty *property);
 
   uint32_t next_mode_id();
-  int SetDisplayActiveMode(int display, uint32_t mode_id);
+  int SetDisplayActiveMode(int display, const DrmMode &mode);
   int SetDpmsMode(int display, uint64_t mode);
 
  private:
@@ -63,6 +65,11 @@ class DrmResources {
   int GetProperty(uint32_t obj_id, uint32_t obj_type, const char *prop_name,
                   DrmProperty *property);
 
+  int CreateDisplayPipe(DrmConnector *connector);
+
+  int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id);
+  int DestroyPropertyBlob(uint32_t blob_id);
+
   int fd_;
   uint32_t mode_id_;
 
diff --git a/hwcomposer.cpp b/hwcomposer.cpp
index 6a30fbe..6c007bc 100644
--- a/hwcomposer.cpp
+++ b/hwcomposer.cpp
@@ -181,13 +181,8 @@ static int hwc_prepare(hwc_composer_device_1_t *dev, size_t num_displays,
     for (int j = 0; j < num_layers; j++) {
       hwc_layer_1_t *layer = &display_contents[i]->hwLayers[j];
 
-      if (crtc->requires_modeset()) {
-        if (layer->compositionType == HWC_OVERLAY)
-          layer->compositionType = HWC_FRAMEBUFFER;
-      } else {
-        if (layer->compositionType == HWC_FRAMEBUFFER)
-          layer->compositionType = HWC_OVERLAY;
-      }
+      if (layer->compositionType == HWC_FRAMEBUFFER)
+        layer->compositionType = HWC_OVERLAY;
     }
   }
 
@@ -254,17 +249,6 @@ static int hwc_set(hwc_composer_device_1_t *dev, size_t num_displays,
     if (!display_contents[i])
       continue;
 
-    DrmCrtc *crtc = ctx->drm.GetCrtcForDisplay(i);
-    if (!crtc) {
-      ALOGE("No crtc for display %d", i);
-      hwc_set_cleanup(num_displays, display_contents, composition);
-      return -ENODEV;
-    }
-    bool use_target = false;
-    if (crtc->requires_modeset()) {
-      use_target = true;
-    }
-
     hwc_display_contents_1_t *dc = display_contents[i];
     int j;
     unsigned num_layers = 0;
@@ -273,16 +257,14 @@ static int hwc_set(hwc_composer_device_1_t *dev, size_t num_displays,
       hwc_layer_1_t *layer = &dc->hwLayers[j];
       if (layer->flags & HWC_SKIP_LAYER)
         continue;
-      if ((use_target && layer->compositionType == HWC_FRAMEBUFFER_TARGET) ||
-          layer->compositionType == HWC_OVERLAY) {
+      if (layer->compositionType == HWC_OVERLAY)
         num_layers++;
-      }
     }
 
     unsigned num_planes = composition->GetRemainingLayers(i, num_layers);
     bool use_pre_compositor = false;
 
-    if (!use_target && num_layers > num_planes) {
+    if (num_layers > num_planes) {
       use_pre_compositor = true;
       // Reserve one of the planes for the result of the pre compositor.
       num_planes--;
@@ -292,13 +274,8 @@ static int hwc_set(hwc_composer_device_1_t *dev, size_t num_displays,
       hwc_layer_1_t *layer = &dc->hwLayers[j];
       if (layer->flags & HWC_SKIP_LAYER)
         continue;
-      if (use_target) {
-        if (layer->compositionType != HWC_FRAMEBUFFER_TARGET)
-          continue;
-      } else {
-        if (layer->compositionType != HWC_OVERLAY)
-          continue;
-      }
+      if (layer->compositionType != HWC_OVERLAY)
+        continue;
 
       ret = hwc_add_layer(i, ctx, layer, composition);
       if (ret) {
@@ -606,12 +583,28 @@ static int hwc_set_active_config(struct hwc_composer_device_1 *dev, int display,
     return -EINVAL;
   }
 
-  int ret = ctx->drm.SetDisplayActiveMode(display, hd->config_ids[index]);
+  DrmConnector *c = ctx->drm.GetConnectorForDisplay(display);
+  if (!c) {
+    ALOGE("Failed to get connector for display %d", display);
+    return -ENODEV;
+  }
+  DrmMode mode;
+  for (DrmConnector::ModeIter iter = c->begin_modes(); iter != c->end_modes();
+       ++iter) {
+    if (iter->id() == hd->config_ids[index]) {
+      mode = *iter;
+      break;
+    }
+  }
+  if (mode.id() != hd->config_ids[index]) {
+    ALOGE("Could not find active mode for %d/%d", index, hd->config_ids[index]);
+    return -ENOENT;
+  }
+  int ret = ctx->drm.SetDisplayActiveMode(display, mode);
   if (ret) {
-    ALOGE("Failed to set config for display %d", display);
+    ALOGE("Failed to set active config %d", ret);
     return ret;
   }
-
   return ret;
 }