Re: [PATCH] drm/i915: Use dpcd read wake for sink crc calls.

From: Daniel Vetter <daniel@ffwll.ch>
To: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
	Rodrigo Vivi <vivijim@rdvivi-budapest.jf.intel.com>
Subject: Re: [PATCH] drm/i915: Use dpcd read wake for sink crc calls.
Date: Wed, 26 Aug 2015 11:06:08 +0200	[thread overview]
Message-ID: <20150826090608.GN20434@phenom.ffwll.local> (raw)
In-Reply-To: <1440112320-21884-1-git-send-email-rodrigo.vivi@intel.com>

On Thu, Aug 20, 2015 at 04:12:00PM -0700, Rodrigo Vivi wrote:
> From: Rodrigo Vivi <vivijim@rdvivi-budapest.jf.intel.com>
> 
> Let's use a native read with retry as suggested per spec to
> fix Sink CRC on SKL when PSR is enabled.
> 
> With PSR enabled panel is probably taking more time to wake
> and dpcd read is faling.
> 
> Cc: Sonika Jindal <sonika.jindal@intel.com>
> Signed-off-by: Rodrigo Vivi <vivijim@rdvivi-budapest.jf.intel.com>

Seems like we should just move the trickery we do in our own version into
the dp helpers in the core if this is needed all over the place?

At least in i915 we use it everywhere and it doesn't seem actively harmful
really ... Maybe the only exception would be the i2c-over-dp_aux code.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index d32ce48..34f5e33 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4037,7 +4037,8 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp)
>  	u8 buf;
>  	int ret = 0;
>  
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0) {
> +	if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_TEST_SINK,
> +				    &buf, 1) < 0) {
>  		DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n");
>  		ret = -EIO;
>  		goto out;
> @@ -4069,7 +4070,8 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp)
>  			return ret;
>  	}
>  
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) < 0)
> +	if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_TEST_SINK_MISC,
> +				    &buf, 1) < 0)
>  		return -EIO;
>  
>  	if (!(buf & DP_TEST_CRC_SUPPORTED))
> @@ -4077,7 +4079,7 @@ static int intel_dp_sink_crc_start(struct intel_dp *intel_dp)
>  
>  	intel_dp->sink_crc.last_count = buf & DP_TEST_COUNT_MASK;
>  
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0)
> +	if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_TEST_SINK, &buf, 1) < 0)
>  		return -EIO;
>  
>  	hsw_disable_ips(intel_crtc);
> @@ -4109,8 +4111,8 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>  	do {
>  		intel_wait_for_vblank(dev, intel_crtc->pipe);
>  
> -		if (drm_dp_dpcd_readb(&intel_dp->aux,
> -				      DP_TEST_SINK_MISC, &buf) < 0) {
> +		if (intel_dp_dpcd_read_wake(&intel_dp->aux,
> +					    DP_TEST_SINK_MISC, &buf, 1) < 0) {
>  			ret = -EIO;
>  			goto stop;
>  		}
> @@ -4123,7 +4125,8 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc)
>  		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) {
> +		if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_TEST_CRC_R_CR,
> +					    crc, 6) < 0) {
>  			ret = -EIO;
>  			goto stop;
>  		}
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx