[Intel-gfx] [PATCH v2 21/21] drm/i915: Allow async update of pageflips.

Patrik Jakobsson patrik.jakobsson at linux.intel.com
Thu May 19 12:10:44 UTC 2016


On Tue, May 17, 2016 at 03:08:04PM +0200, Maarten Lankhorst wrote:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>

We could have had a short note on what the patch does, though reading it is
quite straight forward. Either way is fine by me.

Reviewed-by: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 350 +++++++++--------------------------
>  1 file changed, 84 insertions(+), 266 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 330a6397f07c..79759cdfada3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -108,8 +108,6 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
>  			    const struct intel_crtc_state *pipe_config);
>  static void chv_prepare_pll(struct intel_crtc *crtc,
>  			    const struct intel_crtc_state *pipe_config);
> -static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
> -static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
>  static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
>  	struct intel_crtc_state *crtc_state);
>  static void skylake_pfit_enable(struct intel_crtc *crtc);
> @@ -10981,7 +10979,7 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_i915_gem_request *req;
> -	int i;
> +	int i, ret;
>  
>  	if (!needs_modeset(&crtc_state->base) && crtc_state->update_pipe) {
>  		work->put_power_domains =
> @@ -11003,7 +11001,14 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
>  					    &dev_priv->rps.mmioflips));
>  	}
>  
> -	intel_frontbuffer_flip_prepare(dev, crtc_state->fb_bits);
> +	ret = drm_crtc_vblank_get(crtc);
> +	I915_STATE_WARN(ret < 0, "enabling vblank failed with %i\n", ret);
> +
> +	if (work->num_planes &&
> +	    work->old_plane_state[0]->base.plane == crtc->primary)
> +		intel_fbc_enable(intel_crtc, work->new_crtc_state, work->new_plane_state[0]);
> +
> +	intel_frontbuffer_flip_prepare(dev, work->fb_bits);
>  
>  	intel_pipe_update_start(intel_crtc);
>  	if (!needs_modeset(&crtc_state->base)) {
> @@ -11022,206 +11027,15 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
>  		struct intel_plane_state *new_plane_state = work->new_plane_state[i];
>  		struct intel_plane *plane = to_intel_plane(new_plane_state->base.plane);
>  
> -		plane->update_plane(&plane->base, crtc_state, new_plane_state);
> +		if (new_plane_state->visible)
> +			plane->update_plane(&plane->base, crtc_state, new_plane_state);
> +		else
> +			plane->disable_plane(&plane->base, crtc);
>  	}
>  
>  	intel_pipe_update_end(intel_crtc, work);
>  }
>  
> -static struct fence *intel_get_excl_fence(struct drm_i915_gem_object *obj)
> -{
> -	struct reservation_object *resv;
> -
> -
> -	if (!obj->base.dma_buf)
> -		return NULL;
> -
> -	resv = obj->base.dma_buf->resv;
> -
> -	/* For framebuffer backed by dmabuf, wait for fence */
> -	while (1) {
> -		struct fence *fence_excl, *ret = NULL;
> -
> -		rcu_read_lock();
> -
> -		fence_excl = rcu_dereference(resv->fence_excl);
> -		if (fence_excl)
> -			ret = fence_get_rcu(fence_excl);
> -
> -		rcu_read_unlock();
> -
> -		if (ret == fence_excl)
> -			return ret;
> -	}
> -}
> -
> -static int intel_crtc_page_flip(struct drm_crtc *crtc,
> -				struct drm_framebuffer *fb,
> -				struct drm_pending_vblank_event *event,
> -				uint32_t page_flip_flags)
> -{
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct drm_plane_state *old_state, *new_state = NULL;
> -	struct drm_crtc_state *new_crtc_state = NULL;
> -	struct drm_framebuffer *old_fb = crtc->primary->state->fb;
> -	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct drm_plane *primary = crtc->primary;
> -	struct intel_flip_work *work;
> -	int ret;
> -
> -	old_state = crtc->primary->state;
> -
> -	if (!crtc->state->active)
> -		return -EINVAL;
> -
> -	/*
> -	 * drm_mode_page_flip_ioctl() should already catch this, but double
> -	 * check to be safe.  In the future we may enable pageflipping from
> -	 * a disabled primary plane.
> -	 */
> -	if (WARN_ON(intel_fb_obj(old_fb) == NULL))
> -		return -EBUSY;
> -
> -	/* Can't change pixel format via MI display flips. */
> -	if (fb->pixel_format != old_fb->pixel_format)
> -		return -EINVAL;
> -
> -	/*
> -	 * TILEOFF/LINOFF registers can't be changed via MI display flips.
> -	 * Note that pitch changes could also affect these register.
> -	 */
> -	if (INTEL_INFO(dev)->gen > 3 &&
> -	    (fb->offsets[0] != old_fb->offsets[0] ||
> -	     fb->pitches[0] != old_fb->pitches[0]))
> -		return -EINVAL;
> -
> -	work = kzalloc(sizeof(*work), GFP_KERNEL);
> -	new_crtc_state = intel_crtc_duplicate_state(crtc);
> -	new_state = intel_plane_duplicate_state(primary);
> -
> -	if (!work || !new_crtc_state || !new_state) {
> -		ret = -ENOMEM;
> -		goto cleanup;
> -	}
> -
> -	drm_framebuffer_unreference(new_state->fb);
> -	drm_framebuffer_reference(fb);
> -	new_state->fb = fb;
> -
> -	work->event = event;
> -	INIT_WORK(&work->unpin_work, intel_unpin_work_fn);
> -	INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
> -
> -	work->new_crtc_state = to_intel_crtc_state(new_crtc_state);
> -	work->old_crtc_state = intel_crtc->config;
> -
> -	work->fb_bits = to_intel_plane(primary)->frontbuffer_bit;
> -	work->new_crtc_state->fb_bits = work->fb_bits;
> -
> -	work->can_async_unpin = true;
> -	work->num_planes = 1;
> -	work->old_plane_state[0] = to_intel_plane_state(old_state);
> -	work->new_plane_state[0] = to_intel_plane_state(new_state);
> -
> -	/* Step 1: vblank waiting and workqueue throttling,
> -	 * similar to intel_atomic_prepare_commit
> -	 */
> -	ret = drm_crtc_vblank_get(crtc);
> -	if (ret)
> -		goto cleanup;
> -
> -	/* We borrow the event spin lock for protecting flip_work */
> -	spin_lock_irq(&dev->event_lock);
> -	if (!list_empty(&intel_crtc->flip_work)) {
> -		struct intel_flip_work *old_work;
> -
> -		old_work = list_last_entry(&intel_crtc->flip_work,
> -					   struct intel_flip_work, head);
> -
> -		/* Before declaring the flip queue wedged, check if
> -		 * the hardware completed the operation behind our backs.
> -		 */
> -		if (pageflip_finished(intel_crtc, old_work)) {
> -			DRM_DEBUG_DRIVER("flip queue: previous flip completed, continuing\n");
> -			page_flip_completed(intel_crtc, old_work);
> -		} else {
> -			DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
> -			spin_unlock_irq(&dev->event_lock);
> -
> -			ret = -EBUSY;
> -			goto cleanup_vblank;
> -		}
> -	}
> -	list_add_tail(&work->head, &intel_crtc->flip_work);
> -	spin_unlock_irq(&dev->event_lock);
> -
> -	if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
> -		flush_workqueue(dev_priv->wq);
> -
> -	/* step 2, similar to intel_prepare_plane_fb */
> -	ret = mutex_lock_interruptible(&dev->struct_mutex);
> -	if (ret)
> -		goto cleanup_work;
> -
> -	ret = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
> -	if (ret)
> -		goto cleanup_unlock;
> -
> -	i915_gem_track_fb(intel_fb_obj(old_fb), obj,
> -			  to_intel_plane(primary)->frontbuffer_bit);
> -
> -	/* point of no return, swap state */
> -	primary->state = new_state;
> -	crtc->state = new_crtc_state;
> -	intel_crtc->config = to_intel_crtc_state(new_crtc_state);
> -	primary->fb = fb;
> -
> -	/* scheduling flip work */
> -	atomic_inc(&intel_crtc->unpin_work_count);
> -
> -	if (obj->last_write_req &&
> -	    !i915_gem_request_completed(obj->last_write_req, true))
> -		i915_gem_request_assign(&work->old_plane_state[0]->wait_req,
> -					obj->last_write_req);
> -
> -	if (obj->base.dma_buf)
> -		work->old_plane_state[0]->base.fence = intel_get_excl_fence(obj);
> -
> -	intel_fbc_pre_update(intel_crtc,
> -			     to_intel_crtc_state(new_crtc_state),
> -			     to_intel_plane_state(new_state));
> -
> -	schedule_work(&work->mmio_work);
> -
> -	mutex_unlock(&dev->struct_mutex);
> -
> -	trace_i915_flip_request(intel_crtc->plane, obj);
> -
> -	return 0;
> -
> -cleanup_unlock:
> -	mutex_unlock(&dev->struct_mutex);
> -cleanup_work:
> -	spin_lock_irq(&dev->event_lock);
> -	list_del(&work->head);
> -	spin_unlock_irq(&dev->event_lock);
> -
> -cleanup_vblank:
> -	drm_crtc_vblank_put(crtc);
> -cleanup:
> -	if (new_state)
> -		intel_plane_destroy_state(primary, new_state);
> -
> -	if (new_crtc_state)
> -		intel_crtc_destroy_state(crtc, new_crtc_state);
> -
> -	intel_free_flip_work(work);
> -	return ret;
> -}
> -
> -
>  /**
>   * intel_wm_need_update - Check whether watermarks need updating
>   * @plane: drm plane
> @@ -11498,8 +11312,6 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
>  
>  static const struct drm_crtc_helper_funcs intel_helper_funcs = {
>  	.mode_set_base_atomic = intel_pipe_set_base_atomic,
> -	.atomic_begin = intel_begin_crtc_commit,
> -	.atomic_flush = intel_finish_crtc_commit,
>  	.atomic_check = intel_crtc_atomic_check,
>  };
>  
> @@ -12921,11 +12733,6 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
>  	struct drm_crtc *crtc;
>  	int i, ret;
>  
> -	if (nonblock) {
> -		DRM_DEBUG_KMS("i915 does not yet support nonblocking commit\n");
> -		return -EINVAL;
> -	}
> -
>  	for_each_crtc_in_state(state, crtc, crtc_state, i) {
>  		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  		struct intel_flip_work *work;
> @@ -12967,6 +12774,11 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
>  		}
>  	}
>  
> +	if (intel_state->modeset && nonblock) {
> +		DRM_DEBUG_ATOMIC("Nonblock modesets are not yet supported!\n");
> +		return -EINVAL;
> +	}
> +
>  	ret = mutex_lock_interruptible(&dev->struct_mutex);
>  	if (ret)
>  		return ret;
> @@ -13120,13 +12932,41 @@ static void intel_schedule_unpin(struct drm_crtc *crtc,
>  	queue_work(dev_priv->wq, &work->unpin_work);
>  }
>  
> +static void intel_schedule_flip(struct drm_crtc *crtc,
> +				struct intel_atomic_state *state,
> +				struct intel_flip_work *work,
> +				bool nonblock)
> +{
> +	struct intel_crtc_state *crtc_state = work->new_crtc_state;
> +
> +	if (crtc_state->base.planes_changed ||
> +	    needs_modeset(&crtc_state->base) ||
> +	    crtc_state->update_pipe) {
> +		if (nonblock)
> +			schedule_work(&work->mmio_work);
> +		else
> +			intel_mmio_flip_work_func(&work->mmio_work);
> +	} else {
> +		int ret;
> +
> +		ret = drm_crtc_vblank_get(crtc);
> +		I915_STATE_WARN(ret < 0, "enabling vblank failed with %i\n", ret);
> +
> +		work->flip_queued_vblank = intel_crtc_get_vblank_counter(to_intel_crtc(crtc));
> +		smp_mb__before_atomic();
> +		atomic_set(&work->pending, 1);
> +	}
> +}
> +
>  static void intel_schedule_update(struct drm_crtc *crtc,
>  				  struct intel_atomic_state *state,
> -				  struct intel_flip_work *work)
> +				  struct intel_flip_work *work,
> +				  bool nonblock)
>  {
>  	struct drm_device *dev = crtc->dev;
> +	struct intel_crtc_state *pipe_config = work->new_crtc_state;
>  
> -	if (work->can_async_unpin) {
> +	if (!pipe_config->base.active && work->can_async_unpin) {
>  		INIT_LIST_HEAD(&work->head);
>  		intel_schedule_unpin(crtc, state, work);
>  		return;
> @@ -13136,7 +12976,10 @@ static void intel_schedule_update(struct drm_crtc *crtc,
>  	list_add_tail(&work->head, &to_intel_crtc(crtc)->flip_work);
>  	spin_unlock_irq(&dev->event_lock);
>  
> -	intel_schedule_unpin(crtc, state, work);
> +	if (!pipe_config->base.active)
> +		intel_schedule_unpin(crtc, state, work);
> +	else
> +		intel_schedule_flip(crtc, state, work, nonblock);
>  }
>  
>  /**
> @@ -13230,11 +13073,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>  
>  	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
>  	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
> +		struct intel_flip_work *work = intel_state->work[i];
>  		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  		bool modeset = needs_modeset(crtc->state);
> -		struct intel_crtc_state *pipe_config =
> -			to_intel_crtc_state(crtc->state);
> -		bool update_pipe = !modeset && pipe_config->update_pipe;
>  
>  		if (modeset && crtc->state->active) {
>  			update_scanline_offset(to_intel_crtc(crtc));
> @@ -13244,22 +13085,6 @@ static int intel_atomic_commit(struct drm_device *dev,
>  		if (!modeset)
>  			intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
>  
> -		if (crtc->state->active &&
> -		    drm_atomic_get_existing_plane_state(state, crtc->primary))
> -			intel_fbc_enable(intel_crtc, pipe_config, to_intel_plane_state(crtc->primary->state));
> -
> -		if (crtc->state->active &&
> -		    (crtc->state->planes_changed || update_pipe))
> -			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
> -	}
> -
> -	/* FIXME: add subpixel order */
> -
> -	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
> -		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -		struct intel_flip_work *work =
> -			intel_state->work[i];
> -
>  		if (!work) {
>  			if (!list_empty_careful(&intel_crtc->flip_work)) {
>  				spin_lock_irq(&dev->event_lock);
> @@ -13279,9 +13104,11 @@ static int intel_atomic_commit(struct drm_device *dev,
>  
>  		intel_state->work[i] = NULL;
>  		intel_prepare_work(crtc, work, state, old_crtc_state);
> -		intel_schedule_update(crtc, intel_state, work);
> +		intel_schedule_update(crtc, intel_state, work, nonblock);
>  	}
>  
> +	/* FIXME: add subpixel order */
> +
>  	drm_atomic_state_free(state);
>  
>  	/* As one of the primary mmio accessors, KMS has a high likelihood
> @@ -13345,11 +13172,38 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
>  	.set_config = drm_atomic_helper_set_config,
>  	.set_property = drm_atomic_helper_crtc_set_property,
>  	.destroy = intel_crtc_destroy,
> -	.page_flip = intel_crtc_page_flip,
> +	.page_flip = drm_atomic_helper_page_flip,
>  	.atomic_duplicate_state = intel_crtc_duplicate_state,
>  	.atomic_destroy_state = intel_crtc_destroy_state,
>  };
>  
> +static struct fence *intel_get_excl_fence(struct drm_i915_gem_object *obj)
> +{
> +	struct reservation_object *resv;
> +
> +
> +	if (!obj->base.dma_buf)
> +		return NULL;
> +
> +	resv = obj->base.dma_buf->resv;
> +
> +	/* For framebuffer backed by dmabuf, wait for fence */
> +	while (1) {
> +		struct fence *fence_excl, *ret = NULL;
> +
> +		rcu_read_lock();
> +
> +		fence_excl = rcu_dereference(resv->fence_excl);
> +		if (fence_excl)
> +			ret = fence_get_rcu(fence_excl);
> +
> +		rcu_read_unlock();
> +
> +		if (ret == fence_excl)
> +			return ret;
> +	}
> +}
> +
>  /**
>   * intel_prepare_plane_fb - Prepare fb for usage on plane
>   * @plane: drm plane to prepare for
> @@ -13536,42 +13390,6 @@ intel_check_primary_plane(struct drm_plane *plane,
>  					     &state->visible);
>  }
>  
> -static void intel_begin_crtc_commit(struct drm_crtc *crtc,
> -				    struct drm_crtc_state *old_crtc_state)
> -{
> -	struct drm_device *dev = crtc->dev;
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct intel_crtc_state *old_intel_state =
> -		to_intel_crtc_state(old_crtc_state);
> -	bool modeset = needs_modeset(crtc->state);
> -
> -	intel_frontbuffer_flip_prepare(dev, to_intel_crtc_state(crtc->state)->fb_bits);
> -
> -	/* Perform vblank evasion around commit operation */
> -	intel_pipe_update_start(intel_crtc);
> -
> -	if (modeset)
> -		return;
> -
> -	if (crtc->state->color_mgmt_changed || to_intel_crtc_state(crtc->state)->update_pipe) {
> -		intel_color_set_csc(crtc->state);
> -		intel_color_load_luts(crtc->state);
> -	}
> -
> -	if (to_intel_crtc_state(crtc->state)->update_pipe)
> -		intel_update_pipe_config(intel_crtc, old_intel_state);
> -	else if (INTEL_INFO(dev)->gen >= 9)
> -		skl_detach_scalers(intel_crtc);
> -}
> -
> -static void intel_finish_crtc_commit(struct drm_crtc *crtc,
> -				     struct drm_crtc_state *old_crtc_state)
> -{
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -
> -	intel_pipe_update_end(intel_crtc, NULL);
> -}
> -
>  /**
>   * intel_plane_destroy - destroy a plane
>   * @plane: plane to destroy
> -- 
> 2.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Intel Sweden AB Registered Office: Knarrarnasgatan 15, 164 40 Kista, Stockholm, Sweden Registration Number: 556189-6027 


More information about the Intel-gfx mailing list