Re: [PATCH] drm/i915: Don't try to use DDR DVFS on CHV when disabled in the BIOS

[Clint Taylor <clinton.a.taylor@intel.com>]

On 09/08/2015 11:05 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> If one disables DDR DVFS in the BIOS, Punit will apparently ignores
> all DDR DVFS request. Currently we assume that DDR DVFS is always
> operational, which leads to errors in dmesg when the DDR DVFS requests
> time out.
>
> Fix the problem by gently prodding Punit during driver load to find out
> whether it will respond to DDR DVFS requests. If the request times out,
> we assume that DDR DVFS has been permanenly disabled in the BIOS and
> no longer perster the Punit about it.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91629
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h |  2 ++
>   drivers/gpu/drm/i915/intel_pm.c | 42 ++++++++++++++++++++++++++++-------------
>   2 files changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index cf1880e..c2af7d8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1940,6 +1940,8 @@ struct drm_i915_private {
>   			struct skl_wm_values skl_hw;
>   			struct vlv_wm_values vlv;
>   		};
> +
> +		uint8_t max_level;
>   	} wm;
>
>   	struct i915_runtime_pm pm;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 64bc77e..1f6b5bb 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -961,8 +961,6 @@ enum vlv_wm_level {
>   	VLV_WM_LEVEL_PM2,
>   	VLV_WM_LEVEL_PM5,
>   	VLV_WM_LEVEL_DDR_DVFS,
> -	CHV_WM_NUM_LEVELS,
> -	VLV_WM_NUM_LEVELS = 1,
>   };
>
>   /* latency must be in 0.1us units. */
> @@ -988,9 +986,13 @@ static void vlv_setup_wm_latency(struct drm_device *dev)
>   	/* all latencies in usec */
>   	dev_priv->wm.pri_latency[VLV_WM_LEVEL_PM2] = 3;
>
> +	dev_priv->wm.max_level = VLV_WM_LEVEL_PM2;
> +
>   	if (IS_CHERRYVIEW(dev_priv)) {
>   		dev_priv->wm.pri_latency[VLV_WM_LEVEL_PM5] = 12;
>   		dev_priv->wm.pri_latency[VLV_WM_LEVEL_DDR_DVFS] = 33;
> +
> +		dev_priv->wm.max_level = VLV_WM_LEVEL_DDR_DVFS;
>   	}
>   }
>
> @@ -1143,10 +1145,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
>   	memset(wm_state, 0, sizeof(*wm_state));
>
>   	wm_state->cxsr = crtc->pipe != PIPE_C && crtc->wm.cxsr_allowed;
> -	if (IS_CHERRYVIEW(dev))
> -		wm_state->num_levels = CHV_WM_NUM_LEVELS;
> -	else
> -		wm_state->num_levels = VLV_WM_NUM_LEVELS;
> +	wm_state->num_levels = to_i915(dev)->wm.max_level + 1;
>
>   	wm_state->num_active_planes = 0;
>
> @@ -1226,7 +1225,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
>   	}
>
>   	/* clear any (partially) filled invalid levels */
> -	for (level = wm_state->num_levels; level < CHV_WM_NUM_LEVELS; level++) {
> +	for (level = wm_state->num_levels; level < to_i915(dev)->wm.max_level + 1; level++) {
>   		memset(&wm_state->wm[level], 0, sizeof(wm_state->wm[level]));
>   		memset(&wm_state->sr[level], 0, sizeof(wm_state->sr[level]));
>   	}
> @@ -1330,10 +1329,7 @@ static void vlv_merge_wm(struct drm_device *dev,
>   	struct intel_crtc *crtc;
>   	int num_active_crtcs = 0;
>
> -	if (IS_CHERRYVIEW(dev))
> -		wm->level = VLV_WM_LEVEL_DDR_DVFS;
> -	else
> -		wm->level = VLV_WM_LEVEL_PM2;
> +	wm->level = to_i915(dev)->wm.max_level;
>   	wm->cxsr = true;
>
>   	for_each_intel_crtc(dev, crtc) {
> @@ -4090,9 +4086,29 @@ void vlv_wm_get_hw_state(struct drm_device *dev)
>   		if (val & DSP_MAXFIFO_PM5_ENABLE)
>   			wm->level = VLV_WM_LEVEL_PM5;
>
> +		/*
> +		 * If DDR DVFS is disabled in the BIOS, Punit
> +		 * will never ack the request. So if that happens
> +		 * assume we don't have to enable/disable DDR DVFS
> +		 * dynamically. To test that just set the REQ_ACK
> +		 * bit to poke the Punit, but don't change the
> +		 * HIGH/LOW bits so that we don't actually change
> +		 * the current state.
> +		 */
>   		val = vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2);
> -		if ((val & FORCE_DDR_HIGH_FREQ) == 0)
> -			wm->level = VLV_WM_LEVEL_DDR_DVFS;
> +		val |= FORCE_DDR_FREQ_REQ_ACK;
> +		vlv_punit_write(dev_priv, PUNIT_REG_DDR_SETUP2, val);
> +
> +		if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2) &
> +			      FORCE_DDR_FREQ_REQ_ACK) == 0, 3)) {
> +			DRM_DEBUG_KMS("Punit not acking DDR DVFS request, "
> +				      "assuming DDR DVFS is disabled\n");
> +			dev_priv->wm.max_level = VLV_WM_LEVEL_PM5;
> +		} else {
> +			val = vlv_punit_read(dev_priv, PUNIT_REG_DDR_SETUP2);
> +			if ((val & FORCE_DDR_HIGH_FREQ) == 0)
> +				wm->level = VLV_WM_LEVEL_DDR_DVFS;
> +		}
>
>   		mutex_unlock(&dev_priv->rps.hw_lock);
>   	}
>

Nice.

Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com>
Tested-by: Clint Taylor <Clinton.A.Taylor@intel.com>

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