Re: [PATCH 07/18] drm/i915: Restore nonblocking awaits for modesetting

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

Maarten, could you give this a look?

On ke, 2016-09-14 at 07:52 +0100, Chris Wilson wrote:
> After combining the dma-buf reservation object and the GEM reservation
> object, we lost the ability to do a nonblocking wait on the i915 request
> (as we blocked upon the reservation object during prepare_fb). We can
> instead convert the reservation object into a fence upon which we can
> asynchronously wait (including a forced timeout in case the DMA fence is
> never signaled).
> 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 78 ++++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 +
>  2 files changed, 51 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5c0ea08dddb3..294df122f7c6 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14383,12 +14383,33 @@ static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>  
>  static void intel_atomic_commit_work(struct work_struct *work)
>  {
> > -	struct drm_atomic_state *state = container_of(work,
> > -						      struct drm_atomic_state,
> > -						      commit_work);
> > +	struct drm_atomic_state *state =
> > +		container_of(work, struct drm_atomic_state, commit_work);
> +
> >  	intel_atomic_commit_tail(state);
>  }
>  
> +static int __i915_sw_fence_call
> +intel_atomic_commit_ready(struct i915_sw_fence *fence,
> > +			  enum i915_sw_fence_notify notify)
> +{
> > +	struct intel_atomic_state *state =
> > +		container_of(fence, struct intel_atomic_state, commit_ready);
> +
> > +	switch (notify) {
> > +	case FENCE_COMPLETE:
> > +		if (state->base.commit_work.func)
> > +			queue_work(system_unbound_wq, &state->base.commit_work);
> > +		break;
> +
> > +	case FENCE_FREE:
> > +		drm_atomic_state_put(&state->base);
> > +		break;
> > +	}
> +
> > +	return NOTIFY_DONE;
> +}
> +
>  static void intel_atomic_track_fbs(struct drm_atomic_state *state)
>  {
> >  	struct drm_plane_state *old_plane_state;
> @@ -14434,11 +14455,14 @@ static int intel_atomic_commit(struct drm_device *dev,
> >  	if (ret)
> >  		return ret;
>  
> > -	INIT_WORK(&state->commit_work, intel_atomic_commit_work);
> > +	drm_atomic_state_get(state);
> > +	i915_sw_fence_init(&intel_state->commit_ready,
> > +			   intel_atomic_commit_ready);
>  
> >  	ret = intel_atomic_prepare_commit(dev, state);
> >  	if (ret) {
> >  		DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
> > +		i915_sw_fence_commit(&intel_state->commit_ready);
> >  		return ret;
> >  	}
>  
> @@ -14449,10 +14473,14 @@ static int intel_atomic_commit(struct drm_device *dev,
> >  	intel_atomic_track_fbs(state);
>  
> >  	drm_atomic_state_get(state);
> > -	if (nonblock)
> > -		queue_work(system_unbound_wq, &state->commit_work);
> > -	else
> > +	INIT_WORK(&state->commit_work,
> > +		  nonblock ? intel_atomic_commit_work : NULL);
> +
> > +	i915_sw_fence_commit(&intel_state->commit_ready);
> > +	if (!nonblock) {
> > +		i915_sw_fence_wait(&intel_state->commit_ready);
> >  		intel_atomic_commit_tail(state);
> > +	}
>  
> >  	return 0;
>  }
> @@ -14568,8 +14596,7 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> >  	struct drm_framebuffer *fb = new_state->fb;
> >  	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> >  	struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb);
> > -	long lret;
> > -	int ret = 0;
> > +	int ret;
>  
> >  	if (!obj && !old_obj)
> >  		return 0;
> @@ -14577,7 +14604,6 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> >  	if (old_obj) {
> >  		struct drm_crtc_state *crtc_state =
> >  			drm_atomic_get_existing_crtc_state(new_state->state, plane->state->crtc);
> > -		long timeout = 0;
>  
> >  		/* Big Hammer, we also need to ensure that any pending
> >  		 * MI_WAIT_FOR_EVENT inside a user batch buffer on the
> @@ -14590,31 +14616,25 @@ intel_prepare_plane_fb(struct drm_plane *plane,
> >  		 * This should only fail upon a hung GPU, in which case we
> >  		 * can safely continue.
> >  		 */
> > -		if (needs_modeset(crtc_state))
> > -			timeout = i915_gem_object_wait(old_obj,
> > -						       I915_WAIT_INTERRUPTIBLE |
> > -						       I915_WAIT_LOCKED,
> > -						       MAX_SCHEDULE_TIMEOUT,
> > -						       NULL);
> > -		if (timeout < 0) {
> > -			/* GPU hangs should have been swallowed by the wait */
> > -			WARN_ON(timeout == -EIO);
> > -			return timeout;
> > +		if (needs_modeset(crtc_state)) {
> > +			ret = i915_sw_fence_await_reservation(&to_intel_atomic_state(new_state->state)->commit_ready,
> > +							      old_obj->resv, NULL,
> > +							      false, 0,
> > +							      GFP_KERNEL);
> > +			if (ret < 0)
> > +				return ret;
> >  		}
> >  	}
>  
> >  	if (!obj)
> >  		return 0;
>  
> > -	/* For framebuffer backed by dmabuf, wait for fence */
> > -	lret = i915_gem_object_wait(obj,
> > -				    I915_WAIT_INTERRUPTIBLE | I915_WAIT_LOCKED,
> > -				    MAX_SCHEDULE_TIMEOUT,
> > -				    NULL);
> > -	if (lret == -ERESTARTSYS)
> > -		return lret;
> -
> > -	WARN(lret < 0, "waiting returns %li\n", lret);
> > +	ret = i915_sw_fence_await_reservation(&to_intel_atomic_state(new_state->state)->commit_ready,
> > +					      obj->resv, NULL,
> > +					      false, 10*HZ,
> > +					      GFP_KERNEL);
> > +	if (ret < 0)
> > +		return ret;
>  
> >  	if (plane->type == DRM_PLANE_TYPE_CURSOR &&
> >  	    INTEL_INFO(dev)->cursor_needs_physical) {
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 15e3050edeb9..cb99a2540863 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -357,6 +357,8 @@ struct intel_atomic_state {
>  
> >  	/* Gen9+ only */
> >  	struct skl_wm_values wm_results;
> +
> > +	struct i915_sw_fence commit_ready;
>  };
>  
>  struct intel_plane_state {
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx