| Message ID | 20250326070054.68355-1-bold.zone2373@fastmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/amd/display: replace use of msleep(<20) with usleep_range for better accuracy | expand |
On 3/26/25 01:00, James Flowers wrote: > msleep < 20ms will often sleep for ~20ms (according to Documentation/timers/timers-howto.rst). Can you elaborate and explain why this change is necessary? > > Signed-off-by: James Flowers <bold.zone2373@fastmail.com> > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > index 2cd35392e2da..2d225735602b 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > @@ -682,7 +682,7 @@ static bool execute_synaptics_rc_command(struct drm_dp_aux *aux, > if (rc_cmd == cmd) > // active is 0 > break; > - msleep(10); > + usleep_range(10000, 10200); > } > > // read rc result thanks, -- Shuah
On Fri, Mar 28, 2025, at 3:46 PM, Shuah Khan wrote: > On 3/26/25 01:00, James Flowers wrote: >> msleep < 20ms will often sleep for ~20ms (according to Documentation/timers/timers-howto.rst). > > Can you elaborate and explain why this change is necessary? scripts/checkpatch.pl highlighted it as a possible issue since msleep can cause the delay to be unexpectedly closer to 20ms. However, perhaps it would be better to submit a V2 with the change as usleep_range(10000, 20000) to match the actual delay range that msleep(10) has, which would help avoid a regression like this: https://gitlab.freedesktop.org/drm/amd/-/issues/3559#note_2532546. Is there any appetite for drm/amd to change instances of msleep(x) (x being < 20) to usleep_range(x*1000, 20000), so that the range of delay is clear to readers unfamiliar with the msleep quirk? Please let me know and I can submit a V2. There is at least one instance (fixed by commit 57995aa8ffb3e47a74763cf9106d34e9e8be9d8d) in drm/amdgpu where msleep(1) seemed to be causing an unexpectedly long delay, and so was replaced with usleep_range. An issue possibly related to that fix is https://gitlab.freedesktop.org/drm/amd/-/issues/462. Currently there are 37 msleep(1) calls that I can find in drm/amd. I'm just thinking that optimization opportunities like above could be found more easily if the delay range is made clear? Best regards, James Flowers
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index 2cd35392e2da..2d225735602b 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -682,7 +682,7 @@ static bool execute_synaptics_rc_command(struct drm_dp_aux *aux, if (rc_cmd == cmd) // active is 0 break; - msleep(10); + usleep_range(10000, 10200); } // read rc result
msleep < 20ms will often sleep for ~20ms (according to Documentation/timers/timers-howto.rst). Signed-off-by: James Flowers <bold.zone2373@fastmail.com> --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)