Re: [PATCH 13/17] drm/radeon: Remove custom dma_fence_ops->wait implementation

On Sun, Apr 29, 2018 at 09:08:31AM +0200, Christian König wrote:

NAK, there is a subtitle but major difference:

...

• if (rdev->needs_reset) {
  

• 	t = -EDEADLK;
  

• 	break;
  

• }
  

Without that the whole radeon GPU reset code breaks.

Oops I've missed that. How does this work when you register a callback using ->enable_signaling and then block on it? Everything just dies?

We have lots of users of that for buffer/fence sharing. A really ugly, but probably working fix for this would be a kthread worker that just looks for ->needs_reset and force-completes all fences with dma_fence_set_error(-EIO), which is kinda what's supposed to happen here anyway. -Daniel

Regards, Christian.

Am 27.04.2018 um 08:17 schrieb Daniel Vetter:

...

It's a copy of dma_fence_default_wait, written slightly differently.

Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch Cc: Alex Deucher alexander.deucher@amd.com Cc: "Christian König" christian.koenig@amd.com Cc: "David (ChunMing) Zhou" David1.Zhou@amd.com Cc: amd-gfx@lists.freedesktop.org

---

drivers/gpu/drm/radeon/radeon_fence.c | 63 --------------------------- 1 file changed, 63 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c index e86f2bd38410..32690a525bfc 100644 --- a/drivers/gpu/drm/radeon/radeon_fence.c +++ b/drivers/gpu/drm/radeon/radeon_fence.c @@ -1051,72 +1051,9 @@ static const char *radeon_fence_get_timeline_name(struct dma_fence *f) } } -static inline bool radeon_test_signaled(struct radeon_fence *fence) -{

• return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->base.flags);

-}

-struct radeon_wait_cb {

• struct dma_fence_cb base;
• struct task_struct *task;

-};

-static void -radeon_fence_wait_cb(struct dma_fence *fence, struct dma_fence_cb *cb) -{

• struct radeon_wait_cb *wait =

• container_of(cb, struct radeon_wait_cb, base);
  

• wake_up_process(wait->task);

-}

-static signed long radeon_fence_default_wait(struct dma_fence *f, bool intr,

• 			     signed long t)
  

-{

• struct radeon_fence *fence = to_radeon_fence(f);
• struct radeon_device *rdev = fence->rdev;
• struct radeon_wait_cb cb;
• cb.task = current;
• if (dma_fence_add_callback(f, &cb.base, radeon_fence_wait_cb))

• return t;
  

• while (t > 0) {

• if (intr)
  

• 	set_current_state(TASK_INTERRUPTIBLE);
  

• else
  

• 	set_current_state(TASK_UNINTERRUPTIBLE);
  

• /*
  

•  * radeon_test_signaled must be called after
  

•  * set_current_state to prevent a race with wake_up_process
  

•  */
  

• if (radeon_test_signaled(fence))
  

• 	break;
  

• if (rdev->needs_reset) {
  

• 	t = -EDEADLK;
  

• 	break;
  

• }
  

• t = schedule_timeout(t);
  

• if (t > 0 && intr && signal_pending(current))
  

• 	t = -ERESTARTSYS;
  

• }
• __set_current_state(TASK_RUNNING);
• dma_fence_remove_callback(f, &cb.base);
• return t;

-}

• const struct dma_fence_ops radeon_fence_ops = { .get_driver_name = radeon_fence_get_driver_name, .get_timeline_name = radeon_fence_get_timeline_name, .enable_signaling = radeon_fence_enable_signaling, .signaled = radeon_fence_is_signaled,
• .wait = radeon_fence_default_wait,
• .release = NULL, };