[Intel-gfx] [PATCH] drm/i915: Add two-stage ILK-style watermark programming (v11)

Matt Roper matthew.d.roper at intel.com
Thu Feb 25 16:48:20 UTC 2016


I never got a CI email for this (probably because fdo was down for a while),
but I can see the results below in patchwork

>  Test gem_cs_prefetch:
>          Subgroup basic-default:
>                  incomplete -> PASS       (ilk-hp8440p)
>  Test kms_flip:
>          Subgroup basic-flip-vs-dpms:
>                  pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE

Pre-existing watermark bug here:
        https://bugs.freedesktop.org/show_bug.cgi?id=93787

I had hoped this patch might also fix that problem, but I guess further
investigation will be needed; it seems to be something very
ILK-specific, not seen on SNB, IVB, etc.


>  Test kms_force_connector_basic:
>          Subgroup force-edid:
>                  skip       -> PASS       (snb-x220t)
>                  pass       -> SKIP       (ivb-t430s)

Pre-existing:
        https://bugs.freedesktop.org/show_bug.cgi?id=93769

>          Subgroup force-load-detect:
>                  dmesg-fail -> FAIL       (snb-x220t)
>                  dmesg-fail -> FAIL       (hsw-gt2)

The dmesg warning is gone now (a locking warning that I don't think my
patch should have affected); the remaining test failure is identical to
what it was before (temp->connection != DRM_MODE_UNKNOWNCONNECTION) and
unrelated.

I don't see a bugzilla for this, which seems strange since it's a
pre-existing defect.  Am I overlooking it or do I need to file a new
one?

>                  fail       -> SKIP       (ilk-hp8440p)

Not sure what is actually plugged into the CI machine, so the skip may
be legitimate/expected?  Doesn't seem related to anything in my patch
anyway.

>  Test kms_pipe_crc_basic:
>          Subgroup suspend-read-crc-pipe-c:
>                  pass       -> DMESG-WARN (bsw-nuc-2)

This one was happening sporadically before my patch; I think it's the
same bug filed here:

  https://bugs.freedesktop.org/show_bug.cgi?id=94164

I'll update the bugzilla to indicate it also happens on this test.

>  Test pm_rpm:
>          Subgroup basic-rte:
>                  pass       -> DMESG-WARN (bsw-nuc-2)
>                  fail       -> PASS       (hsw-brixbox)

Same bug mentioned above (94164)



Matt


On Tue, Feb 23, 2016 at 05:20:13PM -0800, Matt Roper wrote:
> In addition to calculating final watermarks, let's also pre-calculate a
> set of intermediate watermark values at atomic check time.  These
> intermediate watermarks are a combination of the watermarks for the old
> state and the new state; they should satisfy the requirements of both
> states which means they can be programmed immediately when we commit the
> atomic state (without waiting for a vblank).  Once the vblank does
> happen, we can then re-program watermarks to the more optimal final
> value.
> 
> v2: Significant rebasing/rewriting.
> 
> v3:
>  - Move 'need_postvbl_update' flag to CRTC state (Daniel)
>  - Don't forget to check intermediate watermark values for validity
>    (Maarten)
>  - Don't due async watermark optimization; just do it at the end of the
>    atomic transaction, after waiting for vblanks.  We do want it to be
>    async eventually, but adding that now will cause more trouble for
>    Maarten's in-progress work.  (Maarten)
>  - Don't allocate space in crtc_state for intermediate watermarks on
>    platforms that don't need it (gen9+).
>  - Move WaCxSRDisabledForSpriteScaling:ivb into intel_begin_crtc_commit
>    now that ilk_update_wm is gone.
> 
> v4:
>  - Add a wm_mutex to cover updates to intel_crtc->active and the
>    need_postvbl_update flag.  Since we don't have async yet it isn't
>    terribly important yet, but might as well add it now.
>  - Change interface to program watermarks.  Platforms will now expose
>    .initial_watermarks() and .optimize_watermarks() functions to do
>    watermark programming.  These should lock wm_mutex, copy the
>    appropriate state values into intel_crtc->active, and then call
>    the internal program watermarks function.
> 
> v5:
>  - Skip intermediate watermark calculation/check during initial hardware
>    readout since we don't trust the existing HW values (and don't have
>    valid values of our own yet).
>  - Don't try to call .optimize_watermarks() on platforms that don't have
>    atomic watermarks yet.  (Maarten)
> 
> v6:
>  - Rebase
> 
> v7:
>  - Further rebase
> 
> v8:
>  - A few minor indentation and line length fixes
> 
> v9:
>  - Yet another rebase since Maarten's patches reworked a bunch of the
>    code (wm_pre, wm_post, etc.) that this was previously based on.
> 
> v10:
>  - Move wm_mutex to dev_priv to protect against racing commits against
>    disjoint CRTC sets. (Maarten)
>  - Drop unnecessary clearing of cstate->wm.need_postvbl_update (Maarten)
> 
> v11:
>  - Now that we've moved to atomic watermark updates, make sure we call
>    the proper function to program watermarks in
>    {ironlake,haswell}_crtc_enable(); the failure to do so on the
>    previous patch iteration led to us not actually programming the
>    watermarks before turning on the CRTC, which was the cause of the
>    underruns that the CI system was seeing.
>  - Fix inverted logic for determining when to optimize watermarks.  We
>    were needlessly optimizing when the intermediate/optimal values were
>    the same (harmless), but not actually optimizing when they differed
>    (also harmless, but wasteful from a power/bandwidth perspective).
> 
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c      |   1 +
>  drivers/gpu/drm/i915/i915_drv.h      |  13 ++-
>  drivers/gpu/drm/i915/intel_atomic.c  |   1 +
>  drivers/gpu/drm/i915/intel_display.c |  97 +++++++++++++++++++--
>  drivers/gpu/drm/i915/intel_drv.h     |  28 +++++-
>  drivers/gpu/drm/i915/intel_pm.c      | 162 ++++++++++++++++++++++++-----------
>  6 files changed, 244 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 1c6d227..36c0cf1 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1010,6 +1010,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>  	mutex_init(&dev_priv->sb_lock);
>  	mutex_init(&dev_priv->modeset_restore_lock);
>  	mutex_init(&dev_priv->av_mutex);
> +	mutex_init(&dev_priv->wm.wm_mutex);
>  
>  	ret = i915_workqueues_init(dev_priv);
>  	if (ret < 0)
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9e76bfc..cf17d62 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -631,7 +631,11 @@ struct drm_i915_display_funcs {
>  			  struct dpll *best_clock);
>  	int (*compute_pipe_wm)(struct intel_crtc *crtc,
>  			       struct drm_atomic_state *state);
> -	void (*program_watermarks)(struct intel_crtc_state *cstate);
> +	int (*compute_intermediate_wm)(struct drm_device *dev,
> +				       struct intel_crtc *intel_crtc,
> +				       struct intel_crtc_state *newstate);
> +	void (*initial_watermarks)(struct intel_crtc_state *cstate);
> +	void (*optimize_watermarks)(struct intel_crtc_state *cstate);
>  	void (*update_wm)(struct drm_crtc *crtc);
>  	int (*modeset_calc_cdclk)(struct drm_atomic_state *state);
>  	void (*modeset_commit_cdclk)(struct drm_atomic_state *state);
> @@ -1980,6 +1984,13 @@ struct drm_i915_private {
>  		};
>  
>  		uint8_t max_level;
> +
> +		/*
> +		 * Should be held around atomic WM register writing; also
> +		 * protects * intel_crtc->wm.active and
> +		 * cstate->wm.need_postvbl_update.
> +		 */
> +		struct mutex wm_mutex;
>  	} wm;
>  
>  	struct i915_runtime_pm pm;
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> index 4625f8a..9682d94 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -97,6 +97,7 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>  	crtc_state->disable_lp_wm = false;
>  	crtc_state->disable_cxsr = false;
>  	crtc_state->wm_changed = false;
> +	crtc_state->wm.need_postvbl_update = false;
>  
>  	return &crtc_state->base;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index deee560..423b99e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4846,7 +4846,42 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
>  			intel_set_memory_cxsr(dev_priv, false);
>  	}
>  
> -	if (!needs_modeset(&pipe_config->base) && pipe_config->wm_changed)
> +	/*
> +	 * IVB workaround: must disable low power watermarks for at least
> +	 * one frame before enabling scaling.  LP watermarks can be re-enabled
> +	 * when scaling is disabled.
> +	 *
> +	 * WaCxSRDisabledForSpriteScaling:ivb
> +	 */
> +	if (pipe_config->disable_lp_wm) {
> +		ilk_disable_lp_wm(dev);
> +		intel_wait_for_vblank(dev, crtc->pipe);
> +	}
> +
> +	/*
> +	 * If we're doing a modeset, we're done.  No need to do any pre-vblank
> +	 * watermark programming here.
> +	 */
> +	if (needs_modeset(&pipe_config->base))
> +		return;
> +
> +	/*
> +	 * For platforms that support atomic watermarks, program the
> +	 * 'intermediate' watermarks immediately.  On pre-gen9 platforms, these
> +	 * will be the intermediate values that are safe for both pre- and
> +	 * post- vblank; when vblank happens, the 'active' values will be set
> +	 * to the final 'target' values and we'll do this again to get the
> +	 * optimal watermarks.  For gen9+ platforms, the values we program here
> +	 * will be the final target values which will get automatically latched
> +	 * at vblank time; no further programming will be necessary.
> +	 *
> +	 * If a platform hasn't been transitioned to atomic watermarks yet,
> +	 * we'll continue to update watermarks the old way, if flags tell
> +	 * us to.
> +	 */
> +	if (dev_priv->display.initial_watermarks != NULL)
> +		dev_priv->display.initial_watermarks(pipe_config);
> +	else if (pipe_config->wm_changed)
>  		intel_update_watermarks(&crtc->base);
>  }
>  
> @@ -4925,7 +4960,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  	 */
>  	intel_crtc_load_lut(crtc);
>  
> -	intel_update_watermarks(crtc);
> +	dev_priv->display.initial_watermarks(intel_crtc->config);
>  	intel_enable_pipe(intel_crtc);
>  
>  	if (intel_crtc->config->has_pch_encoder)
> @@ -5024,7 +5059,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  	if (!intel_crtc->config->has_dsi_encoder)
>  		intel_ddi_enable_transcoder_func(crtc);
>  
> -	intel_update_watermarks(crtc);
> +	dev_priv->display.initial_watermarks(pipe_config);
>  	intel_enable_pipe(intel_crtc);
>  
>  	if (intel_crtc->config->has_pch_encoder)
> @@ -11800,6 +11835,7 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct drm_plane *plane = plane_state->plane;
>  	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct intel_plane_state *old_plane_state =
>  		to_intel_plane_state(plane->state);
>  	int idx = intel_crtc->base.base.id, ret;
> @@ -11859,6 +11895,11 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  		pipe_config->wm_changed = true;
>  	}
>  
> +	/* Pre-gen9 platforms need two-step watermark updates */
> +	if (pipe_config->wm_changed && INTEL_INFO(dev)->gen < 9 &&
> +	    dev_priv->display.optimize_watermarks)
> +		to_intel_crtc_state(crtc_state)->wm.need_postvbl_update = true;
> +
>  	if (visible || was_visible)
>  		intel_crtc->atomic.fb_bits |=
>  			to_intel_plane(plane)->frontbuffer_bit;
> @@ -11983,8 +12024,29 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
>  	ret = 0;
>  	if (dev_priv->display.compute_pipe_wm) {
>  		ret = dev_priv->display.compute_pipe_wm(intel_crtc, state);
> -		if (ret)
> +		if (ret) {
> +			DRM_DEBUG_KMS("Target pipe watermarks are invalid\n");
>  			return ret;
> +		}
> +	}
> +
> +	if (dev_priv->display.compute_intermediate_wm &&
> +	    !to_intel_atomic_state(state)->skip_intermediate_wm) {
> +		if (WARN_ON(!dev_priv->display.compute_pipe_wm))
> +			return 0;
> +
> +		/*
> +		 * Calculate 'intermediate' watermarks that satisfy both the
> +		 * old state and the new state.  We can program these
> +		 * immediately.
> +		 */
> +		ret = dev_priv->display.compute_intermediate_wm(crtc->dev,
> +								intel_crtc,
> +								pipe_config);
> +		if (ret) {
> +			DRM_DEBUG_KMS("No valid intermediate pipe watermarks are possible\n");
> +			return ret;
> +		}
>  	}
>  
>  	if (INTEL_INFO(dev)->gen >= 9) {
> @@ -13452,6 +13514,7 @@ static int intel_atomic_commit(struct drm_device *dev,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_crtc *crtc;
> +	struct intel_crtc_state *intel_cstate;
>  	int ret = 0, i;
>  	bool hw_check = intel_state->modeset;
>  
> @@ -13555,6 +13618,20 @@ static int intel_atomic_commit(struct drm_device *dev,
>  
>  	drm_atomic_helper_wait_for_vblanks(dev, state);
>  
> +	/*
> +	 * Now that the vblank has passed, we can go ahead and program the
> +	 * optimal watermarks on platforms that need two-step watermark
> +	 * programming.
> +	 *
> +	 * TODO: Move this (and other cleanup) to an async worker eventually.
> +	 */
> +	for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +		intel_cstate = to_intel_crtc_state(crtc->state);
> +
> +		if (dev_priv->display.optimize_watermarks)
> +			dev_priv->display.optimize_watermarks(intel_cstate);
> +	}
> +
>  	mutex_lock(&dev->struct_mutex);
>  	drm_atomic_helper_cleanup_planes(dev, state);
>  	mutex_unlock(&dev->struct_mutex);
> @@ -15225,7 +15302,7 @@ static void sanitize_watermarks(struct drm_device *dev)
>  	int i;
>  
>  	/* Only supported on platforms that use atomic watermark design */
> -	if (!dev_priv->display.program_watermarks)
> +	if (!dev_priv->display.optimize_watermarks)
>  		return;
>  
>  	/*
> @@ -15246,6 +15323,13 @@ retry:
>  	if (WARN_ON(IS_ERR(state)))
>  		goto fail;
>  
> +	/*
> +	 * Hardware readout is the only time we don't want to calculate
> +	 * intermediate watermarks (since we don't trust the current
> +	 * watermarks).
> +	 */
> +	to_intel_atomic_state(state)->skip_intermediate_wm = true;
> +
>  	ret = intel_atomic_check(dev, state);
>  	if (ret) {
>  		/*
> @@ -15268,7 +15352,8 @@ retry:
>  	for_each_crtc_in_state(state, crtc, cstate, i) {
>  		struct intel_crtc_state *cs = to_intel_crtc_state(cstate);
>  
> -		dev_priv->display.program_watermarks(cs);
> +		cs->wm.need_postvbl_update = true;
> +		dev_priv->display.optimize_watermarks(cs);
>  	}
>  
>  	drm_atomic_state_free(state);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4852049..a9f7641 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -260,6 +260,12 @@ struct intel_atomic_state {
>  
>  	struct intel_shared_dpll_config shared_dpll[I915_NUM_PLLS];
>  	struct intel_wm_config wm_config;
> +
> +	/*
> +	 * Current watermarks can't be trusted during hardware readout, so
> +	 * don't bother calculating intermediate watermarks.
> +	 */
> +	bool skip_intermediate_wm;
>  };
>  
>  struct intel_plane_state {
> @@ -509,13 +515,29 @@ struct intel_crtc_state {
>  
>  	struct {
>  		/*
> -		 * optimal watermarks, programmed post-vblank when this state
> -		 * is committed
> +		 * Optimal watermarks, programmed post-vblank when this state
> +		 * is committed.
>  		 */
>  		union {
>  			struct intel_pipe_wm ilk;
>  			struct skl_pipe_wm skl;
>  		} optimal;
> +
> +		/*
> +		 * Intermediate watermarks; these can be programmed immediately
> +		 * since they satisfy both the current configuration we're
> +		 * switching away from and the new configuration we're switching
> +		 * to.
> +		 */
> +		struct intel_pipe_wm intermediate;
> +
> +		/*
> +		 * Platforms with two-step watermark programming will need to
> +		 * update watermark programming post-vblank to switch from the
> +		 * safe intermediate watermarks to the optimal final
> +		 * watermarks.
> +		 */
> +		bool need_postvbl_update;
>  	} wm;
>  };
>  
> @@ -601,6 +623,7 @@ struct intel_crtc {
>  			struct intel_pipe_wm ilk;
>  			struct skl_pipe_wm skl;
>  		} active;
> +
>  		/* allow CxSR on this pipe */
>  		bool cxsr_allowed;
>  	} wm;
> @@ -1566,6 +1589,7 @@ void skl_wm_get_hw_state(struct drm_device *dev);
>  void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv,
>  			  struct skl_ddb_allocation *ddb /* out */);
>  uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config);
> +bool ilk_disable_lp_wm(struct drm_device *dev);
>  int sanitize_rc6_option(const struct drm_device *dev, int enable_rc6);
>  
>  /* intel_sdvo.c */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 347d4df..ccdb581 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2278,6 +2278,29 @@ static void skl_setup_wm_latency(struct drm_device *dev)
>  	intel_print_wm_latency(dev, "Gen9 Plane", dev_priv->wm.skl_latency);
>  }
>  
> +static bool ilk_validate_pipe_wm(struct drm_device *dev,
> +				 struct intel_pipe_wm *pipe_wm)
> +{
> +	/* LP0 watermark maximums depend on this pipe alone */
> +	const struct intel_wm_config config = {
> +		.num_pipes_active = 1,
> +		.sprites_enabled = pipe_wm->sprites_enabled,
> +		.sprites_scaled = pipe_wm->sprites_scaled,
> +	};
> +	struct ilk_wm_maximums max;
> +
> +	/* LP0 watermarks always use 1/2 DDB partitioning */
> +	ilk_compute_wm_maximums(dev, 0, &config, INTEL_DDB_PART_1_2, &max);
> +
> +	/* At least LP0 must be valid */
> +	if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0])) {
> +		DRM_DEBUG_KMS("LP0 watermark invalid\n");
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  /* Compute new watermarks for the pipe */
>  static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
>  			       struct drm_atomic_state *state)
> @@ -2292,10 +2315,6 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
>  	struct intel_plane_state *sprstate = NULL;
>  	struct intel_plane_state *curstate = NULL;
>  	int level, max_level = ilk_wm_max_level(dev);
> -	/* LP0 watermark maximums depend on this pipe alone */
> -	struct intel_wm_config config = {
> -		.num_pipes_active = 1,
> -	};
>  	struct ilk_wm_maximums max;
>  
>  	cstate = intel_atomic_get_crtc_state(state, intel_crtc);
> @@ -2319,21 +2338,18 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
>  			curstate = to_intel_plane_state(ps);
>  	}
>  
> -	config.sprites_enabled = sprstate->visible;
> -	config.sprites_scaled = sprstate->visible &&
> +	pipe_wm->pipe_enabled = cstate->base.active;
> +	pipe_wm->sprites_enabled = sprstate->visible;
> +	pipe_wm->sprites_scaled = sprstate->visible &&
>  		(drm_rect_width(&sprstate->dst) != drm_rect_width(&sprstate->src) >> 16 ||
>  		drm_rect_height(&sprstate->dst) != drm_rect_height(&sprstate->src) >> 16);
>  
> -	pipe_wm->pipe_enabled = cstate->base.active;
> -	pipe_wm->sprites_enabled = config.sprites_enabled;
> -	pipe_wm->sprites_scaled = config.sprites_scaled;
> -
>  	/* ILK/SNB: LP2+ watermarks only w/o sprites */
>  	if (INTEL_INFO(dev)->gen <= 6 && sprstate->visible)
>  		max_level = 1;
>  
>  	/* ILK/SNB/IVB: LP1+ watermarks only w/o scaling */
> -	if (config.sprites_scaled)
> +	if (pipe_wm->sprites_scaled)
>  		max_level = 0;
>  
>  	ilk_compute_wm_level(dev_priv, intel_crtc, 0, cstate,
> @@ -2342,12 +2358,8 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
>  	if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>  		pipe_wm->linetime = hsw_compute_linetime_wm(dev, cstate);
>  
> -	/* LP0 watermarks always use 1/2 DDB partitioning */
> -	ilk_compute_wm_maximums(dev, 0, &config, INTEL_DDB_PART_1_2, &max);
> -
> -	/* At least LP0 must be valid */
> -	if (!ilk_validate_wm_level(0, &max, &pipe_wm->wm[0]))
> -		return -EINVAL;
> +	if (!ilk_validate_pipe_wm(dev, pipe_wm))
> +		return false;
>  
>  	ilk_compute_wm_reg_maximums(dev, 1, &max);
>  
> @@ -2372,6 +2384,59 @@ static int ilk_compute_pipe_wm(struct intel_crtc *intel_crtc,
>  }
>  
>  /*
> + * Build a set of 'intermediate' watermark values that satisfy both the old
> + * state and the new state.  These can be programmed to the hardware
> + * immediately.
> + */
> +static int ilk_compute_intermediate_wm(struct drm_device *dev,
> +				       struct intel_crtc *intel_crtc,
> +				       struct intel_crtc_state *newstate)
> +{
> +	struct intel_pipe_wm *a = &newstate->wm.intermediate;
> +	struct intel_pipe_wm *b = &intel_crtc->wm.active.ilk;
> +	int level, max_level = ilk_wm_max_level(dev);
> +
> +	/*
> +	 * Start with the final, target watermarks, then combine with the
> +	 * currently active watermarks to get values that are safe both before
> +	 * and after the vblank.
> +	 */
> +	*a = newstate->wm.optimal.ilk;
> +	a->pipe_enabled |= b->pipe_enabled;
> +	a->sprites_enabled |= b->sprites_enabled;
> +	a->sprites_scaled |= b->sprites_scaled;
> +
> +	for (level = 0; level <= max_level; level++) {
> +		struct intel_wm_level *a_wm = &a->wm[level];
> +		const struct intel_wm_level *b_wm = &b->wm[level];
> +
> +		a_wm->enable &= b_wm->enable;
> +		a_wm->pri_val = max(a_wm->pri_val, b_wm->pri_val);
> +		a_wm->spr_val = max(a_wm->spr_val, b_wm->spr_val);
> +		a_wm->cur_val = max(a_wm->cur_val, b_wm->cur_val);
> +		a_wm->fbc_val = max(a_wm->fbc_val, b_wm->fbc_val);
> +	}
> +
> +	/*
> +	 * We need to make sure that these merged watermark values are
> +	 * actually a valid configuration themselves.  If they're not,
> +	 * there's no safe way to transition from the old state to
> +	 * the new state, so we need to fail the atomic transaction.
> +	 */
> +	if (!ilk_validate_pipe_wm(dev, a))
> +		return -EINVAL;
> +
> +	/*
> +	 * If our intermediate WM are identical to the final WM, then we can
> +	 * omit the post-vblank programming; only update if it's different.
> +	 */
> +	if (memcmp(a, &newstate->wm.optimal.ilk, sizeof(*a)) == 0)
> +		newstate->wm.need_postvbl_update = false;
> +
> +	return 0;
> +}
> +
> +/*
>   * Merge the watermarks from all active pipes for a specific level.
>   */
>  static void ilk_merge_wm_level(struct drm_device *dev,
> @@ -2383,9 +2448,7 @@ static void ilk_merge_wm_level(struct drm_device *dev,
>  	ret_wm->enable = true;
>  
>  	for_each_intel_crtc(dev, intel_crtc) {
> -		const struct intel_crtc_state *cstate =
> -			to_intel_crtc_state(intel_crtc->base.state);
> -		const struct intel_pipe_wm *active = &cstate->wm.optimal.ilk;
> +		const struct intel_pipe_wm *active = &intel_crtc->wm.active.ilk;
>  		const struct intel_wm_level *wm = &active->wm[level];
>  
>  		if (!active->pipe_enabled)
> @@ -2533,15 +2596,14 @@ static void ilk_compute_wm_results(struct drm_device *dev,
>  
>  	/* LP0 register values */
>  	for_each_intel_crtc(dev, intel_crtc) {
> -		const struct intel_crtc_state *cstate =
> -			to_intel_crtc_state(intel_crtc->base.state);
>  		enum pipe pipe = intel_crtc->pipe;
> -		const struct intel_wm_level *r = &cstate->wm.optimal.ilk.wm[0];
> +		const struct intel_wm_level *r =
> +			&intel_crtc->wm.active.ilk.wm[0];
>  
>  		if (WARN_ON(!r->enable))
>  			continue;
>  
> -		results->wm_linetime[pipe] = cstate->wm.optimal.ilk.linetime;
> +		results->wm_linetime[pipe] = intel_crtc->wm.active.ilk.linetime;
>  
>  		results->wm_pipe[pipe] =
>  			(r->pri_val << WM0_PIPE_PLANE_SHIFT) |
> @@ -2748,7 +2810,7 @@ static void ilk_write_wm_values(struct drm_i915_private *dev_priv,
>  	dev_priv->wm.hw = *results;
>  }
>  
> -static bool ilk_disable_lp_wm(struct drm_device *dev)
> +bool ilk_disable_lp_wm(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> @@ -3643,11 +3705,9 @@ static void ilk_compute_wm_config(struct drm_device *dev,
>  	}
>  }
>  
> -static void ilk_program_watermarks(struct intel_crtc_state *cstate)
> +static void ilk_program_watermarks(struct drm_i915_private *dev_priv)
>  {
> -	struct drm_crtc *crtc = cstate->base.crtc;
> -	struct drm_device *dev = crtc->dev;
> -	struct drm_i915_private *dev_priv = to_i915(dev);
> +	struct drm_device *dev = dev_priv->dev;
>  	struct intel_pipe_wm lp_wm_1_2 = {}, lp_wm_5_6 = {}, *best_lp_wm;
>  	struct ilk_wm_maximums max;
>  	struct intel_wm_config config = {};
> @@ -3678,28 +3738,28 @@ static void ilk_program_watermarks(struct intel_crtc_state *cstate)
>  	ilk_write_wm_values(dev_priv, &results);
>  }
>  
> -static void ilk_update_wm(struct drm_crtc *crtc)
> +static void ilk_initial_watermarks(struct intel_crtc_state *cstate)
>  {
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct intel_crtc_state *cstate = to_intel_crtc_state(crtc->state);
> -
> -	WARN_ON(cstate->base.active != intel_crtc->active);
> +	struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
>  
> -	/*
> -	 * IVB workaround: must disable low power watermarks for at least
> -	 * one frame before enabling scaling.  LP watermarks can be re-enabled
> -	 * when scaling is disabled.
> -	 *
> -	 * WaCxSRDisabledForSpriteScaling:ivb
> -	 */
> -	if (cstate->disable_lp_wm) {
> -		ilk_disable_lp_wm(crtc->dev);
> -		intel_wait_for_vblank(crtc->dev, intel_crtc->pipe);
> -	}
> +	mutex_lock(&dev_priv->wm.wm_mutex);
> +	intel_crtc->wm.active.ilk = cstate->wm.intermediate;
> +	ilk_program_watermarks(dev_priv);
> +	mutex_unlock(&dev_priv->wm.wm_mutex);
> +}
>  
> -	intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk;
> +static void ilk_optimize_watermarks(struct intel_crtc_state *cstate)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(cstate->base.crtc->dev);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(cstate->base.crtc);
>  
> -	ilk_program_watermarks(cstate);
> +	mutex_lock(&dev_priv->wm.wm_mutex);
> +	if (cstate->wm.need_postvbl_update) {
> +		intel_crtc->wm.active.ilk = cstate->wm.optimal.ilk;
> +		ilk_program_watermarks(dev_priv);
> +	}
> +	mutex_unlock(&dev_priv->wm.wm_mutex);
>  }
>  
>  static void skl_pipe_wm_active_state(uint32_t val,
> @@ -7076,9 +7136,13 @@ void intel_init_pm(struct drm_device *dev)
>  		     dev_priv->wm.spr_latency[1] && dev_priv->wm.cur_latency[1]) ||
>  		    (!IS_GEN5(dev) && dev_priv->wm.pri_latency[0] &&
>  		     dev_priv->wm.spr_latency[0] && dev_priv->wm.cur_latency[0])) {
> -			dev_priv->display.update_wm = ilk_update_wm;
>  			dev_priv->display.compute_pipe_wm = ilk_compute_pipe_wm;
> -			dev_priv->display.program_watermarks = ilk_program_watermarks;
> +			dev_priv->display.compute_intermediate_wm =
> +				ilk_compute_intermediate_wm;
> +			dev_priv->display.initial_watermarks =
> +				ilk_initial_watermarks;
> +			dev_priv->display.optimize_watermarks =
> +				ilk_optimize_watermarks;
>  		} else {
>  			DRM_DEBUG_KMS("Failed to read display plane latency. "
>  				      "Disable CxSR\n");
> -- 
> 2.1.4
> 

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


More information about the Intel-gfx mailing list