From: "Kazlauskas, Nicholas" <nicholas.kazlauskas@amd.com>
To: Stylon Wang <stylon.wang@amd.com>, amd-gfx@lists.freedesktop.org
Cc: Harry.Wentland@amd.com, Rodrigo.Siqueira@amd.com,
mdaenzer@redhat.com, contact@emersion.fr
Subject: Re: [PATCH] drm/amd/display: Fix error handling on waiting for completion
Date: Tue, 26 Oct 2021 09:29:26 -0400 [thread overview]
Message-ID: <6f604281-c8ad-5e9e-4a7a-78d7153be9ff@amd.com> (raw)
In-Reply-To: <20211026110740.152936-1-stylon.wang@amd.com>
On 2021-10-26 7:07 a.m., 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@amd.com>
I think this is okay, but I'll write out how I think these work here in
case anyone has corrections.
Both variants allow the thread to sleep, but the interruptible variant
can waken due to signals. These signals are a secondary wakeup event and
would require use to restart the wait and (probably) keep track of how
long we were waiting before.
We want wakeup only on completion, so we should be using the
`wait_for_completion_timeout()` variants instead in most (if not all?)
cases in our display driver.
This probably has some nuances that matter more for different variants
of UAPI, but with this understanding I think this is:
Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
Now, if we could revive that patch series I had from the other year and
outright drop `do_aquire_global_lock()`...
Regards,
Nicholas Kazlauskas
> ---
> 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)
>
next prev parent reply other threads:[~2021-10-26 13:29 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-26 11:07 [PATCH] drm/amd/display: Fix error handling on waiting for completion Stylon Wang
2021-10-26 13:29 ` Kazlauskas, Nicholas [this message]
2021-10-26 15:51 ` Michel Dänzer
2021-10-26 16:03 ` Kazlauskas, Nicholas
2021-10-26 16:08 ` Simon Ser
2021-10-26 16:11 ` Michel Dänzer
2021-10-27 7:20 ` Christian König
2021-11-01 15:00 ` Wang, Chao-kai (Stylon)
2021-11-03 14:54 ` Michel Dänzer
2021-11-03 14:54 ` Michel Dänzer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6f604281-c8ad-5e9e-4a7a-78d7153be9ff@amd.com \
--to=nicholas.kazlauskas@amd.com \
--cc=Harry.Wentland@amd.com \
--cc=Rodrigo.Siqueira@amd.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=contact@emersion.fr \
--cc=mdaenzer@redhat.com \
--cc=stylon.wang@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.