linux-rt-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 6.1-rt/5.15-rt 0/1] Backport i915 reset fix from v6.4-rt
@ 2023-08-19  2:45 paul.gortmaker
  2023-08-19  2:45 ` [PATCH 1/1] drm/i915: Do not disable preemption for resets paul.gortmaker
  2023-08-23 13:39 ` [PATCH 6.1-rt/5.15-rt 0/1] Backport i915 reset fix from v6.4-rt Sebastian Andrzej Siewior
  0 siblings, 2 replies; 5+ messages in thread
From: paul.gortmaker @ 2023-08-19  2:45 UTC (permalink / raw)
  To: Clark Williams, Joseph Salisbury
  Cc: linux-rt-users, Sebastian Andrzej Siewior, Tvrtko Ursulin

From: Paul Gortmaker <paul.gortmaker@windriver.com>

I'd originally reported an i915 regression caused by a linux-stable
backport, and then later posted an RFC fix for it:

https://lore.kernel.org/linux-rt-users/ZJTuDi1HNp9L2zxY@windriver.com/

That eventually led to further discussion and an eventual fix from
Tvrtko that Sebastian put into the v6.4-rt queue, which is great.

However, the original regression was reported on v5.15-rt.  This closes
the loop by backporting the v6.4-rt change to v5.15-rt/v6.1-rt -- as it
turns out, the same backport applies cleanly to both baselines.

The only small change required is to fix up the context a bit in the 5th
and final chunk, because kernel versions before v6.4 dont have the commit
b7d70b8b06ed ("drm/i915/gsc: implement wa 14015076503")

Paul.
--

Cc: Clark Williams <williams@redhat.com>		[6.1-rt]
Cc: Joseph Salisbury <joseph.salisbury@canonical.com>	[5.15-rt]

---

Tvrtko Ursulin (1):
  drm/i915: Do not disable preemption for resets

 drivers/gpu/drm/i915/gt/intel_reset.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

-- 
2.40.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/1] drm/i915: Do not disable preemption for resets
  2023-08-19  2:45 [PATCH 6.1-rt/5.15-rt 0/1] Backport i915 reset fix from v6.4-rt paul.gortmaker
@ 2023-08-19  2:45 ` paul.gortmaker
  2023-08-23 13:39 ` [PATCH 6.1-rt/5.15-rt 0/1] Backport i915 reset fix from v6.4-rt Sebastian Andrzej Siewior
  1 sibling, 0 replies; 5+ messages in thread
From: paul.gortmaker @ 2023-08-19  2:45 UTC (permalink / raw)
  To: Clark Williams, Joseph Salisbury
  Cc: linux-rt-users, Sebastian Andrzej Siewior, Tvrtko Ursulin

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

[commit 40cd2835ced288789a685aa4aa7bc04b492dcd45 in linux-rt-devel]

Commit ade8a0f59844 ("drm/i915: Make all GPU resets atomic") added a
preempt disable section over the hardware reset callback to prepare the
driver for being able to reset from atomic contexts.

