[PATCH 2/3] drm/amdgpu/vcn3: replace ip based software ring decode with common vcn software ring decode

[Christian König]

Am 04.05.22 um 15:23 schrieb James Zhu:
> Replace ip based software ring decode with common vcn software ring decode.
>
> Signed-off-by: James Zhu <James.Zhu at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 82 ++++-----------------------
>   drivers/gpu/drm/amd/amdgpu/vcn_v3_0.h | 12 ----
>   2 files changed, 11 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> index c7280ca5e836..f761c569fcc6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> @@ -30,6 +30,7 @@
>   #include "soc15d.h"
>   #include "vcn_v2_0.h"
>   #include "mmsch_v3_0.h"
> +#include "vcn_sw_ring.h"
>   
>   #include "vcn/vcn_3_0_0_offset.h"
>   #include "vcn/vcn_3_0_0_sh_mask.h"
> @@ -1731,67 +1732,6 @@ static void vcn_v3_0_dec_ring_set_wptr(struct amdgpu_ring *ring)
>   	}
>   }
>   
> -void vcn_v3_0_dec_sw_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
> -      u64 seq, uint32_t flags)
> -{
> -	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> -
> -	amdgpu_ring_write(ring, VCN_DEC_SW_CMD_FENCE);
> -	amdgpu_ring_write(ring, addr);
> -	amdgpu_ring_write(ring, upper_32_bits(addr));
> -	amdgpu_ring_write(ring, seq);
> -	amdgpu_ring_write(ring, VCN_DEC_SW_CMD_TRAP);
> -}
> -
> -void vcn_v3_0_dec_sw_ring_insert_end(struct amdgpu_ring *ring)
> -{
> -	amdgpu_ring_write(ring, VCN_DEC_SW_CMD_END);
> -}
> -
> -void vcn_v3_0_dec_sw_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_job *job,
> -        struct amdgpu_ib *ib, uint32_t flags)
> -{
> -	uint32_t vmid = AMDGPU_JOB_GET_VMID(job);
> -
> -	amdgpu_ring_write(ring, VCN_DEC_SW_CMD_IB);
> -	amdgpu_ring_write(ring, vmid);
> -	amdgpu_ring_write(ring, lower_32_bits(ib->gpu_addr));
> -	amdgpu_ring_write(ring, upper_32_bits(ib->gpu_addr));
> -	amdgpu_ring_write(ring, ib->length_dw);
> -}
> -
> -void vcn_v3_0_dec_sw_ring_emit_reg_wait(struct amdgpu_ring *ring, uint32_t reg,
> -        uint32_t val, uint32_t mask)
> -{
> -	amdgpu_ring_write(ring, VCN_DEC_SW_CMD_REG_WAIT);
> -	amdgpu_ring_write(ring, reg << 2);
> -	amdgpu_ring_write(ring, mask);
> -	amdgpu_ring_write(ring, val);
> -}
> -
> -void vcn_v3_0_dec_sw_ring_emit_vm_flush(struct amdgpu_ring *ring,
> -        uint32_t vmid, uint64_t pd_addr)
> -{
> -	struct amdgpu_vmhub *hub = &ring->adev->vmhub[ring->funcs->vmhub];
> -	uint32_t data0, data1, mask;
> -
> -	pd_addr = amdgpu_gmc_emit_flush_gpu_tlb(ring, vmid, pd_addr);
> -
> -	/* wait for register write */
> -	data0 = hub->ctx0_ptb_addr_lo32 + vmid * hub->ctx_addr_distance;
> -	data1 = lower_32_bits(pd_addr);
> -	mask = 0xffffffff;
> -	vcn_v3_0_dec_sw_ring_emit_reg_wait(ring, data0, data1, mask);
> -}
> -
> -void vcn_v3_0_dec_sw_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
> -      uint32_t val)
> -{
> -	amdgpu_ring_write(ring, VCN_DEC_SW_CMD_REG_WRITE);
> -	amdgpu_ring_write(ring,	reg << 2);
> -	amdgpu_ring_write(ring, val);
> -}
> -
>   static const struct amdgpu_ring_funcs vcn_v3_0_dec_sw_ring_vm_funcs = {
>   	.type = AMDGPU_RING_TYPE_VCN_DEC,
>   	.align_mask = 0x3f,
> @@ -1804,22 +1744,22 @@ static const struct amdgpu_ring_funcs vcn_v3_0_dec_sw_ring_vm_funcs = {
>   	.emit_frame_size =
>   		SOC15_FLUSH_GPU_TLB_NUM_WREG * 3 +
>   		SOC15_FLUSH_GPU_TLB_NUM_REG_WAIT * 4 +
> -		4 + /* vcn_v3_0_dec_sw_ring_emit_vm_flush */
> -		5 + 5 + /* vcn_v3_0_dec_sw_ring_emit_fdec_swe x2 vm fdec_swe */
> -		1, /* vcn_v3_0_dec_sw_ring_insert_end */
> -	.emit_ib_size = 5, /* vcn_v3_0_dec_sw_ring_emit_ib */
> -	.emit_ib = vcn_v3_0_dec_sw_ring_emit_ib,
> -	.emit_fence = vcn_v3_0_dec_sw_ring_emit_fence,
> -	.emit_vm_flush = vcn_v3_0_dec_sw_ring_emit_vm_flush,
> +		4 + /* vcn_dec_sw_ring_emit_vm_flush */
> +		5 + 5 + /* vcn_dec_sw_ring_emit_fdec_swe x2 vm fdec_swe */
> +		1, /* vcn_dec_sw_ring_insert_end */

Maybe add that calculation as define to vcn_sw_ring.h.

Apart from that looks really good to me.

Regards,
Christian.

> +	.emit_ib_size = 5, /* vcn_dec_sw_ring_emit_ib */
> +	.emit_ib = vcn_dec_sw_ring_emit_ib,
> +	.emit_fence = vcn_dec_sw_ring_emit_fence,
> +	.emit_vm_flush = vcn_dec_sw_ring_emit_vm_flush,
>   	.test_ring = amdgpu_vcn_dec_sw_ring_test_ring,
>   	.test_ib = NULL,//amdgpu_vcn_dec_sw_ring_test_ib,
>   	.insert_nop = amdgpu_ring_insert_nop,
> -	.insert_end = vcn_v3_0_dec_sw_ring_insert_end,
> +	.insert_end = vcn_dec_sw_ring_insert_end,
>   	.pad_ib = amdgpu_ring_generic_pad_ib,
>   	.begin_use = amdgpu_vcn_ring_begin_use,
>   	.end_use = amdgpu_vcn_ring_end_use,
> -	.emit_wreg = vcn_v3_0_dec_sw_ring_emit_wreg,
> -	.emit_reg_wait = vcn_v3_0_dec_sw_ring_emit_reg_wait,
> +	.emit_wreg = vcn_dec_sw_ring_emit_wreg,
> +	.emit_reg_wait = vcn_dec_sw_ring_emit_reg_wait,
>   	.emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
>   };
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.h b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.h
> index 7a6655d3b79d..31683582d778 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.h
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.h
> @@ -26,16 +26,4 @@
>   
>   extern const struct amdgpu_ip_block_version vcn_v3_0_ip_block;
>   
> -void vcn_v3_0_dec_sw_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
> -      u64 seq, uint32_t flags);
> -void vcn_v3_0_dec_sw_ring_insert_end(struct amdgpu_ring *ring);
> -void vcn_v3_0_dec_sw_ring_emit_ib(struct amdgpu_ring *ring, struct amdgpu_job *job,
> -      struct amdgpu_ib *ib, uint32_t flags);
> -void vcn_v3_0_dec_sw_ring_emit_reg_wait(struct amdgpu_ring *ring, uint32_t reg,
> -      uint32_t val, uint32_t mask);
> -void vcn_v3_0_dec_sw_ring_emit_vm_flush(struct amdgpu_ring *ring,
> -      uint32_t vmid, uint64_t pd_addr);
> -void vcn_v3_0_dec_sw_ring_emit_wreg(struct amdgpu_ring *ring, uint32_t reg,
> -      uint32_t val);
> -
>   #endif /* __VCN_V3_0_H__ */