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

[Jani Nikula]

On Tue, 08 Sep 2015, Clint Taylor <clinton.a.taylor at intel.com> wrote:
> On 09/08/2015 11:05 AM, ville.syrjala at linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala at 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 at 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 at intel.com>
> Tested-by: Clint Taylor <Clinton.A.Taylor at intel.com>

Pushed to drm-intel-next-fixes, thanks for the patch and review.

BR,
Jani.


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

-- 
Jani Nikula, Intel Open Source Technology Center