From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lyude Paul Subject: Re: [Intel-gfx] [PATCH 2/2] drm/i915: Get rid of intel_dp_dpcd_read_wake() Date: Mon, 21 Mar 2016 12:37:47 -0400 Message-ID: <1458578267.3031.4.camel@redhat.com> References: <1458229245-8634-1-git-send-email-cpaul@redhat.com> <1458229245-8634-2-git-send-email-cpaul@redhat.com> <20160318141345.GG4329@intel.com> <20160318161235.GN4329@intel.com> <20160318164140.GO4329@intel.com> <20160318180029.GL14170@phenom.ffwll.local> <20160318180553.GS4329@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <20160318180553.GS4329@intel.com> Sender: linux-kernel-owner@vger.kernel.org To: Ville =?ISO-8859-1?Q?Syrj=E4l=E4?= , Daniel Vetter , intel-gfx@lists.freedesktop.org, arthur.j.runyan@intel.com, open list , dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, 2016-03-18 at 20:05 +0200, Ville Syrj=C3=A4l=C3=A4 wrote: > On Fri, Mar 18, 2016 at 07:00:29PM +0100, Daniel Vetter wrote: > >=20 > > On Fri, Mar 18, 2016 at 06:41:40PM +0200, Ville Syrj=C3=A4l=C3=A4 w= rote: > > >=20 > > > On Fri, Mar 18, 2016 at 06:12:35PM +0200, Ville Syrj=C3=A4l=C3=A4= wrote: > > > >=20 > > > > On Fri, Mar 18, 2016 at 04:13:45PM +0200, Ville Syrj=C3=A4l=C3=A4= wrote: > > > > >=20 > > > > > On Thu, Mar 17, 2016 at 11:40:45AM -0400, Lyude wrote: > > > > > >=20 > > > > > > Since we've fixed up drm_dp_dpcd_read() to allow for retrie= s when > > > > > > things > > > > > > timeout, there's no use for having this function anymore. G= ood > > > > > > riddens. > > > > > >=20 > > > > > > Signed-off-by: Lyude > > > > > > --- > > > > > > =C2=A0drivers/gpu/drm/i915/intel_dp.c | 79 ++++++++++++----= ------------ > > > > > > ------------- > > > > > > =C2=A01 file changed, 22 insertions(+), 57 deletions(-) > > > > > >=20 > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > > > > > b/drivers/gpu/drm/i915/intel_dp.c > > > > > > index cdc2c15..fb4cbbe5 100644 > > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > > > @@ -3190,47 +3190,14 @@ static void chv_dp_post_pll_disable= (struct > > > > > > intel_encoder *encoder) > > > > > > =C2=A0} > > > > > > =C2=A0 > > > > > > =C2=A0/* > > > > > > - * Native read with retry for link status and receiver cap= ability > > > > > > reads for > > > > > > - * cases where the sink may still be asleep. > > > > > > - * > > > > > > - * Sinks are *supposed* to come up within 1ms from an off = state, > > > > > > but we're also > > > > > > - * supposed to retry 3 times per the spec. > > > > > > - */ > > > > > > -static ssize_t > > > > > > -intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned i= nt > > > > > > offset, > > > > > > - void *buffer, size_t size) > > > > > > -{ > > > > > > - ssize_t ret; > > > > > > - int i; > > > > > > - > > > > > > - /* > > > > > > - =C2=A0* Sometime we just get the same incorrect byte repe= ated > > > > > > - =C2=A0* over the entire buffer. Doing just one throw away= read > > > > > > - =C2=A0* initially seems to "solve" it. > > > > > > - =C2=A0*/ > > > > > > - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); > > > > > NAK > > > > >=20 > > > > > If people keep intentionally breaking my shit I'm going to be= come > > > > > really grumpy soon. > > > > Oh, and just in case someone wants to come up with a better klu= dge, > > > > I just spent a few minutes analyzing the behavior of this crapp= y > > > > monitor a. > > > >=20 > > > > What happens is that when the monitor is fully powered up (LED = is blue) > > > > things are fine. After the monitor goes to sleep (LED turns ora= nge) > > > > the first DPCD read will produce garbage. Further DPCD reads ar= e fine, > > > > even if I wait a significant amount of time between the reads, = as long > > > > as the monitor didn't do a power on->off cycle in between. So i= t looks > > > > like it's always just the first read after power down that gets > > > > corrupted. > > > >=20 > > > > Now I think I'll go and test how writes behave, assuming I can = find a > > > > decently sized chunk of DPCD address space I can write. And may= be I > > > > should also try i2c-over-aux... > > > The first DPCD write after powerdown also got corrupted. But i2c-= over-aux > > > seems unaffected for whatever reason. > > Do you have an amd card nearby to test there? > Nope. I also work on radeon so I have plenty. The issue with that though is t= hat this is an issue that only really happens with the thinkpad docks, and as fa= r as I'm aware we don't have any ThinkPads that have AMD hardware in them=E2=80=A6 > >=20 > > Would be interesting to > > confirm that this is indeed a sink bug, and hence that it really al= l > > should be in the shared code. > > -Daniel > > --=C2=A0 > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch