From mboxrd@z Thu Jan 1 00:00:00 1970 From: Todd Previte Subject: Re: [PATCH 1/2] drm/i915: Do a dummy DPCD read before the actual read Date: Fri, 17 Oct 2014 09:38:31 -0700 Message-ID: <54414607.30902@gmail.com> References: <1413481570-18288-1-git-send-email-ville.syrjala@linux.intel.com> <87fvenm6hi.fsf@intel.com> <20141017085902.GO4284@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; Format="flowed" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-pd0-f177.google.com (mail-pd0-f177.google.com [209.85.192.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 6CAB06E356 for ; Fri, 17 Oct 2014 09:38:28 -0700 (PDT) Received: by mail-pd0-f177.google.com with SMTP id v10so1063277pde.36 for ; Fri, 17 Oct 2014 09:38:28 -0700 (PDT) Received: from [192.168.1.212] (ip72-201-95-47.ph.ph.cox.net. [72.201.95.47]) by mx.google.com with ESMTPSA id gu10sm1973018pbc.72.2014.10.17.09.38.27 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 17 Oct 2014 09:38:27 -0700 (PDT) In-Reply-To: <20141017085902.GO4284@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On 10/17/2014 1:59 AM, Ville Syrj=E4l=E4 wrote: > On Fri, Oct 17, 2014 at 11:43:21AM +0300, Jani Nikula wrote: >> On Thu, 16 Oct 2014, ville.syrjala@linux.intel.com wrote: >>> From: Ville Syrj=E4l=E4 >>> >>> Sometimes we seem to get utter garbage from DPCD reads. The resulting >>> buffer is filled with the same byte, and the operation completed without >>> errors. My HP ZR24w monitor seems particularly susceptible to this >>> problem once it's gone into a sleep mode. >>> >>> The issue seems to happen only for the first AUX message that wakes the >>> sink up. But as the first AUX read we often do is the DPCD receiver >>> cap it does wreak a bit of havoc with subsequent link training etc. when >>> the receiver cap bw/lane/etc. information is garbage. >> This makes me suspect our sink dpms and wake handling even more than I >> already did. Someone(tm) should dig into the DP and hw specs again with >> fresh eyes... > Yeah when I last looked at the spec it was rather vague on the subject. > On the other hand it seems to suggest that you're supposed to wake up > the sink via DP_SET_POWER before any other AUX transfer, but on the > other hand it seems to say AUX is fine as long as you remember to do > the extended retry in case the AUX circuitry was asleep in the sink. The sink is supposed to exit power-down mode within 1ms of detecting a = differential signal voltage on the AUX lines, according to the spec. So = in theory, any AUX transactions should be able to wake up the sink = device. As you pointed out, the AUX retries will catch the case where = the AUX channel is powered down. I know of one instance where the write = to SET_POWER is required, and that's in one of the compliance tests. = Outside of that though, I haven't seen anything where it's mandatory. -T > > However DPCD 1.0 doesn't even support DP_SET_POWER so that does suggest > that it's not really mandatory to wake things up first. Well, assuming > DPCD 1.0 devices even exist. > > I was a bit suspicious of our AUX code as well, but IIRC didn't spot > anything obviously incorrect there when I had a look. So might be this > is just the sink being a bit buggy. > >>> A sufficient workaround seems to be to perform a single byte dummy read >>> before reading the actual data. I suppose that just wakes up the sink >>> sufficiently and we can just throw away the returned data in case it's >>> crap. DP_DPCD_REV seems like a sufficiently safe location to read here. >> Seems like a pretty harmless thing to do, and we already do loads of >> dpcd reads anyway. We should throw this at some related bugs. >> >> So ack. >> >> BR, >> Jani. >> >> >>> Signed-off-by: Ville Syrj=E4l=E4 >>> --- >>> drivers/gpu/drm/i915/intel_dp.c | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/int= el_dp.c >>> index 64c8e04..f07f02c 100644 >>> --- a/drivers/gpu/drm/i915/intel_dp.c >>> +++ b/drivers/gpu/drm/i915/intel_dp.c >>> @@ -2870,6 +2870,13 @@ intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, = unsigned int offset, >>> ssize_t ret; >>> int i; >>> = >>> + /* >>> + * Sometime we just get the same incorrect byte repeated >>> + * over the entire buffer. Doing just one throw away read >>> + * initially seems to "solve" it. >>> + */ >>> + drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); >>> + >>> for (i =3D 0; i < 3; i++) { >>> ret =3D drm_dp_dpcd_read(aux, offset, buffer, size); >>> if (ret =3D=3D size) >>> -- = >>> 2.0.4 >>> >>> _______________________________________________ >>> Intel-gfx mailing list >>> Intel-gfx@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> -- = >> Jani Nikula, Intel Open Source Technology Center