| Message ID | 1520468931-11695-1-git-send-email-matthew.s.atwood@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Matt, On Wed, Mar 07, 2018 at 04:28:51PM -0800, 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> Tested-by: Benson Leung <bleung@chromium.org> V5 passes link training on that same panel from before with 8th bit set in DPCD 0x000e. Thanks, Benson
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. > + > + 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. > + > + 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. > > #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 */ > > #define DP_ADAPTER_CAP 0x00f /* 1.2 */ > # define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
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)
On Fri, Mar 09, 2018 at 11:49:44PM +0000, Atwood, Matthew S wrote: > 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. yep, I believe we need this anyways even without a necessarily match. And DPCD_REV_ seems better indeed. > > > > > > > > > > > #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 I thought you had convinced me otherwise that day. The other bit on this range didn't exist on this so the mask wouldn't be needed, but the max value there is anyways == 4 so it can apply anyways. but up to you how you want to proceed here. > > 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
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); + + 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); + + 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 #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 */ #define DP_ADAPTER_CAP 0x00f /* 1.2 */ # define DP_FORCE_LOAD_SENSE_CAP (1 << 0)