On Wednesday, October 3, 2018 1:50:29 PM PDT José Roberto de Souza wrote:
> Main link stand by is only valid for PSR1 as it is not possible to
> enable PSR2 and keep main link on but even for PSR1 it is not useful
> information and it can be removed without any drawbacks.
> But if someone still wants to check that this patch is providing the
> full PSR_CTL register value so user can check if bit 27 is set, this
> will also expose more information about how PSR1 and PSR2 was
> configured in source.
Yes, knowing the full configuration (idle frames, training pattern )  is very 
useful for debug. 

> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>
> Suggested-by: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>
> Signed-off-by: José Roberto de Souza <jose.so...@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c index f42e93b71e67..48e65becd035
> 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2712,8 +2712,8 @@ psr_source_status(struct drm_i915_private *dev_priv,
> struct seq_file *m) static int i915_edp_psr_status(struct seq_file *m, void
> *data)
>  {
>       struct drm_i915_private *dev_priv = node_to_i915(m->private);
> -     u32 psrperf = 0;
> -     bool enabled = false;
> +     u32 val;
> +     bool enabled;
>       bool sink_support;
> 
>       if (!HAS_PSR(dev_priv))
> @@ -2733,24 +2733,23 @@ static int i915_edp_psr_status(struct seq_file *m,
> void *data) seq_printf(m, "Busy frontbuffer bits: 0x%03x\n",
>                  dev_priv->psr.busy_frontbuffer_bits);
> 
> -     if (dev_priv->psr.psr2_enabled)
> -             enabled = I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE;
> -     else
> -             enabled = I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
> -
> -     seq_printf(m, "Main link in standby mode: %s\n",
> -                yesno(dev_priv->psr.link_standby));
> +     if (dev_priv->psr.psr2_enabled) {
> +             val = I915_READ(EDP_PSR2_CTL);
> +             enabled = val & EDP_PSR2_ENABLE;
> +     } else {
> +             val = I915_READ(EDP_PSR_CTL);
> +             enabled = val & EDP_PSR_ENABLE;
> +     }
> 
> -     seq_printf(m, "HW Enabled & Active bit: %s\n", yesno(enabled));
> +     seq_printf(m, "HW enabled: %s [0x%x]\n", yesno(enabled), val);

The status register is printed as <hex reg value>[<mapped string>].  Can we 
please keep this interface consistent? Also, this change will need changes in 
IGTs.

With the print format changed
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandi...@intel.com>

> 
>       /*
>        * SKL+ Perf counter is reset to 0 everytime DC state is entered
>        */
>       if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> -             psrperf = I915_READ(EDP_PSR_PERF_CNT) &
> -                     EDP_PSR_PERF_CNT_MASK;
> +             val = I915_READ(EDP_PSR_PERF_CNT) & EDP_PSR_PERF_CNT_MASK;
> 
> -             seq_printf(m, "Performance_Counter: %u\n", psrperf);
> +             seq_printf(m, "Performance_Counter: %u\n", val);
>       }
> 
>       psr_source_status(dev_priv, m);




_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to