Re: [Intel-gfx] [PATCH 5/7] drm/i915: Disable use of hwsp_cacheline for kernel_context

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Currently on execlists, we use a local hwsp for the kernel_context,
> rather than the engine's HWSP, as this is the default for execlists.
> However, seqno rollover requires allocating a new HWSP cachline, and may

s/cachline/cacheline

> require pinning a new HWSP page in the GTT. This operation requiring

s/GTT/GGTT

> pinning in the GGTT is not allowed within the kernel_context timeline,
> as doing so may require re-entering the kernel_context in order to evict
> from the GGTT. As we want to avoid requiring a new HWSP for the
> kernel_context, we can use the permanently pinned engine's HWSP instead.
> However to do so we must prevent the use of semaphores reading the
> kernel_context's HWSP, as the use of semaphores do not support rollover

Back in a time, it was called seqno wrap. But I guess the cacheline
jump warrants a rollover.

> onto the same cacheline. Fortunately, the kernel_context is mostly
> isolated, so unlikely to give benefit to semaphores.
>
> Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c    | 14 ++++++++++++--
>  drivers/gpu/drm/i915/gt/selftest_lrc.c | 12 +++++++++---
>  drivers/gpu/drm/i915/i915_request.c    | 14 +++++++++-----
>  3 files changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 70d91ad923ef..902d440ef07d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -2964,7 +2964,8 @@ static int gen8_emit_init_breadcrumb(struct i915_request *rq)
>  {
>  	u32 *cs;
>  
> -	GEM_BUG_ON(!i915_request_timeline(rq)->has_initial_breadcrumb);
> +	if (!i915_request_timeline(rq)->has_initial_breadcrumb)
> +		return 0;
>  
>  	cs = intel_ring_begin(rq, 6);
>  	if (IS_ERR(cs))
> @@ -4616,8 +4617,17 @@ static int __execlists_context_alloc(struct intel_context *ce,
>  
>  	if (!ce->timeline) {
>  		struct intel_timeline *tl;
> +		struct i915_vma *hwsp;
> +
> +		/*
> +		 * Use the static global HWSP for the kernel context, and
> +		 * a dynamically allocated cacheline for everyone else.
> +		 */
> +		hwsp = NULL;
> +		if (unlikely(intel_context_is_barrier(ce)))
> +			hwsp = engine->status_page.vma;
>  
> -		tl = intel_timeline_create(engine->gt, NULL);
> +		tl = intel_timeline_create(engine->gt, hwsp);
>  		if (IS_ERR(tl)) {
>  			ret = PTR_ERR(tl);
>  			goto error_deref_obj;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index ccd4cd2c202d..6f458f6d5523 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -3494,15 +3494,21 @@ static int bond_virtual_engine(struct intel_gt *gt,
>  	rq[0] = ERR_PTR(-ENOMEM);
>  	for_each_engine(master, gt, id) {
>  		struct i915_sw_fence fence = {};
> +		struct intel_context *ce;
>  
>  		if (master->class == class)
>  			continue;
>  
> +		ce = intel_context_create(master);
> +		if (IS_ERR(ce)) {
> +			err = PTR_ERR(ce);
> +			goto out;
> +		}
> +
>  		memset_p((void *)rq, ERR_PTR(-EINVAL), ARRAY_SIZE(rq));
>  
> -		rq[0] = igt_spinner_create_request(&spin,
> -						   master->kernel_context,
> -						   MI_NOOP);
> +		rq[0] = igt_spinner_create_request(&spin, ce, MI_NOOP);
> +		intel_context_put(ce);
>  		if (IS_ERR(rq[0])) {
>  			err = PTR_ERR(rq[0]);
>  			goto out;
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 0ecc2cf64216..1adb8cf35f75 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -886,6 +886,12 @@ emit_semaphore_wait(struct i915_request *to,
>  		    struct i915_request *from,
>  		    gfp_t gfp)
>  {
> +	if (!intel_context_use_semaphores(to->context))
> +		goto await_fence;
> +
> +	if (!rcu_access_pointer(from->hwsp_cacheline))
> +		goto await_fence;
> +
>  	/* Just emit the first semaphore we see as request space is limited. */
>  	if (already_busywaiting(to) & from->engine->mask)
>  		goto await_fence;
> @@ -931,12 +937,8 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from)
>  		ret = i915_sw_fence_await_sw_fence_gfp(&to->submit,
>  						       &from->submit,
>  						       I915_FENCE_GFP);
> -	else if (intel_context_use_semaphores(to->context))
> -		ret = emit_semaphore_wait(to, from, I915_FENCE_GFP);
>  	else
> -		ret = i915_sw_fence_await_dma_fence(&to->submit,
> -						    &from->fence, 0,
> -						    I915_FENCE_GFP);
> +		ret = emit_semaphore_wait(to, from, I915_FENCE_GFP);
>  	if (ret < 0)
>  		return ret;
>  
> @@ -1035,6 +1037,8 @@ __i915_request_await_execution(struct i915_request *to,
>  {
>  	int err;
>  
> +	GEM_BUG_ON(intel_context_is_barrier(from->context));
> +
>  	/* Submit both requests at the same time */
>  	err = __await_execution(to, from, hook, I915_FENCE_GFP);
>  	if (err)
> -- 
> 2.25.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx