Advertising supported Dmabuf modifiers of DRM device

[Philipp Zabel]

Hi Daniel, Emre,

On Fri, 2018-07-27 at 11:23 +0100, Daniel Stone wrote:
> Hi Emre,
> 
> On Fri, 27 Jul 2018 at 07:44, Ucan, Emre (ADITG/ESB)
> <eucan at de.adit-jv.com> wrote:
> > IMO, entire point of having wayland surfaces and buffers to use them for display scanout.
> > 
> > If an application does not want to display its rendered content, it can use surfaceless context or render to texture etc.
> 
> Sure, and where possible we should use the overlay! But if there are
> more Wayland surface views than there are overlay planes, it is not
> possible to show all them on the overlay. In that case, we should fall
> back to the most efficient method, i.e. what is best for the GPU.
> 
> Taking i.MX6 as an example - if we know we can never show at least one
> view on an overlay (because it's transformed, or too many views, etc),
> then we should not waste any time and energy having that surface do a
> linear resolve. We can just skip this step and have the GPU output
> tiled and then consume tiled during composition.

Since you mention i.MX6, it can support an alpha transparent overlay.
I have attached a patch to test this locally below, but that doesn't
check for alpha plane support. Would it be enough to check whether pixel
formats with alpha channel are advertised as supported by the plane?

> > Furthermore, we don’t have information in weston about content producing device.
> > If an application wants to render with gpu, it should be responsibility of the application to query and use gpu friendly modifiers.
> > We can provide them a list of scanout friendly modifiers which application can use. Then, application should choose a format which would work for rendering and scanout.
> 
> Right, but again this may lead us to lose performance if the
> application tries to find the lowest-common-denominator layout, when
> it doesn't need to. Hence the suggestion for the compositor suggesting
> the most appropriate sets of modifiers to clients depending on the
> situation.

Could this be used to notify Vulkan clients that they should reallocate
their swapchain for better performance, by setting swapchain status to
VK_SUBOPTIMAL_KHR?

regards
Philipp

----------8<----------
Author: Philipp Zabel <p.zabel at pengutronix.de>
Date:   Tue May 8 13:00:32 2018 +0200
Subject: compositor-drm: only scanout needs to be opaque, overlay planes may support transparency
    
Defer to drm_output_prepare_overlay_view to decide whether non-opaque
views an be put onto an overlay plane.
    
Signed-off-by: Philipp Zabel <p.zabel at pengutronix.de>

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 763e18c02160..1f701df88985 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -1982,6 +1982,11 @@ drm_output_prepare_scanout_view(struct drm_output_state *output_state,
 	if (!fb)
 		return NULL;
 
+	if (!pixel_format_is_opaque(fb->format)) {
+		drm_fb_unref(fb);
+		return NULL;
+	}
+
 	/* Can't change formats with just a pageflip */
 	if (!b->atomic_modeset && fb->format->format != output->gbm_format) {
 		drm_fb_unref(fb);
@@ -3402,6 +3407,7 @@ drm_output_propose_state(struct weston_output *output_base,
 		pixman_region32_t clipped_view;
 		bool totally_occluded = false;
 		bool overlay_occluded = false;
+		bool view_is_opaque;
 
 		/* If this view doesn't touch our output at all, there's no
 		 * reason to do anything with it. */
@@ -3459,17 +3465,17 @@ drm_output_propose_state(struct weston_output *output_base,
 
 		/* If sprites are disabled or the view is not fully opaque, we
 		 * must put the view into the renderer - unless it has already
-		 * been placed in the cursor plane, which can handle alpha. */
+		 * been placed in the cursor plane, or it can be placed in
+		 * another plane that can handle alpha. */
 		if (!ps && !planes_ok)
 			force_renderer = true;
-		if (!ps && !drm_view_is_opaque(ev))
-			force_renderer = true;
+		view_is_opaque = drm_view_is_opaque(ev);
 
 		/* Only try to place scanout surfaces in planes-only mode; in
 		 * mixed mode, we have already failed to place a view on the
 		 * scanout surface, forcing usage of the renderer on the
 		 * scanout plane. */
-		if (!ps && !force_renderer && !renderer_ok)
+		if (!ps && view_is_opaque && !force_renderer && !renderer_ok)
 			ps = drm_output_prepare_scanout_view(state, ev, mode);
 
 		if (!ps && !overlay_occluded && !force_renderer)
@@ -3484,9 +3490,10 @@ drm_output_propose_state(struct weston_output *output_base,
 			 * be added to the renderer region nor the occluded
 			 * region. */
 			if (ps->plane->type != WDRM_PLANE_TYPE_CURSOR) {
-				pixman_region32_union(&occluded_region,
-						      &occluded_region,
-						      &clipped_view);
+				if (view_is_opaque)
+					pixman_region32_union(&occluded_region,
+							      &occluded_region,
+							      &clipped_view);
 				pixman_region32_fini(&clipped_view);
 			}
 			continue;
---------->8----------