On Tue, Aug 17, 2021 at 11:21:27AM +0200, Daniel Vetter wrote:
On Mon, Aug 16, 2021 at 06:51:23AM -0700, Matthew Brost wrote:
Progagating errors to dependent fences is wrong, don't do it. Selftest in following patch exposes this bug.
Please explain what "this bug" is, it's hard to read minds, especially at a distance in spacetime :-)
Not a very good explaination.
Fixes: 8e9f84cf5cac ("drm/i915/gt: Propagate change in error status to children on unhold")
I think it would be better to outright revert this, instead of just disabling it like this.
I tried revert and git did some really odd things that I couldn't resolve, hence the new patch.
Also please cite the dma_fence error propagation revert from Jason:
commit 93a2711cddd5760e2f0f901817d71c93183c3b87 Author: Jason Ekstrand jason@jlekstrand.net Date: Wed Jul 14 14:34:16 2021 -0500
Revert "drm/i915: Propagate errors on awaiting already signaled fences"Maybe in full, if you need the justification.
Will site.
Signed-off-by: Matthew Brost matthew.brost@intel.com Cc: stable@vger.kernel.org
Unless "this bug" is some real world impact thing I wouldn't put cc: stable on this.
Got it.
Matt
-Daniel
drivers/gpu/drm/i915/gt/intel_execlists_submission.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c index de5f9c86b9a4..cafb0608ffb4 100644 --- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c +++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c @@ -2140,10 +2140,6 @@ static void __execlists_unhold(struct i915_request *rq) if (p->flags & I915_DEPENDENCY_WEAK) continue;
/* Propagate any change in error status */if (rq->fence.error)i915_request_set_error_once(w, rq->fence.error);if (w->engine != rq->engine) continue;-- 2.32.0
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch