diff mbox

[1/2] drm/dp: retry AUX transactions 32 times

Message ID 1416971843-2122-1-git-send-email-airlied@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dave Airlie Nov. 26, 2014, 3:17 a.m. UTC
From: Dave Airlie <airlied@redhat.com>

At least on two MST devices I've tested with, when
they are link training downstream, they are totally
unable to handle aux ch msgs, so they defer like nuts.
I tried 16, it wasn't enough, 32 seems better.

This fixes one Dell 4k monitor and one of the
MST hubs.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tom Stellard Nov. 26, 2014, 3:20 a.m. UTC | #1
On Wed, Nov 26, 2014 at 01:17:22PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> At least on two MST devices I've tested with, when
> they are link training downstream, they are totally
> unable to handle aux ch msgs, so they defer like nuts.
> I tried 16, it wasn't enough, 32 seems better.
> 
> This fixes one Dell 4k monitor and one of the
> MST hubs.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 959e207..db0993f 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -189,7 +189,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>  	 * retry native transactions, so retry 7 times like for I2C-over-AUX

Should this comment be updated too?

-Tom

>  	 * transactions.
>  	 */
> -	for (retry = 0; retry < 7; retry++) {
> +	for (retry = 0; retry < 32; retry++) {
>  
>  		mutex_lock(&aux->hw_mutex);
>  		err = aux->transfer(aux, &msg);
> -- 
> 2.1.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Dave Airlie Nov. 26, 2014, 3:22 a.m. UTC | #2
On 26 November 2014 at 13:20, Tom Stellard <tom@stellard.net> wrote:
> On Wed, Nov 26, 2014 at 01:17:22PM +1000, Dave Airlie wrote:
>> From: Dave Airlie <airlied@redhat.com>
>>
>> At least on two MST devices I've tested with, when
>> they are link training downstream, they are totally
>> unable to handle aux ch msgs, so they defer like nuts.
>> I tried 16, it wasn't enough, 32 seems better.
>>
>> This fixes one Dell 4k monitor and one of the
>> MST hubs.
>>
>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>> ---
>>  drivers/gpu/drm/drm_dp_helper.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> index 959e207..db0993f 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -189,7 +189,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>>        * retry native transactions, so retry 7 times like for I2C-over-AUX
>
> Should this comment be updated too?

Yes it should, good catch!

Thanks,
Dave.
Jani Nikula Nov. 26, 2014, 9:24 a.m. UTC | #3
On Wed, 26 Nov 2014, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@redhat.com>
>
> At least on two MST devices I've tested with, when
> they are link training downstream, they are totally
> unable to handle aux ch msgs, so they defer like nuts.
> I tried 16, it wasn't enough, 32 seems better.

I think we have a bug with some dongle that defers a lot too. One idea
was to use a progressively longer delay for DP_AUX_NATIVE_REPLY_DEFER.

How about drm_dp_i2c_do_msg(), that one can receive native aux defer
too, along with i2c defer. It feels like both places should have the
same limit for native aux defer, but if you want to keep different i2c
defer retry limit it gets a bit trickier.

BR,
Jani.


>
> This fixes one Dell 4k monitor and one of the
> MST hubs.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 959e207..db0993f 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -189,7 +189,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>  	 * retry native transactions, so retry 7 times like for I2C-over-AUX
>  	 * transactions.
>  	 */
> -	for (retry = 0; retry < 7; retry++) {
> +	for (retry = 0; retry < 32; retry++) {
>  
>  		mutex_lock(&aux->hw_mutex);
>  		err = aux->transfer(aux, &msg);
> -- 
> 2.1.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Dave Airlie Dec. 8, 2014, 11:55 p.m. UTC | #4
On 26 November 2014 at 19:24, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Wed, 26 Nov 2014, Dave Airlie <airlied@gmail.com> wrote:
>> From: Dave Airlie <airlied@redhat.com>
>>
>> At least on two MST devices I've tested with, when
>> they are link training downstream, they are totally
>> unable to handle aux ch msgs, so they defer like nuts.
>> I tried 16, it wasn't enough, 32 seems better.
>
> I think we have a bug with some dongle that defers a lot too. One idea
> was to use a progressively longer delay for DP_AUX_NATIVE_REPLY_DEFER.
>
> How about drm_dp_i2c_do_msg(), that one can receive native aux defer
> too, along with i2c defer. It feels like both places should have the
> same limit for native aux defer, but if you want to keep different i2c
> defer retry limit it gets a bit trickier.

I think we want to keep the spec limit there, since its in the spec,

But you might be right on wanting to extend the defer therre as well if
we can, I'll probably tackle that in a second patch.

Dave.
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 959e207..db0993f 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -189,7 +189,7 @@  static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
 	 * retry native transactions, so retry 7 times like for I2C-over-AUX
 	 * transactions.
 	 */
-	for (retry = 0; retry < 7; retry++) {
+	for (retry = 0; retry < 32; retry++) {
 
 		mutex_lock(&aux->hw_mutex);
 		err = aux->transfer(aux, &msg);