[PATCH 3/9] drm/amdgpu: Use dynamical reserved vm size

Zhu, Rex Rex.Zhu at amd.com
Thu Dec 6 14:13:18 UTC 2018



> -----Original Message-----
> From: Christian König <ckoenig.leichtzumerken at gmail.com>
> Sent: Thursday, December 6, 2018 8:33 PM
> To: Zhu, Rex <Rex.Zhu at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 3/9] drm/amdgpu: Use dynamical reserved vm size
> 
> Am 06.12.18 um 13:14 schrieb Rex Zhu:
> > Use dynamical reserved vm size instand of hardcode.
> >
> > driver always reserve AMDGPU_VA_RESERVED_SIZE at the bottom of VM
> > space.
> >
> > for gfx/sdma mcbp feature,
> > reserve AMDGPU_VA_RESERVED_SIZ * AMDGPU_VM_MAX_NUM_CTX at
> the top of
> > VM space.
> > For sriov, only need to reserve AMDGPU_VA_RESERVED_SIZE at the top.
> >
> > Signed-off-by: Rex Zhu <Rex.Zhu at amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c |  8 ++++++++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_csa.h |  1 +
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 10 ++++++++++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  3 +--
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  1 +
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  1 +
> >   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c   |  2 +-
> >   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c   |  2 +-
> >   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c   |  2 +-
> >   9 files changed, 25 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> > index 44b046f..caa71c7 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.c
> > @@ -24,6 +24,14 @@
> >
> >   #include "amdgpu.h"
> >
> > +uint64_t amdgpu_get_reserved_csa_size(struct amdgpu_device *adev) {
> > +	if (amdgpu_sriov_vf(adev))
> > +		return AMDGPU_VA_RESERVED_SIZE;
> > +	else
> > +		return AMDGPU_VA_RESERVED_SIZE *
> AMDGPU_VM_MAX_NUM_CTX;
> 
> Do we really want AMDGPU_VA_RESERVED_SIZE *
> AMDGPU_VM_MAX_NUM_CTX here?
> 
> AMDGPU_CSA_SIZE * AMDGPU_VM_MAX_NUM_CTX should be sufficient I
> think.

Yes, You are right. I will fix this error.
Thanks.


> And we should name the function to amdgpu_csa_get_reserved_vm_space().
> 
> > +}
> > +
> >   uint64_t amdgpu_csa_vaddr(struct amdgpu_device *adev, uint32_t id)
> >   {
> >   	uint64_t addr = adev->vm_manager.max_pfn <<
> AMDGPU_GPU_PAGE_SHIFT;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.h
> > index aaf1fba..7159d6d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_csa.h
> > @@ -27,6 +27,7 @@
> >
> >   #define AMDGPU_CSA_SIZE		(128 * 1024)
> >
> > +uint64_t amdgpu_get_reserved_csa_size(struct amdgpu_device *adev);
> >   uint32_t amdgpu_get_total_csa_size(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, diff --git
> a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > index 7b3d1eb..4c12de8 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> > @@ -557,6 +557,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev,
> void *data,
> >   	struct ww_acquire_ctx ticket;
> >   	struct list_head list, duplicates;
> >   	uint64_t va_flags;
> > +	uint64_t va_reserved, va_top;
> >   	int r = 0;
> >
> >   	if (args->va_address < AMDGPU_VA_RESERVED_SIZE) { @@ -565,6
> +566,15
> > @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
> >   			args->va_address, AMDGPU_VA_RESERVED_SIZE);
> >   		return -EINVAL;
> >   	}
> > +	va_top = adev->vm_manager.max_pfn * AMDGPU_GPU_PAGE_SIZE;
> > +	va_reserved = va_top - adev->vm_manager.reserved_vm_size;
> > +
> > +	if (args->va_address > va_reserved && args->va_address < va_top) {
> > +		dev_dbg(&dev->pdev->dev,
> > +			"va_address 0x%LX is in reserved area 0x%LX\n",
> > +			args->va_address, adev-
> >vm_manager.reserved_vm_size);
> > +		return -EINVAL;
> > +	}
> >
> >   	if (args->va_address >= AMDGPU_GMC_HOLE_START &&
> >   	    args->va_address < AMDGPU_GMC_HOLE_END) { diff --git
> > a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > index f736bda..52e4e90 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> > @@ -702,7 +702,7 @@ static int amdgpu_info_ioctl(struct drm_device
> *dev, void *data, struct drm_file
> >   			dev_info.ids_flags |=
> AMDGPU_IDS_FLAGS_PREEMPTION;
> >
> >   		vm_size = adev->vm_manager.max_pfn *
> AMDGPU_GPU_PAGE_SIZE;
> > -		vm_size -= AMDGPU_VA_RESERVED_SIZE;
> > +		vm_size -= adev->vm_manager.reserved_vm_size;
> >
> >   		/* Older VCE FW versions are buggy and can handle only
> 40bits */
> >   		if (adev->vce.fw_version &&
> > @@ -977,7 +977,6 @@ 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, 1) &
> AMDGPU_GMC_HOLE_MASK;
> >   		r = amdgpu_map_static_csa(adev, &fpriv->vm, adev-
> >virt.csa_obj,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > index 352b304..2f2b9f4 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > @@ -2901,6 +2901,7 @@ void amdgpu_vm_adjust_size(struct
> amdgpu_device *adev, uint32_t min_vm_size,
> >   	}
> >
> >   	adev->vm_manager.max_pfn = (uint64_t)vm_size << 18;
> > +	adev->vm_manager.reserved_vm_size =
> > +amdgpu_get_reserved_csa_size(adev);
> 
> Maybe calculate that directly as max_pfn - reserved and name it something
> like max_user_pfn.

Good suggestion.
Thanks

Regards
Rex

> Regards,
> Christian.
> 
> >
> >   	tmp = roundup_pow_of_two(adev->vm_manager.max_pfn);
> >   	if (amdgpu_vm_block_size != -1)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > index 2a8898d..ec650ac 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> > @@ -260,6 +260,7 @@ struct amdgpu_vm_manager {
> >   	unsigned
> 	seqno[AMDGPU_MAX_RINGS];
> >
> >   	uint64_t				max_pfn;
> > +	uint64_t				reserved_vm_size;
> >   	uint32_t				num_level;
> >   	uint32_t				block_size;
> >   	uint32_t				fragment_size;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> > index 2821d1d..3351b56 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c
> > @@ -471,7 +471,7 @@ static void gmc_v6_0_set_prt(struct
> amdgpu_device *adev, bool enable)
> >   	if (enable) {
> >   		uint32_t low = AMDGPU_VA_RESERVED_SIZE >>
> AMDGPU_GPU_PAGE_SHIFT;
> >   		uint32_t high = adev->vm_manager.max_pfn -
> > -			(AMDGPU_VA_RESERVED_SIZE >>
> AMDGPU_GPU_PAGE_SHIFT);
> > +				(adev->vm_manager.reserved_vm_size >>
> AMDGPU_GPU_PAGE_SHIFT);
> >
> >   		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, low);
> >   		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, low); diff --
> git
> > a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > index 761dcfb..92bd175 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c
> > @@ -568,7 +568,7 @@ static void gmc_v7_0_set_prt(struct
> amdgpu_device *adev, bool enable)
> >   	if (enable) {
> >   		uint32_t low = AMDGPU_VA_RESERVED_SIZE >>
> AMDGPU_GPU_PAGE_SHIFT;
> >   		uint32_t high = adev->vm_manager.max_pfn -
> > -			(AMDGPU_VA_RESERVED_SIZE >>
> AMDGPU_GPU_PAGE_SHIFT);
> > +				(adev->vm_manager.reserved_vm_size >>
> AMDGPU_GPU_PAGE_SHIFT);
> >
> >   		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, low);
> >   		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, low); diff --
> git
> > a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > index 531aaf3..d909ddf 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c
> > @@ -772,7 +772,7 @@ static void gmc_v8_0_set_prt(struct
> amdgpu_device *adev, bool enable)
> >   	if (enable) {
> >   		uint32_t low = AMDGPU_VA_RESERVED_SIZE >>
> AMDGPU_GPU_PAGE_SHIFT;
> >   		uint32_t high = adev->vm_manager.max_pfn -
> > -			(AMDGPU_VA_RESERVED_SIZE >>
> AMDGPU_GPU_PAGE_SHIFT);
> > +				(adev->vm_manager.reserved_vm_size >>
> AMDGPU_GPU_PAGE_SHIFT);
> >
> >   		WREG32(mmVM_PRT_APERTURE0_LOW_ADDR, low);
> >   		WREG32(mmVM_PRT_APERTURE1_LOW_ADDR, low);



More information about the amd-gfx mailing list