[PATCH 2/2] compositor: finish frame if redraw fails

Kristian Høgsberg hoegsberg at gmail.com
Tue Oct 22 21:45:23 CEST 2013


On Tue, Oct 22, 2013 at 05:11:26PM +0200, David Herrmann wrote:
> If we are about to finish a frame, but a redraw is pending and we let the
> compositor redraw, we need to check for errors. If the redraw fails and
> the backend cannot schedule a page-flip, we need to finish the frame,
> anyway.
> 
> All backends except DRM use a timer to schedule frames. Hence, they cannot
> fail. But for DRM, we need to be able to handle drmModePageFlip() failures
> in case access got revoked.
> 
> This fixes a bug where logind+drm caused keyboard input to be missed as we
> didn't reenable it after a failed page-flip during deactivation.

Yes, that's better, applied both of these patches.  Aside from being
unable to turn off sprites and hw cursor, the async deactivate also
means that we can lose drm master at any time, in particular between
starting repaint and pageflip.  So we need to handle this better and I
think these two patches covers it.

There's one last thing that doesn't quite work yet - when I switch to
weston, it doesn't immediately (re)set the current framebuffers.  I
have to trigger a repaint by moving the mouse or such.  I'm not sure
why that doesn't work, we still have the call to
drm_compositor_set_modes() and that should run with drm master set and
restore the last mode and buffer.

Kristian

