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

["Koenig, Christian" <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 9047 bytes --]

Well you can already use the per submission priority for the BOs.

Additional to that as I said for per VM BOs we can add a priority to sort them in the LRU.

Not sure how effective both of those actually are.

Regards,
Christian.

Am 07.03.19 um 14:09 schrieb Zhou, David(ChunMing):
Yes, you are right, thanks to point it out. Will see if there is other way.

-David

send from my phone

-------- Original Message --------
Subject: Re: [PATCH] drm/amdgpu: enable bo priority setting from user space
From: Christian König
To: "Zhou, David(ChunMing)" ,amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
CC:

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

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  {



[-- Attachment #1.2: Type: text/html, Size: 19000 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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