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.nik...@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

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

> ---
>  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

-- 
Ville Syrjälä
Intel

Reply via email to