[PATCH] drm/amdgpu: set start timestamp of fence in the right place

[Huang, Trigger]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of
> jiadong.zhu at amd.com
> Sent: Tuesday, July 9, 2024 2:56 PM
> To: amd-gfx at lists.freedesktop.org
> Cc: Zhu, Jiadong <Jiadong.Zhu at amd.com>
> 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 at amd.com>
>
> 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 at amd.com>
> ---
>  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