From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3814E3A1D01 for ; Mon, 3 Aug 2026 09:04:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.251.105.195 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785747851; cv=none; b=kwSQtPyzNegp1xwBP1OZhxKsq3BWScdnZ7dGGKUYLkZtbtWJvIipMAw1wVH3HU64Mt2ElQKsggVGC8EWX8Vj7ryHVliXVJw2BSBuEH7Yv6Kg8jX9muwCVD6/4KF0hrNzZAAlPpGxM9/8abM/S8JQgeIMOkqerhcgpapfEfeTlYI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785747851; c=relaxed/simple; bh=+NvMpyWSxZkkzCvfX/lFp2kS29u7RkUL1Ej41vE7KiY=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=uIQpW1wk8eGXwJOjQdsKGNaqnmlaYONy72HAABo3rgTBO69HPq8dM8iQl6Ibww5+SnQaZnyKAE/KreKFrn4mqy7wWDcts7tFdVrvuTs2rYFXdGTEeZPjRiy2Kn3XH3oJxabP4Rhs0LDGOsjOVwzSFN8faNr4O3B2+G91SfxhoSU= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b=bCWsW03T; arc=none smtp.client-ip=148.251.105.195 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b="bCWsW03T" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1785747842; bh=+NvMpyWSxZkkzCvfX/lFp2kS29u7RkUL1Ej41vE7KiY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=bCWsW03T0kCOM05GN8hC5utHgwnseyHmWb6v0jzPFpDELisGKnKt+3OwNhmOhUS2p bKXLYYY7AIaN4zgXoPa1/tdPNOPvELJew9c3NGn+BguveXICBpK8ZGkJPLMybnLD+3 5AD5u3k37JIBkzuxnHmZ6Q6yfSiT3Tk0I+stHXWgXj4Y9+xYemc9EOJMA0Qaj3XYXu jDd93CYe8kI3h/5LY4am+Ju2fvTm4eZgSaspConFG86h/ZDDfy2grjbP5CaszVUP44 qDsFRzkviGxTraRsoxTTOnMYYlCLsCEsCvcGbwhe5truBQkMGXd3O1Jnjh8M7+bHnL 7hF+JHonmUtPg== Received: from fedora1.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 18A9517E0667; Mon, 03 Aug 2026 11:04:02 +0200 (CEST) Date: Mon, 3 Aug 2026 11:03:58 +0200 From: Boris Brezillon To: Nicolas Frattaroli Cc: Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Steven Rostedt , Ben Segall , Mel Gorman , Valentin Schneider , K Prateek Nayak , Steven Price , Liviu Dudau , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Grant Likely , Heiko Stuebner , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, kernel@collabora.com Subject: Re: [PATCH v2 3/3] drm/panthor: Add tracepoints for cache flushing Message-ID: <20260803110358.31afad0b@fedora1.home> In-Reply-To: <20260730-panthor-cache-flush-fix-v2-3-28790478bfff@collabora.com> References: <20260730-panthor-cache-flush-fix-v2-0-28790478bfff@collabora.com> <20260730-panthor-cache-flush-fix-v2-3-28790478bfff@collabora.com> Organization: Collabora X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; x86_64-redhat-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Thu, 30 Jul 2026 13:45:16 +0200 Nicolas Frattaroli wrote: > Add two new event tracepoints: gpu_cache_flush_start to be emitted after > acquiring the flush mutex and reqs spinlock, and gpu_cache_flush_end to > be emitted when leaving the function. > > This allows debugging the duration a flush takes irrespective of initial > function entry lock contention by subtracting the start tracepoint's > timestamp from the end tracepoint timestamp, and additionally contains > information such as which caches were flushed. > > Signed-off-by: Nicolas Frattaroli > --- > drivers/gpu/drm/panthor/panthor_gpu.c | 3 ++ > drivers/gpu/drm/panthor/panthor_trace.h | 49 +++++++++++++++++++++++++++++++++ > 2 files changed, 52 insertions(+) > > diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c b/drivers/gpu/drm/panthor/panthor_gpu.c > index f015bde80abf..a25955ad668b 100644 > --- a/drivers/gpu/drm/panthor/panthor_gpu.c > +++ b/drivers/gpu/drm/panthor/panthor_gpu.c > @@ -336,6 +336,7 @@ int panthor_gpu_flush_caches(struct panthor_device *ptdev, > guard(mutex)(&ptdev->gpu->cache_flush_lock); > > spin_lock(&ptdev->gpu->reqs_lock); > + trace_gpu_cache_flush_start(ptdev->base.dev, l2, lsc, other); > if (!(ptdev->gpu->pending_reqs & GPU_IRQ_CLEAN_CACHES_COMPLETED)) { > ptdev->gpu->pending_reqs |= GPU_IRQ_CLEAN_CACHES_COMPLETED; > gpu_write(gpu->iomem, GPU_CMD, GPU_FLUSH_CACHES(l2, lsc, other)); > @@ -345,6 +346,7 @@ int panthor_gpu_flush_caches(struct panthor_device *ptdev, > > if (ret) { > spin_unlock(&ptdev->gpu->reqs_lock); > + trace_gpu_cache_flush_end(ptdev->base.dev, l2, lsc, other); Should we add a status to the end event, so that faulty flushes can be filtered out (those can be immediate, or timeout=100ms depending on where the failure happens, but they are not reflecting anything useful, and would pollute the stats)? Actually, if what we care about is the time it takes to do a flush, do we even need those start/end events, can't we pass the time as an argument and do the math in the function like we do in the job IRQ handler? > return ret; > } > > @@ -358,6 +360,7 @@ int panthor_gpu_flush_caches(struct panthor_device *ptdev, > ptdev->gpu->pending_reqs &= ~GPU_IRQ_CLEAN_CACHES_COMPLETED; > } > spin_unlock(&ptdev->gpu->reqs_lock); > + trace_gpu_cache_flush_end(ptdev->base.dev, l2, lsc, other); > > if (ret) { > panthor_device_schedule_reset(ptdev); > diff --git a/drivers/gpu/drm/panthor/panthor_trace.h b/drivers/gpu/drm/panthor/panthor_trace.h > index 6ffeb4fe6599..6951b95b1de7 100644 > --- a/drivers/gpu/drm/panthor/panthor_trace.h > +++ b/drivers/gpu/drm/panthor/panthor_trace.h > @@ -76,6 +76,55 @@ TRACE_EVENT(gpu_job_irq, > __entry->events, __entry->duration_ns) > ); > > +DECLARE_EVENT_CLASS(gpu_cache_flush_template, > + TP_PROTO(const struct device *dev, u32 l2, u32 lsc, u32 other), > + TP_ARGS(dev, l2, lsc, other), > + TP_STRUCT__entry( > + __string(dev_name, dev_name(dev)) > + __field(u32, l2) > + __field(u32, lsc) > + __field(u32, other) > + ), > + TP_fast_assign( > + __assign_str(dev_name); > + __entry->l2 = l2; > + __entry->lsc = lsc; > + __entry->other = other; > + ), > + TP_printk("%s: l2=0x%x lsc=0x%x other=0x%x", __get_str(dev_name), > + __entry->l2, __entry->lsc, __entry->other) > +); > + > +/** > + * gpu_cache_flush_start - called after cache flush locks taken, before flush > + * @dev: pointer to the &struct device, for printing the device name > + * @l2: "l2" flush flags > + * @lsc: "lsc" flush flags > + * @other: "other" flush flags > + * > + * Fires after any initial lock contention around the locks needed for flushing > + * caches, but before the actual cache flush is requested. > + */ > +DEFINE_EVENT(gpu_cache_flush_template, gpu_cache_flush_start, > + TP_PROTO(const struct device *dev, u32 l2, u32 lsc, u32 other), > + TP_ARGS(dev, l2, lsc, other) > +); > + > +/** > + * gpu_cache_flush_end - called after cache flush > + * @dev: pointer to the &struct device, for printing the device name > + * @l2: "l2" flush flags > + * @lsc: "lsc" flush flags > + * @other: "other" flush flags > + * > + * Fires after either the cache flush is complete, or has failed. Can be used > + * together with gpu_cache_flush_start to get how long the flush has taken. > + */ > +DEFINE_EVENT(gpu_cache_flush_template, gpu_cache_flush_end, > + TP_PROTO(const struct device *dev, u32 l2, u32 lsc, u32 other), > + TP_ARGS(dev, l2, lsc, other) > +); > + > #endif /* __PANTHOR_TRACE_H__ */ > > #undef TRACE_INCLUDE_PATH >