Advertising supported Dmabuf modifiers of DRM device

Submitted by Philipp Zabel on July 27, 2018, 1:10 p.m.

Details

Message ID 1532697009.4694.6.camel@pengutronix.de
State New
Series "Advertising supported Dmabuf modifiers of DRM device"
Headers show

Commit Message

Philipp Zabel July 27, 2018, 1:10 p.m.
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@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@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@pengutronix.de>

---------->8----------

Patch hide | download patch | download mbox

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;

Comments

Daniel Stone July 27, 2018, 4:13 p.m.
Hi Philipp,

On Fri, 27 Jul 2018 at 14:10, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> On Fri, 2018-07-27 at 11:23 +0100, Daniel Stone wrote:
> > 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?

To be honest, I'm not entirely sure. What I would want to check is the
damage processing: that the area under the alpha-blended plane still
got updated through the renderer if required. It should be pretty easy
to check by putting simple-shm under an alpha-blended plane, and
seeing if that got updated. Probably the tri-fan debug mode would help
here as well.

> > > 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?

Yeah, exactly!

Cheers,
Daniel