On 2021-10-26 11:51 a.m., Michel Dänzer wrote:
On 2021-10-26 13:07, Stylon Wang wrote:
[Why]
In GNOME Settings->Display the switching from mirror mode to single display
occasionally causes wait_for_completion_interruptible_timeout() to return
-ERESTARTSYS and fails atomic check.
[How]
Replace the call with wait_for_completion_timeout() since the waiting for
hw_done and flip_done completion doesn't need to worry about interruption
from signal.
Signed-off-by: Stylon Wang <stylon.wang@xxxxxxx>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 4cd64529b180..b8f4ff323de1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -9844,10 +9844,10 @@ static int do_aquire_global_lock(struct drm_device *dev,
* Make sure all pending HW programming completed and
* page flips done
*/
- ret = wait_for_completion_interruptible_timeout(&commit->hw_done, 10*HZ);
+ ret = wait_for_completion_timeout(&commit->hw_done, 10*HZ);
if (ret > 0)
- ret = wait_for_completion_interruptible_timeout(
+ ret = wait_for_completion_timeout(
&commit->flip_done, 10*HZ);
if (ret == 0)
The *_interruptible_* variant is needed so that the display manager process can be killed while it's waiting here, which could take up to 10 seconds (per the timeout).
What's the problem with -ERESTARTSYS? Either the ioctl should be restarted automatically, or if it bounces back to user space, that needs to be able to retry the ioctl while it returns -1 and errno == EINTR. drmIoctl handles this transparently.
Thanks for the insight Michel!
If it's just an error in the log without a functional issue then maybe
we should downgrade it to a debug statement in the case where it returns
-ERESTARTSYS.
If this is a functional issue (DRM not automatically retrying the
commit?) then maybe we should take a deeper look into the IOCTL itself.
Regards,
Nicholas Kazlauskas
