Re: [PATCH 1/4] drm/i915/dp: retry i2c-over-aux seven times on AUX DEFER

Daniel Vetter
2013-09-27T19:59:36Z

On Fri, Sep 27, 2013 at 12:07:29PM -0700, Todd Previte wrote:
> Yep. Good to go.
>
> Reviewed-by: Todd Previte <tprevite@gmail.com>

Queued for -next, thanks for the patch.
-Daniel
>
>
> On Fri, Sep 27, 2013 at 4:51 AM, Jani Nikula <jani.nikula@linux.intel.com>wrote:
>
> > On Fri, 20 Sep 2013, Todd Previte <tprevite@gmail.com> wrote:
> > > On 09/20/2013 06:42 AM, Jani Nikula wrote:
> > >> Per DP1.2 spec.
> > >>
> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > >> ---
> > >> drivers/gpu/drm/i915/intel_dp.c | 7 ++++++-
> > >> 1 file changed, 6 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > >> index 9770160..6626514 100644
> > >> --- a/drivers/gpu/drm/i915/intel_dp.c
> > >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> > >> @@ -654,7 +654,12 @@ intel_dp_i2c_aux_ch(struct i2c_adapter *adapter,
> > int mode,
> > >> break;
> > >> }
> > >>
> > >> - for (retry = 0; retry < 5; retry++) {
> > >> + /*
> > >> + * DP1.2 sections 2.7.7.1.5.6.1 and 2.7.7.1.6.6.1: A DP Source
> > device is
> > >> + * required to retry at least seven times upon receiving AUX_DEFER
> > >> + * before giving up the AUX transaction.
> > >> + */
> > >> + for (retry = 0; retry < 7; retry++) {
> > >> ret = intel_dp_aux_ch(intel_dp,
> > >> msg, msg_bytes,
> > >> reply, reply_bytes);
> > >>
> > > Hey Jani,
> > >
> > > Although it's not explicitly stated in the specification (I went through
> > > both the 1.1a and 1.2a specifications and couldn't find it), I think the
> > > the retry count of 7 applies to all AUX transactions, both native and
> > > I2C. That's alluded to in the description of the link training sequence
> > > (pg 382, 2nd paragraph from the bottom) where the sink may defer up to
> > > seven times before the source can abort training.
> > >
> > > With that in mind, it might be a good idea to use a defined constant for
> > > the retry count for both native and I2C AUX transactions. That would
> > > keep it consistent throughout the code and be easier to update moving
> > > forward should the specification change.
> >
> > Todd, are you willing to give your reviewed-by on this one patch per our
> > irc discussion? We can do further cleanups later.
> >
> > I've sent a new version of 3/4 with the #defines separately.
> >
> >
> > BR,
> > Jani.
> >
> >
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
> >

> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch