Re: [PATCH] drm/amdgpu: enable bo priority setting from user space

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

Am 07.03.19 um 10:15 schrieb Chunming Zhou:
> Signed-off-by: Chunming Zhou <david1.zhou@xxxxxxx>

Well NAK to the whole approach.

The TTM priority is a global priority, but processes are only allowed to 
specific the priority inside their own allocations. So this approach 
will never fly upstream.

What you can do is to add a priority for per vm BOs to affect their sort 
order on the LRU, but I doubt that this will have much of an effect.

Regards,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c     |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    | 13 +++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h    |  2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c |  3 ++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  1 +
>   include/drm/ttm/ttm_bo_driver.h            |  9 ++++++++-
>   include/uapi/drm/amdgpu_drm.h              |  3 +++
>   7 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> index 5cbde74b97dd..70a6baf20c22 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
> @@ -144,6 +144,7 @@ static int amdgpufb_create_pinned_object(struct amdgpu_fbdev *rfbdev,
>   	size = mode_cmd->pitches[0] * height;
>   	aligned_size = ALIGN(size, PAGE_SIZE);
>   	ret = amdgpu_gem_object_create(adev, aligned_size, 0, domain,
> +				       TTM_BO_PRIORITY_NORMAL,
>   				       AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED |
>   				       AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
>   				       AMDGPU_GEM_CREATE_VRAM_CLEARED,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index d21dd2f369da..7c1c2362c67e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -44,6 +44,7 @@ void amdgpu_gem_object_free(struct drm_gem_object *gobj)
>   
>   int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
>   			     int alignment, u32 initial_domain,
> +			     enum ttm_bo_priority priority,
>   			     u64 flags, enum ttm_bo_type type,
>   			     struct reservation_object *resv,
>   			     struct drm_gem_object **obj)
> @@ -60,6 +61,7 @@ int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
>   	bp.type = type;
>   	bp.resv = resv;
>   	bp.preferred_domain = initial_domain;
> +	bp.priority = priority;
>   retry:
>   	bp.flags = flags;
>   	bp.domain = initial_domain;
> @@ -229,6 +231,14 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>   	if (args->in.domains & ~AMDGPU_GEM_DOMAIN_MASK)
>   		return -EINVAL;
>   
> +	/* check priority */
> +	if (args->in.priority == 0) {
> +		/* default is normal */
> +		args->in.priority = TTM_BO_PRIORITY_NORMAL;
> +	} else if (args->in.priority > TTM_MAX_BO_PRIORITY) {
> +		args->in.priority = TTM_MAX_BO_PRIORITY;
> +		DRM_ERROR("priority specified from user space is over MAX priority\n");
> +	}
>   	/* create a gem object to contain this object in */
>   	if (args->in.domains & (AMDGPU_GEM_DOMAIN_GDS |
>   	    AMDGPU_GEM_DOMAIN_GWS | AMDGPU_GEM_DOMAIN_OA)) {
> @@ -252,6 +262,7 @@ int amdgpu_gem_create_ioctl(struct drm_device *dev, void *data,
>   
>   	r = amdgpu_gem_object_create(adev, size, args->in.alignment,
>   				     (u32)(0xffffffff & args->in.domains),
> +				     args->in.priority - 1,
>   				     flags, ttm_bo_type_device, resv, &gobj);
>   	if (flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID) {
>   		if (!r) {
> @@ -304,6 +315,7 @@ int amdgpu_gem_userptr_ioctl(struct drm_device *dev, void *data,
>   
>   	/* create a gem object to contain this object in */
>   	r = amdgpu_gem_object_create(adev, args->size, 0, AMDGPU_GEM_DOMAIN_CPU,
> +				     TTM_BO_PRIORITY_NORMAL,
>   				     0, ttm_bo_type_device, NULL, &gobj);
>   	if (r)
>   		return r;
> @@ -755,6 +767,7 @@ int amdgpu_mode_dumb_create(struct drm_file *file_priv,
>   	domain = amdgpu_bo_get_preferred_pin_domain(adev,
>   				amdgpu_display_supported_domains(adev));
>   	r = amdgpu_gem_object_create(adev, args->size, 0, domain,
> +				     TTM_BO_PRIORITY_NORMAL,
>   				     AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED,
>   				     ttm_bo_type_device, NULL, &gobj);
>   	if (r)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
> index f1ddfc50bcc7..47b0a8190948 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.h
> @@ -61,7 +61,7 @@ extern const struct dma_buf_ops amdgpu_dmabuf_ops;
>    */
>   void amdgpu_gem_force_release(struct amdgpu_device *adev);
>   int amdgpu_gem_object_create(struct amdgpu_device *adev, unsigned long size,
> -			     int alignment, u32 initial_domain,
> +			     int alignment, u32 initial_domain, u32 priority,
>   			     u64 flags, enum ttm_bo_type type,
>   			     struct reservation_object *resv,
>   			     struct drm_gem_object **obj);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index fd9c4beeaaa4..c85304e03021 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -494,8 +494,9 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
>   
>   	bo->tbo.bdev = &adev->mman.bdev;
>   	amdgpu_bo_placement_from_domain(bo, bp->domain);
> +	bo->tbo.priority = bp->priority;
>   	if (bp->type == ttm_bo_type_kernel)
> -		bo->tbo.priority = 1;
> +		bo->tbo.priority = TTM_BO_PRIORITY_VERYHIGH;
>   
>   	r = ttm_bo_init_reserved(&adev->mman.bdev, &bo->tbo, size, bp->type,
>   				 &bo->placement, page_align, &ctx, acc_size,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index 9291c2f837e9..091a7884a821 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -39,6 +39,7 @@ struct amdgpu_bo_param {
>   	int				byte_align;
>   	u32				domain;
>   	u32				preferred_domain;
> +	u32				priority;
>   	u64				flags;
>   	enum ttm_bo_type		type;
>   	struct reservation_object	*resv;
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index cbf3180cb612..53f39ed540d7 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -43,7 +43,14 @@
>   #include "ttm_placement.h"
>   #include "ttm_tt.h"
>   
> -#define TTM_MAX_BO_PRIORITY	4U
> +enum ttm_bo_priority {
> +	TTM_BO_PRIORITY_VERYLOW = 0,
> +	TTM_BO_PRIORITY_LOW,
> +	TTM_BO_PRIORITY_NORMAL,
> +	TTM_BO_PRIORITY_HIGH,
> +	TTM_BO_PRIORITY_VERYHIGH,
> +	TTM_MAX_BO_PRIORITY
> +};
>   
>   #define TTM_MEMTYPE_FLAG_FIXED         (1 << 0)	/* Fixed (on-card) PCI memory */
>   #define TTM_MEMTYPE_FLAG_MAPPABLE      (1 << 1)	/* Memory mappable */
> diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
> index b7718bfdf8ad..b74a7583d7f3 100644
> --- a/include/uapi/drm/amdgpu_drm.h
> +++ b/include/uapi/drm/amdgpu_drm.h
> @@ -138,6 +138,9 @@ struct drm_amdgpu_gem_create_in  {
>   	__u64 domains;
>   	/** allocation flags */
>   	__u64 domain_flags;
> +	/** priority */
> +	__u32 priority;
> +	__u32 pad;
>   };
>   
>   struct drm_amdgpu_gem_create_out  {

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