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

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

[Dave Airlie]

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
 	 * 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

[PATCH 2/2] drm/fb_helper: move deferred fb checking into restore mode

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

On MST systems the monitors don't appear when we set the fb up,
but plymouth opens the drm device and holds it open while they
come up, when plymouth finishes and lastclose gets called we
don't do the delayed fb probe, so the monitor never appears on the
console.

Fix this by moving the delayed checking into the mode restore.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index c7f2778..8ba36cf 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -350,6 +350,11 @@ bool drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
 	drm_modeset_lock_all(dev);
 	ret = restore_fbdev_mode(fb_helper);
 	drm_modeset_unlock_all(dev);
+
+	if (fb_helper->delayed_hotplug) {
+		fb_helper->delayed_hotplug = false;
+		drm_fb_helper_hotplug_event(fb_helper);
+	}
 	return ret;
 }
 EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
@@ -888,10 +893,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
 
 	drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
 
-	if (fb_helper->delayed_hotplug) {
-		fb_helper->delayed_hotplug = false;
-		drm_fb_helper_hotplug_event(fb_helper);
-	}
 	return 0;
 }
 EXPORT_SYMBOL(drm_fb_helper_set_par);
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Tom Stellard]

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Dave Airlie]

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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 2/2] drm/fb_helper: move deferred fb checking into restore mode

[Daniel Vetter]

On Wed, Nov 26, 2014 at 01:17:23PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> On MST systems the monitors don't appear when we set the fb up,
> but plymouth opens the drm device and holds it open while they
> come up, when plymouth finishes and lastclose gets called we
> don't do the delayed fb probe, so the monitor never appears on the
> console.
> 
> Fix this by moving the delayed checking into the mode restore.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index c7f2778..8ba36cf 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -350,6 +350,11 @@ bool drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
>  	drm_modeset_lock_all(dev);
>  	ret = restore_fbdev_mode(fb_helper);
>  	drm_modeset_unlock_all(dev);
> +
> +	if (fb_helper->delayed_hotplug) {
> +		fb_helper->delayed_hotplug = false;
> +		drm_fb_helper_hotplug_event(fb_helper);

Preexisting bug but ->delayed_hotplug is protected in the hotplug handler
by mode_config.mutex. But fixing that will get annoying so not sure if
it's worth it. Just wrapping the access with the lock shouldn't hurt
though I think.
-Daniel

> +	}
>  	return ret;
>  }
>  EXPORT_SYMBOL(drm_fb_helper_restore_fbdev_mode_unlocked);
> @@ -888,10 +893,6 @@ int drm_fb_helper_set_par(struct fb_info *info)
>  
>  	drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper);
>  
> -	if (fb_helper->delayed_hotplug) {
> -		fb_helper->delayed_hotplug = false;
> -		drm_fb_helper_hotplug_event(fb_helper);
> -	}
>  	return 0;
>  }
>  EXPORT_SYMBOL(drm_fb_helper_set_par);
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[Jani Nikula]

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

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/fb_helper: move deferred fb checking

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  366/366              366/366
ILK                 -2              371/371              369/371
SNB                                  450/450              450/450
IVB                                  498/498              498/498
BYT                                  289/289              289/289
HSW                 -1              564/564              563/564
BDW                                  417/417              417/417
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 ILK  igt_gem_reset_stats_close-pending-fork-render      TIMEOUT(22, M37M26)PASS(1, M26)      TIMEOUT(1, M37)
 ILK  igt_kms_flip_vblank-vs-hang      TIMEOUT(21, M37M26)PASS(1, M26)      TIMEOUT(1, M37)
*HSW  igt_kms_fence_pin_leak      PASS(6, M20M19)      DMESG_WARN(1, M19)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Dave Airlie]

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.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel