Re: [PATCH v2 02/11] drm/i915: Remove stallcheck special handling.

[Ander Conselvan De Oliveira <conselvan2@gmail.com>]

On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
> Re-use unpin_work->pending, but also set vblank count before
> intel_mark_page_flip_active to be sure.

Be sure of what?

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  | 11 ++++++-----
>  drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++-------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>  3 files changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9640738aabf2..df8073a2ffbe 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -582,9 +582,14 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> void *data)
>  			seq_printf(m, "No flip due on pipe %c (plane %c)\n",
>  				   pipe, plane);
>  		} else {
> +			u32 pending;
>  			u32 addr;
>  
> -			if (atomic_read(&work->pending) <
> INTEL_FLIP_COMPLETE) {
> +			pending = atomic_read(&work->pending);
> +			if (pending == INTEL_FLIP_INACTIVE) {
> +				seq_printf(m, "Flip ioctl preparing on pipe
> %c (plane %c)\n",
> +					   pipe, plane);
> +			} else if (pending >= INTEL_FLIP_COMPLETE) {
>  				seq_printf(m, "Flip queued on pipe %c (plane
> %c)\n",
>  					   pipe, plane);
>  			} else {
> @@ -606,10 +611,6 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> void *data)
>  				   work->flip_queued_vblank,
>  				   work->flip_ready_vblank,
>  				   drm_crtc_vblank_count(&crtc->base));
> -			if (work->enable_stall_check)
> -				seq_puts(m, "Stall check enabled, ");
> -			else
> -				seq_puts(m, "Stall check waiting for page
> flip ioctl, ");
>  			seq_printf(m, "%d prepares\n", atomic_read(&work
> ->pending));
>  
>  			if (INTEL_INFO(dev)->gen >= 4)
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f2be54a48727..618e034a7a5e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11415,8 +11415,6 @@ static void intel_do_mmio_flip(struct intel_mmio_flip
> *mmio_flip)
>  	if (work == NULL)
>  		return;
>  
> -	intel_mark_page_flip_active(work);
> -
>  	intel_pipe_update_start(crtc);
>  
>  	if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
> @@ -11426,6 +11424,8 @@ static void intel_do_mmio_flip(struct intel_mmio_flip
> *mmio_flip)
>  		ilk_do_mmio_flip(crtc, work);
>  
>  	intel_pipe_update_end(crtc);
> +
> +	intel_mark_page_flip_active(work);

Is this to avoid triggering the stall check during the wait from a vblank
evasion?


>  }
>  
>  static void intel_mmio_flip_work_func(struct work_struct *work)
> @@ -11492,15 +11492,11 @@ static bool __intel_pageflip_stall_check(struct
> drm_device *dev,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_unpin_work *work = intel_crtc->unpin_work;
>  	u32 addr;
> +	u32 pending;
>  
> -	if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE)
> -		return true;
> -
> -	if (atomic_read(&work->pending) < INTEL_FLIP_PENDING)
> -		return false;
> -
> -	if (!work->enable_stall_check)
> -		return false;
> +	pending = atomic_read(&work->pending);
> +	if (pending != INTEL_FLIP_PENDING)
> +		return pending == INTEL_FLIP_COMPLETE;
>  
>  	if (work->flip_ready_vblank == 0) {
>  		if (work->flip_queued_req &&
> @@ -11676,6 +11672,11 @@ static int intel_crtc_page_flip(struct drm_crtc
> *crtc,
>  	 */
>  	if (!mmio_flip) {
>  		ret = i915_gem_object_sync(obj, engine, &request);
> +		if (!ret && !request) {
> +			request = i915_gem_request_alloc(engine, NULL);
> +			ret = PTR_ERR_OR_ZERO(request);
> +		}
> +
>  		if (ret)
>  			goto cleanup_pending;
>  	}
> @@ -11687,6 +11688,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  	work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary),
>  						  obj, 0);
>  	work->gtt_offset += intel_crtc->dspaddr_offset;
> +	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>  
>  	if (mmio_flip) {
>  		ret = intel_queue_mmio_flip(dev, crtc, obj);
> @@ -11696,14 +11698,6 @@ static int intel_crtc_page_flip(struct drm_crtc
> *crtc,
>  		i915_gem_request_assign(&work->flip_queued_req,
>  					obj->last_write_req);
>  	} else {
> -		if (!request) {
> -			request = i915_gem_request_alloc(engine, NULL);
> -			if (IS_ERR(request)) {
> -				ret = PTR_ERR(request);
> -				goto cleanup_unpin;
> -			}
> -		}
> -
>  		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj,
> request,
>  						   page_flip_flags);
>  		if (ret)
> @@ -11716,7 +11710,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  		i915_add_request_no_flush(request);
>  
>  	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);

Do we still need the assigment above?

Ander

> -	work->enable_stall_check = true;
>  
>  	i915_gem_track_fb(intel_fb_obj(work->old_fb), obj,
>  			  to_intel_plane(primary)->frontbuffer_bit);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index e0fcfa1683cc..ce2a6d985bbe 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -952,7 +952,6 @@ struct intel_unpin_work {
>  	struct drm_i915_gem_request *flip_queued_req;
>  	u32 flip_queued_vblank;
>  	u32 flip_ready_vblank;
> -	bool enable_stall_check;
>  };
>  
>  struct intel_load_detect_pipe {
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel