| Message ID | 1405365047-6866-13-git-send-email-tprevite@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 14, 2014 at 12:10:47PM -0700, Todd Previte wrote: > The Displayport Link Layer Compliance Testing Specification 1.2 rev 1.1 specifies that > repeated AUX transactions after a failure (NACK, DEFER or no response) must have > a minimum delay of 400us before the resend can occur. Tests 4.2.1.1 and 4.2.1.2 are two > tests that require this specifically. > > Signed-off-by: Todd Previte <tprevite@gmail.com> Since this is a minimal timeout ... shouldn't we put it into the dp helpers instead? -Daniel > --- > drivers/gpu/drm/i915/intel_dp.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 0e207aaf..f0664cd 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -573,8 +573,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, > DP_AUX_CH_CTL_RECEIVE_ERROR); > > if (status & (DP_AUX_CH_CTL_TIME_OUT_ERROR | > - DP_AUX_CH_CTL_RECEIVE_ERROR)) > + DP_AUX_CH_CTL_RECEIVE_ERROR)) { > + /* DP compliance requires 400us delay for errors/timeouts > + (DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2) */ > + udelay(400); > continue; > + } > if (status & DP_AUX_CH_CTL_DONE) > break; > } > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> Daniel Vetter <mailto:daniel@ffwll.ch> > Tuesday, July 15, 2014 12:46 AM > On Mon, Jul 14, 2014 at 12:10:47PM -0700, Todd Previte wrote: >> The Displayport Link Layer Compliance Testing Specification 1.2 rev 1.1 specifies that >> repeated AUX transactions after a failure (NACK, DEFER or no response) must have >> a minimum delay of 400us before the resend can occur. Tests 4.2.1.1 and 4.2.1.2 are two >> tests that require this specifically. >> >> Signed-off-by: Todd Previte<tprevite@gmail.com> > > Since this is a minimal timeout ... shouldn't we put it into the dp > helpers instead? > -Daniel This delay catches the case where the sink device either does not respond at all or responds with an invalid AUX transaction. These are lower level errors than the DRM helper functions are supposed to handle. I mistakenly mentioned NACK/DEFER in the commit message - that's not an accurate description of what this patch does. I will fix the commit message for v3 of this patch. -T >> --- >> drivers/gpu/drm/i915/intel_dp.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 0e207aaf..f0664cd 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -573,8 +573,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, >> DP_AUX_CH_CTL_RECEIVE_ERROR); >> >> if (status& (DP_AUX_CH_CTL_TIME_OUT_ERROR | >> - DP_AUX_CH_CTL_RECEIVE_ERROR)) >> + DP_AUX_CH_CTL_RECEIVE_ERROR)) { >> + /* DP compliance requires 400us delay for errors/timeouts >> + (DP CTS 1.2 Core Rev 1.1, 4.2.1.1& 4.2.1.2) */ >> + udelay(400); >> continue; >> + } >> if (status& DP_AUX_CH_CTL_DONE) >> break; >> } >> -- >> 1.9.1 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > Todd Previte <mailto:tprevite@gmail.com> > Monday, July 14, 2014 12:10 PM > The Displayport Link Layer Compliance Testing Specification 1.2 rev > 1.1 specifies that > repeated AUX transactions after a failure (NACK, DEFER or no response) > must have > a minimum delay of 400us before the resend can occur. Tests 4.2.1.1 > and 4.2.1.2 are two > tests that require this specifically. > > Signed-off-by: Todd Previte <tprevite@gmail.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > b/drivers/gpu/drm/i915/intel_dp.c > index 0e207aaf..f0664cd 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -573,8 +573,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, > DP_AUX_CH_CTL_RECEIVE_ERROR); > > if (status & (DP_AUX_CH_CTL_TIME_OUT_ERROR | > - DP_AUX_CH_CTL_RECEIVE_ERROR)) > + DP_AUX_CH_CTL_RECEIVE_ERROR)) { > + /* DP compliance requires 400us delay for errors/timeouts > + (DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2) */ > + udelay(400); > continue; > + } > if (status & DP_AUX_CH_CTL_DONE) > break; > } > Todd Previte <mailto:tprevite@gmail.com> > Monday, July 14, 2014 12:10 PM > > V2: > - Addressed review feedback from the mailing list > - Broke up patches into smaller, easily managed chunks > - Reordered the patches such that they can be applied in order > - Fixed checkpatch.pl errors across the patchset > - Updated and enhanced functionality for the EDID test function > - Completely revamped the mode set operations for compliance testing >
2014-07-15 12:34 GMT-03:00 Todd Previte <tprevite@gmail.com>: > > Daniel Vetter <daniel@ffwll.ch> > Tuesday, July 15, 2014 12:46 AM > > On Mon, Jul 14, 2014 at 12:10:47PM -0700, Todd Previte wrote: > > The Displayport Link Layer Compliance Testing Specification 1.2 rev 1.1 specifies that > repeated AUX transactions after a failure (NACK, DEFER or no response) must have > a minimum delay of 400us before the resend can occur. Tests 4.2.1.1 and 4.2.1.2 are two > tests that require this specifically. > > Signed-off-by: Todd Previte <tprevite@gmail.com> <tprevite@gmail.com> > > Since this is a minimal timeout ... shouldn't we put it into the dp > helpers instead? > -Daniel > > This delay catches the case where the sink device either does not respond > at all or responds with an invalid AUX transaction. These are lower level > errors than the DRM helper functions are supposed to handle. I mistakenly > mentioned NACK/DEFER in the commit message - that's not an accurate > description of what this patch does. I will fix the commit message for v3 > of this patch. > > I agree your patch idea seems to be correct, but you should probably also do a little review of drm_dp_dpcd_access() since it also retries everything 7 times and already has 400ms sleep. Just one minor comment: why not use usleep_range() instead of udelay()? > > -T > > --- > drivers/gpu/drm/i915/intel_dp.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 0e207aaf..f0664cd 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -573,8 +573,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, > DP_AUX_CH_CTL_RECEIVE_ERROR); > > if (status & (DP_AUX_CH_CTL_TIME_OUT_ERROR | > - DP_AUX_CH_CTL_RECEIVE_ERROR)) > + DP_AUX_CH_CTL_RECEIVE_ERROR)) { > + /* DP compliance requires 400us delay for errors/timeouts > + (DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2) */ > + udelay(400); > continue; > + } > if (status & DP_AUX_CH_CTL_DONE) > break; > } > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing listIntel-gfx@lists.freedesktop.orghttp://lists.freedesktop.org/mailman/listinfo/intel-gfx > > Todd Previte <tprevite@gmail.com> > Monday, July 14, 2014 12:10 PM > The Displayport Link Layer Compliance Testing Specification 1.2 rev 1.1 > specifies that > repeated AUX transactions after a failure (NACK, DEFER or no response) > must have > a minimum delay of 400us before the resend can occur. Tests 4.2.1.1 and > 4.2.1.2 are two > tests that require this specifically. > > Signed-off-by: Todd Previte <tprevite@gmail.com> <tprevite@gmail.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > b/drivers/gpu/drm/i915/intel_dp.c > index 0e207aaf..f0664cd 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -573,8 +573,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, > DP_AUX_CH_CTL_RECEIVE_ERROR); > > if (status & (DP_AUX_CH_CTL_TIME_OUT_ERROR | > - DP_AUX_CH_CTL_RECEIVE_ERROR)) > + DP_AUX_CH_CTL_RECEIVE_ERROR)) { > + /* DP compliance requires 400us delay for errors/timeouts > + (DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2) */ > + udelay(400); > continue; > + } > if (status & DP_AUX_CH_CTL_DONE) > break; > } > Todd Previte <tprevite@gmail.com> > Monday, July 14, 2014 12:10 PM > > V2: > - Addressed review feedback from the mailing list > - Broke up patches into smaller, easily managed chunks > - Reordered the patches such that they can be applied in order > - Fixed checkpatch.pl errors across the patchset > - Updated and enhanced functionality for the EDID test function > - Completely revamped the mode set operations for compliance testing > > > -- > Sent with Postbox <http://www.getpostbox.com> > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 0e207aaf..f0664cd 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -573,8 +573,12 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, DP_AUX_CH_CTL_RECEIVE_ERROR); if (status & (DP_AUX_CH_CTL_TIME_OUT_ERROR | - DP_AUX_CH_CTL_RECEIVE_ERROR)) + DP_AUX_CH_CTL_RECEIVE_ERROR)) { + /* DP compliance requires 400us delay for errors/timeouts + (DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2) */ + udelay(400); continue; + } if (status & DP_AUX_CH_CTL_DONE) break; }
The Displayport Link Layer Compliance Testing Specification 1.2 rev 1.1 specifies that repeated AUX transactions after a failure (NACK, DEFER or no response) must have a minimum delay of 400us before the resend can occur. Tests 4.2.1.1 and 4.2.1.2 are two tests that require this specifically. Signed-off-by: Todd Previte <tprevite@gmail.com> --- drivers/gpu/drm/i915/intel_dp.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)