[PATCH weston 66/68] compositor-drm: Test plane states before application
Fabien DESSENNE
fabien.dessenne at st.com
Thu Jan 19 10:52:43 UTC 2017
Hi Daniel
On 09/12/16 20:58, Daniel Stone wrote:
> Generate an output state in two stages. Firstly, attempt to run through
> and generate a configuration with all views in planes. If this succeeds,
> we can bypass the renderer completely.
>
> If this fails, we know we need to have the renderer output used as a
> base on the scanout plane, and can incrementally attempt to construct a
> working state, by trying the combination of our old renderer state with
> planes added one-by-one. If they pass the test commit stage, we can use
> it, so we stash that state and continue on.
>
> Signed-off-by: Daniel Stone <daniels at collabora.com>
>
> Differential Revision: https://phabricator.freedesktop.org/D1535
> ---
> libweston/compositor-drm.c | 135 +++++++++++++++++++++++++++++++++------------
> 1 file changed, 101 insertions(+), 34 deletions(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index e51a5b2..7c8b5d9 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -1669,12 +1669,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)
> @@ -1687,7 +1690,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;
> }
> @@ -1706,10 +1718,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_output_apply_state(output_state, mode);
"mode" is not the correct parameter (it is an 'enum
drm_output_propose_state_mode' and drm_output_apply_state expects an
'enum drm_output_apply_state_mode mode').
Use DRM_OUTPUT_APPLY_STATE_TEST instead of mode
> + 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;
> }
>
> @@ -1780,7 +1803,9 @@ drm_output_render(struct drm_output *output, pixman_region32_t *damage)
> * want to render. */
> scanout_state = drm_output_state_get_plane(output->state_pending,
> 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) &&
> @@ -1803,6 +1828,7 @@ drm_output_render(struct drm_output *output, pixman_region32_t *damage)
> return;
> }
>
> + drm_fb_unref(scanout_state->fb);
> scanout_state->fb = fb;
> scanout_state->output = output;
>
> @@ -2401,24 +2427,24 @@ atomic_flip_handler(int fd, unsigned int frame,
>
> 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;
> struct drm_backend *b = to_drm_backend(ec);
> struct drm_plane *p;
> - 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)
> return NULL;
>
> wl_list_for_each(p, &b->plane_list, link) {
> + struct drm_plane_state *state = NULL;
> +
> if (p->type != WDRM_PLANE_TYPE_OVERLAY)
> continue;
>
> @@ -2444,28 +2470,30 @@ drm_output_prepare_overlay_view(struct drm_output_state *output_state,
> continue;
> }
>
> - break;
> - }
> + state->fb = fb;
> + state->ev = ev;
> + state->output = output;
>
> - /* No sprites available */
> - if (!state) {
> - drm_fb_unref(fb);
> - return NULL;
> - }
> + 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;
> + }
>
> - 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)
> + return state;
>
> - 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_output_apply_state(output_state, mode);
Same remark here
> + if (ret == 0)
> + return state;
>
> - return state;
> + drm_plane_state_put_back(state);
> + }
>
> -err:
> - drm_plane_state_put_back(state);
> + drm_fb_unref(fb);
> return NULL;
> }
>
> @@ -2665,12 +2693,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);
> assert(!output->state_pending);
> state = drm_output_state_duplicate(output->state_cur,
> 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:
> @@ -2704,6 +2762,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(&surface_overlap);
> pixman_region32_subtract(&surface_overlap,
> @@ -2737,17 +2798,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)
> - 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)
> continue;
> - if (!renderer_ok)
> - goto err;
>
> pixman_region32_union(&renderer_region,
> &renderer_region,
> @@ -2756,6 +2813,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_output_apply_state(state, DRM_OUTPUT_APPLY_STATE_TEST);
> + if (ret != 0)
> + goto err;
> +
> return state;
>
> err:
> @@ -2776,7 +2838,12 @@ drm_assign_planes(struct weston_output *output_base)
> struct weston_plane *primary = &output_base->compositor->primary_plane;
>
> state = drm_output_propose_state(output_base,
> - DRM_OUTPUT_PROPOSE_STATE_MIXED);
> + DRM_OUTPUT_PROPOSE_STATE_PLANES_ONLY);
> + if (!state)
> + state = drm_output_propose_state(output_base,
> + DRM_OUTPUT_PROPOSE_STATE_MIXED);
> +
> + assert(state);
>
> wl_list_for_each(ev, &output_base->compositor->view_list, link) {
> struct drm_plane *target_plane = NULL;
Fabien
BR
More information about the wayland-devel
mailing list