From patchwork Tue Jan 23 07:25:38 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Airlie X-Patchwork-Id: 13526926 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 76B38C47258 for ; Tue, 23 Jan 2024 07:25:53 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 3576310E14D; Tue, 23 Jan 2024 07:25:49 +0000 (UTC) Received: from us-smtp-delivery-44.mimecast.com (us-smtp-delivery-44.mimecast.com [207.211.30.44]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3435A10E41B for ; Tue, 23 Jan 2024 07:25:47 +0000 (UTC) Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-235-ZbVuWzwbMlWUt0MHVum_PQ-1; Tue, 23 Jan 2024 02:25:41 -0500 X-MC-Unique: ZbVuWzwbMlWUt0MHVum_PQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.rdu2.redhat.com [10.11.54.8]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 6049510135ED; Tue, 23 Jan 2024 07:25:41 +0000 (UTC) Received: from dreadlord.redhat.com (unknown [10.64.136.44]) by smtp.corp.redhat.com (Postfix) with ESMTP id 58402C0FDCA; Tue, 23 Jan 2024 07:25:40 +0000 (UTC) From: Dave Airlie To: dri-devel@lists.freedesktop.org Subject: [PATCH] nouveau: rip out fence irq allow/block sequences. Date: Tue, 23 Jan 2024 17:25:38 +1000 Message-ID: <20240123072538.1290035-1-airlied@gmail.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.8 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: gmail.com 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: , Cc: nouveau@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: Dave Airlie fences are signalled on nvidia hw using non-stall interrupts. non-stall interrupts are not latched from my reading. When nouveau emits a fence, it requests a NON_STALL signalling, but it only calls the interface to allow the non-stall irq to happen after it has already emitted the fence. A recent change eacabb546271 ("nouveau: push event block/allowing out of the fence context") made this worse by pushing out the fence allow/block to a workqueue. However I can't see how this could ever work great, since when enable signalling is called, the semaphore has already been emitted to the ring, and the hw could already have tried to set the bits, but it's been masked off. Changing the allowed mask later won't make the interrupt get called again. For now rip all of this out. This fixes a bunch of stalls seen running VK CTS sync tests. Signed-off-by: Dave Airlie --- drivers/gpu/drm/nouveau/nouveau_fence.c | 77 +++++-------------------- drivers/gpu/drm/nouveau/nouveau_fence.h | 2 - 2 files changed, 16 insertions(+), 63 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c index 5057d976fa57..d6d50cdccf75 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fence.c +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c @@ -50,24 +50,14 @@ nouveau_fctx(struct nouveau_fence *fence) return container_of(fence->base.lock, struct nouveau_fence_chan, lock); } -static int +static void nouveau_fence_signal(struct nouveau_fence *fence) { - int drop = 0; - dma_fence_signal_locked(&fence->base); list_del(&fence->head); rcu_assign_pointer(fence->channel, NULL); - if (test_bit(DMA_FENCE_FLAG_USER_BITS, &fence->base.flags)) { - struct nouveau_fence_chan *fctx = nouveau_fctx(fence); - - if (atomic_dec_and_test(&fctx->notify_ref)) - drop = 1; - } - dma_fence_put(&fence->base); - return drop; } static struct nouveau_fence * @@ -93,8 +83,7 @@ nouveau_fence_context_kill(struct nouveau_fence_chan *fctx, int error) if (error) dma_fence_set_error(&fence->base, error); - if (nouveau_fence_signal(fence)) - nvif_event_block(&fctx->event); + nouveau_fence_signal(fence); } fctx->killed = 1; spin_unlock_irqrestore(&fctx->lock, flags); @@ -103,8 +92,8 @@ nouveau_fence_context_kill(struct nouveau_fence_chan *fctx, int error) void nouveau_fence_context_del(struct nouveau_fence_chan *fctx) { - cancel_work_sync(&fctx->allow_block_work); nouveau_fence_context_kill(fctx, 0); + nvif_event_block(&fctx->event); nvif_event_dtor(&fctx->event); fctx->dead = 1; @@ -127,11 +116,10 @@ nouveau_fence_context_free(struct nouveau_fence_chan *fctx) kref_put(&fctx->fence_ref, nouveau_fence_context_put); } -static int +static void nouveau_fence_update(struct nouveau_channel *chan, struct nouveau_fence_chan *fctx) { struct nouveau_fence *fence; - int drop = 0; u32 seq = fctx->read(chan); while (!list_empty(&fctx->pending)) { @@ -140,10 +128,8 @@ nouveau_fence_update(struct nouveau_channel *chan, struct nouveau_fence_chan *fc if ((int)(seq - fence->base.seqno) < 0) break; - drop |= nouveau_fence_signal(fence); + nouveau_fence_signal(fence); } - - return drop; } static int @@ -160,26 +146,13 @@ nouveau_fence_wait_uevent_handler(struct nvif_event *event, void *repv, u32 repc fence = list_entry(fctx->pending.next, typeof(*fence), head); chan = rcu_dereference_protected(fence->channel, lockdep_is_held(&fctx->lock)); - if (nouveau_fence_update(chan, fctx)) - ret = NVIF_EVENT_DROP; + nouveau_fence_update(chan, fctx); } spin_unlock_irqrestore(&fctx->lock, flags); return ret; } -static void -nouveau_fence_work_allow_block(struct work_struct *work) -{ - struct nouveau_fence_chan *fctx = container_of(work, struct nouveau_fence_chan, - allow_block_work); - - if (atomic_read(&fctx->notify_ref) == 0) - nvif_event_block(&fctx->event); - else - nvif_event_allow(&fctx->event); -} - void nouveau_fence_context_new(struct nouveau_channel *chan, struct nouveau_fence_chan *fctx) { @@ -191,7 +164,6 @@ nouveau_fence_context_new(struct nouveau_channel *chan, struct nouveau_fence_cha } args; int ret; - INIT_WORK(&fctx->allow_block_work, nouveau_fence_work_allow_block); INIT_LIST_HEAD(&fctx->flip); INIT_LIST_HEAD(&fctx->pending); spin_lock_init(&fctx->lock); @@ -216,6 +188,12 @@ nouveau_fence_context_new(struct nouveau_channel *chan, struct nouveau_fence_cha &args.base, sizeof(args), &fctx->event); WARN_ON(ret); + + /* + * Always allow non-stall irq events - previously this code tried to + * enable/disable them, but that just seems racy as nonstall irqs are unlatched. + */ + nvif_event_allow(&fctx->event); } int @@ -247,8 +225,7 @@ nouveau_fence_emit(struct nouveau_fence *fence) return -ENODEV; } - if (nouveau_fence_update(chan, fctx)) - nvif_event_block(&fctx->event); + nouveau_fence_update(chan, fctx); list_add_tail(&fence->head, &fctx->pending); spin_unlock_irq(&fctx->lock); @@ -271,8 +248,8 @@ nouveau_fence_done(struct nouveau_fence *fence) spin_lock_irqsave(&fctx->lock, flags); chan = rcu_dereference_protected(fence->channel, lockdep_is_held(&fctx->lock)); - if (chan && nouveau_fence_update(chan, fctx)) - nvif_event_block(&fctx->event); + if (chan) + nouveau_fence_update(chan, fctx); spin_unlock_irqrestore(&fctx->lock, flags); } return dma_fence_is_signaled(&fence->base); @@ -530,32 +507,10 @@ static const struct dma_fence_ops nouveau_fence_ops_legacy = { .release = nouveau_fence_release }; -static bool nouveau_fence_enable_signaling(struct dma_fence *f) -{ - struct nouveau_fence *fence = from_fence(f); - struct nouveau_fence_chan *fctx = nouveau_fctx(fence); - bool ret; - bool do_work; - - if (atomic_inc_return(&fctx->notify_ref) == 0) - do_work = true; - - ret = nouveau_fence_no_signaling(f); - if (ret) - set_bit(DMA_FENCE_FLAG_USER_BITS, &fence->base.flags); - else if (atomic_dec_and_test(&fctx->notify_ref)) - do_work = true; - - if (do_work) - schedule_work(&fctx->allow_block_work); - - return ret; -} - static const struct dma_fence_ops nouveau_fence_ops_uevent = { .get_driver_name = nouveau_fence_get_get_driver_name, .get_timeline_name = nouveau_fence_get_timeline_name, - .enable_signaling = nouveau_fence_enable_signaling, + .enable_signaling = nouveau_fence_no_signaling, .signaled = nouveau_fence_is_signaled, .release = nouveau_fence_release }; diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.h b/drivers/gpu/drm/nouveau/nouveau_fence.h index 28f5cf013b89..380bb0397ed2 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fence.h +++ b/drivers/gpu/drm/nouveau/nouveau_fence.h @@ -46,8 +46,6 @@ struct nouveau_fence_chan { char name[32]; struct nvif_event event; - struct work_struct allow_block_work; - atomic_t notify_ref; int dead, killed; };