All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ander Conselvan De Oliveira <conselvan2@gmail.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2 05/11] drm/i915: Allow mmio updates on all platforms, v2.
Date: Fri, 15 Apr 2016 15:31:50 +0300	[thread overview]
Message-ID: <1460723510.6600.36.camel@gmail.com> (raw)
In-Reply-To: <1460539104-15661-6-git-send-email-maarten.lankhorst@linux.intel.com>

On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
> and unpinning.

I think the rename should be a separate patch.

Ander

>  Use flip_queued_req to hold the wait request in the
> mmio case and allow the vblank interrupt to complete mmio work to
> have mmio flips run correctly on g4 and earlier.
> 
> Changes since v1:
> - Add smp_mb__after_atomic() to __intel_pageflip_stall_check,
>   to match the smp_mb__before_atomic() in pipe_update_end().
> - Check for cur != flip_queued_vblank in pageflip_stall_check.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |   4 +-
>  drivers/gpu/drm/i915/intel_display.c | 271 +++++++++++-----------------------
> -
>  drivers/gpu/drm/i915/intel_drv.h     |  18 +--
>  drivers/gpu/drm/i915/intel_sprite.c  |   8 +-
>  4 files changed, 95 insertions(+), 206 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index c3b029e7bffd..5662cd5a1a9d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -574,10 +574,10 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> void *data)
>  	for_each_intel_crtc(dev, crtc) {
>  		const char pipe = pipe_name(crtc->pipe);
>  		const char plane = plane_name(crtc->plane);
> -		struct intel_unpin_work *work;
> +		struct intel_flip_work *work;
>  
>  		spin_lock_irq(&dev->event_lock);
> -		work = crtc->unpin_work;
> +		work = crtc->flip_work;
>  		if (work == NULL) {
>  			seq_printf(m, "No flip due on pipe %c (plane %c)\n",
>  				   pipe, plane);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f1a895153e64..b614f118b973 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -48,6 +48,11 @@
>  #include <linux/reservation.h>
>  #include <linux/dma-buf.h>
>  
> +static bool is_mmio_work(struct intel_flip_work *work)
> +{
> +	return work->mmio_work.func;
> +}
> +
>  /* Primary plane formats for gen <= 3 */
>  static const uint32_t i8xx_primary_formats[] = {
>  	DRM_FORMAT_C8,
> @@ -3285,7 +3290,7 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc
> *crtc)
>  		return false;
>  
>  	spin_lock_irq(&dev->event_lock);
> -	pending = to_intel_crtc(crtc)->unpin_work != NULL;
> +	pending = to_intel_crtc(crtc)->flip_work != NULL;
>  	spin_unlock_irq(&dev->event_lock);
>  
>  	return pending;
> @@ -3864,7 +3869,7 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
>  		if (atomic_read(&crtc->unpin_work_count) == 0)
>  			continue;
>  
> -		if (crtc->unpin_work)
> +		if (crtc->flip_work)
>  			intel_wait_for_vblank(dev, crtc->pipe);
>  
>  		return true;
> @@ -3876,11 +3881,11 @@ bool intel_has_pending_fb_unpin(struct drm_device
> *dev)
>  static void page_flip_completed(struct intel_crtc *intel_crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> -	struct intel_unpin_work *work = intel_crtc->unpin_work;
> +	struct intel_flip_work *work = intel_crtc->flip_work;
>  
> -	/* ensure that the unpin work is consistent wrt ->pending. */
> +	/* ensure that the flip work is consistent wrt ->pending. */
>  	smp_rmb();
> -	intel_crtc->unpin_work = NULL;
> +	intel_crtc->flip_work = NULL;
>  
>  	if (work->event)
>  		drm_send_vblank_event(intel_crtc->base.dev,
> @@ -3890,7 +3895,7 @@ static void page_flip_completed(struct intel_crtc
> *intel_crtc)
>  	drm_crtc_vblank_put(&intel_crtc->base);
>  
>  	wake_up_all(&dev_priv->pending_flip_queue);
> -	queue_work(dev_priv->wq, &work->work);
> +	queue_work(dev_priv->wq, &work->unpin_work);
>  
>  	trace_i915_flip_complete(intel_crtc->plane,
>  				 work->pending_flip_obj);
> @@ -3914,9 +3919,11 @@ static int intel_crtc_wait_for_pending_flips(struct
> drm_crtc *crtc)
>  
>  	if (ret == 0) {
>  		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +		struct intel_flip_work *work;
>  
>  		spin_lock_irq(&dev->event_lock);
> -		if (intel_crtc->unpin_work) {
> +		work = intel_crtc->flip_work;
> +		if (work && !is_mmio_work(work)) {
>  			WARN_ONCE(1, "Removing stuck page flip\n");
>  			page_flip_completed(intel_crtc);
>  		}
> @@ -6308,7 +6315,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc
> *crtc)
>  		return;
>  
>  	if (to_intel_plane_state(crtc->primary->state)->visible) {
> -		WARN_ON(intel_crtc->unpin_work);
> +		WARN_ON(intel_crtc->flip_work);
>  
>  		intel_pre_disable_primary_noatomic(crtc);
>  
> @@ -10881,15 +10888,16 @@ static void intel_crtc_destroy(struct drm_crtc
> *crtc)
>  {
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct drm_device *dev = crtc->dev;
> -	struct intel_unpin_work *work;
> +	struct intel_flip_work *work;
>  
>  	spin_lock_irq(&dev->event_lock);
> -	work = intel_crtc->unpin_work;
> -	intel_crtc->unpin_work = NULL;
> +	work = intel_crtc->flip_work;
> +	intel_crtc->flip_work = NULL;
>  	spin_unlock_irq(&dev->event_lock);
>  
>  	if (work) {
> -		cancel_work_sync(&work->work);
> +		cancel_work_sync(&work->mmio_work);
> +		cancel_work_sync(&work->unpin_work);
>  		kfree(work);
>  	}
>  
> @@ -10900,12 +10908,15 @@ static void intel_crtc_destroy(struct drm_crtc
> *crtc)
>  
>  static void intel_unpin_work_fn(struct work_struct *__work)
>  {
> -	struct intel_unpin_work *work =
> -		container_of(__work, struct intel_unpin_work, work);
> +	struct intel_flip_work *work =
> +		container_of(__work, struct intel_flip_work, unpin_work);
>  	struct intel_crtc *crtc = to_intel_crtc(work->crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_plane *primary = crtc->base.primary;
>  
> +	if (is_mmio_work(work))
> +		flush_work(&work->mmio_work);
> +
>  	mutex_lock(&dev->struct_mutex);
>  	intel_unpin_fb_obj(work->old_fb, primary->state->rotation);
>  	drm_gem_object_unreference(&work->pending_flip_obj->base);
> @@ -10971,16 +10982,16 @@ static bool page_flip_finished(struct intel_crtc
> *crtc)
>  	 * anyway, we don't really care.
>  	 */
>  	return (I915_READ(DSPSURFLIVE(crtc->plane)) & ~0xfff) ==
> -		crtc->unpin_work->gtt_offset &&
> +		crtc->flip_work->gtt_offset &&
>  		g4x_flip_count_after_eq(I915_READ(PIPE_FLIPCOUNT_G4X(crtc
> ->pipe)),
> -				    crtc->unpin_work->flip_count);
> +				    crtc->flip_work->flip_count);
>  }
>  
>  static void do_intel_finish_page_flip(struct drm_device *dev,
>  				      struct drm_crtc *crtc)
>  {
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct intel_unpin_work *work;
> +	struct intel_flip_work *work;
>  	unsigned long flags;
>  
>  	/* Ignore early vblank irqs */
> @@ -10992,11 +11003,11 @@ static void do_intel_finish_page_flip(struct
> drm_device *dev,
>  	 * lost pageflips) so needs the full irqsave spinlocks.
>  	 */
>  	spin_lock_irqsave(&dev->event_lock, flags);
> -	work = intel_crtc->unpin_work;
> +	work = intel_crtc->flip_work;
>  
>  	if (work == NULL ||
>  	    atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
> -	   !page_flip_finished(intel_crtc)) {
> +	    !page_flip_finished(intel_crtc)) {
>  		spin_unlock_irqrestore(&dev->event_lock, flags);
>  		return;
>  	}
> @@ -11022,7 +11033,7 @@ void intel_finish_page_flip_plane(struct drm_device
> *dev, int plane)
>  	do_intel_finish_page_flip(dev, crtc);
>  }
>  
> -static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
> +static inline void intel_mark_page_flip_active(struct intel_flip_work *work)
>  {
>  	/* Ensure that the work item is consistent when activating it ... */
>  	smp_wmb();
> @@ -11059,10 +11070,9 @@ static int intel_gen2_queue_flip(struct drm_device
> *dev,
>  	intel_ring_emit(engine, MI_DISPLAY_FLIP |
>  			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
>  	intel_ring_emit(engine, fb->pitches[0]);
> -	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
> +	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
>  	intel_ring_emit(engine, 0); /* aux display base address, unused */
>  
> -	intel_mark_page_flip_active(intel_crtc->unpin_work);
>  	return 0;
>  }
>  
> @@ -11091,10 +11101,9 @@ static int intel_gen3_queue_flip(struct drm_device
> *dev,
>  	intel_ring_emit(engine, MI_DISPLAY_FLIP_I915 |
>  			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
>  	intel_ring_emit(engine, fb->pitches[0]);
> -	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
> +	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
>  	intel_ring_emit(engine, MI_NOOP);
>  
> -	intel_mark_page_flip_active(intel_crtc->unpin_work);
>  	return 0;
>  }
>  
> @@ -11122,7 +11131,7 @@ static int intel_gen4_queue_flip(struct drm_device
> *dev,
>  	intel_ring_emit(engine, MI_DISPLAY_FLIP |
>  			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
>  	intel_ring_emit(engine, fb->pitches[0]);
> -	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset |
> +	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset |
>  			obj->tiling_mode);
>  
>  	/* XXX Enabling the panel-fitter across page-flip is so far
> @@ -11133,7 +11142,6 @@ static int intel_gen4_queue_flip(struct drm_device
> *dev,
>  	pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
>  	intel_ring_emit(engine, pf | pipesrc);
>  
> -	intel_mark_page_flip_active(intel_crtc->unpin_work);
>  	return 0;
>  }
>  
> @@ -11157,7 +11165,7 @@ static int intel_gen6_queue_flip(struct drm_device
> *dev,
>  	intel_ring_emit(engine, MI_DISPLAY_FLIP |
>  			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
>  	intel_ring_emit(engine, fb->pitches[0] | obj->tiling_mode);
> -	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
> +	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
>  
>  	/* Contrary to the suggestions in the documentation,
>  	 * "Enable Panel Fitter" does not seem to be required when page
> @@ -11169,7 +11177,6 @@ static int intel_gen6_queue_flip(struct drm_device
> *dev,
>  	pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
>  	intel_ring_emit(engine, pf | pipesrc);
>  
> -	intel_mark_page_flip_active(intel_crtc->unpin_work);
>  	return 0;
>  }
>  
> @@ -11261,10 +11268,9 @@ static int intel_gen7_queue_flip(struct drm_device
> *dev,
>  
>  	intel_ring_emit(engine, MI_DISPLAY_FLIP_I915 | plane_bit);
>  	intel_ring_emit(engine, (fb->pitches[0] | obj->tiling_mode));
> -	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
> +	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
>  	intel_ring_emit(engine, (MI_NOOP));
>  
> -	intel_mark_page_flip_active(intel_crtc->unpin_work);
>  	return 0;
>  }
>  
> @@ -11282,9 +11288,6 @@ static bool use_mmio_flip(struct intel_engine_cs
> *engine,
>  	if (engine == NULL)
>  		return true;
>  
> -	if (INTEL_INFO(engine->dev)->gen < 5)
> -		return false;
> -
>  	if (i915.use_mmio_flip < 0)
>  		return false;
>  	else if (i915.use_mmio_flip > 0)
> @@ -11299,126 +11302,21 @@ static bool use_mmio_flip(struct intel_engine_cs
> *engine,
>  		return engine != i915_gem_request_get_engine(obj
> ->last_write_req);
>  }
>  
> -static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> -			     unsigned int rotation,
> -			     struct intel_unpin_work *work)
> +static void intel_mmio_flip_work_func(struct work_struct *w)
>  {
> -	struct drm_device *dev = intel_crtc->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
> -	const enum pipe pipe = intel_crtc->pipe;
> -	u32 ctl, stride, tile_height;
> -
> -	ctl = I915_READ(PLANE_CTL(pipe, 0));
> -	ctl &= ~PLANE_CTL_TILED_MASK;
> -	switch (fb->modifier[0]) {
> -	case DRM_FORMAT_MOD_NONE:
> -		break;
> -	case I915_FORMAT_MOD_X_TILED:
> -		ctl |= PLANE_CTL_TILED_X;
> -		break;
> -	case I915_FORMAT_MOD_Y_TILED:
> -		ctl |= PLANE_CTL_TILED_Y;
> -		break;
> -	case I915_FORMAT_MOD_Yf_TILED:
> -		ctl |= PLANE_CTL_TILED_YF;
> -		break;
> -	default:
> -		MISSING_CASE(fb->modifier[0]);
> -	}
> -
> -	/*
> -	 * The stride is either expressed as a multiple of 64 bytes chunks
> for
> -	 * linear buffers or in number of tiles for tiled buffers.
> -	 */
> -	if (intel_rotation_90_or_270(rotation)) {
> -		/* stride = Surface height in tiles */
> -		tile_height = intel_tile_height(dev_priv, fb->modifier[0],
> 0);
> -		stride = DIV_ROUND_UP(fb->height, tile_height);
> -	} else {
> -		stride = fb->pitches[0] /
> -			intel_fb_stride_alignment(dev_priv, fb->modifier[0],
> -						  fb->pixel_format);
> -	}
> -
> -	/*
> -	 * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
> -	 * PLANE_SURF updates, the update is then guaranteed to be atomic.
> -	 */
> -	I915_WRITE(PLANE_CTL(pipe, 0), ctl);
> -	I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
> -
> -	I915_WRITE(PLANE_SURF(pipe, 0), work->gtt_offset);
> -	POSTING_READ(PLANE_SURF(pipe, 0));
> -}
> -
> -static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc,
> -			     struct intel_unpin_work *work)
> -{
> -	struct drm_device *dev = intel_crtc->base.dev;
> +	struct intel_flip_work *work =
> +		container_of(w, struct intel_flip_work, mmio_work);
> +	struct intel_crtc *crtc = to_intel_crtc(work->crtc);
> +	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_framebuffer *intel_fb =
> -		to_intel_framebuffer(intel_crtc->base.primary->fb);
> -	struct drm_i915_gem_object *obj = intel_fb->obj;
> -	i915_reg_t reg = DSPCNTR(intel_crtc->plane);
> -	u32 dspcntr;
> -
> -	dspcntr = I915_READ(reg);
> -
> -	if (obj->tiling_mode != I915_TILING_NONE)
> -		dspcntr |= DISPPLANE_TILED;
> -	else
> -		dspcntr &= ~DISPPLANE_TILED;
> +	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
> +	struct drm_i915_gem_object *obj = intel_fb_obj(primary->base.state
> ->fb);
>  
> -	I915_WRITE(reg, dspcntr);
> -
> -	I915_WRITE(DSPSURF(intel_crtc->plane), work->gtt_offset);
> -	POSTING_READ(DSPSURF(intel_crtc->plane));
> -}
> -
> -/*
> - * XXX: This is the temporary way to update the plane registers until we get
> - * around to using the usual plane update functions for MMIO flips
> - */
> -static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
> -{
> -	struct intel_crtc *crtc = mmio_flip->crtc;
> -	struct intel_unpin_work *work;
> -
> -	spin_lock_irq(&crtc->base.dev->event_lock);
> -	work = crtc->unpin_work;
> -	spin_unlock_irq(&crtc->base.dev->event_lock);
> -	if (work == NULL)
> -		return;
> -
> -	intel_pipe_update_start(crtc);
> -
> -	if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
> -		skl_do_mmio_flip(crtc, mmio_flip->rotation, work);
> -	else
> -		/* use_mmio_flip() retricts MMIO flips to ilk+ */
> -		ilk_do_mmio_flip(crtc, work);
> -
> -	intel_pipe_update_end(crtc);
> -
> -	intel_mark_page_flip_active(work);
> -}
> -
> -static void intel_mmio_flip_work_func(struct work_struct *work)
> -{
> -	struct intel_mmio_flip *mmio_flip =
> -		container_of(work, struct intel_mmio_flip, work);
> -	struct intel_framebuffer *intel_fb =
> -		to_intel_framebuffer(mmio_flip->crtc->base.primary->fb);
> -	struct drm_i915_gem_object *obj = intel_fb->obj;
> -
> -	if (mmio_flip->req) {
> -		WARN_ON(__i915_wait_request(mmio_flip->req,
> -					    mmio_flip->crtc->reset_counter,
> +	if (work->flip_queued_req)
> +		WARN_ON(__i915_wait_request(work->flip_queued_req,
> +					    crtc->reset_counter,
>  					    false, NULL,
> -					    &mmio_flip->i915
> ->rps.mmioflips));
> -		i915_gem_request_unreference__unlocked(mmio_flip->req);
> -	}
> +					    &dev_priv->rps.mmioflips));
>  
>  	/* For framebuffer backed by dmabuf, wait for fence */
>  	if (obj->base.dma_buf)
> @@ -11426,29 +11324,11 @@ static void intel_mmio_flip_work_func(struct
> work_struct *work)
>  							    false, false,
>  							   
>  MAX_SCHEDULE_TIMEOUT) < 0);
>  
> -	intel_do_mmio_flip(mmio_flip);
> -	kfree(mmio_flip);
> -}
> -
> -static int intel_queue_mmio_flip(struct drm_device *dev,
> -				 struct drm_crtc *crtc,
> -				 struct drm_i915_gem_object *obj)
> -{
> -	struct intel_mmio_flip *mmio_flip;
> -
> -	mmio_flip = kmalloc(sizeof(*mmio_flip), GFP_KERNEL);
> -	if (mmio_flip == NULL)
> -		return -ENOMEM;
> -
> -	mmio_flip->i915 = to_i915(dev);
> -	mmio_flip->req = i915_gem_request_reference(obj->last_write_req);
> -	mmio_flip->crtc = to_intel_crtc(crtc);
> -	mmio_flip->rotation = crtc->primary->state->rotation;
> -
> -	INIT_WORK(&mmio_flip->work, intel_mmio_flip_work_func);
> -	schedule_work(&mmio_flip->work);
> -
> -	return 0;
> +	intel_pipe_update_start(crtc);
> +	primary->update_plane(&primary->base,
> +			      crtc->config,
> +			      to_intel_plane_state(primary->base.state));
> +	intel_pipe_update_end(crtc, work);
>  }
>  
>  static int intel_default_queue_flip(struct drm_device *dev,
> @@ -11466,7 +11346,7 @@ static bool __intel_pageflip_stall_check(struct
> drm_device *dev,
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct intel_unpin_work *work = intel_crtc->unpin_work;
> +	struct intel_flip_work *work = intel_crtc->flip_work;
>  	u32 addr;
>  	u32 pending;
>  
> @@ -11474,6 +11354,15 @@ static bool __intel_pageflip_stall_check(struct
> drm_device *dev,
>  	if (pending == INTEL_FLIP_INACTIVE)
>  		return false;
>  
> +	smp_mb__after_atomic();
> +
> +	if (is_mmio_work(work)) {
> +		u32 cur = intel_crtc_get_vblank_counter(intel_crtc);
> +
> +		/* MMIO work completes when vblank is different from
> flip_queued_vblank. */
> +		return cur != work->flip_queued_vblank;
> +	}
> +
>  	if (work->flip_ready_vblank == 0) {
>  		if (work->flip_queued_req &&
>  		    !i915_gem_request_completed(work->flip_queued_req, true))
> @@ -11504,7 +11393,7 @@ void intel_check_page_flip(struct drm_device *dev, int
> pipe)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct intel_unpin_work *work;
> +	struct intel_flip_work *work;
>  
>  	WARN_ON(!in_interrupt());
>  
> @@ -11512,14 +11401,15 @@ void intel_check_page_flip(struct drm_device *dev,
> int pipe)
>  		return;
>  
>  	spin_lock(&dev->event_lock);
> -	work = intel_crtc->unpin_work;
> +	work = intel_crtc->flip_work;
>  	if (work != NULL && __intel_pageflip_stall_check(dev, crtc)) {
> -		WARN_ONCE(1, "Kicking stuck page flip: queued at %d, now
> %d\n",
> +		WARN_ONCE(!is_mmio_work(work),
> +			  "Kicking stuck page flip: queued at %d, now %d\n",
>  			 work->flip_queued_vblank, drm_vblank_count(dev,
> pipe));
>  		page_flip_completed(intel_crtc);
>  		work = NULL;
>  	}
> -	if (work != NULL &&
> +	if (work != NULL && !is_mmio_work(work) &&
>  	    drm_vblank_count(dev, pipe) - work->flip_queued_vblank > 1)
>  		intel_queue_rps_boost_for_request(dev, work
> ->flip_queued_req);
>  	spin_unlock(&dev->event_lock);
> @@ -11537,7 +11427,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct drm_plane *primary = crtc->primary;
>  	enum pipe pipe = intel_crtc->pipe;
> -	struct intel_unpin_work *work;
> +	struct intel_flip_work *work;
>  	struct intel_engine_cs *engine;
>  	bool mmio_flip;
>  	struct drm_i915_gem_request *request = NULL;
> @@ -11574,15 +11464,15 @@ static int intel_crtc_page_flip(struct drm_crtc
> *crtc,
>  	work->event = event;
>  	work->crtc = crtc;
>  	work->old_fb = old_fb;
> -	INIT_WORK(&work->work, intel_unpin_work_fn);
> +	INIT_WORK(&work->unpin_work, intel_unpin_work_fn);
>  
>  	ret = drm_crtc_vblank_get(crtc);
>  	if (ret)
>  		goto free_work;
>  
> -	/* We borrow the event spin lock for protecting unpin_work */
> +	/* We borrow the event spin lock for protecting flip_work */
>  	spin_lock_irq(&dev->event_lock);
> -	if (intel_crtc->unpin_work) {
> +	if (intel_crtc->flip_work) {
>  		/* Before declaring the flip queue wedged, check if
>  		 * the hardware completed the operation behind our backs.
>  		 */
> @@ -11598,7 +11488,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  			return -EBUSY;
>  		}
>  	}
> -	intel_crtc->unpin_work = work;
> +	intel_crtc->flip_work = work;
>  	spin_unlock_irq(&dev->event_lock);
>  
>  	if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
> @@ -11667,23 +11557,22 @@ static int intel_crtc_page_flip(struct drm_crtc
> *crtc,
>  	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>  
>  	if (mmio_flip) {
> -		ret = intel_queue_mmio_flip(dev, crtc, obj);
> -		if (ret)
> -			goto cleanup_unpin;
> +		INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
>  
>  		i915_gem_request_assign(&work->flip_queued_req,
>  					obj->last_write_req);
> +
> +		schedule_work(&work->mmio_work);
>  	} else {
> +		i915_gem_request_assign(&work->flip_queued_req, request);
>  		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj,
> request,
>  						   page_flip_flags);
>  		if (ret)
>  			goto cleanup_unpin;
>  
> -		i915_gem_request_assign(&work->flip_queued_req, request);
> -	}
> -
> -	if (request)
> +		intel_mark_page_flip_active(work);
>  		i915_add_request_no_flush(request);
> +	}
>  
>  	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>  
> @@ -11713,7 +11602,7 @@ cleanup:
>  	drm_framebuffer_unreference(work->old_fb);
>  
>  	spin_lock_irq(&dev->event_lock);
> -	intel_crtc->unpin_work = NULL;
> +	intel_crtc->flip_work = NULL;
>  	spin_unlock_irq(&dev->event_lock);
>  
>  	drm_crtc_vblank_put(crtc);
> @@ -14018,7 +13907,7 @@ static void intel_finish_crtc_commit(struct drm_crtc
> *crtc,
>  {
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
> -	intel_pipe_update_end(intel_crtc);
> +	intel_pipe_update_end(intel_crtc, NULL);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index a2712f6f1eb3..d46aff0350a0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -592,14 +592,6 @@ struct vlv_wm_state {
>  	bool cxsr;
>  };
>  
> -struct intel_mmio_flip {
> -	struct work_struct work;
> -	struct drm_i915_private *i915;
> -	struct drm_i915_gem_request *req;
> -	struct intel_crtc *crtc;
> -	unsigned int rotation;
> -};
> -
>  struct intel_crtc {
>  	struct drm_crtc base;
>  	enum pipe pipe;
> @@ -614,7 +606,7 @@ struct intel_crtc {
>  	unsigned long enabled_power_domains;
>  	bool lowfreq_avail;
>  	struct intel_overlay *overlay;
> -	struct intel_unpin_work *unpin_work;
> +	struct intel_flip_work *flip_work;
>  
>  	atomic_t unpin_work_count;
>  
> @@ -937,8 +929,10 @@ intel_get_crtc_for_plane(struct drm_device *dev, int
> plane)
>  	return dev_priv->plane_to_crtc_mapping[plane];
>  }
>  
> -struct intel_unpin_work {
> -	struct work_struct work;
> +struct intel_flip_work {
> +	struct work_struct unpin_work;
> +	struct work_struct mmio_work;
> +
>  	struct drm_crtc *crtc;
>  	struct drm_framebuffer *old_fb;
>  	struct drm_i915_gem_object *pending_flip_obj;
> @@ -1618,7 +1612,7 @@ int intel_plane_init(struct drm_device *dev, enum pipe
> pipe, int plane);
>  int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>  			      struct drm_file *file_priv);
>  void intel_pipe_update_start(struct intel_crtc *crtc);
> -void intel_pipe_update_end(struct intel_crtc *crtc);
> +void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work
> *work);
>  
>  /* intel_tv.c */
>  void intel_tv_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index e2de6b0df5a8..8ec7ce549835 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -151,13 +151,19 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
>   * re-enables interrupts and verifies the update was actually completed
>   * before a vblank using the value of @start_vbl_count.
>   */
> -void intel_pipe_update_end(struct intel_crtc *crtc)
> +void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work
> *work)
>  {
>  	enum pipe pipe = crtc->pipe;
>  	int scanline_end = intel_get_crtc_scanline(crtc);
>  	u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc);
>  	ktime_t end_vbl_time = ktime_get();
>  
> +	if (work) {
> +		work->flip_queued_vblank = end_vbl_count;
> +		smp_mb__before_atomic();
> +		atomic_set(&work->pending, INTEL_FLIP_PENDING);
> +	}
> +
>  	trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
>  
>  	local_irq_enable();
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2016-04-15 12:31 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-13  9:18 [PATCH v2 00/11] drm/i915: Rework page flip to be more atomic like, and remove cs flips Maarten Lankhorst
2016-04-13  9:18 ` [PATCH v2 01/11] drm/core: Add drm_accurate_vblank_count, v4 Maarten Lankhorst
2016-04-13  9:18 ` [PATCH v2 02/11] drm/i915: Remove stallcheck special handling Maarten Lankhorst
2016-04-15  7:07   ` Ander Conselvan De Oliveira
2016-04-18  5:31     ` Maarten Lankhorst
2016-04-18  7:57       ` Ander Conselvan De Oliveira
2016-04-18  8:23         ` Maarten Lankhorst
2016-04-18 10:00         ` [PATCH v2.1 02/11] drm/i915: Remove stallcheck special handling, v2 Maarten Lankhorst
2016-04-13  9:18 ` [PATCH v2 03/11] drm/i915: Remove intel_prepare_page_flip Maarten Lankhorst
2016-04-15 12:21   ` Ander Conselvan De Oliveira
2016-04-18  5:27     ` Maarten Lankhorst
2016-04-18 10:09     ` [PATCH v2.1 03/11] drm/i915: Remove intel_prepare_page_flip, v2 Maarten Lankhorst
2016-04-18 10:10       ` [PATCH v2.2 " Maarten Lankhorst
2016-04-13  9:18 ` [PATCH v2 04/11] drm/i915: Add support for detecting vblanks when hw frame counter is unavailable Maarten Lankhorst
2016-04-13  9:18 ` [PATCH v2 05/11] drm/i915: Allow mmio updates on all platforms, v2 Maarten Lankhorst
2016-04-15 12:31   ` Ander Conselvan De Oliveira [this message]
2016-04-18  5:47     ` Maarten Lankhorst
2016-04-13  9:18 ` [PATCH v2 06/11] drm/i915: Convert flip_work to a list Maarten Lankhorst
2016-04-13  9:18 ` [PATCH v2 07/11] drm/i915: Add the exclusive fence to plane_state Maarten Lankhorst
2016-04-13  9:18 ` [PATCH v2 08/11] drm/i915: Rework intel_crtc_page_flip to be almost atomic, v3 Maarten Lankhorst
2016-04-13  9:18 ` [PATCH v2 09/11] drm/i915: Remove cs based page flip support Maarten Lankhorst
2016-04-13  9:18 ` [PATCH v2 10/11] drm/i915: Remove use_mmio_flip kernel parameter Maarten Lankhorst
2016-04-13  9:18 ` [PATCH v2 11/11] drm/i915: Remove queue_flip pointer Maarten Lankhorst
2016-04-13 17:55 ` ✗ Fi.CI.BAT: failure for drm/i915: Rework page flip to be more atomic like, and remove cs flips Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1460723510.6600.36.camel@gmail.com \
    --to=conselvan2@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.