Re: [PATCH 10/10] drm/i915/psr: EDP_PSR_PERF_CNT not valid for psr2

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Jan 02, 2017 at 05:01:03PM +0530, vathsala nagaraju wrote:
> PSR1 and PSR2 enable sequence are mutually exclusive.
> Register SRD_PERF_COUNT increments while system is in psr1.
> This register is not valid for psr2.while in psr2,SRD_PERF_COUNT
> is always 0.
> Reporting psr perfcount from SRD_PERF_COUNT is not valid for psr2 case.
> Also, if dc6 is disabled via kernel parameter i915.enable_dc=0,
> EDP_PSR_PERF_CNT can be reported for SKL+ platforms for debug
> purpose.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Jim Bride <jim.bride@linux.intel.com>
> Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 8b0e3f9..4136cec 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2539,6 +2539,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
>  	u32 stat[3];
>  	enum pipe pipe;
>  	bool enabled = false;
> +	bool dc6_enabled = false;
>  
>  	if (!HAS_PSR(dev_priv)) {
>  		seq_puts(m, "PSR not supported\n");
> @@ -2598,11 +2599,20 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
>  
>  	/*
>  	 * VLV/CHV PSR has no kind of performance counter
> +	 * EDP_PSR_PERF_CNT is not valid for psr2.
>  	 * SKL+ Perf counter is reset to 0 everytime DC state is entered
> +	 * if we want to read EDP_PSR_PERF_CNT for debug purpose on SKL+,
> +	 * disable dc state in kernel parameter i915.enable_dc=0.
>  	 */
> -	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
> +
> +	dc6_enabled = ((I915_READ(DC_STATE_EN) &
> +			DC_STATE_EN_UPTO_DC5_DC6_MASK) ==
> +			DC_STATE_EN_UPTO_DC6);

I don't like this approach because if here on this line there is no
guarantee anymore that DC state is not enabled.
Ideally DC state is enabled most of the time so let's just avoid
the perf counter 0 that just confuses people.

> +
> +	if ((!dev_priv->psr.psr2_support && !dc6_enabled) ||
> +	     IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) {
>  		psrperf = I915_READ(EDP_PSR_PERF_CNT) &
> -			EDP_PSR_PERF_CNT_MASK;
> +			  EDP_PSR_PERF_CNT_MASK;
>  
>  		seq_printf(m, "Performance_Counter: %u\n", psrperf);
>  	}
> -- 
> 1.9.1
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel