Re: [PATCH v14 32/41] compositor-drm: Split drm_assign_planes in two

[Pekka Paalanen]

On Wed, 20 Dec 2017 12:26:49 +
Daniel Stone  wrote:

> Move drm_assign_planes into two functions: one which proposes a plane
> configuration, and another which applies that state to the Weston
> internal structures. This will be used to try multiple configurations
> and see which is supported.
> 
> Signed-off-by: Daniel Stone 
> ---
>  libweston/compositor-drm.c | 109 
> ++---
>  1 file changed, 74 insertions(+), 35 deletions(-)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 3ff06c01c..049c80932 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -357,6 +357,8 @@ struct drm_plane_state {
>  
>   struct drm_fb *fb;
>  
> + struct weston_view *ev; /**< maintained for drm_assign_planes only */
> +
>   int32_t src_x, src_y;
>   uint32_t src_w, src_h;
>   int32_t dest_x, dest_y;

> +static void
> +drm_assign_planes(struct weston_output *output_base, void *repaint_data)
> +{
> + struct drm_backend *b = to_drm_backend(output_base->compositor);
> + struct drm_pending_state *pending_state = repaint_data;
> + struct drm_output *output = to_drm_output(output_base);
> + struct drm_output_state *state;
> + struct drm_plane_state *plane_state;
> + struct weston_view *ev;
> + struct weston_plane *primary = &output_base->compositor->primary_plane;
> +
> + state = drm_output_propose_state(output_base, pending_state);
>  
> - if (next_plane == primary ||
> - (output->cursor_plane &&
> -  next_plane == &output->cursor_plane->base)) {
> - /* cursor plane involves a copy */
> + wl_list_for_each(ev, &output_base->compositor->view_list, link) {
> + struct drm_plane *target_plane = NULL;
> +
> + /* Test whether this buffer can ever go into a plane:
> +  * non-shm, or small enough to be a cursor.
> +  *
> +  * Also, keep a reference when using the pixman renderer.
> +  * That makes it possible to do a seamless switch to the GL
> +  * renderer and since the pixman renderer keeps a reference
> +  * to the buffer anyway, there is no side effects.
> +  */
> + if (b->use_pixman ||
> + (ev->surface->buffer_ref.buffer &&
> + 
> (!wl_shm_buffer_get(ev->surface->buffer_ref.buffer->resource) ||
> +  (ev->surface->width <= b->cursor_width &&
> +   ev->surface->height <= b->cursor_height
> + ev->surface->keep_buffer = true;
> + else
> + ev->surface->keep_buffer = false;
> +
> + /* This is a bit unpleasant, but lacking a temporary place to
> +  * hang a plane off the view, we have to do a nested walk.
> +  * Our first-order iteration has to be planes rather than
> +  * views, because otherwise we won't reset views which were
> +  * previously on planes to being on the primary plane. */
> + wl_list_for_each(plane_state, &state->plane_list, link) {

I believe the plane list will be short enough for the foreseeable
future to not warrant a more complicated solution. One option would be
to hang data off the view destroy signal, but then looking that data up
is probably in the same order of magnitude than this trivial loop.

> + if (plane_state->ev == ev) {
> + plane_state->ev = NULL;
> + target_plane = plane_state->plane;
> + break;
> + }
> + }
> +
> + if (target_plane)
> + weston_view_move_to_plane(ev, &target_plane->base);
> + else
> + weston_view_move_to_plane(ev, primary);
> +
> + if (!target_plane ||
> + target_plane->type == WDRM_PLANE_TYPE_CURSOR) {
> + /* cursor plane & renderer involve a copy */
>   ev->psf_flags = 0;
>   } else {

Reviewed-by: Pekka Paalanen 


Thanks,
pq


pgpAhVSvyHvqX.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH v14 32/41] compositor-drm: Split drm_assign_planes in two

[Daniel Stone]

Move drm_assign_planes into two functions: one which proposes a plane
configuration, and another which applies that state to the Weston
internal structures. This will be used to try multiple configurations
and see which is supported.

Signed-off-by: Daniel Stone 
---
 libweston/compositor-drm.c | 109 ++---
 1 file changed, 74 insertions(+), 35 deletions(-)

diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
index 3ff06c01c..049c80932 100644
--- a/libweston/compositor-drm.c
+++ b/libweston/compositor-drm.c
@@ -357,6 +357,8 @@ struct drm_plane_state {
 
struct drm_fb *fb;
 
+   struct weston_view *ev; /**< maintained for drm_assign_planes only */
+
int32_t src_x, src_y;
uint32_t src_w, src_h;
int32_t dest_x, dest_y;
@@ -1798,6 +1800,7 @@ drm_output_prepare_scanout_view(struct drm_output_state 
*output_state,
}
 
state->fb = fb;
+   state->ev = ev;
state->output = output;
drm_plane_state_coords_for_view(state, ev);
 
@@ -2847,6 +2850,7 @@ drm_output_prepare_overlay_view(struct drm_output_state 
*output_state,
}
 
state->fb = fb;
+   state->ev = ev;
state->output = output;
 
drm_plane_state_coords_for_view(state, ev);
@@ -2983,6 +2987,7 @@ drm_output_prepare_cursor_view(struct drm_output_state 
*output_state,
}
 
output->cursor_view = ev;
+   plane_state->ev = ev;
 
plane_state->fb =
drm_fb_ref(output->gbm_cursor_fb[output->current_cursor]);
@@ -3050,17 +3055,15 @@ err:
drmModeSetCursor(b->drm.fd, output->crtc_id, 0, 0, 0);
 }
 
-static void
-drm_assign_planes(struct weston_output *output_base, void *repaint_data)
+static struct drm_output_state *
+drm_output_propose_state(struct weston_output *output_base,
+struct drm_pending_state *pending_state)
 {
-   struct drm_backend *b = to_drm_backend(output_base->compositor);
-   struct drm_pending_state *pending_state = repaint_data;
struct drm_output *output = to_drm_output(output_base);
struct drm_output_state *state;
-   struct drm_plane_state *plane_state;
struct weston_view *ev;
pixman_region32_t surface_overlap, renderer_region;
-   struct weston_plane *primary, *next_plane;
+   struct weston_plane *primary = &output_base->compositor->primary_plane;
 
assert(!output->state_last);
state = drm_output_state_duplicate(output->state_cur,
@@ -3081,35 +3084,21 @@ drm_assign_planes(struct weston_output *output_base, 
void *repaint_data)
 * as we do for flipping full screen surfaces.
 */
pixman_region32_init(&renderer_region);
-   primary = &output_base->compositor->primary_plane;
 
wl_list_for_each(ev, &output_base->compositor->view_list, link) {
-   struct weston_surface *es = ev->surface;
-
-   /* Test whether this buffer can ever go into a plane:
-* non-shm, or small enough to be a cursor.
-*
-* Also, keep a reference when using the pixman renderer.
-* That makes it possible to do a seamless switch to the GL
-* renderer and since the pixman renderer keeps a reference
-* to the buffer anyway, there is no side effects.
-*/
-   if (b->use_pixman ||
-   (es->buffer_ref.buffer &&
-   (!wl_shm_buffer_get(es->buffer_ref.buffer->resource) ||
-(ev->surface->width <= b->cursor_width &&
- ev->surface->height <= b->cursor_height
-   es->keep_buffer = true;
-   else
-   es->keep_buffer = false;
+   struct weston_plane *next_plane = NULL;
 
+   /* Since we process views from top to bottom, we know that if
+* the view intersects the calculated renderer region, it must
+* be part of, or occluded by, it, and cannot go on a plane. */
pixman_region32_init(&surface_overlap);
pixman_region32_intersect(&surface_overlap, &renderer_region,
  &ev->transform.boundingbox);
 
-   next_plane = NULL;
if (pixman_region32_not_empty(&surface_overlap))
next_plane = primary;
+   pixman_region32_fini(&surface_overlap);
+
if (next_plane == NULL)
next_plane = drm_output_prepare_cursor_view(state, ev);
 
@@ -3122,17 +3111,70 @@ drm_assign_planes(struct weston_output *output_base, 
void *repaint_data)
if (next_plane == NULL)
next_plane = primary;
 
-   weston_view_move_to_plane(ev, next_plane);
-
if (next_plane == primary)
pixman_region32_union(&renderer_region,