On Wed, 2021-02-10 at 09:55 +0100, Thomas Zimmermann wrote:
Hi
Am 10.02.21 um 09:33 schrieb Khaled Almahallawy:
The number of AUX retries specified in the DP specs is 7. Currently, to make Dell 4k monitors happier, the number of retries are 32. i915 also retries 5 times (intel_dp_aux_xfer) which means in the case of AUX timeout we actually retries 32 * 5 = 160 times.
So making the number of aux retires a variable to allow for fine tuning and optimization of aux timing.
Signed-off-by: Khaled Almahallawy khaled.almahallawy@intel.com
drivers/gpu/drm/drm_dp_helper.c | 10 +++------- include/drm/drm_dp_helper.h | 1 + 2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index eedbb48815b7..8fdf57b4a06c 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -249,13 +249,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
mutex_lock(&aux->hw_mutex);
- /*
* The specification doesn't give any recommendation on howoften to
* retry native transactions. We used to retry 7 times like for* aux i2c transactions but real world devices this wasn't* sufficient, bump to 32 which makes Dell 4k monitors happier.*/- for (retry = 0; retry < 32; retry++) {
- for (retry = 0; retry < aux->num_retries; retry++) { if (ret != 0 && ret != -ETIMEDOUT) { usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100);
@@ -1744,6 +1738,8 @@ void drm_dp_aux_init(struct drm_dp_aux *aux) aux->ddc.retries = 3;
aux->ddc.lock_ops = &drm_dp_i2c_lock_ops;
- /*Still making the Dell 4k monitors happier*/
The original comment was helpful; this one isn't.
Noted and I apologize for the comment. Was just copy/past original comment.
Besides that, what problem does this patchset address? Too much probing?
The problem mainly with disconnect. When disconnecting, AUX read will fail because of timing out. The 32 retries cause the disconnect flow especially for Type-C/TBT Docks with MST hubs taking a long time. Just trying to reduce this disconnect time. In addition as I noted, i915 does retry in addition to retries by this function.
Currently this function retry for 4 AUX situations: AUX_NAK, AUX_DEFER, I/O error reported by driver and AUX timeout.
If I connect a Dell monitor to an Intel card, how often does it have to probe?
I guess it depends on how you connect (direct, behind MST hub, behind DOCK with MST). But I believe the most time consuming is the disconnect flow.
Thanks Khaled
Best regards Thomas
- aux->num_retries = 32; } EXPORT_SYMBOL(drm_dp_aux_init);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index edffd1dcca3e..16cbfc8f5e66 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -1876,6 +1876,7 @@ struct drm_dp_aux { struct mutex hw_mutex; struct work_struct crc_work; u8 crc_count;
- int num_retries; ssize_t (*transfer)(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg); /**