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

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

On Wed, 2015-08-26 at 11:06 +0200, Daniel Vetter wrote:
> 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?

I've wondered this, but I thought there was a good reason to let this
trick separated.

> 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.

Why this would be the exception? Maybe this was the good reason?


> -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
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx