Re: [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4

["Atwood, Matthew S" <matthew.s.atwood@intel.com>]

On Thu, 2018-03-08 at 09:22 +0200, Jani Nikula wrote:
> On Wed, 07 Mar 2018, matthew.s.atwood@intel.com wrote:
> > 
> > From: Matt Atwood <matthew.s.atwood@intel.com>
> > 
> > DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheeme
> > from 8
> > bits to 7 in DPCD 0x000e. The 8th bit is used to identify extended
> > receiver capabilities. For panels that use this new feature wait
> > interval
> > would be increased by 512 ms, when spec is max 16 ms. This behavior
> > is
> > described in table 2-158 of DP 1.4 spec address 0000eh.
> > 
> > With the introduction of DP 1.4 spec main link clock recovery was
> > standardized to 100 us regardless of TRAINING_AUX_RD_INTERVAL
> > value.
> > 
> > To avoid breaking panels that are not spec compiant we now warn on
> > invalid values.
> > 
> > V2: commit title/message, masking all 7 bits, warn on out of spec
> > values.
> > V3: commit message, make link train clock recovery follow DP 1.4
> > spec.
> > V4: style changes
> > V5: typo
> > 
> > Signed-off-by: Matt Atwood <matthew.s.atwood@intel.com>
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++----
> >  include/drm/drm_dp_helper.h     |  6 ++++++
> >  2 files changed, 20 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c
> > b/drivers/gpu/drm/drm_dp_helper.c
> > index adf79be..cdb04c9 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -119,18 +119,28 @@ u8
> > drm_dp_get_adjust_request_pre_emphasis(const u8
> > link_status[DP_LINK_STATUS_SI
> >  EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
> >  
> >  void drm_dp_link_train_clock_recovery_delay(const u8
> > dpcd[DP_RECEIVER_CAP_SIZE]) {
> > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> > DP_TRAINING_AUX_RD_MASK;
> > +
> > +	if (rd_interval > 4)
> > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max
> > 4)", rd_interval);
> \n missing.
will do
> 
> > 
> > +
> > +	if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DP_REV_14))
> >  		udelay(100);
> >  	else
> > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> > +		mdelay(rd_interval * 4);
> >  }
> >  EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
> >  
> >  void drm_dp_link_train_channel_eq_delay(const u8
> > dpcd[DP_RECEIVER_CAP_SIZE]) {
> > -	if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> > +	int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> > DP_TRAINING_AUX_RD_MASK;
> > +
> > +	if (rd_interval > 4)
> > +		DRM_DEBUG_KMS("AUX interval %d, out of range (max
> > 4)", rd_interval);
> \n missing.
will do
> 
> > 
> > +
> > +	if (rd_interval == 0)
> >  		udelay(400);
> >  	else
> > -		mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
> > +		mdelay(rd_interval * 4);
> >  }
> >  EXPORT_SYMBOL(drm_dp_link_train_channel_eq_delay);
> >  
> > diff --git a/include/drm/drm_dp_helper.h
> > b/include/drm/drm_dp_helper.h
> > index da58a42..1269ef8 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -64,6 +64,11 @@
> >  /* AUX CH addresses */
> >  /* DPCD */
> >  #define DP_DPCD_REV                         0x000
> > +# define DP_REV_10                          0x10
> > +# define DP_REV_11                          0x11
> > +# define DP_REV_12                          0x12
> > +# define DP_REV_13                          0x13
> > +# define DP_REV_14                          0x14
> I am not sure what good these buy us, but if people think they're the
> way to go, then so be it. Just bear in mind that per spec, "The DPCD
> revision number does not necessarily match the DisplayPort version
> number." so "DP_REV" doesn't actually mean *DP* revision.
> 
> 
> BR,
> Jani.
you're right likely a better name is DPCD_REV_XX. I think we sill want
to base the main-link clock recovery on time on this value. Next
revision will include this naming convention. 
> 
> > 
> >  
> >  #define DP_MAX_LINK_RATE                    0x001
> >  
> > @@ -118,6 +123,7 @@
> >  # define DP_DPCD_DISPLAY_CONTROL_CAPABLE     (1 << 3) /* edp v1.2
> > or higher */
> >  
> >  #define DP_TRAINING_AUX_RD_INTERVAL         0x00e   /* XXX 1.2? */
> > +# define DP_TRAINING_AUX_RD_MASK            0x7F    /* 1.3 */
Rodrigo has shown me a DP 1.2 spec that had this change and conflicts
with my copy so I'll be changing to XXX 1.2

Matt
> >  
> >  #define DP_ADAPTER_CAP			    0x00f   /* 1.2
> > */
> >  # define DP_FORCE_LOAD_SENSE_CAP	    (1 << 0)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx