Re: [PATCH 6/7] drm/i915: Save latest known sink CRC to compensate delayed counter reset.

From: Rafael Antognolli <rafael.antognolli@intel.com>
To: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 6/7] drm/i915: Save latest known sink CRC to compensate delayed counter reset.
Date: Tue, 28 Jul 2015 13:18:46 -0700	[thread overview]
Message-ID: <20150728201846.GC5902@intel.com> (raw)
In-Reply-To: <1437694550-5667-5-git-send-email-rodrigo.vivi@intel.com>

On Thu, Jul 23, 2015 at 04:35:49PM -0700, Rodrigo Vivi wrote:
> By Vesa DP 1.2 Spec TEST_CRC_COUNT should be
> "reset to 0 when TEST_SINK bit 0 = 0."
> 
> However for some strange reason when PSR is enabled in
> certain platforms this is not true. At least not immediatelly.
> 
> So we face cases like this:
> 
> first get_sink_crc operation:
> 	     count: 0, crc: 000000000000
> 	     count: 1, crc: c101c101c101
> returned expected crc: c101c101c101
> 
> secont get_sink_crc operation:
> 	     count: 1, crc: c101c101c101
> 	     count: 0, crc: 000000000000
> 	     count: 1, crc: 0000c1010000
> should return expected crc: 0000c1010000
> 
> But also the reset to 0 should be faster resulting into:
> 
> get_sink_crc operation:
> 	     count: 1, crc: c101c101c101
> 	     count: 1, crc: 0000c1010000
> should return expected crc: 0000c1010000
> 
> So in order to know that the second one is valid one
> we need to compare the pair (count, crc) with latest (count, crc).
> 
> If the pair changed you have your valid CRC.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

We discussed this before, unfortunately I don't see any other way to do
this, since there is no way to know that the crc count really restarted,
became 0 and then 1 again. So I think this workaround is needed.

Reviewed-by: Rafael Antognolli <rafael.antognolli@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 42 +++++++++++++++++++++++++---------------
>  drivers/gpu/drm/i915/intel_drv.h |  8 +++++++-
>  2 files changed, 33 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 3ba031d..c7372a1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3978,7 +3978,7 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp)
>  		goto out;
>  	}
>  
> -	intel_dp->sink_crc_started = false;
> +	intel_dp->sink_crc.started = false;
>   out:
>  	hsw_enable_ips(intel_crtc);
>  	return ret;
> @@ -3991,7 +3991,7 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp)
>  	u8 buf;
>  	int ret;
>  
> -	if (intel_dp->sink_crc_started) {
> +	if (intel_dp->sink_crc.started) {
>  		ret = intel_dp_sink_crc_stop(intel_dp);
>  		if (ret)
>  			return ret;
> @@ -4003,6 +4003,8 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp)
>  	if (!(buf & DP_TEST_CRC_SUPPORTED))
>  		return -ENOTTY;
>  
> +	intel_dp->sink_crc.last_count = buf & DP_TEST_COUNT_MASK;
> +
>  	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0)
>  		return -EIO;
>  
> @@ -4014,7 +4016,7 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp)
>  		return -EIO;
>  	}
>  
> -	intel_dp->sink_crc_started = true;
> +	intel_dp->sink_crc.started = true;
>  	return 0;
>  }
>  
> @@ -4024,29 +4026,39 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>  	struct drm_device *dev = dig_port->base.base.dev;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(dig_port->base.base.crtc);
>  	u8 buf;
> -	int test_crc_count;
> +	int count, ret;
>  	int attempts = 6;
> -	int ret;
>  
>  	ret = intel_dp_sink_crc_start(intel_dp);
>  	if (ret)
>  		return ret;
>  
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0) {
> -		ret = -EIO;
> -		goto stop;
> -	}
> -
> -	test_crc_count = buf & DP_TEST_COUNT_MASK;
> -
>  	do {
> +		intel_wait_for_vblank(dev, intel_crtc->pipe);
> +
>  		if (drm_dp_dpcd_readb(&intel_dp->aux,
>  				      DP_TEST_SINK_MISC, &buf) < 0) {
>  			ret = -EIO;
>  			goto stop;
>  		}
> -		intel_wait_for_vblank(dev, intel_crtc->pipe);
> -	} while (--attempts && (buf & DP_TEST_COUNT_MASK) == test_crc_count);
> +		count = buf & DP_TEST_COUNT_MASK;
> +		/*
> +		 * Count might be reset during the loop. In this case
> +		 * last known count needs to be reset as well.
> +		 */
> +		if (count == 0)
> +			intel_dp->sink_crc.last_count = 0;
> +
> +		if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0) {
> +			ret = -EIO;
> +			goto stop;
> +		}
> +	} while (--attempts && (count == 0 || (count == intel_dp->sink_crc.last_count &&
> +					       !memcmp(intel_dp->sink_crc.last_crc, crc,
> +						       6 * sizeof(u8)))));
> +
> +	intel_dp->sink_crc.last_count = buf & DP_TEST_COUNT_MASK;
> +	memcpy(intel_dp->sink_crc.last_crc, crc, 6 * sizeof(u8));
>  
>  	if (attempts == 0) {
>  		DRM_DEBUG_KMS("Panel is unable to calculate CRC after 6 vblanks\n");
> @@ -4054,8 +4066,6 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>  		goto stop;
>  	}
>  
> -	if (drm_dp_dpcd_read(&intel_dp->aux, DP_TEST_CRC_R_CR, crc, 6) < 0)
> -		ret = -EIO;
>  stop:
>  	intel_dp_sink_crc_stop(intel_dp);
>  	return ret;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index cc74400..c072820 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -697,6 +697,12 @@ enum link_m_n_set {
>  	M2_N2
>  };
>  
> +struct sink_crc {
> +	bool started;
> +	u8 last_crc[6];
> +	int last_count;
> +};
> +
>  struct intel_dp {
>  	uint32_t output_reg;
>  	uint32_t aux_ch_ctl_reg;
> @@ -714,7 +720,7 @@ struct intel_dp {
>  	/* sink rates as reported by DP_SUPPORTED_LINK_RATES */
>  	uint8_t num_sink_rates;
>  	int sink_rates[DP_MAX_SUPPORTED_RATES];
> -	bool sink_crc_started;
> +	struct sink_crc sink_crc;
>  	struct drm_dp_aux aux;
>  	uint8_t train_set[4];
>  	int panel_power_up_delay;
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx