All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ander Conselvan de Oliveira <conselvan2@gmail.com>
To: Matt Roper <matthew.d.roper@intel.com>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/5] drm/i915: Move vblank evasion to commit (v3)
Date: Tue, 23 Dec 2014 12:01:34 +0200	[thread overview]
Message-ID: <54993D7E.5010402@gmail.com> (raw)
In-Reply-To: <1418689401-22957-3-git-send-email-matthew.d.roper@intel.com>

On 12/16/2014 02:23 AM, Matt Roper wrote:
> Move the vblank evasion up from the low-level, hw-specific
> update_plane() handlers to the general plane commit operation.
> Everything inside commit should now be non-sleeping, so this brings us
> closer to how vblank evasion will behave once we move over to atomic.
>
> v2:
>   - Restore lost intel_crtc->active check on vblank evasion
>
> v3:
>   - Replace assert_pipe_enabled() in intel_disable_primary_hw_plane()
>     with an intel_crtc->active test; it turns out assert_pipe_enabled()
>     grabs some mutexes and can sleep, which we can't do with interrupts
>     disabled.

Sounds like this should have gone in the previous patch, or a separate one.

Ander

>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 13 ++++++++++-
>   drivers/gpu/drm/i915/intel_drv.h     |  4 ++++
>   drivers/gpu/drm/i915/intel_sprite.c  | 42 ------------------------------------
>   3 files changed, 16 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5d90114..ce552d1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2165,7 +2165,8 @@ static void intel_disable_primary_hw_plane(struct drm_plane *plane,
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>
> -	assert_pipe_enabled(dev_priv, intel_crtc->pipe);
> +	if (WARN_ON(!intel_crtc->active))
> +		return;
>
>   	if (!intel_crtc->primary_enabled)
>   		return;
> @@ -11861,6 +11862,12 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc)
>
>   	if (intel_crtc->atomic.update_wm)
>   		intel_update_watermarks(crtc);
> +
> +	/* Perform vblank evasion around commit operation */
> +	if (intel_crtc->active)
> +		intel_crtc->atomic.evade =
> +			intel_pipe_update_start(intel_crtc,
> +						&intel_crtc->atomic.start_vbl_count);
>   }
>
>   static void intel_finish_crtc_commit(struct drm_crtc *crtc)
> @@ -11869,6 +11876,10 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc)
>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   	struct drm_plane *p;
>
> +	if (intel_crtc->atomic.evade)
> +		intel_pipe_update_end(intel_crtc,
> +				      intel_crtc->atomic.start_vbl_count);
> +
>   	if (intel_crtc->atomic.wait_vblank)
>   		intel_wait_for_vblank(dev, intel_crtc->pipe);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a03bd72..1934156 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -428,6 +428,10 @@ struct skl_pipe_wm {
>    * and thus can't be run with interrupts disabled.
>    */
>   struct intel_crtc_atomic_commit {
> +	/* vblank evasion */
> +	bool evade;
> +	unsigned start_vbl_count;
> +
>   	/* Sleepable operations to perform before commit */
>   	bool wait_for_flips;
>   	bool disable_fbc;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index ff7d6a1..2520748 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -412,8 +412,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>   	u32 sprctl;
>   	unsigned long sprsurf_offset, linear_offset;
>   	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> -	u32 start_vbl_count;
> -	bool atomic_update;
>
>   	sprctl = I915_READ(SPCNTR(pipe, plane));
>
> @@ -502,8 +500,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>   		linear_offset += src_h * fb->pitches[0] + src_w * pixel_size;
>   	}
>
> -	atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
> -
>   	intel_update_primary_plane(intel_crtc);
>
>   	if (IS_CHERRYVIEW(dev) && pipe == PIPE_B)
> @@ -525,9 +521,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>   		   sprsurf_offset);
>
>   	intel_flush_primary_plane(dev_priv, intel_crtc->plane);
> -
> -	if (atomic_update)
> -		intel_pipe_update_end(intel_crtc, start_vbl_count);
>   }
>
>   static void
> @@ -539,10 +532,6 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   	int pipe = intel_plane->pipe;
>   	int plane = intel_plane->plane;
> -	u32 start_vbl_count;
> -	bool atomic_update;
> -
> -	atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
>
>   	intel_update_primary_plane(intel_crtc);
>
> @@ -553,9 +542,6 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
>
>   	intel_flush_primary_plane(dev_priv, intel_crtc->plane);
>
> -	if (atomic_update)
> -		intel_pipe_update_end(intel_crtc, start_vbl_count);
> -
>   	intel_update_sprite_watermarks(dplane, crtc, 0, 0, 0, false, false);
>   }
>
> @@ -626,8 +612,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>   	u32 sprctl, sprscale = 0;
>   	unsigned long sprsurf_offset, linear_offset;
>   	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> -	u32 start_vbl_count;
> -	bool atomic_update;
>
>   	sprctl = I915_READ(SPRCTL(pipe));
>
> @@ -711,8 +695,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>   		}
>   	}
>
> -	atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
> -
>   	intel_update_primary_plane(intel_crtc);
>
>   	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
> @@ -735,9 +717,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>   		   i915_gem_obj_ggtt_offset(obj) + sprsurf_offset);
>
>   	intel_flush_primary_plane(dev_priv, intel_crtc->plane);
> -
> -	if (atomic_update)
> -		intel_pipe_update_end(intel_crtc, start_vbl_count);
>   }
>
>   static void
> @@ -748,10 +727,6 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
>   	struct intel_plane *intel_plane = to_intel_plane(plane);
>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   	int pipe = intel_plane->pipe;
> -	u32 start_vbl_count;
> -	bool atomic_update;
> -
> -	atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
>
>   	intel_update_primary_plane(intel_crtc);
>
> @@ -764,9 +739,6 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
>
>   	intel_flush_primary_plane(dev_priv, intel_crtc->plane);
>
> -	if (atomic_update)
> -		intel_pipe_update_end(intel_crtc, start_vbl_count);
> -
>   	/*
>   	 * Avoid underruns when disabling the sprite.
>   	 * FIXME remove once watermark updates are done properly.
> @@ -845,8 +817,6 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>   	unsigned long dvssurf_offset, linear_offset;
>   	u32 dvscntr, dvsscale;
>   	int pixel_size = drm_format_plane_cpp(fb->pixel_format, 0);
> -	u32 start_vbl_count;
> -	bool atomic_update;
>
>   	dvscntr = I915_READ(DVSCNTR(pipe));
>
> @@ -921,8 +891,6 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>   		linear_offset += src_h * fb->pitches[0] + src_w * pixel_size;
>   	}
>
> -	atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
> -
>   	intel_update_primary_plane(intel_crtc);
>
>   	I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
> @@ -940,9 +908,6 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>   		   i915_gem_obj_ggtt_offset(obj) + dvssurf_offset);
>
>   	intel_flush_primary_plane(dev_priv, intel_crtc->plane);
> -
> -	if (atomic_update)
> -		intel_pipe_update_end(intel_crtc, start_vbl_count);
>   }
>
>   static void
> @@ -953,10 +918,6 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
>   	struct intel_plane *intel_plane = to_intel_plane(plane);
>   	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>   	int pipe = intel_plane->pipe;
> -	u32 start_vbl_count;
> -	bool atomic_update;
> -
> -	atomic_update = intel_pipe_update_start(intel_crtc, &start_vbl_count);
>
>   	intel_update_primary_plane(intel_crtc);
>
> @@ -968,9 +929,6 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
>
>   	intel_flush_primary_plane(dev_priv, intel_crtc->plane);
>
> -	if (atomic_update)
> -		intel_pipe_update_end(intel_crtc, start_vbl_count);
> -
>   	/*
>   	 * Avoid underruns when disabling the sprite.
>   	 * FIXME remove once watermark updates are done properly.
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2014-12-23 10:01 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-16  0:23 [PATCH 0/5] i915 atomic plane helper conversion (v4) Matt Roper
2014-12-16  0:23 ` [PATCH 1/5] drm/i915: Refactor work that can sleep out of commit (v5) Matt Roper
2014-12-22 14:12   ` Ander Conselvan de Oliveira
2014-12-16  0:23 ` [PATCH 2/5] drm/i915: Move vblank evasion to commit (v3) Matt Roper
2014-12-23 10:01   ` Ander Conselvan de Oliveira [this message]
2014-12-16  0:23 ` [PATCH 3/5] drm/i915: Clarify sprite plane function names (v3) Matt Roper
2014-12-23 10:08   ` Ander Conselvan de Oliveira
2015-01-05  9:54     ` Daniel Vetter
2014-12-16  0:23 ` [PATCH 4/5] drm/i915: Move to atomic plane helpers (v8) Matt Roper
2014-12-23 13:27   ` Ander Conselvan de Oliveira
2014-12-16  0:23 ` [PATCH 5/5] drm/i915: Drop unused position fields (v2) Matt Roper
2014-12-16  5:31   ` shuang.he
2014-12-23 13:34   ` Ander Conselvan de Oliveira

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=54993D7E.5010402@gmail.com \
    --to=conselvan2@gmail.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=matthew.d.roper@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.