On 2025-11-05 21:42:53 [+0100], Maarten Lankhorst wrote:
> Hey,
Hi,
sorry for the long break. I somehow marked this as read by accident.
Today I just checked if there was some progress and noticed that there
was indeed a reply…
> Den 2025-11-05 kl. 14:47, skrev Sebastian Andrzej Siewior:
> > i915.
> > - drm/i915/display: Move vblank put until after critical section
> > - intel_vblank_evade() is invoked with irq-off
> > - within its callchain intel_vblank_section_enter() does spin_lock()
> > in I915. XE does nothing so it is fine.
> > - intel_crtc_scanlines_since_frame_timestamp() does a while loop
> > What is the expected/ possible worst case here and when does it happen?
> The typical case is no vblank happened between those reads, and returns
> without looping.
> The worst case is a vblank happening in the hardware during that loop. If
> that happens, it's retried once.
> PIPE_FRMTMSTMP triggers after every completed frame update.
> You have to complete the loop exactly once every 16.7 ms at 60 fps to loop
> forever at exactly the wrong time and then keep hitting it.
> It looks like only a specific output on a few specific platforms are using it.
I see. I am just worried that this loops sometimes too often or at the
wrong time. But you are saying that this does not happen?
> > While at it, I noticed:
> > local_irq_disable();
> > if (new_plane_state->uapi.visible) {
> > intel_plane_update_noarm(NULL, plane, crtc_state,
> > new_plane_state);
> > if (plane->fbc)
> > intel_fbc_dirty_rect_update_noarm(dsb, plane);
> > if (!HAS_FBC_DIRTY_RECT(display))
> > return;
> > mutex_lock(&fbc->lock); <----
> >
> Eek, how did that slip through CI? There should be no lock whatsoever
> in there since that mutex would blow up on !RT too...
yeah ;) Well I can't tell if this is magically somehow excluded but
based on review in looks possible.
> > Haven't checked the callbacks but it feels like a lot of code with
> > disabled interrupts.
> >
> > - The GEM_BUG_ON(!irqs_disabled) in __i915_request_submit()/
> > __i915_request_unsubmit() case bugs/ warnings:
> > | WARNING: CPU: 3 PID: 2115 at drivers/gpu/drm/i915/i915_request.c:611
> > __i915_request_submit+0x1db/0x1f0 [i915]
> > | CPU: 3 UID: 0 PID: 2115 Comm: modprobe Not tainted 6.18.0-rc1+ #17
> > PREEMPT_{RT,(lazy)}
> > | Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z68 Pro3-M,
> > BIOS P2.30 06/29/2012
> > | RIP: 0010:__i915_request_submit+0x1db/0x1f0 [i915]
> > | Call Trace:
> > | <TASK>
> > | i915_request_submit+0x29/0x40 [i915]
> > | i9xx_submit_request+0xe/0x70 [i915]
> > | submit_notify+0xc1/0x230 [i915]
> > | __i915_sw_fence_complete+0x88/0x290 [i915]
> > | __engine_park+0x2d2/0x410 [i915]
> > | ____intel_wakeref_put_last+0x25/0x90 [i915]
> > | intel_gt_resume.part.0+0x2ec/0x380 [i915]
> > | intel_gt_init+0x14d/0x3d0 [i915]
> > | i915_gem_init+0x14b/0x290 [i915]
> > | i915_driver_probe+0x74a/0xc10 [i915]
> > | i915_pci_probe+0xd7/0x190 [i915]
> > | local_pci_probe+0x41/0x80
> I didn't apply the i915 specific patches that I saw in the -rt patchset, I
> only tested the xe display.
> For i915, I did -DNOTRACE to the entire subdirectory to disable tracing on
> PREEMPT_RT,
> but the other non-display related patches are still needed, did you apply
> those?
Nope, I did just test what you provided. Your cover letter suggested
that it might be enough for i915 or I misunderstood.
I don't remember seeing any events for i915. So this -DNOTRACE must be
part of the existing patches somewhere…
> - 0003-drm-i915-Don-t-check-for-atomic-context-on-PREEMPT_R.patch
> (0004 can be droped since upstream commit ed1fbee3debb ("drm/i915: Disable
> tracepoints for PREEMPT_RT"))
> - 0005-drm-i915-gt-Use-spin_lock_irq-instead-of-local_irq_d.patch
> - 0006-drm-i915-Drop-the-irqs_disabled-check.patch
> - 0007-drm-i915-guc-Consider-also-RCU-depth-in-busy-loop.patch
>
> > - The change of irq-enable is not enough:
> > | BUG: sleeping function called from invalid context at
> > kernel/locking/spinlock_rt.c:48
> > | in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 2115, name:
> > modprobe
> > | preempt_count: 0, expected: 0
> > | RCU nest depth: 0, expected: 0
> > | 4 locks held by modprobe/2115:
> > | #0: ffff99b9425161a0 (&dev->mutex){....}-{4:4}, at:
> > __driver_attach+0xaf/0x1c0
> > | #1: ffffaa224810f6c0 (crtc_ww_class_acquire){+.+.}-{0:0}, at:
> > intel_initial_commit+0x4c/0x200 [i915]
> > | #2: ffffaa224810f6e8 (crtc_ww_class_mutex){+.+.}-{4:4}, at:
> > intel_initial_commit+0x4c/0x200 [i915]
> > | #3: ffff99b94a6c9030 (&uncore->lock){+.+.}-{3:3}, at:
> > gen6_write32+0x50/0x290 [i915]
> > | irq event stamp: 513344
> > | hardirqs last enabled at (513343): [<ffffffff8ba8d84c>]
> > _raw_spin_unlock_irqrestore+0x4c/0x60
> > | hardirqs last disabled at (513344): [<ffffffffc1543646>]
> > intel_pipe_update_start+0x216/0x2c0 [i915]
> > | softirqs last enabled at (512766): [<ffffffff8af045cf>]
> > __local_bh_enable_ip+0x10f/0x170
> > | softirqs last disabled at (512712): [<ffffffffc14dfb6a>]
> > __i915_request_queue+0x3a/0x70 [i915]
> > | CPU: 3 UID: 0 PID: 2115 Comm: modprobe Tainted: G W
> > 6.18.0-rc1+ #17 PREEMPT_{RT,(lazy)}
> > | Tainted: [W]=WARN
> > | Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z68 Pro3-M,
> > BIOS P2.30 06/29/2012
> > | Call Trace:
> > | <TASK>
> > | dump_stack_lvl+0x68/0x90
> > | __might_resched.cold+0xf0/0x12b
> > | rt_spin_lock+0x5f/0x200
> > | gen6_write32+0x50/0x290 [i915]
> > | ilk_set_pipeconf+0x12d/0x230 [i915]
> > | ilk_color_commit_arm+0x2d/0x70 [i915]
> > | intel_update_crtc+0x15b/0x690 [i915]
> > | intel_commit_modeset_enables+0xa6/0xd0 [i915]
> > | intel_atomic_commit_tail+0xd55/0x19a0 [i915]
> > | intel_atomic_commit+0x25d/0x2a0 [i915]
> > | drm_atomic_commit+0xad/0xe0 [drm]
> > | intel_initial_commit+0x16c/0x200 [i915]
> > | intel_display_driver_probe+0x2e/0x80 [i915]
> > | i915_driver_probe+0x791/0xc10 [i915]
> > | i915_pci_probe+0xd7/0x190 [i915]
> >
> > This is the intel_pipe_update_start()/ intel_pipe_update_end() part in
> > intel_update_crtc().
>
> From the log it seems ilk_set_pipeconf() needs an update too.
> Can you check if the warnings there go away when you replace
> intel_de_write() with intel_de_write_fw() and the
> intel_de_posting_read() with intel_de_read_fw() in ilk_set_pipeconf()
> and the listed i915 patches from PREEMPT_RT series applied?
I added the following patches
- 0005-drm-i915-gt-Use-spin_lock_irq-instead-of-local_irq_d.patch
- 0006-drm-i915-Drop-the-irqs_disabled-check.patch
- 0007-drm-i915-guc-Consider-also-RCU-depth-in-busy-loop.patch
- drm-i915-Consider-RCU-read-section-as-atomic.patch
from
https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git/tree/patches?h=v6.18-rc4-rt3-patches
plus the diff below (which is mostly
0002-drm-i915-Don-t-disable-interrupts-on-PREEMPT_RT-duri.patch that
clashes now with one of the earlier patches). This looks without any
errors/ warnings.
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index ac433023636c3..3d4537c7ce672 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -3169,8 +3169,8 @@ void ilk_set_pipeconf(const struct intel_crtc_state
*crtc_state)
val |= TRANSCONF_FRAME_START_DELAY(crtc_state->framestart_delay - 1);
val |= TRANSCONF_MSA_TIMING_DELAY(crtc_state->msa_timing_delay);
- intel_de_write(display, TRANSCONF(display, cpu_transcoder), val);
- intel_de_posting_read(display, TRANSCONF(display, cpu_transcoder));
+ intel_de_write_fw(display, TRANSCONF(display, cpu_transcoder), val);
+ intel_de_read_fw(display, TRANSCONF(display, cpu_transcoder));
}
static void hsw_set_transconf(const struct intel_crtc_state *crtc_state)
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 5e939004b6463..40a9234e6e5dc 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
diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c
b/drivers/gpu/drm/i915/display/intel_crtc.c
index ef429b81a5228..d69b3763380c3 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -570,7 +570,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;
@@ -588,7 +589,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)
diff --git a/drivers/gpu/drm/i915/display/intel_cursor.c
b/drivers/gpu/drm/i915/display/intel_cursor.c
index 6b3bc8d94e51a..d9f1e7e72f500 100644
--- a/drivers/gpu/drm/i915/display/intel_cursor.c
+++ b/drivers/gpu/drm/i915/display/intel_cursor.c
@@ -922,11 +922,13 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
*/
intel_psr_wait_for_idle_locked(crtc_state);
- local_irq_disable();
+ if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+ local_irq_disable();
intel_vblank_evade(&evade);
} else {
- local_irq_disable();
+ if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+ local_irq_disable();
}
if (new_plane_state->uapi.visible) {
@@ -936,7 +938,8 @@ intel_legacy_cursor_update(struct drm_plane *_plane,
intel_plane_disable_arm(NULL, plane, crtc_state);
}
- local_irq_enable();
+ if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+ local_irq_enable();
intel_psr_unlock(crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c
b/drivers/gpu/drm/i915/display/intel_vblank.c
index a7f9195215f30..ad62e32da06a3 100644
--- a/drivers/gpu/drm/i915/display/intel_vblank.c
+++ b/drivers/gpu/drm/i915/display/intel_vblank.c
@@ -750,7 +750,8 @@ int intel_vblank_evade(struct intel_vblank_evade_ctx *evade)
return 0;
while (!vblank_evadable(evade, &scanline)) {
- local_irq_enable();
+ if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+ local_irq_enable();
DEFINE_WAIT(wait);
while (!vblank_evadable(evade, &scanline) && timeout > 0) {
@@ -759,7 +760,8 @@ int intel_vblank_evade(struct intel_vblank_evade_ctx *evade)
}
finish_wait(wq, &wait);
- local_irq_disable();
+ if (!IS_ENABLED(CONFIG_PREEMPT_RT))
+ local_irq_disable();
if (!timeout) {
drm_dbg_kms(display->drm,
> Kind regards,
> ~Maarten Lankhorst
Sebastian
