On Fri, Jul 13, 2012 at 04:08:15PM +0200, Christian K?nig wrote:
> Const IBs are executed on the CE not the CP, so we can't
> fence them in the normal way.
>
> So submit them directly before the IB instead, just as
> the documentation says.
>
> Signed-off-by: Christian K?nig <deathsimple at vodafone.de>
> ---
> drivers/gpu/drm/radeon/r100.c | 2 +-
> drivers/gpu/drm/radeon/r600.c | 2 +-
> drivers/gpu/drm/radeon/radeon.h | 3 ++-
> drivers/gpu/drm/radeon/radeon_cs.c | 25 +++++++++++--------------
> drivers/gpu/drm/radeon/radeon_ring.c | 10 +++++++++-
> 5 files changed, 24 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
> index e0f5ae8..4ee5a74 100644
> --- a/drivers/gpu/drm/radeon/r100.c
> +++ b/drivers/gpu/drm/radeon/r100.c
> @@ -3693,7 +3693,7 @@ int r100_ib_test(struct radeon_device *rdev, struct
> radeon_ring *ring)
> ib.ptr[6] = PACKET2(0);
> ib.ptr[7] = PACKET2(0);
> ib.length_dw = 8;
> - r = radeon_ib_schedule(rdev, &ib);
> + r = radeon_ib_schedule(rdev, &ib, NULL);
> if (r) {
> radeon_scratch_free(rdev, scratch);
> radeon_ib_free(rdev, &ib);
> diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
> index 3156d25..c2e5069 100644
> --- a/drivers/gpu/drm/radeon/r600.c
> +++ b/drivers/gpu/drm/radeon/r600.c
> @@ -2619,7 +2619,7 @@ int r600_ib_test(struct radeon_device *rdev, struct
> radeon_ring *ring)
> ib.ptr[1] = ((scratch - PACKET3_SET_CONFIG_REG_OFFSET) >> 2);
> ib.ptr[2] = 0xDEADBEEF;
> ib.length_dw = 3;
> - r = radeon_ib_schedule(rdev, &ib);
> + r = radeon_ib_schedule(rdev, &ib, NULL);
> if (r) {
> radeon_scratch_free(rdev, scratch);
> radeon_ib_free(rdev, &ib);
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 2cb355b..2d7f06c 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -751,7 +751,8 @@ struct si_rlc {
> int radeon_ib_get(struct radeon_device *rdev, int ring,
> struct radeon_ib *ib, unsigned size);
> void radeon_ib_free(struct radeon_device *rdev, struct radeon_ib *ib);
> -int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib);
> +int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib,
> + struct radeon_ib *const_ib);
> int radeon_ib_pool_init(struct radeon_device *rdev);
> void radeon_ib_pool_fini(struct radeon_device *rdev);
> int radeon_ib_ring_tests(struct radeon_device *rdev);
> diff --git a/drivers/gpu/drm/radeon/radeon_cs.c
> b/drivers/gpu/drm/radeon/radeon_cs.c
> index 553da67..d0be5d5 100644
> --- a/drivers/gpu/drm/radeon/radeon_cs.c
> +++ b/drivers/gpu/drm/radeon/radeon_cs.c
> @@ -354,7 +354,7 @@ static int radeon_cs_ib_chunk(struct radeon_device *rdev,
> }
> radeon_cs_sync_rings(parser);
> parser->ib.vm_id = 0;
> - r = radeon_ib_schedule(rdev, &parser->ib);
> + r = radeon_ib_schedule(rdev, &parser->ib, NULL);
> if (r) {
> DRM_ERROR("Failed to schedule IB !\n");
> }
> @@ -452,25 +452,22 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device
> *rdev,
> }
> radeon_cs_sync_rings(parser);
>
> + parser->ib.vm_id = vm->id;
> + /* ib pool is bind at 0 in virtual address space,
> + * so gpu_addr is the offset inside the pool bo
> + */
> + parser->ib.gpu_addr = parser->ib.sa_bo->soffset;
> +
> if ((rdev->family >= CHIP_TAHITI) &&
> (parser->chunk_const_ib_idx != -1)) {
> parser->const_ib.vm_id = vm->id;
> - /* ib pool is bind at 0 in virtual address space to gpu_addr is
> the
> - * offset inside the pool bo
> - */
> + /* same reason as above */
> parser->const_ib.gpu_addr = parser->const_ib.sa_bo->soffset;
> - r = radeon_ib_schedule(rdev, &parser->const_ib);
> - if (r)
> - goto out;
> + r = radeon_ib_schedule(rdev, &parser->ib, &parser->const_ib);
> + } else {
> + r = radeon_ib_schedule(rdev, &parser->ib, NULL);
> }
>
> - parser->ib.vm_id = vm->id;
> - /* ib pool is bind at 0 in virtual address space to gpu_addr is the
> - * offset inside the pool bo
> - */
> - parser->ib.gpu_addr = parser->ib.sa_bo->soffset;
> - parser->ib.is_const_ib = false;
> - r = radeon_ib_schedule(rdev, &parser->ib);
> out:
> if (!r) {
> if (vm->fence) {
> diff --git a/drivers/gpu/drm/radeon/radeon_ring.c
> b/drivers/gpu/drm/radeon/radeon_ring.c
> index 75cbe46..c48c354 100644
> --- a/drivers/gpu/drm/radeon/radeon_ring.c
> +++ b/drivers/gpu/drm/radeon/radeon_ring.c
> @@ -74,7 +74,8 @@ void radeon_ib_free(struct radeon_device *rdev, struct
> radeon_ib *ib)
> radeon_fence_unref(&ib->fence);
> }
>
> -int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib)
> +int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib,
> + struct radeon_ib *const_ib)
Since you are modifying an uncommented function, comments should be
added, per the new documentation rules.
I don't mean to be picky, but there is no point having documentation
rules if they aren't enforced.
> {
> struct radeon_ring *ring = &rdev->ring[ib->ring];
> bool need_sync = false;
> @@ -105,6 +106,10 @@ int radeon_ib_schedule(struct radeon_device *rdev,
> struct radeon_ib *ib)
> if (!need_sync) {
> radeon_semaphore_free(rdev, &ib->semaphore, NULL);
> }
> + if (const_ib) {
> + radeon_ring_ib_execute(rdev, const_ib->ring, const_ib);
> + radeon_semaphore_free(rdev, &const_ib->semaphore, NULL);
> + }
> radeon_ring_ib_execute(rdev, ib->ring, ib);
> r = radeon_fence_emit(rdev, &ib->fence, ib->ring);
> if (r) {
> @@ -112,6 +117,9 @@ int radeon_ib_schedule(struct radeon_device *rdev, struct
> radeon_ib *ib)
> radeon_ring_unlock_undo(rdev, ring);
> return r;
> }
> + if (const_ib) {
> + const_ib->fence = radeon_fence_ref(ib->fence);
> + }
> radeon_ring_unlock_commit(rdev, ring);
> return 0;
> }
> --
> 1.7.9.5
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