In retrospect I can see that the work item at a time was about removing
the struct mutex from the reset path. Code base also briefly entertained
the idea of doing the reset under stop_machine in order to serialize
userspace mmap and temporary glitch in the fence registers (see
eb8d0f5af4ec ("drm/i915: Remove GPU reset dependence on struct_mutex"),
but that never materialized and was soon removed in 2caffbf11762
("drm/i915: Revoke mmaps and prevent access to fence registers across
reset") and replaced with a SRCU based solution.

As such, as far as I can see, today we still have a requirement that
resets must not sleep (invoked from submission tasklets), but no need to
support invoking them from a truly atomic context.

Given that the preemption section is problematic on RT kernels, since the
uncore lock becomes a sleeping lock and so is invalid in such section,
lets try and remove it. Potential downside is that our short waits on GPU
to complete the reset may get extended if CPU scheduling interferes, but
in practice that probably isn't a deal breaker.

In terms of mechanics, since the preemption disabled block is being
removed we just need to replace a few of the wait_for_atomic macros into
busy looping versions which will work (and not complain) when called from
non-atomic sections.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Chris Wilson <chris.p.wilson@intel.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Acked-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Link: https://lore.kernel.org/r/20230705093025.3689748-1-tvrtko.ursulin@linux.intel.com
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
[PG: backport from v6.4-rt ; minor context fixup caused by b7d70b8b06ed]
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/gpu/drm/i915/gt/intel_reset.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 10b930eaa8cb..6108a449cd19 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -174,13 +174,13 @@ static int i915_do_reset(struct intel_gt *gt,
 	/* Assert reset for at least 20 usec, and wait for acknowledgement. */
 	pci_write_config_byte(pdev, I915_GDRST, GRDOM_RESET_ENABLE);
 	udelay(50);
-	err = wait_for_atomic(i915_in_reset(pdev), 50);
+	err = _wait_for_atomic(i915_in_reset(pdev), 50, 0);
 
 	/* Clear the reset request. */
 	pci_write_config_byte(pdev, I915_GDRST, 0);
 	udelay(50);
 	if (!err)
-		err = wait_for_atomic(!i915_in_reset(pdev), 50);
+		err = _wait_for_atomic(!i915_in_reset(pdev), 50, 0);
 
 	return err;
 }
@@ -200,7 +200,7 @@ static int g33_do_reset(struct intel_gt *gt,
 	struct pci_dev *pdev = to_pci_dev(gt->i915->drm.dev);
 
 	pci_write_config_byte(pdev, I915_GDRST, GRDOM_RESET_ENABLE);
-	return wait_for_atomic(g4x_reset_complete(pdev), 50);
+	return _wait_for_atomic(g4x_reset_complete(pdev), 50, 0);
 }
 
 static int g4x_do_reset(struct intel_gt *gt,
@@ -217,7 +217,7 @@ static int g4x_do_reset(struct intel_gt *gt,
 
 	pci_write_config_byte(pdev, I915_GDRST,
 			      GRDOM_MEDIA | GRDOM_RESET_ENABLE);
-	ret =  wait_for_atomic(g4x_reset_complete(pdev), 50);
+	ret =  _wait_for_atomic(g4x_reset_complete(pdev), 50, 0);
 	if (ret) {
 		GT_TRACE(gt, "Wait for media reset failed\n");
 		goto out;
@@ -225,7 +225,7 @@ static int g4x_do_reset(struct intel_gt *gt,
 
 	pci_write_config_byte(pdev, I915_GDRST,
 			      GRDOM_RENDER | GRDOM_RESET_ENABLE);
-	ret =  wait_for_atomic(g4x_reset_complete(pdev), 50);
+	ret =  _wait_for_atomic(g4x_reset_complete(pdev), 50, 0);
 	if (ret) {
 		GT_TRACE(gt, "Wait for render reset failed\n");
 		goto out;
@@ -718,9 +718,7 @@ int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
 	intel_uncore_forcewake_get(gt->uncore, FORCEWAKE_ALL);
 	for (retry = 0; ret == -ETIMEDOUT && retry < retries; retry++) {
 		GT_TRACE(gt, "engine_mask=%x\n", engine_mask);
-		preempt_disable();
 		ret = reset(gt, engine_mask, retry);
-		preempt_enable();
 	}
 	intel_uncore_forcewake_put(gt->uncore, FORCEWAKE_ALL);
 
-- 
2.40.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 6.1-rt/5.15-rt 0/1] Backport i915 reset fix from v6.4-rt
  2023-08-19  2:45 [PATCH 6.1-rt/5.15-rt 0/1] Backport i915 reset fix from v6.4-rt paul.gortmaker
  2023-08-19  2:45 ` [PATCH 1/1] drm/i915: Do not disable preemption for resets paul.gortmaker
@ 2023-08-23 13:39 ` Sebastian Andrzej Siewior
  2023-08-23 17:57   ` Joseph Salisbury
  2023-10-18 19:58   ` Joseph Salisbury
  1 sibling, 2 replies; 5+ messages in thread
From: Sebastian Andrzej Siewior @ 2023-08-23 13:39 UTC (permalink / raw)
  To: paul.gortmaker
  Cc: Clark Williams, Joseph Salisbury, linux-rt-users, Tvrtko Ursulin

On 2023-08-18 22:45:24 [-0400], paul.gortmaker@windriver.com wrote:
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
Hi Paul,

> However, the original regression was reported on v5.15-rt.  This closes
> the loop by backporting the v6.4-rt change to v5.15-rt/v6.1-rt -- as it
> turns out, the same backport applies cleanly to both baselines.

Thanks for the ping. I've sent a backport request
	https://lore.kernel.org/all/20230727105040.4V9DQ5pM@linutronix.de/

which includes the patch. It didn't go very so far but I am full of hope
;)

Sebastian

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 6.1-rt/5.15-rt 0/1] Backport i915 reset fix from v6.4-rt
  2023-08-23 13:39 ` [PATCH 6.1-rt/5.15-rt 0/1] Backport i915 reset fix from v6.4-rt Sebastian Andrzej Siewior
@ 2023-08-23 17:57   ` Joseph Salisbury
  2023-10-18 19:58   ` Joseph Salisbury
  1 sibling, 0 replies; 5+ messages in thread
From: Joseph Salisbury @ 2023-08-23 17:57 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior, paul.gortmaker
  Cc: Clark Williams, linux-rt-users, Tvrtko Ursulin



On 8/23/23 09:39, Sebastian Andrzej Siewior wrote:
> On 2023-08-18 22:45:24 [-0400], paul.gortmaker@windriver.com wrote:
>> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> Hi Paul,
>
>> However, the original regression was reported on v5.15-rt.  This closes
>> the loop by backporting the v6.4-rt change to v5.15-rt/v6.1-rt -- as it
>> turns out, the same backport applies cleanly to both baselines.
> Thanks for the ping. I've sent a backport request
> 	https://lore.kernel.org/all/20230727105040.4V9DQ5pM@linutronix.de/
>
> which includes the patch. It didn't go very so far but I am full of hope
> ;)
>
> Sebastian
Hi Paul,

I will put this on my list to backport to v5.15-rt stable.  I usually 
wait until requested backports land in the next newer version, which is 
v6.1-rt, before I apply to v5.15-rt.

Thanks,

Joe


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 6.1-rt/5.15-rt 0/1] Backport i915 reset fix from v6.4-rt
  2023-08-23 13:39 ` [PATCH 6.1-rt/5.15-rt 0/1] Backport i915 reset fix from v6.4-rt Sebastian Andrzej Siewior
  2023-08-23 17:57   ` Joseph Salisbury
@ 2023-10-18 19:58   ` Joseph Salisbury
  1 sibling, 0 replies; 5+ messages in thread
From: Joseph Salisbury @ 2023-10-18 19:58 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior, paul.gortmaker
  Cc: Clark Williams, linux-rt-users, Tvrtko Ursulin



On 8/23/23 09:39, Sebastian Andrzej Siewior wrote:
> On 2023-08-18 22:45:24 [-0400], paul.gortmaker@windriver.com wrote:
>> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> Hi Paul,
>
>> However, the original regression was reported on v5.15-rt.  This closes
>> the loop by backporting the v6.4-rt change to v5.15-rt/v6.1-rt -- as it
>> turns out, the same backport applies cleanly to both baselines.
> Thanks for the ping. I've sent a backport request
> 	https://lore.kernel.org/all/20230727105040.4V9DQ5pM@linutronix.de/
>
> which includes the patch. It didn't go very so far but I am full of hope
> ;)
>
> Sebastian
Hi Paul,

I just announced a release candidate that has this patch.  The version 
is: v5.15.133-rt70-rc1.

If there are no negative reports, this will be the next v5.15-rt 
release, which only contains the patches in the announcement.

Thanks,

Joe


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-10-18 19:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-19  2:45 [PATCH 6.1-rt/5.15-rt 0/1] Backport i915 reset fix from v6.4-rt paul.gortmaker
2023-08-19  2:45 ` [PATCH 1/1] drm/i915: Do not disable preemption for resets paul.gortmaker
2023-08-23 13:39 ` [PATCH 6.1-rt/5.15-rt 0/1] Backport i915 reset fix from v6.4-rt Sebastian Andrzej Siewior
2023-08-23 17:57   ` Joseph Salisbury
2023-10-18 19:58   ` Joseph Salisbury

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).