Re: [Intel-gfx] [PATCH 17/20] drm/i915/gt: Declare when we enabled timeslicing

From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Cc: Kenneth Graunke <kenneth@whitecape.org>
Subject: Re: [Intel-gfx] [PATCH 17/20] drm/i915/gt: Declare when we enabled timeslicing
Date: Fri, 28 Feb 2020 12:45:42 +0000	[thread overview]
Message-ID: <eae19a92-c807-4663-4509-f47e33f1209c@linux.intel.com> (raw)
In-Reply-To: <20200227085723.1961649-17-chris@chris-wilson.co.uk>

On 27/02/2020 08:57, Chris Wilson wrote:
> Let userspace know if they can trust timeslicing by including it as part
> of the I915_PARAM_HAS_SCHEDULER::I915_SCHEDULER_CAP_TIMESLICING
> 
> v2: Only declare timeslicing if we can safely preempt userspace.
> 
> Fixes: 8ee36e048c98 ("drm/i915/execlists: Minimalistic timeslicing")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine.h      | 3 ++-
>   drivers/gpu/drm/i915/gt/intel_engine_user.c | 5 +++++
>   include/uapi/drm/i915_drm.h                 | 1 +
>   3 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index 29c8c03c5caa..a32dc82a90d4 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -326,7 +326,8 @@ intel_engine_has_timeslices(const struct intel_engine_cs *engine)
>   	if (!IS_ACTIVE(CONFIG_DRM_I915_TIMESLICE_DURATION))
>   		return false;
>   
> -	return intel_engine_has_semaphores(engine);
> +	return (intel_engine_has_semaphores(engine) &&
> +		intel_engine_has_preemption(engine));

This could be the fixes bit.

AFAIR has_semaphores is can we preempt between batches and 
has_preemption is can we preempt inside them. What was the consequence 
of not considering this in has_timeslices?

>   }
>   
>   #endif /* _INTEL_RINGBUFFER_H_ */
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_user.c b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> index 848decee9066..b84fdd722781 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_user.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_user.c
> @@ -121,6 +121,11 @@ static void set_scheduler_caps(struct drm_i915_private *i915)
>   			else
>   				disabled |= BIT(map[i].sched);
>   		}
> +
> +		if (intel_engine_has_timeslices(engine))
> +			enabled |= I915_SCHEDULER_CAP_TIMESLICING;
> +		else
> +			disabled |= I915_SCHEDULER_CAP_TIMESLICING;
>   	}
>   
>   	i915->caps.scheduler = enabled & ~disabled;
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 2813e579b480..4f903431a3fe 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -523,6 +523,7 @@ typedef struct drm_i915_irq_wait {
>   #define   I915_SCHEDULER_CAP_PREEMPTION	(1ul << 2)
>   #define   I915_SCHEDULER_CAP_SEMAPHORES	(1ul << 3)
>   #define   I915_SCHEDULER_CAP_ENGINE_BUSY_STATS	(1ul << 4)
> +#define   I915_SCHEDULER_CAP_TIMESLICING	(1ul << 5)

And this would be the new uapi bit.

Split into two patches?

Regards,

Tvrtko

>   
>   #define I915_PARAM_HUC_STATUS		 42
>   
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx