[hwc,3/4] drm_hwcomposer: Cleanup gl precompositor init and provide uses_GL flag

Message ID 1524769557-6108-3-git-send-email-john.stultz@linaro.org
State New
Headers show
Series
  • [hwc,1/4] drm_hwcomposer: Andorid.mk : Mark libdrmhwc_utils as vendor module
Related show

Commit Message

John Stultz April 26, 2018, 7:05 p.m.
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.

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>
Reviewed-by: Rob Herring <robh@kernel.org>

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

Robert Foss April 26, 2018, 7:43 p.m. | #1
This patch is:
Acked-by: Robert Foss <robert.foss@collabora.com>


On 04/26/2018 09:05 PM, John Stultz 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.
> 
> 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>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
>   drmdisplaycompositor.cpp | 40 +++++++++++++++++++++-------------------
>   drmdisplaycompositor.h   |  3 +++
>   2 files changed, 24 insertions(+), 19 deletions(-)
> 
> diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> index e570923..40af3be 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;
>

Patch

diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
index e570923..40af3be 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;