On Tue, Aug 6, 2024 at 12:00 PM David (Ming Qiang) Wu <David.Wu3@xxxxxxx> wrote:
>
> Because gpu_addr is updated in the calling routine
> (amdgpu_cs_patch_ibs()),it is removed in the callback.
>
> Use .patch_cs_in_place instead of .parse_cs for
> amdgpu_vce_ring_parse_cs_vm() as there is no need for keeping
> a temporary IB, therefore ib->sa_bo is NULL and amdgpu_ib_free()
> is removed.
Took me a while to page this all back into my head, but this works
because in this case, we still use the user's IB, we just use the
parsing to deal with the session handles. This code really should
have used patch_cs_in_place from the beginning, but it didn't exist at
that point.
Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
>
> Signed-off-by: David (Ming Qiang) Wu <David.Wu3@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 1 -
> drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 2 --
> drivers/gpu/drm/amd/amdgpu/vce_v3_0.c | 2 +-
> drivers/gpu/drm/amd/amdgpu/vce_v4_0.c | 2 +-
> 4 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> index 07d930339b07..98863e920e00 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
> @@ -1088,7 +1088,6 @@ int amdgpu_uvd_ring_parse_cs(struct amdgpu_cs_parser *parser,
> int r;
>
> job->vm = NULL;
> - ib->gpu_addr = amdgpu_sa_bo_gpu_addr(ib->sa_bo);
>
> if (ib->length_dw % 16) {
> DRM_ERROR("UVD IB length (%d) not 16 dwords aligned!\n",
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> index 968ca2c84ef7..6efb15c5fa1a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
> @@ -749,7 +749,6 @@ int amdgpu_vce_ring_parse_cs(struct amdgpu_cs_parser *p,
> int i, r = 0;
>
> job->vm = NULL;
> - ib->gpu_addr = amdgpu_sa_bo_gpu_addr(ib->sa_bo);
>
> for (idx = 0; idx < ib->length_dw;) {
> uint32_t len = amdgpu_ib_get_value(ib, idx);
> @@ -1044,7 +1043,6 @@ int amdgpu_vce_ring_parse_cs_vm(struct amdgpu_cs_parser *p,
> if (!r) {
> /* No error, free all destroyed handle slots */
> tmp = destroyed;
> - amdgpu_ib_free(p->adev, ib, NULL);
> } else {
> /* Error during parsing, free all allocated handle slots */
> tmp = allocated;
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> index 32517c364cf7..4bfba2931b08 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v3_0.c
> @@ -950,7 +950,7 @@ static const struct amdgpu_ring_funcs vce_v3_0_ring_vm_funcs = {
> .get_rptr = vce_v3_0_ring_get_rptr,
> .get_wptr = vce_v3_0_ring_get_wptr,
> .set_wptr = vce_v3_0_ring_set_wptr,
> - .parse_cs = amdgpu_vce_ring_parse_cs_vm,
> + .patch_cs_in_place = amdgpu_vce_ring_parse_cs_vm,
> .emit_frame_size =
> 6 + /* vce_v3_0_emit_vm_flush */
> 4 + /* vce_v3_0_emit_pipeline_sync */
> diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> index 06d787385ad4..0748bf44c880 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vce_v4_0.c
> @@ -1102,7 +1102,7 @@ static const struct amdgpu_ring_funcs vce_v4_0_ring_vm_funcs = {
> .get_rptr = vce_v4_0_ring_get_rptr,
> .get_wptr = vce_v4_0_ring_get_wptr,
> .set_wptr = vce_v4_0_ring_set_wptr,
> - .parse_cs = amdgpu_vce_ring_parse_cs_vm,
> + .patch_cs_in_place = amdgpu_vce_ring_parse_cs_vm,
> .emit_frame_size =
> SOC15_FLUSH_GPU_TLB_NUM_WREG * 3 +
> SOC15_FLUSH_GPU_TLB_NUM_REG_WAIT * 4 +
> --
> 2.34.1
>
