clients/simple-dmabuf-feedback.c

@@ -1494,43 +1494,31 @@ create_display()
 }
 
 /* Simple client to test the dma-buf feedback implementation. This does not
- * replace the need to implement a dma-buf feedback test that can be run in
- * the CI. But as we still don't know exactly how to do this, this client
- * can be helpful to run tests manually.
+ * replace the need to implement a dma-buf feedback test that can be run in the
+ * CI. But as we still don't know exactly how to do this, this client can be
+ * helpful to run tests manually. It can also be helpful to test other
+ * compositors.
  *
- * In order to use this, we have to hack the DRM-backend to pretend that
- * INITIAL_BUFFER_FORMAT is not supported by the planes of the underlying
- * hardware. In Weston, we have to do this in
- * drm_output_prepare_plane_view(), more specifically in the part where
- * we call drm_output_plane_view_has_valid_format(). So we'd have something
- * like this:
+ * In order to use this client, we have to hack the DRM-backend of the
+ * compositor to pretend that INITIAL_BUFFER_FORMAT is not supported by the
+ * planes of the underlying hardware.
  *
- *     // in this example, INITIAL_BUFFER_FORMAT == DRM_FORMAT_XRGB8888
+ * How this client works:
  *
- *     bool fake_unsupported_format = false;
- *     if (fb && fb->format->format == DRM_FORMAT_XRGB8888)
- *             fake_unsupported_format = true;
+ * This client creates a surface and buffers for it with the same resolution of
+ * the output mode in use. Also, it sets the surface to fullscreen. So we have
+ * everything set to allow the surface to be placed in an overlay plane. But as
+ * these buffers are created with INITIAL_BUFFER_FORMAT, they are not eligible
+ * for direct scanout.
  *
- *     if (!drm_output_plane_view_has_valid_format(plane, state, ev, fb) ||
- *         fake_unsupported_format)
- *         ...
- *
- * It creates a surface and buffers for it with the same resolution of the
- * output mode in use. Also, it sets the surface to fullscreen. So we have
- * everything set to allow the surface to be placed in a plane. But as
- * these buffers are created with INITIAL_BUFFER_FORMAT, they are placed in
- * the renderer.
- *
- * When the compositor creates the client surface, it adds only the
- * renderer tranche to its dma-buf feedback object and send the feedback to
- * the client. But as the repaint cycles start and Weston detects that the
- * only reason why the surface has not been placed in a plane was the
- * incompatibility between the framebuffer format and the ones supported by
- * the planes of the underlying hardware, Weston adds a scanout tranche to
- * the surface dma-buf feedback and resend them. In this tranche the client
- * can find pairs of formats and modifiers supported by the planes, and so
- * it can recreate its buffers using one of these pairs in order to
- * increase the chances of its surface end up in a plane. */
+ * When Weston creates a client's surface, it adds only the renderer tranche to
+ * its dma-buf feedback object and send the feedback to the client. But as the
+ * repaint cycles start and Weston detects that the view of the client is not
+ * eligible for direct scanout because of the incompatibility of the
+ * framebuffer's format/modifier pair and the KMS device, it adds a scanout
+ * tranche to the feedback and resend it. In this scanout tranche the client can
+ * find parameters to re-allocate its buffers and increase its chances of
+ * hitting direct scanout. */
 int
 main(int argc, char **argv)
 {

libweston/backend-drm/state-propose.c

@@ -335,7 +335,7 @@ drm_output_check_zpos_plane_states(struct drm_output_state *state)
 	}
 }
 
