Ozone: Dont hardcode format to YUV when using Overlay Composition.

ui/ozone/platform/drm/gpu/drm_overlay_validator.h

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef UI_OZONE_PLATFORM_DRM_GPU_DRM_OVERLAY_VALIDATOR_H_
 #define UI_OZONE_PLATFORM_DRM_GPU_DRM_OVERLAY_VALIDATOR_H_
 
 #include "base/containers/mru_cache.h"
 #include "ui/ozone/platform/drm/gpu/overlay_plane.h"
 
@@ skipping 11 matching lines @@
   ~DrmOverlayValidator();
 
   // Tests if configurations |params| are compatible with |window_| and finds
   // which of these configurations can be promoted to Overlay composition
   // without failing the page flip. It expects |params| to be sorted by z_order.
   std::vector<OverlayCheck_Params> TestPageFlip(
       const std::vector<OverlayCheck_Params>& params,
       const OverlayPlaneList& last_used_planes,
       ScanoutBufferGenerator* buffer_generator);
 
-  // Tries to predict preferred format supported by hardware planes for |plane|
-  // for the given combination of |plane_list|. Using this format can help
-  // reduce read memory bandwidth during scanout for this plane.
-  uint32_t GetOptimalBufferFormat(const OverlayPlane& plane,
-                                  const OverlayPlaneList& plane_list) const;
+  OverlayPlaneList ValidatePlanesForPageFlip(

[dnicoara, 2016/01/13 17:26:51]

Could you please add some comments describing this. In particular, focus on the
fact that this optimizes formats and such but doesn't actually change anything
in how planes are presented, etc.

Also, the name isn't quite right. At first I though this was a second "test page
flip" which didn't seem right. Maybe something like OptimizeBuffersForPageFlip()
or HandleBufferHints()? Feel free to suggest others.

[kalyank, 2016/01/13 19:52:33]

Done.

+      const OverlayPlaneList& planes,
+      ScanoutBufferGenerator* buffer_generator);

[dnicoara, 2016/01/13 17:26:51]

Maybe we should pass in the buffer generator in the constructor. It'll simplify
this call and TestPageFlip().

[kalyank, 2016/01/13 19:52:33]

Done.

 
   // Clears internal cache of validated overlay configurations. This should be
   // usually called when |window_| size has changed or moved controller.
   void ClearCache();
 
  private:
   // Contains hints which can be used to reduce display read memory bandwidth,
   // for a given OverlayPlane. These are useful in case of video to determine
   // if converting the buffer storage format before composition could lead to
   // any potential bandwidth savings. Other useful hint is to determine if
   // scaling needs to be done before page flip or can be handled by plane.
   struct OverlayHints {
     OverlayHints(uint32_t optimal_format, bool handle_scaling);
     ~OverlayHints();
     // Optimal buffer storage format supported by hardware for a given
     // OverlayPlane. This hint can be ignored and still compositing an
     // OverlayPlane should not fail page flip or cause any visual artifacts.
     uint32_t optimal_format;
     // Hints if buffer scaling needs to be done before page flip as plane cannot
     // support it. Ignoring this hint may result in displaying buffer with wrong
     // resolution.
     bool handle_scaling;
   };
 
+  // Expectation is that we have same combination of layers being used over

[dnicoara, 2016/01/13 17:26:51]

Do you have an idea of what the overhead would be? I'm wondering if the extra
complexity is needed since changing the overlay configuration would not be a
frequent operation and this would only help the first frame after the change.

In fact, I'm almost tempted to say this is optimized at the wrong layer. If we
needed to apply scaling, I imagine we'd want to cache the new buffer within the
GbmPixmap such that we'd re-use the buffer every time.

It seems like the "void CopyBuffer(const scoped_refptr<ScanoutBuffer>, const
scoped_refptr<ScanoutBuffer>)" should be more like:
"scoped_refptr<ScanoutBuffer> GetProcessedBuffer(BufferFormat)"

[kalyank, 2016/01/13 19:52:33]

We would end up allocating new buffers every frame.
I started of on this but needed to have access to DrmDevice/Buffer generator etc
here.

[dnicoara, 2016/01/13 20:21:53]

I think we can just drop this. The more I read it the more I'm convinced that
it'll cause issues (Based on my reading it looks like it'll re-use buffers
incorrectly as well). Anyways, I think the old approach you were trying (the one
mentioned bellow) can work. Could you try that first?
The buffer has access to DrmDevice and we could just allocate without the buffer
generator. We're replacing one buffer with another and given that we start with
a GbmBuffer, I think it is fair to assume you'll get a GbmBuffer.

Let me know if you still have concerns.

[kalyank, 2016/01/14 06:24:05]

Done.

+  // a period of time. If this combination needs to process buffers in
+  // ValidatePlanesForPageFlip, it would be beneficial to hold on to these
+  // buffers so that they can be recycled instead of re-creating them for every
+  // frame.
+  struct OverlayBuffer {
+    OverlayBuffer(const scoped_refptr<ScanoutBuffer>& buffer, bool is_in_use);
+    ~OverlayBuffer();
+
+    scoped_refptr<ScanoutBuffer> buffer;
+    bool is_in_use;
+  };
+
   using OverlayHintsList = std::vector<OverlayHints>;
 
   // Check if we can optimize format for reducing Display controller read
   // bandwidth for |plane_list| and cache the value.
   void UpdateOverlayHintsCache(
       const scoped_refptr<DrmDevice>& drm,
       const OverlayPlaneList& plane_list,
       ScanoutBufferGenerator* buffer_generator,
       std::vector<scoped_refptr<ScanoutBuffer>>* reusable_buffers);
 
+  scoped_refptr<ScanoutBuffer> GetBufferForPageFlip(
+      const scoped_refptr<DrmDevice>& drm_device,
+      ScanoutBufferGenerator* buffer_generator,
+      const gfx::Size& size,
+      uint32_t format);
+
   DrmWindow* window_;  // Not owned.
 
+  std::vector<OverlayBuffer> overlay_buffers_cache_;
+
   // List of all configurations which have been validated.
   base::MRUCacheBase<OverlayPlaneList,
                      OverlayHintsList,
                      base::MRUCacheNullDeletor<OverlayHintsList>>
       overlay_hints_cache_;
 
   DISALLOW_COPY_AND_ASSIGN(DrmOverlayValidator);
 };
 
 }  // namespace ui
 
 #endif  // UI_OZONE_PLATFORM_DRM_GPU_DRM_OVERLAY_VALIDATOR_H_