Re: [PATCH v4 06/10] drm/amdgpu: set FW parameters in v11_struct

["Christian König" <christian.koenig@amd.com>]

Am 24.04.23 um 19:38 schrieb Shashank Sharma:
> From: Arvind Yadav <arvind.yadav@amd.com>
>
> This patch sets new parameters defined in v11_struct for
> usermode queue with the respective allocated gpu context
> space addresses.
>
> V4: Split the patch into two parts, keep only the set parameter
>      part in this series (Alex).
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian Koenig <christian.koenig@amd.com>
> Cc: Shashank Sharma <shashank.sharma@amd.com>
> Signed-off-by: Arvind Yadav <arvind.yadav@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 19 ++++++++++++++++++-
>   1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> index f6b33faea86f..86de35292f69 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> @@ -6411,6 +6411,21 @@ const struct amdgpu_ip_block_version gfx_v11_0_ip_block =
>   	.funcs = &gfx_v11_0_ip_funcs,
>   };
>   
> +static void gfx_v11_userq_set_ctx_space(struct amdgpu_userq_mgr *uq_mgr,
> +					struct amdgpu_usermode_queue *queue)
> +{
> +	struct v11_gfx_mqd *mqd = queue->mqd.cpu_ptr;
> +
> +	mqd->shadow_base_lo = queue->shadow_ctx_gpu_addr & 0xfffffffc;
> +	mqd->shadow_base_hi = upper_32_bits(queue->shadow_ctx_gpu_addr);
> +
> +	mqd->gds_bkup_base_lo = queue->gds_ctx_gpu_addr & 0xfffffffc;
> +	mqd->gds_bkup_base_hi = upper_32_bits(queue->gds_ctx_gpu_addr);
> +
> +	mqd->fw_work_area_base_lo = queue->fw_ctx_gpu_addr & 0xfffffffc;
> +	mqd->fw_work_area_base_lo = upper_32_bits(queue->fw_ctx_gpu_addr);

Mhm, first of all please use lower_32_bits() here instead of the mask.

Then are you sure that is correct? The shadow, csa, gds are supposed to 
be VAs as far as I know, but you use MC addresses here.

Christian.

> +}
> +
>   static int gfx_v11_userq_create_ctx_space(struct amdgpu_userq_mgr *uq_mgr,
>   					  struct amdgpu_usermode_queue *queue)
>   {
> @@ -6484,12 +6499,14 @@ gfx_v11_userq_mqd_create(struct amdgpu_userq_mgr *uq_mgr, struct amdgpu_usermode
>   	queue->userq_prop.use_doorbell = true;
>   	queue->userq_prop.mqd_gpu_addr = mqd->gpu_addr;
>   	r = gfx_v11_mqd->init_mqd(adev, (void *)mqd->cpu_ptr, &queue->userq_prop);
> -	amdgpu_bo_unreserve(mqd->obj);
>   	if (r) {
> +		amdgpu_bo_unreserve(mqd->obj);
>   		DRM_ERROR("Failed to init MQD for queue\n");
>   		goto free_ctx;
>   	}
>   
> +	gfx_v11_userq_set_ctx_space(uq_mgr, queue);
> +	amdgpu_bo_unreserve(mqd->obj);
>   	DRM_DEBUG_DRIVER("MQD for queue %d created\n", queue->queue_id);
>   	return 0;
>