[Intel-gfx] [PATCH] drm/i915/gt: Make timeslice duration configurable
Mika Kuoppala
mika.kuoppala at linux.intel.com
Tue Oct 29 16:11:58 UTC 2019
Chris Wilson <chris at chris-wilson.co.uk> writes:
> Execlists uses a scheduling quantum (a timeslice) to alternate execution
> between ready-to-run contexts of equal priority. This ensures that all
> users (though only if they of equal importance) have the opportunity to
> run and prevents livelocks where contexts may have implicit ordering due
> to userspace semaphores. However, not all workloads necessarily benefit
> from timeslicing and in the extreme some sysadmin may want to disable or
> reduce the timeslicing granularity.
>
> The timeslicing mechanism can be compiled out with
s/compiled/disabled
>
> ./scripts/config --set-val DRM_I915_TIMESLICE_DURATION 0
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
> drivers/gpu/drm/i915/Kconfig.profile | 15 ++++++++
> drivers/gpu/drm/i915/gt/intel_engine.h | 9 +++++
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +
> drivers/gpu/drm/i915/gt/intel_engine_types.h | 1 +
> drivers/gpu/drm/i915/gt/intel_lrc.c | 39 ++++++++++++++------
> drivers/gpu/drm/i915/gt/selftest_lrc.c | 13 ++++++-
> 6 files changed, 65 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/Kconfig.profile b/drivers/gpu/drm/i915/Kconfig.profile
> index 8ab7af5eb311..1799537a3228 100644
> --- a/drivers/gpu/drm/i915/Kconfig.profile
> +++ b/drivers/gpu/drm/i915/Kconfig.profile
> @@ -59,3 +59,18 @@ config DRM_I915_STOP_TIMEOUT
> damage as the system is reset in order to recover. The corollary is
> that the reset itself may take longer and so be more disruptive to
> interactive or low latency workloads.
> +
> +config DRM_I915_TIMESLICE_DURATION
> + int "Scheduling quantum for userspace batches (ms, jiffy granularity)"
> + default 1 # milliseconds
> + help
> + When two user batches of equal priority are executing, we will
> + alternate execution of each batch to ensure forward progress of
> + all users. This is necessary in some cases where there may be
> + an implicit dependency between those batches that requires
> + concurrent execution in order for them to proceed, e.g. they
> + interact with each other via userspace semaphores. Each context
> + is scheduled for execution for the timeslice duration, before
> + switching to the next context.
> +
> + May be 0 to disable timeslicing.
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index c6895938b626..0597b77f5818 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -335,4 +335,13 @@ intel_engine_has_preempt_reset(const struct intel_engine_cs *engine)
> return intel_engine_has_preemption(engine);
> }
>
> +static inline bool
> +intel_engine_has_timeslices(const struct intel_engine_cs *engine)
> +{
> + if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
> + return 0;
s/0/false;
> +
> + return intel_engine_has_semaphores(engine);
> +}
> +
> #endif /* _INTEL_RINGBUFFER_H_ */
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 9cc1ea6519ec..2afa2ef90482 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -315,6 +315,8 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
> CONFIG_DRM_I915_PREEMPT_TIMEOUT;
> engine->props.stop_timeout_ms =
> CONFIG_DRM_I915_STOP_TIMEOUT;
> + engine->props.timeslice_duration_ms =
> + CONFIG_DRM_I915_TIMESLICE_DURATION;
>
> /*
> * To be overridden by the backend on setup. However to facilitate
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> index e8ea12b96755..c5d1047a4bc5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> @@ -523,6 +523,7 @@ struct intel_engine_cs {
> unsigned long heartbeat_interval_ms;
> unsigned long preempt_timeout_ms;
> unsigned long stop_timeout_ms;
> + unsigned long timeslice_duration_ms;
> } props;
> };
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 2f474c1f5c54..6fb3def5ba16 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1467,7 +1467,7 @@ need_timeslice(struct intel_engine_cs *engine, const struct i915_request *rq)
> {
> int hint;
>
> - if (!intel_engine_has_semaphores(engine))
> + if (!intel_engine_has_timeslices(engine))
> return false;
>
> if (list_is_last(&rq->sched.link, &engine->active.requests))
> @@ -1488,15 +1488,32 @@ switch_prio(struct intel_engine_cs *engine, const struct i915_request *rq)
> return rq_prio(list_next_entry(rq, sched.link));
> }
>
> -static bool
> -enable_timeslice(const struct intel_engine_execlists *execlists)
> +static inline unsigned long
> +timeslice(const struct intel_engine_cs *engine)
could be timeslice_duration, but not insisting
> +{
> + return READ_ONCE(engine->props.timeslice_duration_ms);
> +}
> +
> +static unsigned long
> +active_timeslice(const struct intel_engine_cs *engine)
> {
> - const struct i915_request *rq = *execlists->active;
> + const struct i915_request *rq = *engine->execlists.active;
>
> if (i915_request_completed(rq))
> - return false;
> + return 0;
> +
> + if (engine->execlists.switch_priority_hint < effective_prio(rq))
> + return 0;
> +
> + return timeslice(engine);
> +}
> +
> +static void set_timeslice(struct intel_engine_cs *engine)
> +{
> + if (!intel_engine_has_timeslices(engine))
> + return;
>
> - return execlists->switch_priority_hint >= effective_prio(rq);
> + set_timer_ms(&engine->execlists.timer, active_timeslice(engine));
> }
>
> static void record_preemption(struct intel_engine_execlists *execlists)
> @@ -1667,8 +1684,9 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
> */
> if (!execlists->timer.expires &&
> need_timeslice(engine, last))
> - mod_timer(&execlists->timer,
> - jiffies + 1);
> + set_timer_ms(&execlists->timer,
> + timeslice(engine));
I tripped into the hidden cancellation in here. Not that it
would happen. Still upset I am of this set_timer_ms(timer, 0)
Reviewed-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> +
> return;
> }
>
> @@ -2092,10 +2110,7 @@ static void process_csb(struct intel_engine_cs *engine)
> execlists_num_ports(execlists) *
> sizeof(*execlists->pending));
>
> - if (enable_timeslice(execlists))
> - mod_timer(&execlists->timer, jiffies + 1);
> - else
> - cancel_timer(&execlists->timer);
> + set_timeslice(engine);
>
> WRITE_ONCE(execlists->pending[0], NULL);
> } else {
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index 9507d750ae14..5ed275ef0984 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -440,6 +440,8 @@ static int live_timeslice_preempt(void *arg)
> * need to preempt the current task and replace it with another
> * ready task.
> */
> + if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
> + return 0;
>
> obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
> if (IS_ERR(obj))
> @@ -514,6 +516,11 @@ static void wait_for_submit(struct intel_engine_cs *engine,
> } while (!i915_request_is_active(rq));
> }
>
> +static long timeslice_threshold(const struct intel_engine_cs *engine)
> +{
> + return 2 * msecs_to_jiffies_timeout(timeslice(engine)) + 1;
> +}
> +
> static int live_timeslice_queue(void *arg)
> {
> struct intel_gt *gt = arg;
> @@ -531,6 +538,8 @@ static int live_timeslice_queue(void *arg)
> * ELSP[1] is already occupied, so must rely on timeslicing to
> * eject ELSP[0] in favour of the queue.)
> */
> + if (!CONFIG_DRM_I915_TIMESLICE_DURATION)
> + return 0;
>
> obj = i915_gem_object_create_internal(gt->i915, PAGE_SIZE);
> if (IS_ERR(obj))
> @@ -608,8 +617,8 @@ static int live_timeslice_queue(void *arg)
> err = -EINVAL;
> }
>
> - /* Timeslice every jiffie, so within 2 we should signal */
> - if (i915_request_wait(rq, 0, 3) < 0) {
> + /* Timeslice every jiffy, so within 2 we should signal */
> + if (i915_request_wait(rq, 0, timeslice_threshold(engine)) < 0) {
> struct drm_printer p =
> drm_info_printer(gt->i915->drm.dev);
>
> --
> 2.24.0.rc1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list