From: "Christian König" <deathsimple@vodafone.de>
To: Maarten Lankhorst <maarten.lankhorst@canonical.com>,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 6/8] drm/radeon: cope with foreign fences inside the reservation object
Date: Wed, 17 Sep 2014 15:09:23 +0200 [thread overview]
Message-ID: <54198803.2040508@vodafone.de> (raw)
In-Reply-To: <1410957305-10190-7-git-send-email-maarten.lankhorst@canonical.com>
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++) {
next prev parent reply other threads:[~2014-09-17 13:09 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-09-17 12:34 [PATCH 00/10] drm fence fixes and cross-dev sync Maarten Lankhorst
2014-09-17 12:34 ` [PATCH 1/8] drm/nouveau: bump driver patchlevel to 1.2.1 Maarten Lankhorst
2014-09-17 12:34 ` [PATCH 2/8] drm/nouveau: specify if interruptible wait is desired in nouveau_fence_sync Maarten Lankhorst
2014-09-17 12:35 ` [PATCH 3/8] drm: Pass dma-buf as argument to gem_prime_import_sg_table Maarten Lankhorst
2014-09-17 12:35 ` [PATCH 4/8] drm/ttm: add reservation_object as argument to ttm_bo_init Maarten Lankhorst
2014-09-17 12:35 ` [PATCH 5/8] drm/radeon: cope with foreign fences inside display Maarten Lankhorst
2014-09-17 13:16 ` Christian König
2014-09-17 12:35 ` [PATCH 6/8] drm/radeon: cope with foreign fences inside the reservation object Maarten Lankhorst
2014-09-17 13:09 ` Christian König [this message]
2014-09-25 9:52 ` Maarten Lankhorst
2014-09-25 10:39 ` [PATCH v2 " Maarten Lankhorst
2014-10-01 8:57 ` Maarten Lankhorst
2014-10-01 9:04 ` Christian König
2014-10-01 9:22 ` Maarten Lankhorst
2014-10-01 12:26 ` Alex Deucher
2014-09-18 3:26 ` [PATCH " Michel Dänzer
2014-09-18 10:22 ` Maarten Lankhorst
2014-09-19 1:19 ` Michel Dänzer
2014-09-17 12:35 ` [PATCH 7/8] drm/radeon: export reservation_object from dmabuf to ttm Maarten Lankhorst
2014-09-17 13:14 ` Christian König
2014-09-18 3:22 ` Michel Dänzer
2014-09-18 12:11 ` [PATCH v2 " Maarten Lankhorst
2014-09-18 13:13 ` Christian König
2014-09-17 12:35 ` [PATCH 8/8] drm/nouveau: " Maarten Lankhorst
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54198803.2040508@vodafone.de \
--to=deathsimple@vodafone.de \
--cc=dri-devel@lists.freedesktop.org \
--cc=maarten.lankhorst@canonical.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.