Re: [Intel-gfx] [PATCH] drm/i915: Add background commentary to "waitboosting"

[Daniel Vetter]

On Thu, Dec 10, 2015 at 10:37:36AM +, Chris Wilson wrote:
> Describe the intent of boosting the GPU frequency to maximum before
> waiting on the GPU.
> 
> RPS waitboosting was introduced with
> 
> commit b29c19b645287f7062e17d70fa4e9781a01a5d88
> Author: Chris Wilson 
> Date:   Wed Sep 25 17:34:56 2013 +0100
> 
> drm/i915: Boost RPS frequency for CPU stalls
> 
> but lacked a concise comment in the code to explain itself.
> 
> Signed-off-by: Chris Wilson 

We might want to consolidate this with the kerneldoc for gen6_rps_boost,
if that ever happens. But for now this is a good place.

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ece7e2908fa3..40a7ade4eafe 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1248,6 +1248,22 @@ int __i915_wait_request(struct drm_i915_gem_request 
> *req,
>   }
>  
>   trace_i915_gem_request_wait_begin(req);
> +
> + /* This client is about to stall waiting for the GPU. In many cases
> +  * this is undesirable and limits the throughput of the system, as
> +  * many clients cannot continue processing user input/output whilst
> +  * asleep. RPS autotuning may take tens of milliseconds to respond
> +  * to the GPU load and thus incurs additional latency for the client.
> +  * We can circumvent that promoting the GPU frequency to maximum
> +  * before we wait. This makes GPU throttle up much more quickly
> +  * (good for benchmarks), but at a cost of spending more power
> +  * processing the workload (bad for battery). Not all clients even
> +  * want their results immediately and for them we should just let
> +  * the GPU select its own frequency to maximise efficiency.
> +  * To prevent a single client from forcing the clocks too high for
> +  * the whole system, we only allow each client to waitboost once
> +  * in a busy period.
> +  */
>   if (INTEL_INFO(req->i915)->gen >= 6)
>   gen6_rps_boost(req->i915, rps, req->emitted_jiffies);
>  
> -- 
> 2.6.2
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Add background commentary to "waitboosting"

[Chris Wilson]

On Fri, Dec 11, 2015 at 07:24:29PM +0100, Daniel Vetter wrote:
> On Thu, Dec 10, 2015 at 10:37:36AM +, Chris Wilson wrote:
> > Describe the intent of boosting the GPU frequency to maximum before
> > waiting on the GPU.
> > 
> > RPS waitboosting was introduced with
> > 
> > commit b29c19b645287f7062e17d70fa4e9781a01a5d88
> > Author: Chris Wilson 
> > Date:   Wed Sep 25 17:34:56 2013 +0100
> > 
> > drm/i915: Boost RPS frequency for CPU stalls
> > 
> > but lacked a concise comment in the code to explain itself.
> > 
> > Signed-off-by: Chris Wilson 
> 
> We might want to consolidate this with the kerneldoc for gen6_rps_boost,
> if that ever happens. But for now this is a good place.

You should end up with both. The function comment will explain what it
does and give reasons why you might want to call it (clients stalls,
detecting missed pageflips, etc), but this code comment should explain
the intent of calling it now. I was tempted to try and put forward some
more objective figures for the benefits of waitboosting, but they grow
stale very quickly so I went with the long winded commentary instead.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx