[PATCH v2 weston 08/16] compositor-drm: Refactor sprite create/destroy into helpers

[Derek Foreman]

On 22/06/15 11:25 AM, Daniel Stone 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.]
> 
> v2: Pass NULL CRTC when disabling planes; don't call weston_plane_init()
>     twice on sprites (though harmless).
> 
> Signed-off-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> Signed-off-by: Daniel Stone <daniels at collabora.com>
> ---
>  src/compositor-drm.c | 123 ++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 88 insertions(+), 35 deletions(-)
> 
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 8c14b4f..dbf1dbe 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -2350,12 +2350,86 @@ err_free:
>  	return -1;
>  }
>  
> +/**
> + * 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
> + * @param kplane DRM plane to create

I guess we want an @return doxy tag too?

Reviewed-By: Derek Foreman <derekf at osg.samsung.com>

> + */
> +static struct drm_plane *
> +drm_plane_create(struct drm_compositor *ec, const drmModePlane *kplane)
> +{
> +	struct drm_plane *plane;
> +
> +	plane = zalloc(sizeof(*plane) + ((sizeof(uint32_t)) *
> +					  kplane->count_formats));
> +	if (!plane) {
> +		weston_log("%s: out of memory\n", __func__);
> +		return NULL;
> +	}
> +
> +	plane->possible_crtcs = kplane->possible_crtcs;
> +	plane->plane_id = kplane->plane_id;
> +	plane->current = NULL;
> +	plane->next = NULL;
> +	plane->output = NULL;
> +	plane->compositor = ec;
> +	plane->count_formats = kplane->count_formats;
> +	memcpy(plane->formats, kplane->formats,
> +	       kplane->count_formats * sizeof(kplane->formats[0]));
> +
> +	weston_plane_init(&plane->base, &ec->base, 0, 0);
> +	wl_list_insert(&ec->sprite_list, &plane->link);
> +
> +	return plane;
> +}
> +
> +/**
> + * 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->compositor->drm.fd,
> +			plane->plane_id, 0, 0, 0,
> +			0, 0, 0, 0, 0, 0, 0, 0);
> +	if (plane->current)
> +		drm_output_release_fb(plane->output, plane->current);
> +	if (plane->next)
> +		drm_output_release_fb(plane->output, plane->next);
> +	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.
> + */
>  static void
>  create_sprites(struct drm_compositor *ec)
>  {
> -	struct drm_plane *plane;
>  	drmModePlaneRes *kplane_res;
>  	drmModePlane *kplane;
> +	struct drm_plane *drm_plane;
>  	uint32_t i;
>  
>  	kplane_res = drmModeGetPlaneResources(ec->drm.fd);
> @@ -2370,53 +2444,32 @@ create_sprites(struct drm_compositor *ec)
>  		if (!kplane)
>  			continue;
>  
> -		plane = zalloc(sizeof(*plane) + ((sizeof(uint32_t)) *
> -						  kplane->count_formats));
> -		if (!plane) {
> -			weston_log("%s: out of memory\n",
> -				__func__);
> -			drmModeFreePlane(kplane);
> +		drm_plane = drm_plane_create(ec, kplane);
> +		drmModeFreePlane(kplane);
> +		if (!drm_plane)
>  			continue;
> -		}
>  
> -		plane->possible_crtcs = kplane->possible_crtcs;
> -		plane->plane_id = kplane->plane_id;
> -		plane->current = NULL;
> -		plane->next = NULL;
> -		plane->compositor = ec;
> -		plane->count_formats = kplane->count_formats;
> -		memcpy(plane->formats, kplane->formats,
> -		       kplane->count_formats * sizeof(kplane->formats[0]));
> -		drmModeFreePlane(kplane);
> -		weston_plane_init(&plane->base, &ec->base, 0, 0);
> -		weston_compositor_stack_plane(&ec->base, &plane->base,
> +		weston_compositor_stack_plane(&ec->base, &drm_plane->base,
>  					      &ec->base.primary_plane);
> -
> -		wl_list_insert(&ec->sprite_list, &plane->link);
>  	}
>  
>  	drmModeFreePlaneResources(kplane_res);
>  }
>  
> +/**
> + * Clean up sprites (overlay planes)
> + *
> + * The counterpart to create_sprites.
> + *
> + * @param compositor Compositor to deallocate sprites for.
> + */
>  static void
>  destroy_sprites(struct drm_compositor *compositor)
>  {
>  	struct drm_plane *plane, *next;
> -	struct drm_output *output;
>  
> -	output = container_of(compositor->base.output_list.next,
> -			      struct drm_output, base.link);
> -
> -	wl_list_for_each_safe(plane, next, &compositor->sprite_list, link) {
> -		drmModeSetPlane(compositor->drm.fd,
> -				plane->plane_id,
> -				output->crtc_id, 0, 0,
> -				0, 0, 0, 0, 0, 0, 0, 0);
> -		drm_output_release_fb(output, plane->current);
> -		drm_output_release_fb(output, plane->next);
> -		weston_plane_release(&plane->base);
> -		free(plane);
> -	}
> +	wl_list_for_each_safe(plane, next, &compositor->sprite_list, link)
> +		drm_plane_destroy(plane);
>  }
>  
>  static int
>