[Nouveau] [PATCH] drm/nouveau: handle same-fb page flips

[Ben Skeggs]

On Fri, Oct 05, 2012 at 09:37:59PM +0200, Marcin Slusarz wrote:
> It's questionable use case, but weston/wayland already relies on this behaviour,
> and other drivers don't care about it, so it's a matter of compatibility.
> Without it, process invoking such page flip hangs in unkillable state, trying
> to reserve the same buffer twice.
> 
> Signed-off-by: Marcin Slusarz <marcin.slusarz at gmail.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_display.c | 20 +++++++++++++++-----
>  1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
> index 61f370d..a52cfd3 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -530,9 +530,11 @@ nouveau_page_flip_reserve(struct nouveau_bo *old_bo,
>  	if (ret)
>  		goto fail;
>  
> -	ret = ttm_bo_reserve(&old_bo->bo, false, false, false, 0);
> -	if (ret)
> -		goto fail_unreserve;
> +	if (likely(old_bo != new_bo)) {
> +		ret = ttm_bo_reserve(&old_bo->bo, false, false, false, 0);
> +		if (ret)
> +			goto fail_unreserve;
> +	}
>  
>  	return 0;
>  
> @@ -551,8 +553,10 @@ nouveau_page_flip_unreserve(struct nouveau_bo *old_bo,
>  	nouveau_bo_fence(new_bo, fence);
>  	ttm_bo_unreserve(&new_bo->bo);
>  
> -	nouveau_bo_fence(old_bo, fence);
> -	ttm_bo_unreserve(&old_bo->bo);
> +	if (likely(old_bo != new_bo)) {
> +		nouveau_bo_fence(old_bo, fence);
> +		ttm_bo_unreserve(&old_bo->bo);
> +	}
>  
>  	nouveau_bo_unpin(old_bo);
>  }
> @@ -624,6 +628,12 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
>  	if (!drm->channel)
>  		return -ENODEV;
>  
> +	if (unlikely(old_bo == new_bo)) {
> +		char name[sizeof(current->comm)];
> +		pr_debug_ratelimited("'%s': useless page flip invoked\n",
> +				get_task_comm(name, current));
> +	}
> +
The patch looks alright, except I think we should just drop this hunk,
no other driver reports it either.

Thoughts?

Ben.

>  	s = kzalloc(sizeof(*s), GFP_KERNEL);
>  	if (!s)
>  		return -ENOMEM;
> -- 
> 1.7.12
>