Re: [PATCH 27/71] drm/i915/chv: Enable Render Standby (RC6) for Cheeryview

[Deepak S <deepak.s@linux.intel.com>]

Thanks for the feedback. I will address all the comments
and I will post cherryview rc6/turbo updated patches within couple of days.

I think the patches need little bit of cleanup.

Thanks
Deepak


On Thursday 10 April 2014 10:36 PM, Ville Syrjälä wrote:
> On Thu, Apr 10, 2014 at 07:51:03PM +0300, Jani Nikula wrote:
>> On Wed, 09 Apr 2014, ville.syrjala@linux.intel.com wrote:
>>> From: Deepak S <deepak.s@intel.com>
>>>
>>> v2: Configure PCBR if BIOS fails allocate pcbr (deepak)
>>>
>>> Signed-off-by: Deepak S <deepak.s@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_pm.c | 101 ++++++++++++++++++++++++++++++++++++++--
>>>   1 file changed, 98 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index 0889af7..909cc0a 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -3184,6 +3184,18 @@ static void gen6_disable_rps(struct drm_device *dev)
>>>   	gen6_disable_rps_interrupts(dev);
>>>   }
>>>   
>>> +static void cherryview_disable_rps(struct drm_device *dev)
>>> +{
>>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>>> +
>>> +	I915_WRITE(GEN6_RC_CONTROL, 0);
>>> +
>>> +	if (dev_priv->vlv_pctx) {
>>> +		drm_gem_object_unreference(&dev_priv->vlv_pctx->base);
>>> +		dev_priv->vlv_pctx = NULL;
>>> +	}
>>> +}
>>> +
>>>   static void valleyview_disable_rps(struct drm_device *dev)
>>>   {
>>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>> @@ -3551,6 +3563,29 @@ int valleyview_rps_min_freq(struct drm_i915_private *dev_priv)
>>>   	return vlv_punit_read(dev_priv, PUNIT_REG_GPU_LFM) & 0xff;
>>>   }
>>>   
>>> +static void cherryview_setup_pctx(struct drm_device *dev)
>>> +{
>>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>>> +	unsigned long pctx_paddr;
>>> +	struct i915_gtt *gtt = &dev_priv->gtt;
>>> +	u32 pcbr;
>>> +	int pctx_size = 32*1024;
>>> +
>>> +	pcbr = I915_READ(VLV_PCBR);
>>> +	if (pcbr >> 12 == 0) {
>>> +		/*
>>> +		 * From the Gunit register HAS:
>>> +		 * The Gfx driver is expected to program this register and ensure
>>> +		 * proper allocation within Gfx stolen memory.  For example, this
>>> +		 * register should be programmed such than the PCBR range does not
>>> +		 * overlap with other relevant ranges.
>>> +		 */
>>> +		pctx_paddr = (dev_priv->mm.stolen_base + gtt->stolen_size - pctx_size);
>>> +		I915_WRITE(VLV_PCBR, pctx_paddr);
>>> +	}
>>> +}
>>> +
>>> +
>>>   static void valleyview_setup_pctx(struct drm_device *dev)
>>>   {
>>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>> @@ -3595,6 +3630,61 @@ out:
>>>   	dev_priv->vlv_pctx = pctx;
>>>   }
>>>   
>>> +static void cherryview_enable_rps(struct drm_device *dev)
>>> +{
>>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>>> +	struct intel_ring_buffer *ring;
>>> +	u32 gtfifodbg, rc6_mode = 0, pcbr;
>>> +	int i;
>>> +
>>> +	WARN_ON(!mutex_is_locked(&dev_priv->rps.hw_lock));
>>> +
>>> +	if ((gtfifodbg = I915_READ(GTFIFODBG))) {
>> Please no assignment within if; this one's easy to split.
>>
>> There's a bunch of other checkpatch issues in the series; I don't
>> personally care about most of them but you might want to run it and see
>> if you want to do something about it.
> Looks like it's a straight up copy-paste from the gen6 and vlv code. So
> someone might want to clean those out as well.
>
> And maybe we should just drop this check for CHV since the GT wake FIFO
> isn't used anymore. But I'm not sure if the register still hold something
> sensible or not.
>
>> BR,
>> Jani.
>>
>>
>>> +		DRM_DEBUG_DRIVER("GT fifo had a previous error %x\n",
>>> +				 gtfifodbg);
>>> +		I915_WRITE(GTFIFODBG, gtfifodbg);
>>> +	}
>>> +
>>> +	cherryview_setup_pctx(dev);
>>> +
>>> +	/* 1a & 1b: Get forcewake during program sequence. Although the driver
>>> +	 * hasn't enabled a state yet where we need forcewake, BIOS may have.*/
>>> +	gen6_gt_force_wake_get(dev_priv, FORCEWAKE_ALL);
>>> +
>>> +	/* 2a: Program RC6 thresholds.*/
>>> +	I915_WRITE(GEN6_RC6_WAKE_RATE_LIMIT, 40 << 16);
>>> +	I915_WRITE(GEN6_RC_EVALUATION_INTERVAL, 125000); /* 12500 * 1280ns */
>>> +	I915_WRITE(GEN6_RC_IDLE_HYSTERSIS, 25); /* 25 * 1280ns */
>>> +
>>> +	for_each_ring(ring, dev_priv, i)
>>> +		I915_WRITE(RING_MAX_IDLE(ring->mmio_base), 10);
>>> +
>>> +	I915_WRITE(GEN6_RC6_THRESHOLD, 50000); /* 50/125ms per EI */
>>> +
>>> +	/* allows RC6 residency counter to work */
>>> +	I915_WRITE(VLV_COUNTER_CONTROL,
>>> +		   _MASKED_BIT_ENABLE(VLV_COUNT_RANGE_HIGH |
>>> +				      VLV_MEDIA_RC6_COUNT_EN |
>>> +				      VLV_RENDER_RC6_COUNT_EN));
>>> +
>>> +	/* Todo: If BIOS has not configured PCBR
>>> +	 *       then allocate in BIOS Reserved */
>>> +
>>> +	/* For now we assume BIOS is allocating and populating the PCBR  */
>>> +	pcbr = I915_READ(VLV_PCBR);
>>> +
>>> +	DRM_DEBUG_DRIVER("PCBR offset : 0x%x\n", pcbr);
>>> +
>>> +	/* 3: Enable RC6 */
>>> +	if (intel_enable_rc6(dev) & INTEL_RC6_ENABLE & (pcbr >> 12))
>>> +		rc6_mode = GEN6_RC_CTL_EI_MODE(1) | VLV_RC_CTL_CTX_RST_PARALLEL;
>>> +
>>> +	I915_WRITE(GEN6_RC_CONTROL, rc6_mode);
>>> +
>>> +	gen6_gt_force_wake_put(dev_priv, FORCEWAKE_ALL);
>>> +}
>>> +
>>> +
>>>   static void valleyview_enable_rps(struct drm_device *dev)
>>>   {
>>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>> @@ -4437,7 +4527,9 @@ void intel_disable_gt_powersave(struct drm_device *dev)
>>>   		cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work);
>>>   		cancel_work_sync(&dev_priv->rps.work);
>>>   		mutex_lock(&dev_priv->rps.hw_lock);
>>> -		if (IS_VALLEYVIEW(dev))
>>> +		if (IS_CHERRYVIEW(dev))
>>> +			cherryview_disable_rps(dev);
>>> +		else if (IS_VALLEYVIEW(dev))
>>>   			valleyview_disable_rps(dev);
>>>   		else
>>>   			gen6_disable_rps(dev);
>>> @@ -4455,7 +4547,9 @@ static void intel_gen6_powersave_work(struct work_struct *work)
>>>   
>>>   	mutex_lock(&dev_priv->rps.hw_lock);
>>>   
>>> -	if (IS_VALLEYVIEW(dev)) {
>>> +	if (IS_CHERRYVIEW(dev)) {
>>> +		cherryview_enable_rps(dev);
>>> +	} else if (IS_VALLEYVIEW(dev)) {
>>>   		valleyview_enable_rps(dev);
>>>   	} else if (IS_BROADWELL(dev)) {
>>>   		gen8_enable_rps(dev);
>>> @@ -4476,7 +4570,7 @@ void intel_enable_gt_powersave(struct drm_device *dev)
>>>   		ironlake_enable_drps(dev);
>>>   		ironlake_enable_rc6(dev);
>>>   		intel_init_emon(dev);
>>> -	} else if (IS_GEN6(dev) || IS_GEN7(dev)) {
>>> +	} else if (INTEL_INFO(dev)->gen >= 6) {
>>>   		if (IS_VALLEYVIEW(dev))
>>>   			valleyview_setup_pctx(dev);
>>>   		/*
>>> @@ -5051,6 +5145,7 @@ static void valleyview_init_clock_gating(struct drm_device *dev)
>>>   		dev_priv->mem_freq = 1333;
>>>   		break;
>>>   	}
>>> +
>>>   	DRM_DEBUG_DRIVER("DDR speed: %d MHz", dev_priv->mem_freq);
>>>   
>>>   	dev_priv->vlv_cdclk_freq = valleyview_cur_cdclk(dev_priv);
>>> -- 
>>> 1.8.3.2
>>>
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> -- 
>> Jani Nikula, Intel Open Source Technology Center