Re: [PATCH 2/9] drm/amdgpu: Refine function amdgpu_csa_vaddr

["Christian König" <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Am 06.12.18 um 13:14 schrieb Rex Zhu:
> on baremetal, driver create csa per ctx.
> So add a function argument: ctx_id to
> get csa gpu addr.
> In Sriov, driver create csa per process,
> so ctx id always 1.
>
> Signed-off-by: Rex Zhu <Rex.Zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c | 5 +++--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_csa.h | 2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 4 ++--
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 6 +++---
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 4 ++--
>   5 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> index 0c590dd..44b046f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> @@ -24,11 +24,12 @@
>   
>   #include "amdgpu.h"
>   
> -uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev)
> +uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev, uint32_t id)
>   {
>   	uint64_t addr = adev->vm_manager.max_pfn << AMDGPU_GPU_PAGE_SHIFT;
>   
> -	addr -= AMDGPU_VA_RESERVED_SIZE;
> +	addr -= AMDGPU_VA_RESERVED_SIZE * id;
> +

It might be better to add the SRIOV handling here instead. E.g. like the 
following:

if (amdgpu_sriov_vf(adev))
     addr -= AMDGPU_VA_RESERVED_SIZE;
else
     addr -= AMDGPU_CSA_SIZE * id;

Apart from that looks good to me, but might be better to re-order the 
patches.

Regards,
Christian.

>   	addr = amdgpu_gmc_sign_extend(addr);
>   
>   	return addr;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.h
> index 524b443..aaf1fba 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.h
> @@ -28,7 +28,7 @@
>   #define AMDGPU_CSA_SIZE		(128 * 1024)
>   
>   uint32_t amdgpu_get_total_csa_size(struct amdgpu_device *adev);
> -uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev);
> +uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev, uint32_t id);
>   int amdgpu_allocate_static_csa(struct amdgpu_device *adev, struct amdgpu_bo **bo,
>   				u32 domain, uint32_t size);
>   int amdgpu_map_static_csa(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 08d04f6..f736bda 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -977,9 +977,9 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
>   		goto error_vm;
>   	}
>   
> -	if (amdgpu_sriov_vf(adev)) {
> -		uint64_t csa_addr = amdgpu_csa_vaddr(adev) & AMDGPU_GMC_HOLE_MASK;
>   
> +	if (amdgpu_sriov_vf(adev)) {
> +		uint64_t csa_addr = amdgpu_csa_vaddr(adev, 1) & AMDGPU_GMC_HOLE_MASK;
>   		r = amdgpu_map_static_csa(adev, &fpriv->vm, adev->virt.csa_obj,
>   						&fpriv->csa_va, csa_addr, AMDGPU_CSA_SIZE);
>   		if (r)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index bdae563..d529cef 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -7192,11 +7192,11 @@ static void gfx_v8_0_ring_emit_ce_meta(struct amdgpu_ring *ring)
>   	} ce_payload = {};
>   
>   	if (ring->adev->virt.chained_ib_support) {
> -		ce_payload_addr = amdgpu_csa_vaddr(ring->adev) +
> +		ce_payload_addr = amdgpu_csa_vaddr(ring->adev, 1) +
>   			offsetof(struct vi_gfx_meta_data_chained_ib, ce_payload);
>   		cnt_ce = (sizeof(ce_payload.chained) >> 2) + 4 - 2;
>   	} else {
> -		ce_payload_addr = amdgpu_csa_vaddr(ring->adev) +
> +		ce_payload_addr = amdgpu_csa_vaddr(ring->adev, 1) +
>   			offsetof(struct vi_gfx_meta_data, ce_payload);
>   		cnt_ce = (sizeof(ce_payload.regular) >> 2) + 4 - 2;
>   	}
> @@ -7220,7 +7220,7 @@ static void gfx_v8_0_ring_emit_de_meta(struct amdgpu_ring *ring)
>   		struct vi_de_ib_state_chained_ib chained;
>   	} de_payload = {};
>   
> -	csa_addr = amdgpu_csa_vaddr(ring->adev);
> +	csa_addr = amdgpu_csa_vaddr(ring->adev, 1);
>   	gds_addr = csa_addr + 4096;
>   	if (ring->adev->virt.chained_ib_support) {
>   		de_payload.chained.gds_backup_addrlo = lower_32_bits(gds_addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 928034c..81c1578 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -4327,7 +4327,7 @@ static void gfx_v9_0_ring_emit_ce_meta(struct amdgpu_ring *ring)
>   	int cnt;
>   
>   	cnt = (sizeof(ce_payload) >> 2) + 4 - 2;
> -	csa_addr = amdgpu_csa_vaddr(ring->adev);
> +	csa_addr = amdgpu_csa_vaddr(ring->adev, 1);
>   
>   	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, cnt));
>   	amdgpu_ring_write(ring, (WRITE_DATA_ENGINE_SEL(2) |
> @@ -4345,7 +4345,7 @@ static void gfx_v9_0_ring_emit_de_meta(struct amdgpu_ring *ring)
>   	uint64_t csa_addr, gds_addr;
>   	int cnt;
>   
> -	csa_addr = amdgpu_csa_vaddr(ring->adev);
> +	csa_addr = amdgpu_csa_vaddr(ring->adev, 1);
>   	gds_addr = csa_addr + 4096;
>   	de_payload.gds_backup_addrlo = lower_32_bits(gds_addr);
>   	de_payload.gds_backup_addrhi = upper_32_bits(gds_addr);

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx