[AMD Official Use Only - AMD Internal Distribution Only]
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
> jiadong.zhu@xxxxxxx
> Sent: Tuesday, July 9, 2024 2:56 PM
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Zhu, Jiadong <Jiadong.Zhu@xxxxxxx>
> Subject: [PATCH] drm/amdgpu: set start timestamp of fence in the right place
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> From: Jiadong Zhu <Jiadong.Zhu@xxxxxxx>
>
> The job's embedded fence is dma_fence which shall not be conversed to
> amdgpu_fence. The start timestamp shall be saved on job for hw_fence.
>
> Signed-off-by: Jiadong Zhu <Jiadong.Zhu@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 33
> ++++++++++++++++++++---
> drivers/gpu/drm/amd/amdgpu/amdgpu_job.h | 4 +++
> 2 files changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> index 2f24a6aa13bf..60c14d2e1c39 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> @@ -88,6 +88,33 @@ static inline struct amdgpu_fence
> *to_amdgpu_fence(struct dma_fence *f)
> return NULL;
> }
>
> +static inline void set_fence_start_time(struct dma_fence *f, ktime_t
> +start_timestamp) {
> + if (f->ops == &amdgpu_fence_ops) {
> + struct amdgpu_fence *__f = container_of(f, struct
> +amdgpu_fence, base);
> +
> + __f->start_timestamp = start_timestamp;
> + } else if (f->ops == &amdgpu_job_fence_ops) {
> + struct amdgpu_job *job = container_of(f, struct
> + amdgpu_job, hw_fence);
> +
> + job->start_timestamp = start_timestamp;
> + }
> +}
> +
> +static inline ktime_t get_fence_start_time(struct dma_fence *f) {
> + if (f->ops == &amdgpu_fence_ops) {
> + struct amdgpu_fence *__f = container_of(f, struct
> +amdgpu_fence, base);
> +
> + return __f->start_timestamp;
> + } else if (f->ops == &amdgpu_job_fence_ops) {
> + struct amdgpu_job *job = container_of(f, struct
> + amdgpu_job, hw_fence);
> +
> + return job->start_timestamp;
> + }
> + return ktime_get();
Here the invocation of ktime_get() should be unnecessary as start_timestamp in both amdgpu_fence_ops and amdgpu_job_fence_ops styles are already set by set_fence_start_time();
Regards,
Trigger
> +}
> +
> /**
> * amdgpu_fence_write - write a fence value
> *
> @@ -197,7 +224,7 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring,
> struct dma_fence **f, struct amd
> }
> }
>
> - to_amdgpu_fence(fence)->start_timestamp = ktime_get();
> + set_fence_start_time(fence, ktime_get());
>
> /* This function can't be called concurrently anyway, otherwise
> * emitting the fence would mess up the hardware ring buffer.
> @@ -428,7 +455,7 @@ u64 amdgpu_fence_last_unsignaled_time_us(struct
> amdgpu_ring *ring)
> return 0;
>
> return ktime_us_delta(ktime_get(),
> - to_amdgpu_fence(fence)->start_timestamp);
> + get_fence_start_time(fence));
> }
>
> /**
> @@ -451,7 +478,7 @@ void amdgpu_fence_update_start_timestamp(struct
> amdgpu_ring *ring, uint32_t seq,
> if (!fence)
> return;
>
> - to_amdgpu_fence(fence)->start_timestamp = timestamp;
> + set_fence_start_time(fence, timestamp);
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> index a963a25ddd62..68a80e31ba0c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> @@ -73,11 +73,15 @@ struct amdgpu_job {
> uint64_t gds_va;
> bool init_shadow;
>
> + /* start timestamp for hw_fence*/
> + ktime_t start_timestamp;
> +
> /* job_run_counter >= 1 means a resubmit job */
> uint32_t job_run_counter;
>
> uint32_t num_ibs;
> struct amdgpu_ib ibs[];
> +
> };
>
> static inline struct amdgpu_ring *amdgpu_job_ring(struct amdgpu_job *job)
> --
> 2.25.1
