diff mbox series

[RFC,1/2] drm_hwcomposer: Cleanup gl precompositor init and provide uses_GL flag

Message ID 1524528404-12331-1-git-send-email-john.stultz@linaro.org
State Superseded
Headers show
Series [RFC,1/2] drm_hwcomposer: Cleanup gl precompositor init and provide uses_GL flag | expand

Commit Message

John Stultz April 24, 2018, 12:06 a.m. UTC 
The drm_hwcomposer has its own GL pre-compositor which is used
to squish layers when there are more layers then planes on the
display hardware. In many ways this duplicates the client-side
GL compositing that is done in SurfaceFlinger, but in theory can
be more highly optimized for the hardware.

Unfortunately, due to these optimizations, the drm_hwcomposer's
pre-compositor becomes somewhat hardware specific (originally
targeting nvidia hardware, I believe).

So on some hardware, the gl precompositor may not actually
initialize due to hardware missing features, or the hardware
supporting different shader APIs.

Rather then try to rework the drm_hwcomposers precompositor
to be more generic, I instead suggest that when the
precompositor fails to initialize, we simply fall back to the
already more widely compatible client compositor in
SurfaceFlinger.

Thus, this patch cleans up some of the precompositor
initialization, which didn't handle failures well.

Feedback or alternative ideas would be greatly appreciated!

Cc: Marissa Wall <marissaw@google.com>
Cc: Sean Paul <seanpaul@google.com>
Cc: Dmitry Shmidt <dimitrysh@google.com>
Cc: Robert Foss <robert.foss@collabora.com>
Cc: Matt Szczesiak <matt.szczesiak@arm.com>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: David Hanna <david.hanna11@gmail.com>
Cc: Rob Herring <rob.herring@linaro.org>
Cc: Alexandru-Cosmin Gheorghe <Alexandru-Cosmin.Gheorghe@arm.com>
Cc: Alistair Strachan <astrachan@google.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>

---
 drmdisplaycompositor.cpp | 40 +++++++++++++++++++++-------------------
 drmdisplaycompositor.h   |  3 +++
 2 files changed, 24 insertions(+), 19 deletions(-)

-- 
2.7.4

Comments

Rob Herring (Arm) April 24, 2018, 1:30 a.m. UTC | #1 
Please use "PATCH hwc" for drm_hwcomposer patches.

On Mon, Apr 23, 2018 at 7:06 PM, John Stultz <john.stultz@linaro.org> wrote:
> The drm_hwcomposer has its own GL pre-compositor which is used
> to squish layers when there are more layers then planes on the
> display hardware. In many ways this duplicates the client-side
> GL compositing that is done in SurfaceFlinger, but in theory can
> be more highly optimized for the hardware.
>
> Unfortunately, due to these optimizations, the drm_hwcomposer's
> pre-compositor becomes somewhat hardware specific (originally
> targeting nvidia hardware, I believe).
>
> So on some hardware, the gl precompositor may not actually
> initialize due to hardware missing features, or the hardware
> supporting different shader APIs.
>
> Rather then try to rework the drm_hwcomposers precompositor
> to be more generic, I instead suggest that when the
> precompositor fails to initialize, we simply fall back to the
> already more widely compatible client compositor in
> SurfaceFlinger.
>
> Thus, this patch cleans up some of the precompositor
> initialization, which didn't handle failures well.
>
> Feedback or alternative ideas would be greatly appreciated!
>
> Cc: Marissa Wall <marissaw@google.com>
> Cc: Sean Paul <seanpaul@google.com>
> Cc: Dmitry Shmidt <dimitrysh@google.com>
> Cc: Robert Foss <robert.foss@collabora.com>
> Cc: Matt Szczesiak <matt.szczesiak@arm.com>
> Cc: Liviu Dudau <Liviu.Dudau@arm.com>
> Cc: David Hanna <david.hanna11@gmail.com>
> Cc: Rob Herring <rob.herring@linaro.org>
> Cc: Alexandru-Cosmin Gheorghe <Alexandru-Cosmin.Gheorghe@arm.com>
> Cc: Alistair Strachan <astrachan@google.com>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>  drmdisplaycompositor.cpp | 40 +++++++++++++++++++++-------------------
>  drmdisplaycompositor.h   |  3 +++
>  2 files changed, 24 insertions(+), 19 deletions(-)

