[PATCH 2/5] drm/nouveau: base fence timeout on time of emission

[PATCH 2/5] drm/nouveau: base fence timeout on time of emission

[Marcin Slusarz]

Wait loop can be interrupted by signal, so if signals are raised
periodically (e.g. SIGALRM) this loop may never finish. Use
emission time as a base for fence timeout.

Signed-off-by: Marcin Slusarz <marcin.slusarz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/gpu/drm/nouveau/nouveau_fence.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c
index a22b9ad..59f92e9 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -44,6 +44,7 @@ struct nouveau_fence {
 
 	uint32_t sequence;
 	bool signalled;
+	unsigned long emitted_at;
 
 	void (*work)(void *priv, bool signalled);
 	void *priv;
@@ -172,6 +173,7 @@ nouveau_fence_emit(struct nouveau_fence *fence)
 	}
 	OUT_RING (chan, fence->sequence);
 	FIRE_RING(chan);
+	fence->emitted_at = jiffies;
 
 	return 0;
 }
@@ -230,7 +232,8 @@ __nouveau_fence_signalled(void *sync_obj, void *sync_arg)
 int
 __nouveau_fence_wait(void *sync_obj, void *sync_arg, bool lazy, bool intr)
 {
-	unsigned long timeout = jiffies + (3 * DRM_HZ);
+	struct nouveau_fence *fence = nouveau_fence(sync_obj);
+	unsigned long timeout = fence->emitted_at + 3 * DRM_HZ;
 	unsigned long sleep_time = NSEC_PER_MSEC / 1000;
 	ktime_t t;
 	int ret = 0;
-- 
1.7.8.5

Re: [PATCH 2/5] drm/nouveau: base fence timeout on time of emission

[Ben Skeggs]

On Mon, 2012-04-23 at 00:18 +0200, Marcin Slusarz wrote:
> Wait loop can be interrupted by signal, so if signals are raised
> periodically (e.g. SIGALRM) this loop may never finish. Use
> emission time as a base for fence timeout.
Ah, thanks for tackling this issue.  It's been long on my list of things
to do, but never quite made it to the top.

Rather than hardcoding the timeout in fence_wait(), I think perhaps
storing "fence->timeout = jiffies + (whatever * HZ);" is a better plan.
I can forsee us wanting longer timeouts for certain operations
(particularly long compute operations) in the future.

Ben.

> 
> Signed-off-by: Marcin Slusarz <marcin.slusarz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/gpu/drm/nouveau/nouveau_fence.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c
> index a22b9ad..59f92e9 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_fence.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
> @@ -44,6 +44,7 @@ struct nouveau_fence {
>  
>  	uint32_t sequence;
>  	bool signalled;
> +	unsigned long emitted_at;
>  
>  	void (*work)(void *priv, bool signalled);
>  	void *priv;
> @@ -172,6 +173,7 @@ nouveau_fence_emit(struct nouveau_fence *fence)
>  	}
>  	OUT_RING (chan, fence->sequence);
>  	FIRE_RING(chan);
> +	fence->emitted_at = jiffies;
>  
>  	return 0;
>  }
> @@ -230,7 +232,8 @@ __nouveau_fence_signalled(void *sync_obj, void *sync_arg)
>  int
>  __nouveau_fence_wait(void *sync_obj, void *sync_arg, bool lazy, bool intr)
>  {
> -	unsigned long timeout = jiffies + (3 * DRM_HZ);
> +	struct nouveau_fence *fence = nouveau_fence(sync_obj);
> +	unsigned long timeout = fence->emitted_at + 3 * DRM_HZ;
>  	unsigned long sleep_time = NSEC_PER_MSEC / 1000;
>  	ktime_t t;
>  	int ret = 0;

Re: [PATCH 2/5] drm/nouveau: base fence timeout on time of emission

[Marcin Slusarz]

On Tue, Apr 24, 2012 at 12:37:34PM +1000, Ben Skeggs wrote:
> On Mon, 2012-04-23 at 00:18 +0200, Marcin Slusarz wrote:
> > Wait loop can be interrupted by signal, so if signals are raised
> > periodically (e.g. SIGALRM) this loop may never finish. Use
> > emission time as a base for fence timeout.
> Ah, thanks for tackling this issue.  It's been long on my list of things
> to do, but never quite made it to the top.
> 
> Rather than hardcoding the timeout in fence_wait(), I think perhaps
> storing "fence->timeout = jiffies + (whatever * HZ);" is a better plan.
> I can forsee us wanting longer timeouts for certain operations
> (particularly long compute operations) in the future.

Yeah, good idea. Later, we will probably need to add code which will keep extending
this timeout as long as gpu is progressing.

Marcin