[Intel-gfx] [PATCH] drm/i915/debugfs: move IPS debugfs to hsw_ips.c

[Jani Nikula]

On Thu, 02 Mar 2023, Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> On Thu, Mar 02, 2023 at 06:16:17PM +0200, Jani Nikula wrote:
>> Follow the style of placing debugfs next to the implementation.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

Thanks!

> Some ideas for future enhancements:
> - only register when IPS is actually supported
> - make it per-crtc since IPS only exists on pipe A
> - report the software state of IPS enable vs. disable
> - there's a false color bit just like with fbc, could
>   hook that up to help verify that it actually works

Yeah, I considered that, but since

#define HAS_IPS(dev_priv)	(IS_HSW_ULT(dev_priv) || IS_BROADWELL(dev_priv))

it's not really high up on the list of things to do... indeed I wanted
to stow this away in hsw_ips.[ch] because of that.

BR,
Jani.


>
>> ---
>>  drivers/gpu/drm/i915/display/hsw_ips.c        | 37 +++++++++++++++++++
>>  drivers/gpu/drm/i915/display/hsw_ips.h        |  2 +
>>  .../drm/i915/display/intel_display_debugfs.c  | 30 +--------------
>>  3 files changed, 41 insertions(+), 28 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/hsw_ips.c b/drivers/gpu/drm/i915/display/hsw_ips.c
>> index 83aa3800245f..2910f5d0f3e2 100644
>> --- a/drivers/gpu/drm/i915/display/hsw_ips.c
>> +++ b/drivers/gpu/drm/i915/display/hsw_ips.c
>> @@ -267,3 +267,40 @@ void hsw_ips_get_config(struct intel_crtc_state *crtc_state)
>>  		crtc_state->ips_enabled = true;
>>  	}
>>  }
>> +
>> +static int hsw_ips_debugfs_status_show(struct seq_file *m, void *unused)
>> +{
>> +	struct drm_i915_private *i915 = m->private;
>> +	intel_wakeref_t wakeref;
>> +
>> +	if (!HAS_IPS(i915))
>> +		return -ENODEV;
>> +
>> +	wakeref = intel_runtime_pm_get(&i915->runtime_pm);
>> +
>> +	seq_printf(m, "Enabled by kernel parameter: %s\n",
>> +		   str_yes_no(i915->params.enable_ips));
>> +
>> +	if (DISPLAY_VER(i915) >= 8) {
>> +		seq_puts(m, "Currently: unknown\n");
>> +	} else {
>> +		if (intel_de_read(i915, IPS_CTL) & IPS_ENABLE)
>> +			seq_puts(m, "Currently: enabled\n");
>> +		else
>> +			seq_puts(m, "Currently: disabled\n");
>> +	}
>> +
>> +	intel_runtime_pm_put(&i915->runtime_pm, wakeref);
>> +
>> +	return 0;
>> +}
>> +
>> +DEFINE_SHOW_ATTRIBUTE(hsw_ips_debugfs_status);
>> +
>> +void hsw_ips_debugfs_register(struct drm_i915_private *i915)
>> +{
>> +	struct drm_minor *minor = i915->drm.primary;
>> +
>> +	debugfs_create_file("i915_ips_status", 0444, minor->debugfs_root,
>> +			    i915, &hsw_ips_debugfs_status_fops);
>> +}
>> diff --git a/drivers/gpu/drm/i915/display/hsw_ips.h b/drivers/gpu/drm/i915/display/hsw_ips.h
>> index 4564dee497d7..7ed6061874f7 100644
>> --- a/drivers/gpu/drm/i915/display/hsw_ips.h
>> +++ b/drivers/gpu/drm/i915/display/hsw_ips.h
>> @@ -8,6 +8,7 @@
>>  
>>  #include <linux/types.h>
>>  
>> +struct drm_i915_private;
>>  struct intel_atomic_state;
>>  struct intel_crtc;
>>  struct intel_crtc_state;
>> @@ -22,5 +23,6 @@ bool hsw_crtc_state_ips_capable(const struct intel_crtc_state *crtc_state);
>>  int hsw_ips_compute_config(struct intel_atomic_state *state,
>>  			   struct intel_crtc *crtc);
>>  void hsw_ips_get_config(struct intel_crtc_state *crtc_state);
>> +void hsw_ips_debugfs_register(struct drm_i915_private *i915);
>>  
>>  #endif /* __HSW_IPS_H__ */
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> index 25013f303c82..8be606bfd2b4 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
>> @@ -8,6 +8,7 @@
>>  #include <drm/drm_debugfs.h>
>>  #include <drm/drm_fourcc.h>
>>  
>> +#include "hsw_ips.h"
>>  #include "i915_debugfs.h"
>>  #include "i915_irq.h"
>>  #include "i915_reg.h"
>> @@ -49,33 +50,6 @@ static int i915_frontbuffer_tracking(struct seq_file *m, void *unused)
>>  	return 0;
>>  }
>>  
>> -static int i915_ips_status(struct seq_file *m, void *unused)
>> -{
>> -	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> -	intel_wakeref_t wakeref;
>> -
>> -	if (!HAS_IPS(dev_priv))
>> -		return -ENODEV;
>> -
>> -	wakeref = intel_runtime_pm_get(&dev_priv->runtime_pm);
>> -
>> -	seq_printf(m, "Enabled by kernel parameter: %s\n",
>> -		   str_yes_no(dev_priv->params.enable_ips));
>> -
>> -	if (DISPLAY_VER(dev_priv) >= 8) {
>> -		seq_puts(m, "Currently: unknown\n");
>> -	} else {
>> -		if (intel_de_read(dev_priv, IPS_CTL) & IPS_ENABLE)
>> -			seq_puts(m, "Currently: enabled\n");
>> -		else
>> -			seq_puts(m, "Currently: disabled\n");
>> -	}
>> -
>> -	intel_runtime_pm_put(&dev_priv->runtime_pm, wakeref);
>> -
>> -	return 0;
>> -}
>> -
>>  static int i915_sr_status(struct seq_file *m, void *unused)
>>  {
>>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>> @@ -1343,7 +1317,6 @@ static const struct file_operations i915_fifo_underrun_reset_ops = {
>>  
>>  static const struct drm_info_list intel_display_debugfs_list[] = {
>>  	{"i915_frontbuffer_tracking", i915_frontbuffer_tracking, 0},
>> -	{"i915_ips_status", i915_ips_status, 0},
>>  	{"i915_sr_status", i915_sr_status, 0},
>>  	{"i915_opregion", i915_opregion, 0},
>>  	{"i915_vbt", i915_vbt, 0},
>> @@ -1385,6 +1358,7 @@ void intel_display_debugfs_register(struct drm_i915_private *i915)
>>  				 ARRAY_SIZE(intel_display_debugfs_list),
>>  				 minor->debugfs_root, minor);
>>  
>> +	hsw_ips_debugfs_register(i915);
>>  	intel_dmc_debugfs_register(i915);
>>  	intel_fbc_debugfs_register(i915);
>>  	intel_hpd_debugfs_register(i915);
>> -- 
>> 2.39.1

-- 
Jani Nikula, Intel Open Source Graphics Center