| Message ID | 1410957305-10190-7-git-send-email-maarten.lankhorst@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 17.09.2014 um 14:35 schrieb Maarten Lankhorst: > Not the whole world is a radeon! :-) > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com> > --- > drivers/gpu/drm/radeon/cik.c | 2 +- > drivers/gpu/drm/radeon/cik_sdma.c | 2 +- > drivers/gpu/drm/radeon/evergreen_dma.c | 2 +- > drivers/gpu/drm/radeon/r600.c | 2 +- > drivers/gpu/drm/radeon/r600_dma.c | 2 +- > drivers/gpu/drm/radeon/radeon.h | 7 ++++--- > drivers/gpu/drm/radeon/radeon_cs.c | 27 ++++++++++++++++++++------- > drivers/gpu/drm/radeon/radeon_fence.c | 3 +++ > drivers/gpu/drm/radeon/radeon_semaphore.c | 24 ++++++++++++++++++------ > drivers/gpu/drm/radeon/radeon_vm.c | 4 ++-- > drivers/gpu/drm/radeon/rv770_dma.c | 2 +- > drivers/gpu/drm/radeon/si_dma.c | 2 +- > 12 files changed, 54 insertions(+), 25 deletions(-) > > diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c > index 0d761f73a7fa..1440b6e9281e 100644 > --- a/drivers/gpu/drm/radeon/cik.c > +++ b/drivers/gpu/drm/radeon/cik.c > @@ -3993,7 +3993,7 @@ struct radeon_fence *cik_copy_cpdma(struct radeon_device *rdev, > return ERR_PTR(r); > } > > - radeon_semaphore_sync_resv(sem, resv, false); > + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); > radeon_semaphore_sync_rings(rdev, sem, ring->idx); > > for (i = 0; i < num_loops; i++) { > diff --git a/drivers/gpu/drm/radeon/cik_sdma.c b/drivers/gpu/drm/radeon/cik_sdma.c > index c01a6100c318..315c595418ec 100644 > --- a/drivers/gpu/drm/radeon/cik_sdma.c > +++ b/drivers/gpu/drm/radeon/cik_sdma.c > @@ -571,7 +571,7 @@ struct radeon_fence *cik_copy_dma(struct radeon_device *rdev, > return ERR_PTR(r); > } > > - radeon_semaphore_sync_resv(sem, resv, false); > + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); > radeon_semaphore_sync_rings(rdev, sem, ring->idx); > > for (i = 0; i < num_loops; i++) { > diff --git a/drivers/gpu/drm/radeon/evergreen_dma.c b/drivers/gpu/drm/radeon/evergreen_dma.c > index 946f37d0b469..5a5686792068 100644 > --- a/drivers/gpu/drm/radeon/evergreen_dma.c > +++ b/drivers/gpu/drm/radeon/evergreen_dma.c > @@ -133,7 +133,7 @@ struct radeon_fence *evergreen_copy_dma(struct radeon_device *rdev, > return ERR_PTR(r); > } > > - radeon_semaphore_sync_resv(sem, resv, false); > + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); > radeon_semaphore_sync_rings(rdev, sem, ring->idx); > > for (i = 0; i < num_loops; i++) { > diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c > index 25f367ac4637..35c22ee9bc4a 100644 > --- a/drivers/gpu/drm/radeon/r600.c > +++ b/drivers/gpu/drm/radeon/r600.c > @@ -2912,7 +2912,7 @@ struct radeon_fence *r600_copy_cpdma(struct radeon_device *rdev, > return ERR_PTR(r); > } > > - radeon_semaphore_sync_resv(sem, resv, false); > + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); > radeon_semaphore_sync_rings(rdev, sem, ring->idx); > > radeon_ring_write(ring, PACKET3(PACKET3_SET_CONFIG_REG, 1)); > diff --git a/drivers/gpu/drm/radeon/r600_dma.c b/drivers/gpu/drm/radeon/r600_dma.c > index fc54224ce87b..674af8db7a35 100644 > --- a/drivers/gpu/drm/radeon/r600_dma.c > +++ b/drivers/gpu/drm/radeon/r600_dma.c > @@ -470,7 +470,7 @@ struct radeon_fence *r600_copy_dma(struct radeon_device *rdev, > return ERR_PTR(r); > } > > - radeon_semaphore_sync_resv(sem, resv, false); > + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); > radeon_semaphore_sync_rings(rdev, sem, ring->idx); > > for (i = 0; i < num_loops; i++) { > diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h > index 9aa75c1af4f4..6cdc5e62fe12 100644 > --- a/drivers/gpu/drm/radeon/radeon.h > +++ b/drivers/gpu/drm/radeon/radeon.h > @@ -587,9 +587,10 @@ bool radeon_semaphore_emit_wait(struct radeon_device *rdev, int ring, > struct radeon_semaphore *semaphore); > void radeon_semaphore_sync_fence(struct radeon_semaphore *semaphore, > struct radeon_fence *fence); > -void radeon_semaphore_sync_resv(struct radeon_semaphore *semaphore, > - struct reservation_object *resv, > - bool shared); > +int radeon_semaphore_sync_resv(struct radeon_device *rdev, > + struct radeon_semaphore *semaphore, > + struct reservation_object *resv, > + bool shared, bool intr); > int radeon_semaphore_sync_rings(struct radeon_device *rdev, > struct radeon_semaphore *semaphore, > int waiting_ring); > diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c > index f662de41ba49..a4a608c8b1ba 100644 > --- a/drivers/gpu/drm/radeon/radeon_cs.c > +++ b/drivers/gpu/drm/radeon/radeon_cs.c > @@ -249,20 +249,21 @@ static int radeon_cs_get_ring(struct radeon_cs_parser *p, u32 ring, s32 priority > return 0; > } > > -static void radeon_cs_sync_rings(struct radeon_cs_parser *p) > +static int radeon_cs_sync_rings(struct radeon_cs_parser *p) > { > - int i; > + int i, r = 0; > > - for (i = 0; i < p->nrelocs; i++) { > + for (i = 0; !r && i < p->nrelocs; i++) { > struct reservation_object *resv; > > if (!p->relocs[i].robj) > continue; > > resv = p->relocs[i].robj->tbo.resv; > - radeon_semaphore_sync_resv(p->ib.semaphore, resv, > - p->relocs[i].tv.shared); > + r = radeon_semaphore_sync_resv(p->rdev, p->ib.semaphore, resv, > + p->relocs[i].tv.shared, true); Break on the first error? > } > + return r; > } > > /* XXX: note that this is called from the legacy UMS CS ioctl as well */ > @@ -472,13 +473,19 @@ static int radeon_cs_ib_chunk(struct radeon_device *rdev, > return r; > } > > + r = radeon_cs_sync_rings(parser); > + if (r) { > + if (r != -ERESTARTSYS) > + DRM_ERROR("Failed to sync rings: %i\n", r); > + return r; > + } > + > if (parser->ring == R600_RING_TYPE_UVD_INDEX) > radeon_uvd_note_usage(rdev); > else if ((parser->ring == TN_RING_TYPE_VCE1_INDEX) || > (parser->ring == TN_RING_TYPE_VCE2_INDEX)) > radeon_vce_note_usage(rdev); > > - radeon_cs_sync_rings(parser); > r = radeon_ib_schedule(rdev, &parser->ib, NULL, true); > if (r) { > DRM_ERROR("Failed to schedule IB !\n"); > @@ -565,7 +572,13 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev, > if (r) { > goto out; > } > - radeon_cs_sync_rings(parser); > + > + r = radeon_cs_sync_rings(parser); > + if (r) { > + if (r != -ERESTARTSYS) > + DRM_ERROR("Failed to sync rings: %i\n", r); > + goto out; > + } > radeon_semaphore_sync_fence(parser->ib.semaphore, vm->fence); > > if ((rdev->family >= CHIP_TAHITI) && > diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c > index af9f2d6bd7d0..0262fe2580d2 100644 > --- a/drivers/gpu/drm/radeon/radeon_fence.c > +++ b/drivers/gpu/drm/radeon/radeon_fence.c > @@ -541,6 +541,9 @@ int radeon_fence_wait(struct radeon_fence *fence, bool intr) > uint64_t seq[RADEON_NUM_RINGS] = {}; > long r; > > + if (unlikely(!to_radeon_fence(&fence->base))) > + return fence_wait(&fence->base, intr); > + > seq[fence->ring] = fence->seq; > r = radeon_fence_wait_seq_timeout(fence->rdev, seq, intr, MAX_SCHEDULE_TIMEOUT); > if (r < 0) { > diff --git a/drivers/gpu/drm/radeon/radeon_semaphore.c b/drivers/gpu/drm/radeon/radeon_semaphore.c > index 4d4b0773638a..68311da39c09 100644 > --- a/drivers/gpu/drm/radeon/radeon_semaphore.c > +++ b/drivers/gpu/drm/radeon/radeon_semaphore.c > @@ -124,27 +124,39 @@ void radeon_semaphore_sync_fence(struct radeon_semaphore *semaphore, > * > * Sync to the fence using this semaphore object > */ > -void radeon_semaphore_sync_resv(struct radeon_semaphore *sema, > - struct reservation_object *resv, > - bool shared) > +int radeon_semaphore_sync_resv(struct radeon_device *rdev, > + struct radeon_semaphore *sema, > + struct reservation_object *resv, > + bool shared, bool intr) > { > struct reservation_object_list *flist; > struct fence *f; > + struct radeon_fence *fence; > unsigned i; > + int r = 0; > > /* always sync to the exclusive fence */ > f = reservation_object_get_excl(resv); > - radeon_semaphore_sync_fence(sema, (struct radeon_fence*)f); > + fence = f ? to_radeon_fence(f) : NULL; > + if (fence && fence->rdev == rdev) > + radeon_semaphore_sync_fence(sema, fence); > + else if (f) > + r = fence_wait(f, intr); Error handling? If anything goes wrong while waiting on the exclusive fence we would overwrite the error with the result of the shared fences. > > flist = reservation_object_get_list(resv); > if (shared || !flist) > - return; > + return r; > > for (i = 0; i < flist->shared_count; ++i) { > f = rcu_dereference_protected(flist->shared[i], > reservation_object_held(resv)); > - radeon_semaphore_sync_fence(sema, (struct radeon_fence*)f); > + fence = to_radeon_fence(f); > + if (fence && fence->rdev == rdev) > + radeon_semaphore_sync_fence(sema, fence); > + else if (!r) > + r = fence_wait(f, intr); Again break on the first error. > } Maybe print the error that something went wrong here instead. Apart from that approach look good to me, Christian. > + return r; > } > > /** > diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c > index ce870959dff8..f6e76c18d7eb 100644 > --- a/drivers/gpu/drm/radeon/radeon_vm.c > +++ b/drivers/gpu/drm/radeon/radeon_vm.c > @@ -698,7 +698,7 @@ int radeon_vm_update_page_directory(struct radeon_device *rdev, > if (ib.length_dw != 0) { > radeon_asic_vm_pad_ib(rdev, &ib); > > - radeon_semaphore_sync_resv(ib.semaphore, pd->tbo.resv, false); > + radeon_semaphore_sync_resv(rdev, ib.semaphore, pd->tbo.resv, false, false); > radeon_semaphore_sync_fence(ib.semaphore, vm->last_id_use); > WARN_ON(ib.length_dw > ndw); > r = radeon_ib_schedule(rdev, &ib, NULL, false); > @@ -825,7 +825,7 @@ static void radeon_vm_update_ptes(struct radeon_device *rdev, > unsigned nptes; > uint64_t pte; > > - radeon_semaphore_sync_resv(ib->semaphore, pt->tbo.resv, false); > + radeon_semaphore_sync_resv(rdev, ib->semaphore, pt->tbo.resv, false, false); > > if ((addr & ~mask) == (end & ~mask)) > nptes = end - addr; > diff --git a/drivers/gpu/drm/radeon/rv770_dma.c b/drivers/gpu/drm/radeon/rv770_dma.c > index c112764adfdf..6ac25027d506 100644 > --- a/drivers/gpu/drm/radeon/rv770_dma.c > +++ b/drivers/gpu/drm/radeon/rv770_dma.c > @@ -67,7 +67,7 @@ struct radeon_fence *rv770_copy_dma(struct radeon_device *rdev, > return ERR_PTR(r); > } > > - radeon_semaphore_sync_resv(sem, resv, false); > + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); > radeon_semaphore_sync_rings(rdev, sem, ring->idx); > > for (i = 0; i < num_loops; i++) { > diff --git a/drivers/gpu/drm/radeon/si_dma.c b/drivers/gpu/drm/radeon/si_dma.c > index 9b0dfbc913f3..381ce21dea2d 100644 > --- a/drivers/gpu/drm/radeon/si_dma.c > +++ b/drivers/gpu/drm/radeon/si_dma.c > @@ -252,7 +252,7 @@ struct radeon_fence *si_copy_dma(struct radeon_device *rdev, > return ERR_PTR(r); > } > > - radeon_semaphore_sync_resv(sem, resv, false); > + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); > radeon_semaphore_sync_rings(rdev, sem, ring->idx); > > for (i = 0; i < num_loops; i++) {
On 17.09.2014 21:35, Maarten Lankhorst wrote: > diff --git a/drivers/gpu/drm/radeon/radeon_semaphore.c b/drivers/gpu/drm/radeon/radeon_semaphore.c > index 4d4b0773638a..68311da39c09 100644 > --- a/drivers/gpu/drm/radeon/radeon_semaphore.c > +++ b/drivers/gpu/drm/radeon/radeon_semaphore.c > @@ -124,27 +124,39 @@ void radeon_semaphore_sync_fence(struct radeon_semaphore *semaphore, > * > * Sync to the fence using this semaphore object > */ > -void radeon_semaphore_sync_resv(struct radeon_semaphore *sema, > - struct reservation_object *resv, > - bool shared) > +int radeon_semaphore_sync_resv(struct radeon_device *rdev, > + struct radeon_semaphore *sema, > + struct reservation_object *resv, > + bool shared, bool intr) The callers of this function would be more readable if it took flags instead of the shared and intr bools.
Hey, Op 18-09-14 om 05:26 schreef Michel Dänzer: > On 17.09.2014 21:35, Maarten Lankhorst wrote: >> diff --git a/drivers/gpu/drm/radeon/radeon_semaphore.c b/drivers/gpu/drm/radeon/radeon_semaphore.c >> index 4d4b0773638a..68311da39c09 100644 >> --- a/drivers/gpu/drm/radeon/radeon_semaphore.c >> +++ b/drivers/gpu/drm/radeon/radeon_semaphore.c >> @@ -124,27 +124,39 @@ void radeon_semaphore_sync_fence(struct radeon_semaphore *semaphore, >> * >> * Sync to the fence using this semaphore object >> */ >> -void radeon_semaphore_sync_resv(struct radeon_semaphore *sema, >> - struct reservation_object *resv, >> - bool shared) >> +int radeon_semaphore_sync_resv(struct radeon_device *rdev, >> + struct radeon_semaphore *sema, >> + struct reservation_object *resv, >> + bool shared, bool intr) > > The callers of this function would be more readable if it took flags instead of the shared and intr bools. This does not match the rest of the TTM design. Things like ttm_bo_reserve take separate bools, not flags. ~Maarten
On 18.09.2014 19:22, Maarten Lankhorst wrote: > Op 18-09-14 om 05:26 schreef Michel Dänzer: >> On 17.09.2014 21:35, Maarten Lankhorst wrote: >>> diff --git a/drivers/gpu/drm/radeon/radeon_semaphore.c b/drivers/gpu/drm/radeon/radeon_semaphore.c >>> index 4d4b0773638a..68311da39c09 100644 >>> --- a/drivers/gpu/drm/radeon/radeon_semaphore.c >>> +++ b/drivers/gpu/drm/radeon/radeon_semaphore.c >>> @@ -124,27 +124,39 @@ void radeon_semaphore_sync_fence(struct radeon_semaphore *semaphore, >>> * >>> * Sync to the fence using this semaphore object >>> */ >>> -void radeon_semaphore_sync_resv(struct radeon_semaphore *sema, >>> - struct reservation_object *resv, >>> - bool shared) >>> +int radeon_semaphore_sync_resv(struct radeon_device *rdev, >>> + struct radeon_semaphore *sema, >>> + struct reservation_object *resv, >>> + bool shared, bool intr) >> >> The callers of this function would be more readable if it took flags instead of the shared and intr bools. > This does not match the rest of the TTM design. Things like > ttm_bo_reserve take separate bools, not flags. So? :)
Op 17-09-14 om 15:09 schreef Christian König: > Am 17.09.2014 um 14:35 schrieb Maarten Lankhorst: >> Not the whole world is a radeon! :-) >> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com> >> --- >> drivers/gpu/drm/radeon/cik.c | 2 +- >> drivers/gpu/drm/radeon/cik_sdma.c | 2 +- >> drivers/gpu/drm/radeon/evergreen_dma.c | 2 +- >> drivers/gpu/drm/radeon/r600.c | 2 +- >> drivers/gpu/drm/radeon/r600_dma.c | 2 +- >> drivers/gpu/drm/radeon/radeon.h | 7 ++++--- >> drivers/gpu/drm/radeon/radeon_cs.c | 27 ++++++++++++++++++++------- >> drivers/gpu/drm/radeon/radeon_fence.c | 3 +++ >> drivers/gpu/drm/radeon/radeon_semaphore.c | 24 ++++++++++++++++++------ >> drivers/gpu/drm/radeon/radeon_vm.c | 4 ++-- >> drivers/gpu/drm/radeon/rv770_dma.c | 2 +- >> drivers/gpu/drm/radeon/si_dma.c | 2 +- >> 12 files changed, 54 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c >> index 0d761f73a7fa..1440b6e9281e 100644 >> --- a/drivers/gpu/drm/radeon/cik.c >> +++ b/drivers/gpu/drm/radeon/cik.c >> @@ -3993,7 +3993,7 @@ struct radeon_fence *cik_copy_cpdma(struct radeon_device *rdev, >> return ERR_PTR(r); >> } >> - radeon_semaphore_sync_resv(sem, resv, false); >> + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); >> radeon_semaphore_sync_rings(rdev, sem, ring->idx); >> for (i = 0; i < num_loops; i++) { >> diff --git a/drivers/gpu/drm/radeon/cik_sdma.c b/drivers/gpu/drm/radeon/cik_sdma.c >> index c01a6100c318..315c595418ec 100644 >> --- a/drivers/gpu/drm/radeon/cik_sdma.c >> +++ b/drivers/gpu/drm/radeon/cik_sdma.c >> @@ -571,7 +571,7 @@ struct radeon_fence *cik_copy_dma(struct radeon_device *rdev, >> return ERR_PTR(r); >> } >> - radeon_semaphore_sync_resv(sem, resv, false); >> + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); >> radeon_semaphore_sync_rings(rdev, sem, ring->idx); >> for (i = 0; i < num_loops; i++) { >> diff --git a/drivers/gpu/drm/radeon/evergreen_dma.c b/drivers/gpu/drm/radeon/evergreen_dma.c >> index 946f37d0b469..5a5686792068 100644 >> --- a/drivers/gpu/drm/radeon/evergreen_dma.c >> +++ b/drivers/gpu/drm/radeon/evergreen_dma.c >> @@ -133,7 +133,7 @@ struct radeon_fence *evergreen_copy_dma(struct radeon_device *rdev, >> return ERR_PTR(r); >> } >> - radeon_semaphore_sync_resv(sem, resv, false); >> + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); >> radeon_semaphore_sync_rings(rdev, sem, ring->idx); >> for (i = 0; i < num_loops; i++) { >> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c >> index 25f367ac4637..35c22ee9bc4a 100644 >> --- a/drivers/gpu/drm/radeon/r600.c >> +++ b/drivers/gpu/drm/radeon/r600.c >> @@ -2912,7 +2912,7 @@ struct radeon_fence *r600_copy_cpdma(struct radeon_device *rdev, >> return ERR_PTR(r); >> } >> - radeon_semaphore_sync_resv(sem, resv, false); >> + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); >> radeon_semaphore_sync_rings(rdev, sem, ring->idx); >> radeon_ring_write(ring, PACKET3(PACKET3_SET_CONFIG_REG, 1)); >> diff --git a/drivers/gpu/drm/radeon/r600_dma.c b/drivers/gpu/drm/radeon/r600_dma.c >> index fc54224ce87b..674af8db7a35 100644 >> --- a/drivers/gpu/drm/radeon/r600_dma.c >> +++ b/drivers/gpu/drm/radeon/r600_dma.c >> @@ -470,7 +470,7 @@ struct radeon_fence *r600_copy_dma(struct radeon_device *rdev, >> return ERR_PTR(r); >> } >> - radeon_semaphore_sync_resv(sem, resv, false); >> + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); >> radeon_semaphore_sync_rings(rdev, sem, ring->idx); >> for (i = 0; i < num_loops; i++) { >> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h >> index 9aa75c1af4f4..6cdc5e62fe12 100644 >> --- a/drivers/gpu/drm/radeon/radeon.h >> +++ b/drivers/gpu/drm/radeon/radeon.h >> @@ -587,9 +587,10 @@ bool radeon_semaphore_emit_wait(struct radeon_device *rdev, int ring, >> struct radeon_semaphore *semaphore); >> void radeon_semaphore_sync_fence(struct radeon_semaphore *semaphore, >> struct radeon_fence *fence); >> -void radeon_semaphore_sync_resv(struct radeon_semaphore *semaphore, >> - struct reservation_object *resv, >> - bool shared); >> +int radeon_semaphore_sync_resv(struct radeon_device *rdev, >> + struct radeon_semaphore *semaphore, >> + struct reservation_object *resv, >> + bool shared, bool intr); >> int radeon_semaphore_sync_rings(struct radeon_device *rdev, >> struct radeon_semaphore *semaphore, >> int waiting_ring); >> diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c >> index f662de41ba49..a4a608c8b1ba 100644 >> --- a/drivers/gpu/drm/radeon/radeon_cs.c >> +++ b/drivers/gpu/drm/radeon/radeon_cs.c >> @@ -249,20 +249,21 @@ static int radeon_cs_get_ring(struct radeon_cs_parser *p, u32 ring, s32 priority >> return 0; >> } >> -static void radeon_cs_sync_rings(struct radeon_cs_parser *p) >> +static int radeon_cs_sync_rings(struct radeon_cs_parser *p) >> { >> - int i; >> + int i, r = 0; >> - for (i = 0; i < p->nrelocs; i++) { >> + for (i = 0; !r && i < p->nrelocs; i++) { >> struct reservation_object *resv; >> if (!p->relocs[i].robj) >> continue; >> resv = p->relocs[i].robj->tbo.resv; >> - radeon_semaphore_sync_resv(p->ib.semaphore, resv, >> - p->relocs[i].tv.shared); >> + r = radeon_semaphore_sync_resv(p->rdev, p->ib.semaphore, resv, >> + p->relocs[i].tv.shared, true); > > Break on the first error? It does, see the !r in the loop. >> } >> + return r; >> } >> /* XXX: note that this is called from the legacy UMS CS ioctl as well */ >> @@ -472,13 +473,19 @@ static int radeon_cs_ib_chunk(struct radeon_device *rdev, >> return r; >> } >> + r = radeon_cs_sync_rings(parser); >> + if (r) { >> + if (r != -ERESTARTSYS) >> + DRM_ERROR("Failed to sync rings: %i\n", r); >> + return r; >> + } >> + >> if (parser->ring == R600_RING_TYPE_UVD_INDEX) >> radeon_uvd_note_usage(rdev); >> else if ((parser->ring == TN_RING_TYPE_VCE1_INDEX) || >> (parser->ring == TN_RING_TYPE_VCE2_INDEX)) >> radeon_vce_note_usage(rdev); >> - radeon_cs_sync_rings(parser); >> r = radeon_ib_schedule(rdev, &parser->ib, NULL, true); >> if (r) { >> DRM_ERROR("Failed to schedule IB !\n"); >> @@ -565,7 +572,13 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev, >> if (r) { >> goto out; >> } >> - radeon_cs_sync_rings(parser); >> + >> + r = radeon_cs_sync_rings(parser); >> + if (r) { >> + if (r != -ERESTARTSYS) >> + DRM_ERROR("Failed to sync rings: %i\n", r); >> + goto out; >> + } >> radeon_semaphore_sync_fence(parser->ib.semaphore, vm->fence); >> if ((rdev->family >= CHIP_TAHITI) && >> diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c >> index af9f2d6bd7d0..0262fe2580d2 100644 >> --- a/drivers/gpu/drm/radeon/radeon_fence.c >> +++ b/drivers/gpu/drm/radeon/radeon_fence.c >> @@ -541,6 +541,9 @@ int radeon_fence_wait(struct radeon_fence *fence, bool intr) >> uint64_t seq[RADEON_NUM_RINGS] = {}; >> long r; >> + if (unlikely(!to_radeon_fence(&fence->base))) >> + return fence_wait(&fence->base, intr); >> + >> seq[fence->ring] = fence->seq; >> r = radeon_fence_wait_seq_timeout(fence->rdev, seq, intr, MAX_SCHEDULE_TIMEOUT); >> if (r < 0) { >> diff --git a/drivers/gpu/drm/radeon/radeon_semaphore.c b/drivers/gpu/drm/radeon/radeon_semaphore.c >> index 4d4b0773638a..68311da39c09 100644 >> --- a/drivers/gpu/drm/radeon/radeon_semaphore.c >> +++ b/drivers/gpu/drm/radeon/radeon_semaphore.c >> @@ -124,27 +124,39 @@ void radeon_semaphore_sync_fence(struct radeon_semaphore *semaphore, >> * >> * Sync to the fence using this semaphore object >> */ >> -void radeon_semaphore_sync_resv(struct radeon_semaphore *sema, >> - struct reservation_object *resv, >> - bool shared) >> +int radeon_semaphore_sync_resv(struct radeon_device *rdev, >> + struct radeon_semaphore *sema, >> + struct reservation_object *resv, >> + bool shared, bool intr) >> { >> struct reservation_object_list *flist; >> struct fence *f; >> + struct radeon_fence *fence; >> unsigned i; >> + int r = 0; >> /* always sync to the exclusive fence */ >> f = reservation_object_get_excl(resv); >> - radeon_semaphore_sync_fence(sema, (struct radeon_fence*)f); >> + fence = f ? to_radeon_fence(f) : NULL; >> + if (fence && fence->rdev == rdev) >> + radeon_semaphore_sync_fence(sema, fence); >> + else if (f) >> + r = fence_wait(f, intr); > > Error handling? If anything goes wrong while waiting on the exclusive fence we would overwrite the error with the result of the shared fences. The if (!r) fence_wait in the shared fences takes care of that. >> flist = reservation_object_get_list(resv); >> if (shared || !flist) >> - return; >> + return r; >> for (i = 0; i < flist->shared_count; ++i) { >> f = rcu_dereference_protected(flist->shared[i], >> reservation_object_held(resv)); >> - radeon_semaphore_sync_fence(sema, (struct radeon_fence*)f); >> + fence = to_radeon_fence(f); >> + if (fence && fence->rdev == rdev) >> + radeon_semaphore_sync_fence(sema, fence); >> + else if (!r) >> + r = fence_wait(f, intr); > > Again break on the first error. Not all the functions check for errors, so if the wait fails I wanted to sync as much as possible. But investigating them it looks like the places that don't check for errors are the .blit, .copy and radeon_vm calls, Those bo's cannot be exported as dma-buf, so I think the resv call can't fail in that case, and I can return immediately if the calls fails after all. I should be able to drop the intr parameter too for now, because it means the interruptible wait can be the default here. ~Maarten
diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c index 0d761f73a7fa..1440b6e9281e 100644 --- a/drivers/gpu/drm/radeon/cik.c +++ b/drivers/gpu/drm/radeon/cik.c @@ -3993,7 +3993,7 @@ struct radeon_fence *cik_copy_cpdma(struct radeon_device *rdev, return ERR_PTR(r); } - radeon_semaphore_sync_resv(sem, resv, false); + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); radeon_semaphore_sync_rings(rdev, sem, ring->idx); for (i = 0; i < num_loops; i++) { diff --git a/drivers/gpu/drm/radeon/cik_sdma.c b/drivers/gpu/drm/radeon/cik_sdma.c index c01a6100c318..315c595418ec 100644 --- a/drivers/gpu/drm/radeon/cik_sdma.c +++ b/drivers/gpu/drm/radeon/cik_sdma.c @@ -571,7 +571,7 @@ struct radeon_fence *cik_copy_dma(struct radeon_device *rdev, return ERR_PTR(r); } - radeon_semaphore_sync_resv(sem, resv, false); + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); radeon_semaphore_sync_rings(rdev, sem, ring->idx); for (i = 0; i < num_loops; i++) { diff --git a/drivers/gpu/drm/radeon/evergreen_dma.c b/drivers/gpu/drm/radeon/evergreen_dma.c index 946f37d0b469..5a5686792068 100644 --- a/drivers/gpu/drm/radeon/evergreen_dma.c +++ b/drivers/gpu/drm/radeon/evergreen_dma.c @@ -133,7 +133,7 @@ struct radeon_fence *evergreen_copy_dma(struct radeon_device *rdev, return ERR_PTR(r); } - radeon_semaphore_sync_resv(sem, resv, false); + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); radeon_semaphore_sync_rings(rdev, sem, ring->idx); for (i = 0; i < num_loops; i++) { diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c index 25f367ac4637..35c22ee9bc4a 100644 --- a/drivers/gpu/drm/radeon/r600.c +++ b/drivers/gpu/drm/radeon/r600.c @@ -2912,7 +2912,7 @@ struct radeon_fence *r600_copy_cpdma(struct radeon_device *rdev, return ERR_PTR(r); } - radeon_semaphore_sync_resv(sem, resv, false); + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); radeon_semaphore_sync_rings(rdev, sem, ring->idx); radeon_ring_write(ring, PACKET3(PACKET3_SET_CONFIG_REG, 1)); diff --git a/drivers/gpu/drm/radeon/r600_dma.c b/drivers/gpu/drm/radeon/r600_dma.c index fc54224ce87b..674af8db7a35 100644 --- a/drivers/gpu/drm/radeon/r600_dma.c +++ b/drivers/gpu/drm/radeon/r600_dma.c @@ -470,7 +470,7 @@ struct radeon_fence *r600_copy_dma(struct radeon_device *rdev, return ERR_PTR(r); } - radeon_semaphore_sync_resv(sem, resv, false); + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); radeon_semaphore_sync_rings(rdev, sem, ring->idx); for (i = 0; i < num_loops; i++) { diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 9aa75c1af4f4..6cdc5e62fe12 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -587,9 +587,10 @@ bool radeon_semaphore_emit_wait(struct radeon_device *rdev, int ring, struct radeon_semaphore *semaphore); void radeon_semaphore_sync_fence(struct radeon_semaphore *semaphore, struct radeon_fence *fence); -void radeon_semaphore_sync_resv(struct radeon_semaphore *semaphore, - struct reservation_object *resv, - bool shared); +int radeon_semaphore_sync_resv(struct radeon_device *rdev, + struct radeon_semaphore *semaphore, + struct reservation_object *resv, + bool shared, bool intr); int radeon_semaphore_sync_rings(struct radeon_device *rdev, struct radeon_semaphore *semaphore, int waiting_ring); diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index f662de41ba49..a4a608c8b1ba 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -249,20 +249,21 @@ static int radeon_cs_get_ring(struct radeon_cs_parser *p, u32 ring, s32 priority return 0; } -static void radeon_cs_sync_rings(struct radeon_cs_parser *p) +static int radeon_cs_sync_rings(struct radeon_cs_parser *p) { - int i; + int i, r = 0; - for (i = 0; i < p->nrelocs; i++) { + for (i = 0; !r && i < p->nrelocs; i++) { struct reservation_object *resv; if (!p->relocs[i].robj) continue; resv = p->relocs[i].robj->tbo.resv; - radeon_semaphore_sync_resv(p->ib.semaphore, resv, - p->relocs[i].tv.shared); + r = radeon_semaphore_sync_resv(p->rdev, p->ib.semaphore, resv, + p->relocs[i].tv.shared, true); } + return r; } /* XXX: note that this is called from the legacy UMS CS ioctl as well */ @@ -472,13 +473,19 @@ static int radeon_cs_ib_chunk(struct radeon_device *rdev, return r; } + r = radeon_cs_sync_rings(parser); + if (r) { + if (r != -ERESTARTSYS) + DRM_ERROR("Failed to sync rings: %i\n", r); + return r; + } + if (parser->ring == R600_RING_TYPE_UVD_INDEX) radeon_uvd_note_usage(rdev); else if ((parser->ring == TN_RING_TYPE_VCE1_INDEX) || (parser->ring == TN_RING_TYPE_VCE2_INDEX)) radeon_vce_note_usage(rdev); - radeon_cs_sync_rings(parser); r = radeon_ib_schedule(rdev, &parser->ib, NULL, true); if (r) { DRM_ERROR("Failed to schedule IB !\n"); @@ -565,7 +572,13 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev, if (r) { goto out; } - radeon_cs_sync_rings(parser); + + r = radeon_cs_sync_rings(parser); + if (r) { + if (r != -ERESTARTSYS) + DRM_ERROR("Failed to sync rings: %i\n", r); + goto out; + } radeon_semaphore_sync_fence(parser->ib.semaphore, vm->fence); if ((rdev->family >= CHIP_TAHITI) && diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c index af9f2d6bd7d0..0262fe2580d2 100644 --- a/drivers/gpu/drm/radeon/radeon_fence.c +++ b/drivers/gpu/drm/radeon/radeon_fence.c @@ -541,6 +541,9 @@ int radeon_fence_wait(struct radeon_fence *fence, bool intr) uint64_t seq[RADEON_NUM_RINGS] = {}; long r; + if (unlikely(!to_radeon_fence(&fence->base))) + return fence_wait(&fence->base, intr); + seq[fence->ring] = fence->seq; r = radeon_fence_wait_seq_timeout(fence->rdev, seq, intr, MAX_SCHEDULE_TIMEOUT); if (r < 0) { diff --git a/drivers/gpu/drm/radeon/radeon_semaphore.c b/drivers/gpu/drm/radeon/radeon_semaphore.c index 4d4b0773638a..68311da39c09 100644 --- a/drivers/gpu/drm/radeon/radeon_semaphore.c +++ b/drivers/gpu/drm/radeon/radeon_semaphore.c @@ -124,27 +124,39 @@ void radeon_semaphore_sync_fence(struct radeon_semaphore *semaphore, * * Sync to the fence using this semaphore object */ -void radeon_semaphore_sync_resv(struct radeon_semaphore *sema, - struct reservation_object *resv, - bool shared) +int radeon_semaphore_sync_resv(struct radeon_device *rdev, + struct radeon_semaphore *sema, + struct reservation_object *resv, + bool shared, bool intr) { struct reservation_object_list *flist; struct fence *f; + struct radeon_fence *fence; unsigned i; + int r = 0; /* always sync to the exclusive fence */ f = reservation_object_get_excl(resv); - radeon_semaphore_sync_fence(sema, (struct radeon_fence*)f); + fence = f ? to_radeon_fence(f) : NULL; + if (fence && fence->rdev == rdev) + radeon_semaphore_sync_fence(sema, fence); + else if (f) + r = fence_wait(f, intr); flist = reservation_object_get_list(resv); if (shared || !flist) - return; + return r; for (i = 0; i < flist->shared_count; ++i) { f = rcu_dereference_protected(flist->shared[i], reservation_object_held(resv)); - radeon_semaphore_sync_fence(sema, (struct radeon_fence*)f); + fence = to_radeon_fence(f); + if (fence && fence->rdev == rdev) + radeon_semaphore_sync_fence(sema, fence); + else if (!r) + r = fence_wait(f, intr); } + return r; } /** diff --git a/drivers/gpu/drm/radeon/radeon_vm.c b/drivers/gpu/drm/radeon/radeon_vm.c index ce870959dff8..f6e76c18d7eb 100644 --- a/drivers/gpu/drm/radeon/radeon_vm.c +++ b/drivers/gpu/drm/radeon/radeon_vm.c @@ -698,7 +698,7 @@ int radeon_vm_update_page_directory(struct radeon_device *rdev, if (ib.length_dw != 0) { radeon_asic_vm_pad_ib(rdev, &ib); - radeon_semaphore_sync_resv(ib.semaphore, pd->tbo.resv, false); + radeon_semaphore_sync_resv(rdev, ib.semaphore, pd->tbo.resv, false, false); radeon_semaphore_sync_fence(ib.semaphore, vm->last_id_use); WARN_ON(ib.length_dw > ndw); r = radeon_ib_schedule(rdev, &ib, NULL, false); @@ -825,7 +825,7 @@ static void radeon_vm_update_ptes(struct radeon_device *rdev, unsigned nptes; uint64_t pte; - radeon_semaphore_sync_resv(ib->semaphore, pt->tbo.resv, false); + radeon_semaphore_sync_resv(rdev, ib->semaphore, pt->tbo.resv, false, false); if ((addr & ~mask) == (end & ~mask)) nptes = end - addr; diff --git a/drivers/gpu/drm/radeon/rv770_dma.c b/drivers/gpu/drm/radeon/rv770_dma.c index c112764adfdf..6ac25027d506 100644 --- a/drivers/gpu/drm/radeon/rv770_dma.c +++ b/drivers/gpu/drm/radeon/rv770_dma.c @@ -67,7 +67,7 @@ struct radeon_fence *rv770_copy_dma(struct radeon_device *rdev, return ERR_PTR(r); } - radeon_semaphore_sync_resv(sem, resv, false); + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); radeon_semaphore_sync_rings(rdev, sem, ring->idx); for (i = 0; i < num_loops; i++) { diff --git a/drivers/gpu/drm/radeon/si_dma.c b/drivers/gpu/drm/radeon/si_dma.c index 9b0dfbc913f3..381ce21dea2d 100644 --- a/drivers/gpu/drm/radeon/si_dma.c +++ b/drivers/gpu/drm/radeon/si_dma.c @@ -252,7 +252,7 @@ struct radeon_fence *si_copy_dma(struct radeon_device *rdev, return ERR_PTR(r); } - radeon_semaphore_sync_resv(sem, resv, false); + radeon_semaphore_sync_resv(rdev, sem, resv, false, false); radeon_semaphore_sync_rings(rdev, sem, ring->idx); for (i = 0; i < num_loops; i++) {
Not the whole world is a radeon! :-) Signed-off-by: Maarten Lankhorst <maarten.lankhorst@canonical.com> --- drivers/gpu/drm/radeon/cik.c | 2 +- drivers/gpu/drm/radeon/cik_sdma.c | 2 +- drivers/gpu/drm/radeon/evergreen_dma.c | 2 +- drivers/gpu/drm/radeon/r600.c | 2 +- drivers/gpu/drm/radeon/r600_dma.c | 2 +- drivers/gpu/drm/radeon/radeon.h | 7 ++++--- drivers/gpu/drm/radeon/radeon_cs.c | 27 ++++++++++++++++++++------- drivers/gpu/drm/radeon/radeon_fence.c | 3 +++ drivers/gpu/drm/radeon/radeon_semaphore.c | 24 ++++++++++++++++++------ drivers/gpu/drm/radeon/radeon_vm.c | 4 ++-- drivers/gpu/drm/radeon/rv770_dma.c | 2 +- drivers/gpu/drm/radeon/si_dma.c | 2 +- 12 files changed, 54 insertions(+), 25 deletions(-)