[PATCH 1/2] drm/amdgpu/display: adjust msleep limit in dp_wait_for_training_aux_rd_interval

[PATCH 1/2] drm/amdgpu/display: adjust msleep limit in dp_wait_for_training_aux_rd_interval

[Alex Deucher]

Some architectures (e.g., ARM) have relatively low udelay limits.
On most architectures, anything longer than 2000us is not recommended.
Change the check to align with other similar checks in DC.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 1f8831156bc4..aa1c67c3c386 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -202,7 +202,7 @@ void dp_wait_for_training_aux_rd_interval(
 	uint32_t wait_in_micro_secs)
 {
 #if defined(CONFIG_DRM_AMD_DC_DCN)
-	if (wait_in_micro_secs > 16000)
+	if (wait_in_micro_secs > 1000)
 		msleep(wait_in_micro_secs/1000);
 	else
 		udelay(wait_in_micro_secs);
-- 
2.34.1

[PATCH 2/2] drm/amdgpu/display: use msleep rather than udelay for long delays

[Alex Deucher]

Some architectures (e.g., ARM) throw an compilation error if the
udelay is too long.  In general udelays of longer than 2000us are
not recommended on any architecture.  Switch to msleep in these
cases.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index aa1c67c3c386..eb4432dca761 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -6941,7 +6941,7 @@ bool dpcd_write_128b_132b_sst_payload_allocation_table(
 			}
 		}
 		retries++;
-		udelay(5000);
+		msleep(5);
 	}
 
 	if (!result && retries == max_retries) {
@@ -6993,7 +6993,7 @@ bool dpcd_poll_for_allocation_change_trigger(struct dc_link *link)
 			break;
 		}
 
-		udelay(5000);
+		msleep(5);
 	}
 
 	if (result == ACT_FAILED) {
-- 
2.34.1

Re: [PATCH 2/2] drm/amdgpu/display: use msleep rather than udelay for long delays

[Harry Wentland]

On 2022-01-20 13:04, Alex Deucher wrote:
> Some architectures (e.g., ARM) throw an compilation error if the
> udelay is too long.  In general udelays of longer than 2000us are
> not recommended on any architecture.  Switch to msleep in these
> cases.
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

Series is
Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> index aa1c67c3c386..eb4432dca761 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> @@ -6941,7 +6941,7 @@ bool dpcd_write_128b_132b_sst_payload_allocation_table(
>  			}
>  		}
>  		retries++;
> -		udelay(5000);
> +		msleep(5);
>  	}
>  
>  	if (!result && retries == max_retries) {
> @@ -6993,7 +6993,7 @@ bool dpcd_poll_for_allocation_change_trigger(struct dc_link *link)
>  			break;
>  		}
>  
> -		udelay(5000);
> +		msleep(5);
>  	}
>  
>  	if (result == ACT_FAILED) {

RE: [PATCH 1/2] drm/amdgpu/display: adjust msleep limit in dp_wait_for_training_aux_rd_interval

[Chen, Guchun]

[Public]

If we change if condition, how about the division by "wait_in_micro_secs/1000", as the sleep time is less now. Shall we adjust it as well?

Regards,
Guchun

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
Sent: Friday, January 21, 2022 2:04 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: [PATCH 1/2] drm/amdgpu/display: adjust msleep limit in dp_wait_for_training_aux_rd_interval

Some architectures (e.g., ARM) have relatively low udelay limits.
On most architectures, anything longer than 2000us is not recommended.
Change the check to align with other similar checks in DC.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 1f8831156bc4..aa1c67c3c386 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -202,7 +202,7 @@ void dp_wait_for_training_aux_rd_interval(
 	uint32_t wait_in_micro_secs)
 {
 #if defined(CONFIG_DRM_AMD_DC_DCN)
-	if (wait_in_micro_secs > 16000)
+	if (wait_in_micro_secs > 1000)
 		msleep(wait_in_micro_secs/1000);
 	else
 		udelay(wait_in_micro_secs);
-- 
2.34.1

Re: [PATCH 1/2] drm/amdgpu/display: adjust msleep limit in dp_wait_for_training_aux_rd_interval

[Deucher, Alexander]

[-- Attachment #1: Type: text/plain, Size: 1990 bytes --]

[Public]

It just changes the limit for when we use msleep vs udelay, not the units.

Alex
________________________________
From: Chen, Guchun <Guchun.Chen@amd.com>
Sent: Thursday, January 20, 2022 8:49 PM
To: Deucher, Alexander <Alexander.Deucher@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: RE: [PATCH 1/2] drm/amdgpu/display: adjust msleep limit in dp_wait_for_training_aux_rd_interval

[Public]

If we change if condition, how about the division by "wait_in_micro_secs/1000", as the sleep time is less now. Shall we adjust it as well?

Regards,
Guchun

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex Deucher
Sent: Friday, January 21, 2022 2:04 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>
Subject: [PATCH 1/2] drm/amdgpu/display: adjust msleep limit in dp_wait_for_training_aux_rd_interval

Some architectures (e.g., ARM) have relatively low udelay limits.
On most architectures, anything longer than 2000us is not recommended.
Change the check to align with other similar checks in DC.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 1f8831156bc4..aa1c67c3c386 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -202,7 +202,7 @@ void dp_wait_for_training_aux_rd_interval(
         uint32_t wait_in_micro_secs)
 {
 #if defined(CONFIG_DRM_AMD_DC_DCN)
-       if (wait_in_micro_secs > 16000)
+       if (wait_in_micro_secs > 1000)
                 msleep(wait_in_micro_secs/1000);
         else
                 udelay(wait_in_micro_secs);
--
2.34.1

[-- Attachment #2: Type: text/html, Size: 3575 bytes --]

RE: [PATCH 1/2] drm/amdgpu/display: adjust msleep limit in dp_wait_for_training_aux_rd_interval

[Chen, Guchun]

[-- Attachment #1: Type: text/plain, Size: 2710 bytes --]

[Public]

Series is: Reviewed-by: Guchun Chen guchun.chen@amd.com<mailto:guchun.chen@amd.com>

Regards,
Guchun

From: Deucher, Alexander <Alexander.Deucher@amd.com>
Sent: Saturday, January 22, 2022 1:00 AM
To: Chen, Guchun <Guchun.Chen@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/amdgpu/display: adjust msleep limit in dp_wait_for_training_aux_rd_interval


[Public]

It just changes the limit for when we use msleep vs udelay, not the units.

Alex
________________________________
From: Chen, Guchun <Guchun.Chen@amd.com<mailto:Guchun.Chen@amd.com>>
Sent: Thursday, January 20, 2022 8:49 PM
To: Deucher, Alexander <Alexander.Deucher@amd.com<mailto:Alexander.Deucher@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com<mailto:Alexander.Deucher@amd.com>>
Subject: RE: [PATCH 1/2] drm/amdgpu/display: adjust msleep limit in dp_wait_for_training_aux_rd_interval

[Public]

If we change if condition, how about the division by "wait_in_micro_secs/1000", as the sleep time is less now. Shall we adjust it as well?

Regards,
Guchun

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> On Behalf Of Alex Deucher
Sent: Friday, January 21, 2022 2:04 AM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com<mailto:Alexander.Deucher@amd.com>>
Subject: [PATCH 1/2] drm/amdgpu/display: adjust msleep limit in dp_wait_for_training_aux_rd_interval

Some architectures (e.g., ARM) have relatively low udelay limits.
On most architectures, anything longer than 2000us is not recommended.
Change the check to align with other similar checks in DC.

Signed-off-by: Alex Deucher <alexander.deucher@amd.com<mailto:alexander.deucher@amd.com>>
---
 drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
index 1f8831156bc4..aa1c67c3c386 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
@@ -202,7 +202,7 @@ void dp_wait_for_training_aux_rd_interval(
         uint32_t wait_in_micro_secs)
 {
 #if defined(CONFIG_DRM_AMD_DC_DCN)
-       if (wait_in_micro_secs > 16000)
+       if (wait_in_micro_secs > 1000)
                 msleep(wait_in_micro_secs/1000);
         else
                 udelay(wait_in_micro_secs);
--
2.34.1

[-- Attachment #2: Type: text/html, Size: 7034 bytes --]

Re: [PATCH 2/2] drm/amdgpu/display: use msleep rather than udelay for long delays

[Lazar, Lijo]

On 1/20/2022 11:34 PM, Alex Deucher wrote:
> Some architectures (e.g., ARM) throw an compilation error if the
> udelay is too long.  In general udelays of longer than 2000us are
> not recommended on any architecture.  Switch to msleep in these
> cases.
> 
> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> index aa1c67c3c386..eb4432dca761 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> @@ -6941,7 +6941,7 @@ bool dpcd_write_128b_132b_sst_payload_allocation_table(
>   			}
>   		}
>   		retries++;
> -		udelay(5000);
> +		msleep(5);

msleep is not recommended for sleeps < 20ms. The original code (modified 
in patch 1) seems to do such a check. As per kernel doc, if precision 
matters recommendation is to use usleep_range().

Thanks,
Lijo

>   	}
>   
>   	if (!result && retries == max_retries) {
> @@ -6993,7 +6993,7 @@ bool dpcd_poll_for_allocation_change_trigger(struct dc_link *link)
>   			break;
>   		}
>   
> -		udelay(5000);
> +		msleep(5);
>   	}
>   
>   	if (result == ACT_FAILED) {
> 

Re: [PATCH 2/2] drm/amdgpu/display: use msleep rather than udelay for long delays

[Harry Wentland]

On 2022-01-25 06:32, Lazar, Lijo wrote:
> 
> 
> On 1/20/2022 11:34 PM, Alex Deucher wrote:
>> Some architectures (e.g., ARM) throw an compilation error if the
>> udelay is too long.  In general udelays of longer than 2000us are
>> not recommended on any architecture.  Switch to msleep in these
>> cases.
>>
>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>   drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
>> index aa1c67c3c386..eb4432dca761 100644
>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
>> @@ -6941,7 +6941,7 @@ bool dpcd_write_128b_132b_sst_payload_allocation_table(
>>               }
>>           }
>>           retries++;
>> -        udelay(5000);
>> +        msleep(5);
> 
> msleep is not recommended for sleeps < 20ms. The original code (modified in patch 1) seems to do such a check. As per kernel doc, if precision matters recommendation is to use usleep_range().
> 

If I remember right the sleep (non-polling) function can take (significantly) longer than the requested time, but never shorter. This should be fine in this case.

Harry

> Thanks,
> Lijo
> 
>>       }
>>         if (!result && retries == max_retries) {
>> @@ -6993,7 +6993,7 @@ bool dpcd_poll_for_allocation_change_trigger(struct dc_link *link)
>>               break;
>>           }
>>   -        udelay(5000);
>> +        msleep(5);
>>       }
>>         if (result == ACT_FAILED) {
>>

Re: [PATCH 2/2] drm/amdgpu/display: use msleep rather than udelay for long delays

[Alex Deucher]

On Tue, Jan 25, 2022 at 6:32 AM Lazar, Lijo <lijo.lazar@amd.com> wrote:
>
>
>
> On 1/20/2022 11:34 PM, Alex Deucher wrote:
> > Some architectures (e.g., ARM) throw an compilation error if the
> > udelay is too long.  In general udelays of longer than 2000us are
> > not recommended on any architecture.  Switch to msleep in these
> > cases.
> >
> > Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >   drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> > index aa1c67c3c386..eb4432dca761 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c
> > @@ -6941,7 +6941,7 @@ bool dpcd_write_128b_132b_sst_payload_allocation_table(
> >                       }
> >               }
> >               retries++;
> > -             udelay(5000);
> > +             msleep(5);
>
> msleep is not recommended for sleeps < 20ms. The original code (modified
> in patch 1) seems to do such a check. As per kernel doc, if precision
> matters recommendation is to use usleep_range().

I'm not sure what to use here.  ARM32 doesn't allow udelay longer than 2000us.

Alex

>
> Thanks,
> Lijo
>
> >       }
> >
> >       if (!result && retries == max_retries) {
> > @@ -6993,7 +6993,7 @@ bool dpcd_poll_for_allocation_change_trigger(struct dc_link *link)
> >                       break;
> >               }
> >
> > -             udelay(5000);
> > +             msleep(5);
> >       }
> >
> >       if (result == ACT_FAILED) {
> >