[Intel-gfx] [RFC 3/4] drm/i915: valleyview: Make intel_set_rps get FORCEWAKE_MEDIA

[Hans de Goede]

Hi,

On 01-01-17 21:24, Chris Wilson wrote:
> On Sun, Jan 01, 2017 at 09:14:02PM +0100, Hans de Goede wrote:
>> All callers of valleyview_set_rps() get at least FORCEWAKE_MEDIA, except
>> for intel_set_rps(). Since intel_set_rps can for example be called from
>> sysfs store functions, there is no guarantee this is already done, so add
>> an intel_uncore_forcewake_get(FORCEWAKE_MEDIA) call to intel_set_rps.
>>
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>> ---
>>  drivers/gpu/drm/i915/intel_pm.c | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 4b12637..cc4fbd7 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -5096,9 +5096,14 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv,
>>
>>  void intel_set_rps(struct drm_i915_private *dev_priv, u8 val)
>>  {
>> -	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
>> +	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
>> +		/* Wake up the media well, as that takes a lot less
>> +		 * power than the Render well.
>> +		 */
>> +		intel_uncore_forcewake_get(dev_priv, FORCEWAKE_MEDIA);
>>  		valleyview_set_rps(dev_priv, val);
>
> Both powerwells are woken for rps. (Taking one but not the other has no
> benefit, and very misleading.)

The comment on why FORCEWAKE_MEDIA is used + code is copy pasted from the
existing code in vlv_set_rps_idle().

 > The forcewake is already held by the
> lower level routines, taking the wakelock in the caller is an optimisation
> that is only interesting if there is a danger from the forcewake being
> dropped mid-sequence (due to preemption whatever).

We're also accessing the punit in valleyview_set_rps() and I've seen several
patches (in the android x86 kernel) suggesting that we need to take a wakelock
while doing this.

Regards,

Hans