Re: [PATCH 03/11] drm/amdgpu: cleanup VM handling in the CS a bit
On Wed, Aug 22, 2018 at 05:05:09PM +0200, Christian König wrote:
> Add a helper function for getting the root PD addr and cleanup join the
> two VM related functions and cleanup the function name.
>
> No functional change.
>
> Signed-off-by: Christian König
Reviewed-by: Huang Rui
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 160 -
> 1 file changed, 74 insertions(+), 86 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index d42d1c8f78f6..17bf63f93c93 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -804,8 +804,9 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser
> *parser, int error,
> amdgpu_bo_unref(>uf_entry.robj);
> }
>
> -static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
> +static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
> {
> + struct amdgpu_ring *ring = to_amdgpu_ring(p->entity->rq->sched);
> struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
> struct amdgpu_device *adev = p->adev;
> struct amdgpu_vm *vm = >vm;
> @@ -814,6 +815,71 @@ static int amdgpu_bo_vm_update_pte(struct
> amdgpu_cs_parser *p)
> struct amdgpu_bo *bo;
> int r;
>
> + /* Only for UVD/VCE VM emulation */
> + if (ring->funcs->parse_cs || ring->funcs->patch_cs_in_place) {
> + unsigned i, j;
> +
> + for (i = 0, j = 0; i < p->nchunks && j < p->job->num_ibs; i++) {
> + struct drm_amdgpu_cs_chunk_ib *chunk_ib;
> + struct amdgpu_bo_va_mapping *m;
> + struct amdgpu_bo *aobj = NULL;
> + struct amdgpu_cs_chunk *chunk;
> + uint64_t offset, va_start;
> + struct amdgpu_ib *ib;
> + uint8_t *kptr;
> +
> + chunk = >chunks[i];
> + ib = >job->ibs[j];
> + chunk_ib = chunk->kdata;
> +
> + if (chunk->chunk_id != AMDGPU_CHUNK_ID_IB)
> + continue;
> +
> + va_start = chunk_ib->va_start & AMDGPU_VA_HOLE_MASK;
> + r = amdgpu_cs_find_mapping(p, va_start, , );
> + if (r) {
> + DRM_ERROR("IB va_start is invalid\n");
> + return r;
> + }
> +
> + if ((va_start + chunk_ib->ib_bytes) >
> + (m->last + 1) * AMDGPU_GPU_PAGE_SIZE) {
> + DRM_ERROR("IB va_start+ib_bytes is invalid\n");
> + return -EINVAL;
> + }
> +
> + /* the IB should be reserved at this point */
> + r = amdgpu_bo_kmap(aobj, (void **));
> + if (r) {
> + return r;
> + }
> +
> + offset = m->start * AMDGPU_GPU_PAGE_SIZE;
> + kptr += va_start - offset;
> +
> + if (ring->funcs->parse_cs) {
> + memcpy(ib->ptr, kptr, chunk_ib->ib_bytes);
> + amdgpu_bo_kunmap(aobj);
> +
> + r = amdgpu_ring_parse_cs(ring, p, j);
> + if (r)
> + return r;
> + } else {
> + ib->ptr = (uint32_t *)kptr;
> + r = amdgpu_ring_patch_cs_in_place(ring, p, j);
> + amdgpu_bo_kunmap(aobj);
> + if (r)
> + return r;
> + }
> +
> + j++;
> + }
> + }
> +
> + if (!p->job->vm)
> + return amdgpu_cs_sync_rings(p);
> +
> +
> r = amdgpu_vm_clear_freed(adev, vm, NULL);
> if (r)
> return r;
> @@ -876,6 +942,12 @@ static int amdgpu_bo_vm_update_pte(struct
> amdgpu_cs_parser *p)
> if (r)
> return r;
>
> + r = reservation_object_reserve_shared(vm->root.base.bo->tbo.resv);
> + if (r)
> + return r;
> +
> + p->job->vm_pd_addr = amdgpu_bo_gpu_offset(vm->root.base.bo);
> +
> if (amdgpu_vm_debug) {
> /* Invalidate all BOs to test for userspace bugs */
> amdgpu_bo_list_for_each_entry(e, p->bo_list) {
> @@ -887,90 +959,6 @@ static int amdgpu_bo_vm_update_pte(struct
> amdgpu_cs_parser *p)
> }
> }
>
> - return r;
> -}
> -
> -static int amdgpu_cs_ib_vm_chunk(struct amdgpu_device *adev,
> - struct amdgpu_cs_parser *p)
> -{
> - struct amdgpu_ring *ring = to_amdgpu_ring(p->entity->rq->sched);
> - struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
> - struct amdgpu_vm *vm = >vm;
> - int r;
>
Re: [PATCH 03/11] drm/amdgpu: cleanup VM handling in the CS a bit
On Wed, Aug 22, 2018 at 11:05 AM Christian König
wrote:
>
> Add a helper function for getting the root PD addr and cleanup join the
> two VM related functions and cleanup the function name.
>
> No functional change.
>
> Signed-off-by: Christian König
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 160 -
> 1 file changed, 74 insertions(+), 86 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index d42d1c8f78f6..17bf63f93c93 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -804,8 +804,9 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser
> *parser, int error,
> amdgpu_bo_unref(>uf_entry.robj);
> }
>
> -static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
> +static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
> {
> + struct amdgpu_ring *ring = to_amdgpu_ring(p->entity->rq->sched);
> struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
> struct amdgpu_device *adev = p->adev;
> struct amdgpu_vm *vm = >vm;
> @@ -814,6 +815,71 @@ static int amdgpu_bo_vm_update_pte(struct
> amdgpu_cs_parser *p)
> struct amdgpu_bo *bo;
> int r;
>
> + /* Only for UVD/VCE VM emulation */
> + if (ring->funcs->parse_cs || ring->funcs->patch_cs_in_place) {
> + unsigned i, j;
> +
> + for (i = 0, j = 0; i < p->nchunks && j < p->job->num_ibs;
> i++) {
> + struct drm_amdgpu_cs_chunk_ib *chunk_ib;
> + struct amdgpu_bo_va_mapping *m;
> + struct amdgpu_bo *aobj = NULL;
> + struct amdgpu_cs_chunk *chunk;
> + uint64_t offset, va_start;
> + struct amdgpu_ib *ib;
> + uint8_t *kptr;
> +
> + chunk = >chunks[i];
> + ib = >job->ibs[j];
> + chunk_ib = chunk->kdata;
> +
> + if (chunk->chunk_id != AMDGPU_CHUNK_ID_IB)
> + continue;
> +
> + va_start = chunk_ib->va_start & AMDGPU_VA_HOLE_MASK;
> + r = amdgpu_cs_find_mapping(p, va_start, , );
> + if (r) {
> + DRM_ERROR("IB va_start is invalid\n");
> + return r;
> + }
> +
> + if ((va_start + chunk_ib->ib_bytes) >
> + (m->last + 1) * AMDGPU_GPU_PAGE_SIZE) {
> + DRM_ERROR("IB va_start+ib_bytes is
> invalid\n");
> + return -EINVAL;
> + }
> +
> + /* the IB should be reserved at this point */
> + r = amdgpu_bo_kmap(aobj, (void **));
> + if (r) {
> + return r;
> + }
> +
> + offset = m->start * AMDGPU_GPU_PAGE_SIZE;
> + kptr += va_start - offset;
> +
> + if (ring->funcs->parse_cs) {
> + memcpy(ib->ptr, kptr, chunk_ib->ib_bytes);
> + amdgpu_bo_kunmap(aobj);
> +
> + r = amdgpu_ring_parse_cs(ring, p, j);
> + if (r)
> + return r;
> + } else {
> + ib->ptr = (uint32_t *)kptr;
> + r = amdgpu_ring_patch_cs_in_place(ring, p, j);
> + amdgpu_bo_kunmap(aobj);
> + if (r)
> + return r;
> + }
> +
> + j++;
> + }
> + }
> +
> + if (!p->job->vm)
> + return amdgpu_cs_sync_rings(p);
> +
> +
> r = amdgpu_vm_clear_freed(adev, vm, NULL);
> if (r)
> return r;
> @@ -876,6 +942,12 @@ static int amdgpu_bo_vm_update_pte(struct
> amdgpu_cs_parser *p)
> if (r)
> return r;
>
> + r = reservation_object_reserve_shared(vm->root.base.bo->tbo.resv);
> + if (r)
> + return r;
> +
> + p->job->vm_pd_addr = amdgpu_bo_gpu_offset(vm->root.base.bo);
> +
> if (amdgpu_vm_debug) {
> /* Invalidate all BOs to test for userspace bugs */
> amdgpu_bo_list_for_each_entry(e, p->bo_list) {
> @@ -887,90 +959,6 @@ static int amdgpu_bo_vm_update_pte(struct
> amdgpu_cs_parser *p)
> }
> }
>
> - return r;
> -}
> -
> -static int amdgpu_cs_ib_vm_chunk(struct amdgpu_device *adev,
> -struct amdgpu_cs_parser *p)
> -{
> - struct amdgpu_ring *ring =
[PATCH 03/11] drm/amdgpu: cleanup VM handling in the CS a bit
Add a helper function for getting the root PD addr and cleanup join the
two VM related functions and cleanup the function name.
No functional change.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 160 -
1 file changed, 74 insertions(+), 86 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index d42d1c8f78f6..17bf63f93c93 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -804,8 +804,9 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser
*parser, int error,
amdgpu_bo_unref(>uf_entry.robj);
}
-static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser *p)
+static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
{
+ struct amdgpu_ring *ring = to_amdgpu_ring(p->entity->rq->sched);
struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
struct amdgpu_device *adev = p->adev;
struct amdgpu_vm *vm = >vm;
@@ -814,6 +815,71 @@ static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser
*p)
struct amdgpu_bo *bo;
int r;
+ /* Only for UVD/VCE VM emulation */
+ if (ring->funcs->parse_cs || ring->funcs->patch_cs_in_place) {
+ unsigned i, j;
+
+ for (i = 0, j = 0; i < p->nchunks && j < p->job->num_ibs; i++) {
+ struct drm_amdgpu_cs_chunk_ib *chunk_ib;
+ struct amdgpu_bo_va_mapping *m;
+ struct amdgpu_bo *aobj = NULL;
+ struct amdgpu_cs_chunk *chunk;
+ uint64_t offset, va_start;
+ struct amdgpu_ib *ib;
+ uint8_t *kptr;
+
+ chunk = >chunks[i];
+ ib = >job->ibs[j];
+ chunk_ib = chunk->kdata;
+
+ if (chunk->chunk_id != AMDGPU_CHUNK_ID_IB)
+ continue;
+
+ va_start = chunk_ib->va_start & AMDGPU_VA_HOLE_MASK;
+ r = amdgpu_cs_find_mapping(p, va_start, , );
+ if (r) {
+ DRM_ERROR("IB va_start is invalid\n");
+ return r;
+ }
+
+ if ((va_start + chunk_ib->ib_bytes) >
+ (m->last + 1) * AMDGPU_GPU_PAGE_SIZE) {
+ DRM_ERROR("IB va_start+ib_bytes is invalid\n");
+ return -EINVAL;
+ }
+
+ /* the IB should be reserved at this point */
+ r = amdgpu_bo_kmap(aobj, (void **));
+ if (r) {
+ return r;
+ }
+
+ offset = m->start * AMDGPU_GPU_PAGE_SIZE;
+ kptr += va_start - offset;
+
+ if (ring->funcs->parse_cs) {
+ memcpy(ib->ptr, kptr, chunk_ib->ib_bytes);
+ amdgpu_bo_kunmap(aobj);
+
+ r = amdgpu_ring_parse_cs(ring, p, j);
+ if (r)
+ return r;
+ } else {
+ ib->ptr = (uint32_t *)kptr;
+ r = amdgpu_ring_patch_cs_in_place(ring, p, j);
+ amdgpu_bo_kunmap(aobj);
+ if (r)
+ return r;
+ }
+
+ j++;
+ }
+ }
+
+ if (!p->job->vm)
+ return amdgpu_cs_sync_rings(p);
+
+
r = amdgpu_vm_clear_freed(adev, vm, NULL);
if (r)
return r;
@@ -876,6 +942,12 @@ static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser
*p)
if (r)
return r;
+ r = reservation_object_reserve_shared(vm->root.base.bo->tbo.resv);
+ if (r)
+ return r;
+
+ p->job->vm_pd_addr = amdgpu_bo_gpu_offset(vm->root.base.bo);
+
if (amdgpu_vm_debug) {
/* Invalidate all BOs to test for userspace bugs */
amdgpu_bo_list_for_each_entry(e, p->bo_list) {
@@ -887,90 +959,6 @@ static int amdgpu_bo_vm_update_pte(struct amdgpu_cs_parser
*p)
}
}
- return r;
-}
-
-static int amdgpu_cs_ib_vm_chunk(struct amdgpu_device *adev,
-struct amdgpu_cs_parser *p)
-{
- struct amdgpu_ring *ring = to_amdgpu_ring(p->entity->rq->sched);
- struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
- struct amdgpu_vm *vm = >vm;
- int r;
-
- /* Only for UVD/VCE VM emulation */
- if (ring->funcs->parse_cs || ring->funcs->patch_cs_in_place) {
- unsigned i, j;
-
- for (i = 0, j = 0; i <
