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

[Christian König]

Am 13.01.2017 um 19:26 schrieb Alex Deucher:

From: Andrey Grodzovsky 

Make pflip atomic friendly. Split the fuinction into
whatever can fail part and the actual flip submit part.
Call the pre-submit function before atomic states
are swapped so in case of error we can fail the
IOCTL.

v2:
Update due to  target_vblank code change.
Fix identetation.
Change return type for amdgpu_crtc_submit_flip to void

Signed-off-by: Andrey Grodzovsky 
Reviewed-by Harry Wentland 
Signed-off-by: Alex Deucher 


The coding style on a couple of functions headers is incorrect, here for 
example:

void amdgpu_crtc_cleanup_flip_ctx(
struct amdgpu_flip_work *work,
struct amdgpu_bo *new_abo)


That should look like:

void amdgpu_crtc_cleanup_flip_ctx(struct amdgpu_flip_work *work,
  struct amdgpu_bo *new_abo)


Regards,
Christian.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

[Alex Deucher]

From: Andrey Grodzovsky 

Make pflip atomic friendly. Split the fuinction into
whatever can fail part and the actual flip submit part.
Call the pre-submit function before atomic states
are swapped so in case of error we can fail the
IOCTL.

v2:
Update due to  target_vblank code change.
Fix identetation.
Change return type for amdgpu_crtc_submit_flip to void

Signed-off-by: Andrey Grodzovsky 
Reviewed-by Harry Wentland 
Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 147 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h|  17 
 2 files changed, 135 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 4986340..d0ad619 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -138,10 +138,56 @@ static void amdgpu_unpin_work_func(struct work_struct 
*__work)
kfree(work);
 }
 
-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)
+
+static void amdgpu_flip_work_cleanup(struct amdgpu_flip_work *work)
+{
+   int i;
+
+   amdgpu_bo_unref(>old_abo);
+   dma_fence_put(work->excl);
+   for (i = 0; i < work->shared_count; ++i)
+   dma_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)
 {
struct drm_device *dev = crtc->dev;
struct amdgpu_device *adev = dev->dev_private;
@@ -154,7 +200,7 @@ int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc,
unsigned long flags;
u64 tiling_flags;
u64 base;
-   int i, r;
+   int r;
 
work = kzalloc(sizeof *work, GFP_KERNEL);
if (work == NULL)
@@ -215,41 +261,84 @@ int amdgpu_crtc_page_flip_target(struct drm_crtc *crtc,
spin_unlock_irqrestore(>dev->event_lock, flags);
r = -EBUSY;
goto pflip_cleanup;
-   }
 
-   amdgpu_crtc->pflip_status = AMDGPU_FLIP_PENDING;
-   amdgpu_crtc->pflip_works = work;
+   }
+   spin_unlock_irqrestore(>dev->event_lock, flags);
 
+   *work_p = work;
+   *new_abo_p = new_abo;
 
-   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(>dev->event_lock, flags);
-   amdgpu_flip_work_func(>flip_work.work);
return 0;
 
 pflip_cleanup:
-   if (unlikely(amdgpu_bo_reserve(new_abo, false) != 0)) {
-   DRM_ERROR("failed to reserve new abo in error path\n");
-   goto cleanup;
-   }
+   amdgpu_crtc_cleanup_flip_ctx(work, new_abo);
+   return r;
+
 unpin:
-   if (unlikely(amdgpu_bo_unpin(new_abo) != 0)) {
-   DRM_ERROR("failed to unpin new abo in error path\n");
-   }
+   amdgpu_flip_cleanup_unpin(work, new_abo);
+   return r;
+
 unreserve:
-   amdgpu_bo_unreserve(new_abo);
+   amdgpu_flip_cleanup_unreserve(work, new_abo);
+   return r;
 
 cleanup:
-   amdgpu_bo_unref(>old_abo);
-   dma_fence_put(work->excl);
-   for (i = 0; i < work->shared_count; ++i)
-