[Intel-gfx] [PATCH 2/2] drm/i915: Expose idle-delays to Kconfig

[Tvrtko Ursulin]

On 22/12/2017 11:33, Chris Wilson wrote:
> We want to expose the parameters for controlling how long it takes for
> us to notice and park the GPU after a stream of requests in order to try
>   and tune the optimal power-efficiency vs latency of a mostly idle system.

Extra space.

> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>   drivers/gpu/drm/i915/Kconfig.profile    | 20 ++++++++++++++++++++
>   drivers/gpu/drm/i915/i915_gem.c         | 12 +++++++++++-
>   drivers/gpu/drm/i915/i915_gem_request.c |  2 +-
>   3 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile
> index 8a230eeb98df..02253e7c18e1 100644
> --- a/drivers/gpu/drm/i915/Kconfig.profile
> +++ b/drivers/gpu/drm/i915/Kconfig.profile
> @@ -24,3 +24,23 @@ config DRM_I915_SPIN_REQUEST_CS
>   	  spin prior to sleeping again.
>   
>   	  May be 0 to disable spinning after being woken.
> +
> +config DRM_I915_GEM_RETIRE_DELAY
> +	int
> +	default 1000 # milliseconds
> +	help
> +	  Before parking the engines and the GPU after the final request is

Copy-paste error.

> +	  We maintain a background job whose purpose is to keep cleaning up
> +	  after userspace, and will be the first to spot an idle system. This
> +	  parameter determines th interval between execution of the woker.

s/th/the/,s/woker/worker/

> +
> +config DRM_I915_GEM_PARK_DELAY
> +	int
> +	default 100 # milliseconds
> +	help
> +	  Before parking the engines and the GPU after the final request is
> +	  retired, we may wait for a small delay to reduce the frequecy of

frequency

> +	  having to park/unpark and so the latency in executing a new request.
> +
> +	  May be 0 to immediately start parking the engines after the last
> +	  request.
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ba9f67c256f4..ba15f0edda26 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3310,6 +3310,16 @@ bool i915_gem_unset_wedged(struct drm_i915_private *i915)
>   	return true;
>   }
>   
> +static inline long retire_delay(void)
> +{
> +	const long delay = msecs_to_jiffies(CONFIG_DRM_I915_GEM_RETIRE_DELAY);
> +
> +	if (CONFIG_DRM_I915_GEM_RETIRE_DELAY >= 1000)
> +		return round_jiffies_up_relative(delay);

Round to a second? I am not sure we want that if exposing a tunable, but 
at least need to say in Kconfig. But I don't see why we would round as I 
said.

> +	else
> +		return delay;
> +}
> +
>   static void
>   i915_gem_retire_work_handler(struct work_struct *work)
>   {
> @@ -3331,7 +3341,7 @@ i915_gem_retire_work_handler(struct work_struct *work)
>   		i915_queue_hangcheck(dev_priv);
>   		queue_delayed_work(dev_priv->wq,
>   				   &dev_priv->gt.retire_work,
> -				   round_jiffies_up_relative(HZ));
> +				   retire_delay());
>   	}

i915_gem_request.c/mark_busy needs to use it as well.

>   }
>   
> diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c
> index bb8b45d4d04d..1c6d7d18e5ee 100644
> --- a/drivers/gpu/drm/i915/i915_gem_request.c
> +++ b/drivers/gpu/drm/i915/i915_gem_request.c
> @@ -312,7 +312,7 @@ static void unreserve_engine(struct intel_engine_cs *engine)
>   		GEM_BUG_ON(!i915->gt.awake);
>   		mod_delayed_work(i915->wq,
>   				 &i915->gt.idle_work,
> -				 msecs_to_jiffies(100));
> +				 msecs_to_jiffies(CONFIG_DRM_I915_GEM_PARK_DELAY));
>   	}
>   
>   	GEM_BUG_ON(!engine->timeline->inflight_seqnos);
> 

Regards,

Tvrtko