Re: [PATCH v4 05/10] drm/amdgpu: create context space for usermode queue

[Shashank Sharma <shashank.sharma@amd.com>]

On 19/05/2023 23:21, Alex Deucher wrote:
> On Mon, Apr 24, 2023 at 1:39 PM Shashank Sharma <shashank.sharma@amd.com> wrote:
>> The FW expects us to allocate at least one page as context
>> space to process gang, process, GDS and FW  related work.
>> This patch creates a joint object for the same, and calculates
>> GPU space offsets for each of these spaces.
>>
>> V1: Addressed review comments on RFC patch:
>>      Alex: Make this function IP specific
>>
>> V2: Addressed review comments from Christian
>>      - Allocate only one object for total FW space, and calculate
>>        offsets for each of these objects.
>>
>> V3: Integration with doorbell manager
>> V4: Review comments:
>>      - Remove shadow from FW space list from cover letter (Alex)
>>      - Alignment of macro (Luben)
>>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Christian Koenig <christian.koenig@amd.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c        | 57 ++++++++++++++++++-
>>   .../gpu/drm/amd/include/amdgpu_userqueue.h    |  6 ++
>>   2 files changed, 61 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
>> index 9f7b14966ac8..f6b33faea86f 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
>> @@ -53,6 +53,11 @@
>>   #define GFX11_NUM_GFX_RINGS            1
>>   #define GFX11_MEC_HPD_SIZE     2048
>>
>> +#define AMDGPU_USERQ_PROC_CTX_SZ   PAGE_SIZE
>> +#define AMDGPU_USERQ_GANG_CTX_SZ   PAGE_SIZE
>> +#define AMDGPU_USERQ_FW_CTX_SZ     PAGE_SIZE
>> +#define AMDGPU_USERQ_GDS_CTX_SZ    PAGE_SIZE
>> +
>>   #define RLCG_UCODE_LOADING_START_ADDRESS       0x00002000L
>>   #define RLC_PG_DELAY_3_DEFAULT_GC_11_0_1       0x1388
>>
>> @@ -6406,6 +6411,44 @@ const struct amdgpu_ip_block_version gfx_v11_0_ip_block =
>>          .funcs = &gfx_v11_0_ip_funcs,
>>   };
>>
>> +static int gfx_v11_userq_create_ctx_space(struct amdgpu_userq_mgr *uq_mgr,
>> +                                         struct amdgpu_usermode_queue *queue)
>> +{
>> +       struct amdgpu_device *adev = uq_mgr->adev;
>> +       struct amdgpu_userq_ctx_space *ctx = &queue->fw_space;
>> +       int r, size;
>> +
>> +       /*
>> +        * The FW expects at least one page space allocated for
>> +        * process ctx, gang ctx, gds ctx, fw ctx each.
>> +        */
>> +       size = AMDGPU_USERQ_PROC_CTX_SZ + AMDGPU_USERQ_FW_CTX_SZ +
>> +              AMDGPU_USERQ_GANG_CTX_SZ + AMDGPU_USERQ_GDS_CTX_SZ;
>> +       r = amdgpu_bo_create_kernel(adev, size, PAGE_SIZE,
>> +                                   AMDGPU_GEM_DOMAIN_GTT,
>> +                                   &ctx->obj,
>> +                                   &ctx->gpu_addr,
>> +                                   &ctx->cpu_ptr);
>> +       if (r) {
>> +               DRM_ERROR("Failed to allocate ctx space bo for userqueue, err:%d\n", r);
>> +               return r;
>> +       }
>> +
>> +       queue->proc_ctx_gpu_addr = ctx->gpu_addr;
>> +       queue->gang_ctx_gpu_addr = queue->proc_ctx_gpu_addr + AMDGPU_USERQ_PROC_CTX_SZ;
>> +       queue->fw_ctx_gpu_addr = queue->gang_ctx_gpu_addr + AMDGPU_USERQ_GANG_CTX_SZ;
>> +       queue->gds_ctx_gpu_addr = queue->fw_ctx_gpu_addr + AMDGPU_USERQ_FW_CTX_SZ;
>> +       return 0;
>> +}
> Please use gfx_v11_0_ naming for consistency.

Noted,

- Shashank

>
> Alex
>
>> +
>> +static void gfx_v11_userq_destroy_ctx_space(struct amdgpu_userq_mgr *uq_mgr,
>> +                                           struct amdgpu_usermode_queue *queue)
>> +{
>> +       struct amdgpu_userq_ctx_space *ctx = &queue->fw_space;
>> +
>> +       amdgpu_bo_free_kernel(&ctx->obj, &ctx->gpu_addr, &ctx->cpu_ptr);
>> +}
>> +
>>   static int
>>   gfx_v11_userq_mqd_create(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode_queue *queue)
>>   {
>> @@ -6426,10 +6469,16 @@ gfx_v11_userq_mqd_create(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode
>>          }
>>
>>          memset(mqd->cpu_ptr, 0, size);
>> +       r = gfx_v11_userq_create_ctx_space(uq_mgr, queue);
>> +       if (r) {
>> +               DRM_ERROR("Failed to create CTX space for userqueue (%d)\n", r);
>> +               goto free_mqd;
>> +       }
>> +
>>          r = amdgpu_bo_reserve(mqd->obj, false);
>>          if (unlikely(r != 0)) {
>>                  DRM_ERROR("Failed to reserve mqd for userqueue (%d)", r);
>> -               goto free_mqd;
>> +               goto free_ctx;
>>          }
>>
>>          queue->userq_prop.use_doorbell = true;
>> @@ -6438,12 +6487,15 @@ gfx_v11_userq_mqd_create(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode
>>          amdgpu_bo_unreserve(mqd->obj);
>>          if (r) {
>>                  DRM_ERROR("Failed to init MQD for queue\n");
>> -               goto free_mqd;
>> +               goto free_ctx;
>>          }
>>
>>          DRM_DEBUG_DRIVER("MQD for queue %d created\n", queue->queue_id);
>>          return 0;
>>
>> +free_ctx:
>> +       gfx_v11_userq_destroy_ctx_space(uq_mgr, queue);
>> +
>>   free_mqd:
>>          amdgpu_bo_free_kernel(&mqd->obj, &mqd->gpu_addr, &mqd->cpu_ptr);
>>          return r;
>> @@ -6454,6 +6506,7 @@ gfx_v11_userq_mqd_destroy(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermod
>>   {
>>          struct amdgpu_userq_ctx_space *mqd = &queue->mqd;
>>
>> +       gfx_v11_userq_destroy_ctx_space(uq_mgr, queue);
>>          amdgpu_bo_free_kernel(&mqd->obj, &mqd->gpu_addr, &mqd->cpu_ptr);
>>   }
>>
>> diff --git a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
>> index e7da27918bd2..11e8ad649f6e 100644
>> --- a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
>> +++ b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h
>> @@ -38,11 +38,17 @@ struct amdgpu_usermode_queue {
>>          int                     queue_id;
>>          int                     queue_type;
>>          uint64_t                doorbell_handle;
>> +       uint64_t                proc_ctx_gpu_addr;
>> +       uint64_t                gang_ctx_gpu_addr;
>> +       uint64_t                gds_ctx_gpu_addr;
>> +       uint64_t                fw_ctx_gpu_addr;
>> +       uint64_t                shadow_ctx_gpu_addr;
>>          uint64_t                flags;
>>          struct amdgpu_mqd_prop  userq_prop;
>>          struct amdgpu_userq_mgr *userq_mgr;
>>          struct amdgpu_vm        *vm;
>>          struct amdgpu_userq_ctx_space mqd;
>> +       struct amdgpu_userq_ctx_space fw_space;
>>   };
>>
>>   struct amdgpu_userq_funcs {
>> --
>> 2.40.0
>>