[PATCH] Revert "drm/amdgpu: Refactor flip into prepare submit and submit. (v2)"

[Christian König]

Am 27.04.2017 um 10:18 schrieb Michel Dänzer:
> From: Michel Dänzer <michel.daenzer at amd.com>
>
> This reverts commit cb341a319f7e66f879d69af929c3dadfc1a8f31e.
>
> The purpose of the refactor was for amdgpu_crtc_prepare/submit_flip to
> be used by the DC code, but that's no longer the case.
>
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 147 ++++++----------------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h    |  17 ----
>   2 files changed, 29 insertions(+), 135 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 7cf226dfb602..0415875656c1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -138,56 +138,10 @@ static void amdgpu_unpin_work_func(struct work_struct *__work)
>   	kfree(work);
>   }
>   
> -
> -static void amdgpu_flip_work_cleanup(struct amdgpu_flip_work *work)
> -{
> -	int i;
> -
> -	amdgpu_bo_unref(&work->old_abo);
> -	fence_put(work->excl);
> -	for (i = 0; i < work->shared_count; ++i)
> -		fence_put(work->shared[i]);
> -	kfree(work->shared);
> -	kfree(work);
> -}
> -
> -static void amdgpu_flip_cleanup_unreserve(
> -		struct amdgpu_flip_work *work,
> -		struct amdgpu_bo *new_abo)
> -{
> -	amdgpu_bo_unreserve(new_abo);
> -	amdgpu_flip_work_cleanup(work);
> -}
> -
> -static void amdgpu_flip_cleanup_unpin(
> -		struct amdgpu_flip_work *work,
> -		struct amdgpu_bo *new_abo)
> -{
> -	if (unlikely(amdgpu_bo_unpin(new_abo) != 0))
> -		DRM_ERROR("failed to unpin new abo in error path\n");
> -	amdgpu_flip_cleanup_unreserve(work, new_abo);
> -}
> -
> -void amdgpu_crtc_cleanup_flip_ctx(
> -		struct amdgpu_flip_work *work,
> -		struct amdgpu_bo *new_abo)
> -{
> -	if (unlikely(amdgpu_bo_reserve(new_abo, false) != 0)) {
> -		DRM_ERROR("failed to reserve new abo in error path\n");
> -		amdgpu_flip_work_cleanup(work);
> -		return;
> -	}
> -	amdgpu_flip_cleanup_unpin(work, new_abo);
> -}
> -
> -int amdgpu_crtc_prepare_flip(
> -				struct drm_crtc *crtc,
> -				struct drm_framebuffer *fb,
> -				struct drm_pending_vblank_event *event,
> -				uint32_t page_flip_flags,
> -				uint32_t target,
> -				struct amdgpu_flip_work **work_p,
> -				struct amdgpu_bo **new_abo_p)
> +int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc,
> +				 struct drm_framebuffer *fb,
> +				 struct drm_pending_vblank_event *event,
> +				 uint32_t page_flip_flags, uint32_t target)
>   {
>   	struct drm_device *dev = crtc->dev;
>   	struct amdgpu_device *adev = dev->dev_private;
> @@ -200,7 +154,7 @@ int amdgpu_crtc_prepare_flip(
>   	unsigned long flags;
>   	u64 tiling_flags;
>   	u64 base;
> -	int r;
> +	int i, r;
>   
>   	work = kzalloc(sizeof *work, GFP_KERNEL);
>   	if (work == NULL)
> @@ -261,84 +215,41 @@ int amdgpu_crtc_prepare_flip(
>   		spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
>   		r = -EBUSY;
>   		goto pflip_cleanup;
> -
>   	}
> -	spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> -
> -	*work_p = work;
> -	*new_abo_p = new_abo;
>   
> -	return 0;
> -
> -pflip_cleanup:
> -	amdgpu_crtc_cleanup_flip_ctx(work, new_abo);
> -	return r;
> -
> -unpin:
> -	amdgpu_flip_cleanup_unpin(work, new_abo);
> -	return r;
> -
> -unreserve:
> -	amdgpu_flip_cleanup_unreserve(work, new_abo);
> -	return r;
> -
> -cleanup:
> -	amdgpu_flip_work_cleanup(work);
> -	return r;
> -
> -}
> -
> -void amdgpu_crtc_submit_flip(
> -		struct drm_crtc *crtc,
> -		struct drm_framebuffer *fb,
> -		struct amdgpu_flip_work *work,
> -		struct amdgpu_bo *new_abo)
> -{
> -	unsigned long flags;
> -	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -
> -	spin_lock_irqsave(&crtc->dev->event_lock, flags);
>   	amdgpu_crtc->pflip_status = AMDGPU_FLIP_PENDING;
>   	amdgpu_crtc->pflip_works = work;
>   
> +
> +	DRM_DEBUG_DRIVER("crtc:%d[%p], pflip_stat:AMDGPU_FLIP_PENDING, work: %p,\n",
> +			 amdgpu_crtc->crtc_id, amdgpu_crtc, work);
>   	/* update crtc fb */
>   	crtc->primary->fb = fb;
>   	spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> -
> -	DRM_DEBUG_DRIVER(
> -			"crtc:%d[%p], pflip_stat:AMDGPU_FLIP_PENDING, work: %p,\n",
> -			amdgpu_crtc->crtc_id, amdgpu_crtc, work);
> -
>   	amdgpu_flip_work_func(&work->flip_work.work);
> -}
> +	return 0;
>   
> -int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc,
> -			  struct drm_framebuffer *fb,
> -			  struct drm_pending_vblank_event *event,
> -			  uint32_t page_flip_flags,
> -			  uint32_t target)
> -{
> -	struct amdgpu_bo *new_abo;
> -	struct amdgpu_flip_work *work;
> -	int r = amdgpu_crtc_prepare_flip(
> -			crtc,
> -			fb,
> -			event,
> -			page_flip_flags,
> -			target,
> -			&work,
> -			&new_abo);
> -
> -	if (r)
> -		return r;
> -
> -	amdgpu_crtc_submit_flip(
> -			crtc,
> -			fb,
> -			work,
> -			new_abo);
> +pflip_cleanup:
> +	if (unlikely(amdgpu_bo_reserve(new_abo, false) != 0)) {

Well here we explicitly shouldn't use an interruptible wait, otherwise 
we would keep the BO pinned.

Christian.

> +		DRM_ERROR("failed to reserve new abo in error path\n");
> +		goto cleanup;
> +	}
> +unpin:
> +	if (unlikely(amdgpu_bo_unpin(new_abo) != 0)) {
> +		DRM_ERROR("failed to unpin new abo in error path\n");
> +	}
> +unreserve:
> +	amdgpu_bo_unreserve(new_abo);
>   
> -	return 0;
> +cleanup:
> +	amdgpu_bo_unref(&work->old_abo);
> +	fence_put(work->excl);
> +	for (i = 0; i < work->shared_count; ++i)
> +		fence_put(work->shared[i]);
> +	kfree(work->shared);
> +	kfree(work);
> +
> +	return r;
>   }
>   
>   int amdgpu_crtc_set_config(struct drm_mode_set *set)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index be3e8d66955e..9362a04eaa59 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -702,23 +702,6 @@ int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc,
>   				 struct drm_framebuffer *fb,
>   				 struct drm_pending_vblank_event *event,
>   				 uint32_t page_flip_flags, uint32_t target);
> -void amdgpu_crtc_cleanup_flip_ctx(
> -		struct amdgpu_flip_work *work,
> -		struct amdgpu_bo *new_abo);
> -int amdgpu_crtc_prepare_flip(struct drm_crtc *crtc,
> -			  struct drm_framebuffer *fb,
> -			  struct drm_pending_vblank_event *event,
> -			  uint32_t page_flip_flags,
> -			  uint32_t target,
> -			  struct amdgpu_flip_work **work,
> -			  struct amdgpu_bo **new_abo);
> -
> -void amdgpu_crtc_submit_flip(
> -		struct drm_crtc *crtc,
> -		struct drm_framebuffer *fb,
> -		struct amdgpu_flip_work *work,
> -		struct amdgpu_bo *new_abo);
> -
>   extern const struct drm_mode_config_funcs amdgpu_mode_funcs;
>   
>   #endif