Re: [PATCH 10/11] drm/i915: Improve PSR debugfs status.

[Vijay Purushothaman <vijay.a.purushothaman@intel.com>]

On 5/16/2014 5:43 AM, Rodrigo Vivi wrote:
> Now we have the active/inactive state for exit and this actually changes the
> HW enable bit the status was a bit confusing for users. So let's provide
> more info.
>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 6636ca2..0ca9376 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1975,10 +1975,12 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
>
>   	seq_printf(m, "Sink_Support: %s\n", yesno(dev_priv->psr.sink_support));
>   	seq_printf(m, "Source_OK: %s\n", yesno(dev_priv->psr.source_ok));
> +	seq_printf(m, "Enabled: %s\n", yesno(dev_priv->psr.enabled));
> +	seq_printf(m, "Active: %s\n", yesno(dev_priv->psr.active));
>
>   	enabled = HAS_PSR(dev) &&
>   		I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
> -	seq_printf(m, "Enabled: %s\n", yesno(enabled));
> +	seq_printf(m, "HW Enabled & Active bit: %s\n", yesno(enabled));
>
>   	if (HAS_PSR(dev))
>   		psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) &

Please remove all references to PSR performance counter. This register 
is primarily meant as a debug register and its implementation is broken 
in the h/w. Whenever the cdclk is gated to save power, the performance 
counter is stopped. But when the clk is re-enabled it doesn't reset the 
counter. This unnecessarily confuses the end users.. When the system 
goes through suspend / resume cycle the performance counter most likely 
will transition from a non-zero value to zero.. I already received few 
queries from our customers related to this performance customer and they 
refuse to believe me when i tell them PSR is still functional when the 
performance counter reports 0 :-)

AFAIK this register definition is missing in open source HSW B spec as 
well..

Thanks,
Vijay
>