From: Matt Atwood matthew.s.atwood@intel.com
For panels that do not follow Display Port specifications mask off invalid values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e describes another feature. Currently the code uses all of DPCD 0x0000e and can cause max wait for 1024 ms instead of 16 ms as specified table 2-158. This address is read for both clock recovery and channel equalization.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com --- drivers/gpu/drm/drm_dp_helper.c | 4 ++-- include/drm/drm_dp_helper.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a7e9b75 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) udelay(100); else - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); + mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4); } EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
@@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) udelay(400); else - mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4); + mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 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..77ba003 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,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 0x7 /* 1.4 */
#define DP_ADAPTER_CAP 0x00f /* 1.2 */ # define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
On Fri, Mar 02, 2018 at 02:25:58PM -0800, matthew.s.atwood@intel.com wrote:
From: Matt Atwood matthew.s.atwood@intel.com
For panels that do not follow Display Port specifications mask off invalid values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e describes another feature. Currently the code uses all of DPCD 0x0000e and can cause max wait for 1024 ms instead of 16 ms as specified table 2-158. This address is read for both clock recovery and channel equalization.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
drivers/gpu/drm/drm_dp_helper.c | 4 ++-- include/drm/drm_dp_helper.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a7e9b75 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) udelay(100); else
mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4);} EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
@@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) udelay(400); else
mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 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..77ba003 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,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 0x7 /* 1.4 */
I confirmed it is already part of 1.2 and 1.3 So probably a good idea to already to s/XXX 1.2?/1.2/ but this is optional and up to you.
With s/1.4/1.2 feel free to add:
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
(well... in a hope that no other bad panel uses 5,6 or 7, that are also reserved values. But I believe these cases shouldn't be as bad as this one you faced here anyways)
#define DP_ADAPTER_CAP 0x00f /* 1.2 */
# define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Fri, 02 Mar 2018, Rodrigo Vivi rodrigo.vivi@intel.com wrote:
On Fri, Mar 02, 2018 at 02:25:58PM -0800, matthew.s.atwood@intel.com wrote:
From: Matt Atwood matthew.s.atwood@intel.com
For panels that do not follow Display Port specifications mask off invalid values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e describes another feature. Currently the code uses all of DPCD 0x0000e and can cause max wait for 1024 ms instead of 16 ms as specified table 2-158. This address is read for both clock recovery and channel equalization.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
drivers/gpu/drm/drm_dp_helper.c | 4 ++-- include/drm/drm_dp_helper.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a7e9b75 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) udelay(100); else
mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4);} EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
@@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) udelay(400); else
mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 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..77ba003 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,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 0x7 /* 1.4 */
I confirmed it is already part of 1.2 and 1.3 So probably a good idea to already to s/XXX 1.2?/1.2/ but this is optional and up to you.
With s/1.4/1.2 feel free to add:
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
Please send v2 Cc: intel-gfx so we get i915 CI on this. It doesn't automatically happen on dri-devel yet.
BR, Jani.
(well... in a hope that no other bad panel uses 5,6 or 7, that are also reserved values. But I believe these cases shouldn't be as bad as this one you faced here anyways)
#define DP_ADAPTER_CAP 0x00f /* 1.2 */
# define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Thanks for the patch. Masking the training AUX RD interval value to get only the allowed values sounds of 0-4 is absolutely needed. Just one nit below:
On Fri, Mar 02, 2018 at 02:25:58PM -0800, matthew.s.atwood@intel.com wrote:
From: Matt Atwood matthew.s.atwood@intel.com
For panels that do not follow Display Port specifications mask off invalid values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable
It would be good to call out DP specification and a particular version number if possible So DP 1.2 specification.
With that change, Reviewed-by: Manasi Navare manasi.d.navare@intel.com
values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e describes another feature. Currently the code uses all of DPCD 0x0000e and can cause max wait for 1024 ms instead of 16 ms as specified table 2-158. This address is read for both clock recovery and channel equalization.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
drivers/gpu/drm/drm_dp_helper.c | 4 ++-- include/drm/drm_dp_helper.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a7e9b75 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) udelay(100); else
mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4);} EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
@@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) udelay(400); else
mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 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..77ba003 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,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 0x7 /* 1.4 */
#define DP_ADAPTER_CAP 0x00f /* 1.2 */
# define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Matt,
On Fri, Mar 02, 2018 at 02:25:58PM -0800, matthew.s.atwood@intel.com wrote:
From: Matt Atwood matthew.s.atwood@intel.com
For panels that do not follow Display Port specifications mask off invalid values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e describes another feature. Currently the code uses all of DPCD 0x0000e and can cause max wait for 1024 ms instead of 16 ms as specified table 2-158. This address is read for both clock recovery and channel equalization.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
Tested-by: Benson Leung bleung@chromium.org
I tested this patch on that particularly problematic panel, with the out of spec value of 9. With the masking, the panel link trains successfully.
Thanks!
On Fri, 02 Mar 2018, matthew.s.atwood@intel.com wrote:
From: Matt Atwood matthew.s.atwood@intel.com
For panels that do not follow Display Port specifications mask off invalid values for DP_TRAINING_AUX_RD_INTERVAL. Specification lists acceptable values 0-4 all other values are reserved and bit 7 of DPCD 0x0000e describes another feature. Currently the code uses all of DPCD 0x0000e and can cause max wait for 1024 ms instead of 16 ms as specified table 2-158. This address is read for both clock recovery and channel equalization.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
Cc: stable@vger.kernel.org
drivers/gpu/drm/drm_dp_helper.c | 4 ++-- include/drm/drm_dp_helper.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a7e9b75 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -122,7 +122,7 @@ void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) udelay(100); else
mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 4);} EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay);
@@ -130,7 +130,7 @@ void drm_dp_link_train_channel_eq_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0) udelay(400); else
mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
mdelay((dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK) * 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..77ba003 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,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 0x7 /* 1.4 */
#define DP_ADAPTER_CAP 0x00f /* 1.2 */ # define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
From: Matt Atwood matthew.s.atwood@intel.com
DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheme from 8 bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for panels that use this new feature, this would cause a wait interval for clock recovery of at least 512 ms, much higher then spec maximum of 16 ms. This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh. To avoid breaking panels that are not spec compliant we now warn on invalid values.
V2: commit title/message, masking all 7 bits, warn on out of spec values.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com --- drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++---- include/drm/drm_dp_helper.h | 1 + 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a718ccc 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) 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..f80acf1 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,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)
Hi Matt,
On Tue, Mar 06, 2018 at 10:37:48AM -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 scheme from 8 bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for panels that use this new feature, this would cause a wait interval for clock recovery of at least 512 ms, much higher then spec maximum of 16 ms. This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh. To avoid breaking panels that are not spec compliant we now warn on invalid values.
V2: commit title/message, masking all 7 bits, warn on out of spec values.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
Tested-by: Benson Leung bleung@chromium.org
Tested this patch on a DP 1.3 panel which sets the EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT bit in DPCD 0000Eh. It has a value of 0x80 in that field, indicating the extended caps, and 400us for the Main-Link Channel Equalization phase.
Confirmed that link training passes normally where prior to this it would fail after the driver waits too long.
Thanks for the fix!
On Tue, Mar 06, 2018 at 10:37:48AM -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 scheme from 8 bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for panels that use this new feature, this would cause a wait interval for clock recovery of at least 512 ms, much higher then spec maximum of 16 ms. This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh. To avoid breaking panels that are not spec compliant we now warn on invalid values.
V2: commit title/message, masking all 7 bits, warn on out of spec values.
this approach is even better imho.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++---- include/drm/drm_dp_helper.h | 1 + 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a718ccc 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) 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..f80acf1 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,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)
2.7.4
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 2018-03-06 at 15:24 -0800, Rodrigo Vivi wrote:
On Tue, Mar 06, 2018 at 10:37:48AM -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 scheme from 8 bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for panels that use this new feature, this would cause a wait interval for clock recovery of at least 512 ms, much higher then spec maximum of 16 ms. This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh. To avoid breaking panels
See comment below:
that are not spec compliant we now warn on
invalid values.
V2: commit title/message, masking all 7 bits, warn on out of spec values.
this approach is even better imho.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++---- include/drm/drm_dp_helper.h | 1 + 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a718ccc 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);
Some default for panels without a valid value? rd_interval = 4; "AUX read interval out of range, using max %d ms"
- if (rd_interval == 0) 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..f80acf1 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,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)
2.7.4
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 2018-03-06 at 16:48 -0800, Dhinakaran Pandiyan wrote:
On Tue, 2018-03-06 at 15:24 -0800, Rodrigo Vivi wrote:
On Tue, Mar 06, 2018 at 10:37:48AM -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 scheme from 8 bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for panels that use this new feature, this would cause a wait interval for clock recovery of at least 512 ms, much higher then spec maximum of 16 ms. This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh. To avoid breaking panels
See comment below:
that are not spec compliant we now warn on
invalid values.
V2: commit title/message, masking all 7 bits, warn on out of spec values.
this approach is even better imho.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++---- include/drm/drm_dp_helper.h | 1 + 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a718ccc 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);Some default for panels without a valid value? rd_interval = 4; "AUX read interval out of range, using max %d ms"
- if (rd_interval == 0) udelay(100); else
mdelay(dpcd[DP_TRAINING_AUX_RD_INTERVAL] * 4);
mdelay(rd_interval * 4);
Btw, DP 1.4 spec also says this is 100 us for *all* values in the register. Updating that for DP 1.4 panels should speed up link training.
} 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..f80acf1 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,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)
2.7.4
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Mar 07, 2018 at 12:24:46AM +0000, Pandiyan, Dhinakaran wrote:
On Tue, 2018-03-06 at 15:24 -0800, Rodrigo Vivi wrote:
On Tue, Mar 06, 2018 at 10:37:48AM -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 scheme from 8 bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for panels that use this new feature, this would cause a wait interval for clock recovery of at least 512 ms, much higher then spec maximum of 16 ms. This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh. To avoid breaking panels
See comment below:
that are not spec compliant we now warn on
invalid values.
V2: commit title/message, masking all 7 bits, warn on out of spec values.
this approach is even better imho.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++---- include/drm/drm_dp_helper.h | 1 + 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a718ccc 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);Some default for panels without a valid value? rd_interval = 4; "AUX read interval out of range, using max %d ms"
The problem with setting the upper bound to 4 is that there are panels that do not follow the spec and expect a longer than 16 ms delay. So if we set the upper bound to 4 in those cases the panels might not work.
So we decided to go with this approach where we tell the users that panel is requesting out of range AUX value but then set it to the value * 4 in the else part.
Manasi
- if (rd_interval == 0) 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..f80acf1 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,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)
2.7.4
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 2018-03-06 at 17:36 -0800, Manasi Navare wrote:
On Wed, Mar 07, 2018 at 12:24:46AM +0000, Pandiyan, Dhinakaran wrote:
On Tue, 2018-03-06 at 15:24 -0800, Rodrigo Vivi wrote:
On Tue, Mar 06, 2018 at 10:37:48AM -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 scheme from 8 bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for panels that use this new feature, this would cause a wait interval for clock recovery of at least 512 ms, much higher then spec maximum of 16 ms. This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh. To avoid breaking panels
See comment below:
that are not spec compliant we now warn on
invalid values.
V2: commit title/message, masking all 7 bits, warn on out of spec values.
this approach is even better imho.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++---- include/drm/drm_dp_helper.h | 1 + 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a718ccc 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);Some default for panels without a valid value? rd_interval = 4; "AUX read interval out of range, using max %d ms"
The problem with setting the upper bound to 4 is that there are panels that do not follow the spec and expect a longer than 16 ms delay. So if we set the upper bound to 4 in those cases the panels might not work.
So we decided to go with this approach where we tell the users that panel is requesting out of range AUX value but then set it to the value * 4 in the else part.
Thanks for the clarification. My concern is if the DPCD is advertizing an out of spec value, it might as well be advertizing a delay that the panel doesn't need. And I thought panel quirks were supposed to be used for working around things like this. To be clear, this is not a big enough concern to block this fix.
Like I said in the other email, this patch refers to DP 1.4, shouldn't the clock recovery delay be updated too (in a separate patch)?
Manasi
- if (rd_interval == 0) 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..f80acf1 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,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)
2.7.4
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Mar 07, 2018 at 02:13:21AM +0000, Pandiyan, Dhinakaran wrote:
On Tue, 2018-03-06 at 17:36 -0800, Manasi Navare wrote:
On Wed, Mar 07, 2018 at 12:24:46AM +0000, Pandiyan, Dhinakaran wrote:
On Tue, 2018-03-06 at 15:24 -0800, Rodrigo Vivi wrote:
On Tue, Mar 06, 2018 at 10:37:48AM -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 scheme from 8 bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for panels that use this new feature, this would cause a wait interval for clock recovery of at least 512 ms, much higher then spec maximum of 16 ms. This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh. To avoid breaking panels
See comment below:
that are not spec compliant we now warn on
invalid values.
V2: commit title/message, masking all 7 bits, warn on out of spec values.
this approach is even better imho.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++---- include/drm/drm_dp_helper.h | 1 + 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a718ccc 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);Some default for panels without a valid value? rd_interval = 4; "AUX read interval out of range, using max %d ms"
The problem with setting the upper bound to 4 is that there are panels that do not follow the spec and expect a longer than 16 ms delay. So if we set the upper bound to 4 in those cases the panels might not work.
So we decided to go with this approach where we tell the users that panel is requesting out of range AUX value but then set it to the value * 4 in the else part.
Thanks for the clarification. My concern is if the DPCD is advertizing an out of spec value, it might as well be advertizing a delay that the panel doesn't need. And I thought panel quirks were supposed to be used for working around things like this. To be clear, this is not a big enough concern to block this fix.
Like I said in the other email, this patch refers to DP 1.4, shouldn't the clock recovery delay be updated too (in a separate patch)?
We clearly need more work here.
I can see here on DP-v1.2a_d11:
00h = 100us for the Main Link Clock Recovery phase 400us for the Main Link Channel Equalization phase and for FAUX training. 01h = 4ms all. 02h = 8ms all. 03h = 12ms all. 04h = 16ms all.
So probably the initial mask on this patch should be marked with /* XXX 1.2? */ because it clearly got introduced in some 1.2 minor release.
But even for DP 1.2 it doesn't seem we are doing it right on the 0 case. It seems that we are using 100us for both channel eq and clock recovery, right? or am I missing something?
Then DP 1.3 keeps same config.
But DP 1.4 change all values.
clock recovery is always 100us and channel eq is depending on this bit * 4 and 400us when bit is zeroed.
But limited to 4.
So we probably need 3 patches here: 1. - This one to protect against bad panels masking it and mentioning DP 1.2, nor 1.3 or 1.4. Also limiting rd_interval to 4 as DK suggested. Panels cannot expect all drivers are using this value * 4 blindly since it is not on spec. 2. - Fix channel eq for 0 case since 1.2. It should be 400us. 3. - For DP version >= 1.4 always use 100us for clock req or follow this register for channel eq.
Thoughts?
Manasi
- if (rd_interval == 0) 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..f80acf1 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,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)
2.7.4
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Mar 07, 2018 at 02:06:08PM -0800, Rodrigo Vivi wrote:
On Wed, Mar 07, 2018 at 02:13:21AM +0000, Pandiyan, Dhinakaran wrote:
On Tue, 2018-03-06 at 17:36 -0800, Manasi Navare wrote:
On Wed, Mar 07, 2018 at 12:24:46AM +0000, Pandiyan, Dhinakaran wrote:
On Tue, 2018-03-06 at 15:24 -0800, Rodrigo Vivi wrote:
On Tue, Mar 06, 2018 at 10:37:48AM -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 scheme from 8 bits to 7 bits in DPCD 0x000e. The 8th bit describes a new feature, for panels that use this new feature, this would cause a wait interval for clock recovery of at least 512 ms, much higher then spec maximum of 16 ms. This behavior is described in table 2-158 of DP 1.4 spec address 0000Eh. To avoid breaking panels
See comment below:
that are not spec compliant we now warn on
invalid values.
V2: commit title/message, masking all 7 bits, warn on out of spec values.
this approach is even better imho.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++---- include/drm/drm_dp_helper.h | 1 + 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..a718ccc 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);Some default for panels without a valid value? rd_interval = 4; "AUX read interval out of range, using max %d ms"
The problem with setting the upper bound to 4 is that there are panels that do not follow the spec and expect a longer than 16 ms delay. So if we set the upper bound to 4 in those cases the panels might not work.
So we decided to go with this approach where we tell the users that panel is requesting out of range AUX value but then set it to the value * 4 in the else part.
Thanks for the clarification. My concern is if the DPCD is advertizing an out of spec value, it might as well be advertizing a delay that the panel doesn't need. And I thought panel quirks were supposed to be used for working around things like this. To be clear, this is not a big enough concern to block this fix.
Like I said in the other email, this patch refers to DP 1.4, shouldn't the clock recovery delay be updated too (in a separate patch)?
We clearly need more work here.
I can see here on DP-v1.2a_d11:
00h = 100us for the Main Link Clock Recovery phase 400us for the Main Link Channel Equalization phase and for FAUX training. 01h = 4ms all. 02h = 8ms all. 03h = 12ms all. 04h = 16ms all.
So probably the initial mask on this patch should be marked with /* XXX 1.2? */ because it clearly got introduced in some 1.2 minor release.
But even for DP 1.2 it doesn't seem we are doing it right on the 0 case. It seems that we are using 100us for both channel eq and clock recovery, right? or am I missing something?
Then DP 1.3 keeps same config.
But DP 1.4 change all values.
clock recovery is always 100us and channel eq is depending on this bit * 4 and 400us when bit is zeroed.
But limited to 4.
So we probably need 3 patches here:
- This one to protect against bad panels masking it and mentioning DP 1.2, nor 1.3 or 1.4. Also limiting rd_interval to 4 as DK suggested. Panels cannot expect all drivers are using this value * 4 blindly since it is not on spec.
So if some panels still expect a greater delay, those will fail link training. But yes if we want them to be DP compliant, just follow the spec, limit it to the max value of 4 with a warning.
- Fix channel eq for 0 case since 1.2. It should be 400us.
Channel eq is 400 us for DP 1.2, 1.3 and 1.4 and then *4 for all other values. We are doing that correctly here. So no change there.
- For DP version >= 1.4 always use 100us for clock req or follow this register for channel eq.
yes this needs to be fixed for DP REV >= 1.4
Manasi
Thoughts?
Manasi
- if (rd_interval == 0) 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..f80acf1 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -118,6 +118,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)
2.7.4
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com --- drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++---- include/drm/drm_dp_helper.h | 4 ++++ 2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..671b823 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..5bac397 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -64,6 +64,9 @@ /* AUX CH addresses */ /* DPCD */ #define DP_DPCD_REV 0x000 +# define DP_REV_12 0x012 +# define DP_REV_13 0x013 +# define DP_REV_14 0x014
#define DP_MAX_LINK_RATE 0x001
@@ -118,6 +121,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 Wed, Mar 7, 2018 at 6:44 PM, 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.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++---- include/drm/drm_dp_helper.h | 4 ++++ 2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..671b823 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))
Was this meant to be dpcd[DP_DPCD_REV] >= DP_REV_14? It doesn't appear to be a bitmask...
Also I think you're supposed to say "if (" rather than "if(".
-ilia
On Wed, Mar 07, 2018 at 03:44:09PM -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.
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
drivers/gpu/drm/drm_dp_helper.c | 18 ++++++++++++++---- include/drm/drm_dp_helper.h | 4 ++++ 2 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..671b823 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);
I thought there were comments about setting this to a max of 4 if its greater than 4.
- if(rd_interval == 0 || (dpcd[DP_DPCD_REV] & DP_REV_14))
^ space needed between if and open bracket
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..5bac397 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -64,6 +64,9 @@ /* AUX CH addresses */ /* DPCD */ #define DP_DPCD_REV 0x000 +# define DP_REV_12 0x012 +# define DP_REV_13 0x013 +# define DP_REV_14 0x014
IMHO, if we are creating these #defines for revisions then we should do it for all values so 0x10, 0x11.
Manasi
#define DP_MAX_LINK_RATE 0x001
@@ -118,6 +121,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)
2.7.4
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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
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..6985ff3 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)
Hey Matt,
Your patch doesn't build. Missing semicolon, dude.
On Wed, Mar 07, 2018 at 04:13:58PM -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
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..6985ff3 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)
Need a semicolon here.
/mnt/host/source/src/third_party/kernel/v4.14/drivers/gpu/drm/drm_dp_helper.c: In function 'drm_dp_link_train_clock_recovery_delay': /mnt/host/source/src/third_party/kernel/v4.14/drivers/gpu/drm/drm_dp_helper.c:131:1: error: expected ';' before '}' token } ^ make[4]: *** [/mnt/host/source/src/third_party/kernel/v4.14/scripts/Makefile.build:320: drivers/gpu/drm/drm_dp_helper.o] Error 1
Thanks, Benson
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); + + 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)
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 (max4)", 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 (max4)", 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 (max4)", 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 (max4)", 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
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 V6: print statement revisions, DP_REV to DPCD_REV, comment correction
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..392e92e 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)\n", 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)\n", 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..9afea9f 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 DPCD_REV_10 0x10 +# define DPCD_REV_11 0x11 +# define DPCD_REV_12 0x12 +# define DPCD_REV_13 0x13 +# define DPCD_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 /* XXX 1.2 */
#define DP_ADAPTER_CAP 0x00f /* 1.2 */ # define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
On Wed, Mar 14, 2018 at 10:40:08AM -0700, 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 V6: print statement revisions, DP_REV to DPCD_REV, comment correction
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..392e92e 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)\n", rd_interval);- if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DP_REV_14))
s/DP_REV_14/DPCD_REV_14 right?
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)\n", 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..9afea9f 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 DPCD_REV_10 0x10 +# define DPCD_REV_11 0x11 +# define DPCD_REV_12 0x12 +# define DPCD_REV_13 0x13 +# define DPCD_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 /* XXX 1.2 */
#define DP_ADAPTER_CAP 0x00f /* 1.2 */
# define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
2.7.4
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Matt,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on v4.16-rc4] [also build test WARNING on next-20180316] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/matthew-s-atwood-intel-com/drm-dp-C... config: i386-randconfig-x003-201810 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386
All warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:10:0, from drivers/gpu/drm/drm_dp_helper.c:23: drivers/gpu/drm/drm_dp_helper.c: In function 'drm_dp_link_train_clock_recovery_delay': drivers/gpu/drm/drm_dp_helper.c:127:48: error: 'DP_REV_14' undeclared (first use in this function); did you mean 'DPCD_REV_14'? if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DP_REV_14)) ^ include/linux/compiler.h:58:30: note: in definition of macro '__trace_if' if (__builtin_constant_p(!!(cond)) ? !!(cond) : \ ^~~~
drivers/gpu/drm/drm_dp_helper.c:127:2: note: in expansion of macro 'if'
if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DP_REV_14)) ^~ drivers/gpu/drm/drm_dp_helper.c:127:48: note: each undeclared identifier is reported only once for each function it appears in if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DP_REV_14)) ^ include/linux/compiler.h:58:30: note: in definition of macro '__trace_if' if (__builtin_constant_p(!!(cond)) ? !!(cond) : \ ^~~~
drivers/gpu/drm/drm_dp_helper.c:127:2: note: in expansion of macro 'if'
if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DP_REV_14)) ^~
vim +/if +127 drivers/gpu/drm/drm_dp_helper.c
23 #include <linux/kernel.h>
24 #include <linux/module.h> 25 #include <linux/delay.h> 26 #include <linux/init.h> 27 #include <linux/errno.h> 28 #include <linux/sched.h> 29 #include <linux/i2c.h> 30 #include <linux/seq_file.h> 31 #include <drm/drm_dp_helper.h> 32 #include <drm/drmP.h> 33 34 #include "drm_crtc_helper_internal.h" 35 36 /** 37 * DOC: dp helpers 38 * 39 * These functions contain some common logic and helpers at various abstraction 40 * levels to deal with Display Port sink devices and related things like DP aux 41 * channel transfers, EDID reading over DP aux channels, decoding certain DPCD 42 * blocks, ... 43 */ 44 45 /* Helpers for DP link training */ 46 static u8 dp_link_status(const u8 link_status[DP_LINK_STATUS_SIZE], int r) 47 { 48 return link_status[r - DP_LANE0_1_STATUS]; 49 } 50 51 static u8 dp_get_lane_status(const u8 link_status[DP_LINK_STATUS_SIZE], 52 int lane) 53 { 54 int i = DP_LANE0_1_STATUS + (lane >> 1); 55 int s = (lane & 1) * 4; 56 u8 l = dp_link_status(link_status, i); 57 return (l >> s) & 0xf; 58 } 59 60 bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE], 61 int lane_count) 62 { 63 u8 lane_align; 64 u8 lane_status; 65 int lane; 66 67 lane_align = dp_link_status(link_status, 68 DP_LANE_ALIGN_STATUS_UPDATED); 69 if ((lane_align & DP_INTERLANE_ALIGN_DONE) == 0) 70 return false; 71 for (lane = 0; lane < lane_count; lane++) { 72 lane_status = dp_get_lane_status(link_status, lane); 73 if ((lane_status & DP_CHANNEL_EQ_BITS) != DP_CHANNEL_EQ_BITS) 74 return false; 75 } 76 return true; 77 } 78 EXPORT_SYMBOL(drm_dp_channel_eq_ok); 79 80 bool drm_dp_clock_recovery_ok(const u8 link_status[DP_LINK_STATUS_SIZE], 81 int lane_count) 82 { 83 int lane; 84 u8 lane_status; 85 86 for (lane = 0; lane < lane_count; lane++) { 87 lane_status = dp_get_lane_status(link_status, lane); 88 if ((lane_status & DP_LANE_CR_DONE) == 0) 89 return false; 90 } 91 return true; 92 } 93 EXPORT_SYMBOL(drm_dp_clock_recovery_ok); 94 95 u8 drm_dp_get_adjust_request_voltage(const u8 link_status[DP_LINK_STATUS_SIZE], 96 int lane) 97 { 98 int i = DP_ADJUST_REQUEST_LANE0_1 + (lane >> 1); 99 int s = ((lane & 1) ? 100 DP_ADJUST_VOLTAGE_SWING_LANE1_SHIFT : 101 DP_ADJUST_VOLTAGE_SWING_LANE0_SHIFT); 102 u8 l = dp_link_status(link_status, i); 103 104 return ((l >> s) & 0x3) << DP_TRAIN_VOLTAGE_SWING_SHIFT; 105 } 106 EXPORT_SYMBOL(drm_dp_get_adjust_request_voltage); 107 108 u8 drm_dp_get_adjust_request_pre_emphasis(const u8 link_status[DP_LINK_STATUS_SIZE], 109 int lane) 110 { 111 int i = DP_ADJUST_REQUEST_LANE0_1 + (lane >> 1); 112 int s = ((lane & 1) ? 113 DP_ADJUST_PRE_EMPHASIS_LANE1_SHIFT : 114 DP_ADJUST_PRE_EMPHASIS_LANE0_SHIFT); 115 u8 l = dp_link_status(link_status, i); 116 117 return ((l >> s) & 0x3) << DP_TRAIN_PRE_EMPHASIS_SHIFT; 118 } 119 EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis); 120 121 void drm_dp_link_train_clock_recovery_delay(const u8 dpcd[DP_RECEIVER_CAP_SIZE]) { 122 int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & DP_TRAINING_AUX_RD_MASK; 123 124 if (rd_interval > 4) 125 DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)\n", rd_interval); 126
127 if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DP_REV_14))
128 udelay(100); 129 else 130 mdelay(rd_interval * 4); 131 } 132 EXPORT_SYMBOL(drm_dp_link_train_clock_recovery_delay); 133
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
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 V6: print statement revisions, DP_REV to DPCD_REV, comment correction V7: 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..793c0ff 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)\n", rd_interval); + + if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DPCD_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)\n", 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..9afea9f 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 DPCD_REV_10 0x10 +# define DPCD_REV_11 0x11 +# define DPCD_REV_12 0x12 +# define DPCD_REV_13 0x13 +# define DPCD_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 /* XXX 1.2 */
#define DP_ADAPTER_CAP 0x00f /* 1.2 */ # define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
On Wed, Mar 14, 2018 at 01:20:06PM -0700, 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 V6: print statement revisions, DP_REV to DPCD_REV, comment correction V7: typo
https://patchwork.freedesktop.org/series/39473/ Checkpatch noticed few lines like this over 80 char.
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..793c0ff 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;
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)\n", rd_interval);
DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)\n", rd_interval);
- if (rd_interval == 0 || (dpcd[DP_DPCD_REV] >= DPCD_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;
ditto
- if (rd_interval > 4)
DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)\n", rd_interval);
ditto
- 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..9afea9f 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 DPCD_REV_10 0x10 +# define DPCD_REV_11 0x11 +# define DPCD_REV_12 0x12 +# define DPCD_REV_13 0x13 +# define DPCD_REV_14 0x14
DP_DPCD_REV_ to match the reg name
#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 /* XXX 1.2 */
maybe add "?" to be in sync with the reg offset?
#define DP_ADAPTER_CAP 0x00f /* 1.2 */
# define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
2.7.4
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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 V6: print statement revisions, DP_REV to DPCD_REV, comment correction V7: typo V8: Style
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com --- drivers/gpu/drm/drm_dp_helper.c | 22 ++++++++++++++++++---- include/drm/drm_dp_helper.h | 6 ++++++ 2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..6bee2df 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -119,18 +119,32 @@ 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)\n", + rd_interval); + + if (rd_interval == 0 || dpcd[DP_DPCD_REV] >= DPCD_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)\n", + 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..8c59ce4 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 DPCD_REV_10 0x10 +# define DPCD_REV_11 0x11 +# define DPCD_REV_12 0x12 +# define DPCD_REV_13 0x13 +# define DPCD_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 /* XXX 1.2? */
#define DP_ADAPTER_CAP 0x00f /* 1.2 */ # define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
On Thu, Mar 15, 2018 at 02:08:51PM -0700, 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 V6: print statement revisions, DP_REV to DPCD_REV, comment correction V7: typo V8: Style
thanks :)
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
Reviewed-by: Rodrigo Vivi rodrigo.vivi@intel.com
drivers/gpu/drm/drm_dp_helper.c | 22 ++++++++++++++++++---- include/drm/drm_dp_helper.h | 6 ++++++ 2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index adf79be..6bee2df 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -119,18 +119,32 @@ 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)\n",rd_interval);- if (rd_interval == 0 || dpcd[DP_DPCD_REV] >= DPCD_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)\n",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..8c59ce4 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 DPCD_REV_10 0x10 +# define DPCD_REV_11 0x11 +# define DPCD_REV_12 0x12 +# define DPCD_REV_13 0x13 +# define DPCD_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 /* XXX 1.2? */
#define DP_ADAPTER_CAP 0x00f /* 1.2 */
# define DP_FORCE_LOAD_SENSE_CAP (1 << 0)
2.7.4
Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi Matt,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v4.16-rc4] [also build test ERROR on next-20180316] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/matthew-s-atwood-intel-com/drm-dp-C... config: x86_64-federa-25 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64
All errors (new ones prefixed by >>):
In file included from drivers/gpu/drm/amd/amdgpu/../display/include/dpcd_defs.h:29:0, from drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:42:
include/drm/drm_dp_helper.h:67:45: error: expected identifier before numeric constant
# define DPCD_REV_10 0x10 ^ drivers/gpu/drm/amd/amdgpu/../display/include/dpcd_defs.h:32:2: note: in expansion of macro 'DPCD_REV_10' DPCD_REV_10 = 0x10, ^~~~~~~~~~~
vim +67 include/drm/drm_dp_helper.h
63 64 /* AUX CH addresses */ 65 /* DPCD */ 66 #define DP_DPCD_REV 0x000
67 # define DPCD_REV_10 0x10
68 # define DPCD_REV_11 0x11 69 # define DPCD_REV_12 0x12 70 # define DPCD_REV_13 0x13 71 # define DPCD_REV_14 0x14 72
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Thu, Mar 15, 2018 at 02:08:51PM -0700, 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 V6: print statement revisions, DP_REV to DPCD_REV, comment correction V7: typo V8: Style
Signed-off-by: Matt Atwood matthew.s.atwood@intel.com
Tested-by: Benson Leung bleung@chromium.org
This version still passes link training on the panel with 8th bit set in DPCD 0x000e.
Thanks, Benson
dri-devel@lists.freedesktop.org
-
Atwood, Matthew S -
Benson Leung -
Ilia Mirkin -
Jani Nikula -
kbuild test robot -
Manasi Navare -
matthew.s.atwood＠intel.com
-
Pandiyan, Dhinakaran -
Rodrigo Vivi