From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id EBA7BF30938 for ; Thu, 5 Mar 2026 10:50:28 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4BAEA10E271; Thu, 5 Mar 2026 10:50:28 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; secure) header.d=linutronix.de header.i=@linutronix.de header.b="Q2pS+r/2"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="TJjXlSOL"; dkim-atps=neutral Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8DDF810E268; Thu, 5 Mar 2026 10:50:26 +0000 (UTC) Date: Thu, 5 Mar 2026 11:50:22 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1772707824; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=DDg2UJxc5VzW285Ko+uegoDs9MrRKsQNSmmAYtPOK+w=; b=Q2pS+r/2sEEe3CoJvx/JfmVY358EduRiVGvn/GrTUeRBQD5y0xcQdvm7/o2xqtseeWGS2Y t0o0gOBx71BjV6wRbhc8hS2dUsQYprqHKiAOpvO0RfYwMGEu4cxSPpATlHYJhBeklMQrI/ NOeug4eihECN2xeLWtNsAqLbOMjwqub5DAWMrDNeAVKWszhZaS/2Cy38ykvtcJRsPQNUNe t03jbCW/MFG6WzHLvAQ1i/kXCpdZRoS7ayVMeOpyml4jo/SAEz+np7pxOBeKsceBss7ash /QfixUeXapqoIwlhHiKvHnuND1qoAq+HvZl9D4t9P1jxKnpvQg7CHn4vjf0E8A== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1772707824; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=DDg2UJxc5VzW285Ko+uegoDs9MrRKsQNSmmAYtPOK+w=; b=TJjXlSOLyv8TRQNb/oxgBcrEV+zKx6/XnnhmOydF5eKI4aRbW/A5i3MceYcgeGyVJ4odIL lLwY9gh9EBoCMFAw== From: Sebastian Andrzej Siewior To: Maarten Lankhorst Cc: intel-gfx@lists.freedesktop.org, intel-xe@lists.freedesktop.org, linux-rt-devel@lists.linux.dev, dri-devel@lists.freedesktop.org Subject: Re: [i915-rt v6 00/24] drm/i915/display: All patches to make PREEMPT_RT work on i915 + xe. Message-ID: <20260305105022.cc1qAMoO@linutronix.de> References: <20260220083657.28815-26-dev@lankhorst.se> <20260224162703.Q_taYjEC@linutronix.de> <20260224165903.2mn24oQy@linutronix.de> <20260225075806.XNtXjZ5E@linutronix.de> <7ee8b784-a714-4329-aab2-a571f25c3baa@lankhorst.se> <20260226120715.6Ug1Qkse@linutronix.de> <20260226141942.Z6vUrEQ3@linutronix.de> <20260226143857.4ZJAFzf6@linutronix.de> <98af7aba-f86f-4ff0-a53b-60e0e9784e37@lankhorst.se> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <98af7aba-f86f-4ff0-a53b-60e0e9784e37@lankhorst.se> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 2026-03-05 11:42:19 [+0100], Maarten Lankhorst wrote: > Hey, Hi, > Den 2026-02-26 kl. 15:38, skrev Sebastian Andrzej Siewior: > > On 2026-02-26 15:19:42 [+0100], To Maarten Lankhorst wrote: > >> On 2026-02-26 13:07:18 [+0100], To Maarten Lankhorst wrote: > >>> series somewhere I could pull and check. In meantime I would look what > >>> causes the lockup on i915. > >> > >> I think I got it. > > > > This the atomic sync as-is, IRQ-Work (FIFO-1) will be preempted by the > > threaded-interrupt (FIFO-50) and the interrupt will poll on > > signaler_active while the irq-work can't make progress. > > > > This will provide the needed sync: > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > > index a2b413982ce64..337f6e88faf05 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > > +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c > > @@ -209,6 +209,7 @@ static void signal_irq_work(struct irq_work *work) > > intel_breadcrumbs_disarm_irq(b); > > > > rcu_read_lock(); > > + spin_lock(&b->signaler_active_sync); > > atomic_inc(&b->signaler_active); > > list_for_each_entry_rcu(ce, &b->signalers, signal_link) { > > struct i915_request *rq; > > @@ -246,6 +247,7 @@ static void signal_irq_work(struct irq_work *work) > > } > > } > > atomic_dec(&b->signaler_active); > > + spin_unlock(&b->signaler_active_sync); > > rcu_read_unlock(); > > > > llist_for_each_safe(signal, sn, signal) { > > @@ -290,6 +292,7 @@ intel_breadcrumbs_create(struct intel_engine_cs *irq_engine) > > init_llist_head(&b->signaled_requests); > > > > spin_lock_init(&b->irq_lock); > > + spin_lock_init(&b->signaler_active_sync); > > init_irq_work(&b->irq_work, signal_irq_work); > > > > b->irq_engine = irq_engine; > > @@ -487,8 +490,11 @@ void intel_context_remove_breadcrumbs(struct intel_context *ce, > > if (release) > > intel_context_put(ce); > > > > - while (atomic_read(&b->signaler_active)) > > + while (atomic_read(&b->signaler_active)) { > > + spin_lock(&b->signaler_active_sync); > > + spin_unlock(&b->signaler_active_sync); > > cpu_relax(); > > + } > > } > > > > static void print_signals(struct intel_breadcrumbs *b, struct drm_printer *p) > > diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs_types.h b/drivers/gpu/drm/i915/gt/intel_breadcrumbs_types.h > > index bdf09fd67b6e7..28dae32628aab 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs_types.h > > +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs_types.h > > @@ -40,6 +40,7 @@ struct intel_breadcrumbs { > > struct list_head signalers; > > struct llist_head signaled_requests; > > atomic_t signaler_active; > > + spinlock_t signaler_active_sync; > > > > spinlock_t irq_lock; /* protects the interrupt from hardirq context */ > > struct irq_work irq_work; /* for use from inside irq_lock */ > > > > Thinking some more, replacing signaler_active with signaler_active_sync might be the best fix. > I'm not sure there's much use for parallel completion of the same breadcrumb, and using completion > might be too heavy handed. We have something similar in timer, tasklet and other "similar" code where on RT you have preemption and therefore the possibility of another user on the same CPU while on !RT it is only possible on a remote CPU. Using the spinlock_t for synchronisation would restrict to one-on-one. The closest API that comes to mind would be a sequence lock. One writer, multiple reader. So that would be an option that you might like ;) If the pure spinlock_t is off the table. > Kind regards, > ~Maarten Lankhorst Sebastian