[Intel-gfx] [PATCH v4 5/7] drm/i915/psr: Remove main link standby mode from debugfs

Dhinakaran Pandiyan dhinakaran.pandiyan at intel.com
Wed Oct 3 21:33:15 UTC 2018


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.pandiyan at intel.com>
> Suggested-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at 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.pandiyan at 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);






More information about the Intel-gfx mailing list