Re: [PATCH 06/15] drm/dp: Read AUX read interval from DPCD

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2854 bytes --]

On Fri, Oct 18, 2019 at 05:27:59PM -0400, Lyude Paul wrote:
> On Tue, 2019-10-15 at 16:35 +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Store the AUX read interval from DPCD, so that it can be used to wait
> > for the durations given in the specification during link training.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  include/drm/drm_dp_helper.h | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> > 
> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> > index 7a537ffc2fb1..6c12de6f7e46 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -25,8 +25,11 @@
> >  
> >  #include <linux/delay.h>
> >  #include <linux/i2c.h>
> > +#include <linux/time64.h>
> >  #include <linux/types.h>
> >  
> > +#include <drm/drm_print.h>
> > +
> >  /*
> >   * Unless otherwise noted, all values are from the DP 1.1a spec.  Note that
> >   * DP and DPCD versions are independent.  Differences from 1.0 are not
> > noted,
> > @@ -1297,6 +1300,36 @@ drm_dp_alternate_scrambler_reset_cap(const u8
> > dpcd[DP_RECEIVER_CAP_SIZE])
> >  			DP_ALTERNATE_SCRAMBLER_RESET_CAP;
> >  }
> >  
> > +/**
> > + * drm_dp_read_aux_interval() - read the AUX read interval from the DPCD
> > + * @dpcd: receiver capacity buffer
> > + *
> > + * Reads the AUX read interval (in microseconds) from the DPCD. Note that
> > the
> > + * TRAINING_AUX_RD_INTERVAL stores the value in units of 4 milliseconds. If
> > no
> > + * read interval is specified and for DPCD v1.4 and later, the read
> > interval
> > + * is always 100 microseconds.
> > + *
> > + * Returns:
> > + * The read AUX interval in microseconds.
> > + */
> > +static inline unsigned int
> > +drm_dp_aux_rd_interval(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
> > +{
> > +	unsigned int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> > +					DP_TRAINING_AUX_RD_MASK;
> > +
> > +	if (rd_interval > 4)
> > +		DRM_DEBUG_KMS("AUX interval %u, out of range (max: 4)\n",
> > +			      rd_interval);
> Do you think it might be worth clamping the value to 4 here?

Yeah, I think that makes sense. It might also be worth increasing the
log level for this, perhaps even make it a WARN to make sure we catch
it when this happens.

> > +
> > +	if (rd_interval > 0 && dpcd[DP_DPCD_REV] < DP_DPCD_REV_14)
> 
> Also small  nit pick: you can just use rd_interval instead of rd_interval > 0

Yeah, I suppose so. I thought > 0 was making it really explicit, but I
don't really mind either way.

Thierry

> > +		rd_interval *= 4 * USEC_PER_MSEC;
> > +	else
> > +		rd_interval = 100;
> > +
> > +	return rd_interval;
> > +}
> > +
> >  /*
> >   * DisplayPort AUX channel
> >   */
> -- 
> Cheers,
> 	Lyude Paul
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel