2019-06-18 01:13:14

by Nathan Chancellor

Subject: arm32 build failure after abe882a39a9c ("drm/amd/display: fix issue with eDP not detected on driver load")

Hi all,

After commit abe882a39a9c ("drm/amd/display: fix issue with eDP not
detected on driver load") in -next, arm32 allyesconfig builds start
failing at link time:

arm-linux-gnueabi-ld: drivers/gpu/drm/amd/display/dc/core/dc_link.o: in
function `dc_link_detect':
dc_link.c:(.text+0x260c): undefined reference to `__bad_udelay'

arm32 only allows a udelay value of up to 2000, see
arch/arm/include/asm/delay.h for more info.

Please look into this when you have a chance!
Nathan

2019-06-25 04:16:33

by Dave Airlie

Subject: Re: arm32 build failure after abe882a39a9c ("drm/amd/display: fix issue with eDP not detected on driver load")

Hi Alex,

please resolve this ASAP, I cannot pull your tree without this fixed
as it breaks my arm builds here.

an 8 second delay there seems pointless and arbitary, an 8 sec delay
there without a comment, seems like a lack of review.

Dave.

On Tue, 18 Jun 2019 at 11:12, Nathan Chancellor
<[email protected]> wrote:
>
> Hi all,
>
> After commit abe882a39a9c ("drm/amd/display: fix issue with eDP not
> detected on driver load") in -next, arm32 allyesconfig builds start
> failing at link time:
>
> arm-linux-gnueabi-ld: drivers/gpu/drm/amd/display/dc/core/dc_link.o: in
> function `dc_link_detect':
> dc_link.c:(.text+0x260c): undefined reference to `__bad_udelay'
>
> arm32 only allows a udelay value of up to 2000, see
> arch/arm/include/asm/delay.h for more info.
>
> Please look into this when you have a chance!
> Nathan

2019-06-25 14:04:04

by Harry Wentland

Subject: [PATCH] drm/amd/display: Use msleep instead of udelay for 8ms wait

arm32's udelay only allows values up to 2000 microseconds. msleep
does the trick for us here as there is no problem if this isn't
microsecond accurate and takes a tad longer.

Signed-off-by: Harry Wentland <[email protected]>
---
drivers/gpu/drm/amd/display/dc/core/dc_link.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 4c31930f1cdf..f5d02f89b3f9 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -548,7 +548,7 @@ static void read_edp_current_link_settings_on_detect(struct dc_link *link)
break;
}

- udelay(8000);
+ msleep(8);
}

ASSERT(status == DC_OK);
--
2.22.0

2019-06-25 14:14:05

by Lucas Stach

Subject: Re: [PATCH] drm/amd/display: Use msleep instead of udelay for 8ms wait

Hi Harry,

Am Dienstag, den 25.06.2019, 10:00 -0400 schrieb Harry Wentland:
> arm32's udelay only allows values up to 2000 microseconds. msleep
> does the trick for us here as there is no problem if this isn't
> microsecond accurate and takes a tad longer.

A "tad" longer in this case means likely double the intended wait.
Please see "SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms)" in
Documentation/timers/timers-howto.txt.

The sleep here should use usleep_range. In general the DC code seems to
use quite a lot of the udelay busy waits. I doubt that many of those
occurrences are in atomic context, so could easily use a sleeping wait.

Digging further this seems to apply across amdgpu, not only DC.

Regards,
Lucas

> Signed-off-by: Harry Wentland <[email protected]>
> ---
>  drivers/gpu/drm/amd/display/dc/core/dc_link.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> index 4c31930f1cdf..f5d02f89b3f9 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> @@ -548,7 +548,7 @@ static void
> read_edp_current_link_settings_on_detect(struct dc_link *link)
>   break;
>   }
>  
> - udelay(8000);
> + msleep(8);
>   }
>  
>   ASSERT(status == DC_OK);

2019-06-25 14:28:19

by Christian König

Subject: Re: [PATCH] drm/amd/display: Use msleep instead of udelay for 8ms wait

Am 25.06.19 um 16:12 schrieb Lucas Stach:
> Hi Harry,
>
> Am Dienstag, den 25.06.2019, 10:00 -0400 schrieb Harry Wentland:
>> arm32's udelay only allows values up to 2000 microseconds. msleep
>> does the trick for us here as there is no problem if this isn't
>> microsecond accurate and takes a tad longer.
> A "tad" longer in this case means likely double the intended wait.
> Please see "SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms)" in
> Documentation/timers/timers-howto.txt.

Oh, thanks so much for the link! I was searching desperately for this
the last time this came up and couldn't find it.

Clearly going to remember now where to find that.

Thanks,
Christian.

>
> The sleep here should use usleep_range. In general the DC code seems to
> use quite a lot of the udelay busy waits. I doubt that many of those
> occurrences are in atomic context, so could easily use a sleeping wait.
>
> Digging further this seems to apply across amdgpu, not only DC.
>
> Regards,
> Lucas
>
>> Signed-off-by: Harry Wentland <[email protected]>
>> ---
>>  drivers/gpu/drm/amd/display/dc/core/dc_link.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
>> b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
>> index 4c31930f1cdf..f5d02f89b3f9 100644
>> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
>> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
>> @@ -548,7 +548,7 @@ static void
>> read_edp_current_link_settings_on_detect(struct dc_link *link)
>>   break;
>>   }
>>
>> - udelay(8000);
>> + msleep(8);
>>   }
>>
>>   ASSERT(status == DC_OK);