> ---
>  src/compositor-drm.c      | 20 ++++++++++++++------
>  src/compositor-fbdev.c    |  4 +++-
>  src/compositor-headless.c |  4 ++--
>  src/compositor-rdp.c      |  3 ++-
>  src/compositor-rpi.c      |  3 ++-
>  src/compositor-wayland.c  |  4 ++--
>  src/compositor-x11.c      |  6 ++++--
>  src/compositor.c          | 14 +++++++++-----
>  src/compositor.h          |  2 +-
>  9 files changed, 39 insertions(+), 21 deletions(-)
> 
> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index 8a70674..9922708 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -576,7 +576,7 @@ drm_output_set_gamma(struct weston_output *output_base,
>  		weston_log("set gamma failed: %m\n");
>  }
>  
> -static void
> +static int
>  drm_output_repaint(struct weston_output *output_base,
>  		   pixman_region32_t *damage)
>  {
> @@ -588,12 +588,12 @@ drm_output_repaint(struct weston_output *output_base,
>  	int ret = 0;
>  
>  	if (output->destroy_pending)
> -		return;
> +		return -1;
>  
>  	if (!output->next)
>  		drm_output_render(output, damage);
>  	if (!output->next)
> -		return;
> +		return -1;
>  
>  	mode = container_of(output->base.current_mode, struct drm_mode, base);
>  	if (!output->current) {
> @@ -603,7 +603,7 @@ drm_output_repaint(struct weston_output *output_base,
>  				     &mode->mode_info);
>  		if (ret) {
>  			weston_log("set mode failed: %m\n");
> -			return;
> +			goto err_pageflip;
>  		}
>  		output_base->set_dpms(output_base, WESTON_DPMS_ON);
>  	}
> @@ -612,7 +612,7 @@ drm_output_repaint(struct weston_output *output_base,
>  			    output->next->fb_id,
>  			    DRM_MODE_PAGE_FLIP_EVENT, output) < 0) {
>  		weston_log("queueing pageflip failed: %m\n");
> -		return;
> +		goto err_pageflip;
>  	}
>  
>  	output->page_flip_pending = 1;
> @@ -664,7 +664,15 @@ drm_output_repaint(struct weston_output *output_base,
>  		output->vblank_pending = 1;
>  	}
>  
> -	return;
> +	return 0;
> +
> +err_pageflip:
> +	if (output->next) {
> +		drm_output_release_fb(output, output->next);
> +		output->next = NULL;
> +	}
> +
> +	return -1;
>  }
>  
>  static void
> diff --git a/src/compositor-fbdev.c b/src/compositor-fbdev.c
> index 80e708d..29de1b8 100644
> --- a/src/compositor-fbdev.c
> +++ b/src/compositor-fbdev.c
> @@ -196,7 +196,7 @@ fbdev_output_repaint_pixman(struct weston_output *base, pixman_region32_t *damag
>  	                             1000000 / output->mode.refresh);
>  }
>  
> -static void
> +static int
>  fbdev_output_repaint(struct weston_output *base, pixman_region32_t *damage)
>  {
>  	struct fbdev_output *output = to_fbdev_output(base);
> @@ -214,6 +214,8 @@ fbdev_output_repaint(struct weston_output *base, pixman_region32_t *damage)
>  		wl_event_source_timer_update(output->finish_frame_timer,
>  	                             1000000 / output->mode.refresh);
>  	}
> +
> +	return 0;
>  }
>  
>  static int
> diff --git a/src/compositor-headless.c b/src/compositor-headless.c
> index 2a0b0c3..9d9f6dd 100644
> --- a/src/compositor-headless.c
> +++ b/src/compositor-headless.c
> @@ -60,7 +60,7 @@ finish_frame_handler(void *data)
>  	return 1;
>  }
>  
> -static void
> +static int
>  headless_output_repaint(struct weston_output *output_base,
>  		       pixman_region32_t *damage)
>  {
> @@ -74,7 +74,7 @@ headless_output_repaint(struct weston_output *output_base,
>  
>  	wl_event_source_timer_update(output->finish_frame_timer, 16);
>  
> -	return;
> +	return 0;
>  }
>  
>  static void
> diff --git a/src/compositor-rdp.c b/src/compositor-rdp.c
> index 69f1d04..8a302f8 100644
> --- a/src/compositor-rdp.c
> +++ b/src/compositor-rdp.c
> @@ -302,7 +302,7 @@ rdp_output_start_repaint_loop(struct weston_output *output)
>  	weston_output_finish_frame(output, msec);
>  }
>  
> -static void
> +static int
>  rdp_output_repaint(struct weston_output *output_base, pixman_region32_t *damage)
>  {
>  	struct rdp_output *output = container_of(output_base, struct rdp_output, base);
> @@ -324,6 +324,7 @@ rdp_output_repaint(struct weston_output *output_base, pixman_region32_t *damage)
>  				 &ec->primary_plane.damage, damage);
>  
>  	wl_event_source_timer_update(output->finish_frame_timer, 16);
> +	return 0;
>  }
>  
>  static void
> diff --git a/src/compositor-rpi.c b/src/compositor-rpi.c
> index 07816a5..db29104 100644
> --- a/src/compositor-rpi.c
> +++ b/src/compositor-rpi.c
> @@ -223,7 +223,7 @@ rpi_output_start_repaint_loop(struct weston_output *output)
>  	weston_output_finish_frame(output, time);
>  }
>  
> -static void
> +static int
>  rpi_output_repaint(struct weston_output *base, pixman_region32_t *damage)
>  {
>  	struct rpi_output *output = to_rpi_output(base);
> @@ -247,6 +247,7 @@ rpi_output_repaint(struct weston_output *base, pixman_region32_t *damage)
>  	/* schedule callback to rpi_output_update_complete() */
>  	rpi_dispmanx_update_submit(update, output);
>  	DBG("frame update submitted\n");
> +	return 0;
>  }
>  
>  static void
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index 7b670d9..519b1bc 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -212,7 +212,7 @@ wayland_output_start_repaint_loop(struct weston_output *output_base)
>  	wl_surface_commit(output->parent.surface);
>  }
>  
> -static void
> +static int
>  wayland_output_repaint(struct weston_output *output_base,
>  		       pixman_region32_t *damage)
>  {
> @@ -227,7 +227,7 @@ wayland_output_repaint(struct weston_output *output_base,
>  
>  	pixman_region32_subtract(&ec->primary_plane.damage,
>  				 &ec->primary_plane.damage, damage);
> -
> +	return 0;
>  }
>  
>  static void
> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> index 44e92ba..65490db 100644
> --- a/src/compositor-x11.c
> +++ b/src/compositor-x11.c
> @@ -326,7 +326,7 @@ x11_output_start_repaint_loop(struct weston_output *output)
>  	weston_output_finish_frame(output, msec);
>  }
>  
> -static void
> +static int
>  x11_output_repaint_gl(struct weston_output *output_base,
>  		      pixman_region32_t *damage)
>  {
> @@ -339,6 +339,7 @@ x11_output_repaint_gl(struct weston_output *output_base,
>  				 &ec->primary_plane.damage, damage);
>  
>  	wl_event_source_timer_update(output->finish_frame_timer, 10);
> +	return 0;
>  }
>  
>  static void
> @@ -446,7 +447,7 @@ set_clip_for_output(struct weston_output *output_base, pixman_region32_t *region
>  }
>  
>  
> -static void
> +static int
>  x11_output_repaint_shm(struct weston_output *output_base,
>  		       pixman_region32_t *damage)
>  {
> @@ -477,6 +478,7 @@ x11_output_repaint_shm(struct weston_output *output_base,
>  	}
>  
>  	wl_event_source_timer_update(output->finish_frame_timer, 10);
> +	return 0;
>  }
>  
>  static int
> diff --git a/src/compositor.c b/src/compositor.c
> index 7e2a394..59a5abd 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -1365,7 +1365,7 @@ weston_compositor_build_surface_list(struct weston_compositor *compositor)
>  	}
>  }
>  
> -static void
> +static int
>  weston_output_repaint(struct weston_output *output, uint32_t msecs)
>  {
>  	struct weston_compositor *ec = output->compositor;
> @@ -1374,6 +1374,7 @@ weston_output_repaint(struct weston_output *output, uint32_t msecs)
>  	struct weston_frame_callback *cb, *cnext;
>  	struct wl_list frame_callback_list;
>  	pixman_region32_t output_damage;
> +	int r;
>  
>  	/* Rebuild the surface list and update surface transforms up front. */
>  	weston_compositor_build_surface_list(ec);
> @@ -1404,7 +1405,7 @@ weston_output_repaint(struct weston_output *output, uint32_t msecs)
>  	if (output->dirty)
>  		weston_output_update_matrix(output);
>  
> -	output->repaint(output, &output_damage);
> +	r = output->repaint(output, &output_damage);
>  
>  	pixman_region32_fini(&output_damage);
>  
> @@ -1422,6 +1423,8 @@ weston_output_repaint(struct weston_output *output, uint32_t msecs)
>  		animation->frame_counter++;
>  		animation->frame(animation, output, msecs);
>  	}
> +
> +	return r;
>  }
>  
>  static int
> @@ -1440,15 +1443,16 @@ weston_output_finish_frame(struct weston_output *output, uint32_t msecs)
>  	struct weston_compositor *compositor = output->compositor;
>  	struct wl_event_loop *loop =
>  		wl_display_get_event_loop(compositor->wl_display);
> -	int fd;
> +	int fd, r;
>  
>  	output->frame_time = msecs;
>  
>  	if (output->repaint_needed &&
>  	    compositor->state != WESTON_COMPOSITOR_SLEEPING &&
>  	    compositor->state != WESTON_COMPOSITOR_OFFSCREEN) {
> -		weston_output_repaint(output, msecs);
> -		return;
> +		r = weston_output_repaint(output, msecs);
> +		if (!r)
> +			return;
>  	}
>  
>  	output->repaint_scheduled = 0;
> diff --git a/src/compositor.h b/src/compositor.h
> index 5ed348b..891e7e4 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -219,7 +219,7 @@ struct weston_output {
>  	struct wl_list mode_list;
>  
>  	void (*start_repaint_loop)(struct weston_output *output);
> -	void (*repaint)(struct weston_output *output,
> +	int (*repaint)(struct weston_output *output,
>  			pixman_region32_t *damage);
>  	void (*destroy)(struct weston_output *output);
>  	void (*assign_planes)(struct weston_output *output);
> -- 
> 1.8.4.1
> 


More information about the wayland-devel mailing list