[PATCH weston v10 17/61] compositor-drm: Refactor sprite create/destroy into helpers

[Pekka Paalanen]

On Tue,  4 Apr 2017 17:54:35 +0100
Daniel Stone <daniels at collabora.com> wrote:

> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> 
> This moves the single sprite creation code from create_sprites() into a
> new function. The readability clean-up is small, but my intention is to
> write an alternate version of create_sprites(), and sharing the single
> sprite creation code is useful.
> 
> [daniels: Genericised from drm_sprite to drm_plane, moving some of the
>           logic back into create_sprites(), also symmetrical
>           drm_plane_destroy.]
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> 
> Differential Revision: https://phabricator.freedesktop.org/D1409
> 
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> ---
>  libweston/compositor-drm.c | 188 ++++++++++++++++++++++++++++-----------------
>  1 file changed, 117 insertions(+), 71 deletions(-)
> 
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index 8a8dd1c2..659f1b59 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -1805,6 +1805,123 @@ init_pixman(struct drm_backend *b)
>  }
>  
>  /**
> + * Create a drm_plane for a hardware plane
> + *
> + * Creates one drm_plane structure for a hardware plane, and initialises its
> + * properties and formats.
> + *
> + * This function does not add the plane to the list of usable planes in Weston
> + * itself; the caller is responsible for this.
> + *
> + * Call drm_plane_destroy to clean up the plane.
> + *
> + * @param ec Compositor to create plane for

That should be b the backend.

> + * @param kplane DRM plane to create
> + */
> +static struct drm_plane *
> +drm_plane_create(struct drm_backend *b, const drmModePlane *kplane)
> +{

> +/**
> + * Destroy one DRM plane
> + *
> + * Destroy a DRM plane, removing it from screen and releasing its retained
> + * buffers in the process. The counterpart to drm_plane_create.
> + *
> + * @param plane Plane to deallocate (will be freed)
> + */
> +static void
> +drm_plane_destroy(struct drm_plane *plane)
> +{
> +	drmModeSetPlane(plane->backend->drm.fd, plane->plane_id, 0, 0, 0,

Previously the code was doing a nasty hack to get a valid CRTC id to
use instead of zero here. Was that just a brainfart, or has something
changed in the kernel?

> +			0, 0, 0, 0, 0, 0, 0, 0);
> +	assert(!plane->fb_last);
> +	assert(!plane->fb_pending);
> +	drm_fb_unref(plane->fb_current);
> +	weston_plane_release(&plane->base);
> +	wl_list_remove(&plane->link);
> +	free(plane);
> +}
> +
> +/**
> + * Initialise sprites (overlay planes)
> + *
> + * Walk the list of provided DRM planes, and add overlay planes.
> + *
> + * Call destroy_sprites to free these planes.
> + *
> + * @param ec Compositor to create sprites for.

That should be b the backend.

> + */
> +static void
> +create_sprites(struct drm_backend *b)
> +{

> +/**
> + * Clean up sprites (overlay planes)
> + *
> + * The counterpart to create_sprites.
> + *
> + * @param compositor Compositor to deallocate sprites for.

That should be backend the backend.

> + */
> +static void
> +destroy_sprites(struct drm_backend *backend)
> +{

> -static void
> -destroy_sprites(struct drm_backend *backend)
> -{
> -	struct drm_plane *plane, *next;
> -	struct drm_output *output;
> -
> -	output = container_of(backend->compositor->output_list.next,
> -			      struct drm_output, base.link);
> -
> -	wl_list_for_each_safe(plane, next, &backend->sprite_list, link) {
> -		drmModeSetPlane(backend->drm.fd,
> -				plane->plane_id,
> -				output->crtc_id, 0, 0,
> -				0, 0, 0, 0, 0, 0, 0, 0);
> -		assert(!plane->fb_last);
> -		assert(!plane->fb_pending);
> -		drm_fb_unref(plane->fb_current);
> -		weston_plane_release(&plane->base);
> -		free(plane);

There was no wl_list_remove()... *facepalm*

Nice to fix that, even though it's yet another change that is not
strictly refactoring.

> -	}
> -}
> -
>  static int
>  create_outputs(struct drm_backend *b, struct udev_device *drm_device)
>  {


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170407/3c726507/attachment.sig>