NAK/Cmnt: [PATCH 1/1] Revert "drm/qxl: simplify qxl_fence_wait"

[John Cabaj]

On 7/15/25 12:32 PM, ghadi.rahme at canonical.com wrote:
> From: Alex Constantino <dreaming.about.electric.sheep at gmail.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/2065153
> 
> This reverts commit 5a838e5d5825c85556011478abde708251cc0776.
> 
> Changes from commit 5a838e5d5825 ("drm/qxl: simplify qxl_fence_wait") would
> result in a '[TTM] Buffer eviction failed' exception whenever it reached a
> timeout.
> Due to a dependency to DMA_FENCE_WARN this also restores some code deleted
> by commit d72277b6c37d ("dma-buf: nuke DMA_FENCE_TRACE macros v2").
> 
> Fixes: 5a838e5d5825 ("drm/qxl: simplify qxl_fence_wait")
> Link: https://lore.kernel.org/regressions/ZTgydqRlK6WX_b29@eldamar.lan/
> Reported-by: Timo Lindfors <timo.lindfors at iki.fi>
> Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1054514
> Signed-off-by: Alex Constantino <dreaming.about.electric.sheep at gmail.com>
> Signed-off-by: Maxime Ripard <mripard at kernel.org>
> Link: https://patchwork.freedesktop.org/patch/msgid/20240404181448.1643-2-dreaming.about.electric.sheep@gmail.com
> (backported from commit 5a838e5d5825c85556011478abde708251cc0776)

This commit SHA is from the original "drm/qxl: simplify qxl_fence_wait" 
commit. We should either cherry-pick the upstream revert (07ed11afb68d: 
Revert "drm/qxl: simplify qxl_fence_wait"), or not use the original 
commit SHA in the patch details.

> [Ghadi Elie Rahme: Had to undo the revert of DMA_FENCE_WARN due to missing patch]
> Signed-off-by: Ghadi Elie Rahme <ghadi.rahme at canonical.com>
> ---
>   drivers/gpu/drm/qxl/qxl_release.c | 50 +++++++++++++++++++++++++++----
>   1 file changed, 45 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
> index b19f2f00b215..d4f26075383d 100644
> --- a/drivers/gpu/drm/qxl/qxl_release.c
> +++ b/drivers/gpu/drm/qxl/qxl_release.c
> @@ -58,16 +58,56 @@ static long qxl_fence_wait(struct dma_fence *fence, bool intr,
>   			   signed long timeout)
>   {
>   	struct qxl_device *qdev;
> +	struct qxl_release *release;
> +	int count = 0, sc = 0;
> +	bool have_drawable_releases;
>   	unsigned long cur, end = jiffies + timeout;
>   
>   	qdev = container_of(fence->lock, struct qxl_device, release_lock);
> +	release = container_of(fence, struct qxl_release, base);
> +	have_drawable_releases = release->type == QXL_RELEASE_DRAWABLE;
>   
> -	if (!wait_event_timeout(qdev->release_event,
> -				(dma_fence_is_signaled(fence) ||
> -				 (qxl_io_notify_oom(qdev), 0)),
> -				timeout))
> -		return 0;
> +retry:
> +	sc++;
> +
> +	if (dma_fence_is_signaled(fence))
> +		goto signaled;
> +
> +	qxl_io_notify_oom(qdev);
> +
> +	for (count = 0; count < 11; count++) {
> +		if (!qxl_queue_garbage_collect(qdev, true))
> +			break;
> +
> +		if (dma_fence_is_signaled(fence))
> +			goto signaled;
> +	}
> +
> +	if (dma_fence_is_signaled(fence))
> +		goto signaled;
> +
> +	if (have_drawable_releases || sc < 4) {
> +		if (sc > 2)
> +			/* back off */
> +			usleep_range(500, 1000);
> +
> +		if (time_after(jiffies, end))
> +			return 0;
> +
> +		if (have_drawable_releases && sc > 300) {
> +			DMA_FENCE_WARN(fence,
> +				       "failed to wait on release %llu after spincount %d\n",
> +				       fence->context & ~0xf0000000, sc);
> +			goto signaled;
> +		}
> +		goto retry;
> +	}
> +	/*
> +	 * yeah, original sync_obj_wait gave up after 3 spins when
> +	 * have_drawable_releases is not set.
> +	 */
>   
> +signaled:
>   	cur = jiffies;
>   	if (time_after(cur, end))
>   		return 0;