drm_hwcomposer: Process modesets via compositor

This patch queues modeset in the compositor for application on
the next frame. This allows us to perform the modeset atomically
with the first frame that comes in after the mode is changed.

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

diff --git a/drmcomposition.cpp b/drmcomposition.cpp
index 634b0bb..55fa00c 100644 (file)
--- a/drmcomposition.cpp
+++ b/drmcomposition.cpp
@@ -90,6 +90,10 @@ int DrmComposition::SetDpmsMode(int display, uint32_t dpms_mode) {
   return composition_map_[display]->SetDpmsMode(dpms_mode);
 }
 
+int DrmComposition::SetDisplayMode(int display, const DrmMode &display_mode) {
+  return composition_map_[display]->SetDisplayMode(display_mode);
+}
+
 std::unique_ptr<DrmDisplayComposition> DrmComposition::TakeDisplayComposition(
     int display) {
   return std::move(composition_map_[display]);
@@ -106,7 +110,8 @@ int DrmComposition::DisableUnusedPlanes() {
      * TODO: re-visit this and potentially disable leftover planes after the
      *       active compositions have gobbled up all they can
      */
-    if (comp->type() == DRM_COMPOSITION_TYPE_EMPTY)
+    if (comp->type() == DRM_COMPOSITION_TYPE_EMPTY ||
+        comp->type() == DRM_COMPOSITION_TYPE_MODESET)
       continue;
 
     DrmCrtc *crtc = drm_->GetCrtcForDisplay(display);

diff --git a/drmcomposition.h b/drmcomposition.h
index a5d9cf1..ad7a5df 100644 (file)
--- a/drmcomposition.h
+++ b/drmcomposition.h
@@ -47,6 +47,7 @@ class DrmComposition {
 
   int SetLayers(size_t num_displays, DrmCompositionDisplayLayersMap *maps);
   int SetDpmsMode(int display, uint32_t dpms_mode);
+  int SetDisplayMode(int display, const DrmMode &display_mode);
 
   std::unique_ptr<DrmDisplayComposition> TakeDisplayComposition(int display);
   DrmDisplayComposition *GetDisplayComposition(int display);

diff --git a/drmdisplaycomposition.cpp b/drmdisplaycomposition.cpp
index 457fb5a..d47b5dc 100644 (file)
--- a/drmdisplaycomposition.cpp
+++ b/drmdisplaycomposition.cpp
@@ -231,6 +231,15 @@ int DrmDisplayComposition::SetDpmsMode(uint32_t dpms_mode) {
   return 0;
 }
 
+int DrmDisplayComposition::SetDisplayMode(const DrmMode &display_mode) {
+  if (!validate_composition_type(DRM_COMPOSITION_TYPE_MODESET))
+    return -EINVAL;
+  display_mode_ = display_mode;
+  dpms_mode_ = DRM_MODE_DPMS_ON;
+  type_ = DRM_COMPOSITION_TYPE_MODESET;
+  return 0;
+}
+
 int DrmDisplayComposition::AddPlaneDisable(DrmPlane *plane) {
   layers_.emplace_back();
   DrmCompositionLayer &c_layer = layers_.back();
@@ -271,6 +280,10 @@ uint64_t DrmDisplayComposition::frame_no() const {
   return frame_no_;
 }
 
+const DrmMode &DrmDisplayComposition::display_mode() const {
+  return display_mode_;
+}
+
 Importer *DrmDisplayComposition::importer() const {
   return importer_;
 }

diff --git a/drmdisplaycomposition.h b/drmdisplaycomposition.h
index 06784f1..57e8521 100644 (file)
--- a/drmdisplaycomposition.h
+++ b/drmdisplaycomposition.h
@@ -35,6 +35,7 @@ enum DrmCompositionType {
   DRM_COMPOSITION_TYPE_EMPTY,
   DRM_COMPOSITION_TYPE_FRAME,
   DRM_COMPOSITION_TYPE_DPMS,
+  DRM_COMPOSITION_TYPE_MODESET,
 };
 
 struct DrmCompositionLayer {
@@ -79,6 +80,7 @@ class DrmDisplayComposition {
                 std::vector<DrmPlane *> *overlay_planes);
   int AddPlaneDisable(DrmPlane *plane);
   int SetDpmsMode(uint32_t dpms_mode);
+  int SetDisplayMode(const DrmMode &display_mode);
 
   void RemoveNoPlaneLayers();
   int SignalPreCompositionDone();
@@ -87,6 +89,7 @@ class DrmDisplayComposition {
   std::vector<DrmCompositionLayer> *GetCompositionLayers();
   int pre_composition_layer_index() const;
   uint32_t dpms_mode() const;
+  const DrmMode &display_mode() const;
 
   uint64_t frame_no() const;
 
@@ -116,6 +119,7 @@ class DrmDisplayComposition {
   std::vector<DrmCompositionLayer> layers_;
   int pre_composition_layer_index_;
   uint32_t dpms_mode_;
+  DrmMode display_mode_;
 
   uint64_t frame_no_;
 };

diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
index 82da4f2..5676f1d 100644 (file)
--- a/drmdisplaycompositor.cpp
+++ b/drmdisplaycompositor.cpp
@@ -46,6 +46,7 @@ DrmDisplayCompositor::DrmDisplayCompositor()
       worker_(this),
       initialized_(false),
       active_(false),
+      needs_modeset_(false),
       framebuffer_index_(0),
       dump_frames_composited_(0),
       dump_last_timestamp_ns_(0) {
@@ -112,6 +113,8 @@ int DrmDisplayCompositor::QueueComposition(
        */
       active_ = (composition->dpms_mode() == DRM_MODE_DPMS_ON);
       break;
+    case DRM_COMPOSITION_TYPE_MODESET:
+      break;
     case DRM_COMPOSITION_TYPE_EMPTY:
       return 0;
     default:
@@ -233,12 +236,65 @@ int DrmDisplayCompositor::ApplyFrame(DrmDisplayComposition *display_comp) {
       return ret;
   }
 
+  DrmConnector *connector = drm_->GetConnectorForDisplay(display_);
+  if (!connector) {
+    ALOGE("Could not locate connector for display %d", display_);
+    return -ENODEV;
+  }
+  DrmCrtc *crtc = drm_->GetCrtcForDisplay(display_);
+  if (!crtc) {
+    ALOGE("Could not locate crtc for display %d", display_);
+    return -ENODEV;
+  }
+
   drmModePropertySetPtr pset = drmModePropertySetAlloc();
   if (!pset) {
     ALOGE("Failed to allocate property set");
     return -ENOMEM;
   }
 
+  uint32_t blob_id = 0;
+  uint64_t old_blob_id;
+  if (needs_modeset_) {
+    DrmProperty old_mode;
+    ret = drm_->GetCrtcProperty(*crtc, crtc->mode_property().name().c_str(),
+                                &old_mode);
+    if (ret) {
+      ALOGE("Failed to get old mode property from crtc %d", crtc->id());
+      drmModePropertySetFree(pset);
+      return ret;
+    }
+    ret = old_mode.value(&old_blob_id);
+    if (ret) {
+      ALOGE("Could not get old blob id value %d", ret);
+      drmModePropertySetFree(pset);
+      return ret;
+    }
+
+    struct drm_mode_modeinfo drm_mode;
+    memset(&drm_mode, 0, sizeof(drm_mode));
+    next_mode_.ToDrmModeModeInfo(&drm_mode);
+
+    ret = drm_->CreatePropertyBlob(&drm_mode, sizeof(struct drm_mode_modeinfo),
+                                   &blob_id);
+    if (ret) {
+      ALOGE("Failed to create mode property blob %d", ret);
+      drmModePropertySetFree(pset);
+      return ret;
+    }
+
+    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);
+      drmModePropertySetFree(pset);
+      drm_->DestroyPropertyBlob(blob_id);
+      return ret;
+    }
+  }
+
   std::vector<DrmCompositionLayer> *layers =
       display_comp->GetCompositionLayers();
   for (DrmCompositionLayer &layer : *layers) {
@@ -248,16 +304,16 @@ int DrmDisplayCompositor::ApplyFrame(DrmDisplayComposition *display_comp) {
       if (ret) {
         ALOGE("Failed to wait for acquire %d/%d", acquire_fence, ret);
         drmModePropertySetFree(pset);
+        drm_->DestroyPropertyBlob(blob_id);
         return ret;
       }
       layer.acquire_fence.Close();
     }
 
     DrmPlane *plane = layer.plane;
-    DrmCrtc *crtc = layer.crtc;
 
     // Disable the plane if there's no crtc
-    if (!crtc) {
+    if (!layer.crtc) {
       ret = drmModePropertySetAdd(pset, plane->id(),
                                   plane->crtc_property().id(), 0) ||
             drmModePropertySetAdd(pset, plane->id(), plane->fb_property().id(),
@@ -312,7 +368,7 @@ int DrmDisplayCompositor::ApplyFrame(DrmDisplayComposition *display_comp) {
 
     ret =
         drmModePropertySetAdd(pset, plane->id(), plane->crtc_property().id(),
-                              crtc->id()) ||
+                              layer.crtc->id()) ||
         drmModePropertySetAdd(pset, plane->id(), plane->fb_property().id(),
                               layer.buffer->fb_id) ||
         drmModePropertySetAdd(pset, plane->id(), plane->crtc_x_property().id(),
@@ -352,13 +408,38 @@ int DrmDisplayCompositor::ApplyFrame(DrmDisplayComposition *display_comp) {
   }
 
   if (!ret) {
-    ret = drmModePropertySetCommit(drm_->fd(), 0, drm_, pset);
-    if (ret)
+    ret = drmModePropertySetCommit(drm_->fd(), DRM_MODE_ATOMIC_ALLOW_MODESET,
+                                   drm_, pset);
+    if (ret) {
       ALOGE("Failed to commit pset ret=%d\n", ret);
+      drmModePropertySetFree(pset);
+      if (needs_modeset_)
+        drm_->DestroyPropertyBlob(blob_id);
+      return ret;
+    }
   }
   if (pset)
     drmModePropertySetFree(pset);
 
+  if (needs_modeset_) {
+    ret = drm_->DestroyPropertyBlob(old_blob_id);
+    if (ret) {
+      ALOGE("Failed to destroy old mode property blob %lld/%d", old_blob_id,
+            ret);
+      return ret;
+    }
+
+    /* TODO: Add dpms to the pset when the kernel supports it */
+    ret = ApplyDpms(display_comp);
+    if (ret) {
+      ALOGE("Failed to apply DPMS after modeset %d\n", ret);
+      return ret;
+    }
+
+    connector->set_active_mode(next_mode_);
+    needs_modeset_ = false;
+  }
+
   return ret;
 }
 
@@ -428,6 +509,10 @@ int DrmDisplayCompositor::Composite() {
       if (ret)
         ALOGE("Failed to apply dpms for display %d", display_);
       return ret;
+    case DRM_COMPOSITION_TYPE_MODESET:
+      next_mode_ = composition->display_mode();
+      needs_modeset_ = true;
+      return 0;
     default:
       ALOGE("Unknown composition type %d", composition->type());
       return -EINVAL;

diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
index 4aadc68..c9973c6 100644 (file)
--- a/drmdisplaycompositor.h
+++ b/drmdisplaycompositor.h
@@ -70,6 +70,9 @@ class DrmDisplayCompositor {
   bool initialized_;
   bool active_;
 
+  DrmMode next_mode_;
+  bool needs_modeset_;
+
   int framebuffer_index_;
   DrmFramebuffer framebuffers_[DRM_DISPLAY_BUFFERS];
   std::unique_ptr<GLWorkerCompositor> pre_compositor_;

diff --git a/drmresources.cpp b/drmresources.cpp
index 32fe5cc..1b7eb25 100644 (file)
--- a/drmresources.cpp
+++ b/drmresources.cpp
@@ -242,7 +242,18 @@ int DrmResources::Init() {
   if (ret)
     return ret;
 
-  return compositor_.Init();
+  ret = compositor_.Init();
+  if (ret)
+    return ret;
+
+  for (auto i = begin_connectors(); i != end_connectors(); ++i) {
+    ret = CreateDisplayPipe(*i);
+    if (ret) {
+      ALOGE("Failed CreateDisplayPipe %d with %d", (*i)->id(), ret);
+      return ret;
+    }
+  }
+  return 0;
 }
 
 int DrmResources::fd() const {
@@ -368,96 +379,36 @@ int DrmResources::CreatePropertyBlob(void *data, size_t length,
 }
 
 int DrmResources::DestroyPropertyBlob(uint32_t blob_id) {
+  if (!blob_id)
+    return 0;
+
   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);
+    ALOGE("Failed to destroy mode property blob %ld/%d", blob_id, 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);
+  std::unique_ptr<DrmComposition> comp(compositor_.CreateComposition(NULL));
+  if (!comp) {
+    ALOGE("Failed to create composition for dpms on %d", display);
     return -ENOMEM;
   }
-
-  ret = drmModePropertySetAdd(pset, crtc->id(), crtc->mode_property().id(),
-                              blob_id) ||
-        drmModePropertySetAdd(pset, connector->id(),
-                              connector->crtc_id_property().id(), crtc->id());
+  int ret = comp->SetDisplayMode(display, mode);
   if (ret) {
-    ALOGE("Failed to add blob %d to pset", blob_id);
-    DestroyPropertyBlob(blob_id);
-    drmModePropertySetFree(pset);
+    ALOGE("Failed to add mode to composition on %d %d", display, ret);
     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);
+  ret = compositor_.QueueComposition(std::move(comp));
   if (ret) {
-    ALOGE("Failed to destroy old mode property blob", old_blob_id);
+    ALOGE("Failed to queue dpms composition on %d %d", display, ret);
     return ret;
   }
-
   return 0;
 }
 

diff --git a/drmresources.h b/drmresources.h
index 1b40d0e..3ec7d2c 100644 (file)
--- a/drmresources.h
+++ b/drmresources.h
@@ -60,6 +60,9 @@ class DrmResources {
   int SetDisplayActiveMode(int display, const DrmMode &mode);
   int SetDpmsMode(int display, uint64_t mode);
 
+  int CreatePropertyBlob(void *data, size_t length, uint32_t *blob_id);
+  int DestroyPropertyBlob(uint32_t blob_id);
+
  private:
   int TryEncoderForDisplay(int display, DrmEncoder *enc);
   int GetProperty(uint32_t obj_id, uint32_t obj_type, const char *prop_name,
@@ -67,9 +70,6 @@ class DrmResources {
 
   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_;