New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 756454 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Drm overlay validation should use actual buffers.

Project Member Reported by dcasta...@chromium.org, Aug 17 2017

Issue description

Currently, when the GLRenderer in the browser needs to check if a set of overlays is valid, it creates an OverlayCandidateList and sends it over to the GPU process for validation via the OverlayCandidateValidator.

OverlayCandidateList doesn't contain a reference to the actual buffers that we're planning to scanout, but only metadata that is later used by DrmOverlayValidator to allocate empty buffers just to do a pageflip test and validate/reject a set of overlays.

Unfortunately the metadata doesn't contain all the information needed, and we end up testing configurations of buffers that might differ from the ones we'll set for the atomic commit.

This can't be fixed adding more information to the metadata since Chrome doesn't even know all the information needed. Buffers imported via exo could be tiled in different ways, or compressed, and Chrome imports them without knowing these details.

Additionally, allocating buffers seems to be expensive, and while we cache them by size, it seems to be the cause of performance issues during resizing (b/64697790), since we end up allocating/deleting buffers every frame just for validation.

We're also wasting memory to keep around empty buffers, on some devices gbm buffers memory might be pinned.


It'd be nice to do the pageflip tests with the actual buffers that we intend to scanout. This should be made easier in the future once we have display compositor and drm thread in the same process.
 
Blockedon: 732805
Labels: Proj-Ozone-DRM Proj-Mustash-Mus-GPU
Status: Available (was: Untriaged)
Blockedon: -732805
Labels: -Pri-3 M-61 Pri-2
Status: Assigned (was: Available)
Discussed this with dcastagna@ and while MUS will make this easier we think it's possible to solve this in a relatively easy way before that and I think we need to as this is likely critical for M61.
Awesome to hear that it can be fixed without the dependency.
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f10ae4adf9ea78feaf8e9140febbe8521314e3e

commit 3f10ae4adf9ea78feaf8e9140febbe8521314e3e
Author: Daniele Castagna <dcastagna@chromium.org>
Date: Fri Aug 18 02:13:32 2017

cc: Set primary plane overlay candidate format.

On cros, when an output surface is displayed as an overlay,
DirectRenderer creates an overlay candidate for it.
Currently the overlay candidate buffer format is not set, and left
to the default RGBX.

When the drm validator tries the configuration of overlays in the drm
thread, it looks for an existing buffer with the same size and format,
and creates one if it can't find it. Since the buffer queue buffers are
allocated as BGRX, we'd end up allocating a full screen RGBX buffer,
instead of reusing the BGRX one, every time we try to validate an
overlay configuration.

This CL sets the buffer format of the overlay candidate to the buffer
format of the buffer queue, so that the primary plane buffer will be
found when testing overlays and we'll avoid creating (and testing)
a buffer with the wrong format.

Bug: 756454, b/64697790
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I21d5fe14af043ed444368a6e1581812fdaec51b2
Reviewed-on: https://chromium-review.googlesource.com/619171
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495426}
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/android_webview/browser/parent_output_surface.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/android_webview/browser/parent_output_surface.h
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/cc/output/direct_renderer.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/cc/output/output_surface.h
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/cc/output/overlay_unittest.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/cc/test/fake_output_surface.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/cc/test/fake_output_surface.h
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/cc/test/pixel_test_output_surface.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/cc/test/pixel_test_output_surface.h
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/components/viz/service/display/gl_renderer_unittest.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/components/viz/service/display_embedder/buffer_queue.h
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/components/viz/service/display_embedder/display_output_surface.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/components/viz/service/display_embedder/display_output_surface.h
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/components/viz/service/display_embedder/display_output_surface_ozone.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/components/viz/service/display_embedder/display_output_surface_ozone.h
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/content/browser/compositor/gpu_browser_compositor_output_surface.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/content/browser/compositor/gpu_browser_compositor_output_surface.h
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/content/browser/compositor/gpu_surfaceless_browser_compositor_output_surface.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/content/browser/compositor/gpu_surfaceless_browser_compositor_output_surface.h
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/content/browser/compositor/offscreen_browser_compositor_output_surface.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/content/browser/compositor/offscreen_browser_compositor_output_surface.h
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/content/browser/compositor/reflector_impl_unittest.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/content/browser/compositor/software_browser_compositor_output_surface.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/content/browser/compositor/software_browser_compositor_output_surface.h
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/content/browser/compositor/vulkan_browser_compositor_output_surface.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/content/browser/compositor/vulkan_browser_compositor_output_surface.h
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/content/renderer/android/synchronous_layer_tree_frame_sink.cc
[modify] https://crrev.com/3f10ae4adf9ea78feaf8e9140febbe8521314e3e/ui/compositor/test/in_process_context_factory.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 25 2017

