<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <p style="font-family:Calibri;font-size:10pt;color:#0000FF;margin:5pt;font-style:normal;font-weight:normal;text-decoration:none;" align="Left"> [AMD Official Use Only - AMD Internal Distribution Only]<br> </p> <br> <div> <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"> Hi Philipp,</div> <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"> 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.</div> <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"> 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.</div> <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"> <br> </div> <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"> Regards,</div> <div class="elementToProof" style="font-family: Aptos, Aptos_EmbeddedFont, Aptos_MSFontService, Calibri, Helvetica, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);"> Teddy</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <hr style="display: inline-block; width: 98%;"> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <b>From:</b> Philipp Stanner<br> <b>Sent:</b> Wednesday, April 09, 2025 08:06<br> <b>To:</b> 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<br> <b>Cc:</b> linux-media@vger.kernel.org; dri-devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org; linux-kernel@vger.kernel.org; amd-gfx@lists.freedesktop.org; etnaviv@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; lima@lists.freedesktop.org; linux-arm-msm@vger.kernel.org; freedreno@lists.freedesktop.org; nouveau@lists.freedesktop.org; virtualization@lists.linux.dev; spice-devel@lists.freedesktop.org; intel-xe@lists.freedesktop.org<br> <b>Subject:</b> [PATCH 2/2] dma-fence: Improve docu for dma_fence_check_and_signal() </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> <br> </div> <div style="font-size: 11pt;">The documentation of the return value of dma_fence_check_and_signal()<br> and dma_fence_check_and_signal_locked() reads as if the returned boolean<br> only describes whether dma_fence_signal() (or similar) has been called<br> before this function call already. That's not the case, since<br> dma_fence_ops.signaled() usually just checks through the sequence number<br> whether the hardware is finished with a fence. That doesn't mean a<br> signaling function has been called already.<br> <br> Make the documentation clearer.<br> <br> Move the Return: documentation to the end, since that's the officially<br> recommended docu style.<br> <br> Signed-off-by: Philipp Stanner <phasta@kernel.org><br> ---<br> include/linux/dma-fence.h | 26 ++++++++++++++++++++------<br> 1 file changed, 20 insertions(+), 6 deletions(-)<br> <br> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h<br> index dc2ad171458b..3df370b2cc7c 100644<br> --- a/include/linux/dma-fence.h<br> +++ b/include/linux/dma-fence.h<br> @@ -385,14 +385,21 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence);<br> * dma_fence_check_and_signal_locked - Checks a fence and signals it if necessary<br> * @fence: the fence to check<br> *<br> - * Returns true if the fence was already signaled, false if not. Since this<br> - * function doesn't enable signaling, it is not guaranteed to ever return<br> - * true if dma_fence_add_callback(), dma_fence_wait() or<br> + * Checks whether the fence was already signaled, and, if not, whether<br> + * &struct dma_fence_ops.signaled indicates that it should be signaled. If so,<br> + * the fence gets signaled here.<br> + *<br> + * Since this function doesn't enable signaling, it is not guaranteed to ever<br> + * return true if dma_fence_add_callback(), dma_fence_wait() or<br> * dma_fence_enable_sw_signaling() haven't been called before.<br> *<br> * This function requires &dma_fence.lock to be held.<br> *<br> * See also dma_fence_check_and_signal().<br> + *<br> + * Return: true if the fence was already signaled, or if<br> + * &struct dma_fence_ops.signaled is implemented and indicates that this fence<br> + * can be treated as signaled; false otherwise.<br> */<br> static inline bool<br> dma_fence_check_and_signal_locked(struct dma_fence *fence)<br> @@ -412,9 +419,12 @@ dma_fence_check_and_signal_locked(struct dma_fence *fence)<br> * dma_fence_check_and_signal - Checks a fence and signals it if necessary<br> * @fence: the fence to check<br> *<br> - * Returns true if the fence was already signaled, false if not. Since this<br> - * function doesn't enable signaling, it is not guaranteed to ever return<br> - * true if dma_fence_add_callback(), dma_fence_wait() or<br> + * Checks whether the fence was already signaled, and, if not, whether<br> + * &struct dma_fence_ops.signaled indicates that it should be signaled. If so,<br> + * the fence gets signaled here.<br> + *<br> + * Since this function doesn't enable signaling, it is not guaranteed to ever<br> + * return true if dma_fence_add_callback(), dma_fence_wait() or<br> * dma_fence_enable_sw_signaling() haven't been called before.<br> *<br> * It's recommended for seqno fences to call dma_fence_signal when the<br> @@ -423,6 +433,10 @@ dma_fence_check_and_signal_locked(struct dma_fence *fence)<br> * value of this function before calling hardware-specific wait instructions.<br> *<br> * See also dma_fence_check_and_signal_locked().<br> + *<br> + * Return: true if the fence was already signaled, or if<br> + * &struct dma_fence_ops.signaled is implemented and indicates that this fence<br> + * can be treated as signaled; false otherwise.<br> */<br> static inline bool<br> dma_fence_check_and_signal(struct dma_fence *fence)<br> --<br> 2.48.1<br> <br> </div> </div> </body> </html>