Hi Daniel,
On Tue, 26 Sep 2017 18:16:09 +0100, Daniel Stone wrote:
> The atomic API can allow us to test state before we apply it, to see if
> it will be valid. Add support for this, which we will later use in
> assign_planes to ensure our plane configuration is valid.
>
> Signed-off-by: Daniel Stone <dani...@collabora.com>
> ---
> libweston/compositor-drm.c | 180
> +++++++++++++++++++++++++++++++++++----------
> 1 file changed, 141 insertions(+), 39 deletions(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 7a720c34..e678fa7d 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -1619,6 +1619,7 @@ drm_pending_state_get_output(struct drm_pending_state
> *pending_state,
> }
>
> static int drm_pending_state_apply(struct drm_pending_state *state);
> +static int drm_pending_state_test(struct drm_pending_state *state);
>
> /**
> * Mark a drm_output_state (the output's last state) as complete. This
> handles
> @@ -1728,12 +1729,15 @@ enum drm_output_propose_state_mode {
>
> static struct drm_plane_state *
> drm_output_prepare_scanout_view(struct drm_output_state *output_state,
> - struct weston_view *ev)
> + struct weston_view *ev,
> + enum drm_output_propose_state_mode mode)
> {
> struct drm_output *output = output_state->output;
> struct drm_plane *scanout_plane = output->scanout_plane;
> struct drm_plane_state *state;
> + struct drm_plane_state *state_old = NULL;
> struct drm_fb *fb;
> + int ret;
>
> fb = drm_fb_get_from_view(output_state, ev);
> if (!fb)
> @@ -1746,7 +1750,16 @@ drm_output_prepare_scanout_view(struct
> drm_output_state *output_state,
> }
>
> state = drm_output_state_get_plane(output_state, scanout_plane);
> - if (state->fb) {
> +
> + /* Check if we've already placed a buffer on this plane; if there's a
> + * buffer there but it comes from GBM, then it's the result of
> + * drm_output_propose_state placing it here for testing purposes. */
> + if (state->fb &&
> + (state->fb->type == BUFFER_GBM_SURFACE ||
> + state->fb->type == BUFFER_PIXMAN_DUMB)) {
> + state_old = calloc(1, sizeof(*state_old));
> + memcpy(state_old, state, sizeof(*state_old));
> + } else if (state->fb) {
> drm_fb_unref(fb);
> return NULL;
> }
> @@ -1765,10 +1778,21 @@ drm_output_prepare_scanout_view(struct
> drm_output_state *output_state,
> state->dest_h != (unsigned) output->base.current_mode->height)
> goto err;
>
> + if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY)
> + return state;
> +
> + ret = drm_pending_state_test(output_state->pending_state);
> + if (ret != 0)
> + goto err;
> +
> return state;
>
> err:
> - drm_plane_state_put_back(state);
> + drm_plane_state_free(state, false);
> + if (state_old) {
> + wl_list_insert(&output_state->plane_list, &state_old->link);
> + state_old->output_state = output_state;
> + }
> return NULL;
> }
>
> @@ -1843,7 +1867,9 @@ drm_output_render(struct drm_output_state *state,
> pixman_region32_t *damage)
> * want to render. */
> scanout_state = drm_output_state_get_plane(state,
> output->scanout_plane);
> - if (scanout_state->fb)
> + if (scanout_state->fb &&
> + scanout_state->fb->type != BUFFER_GBM_SURFACE &&
> + scanout_state->fb->type != BUFFER_PIXMAN_DUMB)
> return;
>
> if (!pixman_region32_not_empty(damage) &&
> @@ -1866,6 +1892,7 @@ drm_output_render(struct drm_output_state *state,
> pixman_region32_t *damage)
> return;
> }
>
> + drm_fb_unref(scanout_state->fb);
> scanout_state->fb = fb;
> scanout_state->output = output;
>
> @@ -2245,8 +2272,14 @@ err:
> return -1;
> }
>
> +enum drm_pending_state_mode {
> + DRM_PENDING_STATE_TEST,
> + DRM_PENDING_STATE_APPLY,
> +};
> +
> static int
> -drm_pending_state_apply_atomic(struct drm_pending_state *pending_state)
> +drm_pending_state_apply_atomic(struct drm_pending_state *pending_state,
> + enum drm_pending_state_mode mode)
> {
> struct drm_backend *b = pending_state->backend;
> struct drm_output_state *output_state, *tmp;
> @@ -2386,8 +2419,15 @@ drm_pending_state_apply_atomic(struct
> drm_pending_state *pending_state)
> goto out;
> }
>
> - flags |= DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
> + if (mode == DRM_PENDING_STATE_TEST)
> + flags |= DRM_MODE_ATOMIC_TEST_ONLY;
> + else
> + flags |= DRM_MODE_PAGE_FLIP_EVENT | DRM_MODE_ATOMIC_NONBLOCK;
> ret = drmModeAtomicCommit(b->drm.fd, req, flags, b);
> +
> + if (mode == DRM_PENDING_STATE_TEST)
> + return ret;
Leaks req and should drmModeAtomicFree(req) before returning or goto out.
Michael
> +
> if (ret != 0) {
> weston_log("atomic: couldn't commit new state: %m\n");
> goto out;
> @@ -2409,6 +2449,22 @@ out:
> }
> #endif
>
> +static int
> +drm_pending_state_test(struct drm_pending_state *pending_state)
> +{
> +#ifdef HAVE_DRM_ATOMIC
> + struct drm_backend *b = pending_state->backend;
> +
> + if (b->atomic_modeset)
> + return drm_pending_state_apply_atomic(pending_state,
> + DRM_PENDING_STATE_TEST);
> +#endif
> +
> + /* We have no way to test state before application on the legacy
> + * modesetting API, so just claim it succeeded. */
> + return 0;
> +}
> +
> static int
> drm_pending_state_apply(struct drm_pending_state *pending_state)
> {
> @@ -2418,7 +2474,8 @@ drm_pending_state_apply(struct drm_pending_state
> *pending_state)
>
> #ifdef HAVE_DRM_ATOMIC
> if (b->atomic_modeset)
> - return drm_pending_state_apply_atomic(pending_state);
> + return drm_pending_state_apply_atomic(pending_state,
> + DRM_PENDING_STATE_APPLY);
> #endif
>
> if (b->state_invalid) {
> @@ -2720,7 +2777,8 @@ atomic_flip_handler(int fd, unsigned int frame,
> unsigned int sec,
>
> static struct drm_plane_state *
> drm_output_prepare_overlay_view(struct drm_output_state *output_state,
> - struct weston_view *ev)
> + struct weston_view *ev,
> + enum drm_output_propose_state_mode mode)
> {
> struct drm_output *output = output_state->output;
> struct weston_compositor *ec = output->base.compositor;
> @@ -2729,9 +2787,7 @@ drm_output_prepare_overlay_view(struct drm_output_state
> *output_state,
> struct drm_plane_state *state = NULL;
> struct drm_fb *fb;
> unsigned int i;
> -
> - if (b->sprites_are_broken)
> - return NULL;
> + int ret;
>
> fb = drm_fb_get_from_view(output_state, ev);
> if (!fb)
> @@ -2763,29 +2819,37 @@ drm_output_prepare_overlay_view(struct
> drm_output_state *output_state,
> continue;
> }
>
> - break;
> - }
> + state->ev = ev;
> + state->output = output;
> + drm_plane_state_coords_for_view(state, ev);
> + if (state->src_w != state->dest_w << 16 ||
> + state->src_h != state->dest_h << 16) {
> + drm_plane_state_put_back(state);
> + continue;
> + }
>
> - /* No sprites available */
> - if (!state) {
> - drm_fb_unref(fb);
> - return NULL;
> - }
> + /* We hold one reference for the lifetime of this function;
> + * from calling drm_fb_get_from_view, to the out label where
> + * we unconditionally drop the reference. So, we take another
> + * reference here to live within the state. */
> + state->fb = drm_fb_ref(fb);
>
> - state->fb = fb;
> - state->ev = ev;
> - state->output = output;
> + /* In planes-only mode, we don't have an incremental state to
> + * test against, so we just hope it'll work. */
> + if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY)
> + goto out;
>
> - drm_plane_state_coords_for_view(state, ev);
> - if (state->src_w != state->dest_w << 16 ||
> - state->src_h != state->dest_h << 16)
> - goto err;
> + ret = drm_pending_state_test(output_state->pending_state);
> + if (ret == 0)
> + goto out;
>
> - return state;
> + drm_plane_state_put_back(state);
> + state = NULL;
> + }
>
> -err:
> - drm_plane_state_put_back(state);
> - return NULL;
> +out:
> + drm_fb_unref(fb);
> + return state;
> }
>
> /**
> @@ -2985,12 +3049,42 @@ drm_output_propose_state(struct weston_output
> *output_base,
> struct weston_view *ev;
> pixman_region32_t surface_overlap, renderer_region, occluded_region;
> bool renderer_ok = (mode != DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
> - bool planes_ok = !b->sprites_are_broken;
> + bool planes_ok = false;
> + bool duplicate_renderer = false;
> + int ret;
> +
> + /* We can only propose planes if we're in plane-only mode (i.e. just
> + * get as much as possible into planes and test at the end), or mixed
> + * mode, where we have our previous renderer buffer (and it's broadly
> + * compatible) to test with. If we don't have these things, then we
> + * don't know whether or not the planes will work, so we conservatively
> + * fall back to just using the renderer. */
> + if (mode == DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY) {
> + if (b->sprites_are_broken)
> + return NULL;
> + planes_ok = true;
> + } else if (output->scanout_plane->state_cur->fb) {
> + struct drm_fb *scanout_fb =
> + output->scanout_plane->state_cur->fb;
> + if ((scanout_fb->type == BUFFER_GBM_SURFACE ||
> + scanout_fb->type == BUFFER_PIXMAN_DUMB) &&
> + scanout_fb->width == output_base->current_mode->width &&
> + scanout_fb->height == output_base->current_mode->height) {
> + planes_ok = true;
> + duplicate_renderer = true;
> + }
> + }
>
> assert(!output->state_last);
> state = drm_output_state_duplicate(output->state_cur,
> pending_state,
> DRM_OUTPUT_STATE_CLEAR_PLANES);
> + if (!planes_ok)
> + return state;
> +
> + if (duplicate_renderer)
> + drm_plane_state_duplicate(state,
> + output->scanout_plane->state_cur);
>
> /*
> * Find a surface for each sprite in the output using some heuristics:
> @@ -3024,6 +3118,9 @@ drm_output_propose_state(struct weston_output
> *output_base,
> if (ev->output_mask != (1u << output->base.id))
> force_renderer = true;
>
> + if (!ev->surface->buffer_ref.buffer)
> + force_renderer = true;
> +
> /* Ignore views we know to be totally occluded. */
> pixman_region32_init(&clipped_view);
> pixman_region32_intersect(&clipped_view,
> @@ -3059,17 +3156,13 @@ drm_output_propose_state(struct weston_output
> *output_base,
> * cursors_are_broken flag. */
> if (!force_renderer && !b->cursors_are_broken)
> ps = drm_output_prepare_cursor_view(state, ev);
> -
> if (!planes_ok || !drm_view_is_opaque(ev))
> force_renderer = true;
>
> - if (!drm_view_is_opaque(ev))
> - force_renderer = true;
> -
> if (!force_renderer && !ps)
> - ps = drm_output_prepare_scanout_view(state, ev);
> + ps = drm_output_prepare_scanout_view(state, ev, mode);
> if (!force_renderer && !ps)
> - ps = drm_output_prepare_overlay_view(state, ev);
> + ps = drm_output_prepare_overlay_view(state, ev, mode);
>
> if (ps) {
> pixman_region32_union(&occluded_region,
> @@ -3077,8 +3170,6 @@ drm_output_propose_state(struct weston_output
> *output_base,
> &clipped_view);
> pixman_region32_fini(&clipped_view);
> continue;
> - }
> -
> if (!renderer_ok) {
> pixman_region32_fini(&clipped_view);
> goto err;
> @@ -3092,6 +3183,11 @@ drm_output_propose_state(struct weston_output
> *output_base,
> pixman_region32_fini(&renderer_region);
> pixman_region32_fini(&occluded_region);
>
> + /* Check to see if this state will actually work. */
> + ret = drm_pending_state_test(state->pending_state);
> + if (ret != 0)
> + goto err;
> +
> return state;
>
> err:
> @@ -3112,8 +3208,14 @@ drm_assign_planes(struct weston_output *output_base,
> void *repaint_data)
> struct weston_view *ev;
> struct weston_plane *primary = &output_base->compositor->primary_plane;
>
> +
> state = drm_output_propose_state(output_base, pending_state,
> - DRM_OUTPUT_PROPOSE_STATE_MIXED);
> + DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
> + if (!state)
> + state = drm_output_propose_state(output_base, pending_state,
> +
> DRM_OUTPUT_PROPOSE_STATE_MIXED);
> +
> + assert(state);
>
> wl_list_for_each(ev, &output_base->compositor->view_list, link) {
> struct drm_plane *target_plane = NULL;
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