Labels: merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/02991e7d50bd9d92ecec8192e68bebc65cb83ae2

commit 02991e7d50bd9d92ecec8192e68bebc65cb83ae2
Author: Daniele Castagna <dcastagna@chromium.org>
Date: Fri Aug 25 20:32:20 2017

cc: Set primary plane overlay candidate format.

Cherry pick of I21d5fe14af043ed444368a6e1581812fdaec51b2 to 61.

On cros, when an output surface is displayed as an overlay,
DirectRenderer creates an overlay candidate for it.
Currently the overlay candidate buffer format is not set, and left
to the default RGBX.

When the drm validator tries the configuration of overlays in the drm
thread, it looks for an existing buffer with the same size and format,
and creates one if it can't find it. Since the buffer queue buffers are
allocated as BGRX, we'd end up allocating a full screen RGBX buffer,
instead of reusing the BGRX one, every time we try to validate an
overlay configuration.

This CL sets the buffer format of the overlay candidate to the buffer
format of the buffer queue, so that the primary plane buffer will be
found when testing overlays and we'll avoid creating (and testing)
a buffer with the wrong format.

TBR: reveman@chromium.org
Bug: 756454, b/64697790
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I21d5fe14af043ed444368a6e1581812fdaec51b2
Reviewed-on: https://chromium-review.googlesource.com/619171
Commit-Queue: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: David Reveman <reveman@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#495426}
Reviewed-on: https://chromium-review.googlesource.com/636389
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#901}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/android_webview/browser/parent_output_surface.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/android_webview/browser/parent_output_surface.h
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/cc/output/direct_renderer.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/cc/output/gl_renderer_unittest.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/cc/output/output_surface.h
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/cc/output/overlay_unittest.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/cc/test/fake_output_surface.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/cc/test/fake_output_surface.h
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/cc/test/pixel_test_output_surface.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/cc/test/pixel_test_output_surface.h
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/components/viz/service/display_embedder/buffer_queue.h
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/components/viz/service/display_embedder/display_output_surface.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/components/viz/service/display_embedder/display_output_surface.h
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/components/viz/service/display_embedder/display_output_surface_ozone.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/components/viz/service/display_embedder/display_output_surface_ozone.h
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/content/browser/compositor/gpu_browser_compositor_output_surface.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/content/browser/compositor/gpu_browser_compositor_output_surface.h
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/content/browser/compositor/gpu_surfaceless_browser_compositor_output_surface.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/content/browser/compositor/gpu_surfaceless_browser_compositor_output_surface.h
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/content/browser/compositor/offscreen_browser_compositor_output_surface.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/content/browser/compositor/offscreen_browser_compositor_output_surface.h
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/content/browser/compositor/reflector_impl_unittest.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/content/browser/compositor/software_browser_compositor_output_surface.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/content/browser/compositor/software_browser_compositor_output_surface.h
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/content/browser/compositor/vulkan_browser_compositor_output_surface.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/content/browser/compositor/vulkan_browser_compositor_output_surface.h
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/content/renderer/android/synchronous_layer_tree_frame_sink.cc
[modify] https://crrev.com/02991e7d50bd9d92ecec8192e68bebc65cb83ae2/ui/compositor/test/in_process_context_factory.cc

Labels: -Proj-Mustash-Mus-GPU
Cleaning up old Proj-Mustash labels.

Sign in to add a comment