On Mon, Jul 23, 2018 at 11:14 AM, Christian König
<ckoenig.leichtzumerken at gmail.com> wrote:
> We are going to need that for the second UVD instance on Vega20.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
A couple of nits below. Patches 1, 2, 4 are:
Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 22 +++++++++++++++-------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 1 +
> 3 files changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index a9f09daeffa3..73f2f8d987cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1751,6 +1751,7 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
> #define amdgpu_vm_write_pte(adev, ib, pe, value, count, incr) ((adev)->vm_manager.vm_pte_funcs->write_pte((ib), (pe), (value), (count), (incr)))
> #define amdgpu_vm_set_pte_pde(adev, ib, pe, addr, count, incr, flags) ((adev)->vm_manager.vm_pte_funcs->set_pte_pde((ib), (pe), (addr), (count), (incr), (flags)))
> #define amdgpu_ring_parse_cs(r, p, ib) ((r)->funcs->parse_cs((p), (ib)))
> +#define amdgpu_ring_patch_cs(r, p, ib) ((r)->funcs->patch_cs((p), (ib)))
> #define amdgpu_ring_test_ring(r) (r)->funcs->test_ring((r))
> #define amdgpu_ring_test_ib(r, t) (r)->funcs->test_ib((r), (t))
> #define amdgpu_ring_get_rptr(r) (r)->funcs->get_rptr((r))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 7c5cc33d0cda..b7840e4bb958 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -916,7 +916,7 @@ static int amdgpu_cs_ib_vm_chunk(struct amdgpu_device *adev,
> int r;
>
> /* Only for UVD/VCE VM emulation */
> - if (p->ring->funcs->parse_cs) {
> + if (p->ring->funcs->parse_cs || p->ring->funcs->patch_cs) {
> unsigned i, j;
>
> for (i = 0, j = 0; i < p->nchunks && j < p->job->num_ibs; i++) {
> @@ -957,12 +957,20 @@ static int amdgpu_cs_ib_vm_chunk(struct amdgpu_device *adev,
> offset = m->start * AMDGPU_GPU_PAGE_SIZE;
> kptr += va_start - offset;
>
> - memcpy(ib->ptr, kptr, chunk_ib->ib_bytes);
> - amdgpu_bo_kunmap(aobj);
> -
> - r = amdgpu_ring_parse_cs(ring, p, j);
> - if (r)
> - return r;
> + if (p->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 {
Maybe make this if (p->ring->funcs->patch_cs) {? Not a big deal
either way. I doubt we'd every have both callbacks defined.
> + ib->ptr = (uint32_t *)kptr;
> + r = amdgpu_ring_patch_cs(ring, p, j);
> + amdgpu_bo_kunmap(aobj);
> + if (r)
> + return r;
> + }
>
> j++;
> }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 5018c0b6bf1a..24c082381488 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -123,6 +123,7 @@ struct amdgpu_ring_funcs {
> void (*set_wptr)(struct amdgpu_ring *ring);
> /* validating and patching of IBs */
> int (*parse_cs)(struct amdgpu_cs_parser *p, uint32_t ib_idx);
> + int (*patch_cs)(struct amdgpu_cs_parser *p, uint32_t ib_idx);
Can we come up with a better name? parse_vm_cs() or
patch_in_place_cs() or something like that for consistency?
Alex
> /* constants to calculate how many DW are needed for an emit */
> unsigned emit_frame_size;
> unsigned emit_ib_size;
> --
> 2.14.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
