[Intel-gfx] [PATCH v4] drm/i915/chv: Set min freq to efficient frequency on chv

[Deepak S]

On Thursday 30 April 2015 07:35 PM, Ville Syrjälä wrote:
> On Thu, Apr 30, 2015 at 02:19:07PM +0300, Ville Syrjälä wrote:
>> On Thu, Apr 30, 2015 at 03:42:42PM +0530, Deepak S wrote:
>>> As you suggested it would be better to extend the VLV WA to
>>> CHV also to make sure we drop the voltage when idle.
>>>
>>> Below is the sequence I think we should follow (based  on your comments).
>>> 1. forcewake power wells
>>> 2. do gfx force clock on
>>> 3. request freq to punit
>>> 4. release gfx force clock on
>>> 5. release forcewake of power wells.
>>>
>>> Please share your thoughts?
>> I'm thinking we shouldn't need the gfx clock force since forcewake
>> should already cause the clock to be enabled.
>>
>> I've also not verified what happens if we drop the forcewake before
>> Punit has actually finished the frequency change. I'll try to hack
>> up some kind of test to see if I can make that happen.
> OK, after some hacking I see that the forcewake is enough, and also
> we don't seem to need to wait for the Punit to finish the frequency
> change before dropping forcewake. It seems to finish the change even
> after forcewake has been dropped.
>
> Also for a bit of extra micro optimization we should perhaps wake up
> the media well only, as that takes a lot less power than the render
> well.

Thanks Ville. I will submit new patch to extend the WA to CHV

Thanks
Deepak

>>> Thanks
>>> Deepak
>>>
>>>>> So based on my tests this patch feels a bit wrong.
>>>>>
>>>>>> Since Punit validates the rps range [RPe, RP0]. This patch
>>>>>> removes unused cherryview_rps_min_freq function.
>>>>>>
>>>>>> v2: Change commit message
>>>>>>
>>>>>> v3: set min_freq before idle_freq (chris)
>>>>>>
>>>>>> v4: Squash 'Remove unused rps min function' patch
>>>>>>
>>>>>> Signed-off-by: Deepak S <deepak.s at linux.intel.com>
>>>>>> Acked-by: Chris Wilson <chris at chris-wilson.co.uk>
>>>>>> ---
>>>>>>    drivers/gpu/drm/i915/intel_pm.c | 21 ++-------------------
>>>>>>    1 file changed, 2 insertions(+), 19 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>>>>> index a7516ed..78c89ff 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>>>>> @@ -4715,24 +4715,6 @@ static int cherryview_rps_guar_freq(struct drm_i915_private *dev_priv)
>>>>>>    	return rp1;
>>>>>>    }
>>>>>>    
>>>>>> -static int cherryview_rps_min_freq(struct drm_i915_private *dev_priv)
>>>>>> -{
>>>>>> -	struct drm_device *dev = dev_priv->dev;
>>>>>> -	u32 val, rpn;
>>>>>> -
>>>>>> -	if (dev->pdev->revision >= 0x20) {
>>>>>> -		val = vlv_punit_read(dev_priv, FB_GFX_FMIN_AT_VMIN_FUSE);
>>>>>> -		rpn = ((val >> FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT) &
>>>>>> -		       FB_GFX_FREQ_FUSE_MASK);
>>>>>> -	} else { /* For pre-production hardware */
>>>>>> -		val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG);
>>>>>> -		rpn = ((val >> PUNIT_GPU_STATIS_GFX_MIN_FREQ_SHIFT) &
>>>>>> -		       PUNIT_GPU_STATUS_GFX_MIN_FREQ_MASK);
>>>>>> -	}
>>>>>> -
>>>>>> -	return rpn;
>>>>>> -}
>>>>>> -
>>>>>>    static int valleyview_rps_guar_freq(struct drm_i915_private *dev_priv)
>>>>>>    {
>>>>>>    	u32 val, rp1;
>>>>>> @@ -4984,7 +4966,8 @@ static void cherryview_init_gt_powersave(struct drm_device *dev)
>>>>>>    			 intel_gpu_freq(dev_priv, dev_priv->rps.rp1_freq),
>>>>>>    			 dev_priv->rps.rp1_freq);
>>>>>>    
>>>>>> -	dev_priv->rps.min_freq = cherryview_rps_min_freq(dev_priv);
>>>>>> +	/* PUnit validated range is only [RPe, RP0] */
>>>>>> +	dev_priv->rps.min_freq = dev_priv->rps.efficient_freq;
>>>>>>    	DRM_DEBUG_DRIVER("min GPU freq: %d MHz (%u)\n",
>>>>>>    			 intel_gpu_freq(dev_priv, dev_priv->rps.min_freq),
>>>>>>    			 dev_priv->rps.min_freq);
>>>>>> -- 
>>>>>> 1.9.1
>>>>> -- 
>>>>> Ville Syrjälä
>>>>> Intel OTC
>>>>> _______________________________________________
>>>>> Intel-gfx mailing list
>>>>> Intel-gfx at lists.freedesktop.org
>>>>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> -- 
>> Ville Syrjälä
>> Intel OTC