[PATCH 1/2] compositor-drm: avoid output_destroy happened before page_flip event

[Kristian Høgsberg]

On Fri, Oct 11, 2013 at 02:43:07PM +0800, Xiong Zhang wrote:
> Currently there is no guarentee that output remove event always happend after
> page_flip event on the same output. So if the following situation occur:
> first: unplug a output
> second: output remove event arrive, output_destrory called adn free output
> third: page_flip event arrive on the destroyed output
> the segment fault will happpen in page_flip_handler()
> 
> this patch add a variable drm_compositor->destroy_pending, if page flip event is pending
> when output remove event arrive, output_destroy will be delayed until page flip finished

This looks good, but it is a little subtle.  I'm mostly convinced that
this will work, but the concern is that we may end up with a client
that is waiting for a frame event that doesn't come.  But as I talked
about in the reply to your other mail, we need to add code that moves
surfaces back to the remaining visible outputs and that will trigger
repaints and frame events for all the surfaces that was on the
unplugged output.

Patch committed, thanks.

Kristian

> Signed-off-by: Xiong Zhang <xiong.y.zhang at intel.com>
> ---
>  src/compositor-drm.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 0ac5efa..ffdec89 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -149,6 +149,7 @@ struct drm_output {
>  
>  	int vblank_pending;
>  	int page_flip_pending;
> +	int destroy_pending;
>  
>  	struct gbm_surface *surface;
>  	struct gbm_bo *cursor_bo[2];
> @@ -577,6 +578,9 @@ drm_output_repaint(struct weston_output *output_base,
>  	struct drm_mode *mode;
>  	int ret = 0;
>  
> +	if (output->destroy_pending)
> +		return;
> +
>  	if (!output->next)
>  		drm_output_render(output, damage);
>  	if (!output->next)
> @@ -664,6 +668,9 @@ drm_output_start_repaint_loop(struct weston_output *output_base)
>  
>  	struct timespec ts;
>  
> +	if (output->destroy_pending)
> +		return;
> +
>  	if (!output->current) {
>  		/* We can't page flip if there's no mode set */
>  		uint32_t msec;
> @@ -704,6 +711,9 @@ vblank_handler(int fd, unsigned int frame, unsigned int sec, unsigned int usec,
>  }
>  
>  static void
> +drm_output_destroy(struct weston_output *output_base);
> +
> +static void
>  page_flip_handler(int fd, unsigned int frame,
>  		  unsigned int sec, unsigned int usec, void *data)
>  {
> @@ -721,7 +731,9 @@ page_flip_handler(int fd, unsigned int frame,
>  
>  	output->page_flip_pending = 0;
>  
> -	if (!output->vblank_pending) {
> +	if (output->destroy_pending)
> +		drm_output_destroy(&output->base);
> +	else if (!output->vblank_pending) {
>  		msecs = sec * 1000 + usec / 1000;
>  		weston_output_finish_frame(&output->base, msecs);
>  
> @@ -1063,6 +1075,12 @@ drm_output_destroy(struct weston_output *output_base)
>  		(struct drm_compositor *) output->base.compositor;
>  	drmModeCrtcPtr origcrtc = output->original_crtc;
>  
> +	if (output->page_flip_pending) {
> +		output->destroy_pending = 1;
> +		weston_log("destroy output while page flip pending\n");
> +		return;
> +	}
> +
>  	if (output->backlight)
>  		backlight_destroy(output->backlight);
>  
> -- 
> 1.8.3.2
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel