[Intel-gfx] [PATCH v4 11/26] drm/i915/slpc: Update sysfs/debugfs interfaces for frequency parameters

[Kamble, Sagar A]

On 9/9/2016 10:43 PM, Chris Wilson wrote:
> On Fri, Sep 09, 2016 at 06:21:30PM +0530, Sagar Arun Kamble wrote:
>> From: Tom O'Rourke <Tom.O'Rourke at intel.com>
>>
>> When SLPC is controlling requested frequency, the rps.cur_freq
>> value is not used to make the frequency request.
>>
>> Requested frequency from register RPNSWREQ has the value
>> most recently requested by SLPC firmware. Adding new sysfs
>> interface gt_req_freq_mhz to know this value.
>> SLPC requested value needs to be made available to i915 without
>> reading RPNSWREQ.
>>
>> v1: Replace HAS_SLPC with intel_slpc_active (Paulo)
>>      Avoid magic numbers (Nick)
>>      Use a function for repeated code (Jon)
>>
>> v2: Add "SLPC Active" to i915_frequency_info output and
>>      don't update cur_freq as it is driver internal request. (Chris)
>>
>> v3: Removing sysfs interface gt_req_freq_mhz out of this patch
>>      for proper division of functionality. (Sagar)
>>
>> v4: idle_freq, boost_freq are also not used with SLPC.
>>
>> Signed-off-by: Tom O'Rourke <Tom.O'Rourke at intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_debugfs.c | 24 ++++++++++++++++++------
>>   drivers/gpu/drm/i915/i915_sysfs.c   |  3 +++
>>   2 files changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 02b627e..71bce32 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -1083,6 +1083,9 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
>>   
>>   	intel_runtime_pm_get(dev_priv);
>>   
>> +	if (intel_slpc_active(dev_priv))
>> +		seq_puts(m, "SLPC Active\n");
>> +
>>   	if (IS_GEN5(dev_priv)) {
>>   		u16 rgvswctl = I915_READ16(MEMSWCTL);
>>   		u16 rgvstat = I915_READ16(MEMSTAT_ILK);
>> @@ -1250,15 +1253,21 @@ static int i915_frequency_info(struct seq_file *m, void *unused)
>>   		seq_printf(m, "Max overclocked frequency: %dMHz\n",
>>   			   intel_gpu_freq(dev_priv, dev_priv->rps.max_freq));
>>   
>> -		seq_printf(m, "Current freq: %d MHz\n",
>> -			   intel_gpu_freq(dev_priv, dev_priv->rps.cur_freq));
>> +		if (!intel_slpc_active(dev_priv)) {
> Just keep printing them, we have the banner upfront, and being able to
> track and compare internal values vs hw state is still important. (And
> the ordering was fairly intentional.)
cur_freq, idle_freq, boost_freq will not be applicable with SLPC.
With SLPC we should rely on value from RPNSWREQ for cur_freq.
>
>> +			seq_printf(m, "Current freq: %d MHz\n",
>> +				   intel_gpu_freq(dev_priv,
>> +						  dev_priv->rps.cur_freq));
>> +			seq_printf(m, "Idle freq: %d MHz\n",
>> +				   intel_gpu_freq(dev_priv,
>> +						  dev_priv->rps.idle_freq));
>> +			seq_printf(m, "Boost freq: %d MHz\n",
>> +				   intel_gpu_freq(dev_priv,
>> +						  dev_priv->rps.boost_freq));
>> +		}
>> +
>>   		seq_printf(m, "Actual freq: %d MHz\n", cagf);
>> -		seq_printf(m, "Idle freq: %d MHz\n",
>> -			   intel_gpu_freq(dev_priv, dev_priv->rps.idle_freq));
>>   		seq_printf(m, "Min freq: %d MHz\n",
>>   			   intel_gpu_freq(dev_priv, dev_priv->rps.min_freq));
>> -		seq_printf(m, "Boost freq: %d MHz\n",
>> -			   intel_gpu_freq(dev_priv, dev_priv->rps.boost_freq));
>>   		seq_printf(m, "Max freq: %d MHz\n",
>>   			   intel_gpu_freq(dev_priv, dev_priv->rps.max_freq));
>>   		seq_printf(m,
>> @@ -2315,6 +2324,9 @@ static int i915_rps_boost_info(struct seq_file *m, void *data)
>>   	struct drm_device *dev = &dev_priv->drm;
>>   	struct drm_file *file;
>>   
>> +	if (intel_slpc_active(dev_priv))
>> +		return -ENODEV;
>> +
>>   	seq_printf(m, "RPS enabled? %d\n", dev_priv->rps.enabled);
>>   	seq_printf(m, "GPU busy? %s [%x]\n",
>>   		   yesno(dev_priv->gt.awake), dev_priv->gt.active_engines);
>> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
>> index 1012eee..020d64e 100644
>> --- a/drivers/gpu/drm/i915/i915_sysfs.c
>> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
>> @@ -299,6 +299,9 @@ static ssize_t gt_cur_freq_mhz_show(struct device *kdev,
>>   {
>>   	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
>>   
>> +	if (intel_slpc_active(dev_priv))
>> +		return -ENODEV;
> Ok, I had a thought that we allowed the user to directly set cur freq,
> but we don't.
> -Chris
>