[PATCH 1/4] drm/i915/gt: Do not schedule normal requests immediately along virtual

[PATCH 1/4] drm/i915/gt: Do not schedule normal requests immediately along virtual

[Chris Wilson]

When we push a virtual request onto the HW, we update the rq->engine to
point to the physical engine. A request that is then submitted by the
user that waits upon the virtual engine, but along the physical engine
in use, will then see that it is due to be submitted to the same engine
and take a shortcut (and be queued without waiting for the completion
fence). However, the virtual request may be preempted (either by higher
priority users, or by timeslicing) and removed from the physical engine
to be migrated over to one of its siblings. The dependent normal request
however is oblivious to the removal of the virtual request and remains
queued to execute on HW, believing that once it reaches the head of its
queue all of its predecessors will have completed executing!

Fixes: 6d06779e8672 ("drm/i915: Load balancing across a virtual engine")
Testcase: igt/gem_exec_balancer/sliced
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: <stable@vger.kernel.org> # v5.3+
---
 drivers/gpu/drm/i915/i915_request.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index c282719ad3ac..51588209bddd 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1074,7 +1074,7 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from)
 			return ret;
 	}
 
-	if (to->engine == from->engine)
+	if (is_power_of_2(to->execution_mask | READ_ONCE(from->execution_mask)))
 		ret = i915_sw_fence_await_sw_fence_gfp(&to->submit,
 						       &from->submit,
 						       I915_FENCE_GFP);
-- 
2.20.1

Re: [PATCH 1/4] drm/i915/gt: Do not schedule normal requests immediately along virtual

[Sasha Levin]

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: 6d06779e8672 ("drm/i915: Load balancing across a virtual engine").

The bot has tested the following trees: v5.6.14, v5.4.42.

v5.6.14: Build OK!
v5.4.42: Failed to apply! Possible dependencies:
    0b718ba1e884 ("drm/i915/gtt: Downgrade Cherryview back to aliasing-ppgtt")
    0f100b70487a ("drm/i915: Push the use-semaphore marker onto the intel_context")
    2e0986a58cc4 ("drm/i915/gem: Cancel contexts when hangchecking is disabled")
    4a3174152147 ("drm/i915/gem: Refine occupancy test in kill_context()")
    5bf05dc58d65 ("drm/i915/tgl: Register state context definition for Gen12")
    7dc56af5260e ("drm/i915/selftests: Verify the LRC register layout between init and HW")
    8f8b1171e1a5 ("drm/i915/perf: Wean ourselves off dev_priv")
    9cd20ef7803c ("drm/i915/perf: allow holding preemption on filtered ctx")
    9f3ccd40acf4 ("drm/i915: Drop GEM context as a direct link from i915_request")
    a0e047156cde ("drm/i915/gem: Make context persistence optional")
    cdb736fa8b8b ("drm/i915: Use engine relative LRIs on context setup")
    eaef5b3c4113 ("drm/i915: Refactor instdone loops on new subslice functions")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha

Re: [Intel-gfx] [PATCH 1/4] drm/i915/gt: Do not schedule normal requests immediately along virtual

[Tvrtko Ursulin]

On 25/05/2020 21:28, Chris Wilson wrote:
> When we push a virtual request onto the HW, we update the rq->engine to
> point to the physical engine. A request that is then submitted by the
> user that waits upon the virtual engine, but along the physical engine
> in use, will then see that it is due to be submitted to the same engine
> and take a shortcut (and be queued without waiting for the completion
> fence). However, the virtual request may be preempted (either by higher
> priority users, or by timeslicing) and removed from the physical engine
> to be migrated over to one of its siblings. The dependent normal request
> however is oblivious to the removal of the virtual request and remains
> queued to execute on HW, believing that once it reaches the head of its
> queue all of its predecessors will have completed executing!
> 
> Fixes: 6d06779e8672 ("drm/i915: Load balancing across a virtual engine")
> Testcase: igt/gem_exec_balancer/sliced
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: <stable@vger.kernel.org> # v5.3+
> ---
>   drivers/gpu/drm/i915/i915_request.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index c282719ad3ac..51588209bddd 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1074,7 +1074,7 @@ i915_request_await_request(struct i915_request *to, struct i915_request *from)
>   			return ret;
>   	}
>   
> -	if (to->engine == from->engine)
> +	if (is_power_of_2(to->execution_mask | READ_ONCE(from->execution_mask)))
>   		ret = i915_sw_fence_await_sw_fence_gfp(&to->submit,
>   						       &from->submit,
>   						       I915_FENCE_GFP);
> 

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

Regards,

Tvrtko