[PATCH 2/2] dma-fence: Improve docu for dma_fence_check_and_signal()

[Li, Yunxiang (Teddy)]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Philipp,

I feel like the problem has two parts. The documentation does not make explicit that DMA_FENCE_FLAG_SIGNALED_BIT is "caching" the hardware state when a fence is backed by hardware, so what dma_fence_is_signaled here is doing is just busting that cache; when the hardware signals the fence, the dma_fence is considered signaled, just with a stale cache. It looks like because of this omission nouveau has made an assumption that there could be a canonical path to signaling dma_fence but in reality, anyone could call dma_fence_signal at any time if it realized that the cache is stale.

I do think that the caching behavior here may be confusing and it could be a good idea to separate out the concept of a software fence vs a hardware fence.

Regards,
Teddy

________________________________
From: Philipp Stanner
Sent: Wednesday, April 09, 2025 08:06
To: Sumit Semwal; Gustavo Padovan; Koenig, Christian; Kuehling, Felix; Deucher, Alexander; Xinhui Pan; David Airlie; Simona Vetter; Maarten Lankhorst; Maxime Ripard; Thomas Zimmermann; Lucas Stach; Russell King; Christian Gmeiner; Jani Nikula; Joonas Lahtinen; Rodrigo Vivi; Tvrtko Ursulin; Frank Binns; Matt Coster; Qiang Yu; Rob Clark; Sean Paul; Konrad Dybcio; Abhinav Kumar; Dmitry Baryshkov; Marijn Suijten; Lyude Paul; Danilo Krummrich; Boris Brezillon; Rob Herring; Steven Price; Dave Airlie; Gerd Hoffmann; Matthew Brost; Philipp Stanner; Huang, Ray; Matthew Auld; Melissa Wen; Maíra Canal; Zack Rusin; Broadcom internal kernel review list; Lucas De Marchi; Thomas Hellström; Bas Nieuwenhuizen; Wang, Yang(Kevin); Zhang, Jesse(Jie); Huang, Tim; Sundararaju, Sathishkumar; Jamadar, Saleemkhan; Khatri, Sunil; Lazar, Lijo; Zhang, Hawking; Ma Jun; Li, Yunxiang (Teddy); Huang, JinHuiEric; Kamal, Asad; SHANMUGAM, SRINIVASAN; Xiao, Jack; Friedrich Vock; Michel Dänzer; Geert Uytterhoeven; Anna-Maria Behnsen; Thomas Gleixner; Frederic Weisbecker; Dan Carpenter
Cc: linux-media at vger.kernel.org; dri-devel at lists.freedesktop.org; linaro-mm-sig at lists.linaro.org; linux-kernel at vger.kernel.org; amd-gfx at lists.freedesktop.org; etnaviv at lists.freedesktop.org; intel-gfx at lists.freedesktop.org; lima at lists.freedesktop.org; linux-arm-msm at vger.kernel.org; freedreno at lists.freedesktop.org; nouveau at lists.freedesktop.org; virtualization at lists.linux.dev; spice-devel at lists.freedesktop.org; intel-xe at lists.freedesktop.org
Subject: [PATCH 2/2] dma-fence: Improve docu for dma_fence_check_and_signal()

The documentation of the return value of dma_fence_check_and_signal()
and dma_fence_check_and_signal_locked() reads as if the returned boolean
only describes whether dma_fence_signal() (or similar) has been called
before this function call already. That's not the case, since
dma_fence_ops.signaled() usually just checks through the sequence number
whether the hardware is finished with a fence. That doesn't mean a
signaling function has been called already.

Make the documentation clearer.

Move the Return: documentation to the end, since that's the officially
recommended docu style.

Signed-off-by: Philipp Stanner <phasta at kernel.org>
---
 include/linux/dma-fence.h | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index dc2ad171458b..3df370b2cc7c 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -385,14 +385,21 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence);
  * dma_fence_check_and_signal_locked - Checks a fence and signals it if necessary
  * @fence: the fence to check
  *
- * Returns true if the fence was already signaled, false if not. Since this
- * function doesn't enable signaling, it is not guaranteed to ever return
- * true if dma_fence_add_callback(), dma_fence_wait() or
+ * Checks whether the fence was already signaled, and, if not, whether
+ * &struct dma_fence_ops.signaled indicates that it should be signaled. If so,
+ * the fence gets signaled here.
+ *
+ * Since this function doesn't enable signaling, it is not guaranteed to ever
+ * return true if dma_fence_add_callback(), dma_fence_wait() or
  * dma_fence_enable_sw_signaling() haven't been called before.
  *
  * This function requires &dma_fence.lock to be held.
  *
  * See also dma_fence_check_and_signal().
+ *
+ * Return: true if the fence was already signaled, or if
+ * &struct dma_fence_ops.signaled is implemented and indicates that this fence
+ * can be treated as signaled; false otherwise.
  */
 static inline bool
 dma_fence_check_and_signal_locked(struct dma_fence *fence)
@@ -412,9 +419,12 @@ dma_fence_check_and_signal_locked(struct dma_fence *fence)
  * dma_fence_check_and_signal - Checks a fence and signals it if necessary
  * @fence: the fence to check
  *
- * Returns true if the fence was already signaled, false if not. Since this
- * function doesn't enable signaling, it is not guaranteed to ever return
- * true if dma_fence_add_callback(), dma_fence_wait() or
+ * Checks whether the fence was already signaled, and, if not, whether
+ * &struct dma_fence_ops.signaled indicates that it should be signaled. If so,
+ * the fence gets signaled here.
+ *
+ * Since this function doesn't enable signaling, it is not guaranteed to ever
+ * return true if dma_fence_add_callback(), dma_fence_wait() or
  * dma_fence_enable_sw_signaling() haven't been called before.
  *
  * It's recommended for seqno fences to call dma_fence_signal when the
@@ -423,6 +433,10 @@ dma_fence_check_and_signal_locked(struct dma_fence *fence)
  * value of this function before calling hardware-specific wait instructions.
  *
  * See also dma_fence_check_and_signal_locked().
+ *
+ * Return: true if the fence was already signaled, or if
+ * &struct dma_fence_ops.signaled is implemented and indicates that this fence
+ * can be treated as signaled; false otherwise.
  */
 static inline bool
 dma_fence_check_and_signal(struct dma_fence *fence)
--
2.48.1

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/freedreno/attachments/20250409/c51865d7/attachment-0001.htm>