Both patches look good to me.

Reviewed-by: Rob Herring <robh@kernel.org>
diff mbox series

Patch

diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
index e556e86..5c6bf9b 100644
--- a/drmdisplaycompositor.cpp
+++ b/drmdisplaycompositor.cpp
@@ -222,6 +222,13 @@  int DrmDisplayCompositor::Init(DrmResources *drm, int display) {
     return ret;
   }
 
+  pre_compositor_.reset(new GLWorkerCompositor());
+  ret = pre_compositor_->Init();
+  if (ret) {
+    ALOGE("Failed to initialize OpenGL compositor %d", ret);
+    pre_compositor_.reset();
+  }
+
   initialized_ = true;
   return 0;
 }
@@ -294,14 +301,16 @@  int DrmDisplayCompositor::ApplySquash(DrmDisplayComposition *display_comp) {
   }
 
   std::vector<DrmCompositionRegion> &regions = display_comp->squash_regions();
-  ret = pre_compositor_->Composite(display_comp->layers().data(),
+  if (pre_compositor_) {
+    ret = pre_compositor_->Composite(display_comp->layers().data(),
                                    regions.data(), regions.size(), fb.buffer(),
                                    display_comp->importer());
-  pre_compositor_->Finish();
+    pre_compositor_->Finish();
 
-  if (ret) {
-    ALOGE("Failed to squash layers");
-    return ret;
+    if (ret) {
+      ALOGE("Failed to squash layers");
+      return ret;
+    }
   }
 
   ret = display_comp->CreateNextTimelineFence();
@@ -328,14 +337,16 @@  int DrmDisplayCompositor::ApplyPreComposite(
   }
 
   std::vector<DrmCompositionRegion> &regions = display_comp->pre_comp_regions();
-  ret = pre_compositor_->Composite(display_comp->layers().data(),
+  if (pre_compositor_) {
+    ret = pre_compositor_->Composite(display_comp->layers().data(),
                                    regions.data(), regions.size(), fb.buffer(),
                                    display_comp->importer());
-  pre_compositor_->Finish();
+    pre_compositor_->Finish();
 
-  if (ret) {
-    ALOGE("Failed to pre-composite layers");
-    return ret;
+    if (ret) {
+      ALOGE("Failed to pre-composite layers");
+      return ret;
+    }
   }
 
   ret = display_comp->CreateNextTimelineFence();
@@ -395,15 +406,6 @@  int DrmDisplayCompositor::PrepareFrame(DrmDisplayComposition *display_comp) {
   std::vector<DrmCompositionRegion> &pre_comp_regions =
       display_comp->pre_comp_regions();
 
-  if (!pre_compositor_) {
-    pre_compositor_.reset(new GLWorkerCompositor());
-    int ret = pre_compositor_->Init();
-    if (ret) {
-      ALOGE("Failed to initialize OpenGL compositor %d", ret);
-      return ret;
-    }
-  }
-
   int squash_layer_index = -1;
   if (squash_regions.size() > 0) {
     squash_framebuffer_index_ = (squash_framebuffer_index_ + 1) % 2;
diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
index f1965fb..ed6c5f9 100644
--- a/drmdisplaycompositor.h
+++ b/drmdisplaycompositor.h
@@ -98,6 +98,9 @@  class DrmDisplayCompositor {
     return &squash_state_;
   }
 
+  bool uses_GL() {
+    return !!pre_compositor_;
+  }
  private:
   struct ModeState {
     bool needs_modeset = false;