-static bool
+static void
 dmabuf_feedback_maybe_update(struct drm_device *device, struct weston_view *ev,
 			     uint32_t try_view_on_plane_failure_reasons)
 {
@@ -344,24 +344,10 @@ dmabuf_feedback_maybe_update(struct drm_device *device, struct weston_view *ev,
 	struct drm_backend *b = device->backend;
 	dev_t scanout_dev = device->drm.devnum;
 	uint32_t scanout_flags = ZWP_LINUX_DMABUF_FEEDBACK_V1_TRANCHE_FLAGS_SCANOUT;
-	uint32_t action_needed = ACTION_NEEDED_NONE;
+	enum actions_needed_dmabuf_feedback action_needed = ACTION_NEEDED_NONE;
 	struct timespec current_time, delta_time;
 	const time_t MAX_TIME_SECONDS = 2;
 
-	/* Find out what we need to do with the dma-buf feedback */
-	if (try_view_on_plane_failure_reasons & FAILURE_REASONS_FORCE_RENDERER)
-		action_needed |= ACTION_NEEDED_REMOVE_SCANOUT_TRANCHE;
-	if (try_view_on_plane_failure_reasons &
-		(FAILURE_REASONS_ADD_FB_FAILED |
-		 FAILURE_REASONS_FB_FORMAT_INCOMPATIBLE |
-		 FAILURE_REASONS_DMABUF_MODIFIER_INVALID |
-		 FAILURE_REASONS_GBM_BO_IMPORT_FAILED |
-		 FAILURE_REASONS_GBM_BO_GET_HANDLE_FAILED))
-		action_needed |= ACTION_NEEDED_ADD_SCANOUT_TRANCHE;
-
-	assert(action_needed != (ACTION_NEEDED_REMOVE_SCANOUT_TRANCHE |
-				 ACTION_NEEDED_ADD_SCANOUT_TRANCHE));
-
 	/* Look for scanout tranche. If not found, add it but in disabled mode
 	 * (we still don't know if we'll have to send it to clients). This
 	 * simplifies the code. */
@@ -372,19 +358,34 @@ dmabuf_feedback_maybe_update(struct drm_device *device, struct weston_view *ev,
 		scanout_tranche =
 			weston_dmabuf_feedback_tranche_create(dmabuf_feedback,
 					b->compositor->dmabuf_feedback_format_table,
-					scanout_dev, scanout_flags,
-					SCANOUT_PREF);
+					scanout_dev, scanout_flags, SCANOUT_PREF);
 		scanout_tranche->active = false;
 	}
 
+	/* Direct scanout won't happen even if client re-allocates using
+	 * params from the scanout tranche, so keep only the renderer tranche. */
+	if (try_view_on_plane_failure_reasons & (FAILURE_REASONS_FORCE_RENDERER |
+						 FAILURE_REASONS_NO_PLANES_AVAILABLE)) {
+		action_needed = ACTION_NEEDED_REMOVE_SCANOUT_TRANCHE;
+	/* Direct scanout may be possible if client re-allocates using the
+	 * params from the scanout tranche. */
+	} else if (try_view_on_plane_failure_reasons & (FAILURE_REASONS_ADD_FB_FAILED |
+							FAILURE_REASONS_FB_FORMAT_INCOMPATIBLE |
+							FAILURE_REASONS_DMABUF_MODIFIER_INVALID |
+							FAILURE_REASONS_GBM_BO_IMPORT_FAILED |
+							FAILURE_REASONS_GBM_BO_GET_HANDLE_FAILED)) {
+		action_needed = ACTION_NEEDED_ADD_SCANOUT_TRANCHE;
+	/* Direct scanout is already possible, so include the scanout tranche. */
+	} else if (try_view_on_plane_failure_reasons == FAILURE_REASONS_NONE) {
+		action_needed = ACTION_NEEDED_ADD_SCANOUT_TRANCHE;
+	}
+
 	/* No actions needed, so disarm timer and return */
 	if (action_needed == ACTION_NEEDED_NONE ||
-	    (action_needed == ACTION_NEEDED_ADD_SCANOUT_TRANCHE &&
-	     scanout_tranche->active) ||
-	    (action_needed == ACTION_NEEDED_REMOVE_SCANOUT_TRANCHE &&
-	     !scanout_tranche->active)) {
+	    (action_needed == ACTION_NEEDED_ADD_SCANOUT_TRANCHE && scanout_tranche->active) ||
+	    (action_needed == ACTION_NEEDED_REMOVE_SCANOUT_TRANCHE && !scanout_tranche->active)) {
 		dmabuf_feedback->action_needed = ACTION_NEEDED_NONE;
-		return false;
+		return;
 	}
 
 	/* We hit this if:
@@ -399,7 +400,7 @@ dmabuf_feedback_maybe_update(struct drm_device *device, struct weston_view *ev,
 	    dmabuf_feedback->action_needed != action_needed) {
 		clock_gettime(CLOCK_MONOTONIC, &dmabuf_feedback->timer);
 		dmabuf_feedback->action_needed = action_needed;
-		return false;
+		return;
 	/* Timer is already on and the action needed when it was set to on does
 	 * not conflict with the most recent needed action we've detected. If
 	 * more than MAX_TIME_SECONDS has passed, we need to resend the dma-buf
@@ -409,7 +410,7 @@ dmabuf_feedback_maybe_update(struct drm_device *device, struct weston_view *ev,
 		delta_time.tv_sec = current_time.tv_sec -
 				    dmabuf_feedback->timer.tv_sec;
 		if (delta_time.tv_sec < MAX_TIME_SECONDS)
-			return false;
+			return;
 	}
 
 	/* If we got here it means that the timer has triggered, so we have
@@ -429,8 +430,6 @@ dmabuf_feedback_maybe_update(struct drm_device *device, struct weston_view *ev,
 
 	/* Set the timer to off */
 	dmabuf_feedback->action_needed = ACTION_NEEDED_NONE;
-
-	return true;
 }
 
 static struct drm_plane_state *