Re: [PATCH 6/9] drm/i915: Disable waitboosting for fence_wait()

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> We want to restrict waitboosting to known process contexts, where we can
> track which clients are receiving waitboosts and prevent excessive power
> wasting. For fence_wait() we do not have any client tracking and so that
> leaves it open to abuse.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_request.c | 7 ++++---
>  drivers/gpu/drm/i915/i915_gem_request.h | 1 +
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index 6528536878f2..bfec4a88707d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -70,7 +70,7 @@ static signed long i915_fence_wait(struct fence *fence,
>  
>  	ret = __i915_wait_request(to_request(fence),
>  				  interruptible, timeout,
> -				  NULL);
> +				  NO_WAITBOOST);
>  	if (ret == -ETIME)
>  		return 0;
>  
> @@ -642,7 +642,7 @@ int __i915_wait_request(struct drm_i915_gem_request *req,
>  	 * forcing the clocks too high for the whole system, we only allow
>  	 * each client to waitboost once in a busy period.
>  	 */
> -	if (INTEL_GEN(req->i915) >= 6)
> +	if (!IS_ERR(rps) && INTEL_GEN(req->i915) >= 6)
>  		gen6_rps_boost(req->i915, rps, req->emitted_jiffies);
>

The intention here is to boost even if rps client is NULL?
Assuming so could you elaborate why.

Thanks,
-Mika


>  	/* Optimistic spin for the next ~jiffie before touching IRQs */
> @@ -713,7 +713,8 @@ complete:
>  			*timeout = 0;
>  	}
>  
> -	if (rps && req->fence.seqno == req->engine->last_submitted_seqno) {
> +	if (!IS_ERR_OR_NULL(rps) &&
> +	    req->fence.seqno == req->engine->last_submitted_seqno) {
>  		/* The GPU is now idle and this client has stalled.
>  		 * Since no other client has submitted a request in the
>  		 * meantime, assume that this client is the only one
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h
> index 6f2c820785f3..f7f497a07893 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.h
> +++ b/drivers/gpu/drm/i915/i915_gem_request.h
> @@ -206,6 +206,7 @@ void __i915_add_request(struct drm_i915_gem_request *req,
>  	__i915_add_request(req, NULL, false)
>  
>  struct intel_rps_client;
> +#define NO_WAITBOOST ERR_PTR(-1)
>  
>  int __i915_wait_request(struct drm_i915_gem_request *req,
>  			bool interruptible,
> -- 
> 2.8.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx