[v5,16/35] drm/i915: Hook scheduler node clean up into retire requests

Commit Message

John Harrison Feb. 18, 2016, 2:27 p.m. UTC

From: John Harrison <[e-mail]>

The scheduler keeps its own lock on various DRM objects in order to
guarantee safe access long after the original execbuff IOCTL has
completed. This is especially important when pre-emption is enabled as
the batch buffer might need to be submitted to the hardware multiple
times. This patch hooks the clean up of these locks into the request
retire function. The request can only be retired after it has
completed on the hardware and thus is no longer eligible for
re-submission. Thus there is no point holding on to the locks beyond
that time.

v3: Updated to not WARN when cleaning a node that is being cancelled.
The clean will happen later so skipping it at the point of
cancellation is fine.

v5: Squashed the i915_scheduler.c portions down into the 'start of
scheduler' patch. [Joonas Lahtinen]

For: VIZ-1587
Signed-off-by: John Harrison <[e-mail]>
Cc: Joonas Lahtinen <[e-mail]>
---
 drivers/gpu/drm/i915/i915_gem.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Jesse Barnes Feb. 19, 2016, 7:44 p.m. UTC | #1

On 02/18/2016 06:27 AM, [e-mail] wrote:
> From: John Harrison <[e-mail]>
> 
> The scheduler keeps its own lock on various DRM objects in order to
> guarantee safe access long after the original execbuff IOCTL has

execbuf is getting bigger, but I'm not sure if it qualifies as "buff" yet.  Intentional misspelling? :)

> completed. This is especially important when pre-emption is enabled as
> the batch buffer might need to be submitted to the hardware multiple
> times. This patch hooks the clean up of these locks into the request
> retire function. The request can only be retired after it has
> completed on the hardware and thus is no longer eligible for
> re-submission. Thus there is no point holding on to the locks beyond
> that time.
> 
> v3: Updated to not WARN when cleaning a node that is being cancelled.
> The clean will happen later so skipping it at the point of
> cancellation is fine.
> 
> v5: Squashed the i915_scheduler.c portions down into the 'start of
> scheduler' patch. [Joonas Lahtinen]
> 
> For: VIZ-1587
> Signed-off-by: John Harrison <[e-mail]>
> Cc: Joonas Lahtinen <[e-mail]>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1ab7256..2dd9b55 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1489,6 +1489,9 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
>  		fence_signal_locked(&request->fence);
>  	}
>  
> +	if (request->scheduler_qe)
> +		i915_scheduler_clean_node(request->scheduler_qe);
> +
>  	i915_gem_request_unreference(request);
>  }
>  
> 

Reviewed-by: Jesse Barnes <[e-mail]>

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1ab7256..2dd9b55 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1489,6 +1489,9 @@  static void i915_gem_request_retire(struct drm_i915_gem_request *request)
 		fence_signal_locked(&request->fence);
 	}
 
+	if (request->scheduler_qe)
+		i915_scheduler_clean_node(request->scheduler_qe);
+
 	i915_gem_request_unreference(request);
 }