Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1450710 > unrolled thread

[PATCH v4 3/6] drm/i915/skl: Only flush pipes when we change the ddb allocation

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 3/6] drm/i915/skl: Only flush pipes when we change the ddb allocation Lyude <cpaul@redhat.com> - 2016-07-26 19:40 +0200
    Re: [PATCH v4 3/6] drm/i915/skl: Only flush pipes when we change  the ddb allocation Matt Roper <matthew.d.roper@intel.com> - 2016-07-28 15:20 +0200

#1450710 — [PATCH v4 3/6] drm/i915/skl: Only flush pipes when we change the ddb allocation

Manual pipe flushes are only necessary in order to make sure that we prevent
pipes with changed ddb allocations from overlapping from one another at
any point in time. Additionally, forcing us to wait for the next vblank
every time we have to update the watermark values because the cursor was
moving between screens will introduce a rather noticable lag for users.

Signed-off-by: Lyude <cpaul@redhat.com>
Cc: stable@vger.kernel.org
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/intel_pm.c | 31 +++++++++++++++++++++++++++++--
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1f6fe8c..d65e897 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1597,6 +1597,7 @@ struct skl_ddb_allocation {
 
 struct skl_wm_values {
 	unsigned dirty_pipes;
+	bool ddb_changed;
 	struct skl_ddb_allocation ddb;
 	uint32_t wm_linetime[I915_MAX_PIPES];
 	uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8ae3f2b..cda4196 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3892,6 +3892,12 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
 	new_ddb = &new_values->ddb;
 	cur_ddb = &dev_priv->wm.skl_hw.ddb;
 
+	/* We only ever need to flush when the ddb allocations change */
+	if (!new_values->ddb_changed)
+		return;
+
+	new_values->ddb_changed = false;
+
 	/*
 	 * First pass: flush the pipes with the new allocation contained into
 	 * the old space.
@@ -3996,6 +4002,22 @@ pipes_modified(struct drm_atomic_state *state)
 	return ret;
 }
 
+static bool
+skl_pipe_ddb_changed(struct skl_ddb_allocation *old,
+		     struct skl_ddb_allocation *new,
+		     enum pipe pipe)
+{
+	if (memcmp(&old->pipe[pipe], &new->pipe[pipe],
+		   sizeof(old->pipe[pipe])) != 0 ||
+	    memcmp(&old->plane[pipe], &new->plane[pipe],
+		   sizeof(old->plane[pipe])) != 0 ||
+	    memcmp(&old->y_plane[pipe], &new->y_plane[pipe],
+		   sizeof(old->y_plane[pipe])) != 0)
+		return true;
+
+	return false;
+}
+
 static int
 skl_compute_ddb(struct drm_atomic_state *state)
 {
@@ -4003,7 +4025,8 @@ skl_compute_ddb(struct drm_atomic_state *state)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct intel_crtc *intel_crtc;
-	struct skl_ddb_allocation *ddb = &intel_state->wm_results.ddb;
+	struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
+	struct skl_ddb_allocation *old_ddb = &dev_priv->wm.skl_hw.ddb;
 	uint32_t realloc_pipes = pipes_modified(state);
 	int ret;
 
@@ -4041,9 +4064,13 @@ skl_compute_ddb(struct drm_atomic_state *state)
 		if (IS_ERR(cstate))
 			return PTR_ERR(cstate);
 
-		ret = skl_allocate_pipe_ddb(cstate, ddb);
+		ret = skl_allocate_pipe_ddb(cstate, new_ddb);
 		if (ret)
 			return ret;
+
+		if (!intel_state->wm_results.ddb_changed &&
+		    skl_pipe_ddb_changed(old_ddb, new_ddb, intel_crtc->pipe))
+			intel_state->wm_results.ddb_changed = true;
 	}
 
 	return 0;
-- 
2.7.4

[toc] | [next] | [standalone]

#1451925 — Re: [PATCH v4 3/6] drm/i915/skl: Only flush pipes when we change the ddb allocation

On Tue, Jul 26, 2016 at 01:34:39PM -0400, Lyude wrote:
> Manual pipe flushes are only necessary in order to make sure that we prevent
> pipes with changed ddb allocations from overlapping from one another at
> any point in time. Additionally, forcing us to wait for the next vblank
> every time we have to update the watermark values because the cursor was
> moving between screens will introduce a rather noticable lag for users.
> 
> Signed-off-by: Lyude <cpaul@redhat.com>
> Cc: stable@vger.kernel.org
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c | 31 +++++++++++++++++++++++++++++--
>  2 files changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1f6fe8c..d65e897 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1597,6 +1597,7 @@ struct skl_ddb_allocation {
>  
>  struct skl_wm_values {
>  	unsigned dirty_pipes;
> +	bool ddb_changed;
>  	struct skl_ddb_allocation ddb;
>  	uint32_t wm_linetime[I915_MAX_PIPES];
>  	uint32_t plane[I915_MAX_PIPES][I915_MAX_PLANES][8];
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8ae3f2b..cda4196 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3892,6 +3892,12 @@ static void skl_flush_wm_values(struct drm_i915_private *dev_priv,
>  	new_ddb = &new_values->ddb;
>  	cur_ddb = &dev_priv->wm.skl_hw.ddb;
>  
> +	/* We only ever need to flush when the ddb allocations change */
> +	if (!new_values->ddb_changed)
> +		return;
> +
> +	new_values->ddb_changed = false;
> +
>  	/*
>  	 * First pass: flush the pipes with the new allocation contained into
>  	 * the old space.
> @@ -3996,6 +4002,22 @@ pipes_modified(struct drm_atomic_state *state)
>  	return ret;
>  }
>  
> +static bool
> +skl_pipe_ddb_changed(struct skl_ddb_allocation *old,
> +		     struct skl_ddb_allocation *new,
> +		     enum pipe pipe)
> +{
> +	if (memcmp(&old->pipe[pipe], &new->pipe[pipe],
> +		   sizeof(old->pipe[pipe])) != 0 ||
> +	    memcmp(&old->plane[pipe], &new->plane[pipe],
> +		   sizeof(old->plane[pipe])) != 0 ||
> +	    memcmp(&old->y_plane[pipe], &new->y_plane[pipe],
> +		   sizeof(old->y_plane[pipe])) != 0)
> +		return true;
> +
> +	return false;
> +}
> +
>  static int
>  skl_compute_ddb(struct drm_atomic_state *state)
>  {
> @@ -4003,7 +4025,8 @@ skl_compute_ddb(struct drm_atomic_state *state)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
>  	struct intel_crtc *intel_crtc;
> -	struct skl_ddb_allocation *ddb = &intel_state->wm_results.ddb;
> +	struct skl_ddb_allocation *new_ddb = &intel_state->wm_results.ddb;
> +	struct skl_ddb_allocation *old_ddb = &dev_priv->wm.skl_hw.ddb;
>  	uint32_t realloc_pipes = pipes_modified(state);
>  	int ret;
>  
> @@ -4041,9 +4064,13 @@ skl_compute_ddb(struct drm_atomic_state *state)
>  		if (IS_ERR(cstate))
>  			return PTR_ERR(cstate);
>  
> -		ret = skl_allocate_pipe_ddb(cstate, ddb);
> +		ret = skl_allocate_pipe_ddb(cstate, new_ddb);
>  		if (ret)
>  			return ret;
> +
> +		if (!intel_state->wm_results.ddb_changed &&

I think you can simplify and drop the first part of this condition here,
but in general,

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>


> +		    skl_pipe_ddb_changed(old_ddb, new_ddb, intel_crtc->pipe))
> +			intel_state->wm_results.ddb_changed = true;
>  	}
>  
>  	return 0;
> -- 
> 2.7.4
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web