Re: [PATCH 01/12] drm/i915: Allow normal clients to always preempt idle priority clients

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 01/12] drm/i915: Allow normal clients to always preempt idle priority clients
Date: Mon, 4 Feb 2019 09:20:26 +0000	[thread overview]
Message-ID: <82bbcac0-2f63-5ed4-856b-f8b6a27a8d35@linux.intel.com> (raw)
In-Reply-To: <20190204084116.3013-1-chris@chris-wilson.co.uk>


On 04/02/2019 08:41, Chris Wilson wrote:
> When first enabling preemption, we hesitated from making it a free-for-all
> where every higher priority client would force a preempt-to-idle cycle
> and take over from all lower priority clients. We hesitated because we
> were uncertain just how well preemption would work in practice, whether
> the preemption latency itself would detract from the latency gains for
> higher priority tasks and whether it would work at all. Since
> introducing preemption, we have been enabling it for more common tasks,
> even giving normal clients a small preemptive boost when they first
> start (to aide fairness and improve interactivity). Now lets take one
> step further and give permission for all normal (priority:0) clients to
> preempt any idle (priority:<0) task so that users running long compute
> jobs do not overly impact other jobs (i.e. their desktop) and the system
> remains responsive under such idle loads.
> 
> References: f6322eddaff7 ("drm/i915/preemption: Allow preemption between submission ports")
> References: b16c765122f9 ("drm/i915: Priority boost for new clients")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: "Bloomfield, Jon" <jon.bloomfield@intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ringbuffer.h | 15 ++++++++++++++-
>   1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 34d0a148e664..983ad1e7914d 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -592,7 +592,20 @@ intel_engine_has_preemption(const struct intel_engine_cs *engine)
>   
>   static inline bool __execlists_need_preempt(int prio, int last)
>   {
> -	return prio > max(0, last);
> +	/*
> +	 * Allow preemption of low -> normal -> high, but we do
> +	 * not allow low priority tasks to preempt other low priority
> +	 * tasks under the impression that latency for low priority
> +	 * tasks does not matter (as much as background throughput),
> +	 * so kiss.
> +	 *
> +	 * More naturally we would write
> +	 * 	prio >= max(0, last);
> +	 * except that we wish to prevent triggering preemption at the same
> +	 * priority level: the task that is running should remain running
> +	 * to preserve FIFO ordering of dependencies.
> +	 */
> +	return prio > max(I915_PRIORITY_NORMAL - 1, last);
>   }
>   
>   static inline void
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx