On 2023-03-29 12:04, Shashank Sharma wrote: > From: Shashank Sharma <contactshashanksha...@gmail.com> > > The FW expects us to allocate atleast one page as context space to > process gang, process, shadow, 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 > > Cc: Alex Deucher <alexander.deuc...@amd.com> > Cc: Christian Koenig <christian.koe...@amd.com> > Signed-off-by: Shashank Sharma <shashank.sha...@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c | 1 + > .../drm/amd/amdgpu/amdgpu_userqueue_gfx_v11.c | 60 ++++++++++++++++++- > .../gpu/drm/amd/include/amdgpu_userqueue.h | 7 +++ > 3 files changed, 66 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c > index 052c2c1e8aed..5672efcbcffc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue.c > @@ -71,6 +71,7 @@ static int amdgpu_userqueue_create(struct drm_file *filp, > union drm_amdgpu_userq > queue->userq_prop.queue_size = mqd_in->queue_size; > > queue->doorbell_handle = mqd_in->doorbell_handle; > + queue->shadow_ctx_gpu_addr = mqd_in->shadow_va; > queue->queue_type = mqd_in->ip_type; > queue->flags = mqd_in->flags; > queue->vm = &fpriv->vm; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue_gfx_v11.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue_gfx_v11.c > index 12e1a785b65a..52de96727f98 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue_gfx_v11.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userqueue_gfx_v11.c > @@ -23,6 +23,51 @@ > #include "amdgpu.h" > #include "amdgpu_userqueue.h" > > +#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 > +
Leaving a single space after the macro name and its value makes it hard to read. Please align the value columns and leave at least 3 spaces--this would make it readable. For instance, #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 Regards, Luben > +static int amdgpu_userq_gfx_v11_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 atleast one page space allocated for > + * process ctx, gang ctx, gds ctx, fw ctx and shadow ctx each. > + */ > + size = AMDGPU_USERQ_PROC_CTX_SZ + AMDGPU_USERQ_GANG_CTX_SZ + > + AMDGPU_USERQ_FW_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; > +} > + > +static void amdgpu_userq_gfx_v11_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 > amdgpu_userq_gfx_v11_mqd_create(struct amdgpu_userq_mgr *uq_mgr, struct > amdgpu_usermode_queue *queue) > { > @@ -43,10 +88,17 @@ amdgpu_userq_gfx_v11_mqd_create(struct amdgpu_userq_mgr > *uq_mgr, struct amdgpu_u > } > > memset(mqd->cpu_ptr, 0, size); > + > + r = amdgpu_userq_gfx_v11_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; > @@ -55,12 +107,15 @@ amdgpu_userq_gfx_v11_mqd_create(struct amdgpu_userq_mgr > *uq_mgr, struct amdgpu_u > 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: > + amdgpu_userq_gfx_v11_destroy_ctx_space(uq_mgr, queue); > + > free_mqd: > amdgpu_bo_free_kernel(&mqd->obj, > &mqd->gpu_addr, > @@ -73,6 +128,7 @@ amdgpu_userq_gfx_v11_mqd_destroy(struct amdgpu_userq_mgr > *uq_mgr, struct amdgpu_ > { > struct amdgpu_userq_ctx_space *mqd = &queue->mqd; > > + amdgpu_userq_gfx_v11_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 2911c88d0fed..8b62ef77cd26 100644 > --- a/drivers/gpu/drm/amd/include/amdgpu_userqueue.h > +++ b/drivers/gpu/drm/amd/include/amdgpu_userqueue.h > @@ -38,10 +38,17 @@ struct amdgpu_usermode_queue { > int queue_type; > uint64_t flags; > 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; > + > struct amdgpu_vm *vm; > struct amdgpu_userq_mgr *userq_mgr; > struct amdgpu_mqd_prop userq_prop; > struct amdgpu_userq_ctx_space mqd; > + struct amdgpu_userq_ctx_space fw_space; > }; > > struct amdgpu_userq_funcs {