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 26579C5DF81 for ; Mon, 24 Aug 2026 07:41:05 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 67EC810E51B; Mon, 24 Aug 2026 07:41:04 +0000 (UTC) Authentication-Results: gabe.freedesktop.org; dkim=pass (2048-bit key; unprotected) header.d=collabora.com header.i=@collabora.com header.b="YfmAFesI"; dkim-atps=neutral Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) by gabe.freedesktop.org (Postfix) with ESMTPS id 291CB10E3B3 for ; Mon, 24 Aug 2026 07:41:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1787557261; bh=3w6JZpSSkSm/pMmEJvNwfCiUqGWmI3ByAme16SGIMuc=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=YfmAFesI29dOlSpjN9gFYbckcdV9hQSrOs65yE8YWoMwqWLO75AZ2fL+41GMH84Xl wij+6qxBYn0XCr1XcRSaB0DTfmE3PiGAAzsOfSQC+ptUwb+4FKZ1LbhCoHCkOEU/+n i9ni/aviqbN/jzg5JGTQVn9mW5DsgDS1SlOC9kRSKQ+V2+R/DMM0fwwgiZ+XKabyrC skhQLC6773LUjjkRtmq/My+9SnVXZbGR8Mr2K+MuCc8Tot5QXbeCizOoJhm7p+juDh 62GMcKc04F4dTRGMdniSLxhgzGj8g3F8nK1/wejkN2pbYo7T/3nDm38X23OiupwfYU U0ZO1uOvMY1Ig== Received: from fedora-21.home (unknown [100.64.0.11]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange secp256r1 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by bali.collaboradmins.com (Postfix) with ESMTPSA id 150D417E084E; Mon, 24 Aug 2026 09:41:01 +0200 (CEST) Date: Mon, 24 Aug 2026 09:40:55 +0200 From: Boris Brezillon To: Liviu Dudau Cc: Steven Price , Chia-I Wu , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v6 9/9] drm/panthor: Automate CSG IRQ processing at group unbind time Message-ID: <20260824094055.32e2aa37@fedora-21.home> In-Reply-To: References: <20260811-panthor-signal-from-irq-v6-0-12f2ab53d735@collabora.com> <20260811-panthor-signal-from-irq-v6-9-12f2ab53d735@collabora.com> Organization: Collabora X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 Tue, 18 Aug 2026 18:20:58 +0100 Liviu Dudau wrote: > On Tue, Aug 11, 2026 at 01:23:43PM +0200, Boris Brezillon wrote: > > Make the sched_process_csg_irq_locked() call part of > > group_unbind_locked() so we don't have to manually call it in > > tick_ctx_apply()/panthor_sched_suspend(). > > > > This implies moving group_[un]bind_locked() around to avoid a > > forward declaration. > > Not sure why group_bind_locked() had to move. Aestethics? Yeah, I tend to keep do/undo helpers close to each other in general. I can make the code motion a separate commit if you prefer. > > Makes the patch a bit harder to review as one function is copied > verbatim while the other gets a line added. Minor sigh in the > end, so please ignore. > > Reviewed-by: Liviu Dudau > > Best regards, > Liviu > > > > > Reviewed-by: Steven Price > > Reviewed-by: Chia-I Wu > > Signed-off-by: Boris Brezillon > > --- > > drivers/gpu/drm/panthor/panthor_sched.c | 182 +++++++++++++++----------------- > > 1 file changed, 84 insertions(+), 98 deletions(-) > > > > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c > > index 8016b0a55173..57308cd90e96 100644 > > --- a/drivers/gpu/drm/panthor/panthor_sched.c > > +++ b/drivers/gpu/drm/panthor/panthor_sched.c > > @@ -989,87 +989,6 @@ group_get(struct panthor_group *group) > > return group; > > } > > > > -/** > > - * group_bind_locked() - Bind a group to a group slot > > - * @group: Group. > > - * @csg_id: Slot. > > - * > > - * Return: 0 on success, a negative error code otherwise. > > - */ > > -static int > > -group_bind_locked(struct panthor_group *group, u32 csg_id) > > -{ > > - struct panthor_device *ptdev = group->ptdev; > > - int ret; > > - > > - lockdep_assert_held(&ptdev->scheduler->lock); > > - > > - if (drm_WARN_ON(&ptdev->base, group->csg_id != -1 || csg_id >= MAX_CSGS || > > - ptdev->scheduler->csg_slots[csg_id].group)) > > - return -EINVAL; > > - > > - ret = panthor_vm_active(group->vm); > > - if (ret) > > - return ret; > > - > > - group_get(group); > > - > > - /* Dummy doorbell allocation: doorbell is assigned to the group and > > - * all queues use the same doorbell. > > - * > > - * TODO: Implement LRU-based doorbell assignment, so the most often > > - * updated queues get their own doorbell, thus avoiding useless checks > > - * on queues belonging to the same group that are rarely updated. > > - */ > > - for (u32 i = 0; i < group->queue_count; i++) > > - group->queues[i]->doorbell_id = csg_id + 1; > > - > > - scoped_guard(spinlock, &ptdev->scheduler->events_lock) { > > - ptdev->scheduler->csg_slots[csg_id].group = group; > > - group->csg_id = csg_id; > > - } > > - > > - return 0; > > -} > > - > > -/** > > - * group_unbind_locked() - Unbind a group from a slot. > > - * @group: Group to unbind. > > - * > > - * Return: 0 on success, a negative error code otherwise. > > - */ > > -static int > > -group_unbind_locked(struct panthor_group *group) > > -{ > > - struct panthor_device *ptdev = group->ptdev; > > - > > - lockdep_assert_held(&ptdev->scheduler->lock); > > - > > - if (drm_WARN_ON(&ptdev->base, group->csg_id < 0 || group->csg_id >= MAX_CSGS)) > > - return -EINVAL; > > - > > - if (drm_WARN_ON(&ptdev->base, group->state == PANTHOR_CS_GROUP_ACTIVE)) > > - return -EINVAL; > > - > > - scoped_guard(spinlock, &ptdev->scheduler->events_lock) { > > - ptdev->scheduler->csg_slots[group->csg_id].group = NULL; > > - group->csg_id = -1; > > - } > > - > > - panthor_vm_idle(group->vm); > > - > > - /* Tiler OOM events will be re-issued next time the group is scheduled. */ > > - atomic_set(&group->tiler_oom, 0); > > - if (cancel_work(&group->tiler_oom_work)) > > - group_put(group); > > - > > - for (u32 i = 0; i < group->queue_count; i++) > > - group->queues[i]->doorbell_id = -1; > > - > > - group_put(group); > > - return 0; > > -} > > - > > static bool > > group_is_idle(struct panthor_group *group) > > { > > @@ -1979,6 +1898,89 @@ void panthor_sched_report_fw_events(struct panthor_device *ptdev, u32 events) > > } > > } > > > > +/** > > + * group_bind_locked() - Bind a group to a group slot > > + * @group: Group. > > + * @csg_id: Slot. > > + * > > + * Return: 0 on success, a negative error code otherwise. > > + */ > > +static int > > +group_bind_locked(struct panthor_group *group, u32 csg_id) > > +{ > > + struct panthor_device *ptdev = group->ptdev; > > + int ret; > > + > > + lockdep_assert_held(&ptdev->scheduler->lock); > > + > > + if (drm_WARN_ON(&ptdev->base, group->csg_id != -1 || csg_id >= MAX_CSGS || > > + ptdev->scheduler->csg_slots[csg_id].group)) > > + return -EINVAL; > > + > > + ret = panthor_vm_active(group->vm); > > + if (ret) > > + return ret; > > + > > + group_get(group); > > + > > + /* Dummy doorbell allocation: doorbell is assigned to the group and > > + * all queues use the same doorbell. > > + * > > + * TODO: Implement LRU-based doorbell assignment, so the most often > > + * updated queues get their own doorbell, thus avoiding useless checks > > + * on queues belonging to the same group that are rarely updated. > > + */ > > + for (u32 i = 0; i < group->queue_count; i++) > > + group->queues[i]->doorbell_id = csg_id + 1; > > + > > + scoped_guard(spinlock, &ptdev->scheduler->events_lock) { > > + ptdev->scheduler->csg_slots[csg_id].group = group; > > + group->csg_id = csg_id; > > + } > > + > > + return 0; > > +} > > + > > +/** > > + * group_unbind_locked() - Unbind a group from a slot. > > + * @group: Group to unbind. > > + * > > + * Return: 0 on success, a negative error code otherwise. > > + */ > > +static int > > +group_unbind_locked(struct panthor_group *group) > > +{ > > + struct panthor_device *ptdev = group->ptdev; > > + > > + lockdep_assert_held(&ptdev->scheduler->lock); > > + > > + if (drm_WARN_ON(&ptdev->base, group->csg_id < 0 || group->csg_id >= MAX_CSGS)) > > + return -EINVAL; > > + > > + if (drm_WARN_ON(&ptdev->base, group->state == PANTHOR_CS_GROUP_ACTIVE)) > > + return -EINVAL; > > + > > + scoped_guard(spinlock, &ptdev->scheduler->events_lock) { > > + /* Process all pending IRQs before returning the slot. */ > > + sched_process_csg_irq_locked(ptdev, group->csg_id); > > + ptdev->scheduler->csg_slots[group->csg_id].group = NULL; > > + group->csg_id = -1; > > + } > > + > > + panthor_vm_idle(group->vm); > > + > > + /* Tiler OOM events will be re-issued next time the group is scheduled. */ > > + atomic_set(&group->tiler_oom, 0); > > + if (cancel_work(&group->tiler_oom_work)) > > + group_put(group); > > + > > + for (u32 i = 0; i < group->queue_count; i++) > > + group->queues[i]->doorbell_id = -1; > > + > > + group_put(group); > > + return 0; > > +} > > + > > static const char *fence_get_driver_name(struct dma_fence *fence) > > { > > return "panthor"; > > @@ -2405,18 +2407,8 @@ tick_ctx_apply(struct panthor_scheduler *sched, struct panthor_sched_tick_ctx *c > > > > /* Unbind evicted groups. */ > > for (prio = PANTHOR_CSG_PRIORITY_COUNT - 1; prio >= 0; prio--) { > > - list_for_each_entry(group, &ctx->old_groups[prio], run_node) { > > - /* This group is gone. Process interrupts to clear > > - * any pending interrupts before we start the new > > - * group. > > - */ > > - if (group->csg_id >= 0) { > > - guard(spinlock)(&sched->events_lock); > > - sched_process_csg_irq_locked(ptdev, group->csg_id); > > - } > > - > > + list_for_each_entry(group, &ctx->old_groups[prio], run_node) > > group_unbind_locked(group); > > - } > > } > > > > for (i = 0; i < sched->csg_slot_count; i++) { > > @@ -3010,12 +3002,6 @@ void panthor_sched_suspend(struct panthor_device *ptdev) > > continue; > > > > group_get(group); > > - > > - if (group->csg_id >= 0) { > > - guard(spinlock)(&sched->events_lock); > > - sched_process_csg_irq_locked(ptdev, group->csg_id); > > - } > > - > > group_unbind_locked(group); > > > > drm_WARN_ON(&group->ptdev->base, !list_empty(&group->run_node)); > > > > -- > > 2.55.0 > > >