Re: [RFC v1 2/2] drm/amdgpu: introduce helper amdgpu_bo_get_pinned_gpu_addr()

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

Am 14.01.25 um 10:54 schrieb Jiang Liu:
> Introduce helper amdgpu_bo_get_pinned_gpu_addr(), which will be
> used to update GPU address of pinned kernel BO during resume.

Clear NAK to the whole approach. Pinned means that the address *never* 
changes.

Hacks like those here are a complete no-go since some firmware uses the 
location of temporary buffers inside their firmware state.

So you always need to resume to the exact same location as it was before 
suspend.

I'm going to reply on the cover letter as well.

Regards,
Christian.

>
> Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.c   | 9 +++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_object.h   | 1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c | 9 +++++++++
>   3 files changed, 19 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index 4f057996ef35..bce939a63a99 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -1555,6 +1555,15 @@ u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo)
>   	return amdgpu_gmc_sign_extend(offset);
>   }
>   
> +/**
> + * amdgpu_bo_get_kernel_gpu_addr - get GPU address of pinned kernel BO
> + */
> +void amdgpu_bo_get_pinned_gpu_addr(struct amdgpu_bo *bo, u64 *gpu_addr)
> +{
> +	if (bo && bo->tbo.pin_count && gpu_addr)
> +		*gpu_addr = amdgpu_bo_gpu_offset(bo);
> +}
> +
>   /**
>    * amdgpu_bo_get_preferred_domain - get preferred domain
>    * @adev: amdgpu device object
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> index ab3fe7b42da7..9022592291a1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
> @@ -305,6 +305,7 @@ int amdgpu_bo_sync_wait_resv(struct amdgpu_device *adev, struct dma_resv *resv,
>   int amdgpu_bo_sync_wait(struct amdgpu_bo *bo, void *owner, bool intr);
>   u64 amdgpu_bo_gpu_offset(struct amdgpu_bo *bo);
>   u64 amdgpu_bo_gpu_offset_no_check(struct amdgpu_bo *bo);
> +void amdgpu_bo_get_pinned_gpu_addr(struct amdgpu_bo *bo, u64 *gpu_addr);
>   void amdgpu_bo_get_memory(struct amdgpu_bo *bo,
>   			  struct amdgpu_mem_stats *stats,
>   			  unsigned int size);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c
> index dde15c6a96e1..40605749b5d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_umsch_mm.c
> @@ -881,6 +881,15 @@ static int umsch_mm_suspend(struct amdgpu_ip_block *ip_block)
>   
>   static int umsch_mm_resume(struct amdgpu_ip_block *ip_block)
>   {
> +	struct amdgpu_device *adev = ip_block->adev;
> +
> +	adev->umsch_mm.sch_ctx_gpu_addr = adev->wb.gpu_addr +
> +					  (adev->umsch_mm.wb_index * 4);
> +	amdgpu_bo_get_pinned_gpu_addr(adev->umsch_mm.cmd_buf_obj,
> +				      &adev->umsch_mm.cmd_buf_gpu_addr);
> +	amdgpu_bo_get_pinned_gpu_addr(adev->umsch_mm.dbglog_bo,
> +				      &adev->umsch_mm.log_gpu_addr);
> +
>   	return umsch_mm_hw_init(ip_block);
>   }
>