diff mbox

[12/14] drm/i915: Keep vblanks enabled during the entire pipe update

Message ID 20170317211808.14693-13-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä March 17, 2017, 9:18 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We currently hold a vblank referenced while trying to evade the
vblank, but we drop it as soon as we've done that. After all the
planes have been committed we are quite likely to grab a new vblank
reference for delivering the flip event. This causes the vblank
interrupt to do a enable->enable->disable ping-pong during many
commits. If we instead hang on to the original vblank reference
across the entire commit we can eliminate that ping-pong.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sprite.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Chris Wilson March 17, 2017, 9:28 p.m. UTC | #1
On Fri, Mar 17, 2017 at 11:18:06PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We currently hold a vblank referenced while trying to evade the
> vblank, but we drop it as soon as we've done that. After all the
> planes have been committed we are quite likely to grab a new vblank
> reference for delivering the flip event. This causes the vblank
> interrupt to do a enable->enable->disable ping-pong during many
> commits. If we instead hang on to the original vblank reference
> across the entire commit we can eliminate that ping-pong.

Does it make sense to take an an extra vblank_get() only if we have an
event to deliver?

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 26c8bdcd7e72..337e884252e5 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -137,8 +137,6 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
>  
>  	finish_wait(wq, &wait);
>  
	if (crtc->base.state->event)
		drm_crtc_vblank_get(&crtc->base);

	drm_crtc_vblank_put(&crtc->base);

>  	crtc->debug.scanline_start = scanline;
>  	crtc->debug.start_vbl_time = ktime_get();
>  	crtc->debug.start_vbl_count = intel_crtc_get_vblank_counter(crtc);

> @@ -185,6 +183,15 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work
>  		crtc->base.state->event = NULL;
>  
> +	/*
> +	 * The reference was taken in intel_pipe_update_start(). It could
> +	 * have been dropped as soon as the vblank was evaded, but we hold
> +	 * on to it until this time to avoid the extra vblank interrupt
> +	 * enable->disable->enable ping-pong whenever we have to deliver
> +	 * an event.
> +	 */
> +	drm_crtc_vblank_put(&crtc->base);

	}

>  	local_irq_enable();
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 26c8bdcd7e72..337e884252e5 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -137,8 +137,6 @@  void intel_pipe_update_start(struct intel_crtc *crtc)
 
 	finish_wait(wq, &wait);
 
-	drm_crtc_vblank_put(&crtc->base);
-
 	crtc->debug.scanline_start = scanline;
 	crtc->debug.start_vbl_time = ktime_get();
 	crtc->debug.start_vbl_count = intel_crtc_get_vblank_counter(crtc);
@@ -185,6 +183,15 @@  void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work
 		crtc->base.state->event = NULL;
 	}
 
+	/*
+	 * The reference was taken in intel_pipe_update_start(). It could
+	 * have been dropped as soon as the vblank was evaded, but we hold
+	 * on to it until this time to avoid the extra vblank interrupt
+	 * enable->disable->enable ping-pong whenever we have to deliver
+	 * an event.
+	 */
+	drm_crtc_vblank_put(&crtc->base);
+
 	local_irq_enable();
 
 	if (intel_vgpu_active(dev_priv))