[PATCH 00/10] drm/i915: PREEMPT_RT related fixups.

[PATCH 00/10] drm/i915: PREEMPT_RT related fixups.

[Sebastian Andrzej Siewior]

Hi,

The following patches are from the PREEMPT_RT queue.  It is mostly about
disabling interrupts/preemption which leads to problems. Unfortunately
DRM_I915_LOW_LEVEL_TRACEPOINTS had to be disabled because it acquires locks
from within trace points. Making the lock a raw_spinlock_t led to higher
latencies during video playback
  https://lore.kernel.org/all/20211006164628.s2mtsdd2jdbfyf7g@linutronix.de/

and I'm not sure if I hit the worse case here.
I tested it on a SandyBridge with built-in i915 by using X, OpenGL and
playing videos without noticing any warnings. However, some code paths
were not entered.
I carry them for some time now and most issues were reported by other
people and they reported that things work for them since.

Sebastian

[PATCH 01/10] drm/i915: Use preempt_disable/enable_rt() where recommended

[Sebastian Andrzej Siewior]

From: Mike Galbraith <umgwanakikbuti@gmail.com>

Mario Kleiner suggest in commit
  ad3543ede630f ("drm/intel: Push get_scanout_position() timestamping into kms driver.")

a spots where preemption should be disabled on PREEMPT_RT. The
difference is that on PREEMPT_RT the intel_uncore::lock disables neither
preemption nor interrupts and so region remains preemptible.

The area covers only register reads and writes. The part that worries me
is:
- __intel_get_crtc_scanline() the worst case is 100us if no match is
  found.

- intel_crtc_scanlines_since_frame_timestamp() not sure how long this
  may take in the worst case.

It was in the RT queue for a while and nobody complained.
Disable preemption on PREEPMPT_RT during timestamping.

[bigeasy: patch description.]

Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
Signed-off-by: Mike Galbraith <umgwanakikbuti@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/gpu/drm/i915/display/intel_vblank.c | 38 +++++++++++++++------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c b/drivers/gpu/drm/i915/display/intel_vblank.c
index baf7354cb6e2c..71d6071121460 100644
--- a/drivers/gpu/drm/i915/display/intel_vblank.c
+++ b/drivers/gpu/drm/i915/display/intel_vblank.c
@@ -276,6 +276,26 @@ int intel_crtc_scanline_to_hw(struct intel_crtc *crtc, int scanline)
  * all register accesses to the same cacheline to be serialized,
  * otherwise they may hang.
  */
+static void intel_vblank_section_enter_irqsave(struct drm_i915_private *i915, unsigned long *flags)
+	__acquires(i915->uncore.lock)
+{
+#ifdef I915
+	spin_lock_irqsave(&i915->uncore.lock, *flags);
+#else
+	*flags = 0;
+#endif
+}
+
+static void intel_vblank_section_exit_irqrestore(struct drm_i915_private *i915, unsigned long flags)
+	__releases(i915->uncore.lock)
+{
+#ifdef I915
+	spin_unlock_irqrestore(&i915->uncore.lock, flags);
+#else
+	if (flags)
+		return;
+#endif
+}
 static void intel_vblank_section_enter(struct drm_i915_private *i915)
 	__acquires(i915->uncore.lock)
 {
@@ -333,10 +353,10 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
 	 * timing critical raw register reads, potentially with
 	 * preemption disabled, so the following code must not block.
 	 */
-	local_irq_save(irqflags);
-	intel_vblank_section_enter(dev_priv);
+	intel_vblank_section_enter_irqsave(dev_priv, &irqflags);
 
-	/* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
+	if (IS_ENABLED(CONFIG_PREEMPT_RT))
+		preempt_disable();
 
 	/* Get optional system timestamp before query. */
 	if (stime)
@@ -400,10 +420,10 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
 	if (etime)
 		*etime = ktime_get();
 
-	/* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */
+	if (IS_ENABLED(CONFIG_PREEMPT_RT))
+		preempt_enable();
 
-	intel_vblank_section_exit(dev_priv);
-	local_irq_restore(irqflags);
+	intel_vblank_section_exit_irqrestore(dev_priv, irqflags);
 
 	/*
 	 * While in vblank, position will be negative
@@ -441,13 +461,11 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
 	unsigned long irqflags;
 	int position;
 
-	local_irq_save(irqflags);
-	intel_vblank_section_enter(dev_priv);
+	intel_vblank_section_enter_irqsave(dev_priv, &irqflags);
 
 	position = __intel_get_crtc_scanline(crtc);
 
-	intel_vblank_section_exit(dev_priv);
-	local_irq_restore(irqflags);
+	intel_vblank_section_exit_irqrestore(dev_priv, irqflags);
 
 	return position;
 }
-- 
2.43.0

[PATCH 02/10] drm/i915: Don't disable interrupts on PREEMPT_RT during atomic updates

[Sebastian Andrzej Siewior]

From: Mike Galbraith <umgwanakikbuti@gmail.com>

Commit
   8d7849db3eab7 ("drm/i915: Make sprite updates atomic")

started disabling interrupts across atomic updates. This breaks on PREEMPT_RT
because within this section the code attempt to acquire spinlock_t locks which
are sleeping locks on PREEMPT_RT.

According to the comment the interrupts are disabled to avoid random delays and
not required for protection or synchronisation.
If this needs to happen with disabled interrupts on PREEMPT_RT, and the
whole section is restricted to register access then all sleeping locks
need to be acquired before interrupts are disabled and some function
maybe moved after enabling interrupts again.
This includes:
- prepare_to_wait() + finish_wait() due its wake queue.
- drm_crtc_vblank_put() -> vblank_disable_fn() drm_device::vbl_lock.
- skl_pfit_enable(), intel_update_plane(), vlv_atomic_update_fifo() and
  maybe others due to intel_uncore::lock
- drm_crtc_arm_vblank_event() due to drm_device::event_lock and
  drm_device::vblank_time_lock.

Don't disable interrupts on PREEMPT_RT during atomic updates.

[bigeasy: drop local locks, commit message]

Signed-off-by: Mike Galbraith <umgwanakikbuti@gmail.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/gpu/drm/i915/display/intel_crtc.c   | 9 ++++++---
 drivers/gpu/drm/i915/display/intel_vblank.c | 6 ++++--
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 25593f6aae7de..22b80004574fa 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -512,7 +512,8 @@ void intel_pipe_update_start(struct intel_atomic_state *state,
 	 */
 	intel_psr_wait_for_idle_locked(new_crtc_state);
 
-	local_irq_disable();
+	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+		local_irq_disable();
 
 	crtc->debug.min_vbl = evade.min;
 	crtc->debug.max_vbl = evade.max;
@@ -530,7 +531,8 @@ void intel_pipe_update_start(struct intel_atomic_state *state,
 	return;
 
 irq_disable:
-	local_irq_disable();
+	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+		local_irq_disable();
 }
 
 #if IS_ENABLED(CONFIG_DRM_I915_DEBUG_VBLANK_EVADE)
@@ -632,7 +634,8 @@ void intel_pipe_update_end(struct intel_atomic_state *state,
 	 */
 	intel_vrr_send_push(new_crtc_state);
 
-	local_irq_enable();
+	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+		local_irq_enable();
 
 	if (intel_vgpu_active(dev_priv))
 		goto out;
diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c b/drivers/gpu/drm/i915/display/intel_vblank.c
index 71d6071121460..d639b51a49195 100644
--- a/drivers/gpu/drm/i915/display/intel_vblank.c
+++ b/drivers/gpu/drm/i915/display/intel_vblank.c
@@ -700,11 +700,13 @@ int intel_vblank_evade(struct intel_vblank_evade_ctx *evade)
 			break;
 		}
 
-		local_irq_enable();
+		if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+			local_irq_enable();
 
 		timeout = schedule_timeout(timeout);
 
-		local_irq_disable();
+		if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+			local_irq_disable();
 	}
 
 	finish_wait(wq, &wait);
-- 
2.43.0

[PATCH 03/10] drm/i915: Don't check for atomic context on PREEMPT_RT

[Sebastian Andrzej Siewior]

The !in_atomic() check in _wait_for_atomic() triggers on PREEMPT_RT
because the uncore::lock is a spinlock_t and does not disable
preemption or interrupts.

Changing the uncore:lock to a raw_spinlock_t doubles the worst case
latency on an otherwise idle testbox during testing. Therefore I'm
currently unsure about changing this.

Link: https://lore.kernel.org/all/20211006164628.s2mtsdd2jdbfyf7g@linutronix.de/
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/gpu/drm/i915/i915_utils.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h
index b45ef0560611a..7c6077d2d3dc0 100644
--- a/drivers/gpu/drm/i915/i915_utils.h
+++ b/drivers/gpu/drm/i915/i915_utils.h
@@ -288,7 +288,7 @@ wait_remaining_ms_from_jiffies(unsigned long timestamp_jiffies, int to_wait_ms)
 #define wait_for(COND, MS)		_wait_for((COND), (MS) * 1000, 10, 1000)
 
 /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */
-#if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT)
+#if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT) && !defined(CONFIG_PREEMPT_RT)
 # define _WAIT_FOR_ATOMIC_CHECK(ATOMIC) WARN_ON_ONCE((ATOMIC) && !in_atomic())
 #else
 # define _WAIT_FOR_ATOMIC_CHECK(ATOMIC) do { } while (0)
-- 
2.43.0

[PATCH 04/10] drm/i915: Disable tracing points on PREEMPT_RT

[Sebastian Andrzej Siewior]

Luca Abeni reported this:
| BUG: scheduling while atomic: kworker/u8:2/15203/0x00000003
| CPU: 1 PID: 15203 Comm: kworker/u8:2 Not tainted 4.19.1-rt3 #10
| Call Trace:
|  rt_spin_lock+0x3f/0x50
|  gen6_read32+0x45/0x1d0 [i915]
|  g4x_get_vblank_counter+0x36/0x40 [i915]
|  trace_event_raw_event_i915_pipe_update_start+0x7d/0xf0 [i915]

The tracing events use trace_i915_pipe_update_start() among other events
use functions acquire spinlock_t locks which are transformed into
sleeping locks on PREEMPT_RT. A few trace points use
intel_get_crtc_scanline(), others use ->get_vblank_counter() wich also
might acquire a sleeping locks on PREEMPT_RT.
At the time the arguments are evaluated within trace point, preemption
is disabled and so the locks must not be acquired on PREEMPT_RT.

Based on this I don't see any other way than disable trace points on
PREMPT_RT.

Reported-by: Luca Abeni <lucabe72@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/gpu/drm/i915/i915_trace.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index ce1cbee1b39dd..c54653cf72c95 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -6,6 +6,10 @@
 #if !defined(_I915_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ)
 #define _I915_TRACE_H_
 
+#ifdef CONFIG_PREEMPT_RT
+#define NOTRACE
+#endif
+
 #include <linux/stringify.h>
 #include <linux/types.h>
 #include <linux/tracepoint.h>
-- 
2.43.0

[PATCH 05/10] drm/i915: skip DRM_I915_LOW_LEVEL_TRACEPOINTS with NOTRACE

[Sebastian Andrzej Siewior]

The order of the header files is important. If this header file is
included after tracepoint.h was included then the NOTRACE here becomes a
nop. Currently this happens for two .c files which use the tracepoitns
behind DRM_I915_LOW_LEVEL_TRACEPOINTS.

Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/gpu/drm/i915/i915_trace.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index c54653cf72c95..3c51620d011b1 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -326,7 +326,7 @@ DEFINE_EVENT(i915_request, i915_request_add,
 	     TP_ARGS(rq)
 );
 
-#if defined(CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS)
+#if defined(CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS) && !defined(NOTRACE)
 DEFINE_EVENT(i915_request, i915_request_guc_submit,
 	     TP_PROTO(struct i915_request *rq),
 	     TP_ARGS(rq)
-- 
2.43.0

Re: [PATCH 05/10] drm/i915: skip DRM_I915_LOW_LEVEL_TRACEPOINTS with NOTRACE

[Steven Rostedt]

On Fri,  5 Apr 2024 16:18:23 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> The order of the header files is important. If this header file is
> included after tracepoint.h was included then the NOTRACE here becomes a
> nop. Currently this happens for two .c files which use the tracepoitns
> behind DRM_I915_LOW_LEVEL_TRACEPOINTS.

The NOTRACE should not be included in the individual trace files.

Can you show where this is an issue. I think this is fixing the symptom
and not the bug itself.

-- Steve


> 
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  drivers/gpu/drm/i915/i915_trace.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index c54653cf72c95..3c51620d011b1 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -326,7 +326,7 @@ DEFINE_EVENT(i915_request, i915_request_add,
>  	     TP_ARGS(rq)
>  );
>  
> -#if defined(CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS)
> +#if defined(CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS) && !defined(NOTRACE)
>  DEFINE_EVENT(i915_request, i915_request_guc_submit,
>  	     TP_PROTO(struct i915_request *rq),
>  	     TP_ARGS(rq)

Re: [PATCH 05/10] drm/i915: skip DRM_I915_LOW_LEVEL_TRACEPOINTS with NOTRACE

[Sebastian Andrzej Siewior]

On 2024-04-08 13:06:50 [-0400], Steven Rostedt wrote:
> On Fri,  5 Apr 2024 16:18:23 +0200
> Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> 
> > The order of the header files is important. If this header file is
> > included after tracepoint.h was included then the NOTRACE here becomes a
> > nop. Currently this happens for two .c files which use the tracepoitns
> > behind DRM_I915_LOW_LEVEL_TRACEPOINTS.
> 
> The NOTRACE should not be included in the individual trace files.
> 
> Can you show where this is an issue. I think this is fixing the symptom
> and not the bug itself.

The previous patch in the series disables trace points. I just checked
the difference with and without this one and there is none. I still have
| # ls -1 /sys/kernel/debug/tracing/events/i915/
|  enable
|  filter
|  g4x_wm
|  intel_cpu_fifo_underrun
|  intel_crtc_vblank_work_end
|  intel_crtc_vblank_work_start
|  intel_fbc_activate
|  intel_fbc_deactivate
|  intel_fbc_nuke
|  intel_frontbuffer_flush
|  intel_frontbuffer_invalidate
|  intel_memory_cxsr
|  intel_pch_fifo_underrun
|  intel_pipe_crc
|  intel_pipe_disable
|  intel_pipe_enable
|  intel_pipe_update_end
|  intel_pipe_update_start
|  intel_pipe_update_vblank_evaded
|  intel_plane_disable_arm
|  intel_plane_update_arm
|  intel_plane_update_noarm
|  vlv_fifo_size
|  vlv_wm

and I *think* there were none. This then leads to
| BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
| in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 794, name: Xorg
| preempt_count: 2, expected: 0
| RCU nest depth: 0, expected: 0
| 3 locks held by Xorg/794:
|  #0: ffffa49201d73c50 (crtc_ww_class_acquire){+.+.}-{0:0}, at: drm_mode_cursor_common+0xdf/0x250 [drm]
|  #1: ffff89ef11c46080 (crtc_ww_class_mutex){+.+.}-{3:3}, at: modeset_lock+0x68/0x1d0 [drm]
|  #2: ffff89ef2224ac70 (&uncore->lock){+.+.}-{2:2}, at: fwtable_read32+0x4d/0x280 [i915]
| Preemption disabled at:
| [<0000000000000000>] 0x0
| CPU: 3 PID: 794 Comm: Xorg Tainted: G        W          6.9.0-rc3-rt1+ #15
| Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z68 Pro3-M, BIOS P2.10 04/24/2012
| Call Trace:
|  <TASK>
|  dump_stack_lvl+0x8d/0xb0
|  __might_resched+0x1a3/0x260
|  rt_spin_lock+0x48/0x100
|  fwtable_read32+0x4d/0x280 [i915]
|  trace_event_raw_event_intel_pipe_update_start+0xeb/0x1d0 [i915]
   ^^
|  intel_pipe_update_start+0x182/0x2f0 [i915]
|  intel_update_crtc+0x3f/0x400 [i915]
|  intel_commit_modeset_enables+0xab/0xd0 [i915]
|  intel_atomic_commit_tail+0x764/0x10b0 [i915]
|  intel_atomic_commit+0x318/0x360 [i915]
|  drm_atomic_commit+0x9e/0xd0 [drm]
|  drm_atomic_helper_disable_plane+0x87/0xe0 [drm_kms_helper]
|  drm_mode_cursor_universal+0x114/0x270 [drm]
|  drm_mode_cursor_common+0x11d/0x250 [drm]
|  drm_mode_cursor_ioctl+0x4b/0x70 [drm]
|  drm_ioctl_kernel+0xb4/0x110 [drm]
|  drm_ioctl+0x27b/0x4d0 [drm]

among a few other things I was not aware of.
So yes, this patch is not needed since it makes no difference but I still
have trace points I would rather not have.
If you a clue how to deal with this properly, I am all yours.

> -- Steve

Sebastian

Re: [PATCH 05/10] drm/i915: skip DRM_I915_LOW_LEVEL_TRACEPOINTS with NOTRACE

[Steven Rostedt]

On Tue, 9 Apr 2024 13:06:01 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> among a few other things I was not aware of.
> So yes, this patch is not needed since it makes no difference but I still
> have trace points I would rather not have.
> If you a clue how to deal with this properly, I am all yours.

I believe you need to do it in the .c file:

Can you try something like this?

diff --git a/drivers/gpu/drm/i915/i915_trace_points.c b/drivers/gpu/drm/i915/i915_trace_points.c
index 463a7177997c..ad0de6110133 100644
--- a/drivers/gpu/drm/i915/i915_trace_points.c
+++ b/drivers/gpu/drm/i915/i915_trace_points.c
@@ -8,7 +8,7 @@
 
 #include "i915_drv.h"
 
-#ifndef __CHECKER__
+#if !DEFINED(__CHECKER__) && !DEFINED(CONFIG_PREEMPT_RT)
 #define CREATE_TRACE_POINTS
 #include "i915_trace.h"
 #endif


Or is it just that one set of trace events that is an issue?

Could you just do:

#if defined(CONFIG_DRM_I915_LOW_LEVEL_TRACEPOINTS) && !defined(CONFIG_PREEMPT_RT)

 ?

-- Steve

Re: [PATCH 05/10] drm/i915: skip DRM_I915_LOW_LEVEL_TRACEPOINTS with NOTRACE

[Sebastian Andrzej Siewior]

On 2024-04-09 11:55:33 [-0400], Steven Rostedt wrote:
> I believe you need to do it in the .c file:
> 
> Can you try something like this?
> 
…
>  ?

I tried and nothing changed because the lowlevel config option isn't the
problem. What about I drop this and replace 4/10 from this series with
the patch below? After enabling all tracing I don't see any events in
events/i915.

Sebastian

--------------------->8----------------------------

From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Date: Thu, 6 Dec 2018 09:52:20 +0100
Subject: [PATCH] drm/i915: Disable tracing points on PREEMPT_RT

Luca Abeni reported this:
| BUG: scheduling while atomic: kworker/u8:2/15203/0x00000003
| CPU: 1 PID: 15203 Comm: kworker/u8:2 Not tainted 4.19.1-rt3 #10
| Call Trace:
|  rt_spin_lock+0x3f/0x50
|  gen6_read32+0x45/0x1d0 [i915]
|  g4x_get_vblank_counter+0x36/0x40 [i915]
|  trace_event_raw_event_i915_pipe_update_start+0x7d/0xf0 [i915]

The tracing events use trace_i915_pipe_update_start() among other events
use functions acquire spinlock_t locks which are transformed into
sleeping locks on PREEMPT_RT. A few trace points use
intel_get_crtc_scanline(), others use ->get_vblank_counter() wich also
might acquire a sleeping locks on PREEMPT_RT.
At the time the arguments are evaluated within trace point, preemption
is disabled and so the locks must not be acquired on PREEMPT_RT.

Based on this I don't see any other way than disable trace points on
PREMPT_RT.

Reported-by: Luca Abeni <lucabe72@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/gpu/drm/i915/display/intel_display_trace.h | 4 ++++
 drivers/gpu/drm/i915/i915_trace.h                  | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display_trace.h b/drivers/gpu/drm/i915/display/intel_display_trace.h
index 7862e7cefe027..e4608d855bfba 100644
--- a/drivers/gpu/drm/i915/display/intel_display_trace.h
+++ b/drivers/gpu/drm/i915/display/intel_display_trace.h
@@ -9,6 +9,10 @@
 #if !defined(__INTEL_DISPLAY_TRACE_H__) || defined(TRACE_HEADER_MULTI_READ)
 #define __INTEL_DISPLAY_TRACE_H__
 
+#ifdef CONFIG_PREEMPT_RT
+#define NOTRACE
+#endif
+
 #include <linux/string_helpers.h>
 #include <linux/types.h>
 #include <linux/tracepoint.h>
diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
index ce1cbee1b39dd..c54653cf72c95 100644
--- a/drivers/gpu/drm/i915/i915_trace.h
+++ b/drivers/gpu/drm/i915/i915_trace.h
@@ -6,6 +6,10 @@
 #if !defined(_I915_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ)
 #define _I915_TRACE_H_
 
+#ifdef CONFIG_PREEMPT_RT
+#define NOTRACE
+#endif
+
 #include <linux/stringify.h>
 #include <linux/types.h>
 #include <linux/tracepoint.h>
-- 
2.43.0

Re: [PATCH 05/10] drm/i915: skip DRM_I915_LOW_LEVEL_TRACEPOINTS with NOTRACE

[Steven Rostedt]

On Wed, 10 Apr 2024 13:08:57 +0200
Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> On 2024-04-09 11:55:33 [-0400], Steven Rostedt wrote:
> > I believe you need to do it in the .c file:
> > 
> > Can you try something like this?
> >   
> …
> >  ?  
> 
> I tried and nothing changed because the lowlevel config option isn't the
> problem. What about I drop this and replace 4/10 from this series with
> the patch below? After enabling all tracing I don't see any events in
> events/i915.

I don't see anything wrong with this approach.

> 
> Sebastian
> 
> --------------------->8----------------------------  
> 
> From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Date: Thu, 6 Dec 2018 09:52:20 +0100
> Subject: [PATCH] drm/i915: Disable tracing points on PREEMPT_RT
> 
> Luca Abeni reported this:
> | BUG: scheduling while atomic: kworker/u8:2/15203/0x00000003
> | CPU: 1 PID: 15203 Comm: kworker/u8:2 Not tainted 4.19.1-rt3 #10
> | Call Trace:
> |  rt_spin_lock+0x3f/0x50
> |  gen6_read32+0x45/0x1d0 [i915]
> |  g4x_get_vblank_counter+0x36/0x40 [i915]
> |  trace_event_raw_event_i915_pipe_update_start+0x7d/0xf0 [i915]
> 
> The tracing events use trace_i915_pipe_update_start() among other events
> use functions acquire spinlock_t locks which are transformed into
> sleeping locks on PREEMPT_RT. A few trace points use
> intel_get_crtc_scanline(), others use ->get_vblank_counter() wich also
> might acquire a sleeping locks on PREEMPT_RT.
> At the time the arguments are evaluated within trace point, preemption
> is disabled and so the locks must not be acquired on PREEMPT_RT.
> 
> Based on this I don't see any other way than disable trace points on
> PREMPT_RT.
> 
> Reported-by: Luca Abeni <lucabe72@gmail.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  drivers/gpu/drm/i915/display/intel_display_trace.h | 4 ++++
>  drivers/gpu/drm/i915/i915_trace.h                  | 4 ++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_trace.h b/drivers/gpu/drm/i915/display/intel_display_trace.h
> index 7862e7cefe027..e4608d855bfba 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_trace.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_trace.h
> @@ -9,6 +9,10 @@
>  #if !defined(__INTEL_DISPLAY_TRACE_H__) || defined(TRACE_HEADER_MULTI_READ)
>  #define __INTEL_DISPLAY_TRACE_H__
>  
> +#ifdef CONFIG_PREEMPT_RT

Hmm, should these be:

 #if defined(CONFIG_PREEMPT_RT) && !defined(NOTRACE)

?

because it's not protected due to the TRACE_HEADER_MULTI_READ.

-- Steve


> +#define NOTRACE
> +#endif
> +
>  #include <linux/string_helpers.h>
>  #include <linux/types.h>
>  #include <linux/tracepoint.h>
> diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h
> index ce1cbee1b39dd..c54653cf72c95 100644
> --- a/drivers/gpu/drm/i915/i915_trace.h
> +++ b/drivers/gpu/drm/i915/i915_trace.h
> @@ -6,6 +6,10 @@
>  #if !defined(_I915_TRACE_H_) || defined(TRACE_HEADER_MULTI_READ)
>  #define _I915_TRACE_H_
>  
> +#ifdef CONFIG_PREEMPT_RT
> +#define NOTRACE
> +#endif
> +
>  #include <linux/stringify.h>
>  #include <linux/types.h>
>  #include <linux/tracepoint.h>

[PATCH 06/10] drm/i915/gt: Queue and wait for the irq_work item.

[Sebastian Andrzej Siewior]

Disabling interrupts and invoking the irq_work function directly breaks
on PREEMPT_RT.
PREEMPT_RT does not invoke all irq_work from hardirq context because
some of the user have spinlock_t locking in the callback function.
These locks are then turned into a sleeping locks which can not be
acquired with disabled interrupts.

Using irq_work_queue() has the benefit that the irqwork will be invoked
in the regular context. In general there is "no" delay between enqueuing
the callback and its invocation because the interrupt is raised right
away on architectures which support it (which includes x86).

Use irq_work_queue() + irq_work_sync() instead invoking the callback
directly.

Reported-by: Clark Williams <williams@redhat.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
index d650beb8ed22f..3dd3e516b80c1 100644
--- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
@@ -317,10 +317,9 @@ void __intel_breadcrumbs_park(struct intel_breadcrumbs *b)
 	/* Kick the work once more to drain the signalers, and disarm the irq */
 	irq_work_sync(&b->irq_work);
 	while (READ_ONCE(b->irq_armed) && !atomic_read(&b->active)) {
-		local_irq_disable();
-		signal_irq_work(&b->irq_work);
-		local_irq_enable();
+		irq_work_queue(&b->irq_work);
 		cond_resched();
+		irq_work_sync(&b->irq_work);
 	}
 }
 
-- 
2.43.0

[PATCH 07/10] drm/i915/gt: Use spin_lock_irq() instead of local_irq_disable() + spin_lock()

[Sebastian Andrzej Siewior]

execlists_dequeue() is invoked from a function which uses
local_irq_disable() to disable interrupts so the spin_lock() behaves
like spin_lock_irq().
This breaks PREEMPT_RT because local_irq_disable() + spin_lock() is not
the same as spin_lock_irq().

execlists_dequeue_irq() and execlists_dequeue() has each one caller
only. If intel_engine_cs::active::lock is acquired and released with the
_irq suffix then it behaves almost as if execlists_dequeue() would be
invoked with disabled interrupts. The difference is the last part of the
function which is then invoked with enabled interrupts.
I can't tell if this makes a difference. From looking at it, it might
work to move the last unlock at the end of the function as I didn't find
anything that would acquire the lock again.

Reported-by: Clark Williams <williams@redhat.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 .../drm/i915/gt/intel_execlists_submission.c    | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
index b061a0a0d6b08..80240ec45cb9f 100644
--- a/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
+++ b/drivers/gpu/drm/i915/gt/intel_execlists_submission.c
@@ -1303,7 +1303,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 	 * and context switches) submission.
 	 */
 
-	spin_lock(&sched_engine->lock);
+	spin_lock_irq(&sched_engine->lock);
 
 	/*
 	 * If the queue is higher priority than the last
@@ -1403,7 +1403,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 				 * Even if ELSP[1] is occupied and not worthy
 				 * of timeslices, our queue might be.
 				 */
-				spin_unlock(&sched_engine->lock);
+				spin_unlock_irq(&sched_engine->lock);
 				return;
 			}
 		}
@@ -1429,7 +1429,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 
 		if (last && !can_merge_rq(last, rq)) {
 			spin_unlock(&ve->base.sched_engine->lock);
-			spin_unlock(&engine->sched_engine->lock);
+			spin_unlock_irq(&engine->sched_engine->lock);
 			return; /* leave this for another sibling */
 		}
 
@@ -1591,7 +1591,7 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 	 */
 	sched_engine->queue_priority_hint = queue_prio(sched_engine);
 	i915_sched_engine_reset_on_empty(sched_engine);
-	spin_unlock(&sched_engine->lock);
+	spin_unlock_irq(&sched_engine->lock);
 
 	/*
 	 * We can skip poking the HW if we ended up with exactly the same set
@@ -1617,13 +1617,6 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
 	}
 }
 
-static void execlists_dequeue_irq(struct intel_engine_cs *engine)
-{
-	local_irq_disable(); /* Suspend interrupts across request submission */
-	execlists_dequeue(engine);
-	local_irq_enable(); /* flush irq_work (e.g. breadcrumb enabling) */
-}
-
 static void clear_ports(struct i915_request **ports, int count)
 {
 	memset_p((void **)ports, NULL, count);
@@ -2478,7 +2471,7 @@ static void execlists_submission_tasklet(struct tasklet_struct *t)
 	}
 
 	if (!engine->execlists.pending[0]) {
-		execlists_dequeue_irq(engine);
+		execlists_dequeue(engine);
 		start_timeslice(engine);
 	}
 
-- 
2.43.0

[PATCH 08/10] drm/i915: Drop the irqs_disabled() check

[Sebastian Andrzej Siewior]

The !irqs_disabled() check triggers on PREEMPT_RT even with
i915_sched_engine::lock acquired. The reason is the lock is transformed
into a sleeping lock on PREEMPT_RT and does not disable interrupts.

There is no need to check for disabled interrupts. The lockdep
annotation below already check if the lock has been acquired by the
caller and will yell if the interrupts are not disabled.

Remove the !irqs_disabled() check.

Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/gpu/drm/i915/i915_request.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 519e096c607cd..466b5ee8ed6d2 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -608,7 +608,6 @@ bool __i915_request_submit(struct i915_request *request)
 
 	RQ_TRACE(request, "\n");
 
-	GEM_BUG_ON(!irqs_disabled());
 	lockdep_assert_held(&engine->sched_engine->lock);
 
 	/*
@@ -717,7 +716,6 @@ void __i915_request_unsubmit(struct i915_request *request)
 	 */
 	RQ_TRACE(request, "\n");
 
-	GEM_BUG_ON(!irqs_disabled());
 	lockdep_assert_held(&engine->sched_engine->lock);
 
 	/*
-- 
2.43.0

[PATCH 09/10] drm/i915/guc: Consider also RCU depth in busy loop.

[Sebastian Andrzej Siewior]

intel_guc_send_busy_loop() looks at in_atomic() and irqs_disabled() to
decide if it should busy-spin while waiting or if it may sleep.
Both checks will report false on PREEMPT_RT if sleeping spinlocks are
acquired leading to RCU splats while the function sleeps.

Check also if RCU has been disabled.

Reported-by: "John B. Wyatt IV" <jwyatt@redhat.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
index be70c46604b49..387cc1911302f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
@@ -360,7 +360,7 @@ static inline int intel_guc_send_busy_loop(struct intel_guc *guc,
 {
 	int err;
 	unsigned int sleep_period_ms = 1;
-	bool not_atomic = !in_atomic() && !irqs_disabled();
+	bool not_atomic = !in_atomic() && !irqs_disabled() && !rcu_preempt_depth();
 
 	/*
 	 * FIXME: Have caller pass in if we are in an atomic context to avoid
-- 
2.43.0

Re: [PATCH 09/10] drm/i915/guc: Consider also RCU depth in busy loop.

[Rodrigo Vivi]

On Fri, Apr 05, 2024 at 04:18:27PM +0200, Sebastian Andrzej Siewior wrote:
> intel_guc_send_busy_loop() looks at in_atomic() and irqs_disabled() to
> decide if it should busy-spin while waiting or if it may sleep.
> Both checks will report false on PREEMPT_RT if sleeping spinlocks are
> acquired leading to RCU splats while the function sleeps.
> 
> Check also if RCU has been disabled.
> 
> Reported-by: "John B. Wyatt IV" <jwyatt@redhat.com>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> index be70c46604b49..387cc1911302f 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h
> @@ -360,7 +360,7 @@ static inline int intel_guc_send_busy_loop(struct intel_guc *guc,
>  {
>  	int err;
>  	unsigned int sleep_period_ms = 1;
> -	bool not_atomic = !in_atomic() && !irqs_disabled();
> +	bool not_atomic = !in_atomic() && !irqs_disabled() && !rcu_preempt_depth();
>  
>  	/*
>  	 * FIXME: Have caller pass in if we are in an atomic context to avoid
> -- 
> 2.43.0
> 

[PATCH 10/10] Revert "drm/i915: Depend on !PREEMPT_RT."

[Sebastian Andrzej Siewior]

Once the known issues are addressed, it should be safe to enable the
driver.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/gpu/drm/i915/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 5932024f8f954..a02162d6b710e 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -3,7 +3,6 @@ config DRM_I915
 	tristate "Intel 8xx/9xx/G3x/G4x/HD Graphics"
 	depends on DRM
 	depends on X86 && PCI
-	depends on !PREEMPT_RT
 	select INTEL_GTT if X86
 	select INTERVAL_TREE
 	# we need shmfs for the swappable backing store, and in particular
-- 
2.43.0

✗ CI.Patch_applied: failure for drm/i915: PREEMPT_RT related fixups.

[Patchwork]

== Series Details ==

Series: drm/i915: PREEMPT_RT related fixups.
URL   : https://patchwork.freedesktop.org/series/132082/
State : failure

== Summary ==

=== Applying kernel patches on branch 'drm-tip' with base: ===
Base commit: 8cb1ee178ef7 drm-tip: 2024y-04m-05d-16h-52m-43s UTC integration manifest
=== git am output follows ===
error: patch failed: drivers/gpu/drm/i915/Kconfig:3
error: drivers/gpu/drm/i915/Kconfig: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Applying: drm/i915: Use preempt_disable/enable_rt() where recommended
Applying: drm/i915: Don't disable interrupts on PREEMPT_RT during atomic updates
Applying: drm/i915: Don't check for atomic context on PREEMPT_RT
Applying: drm/i915: Disable tracing points on PREEMPT_RT
Applying: drm/i915: skip DRM_I915_LOW_LEVEL_TRACEPOINTS with NOTRACE
Applying: drm/i915/gt: Queue and wait for the irq_work item.
Applying: drm/i915/gt: Use spin_lock_irq() instead of local_irq_disable() + spin_lock()
Applying: drm/i915: Drop the irqs_disabled() check
Applying: drm/i915/guc: Consider also RCU depth in busy loop.
Applying: Revert "drm/i915: Depend on !PREEMPT_RT."
Patch failed at 0010 Revert "drm/i915: Depend on !PREEMPT_RT."
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

✗ CI.Patch_applied: failure for drm/i915: PREEMPT_RT related fixups. (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: PREEMPT_RT related fixups. (rev2)
URL   : https://patchwork.freedesktop.org/series/132082/
State : failure

== Summary ==

=== Applying kernel patches on branch 'drm-tip' with base: ===
Base commit: 284b05a047b8 drm-tip: 2024y-04m-10d-11h-35m-06s UTC integration manifest
=== git am output follows ===
error: patch failed: drivers/gpu/drm/i915/i915_trace.h:6
error: drivers/gpu/drm/i915/i915_trace.h: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Applying: drm/i915: Use preempt_disable/enable_rt() where recommended
Applying: drm/i915: Don't disable interrupts on PREEMPT_RT during atomic updates
Applying: drm/i915: Don't check for atomic context on PREEMPT_RT
Applying: drm/i915: Disable tracing points on PREEMPT_RT
Applying: drm/i915: skip DRM_I915_LOW_LEVEL_TRACEPOINTS with NOTRACE
Patch failed at 0005 drm/i915: skip DRM_I915_LOW_LEVEL_TRACEPOINTS with NOTRACE
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Re: [PATCH 00/10] drm/i915: PREEMPT_RT related fixups.

[Sebastian Andrzej Siewior]

On 2024-04-05 16:18:18 [+0200], To intel-gfx@lists.freedesktop.org wrote:
Hi,

> The following patches are from the PREEMPT_RT queue.  It is mostly about
> disabling interrupts/preemption which leads to problems. Unfortunately
…

Could I please get some feedback? I didn't receive anything but
automated mails from bots and I can't tell if this is a problem or not.

As of -rc2 I noticed that I can drop
	[PATCH 06/10] drm/i915/gt: Queue and wait for the irq_work item.

from the series.

Sebastian

Re: [PATCH 00/10] drm/i915: PREEMPT_RT related fixups.

[Tvrtko Ursulin]

Hi Sebastian,

On 05/06/2024 11:01, Sebastian Andrzej Siewior wrote:
> On 2024-04-05 16:18:18 [+0200], To intel-gfx@lists.freedesktop.org wrote:
> Hi,
> 
>> The following patches are from the PREEMPT_RT queue.  It is mostly about
>> disabling interrupts/preemption which leads to problems. Unfortunately
> …
> 
> Could I please get some feedback? I didn't receive anything but
> automated mails from bots and I can't tell if this is a problem or not.
> 
> As of -rc2 I noticed that I can drop
> 	[PATCH 06/10] drm/i915/gt: Queue and wait for the irq_work item.
> 
> from the series.

Previous CI results have unfortunately expired by now. I have tried 
re-queuing it but it also does not apply any longer so I'm afraid you 
will have to respin before anyone can see the results.

And this is not to say that I can promise someone will look at it or 
when. Maybe Jani you could ask for volunteers regarding the display 
related patches (head of the series) and Rodrigo you about the GuC 
change in 9/10?

Regards,

Tvrtko

Re: [PATCH 00/10] drm/i915: PREEMPT_RT related fixups.

[Sebastian Andrzej Siewior]

On 2024-06-11 14:06:00 [+0100], Tvrtko Ursulin wrote:
> 
> Hi Sebastian,
Hi Tvrtko,

> Previous CI results have unfortunately expired by now. I have tried
> re-queuing it but it also does not apply any longer so I'm afraid you will
> have to respin before anyone can see the results.

No worries, I rebased it for latest RT release, I can respost them.

> And this is not to say that I can promise someone will look at it or when.
> Maybe Jani you could ask for volunteers regarding the display related
> patches (head of the series) and Rodrigo you about the GuC change in 9/10?

9/10 looks easy but okay. I'm going to Cc Jani for series and hope for
the best.
2/10 is needed for the XE driver since it shares code with i915. It
seems fine otherwise. I can post them one-by-one if this makes it easier
for CI. Patches like 2/10 or 8/10 should make no impact on CI.

Thank you for the sign of life ;)

> Regards,
> 
> Tvrtko

Sebastian

Re: [PATCH 00/10] drm/i915: PREEMPT_RT related fixups.

[Rodrigo Vivi]

On Tue, Jun 11, 2024 at 05:14:26PM +0200, Sebastian Andrzej Siewior wrote:
> On 2024-06-11 14:06:00 [+0100], Tvrtko Ursulin wrote:
> > 
> > Hi Sebastian,
> Hi Tvrtko,
> 
> > Previous CI results have unfortunately expired by now. I have tried
> > re-queuing it but it also does not apply any longer so I'm afraid you will
> > have to respin before anyone can see the results.
> 
> No worries, I rebased it for latest RT release, I can respost them.
> 
> > And this is not to say that I can promise someone will look at it or when.
> > Maybe Jani you could ask for volunteers regarding the display related
> > patches (head of the series) and Rodrigo you about the GuC change in 9/10?
> 
> 9/10 looks easy but okay.

That is indeed the obvious place.

> I'm going to Cc Jani for series and hope for
> the best.
> 2/10 is needed for the XE driver since it shares code with i915. 

I believe you meant patch 1 right?
We are trying to eliminate the
#if I915
for the shared display code with Xe... we probably need to think
more deeply about that.

> It
> seems fine otherwise. I can post them one-by-one if this makes it easier
> for CI. Patches like 2/10 or 8/10 should make no impact on CI.
> 
> Thank you for the sign of life ;)
> 
> > Regards,
> > 
> > Tvrtko
> 
> Sebastian

Re: [PATCH 00/10] drm/i915: PREEMPT_RT related fixups.

[Sebastian Andrzej Siewior]

On 2024-06-11 18:25:07 [-0400], Rodrigo Vivi wrote:
> > 2/10 is needed for the XE driver since it shares code with i915. 
> 
> I believe you meant patch 1 right?
> We are trying to eliminate the
> #if I915
> for the shared display code with Xe... we probably need to think
> more deeply about that.

Maybe, my notes say otherwise but I could have slipped in the patch
number while looking up… I remember I noticed that the uncore lock isn't
used, it was only used on my "older" hardware.
Anyway, I could double check if this becomes urgent for some reason.

Sebastian