[PATCH 2/3] drm/xe/query: Move timestamp reg to hwe_read_timestamp()

[Lucas De Marchi]

On Thu, Oct 10, 2024 at 03:33:30PM +0530, Pottumuttu, Sai Teja wrote:
>
>On 10-10-2024 09:18, Lucas De Marchi wrote:
>>__read_timestamps() is actually reading the timestamp from a certain
>>hwe. Use it as parameter, move register declarations to be inside that
>>function and rename it.
>>
>>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>---
>>  drivers/gpu/drm/xe/xe_query.c | 22 +++++++---------------
>>  1 file changed, 7 insertions(+), 15 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
>>index a1f4cc25bea68..d86959076b078 100644
>>--- a/drivers/gpu/drm/xe/xe_query.c
>>+++ b/drivers/gpu/drm/xe/xe_query.c
>>@@ -85,16 +85,13 @@ static __ktime_func_t __clock_id_to_func(clockid_t clk_id)
>>  }
>>  static void
>>-__read_timestamps(struct xe_gt *gt,
>>-		  struct xe_reg lower_reg,
>>-		  struct xe_reg upper_reg,
>>-		  u64 *engine_ts,
>>-		  u64 *cpu_ts,
>>-		  u64 *cpu_delta,
>>-		  __ktime_func_t cpu_clock)
>>+hwe_read_timestamp(struct xe_hw_engine *hwe, u64 *engine_ts, u64 *cpu_ts,
>
>Nit: Probably the function name should be hwe_read_timestamps (s at 
>last), just like the previous name and also as its reads multiple 
>timestamps I believe.

I went back and forth on the name... this function is actually reading
the engine timestamp and the additional args for CPU timestamp/delta are
only reporting "how long did it take to read the engine timestamp"

so.... the change was intentional.

>
>With that LGTM,
>
>Reviewed-by: Sai Teja Pottumuttu <sai.teja.pottumuttu at intel.com>

thanks
Lucas De Marchi

>
>>+		   u64 *cpu_delta, __ktime_func_t cpu_clock)
>>  {
>>-	struct xe_mmio *mmio = &gt->mmio;
>>+	struct xe_mmio *mmio = &hwe->gt->mmio;
>>  	u32 upper, lower, old_upper, loop = 0;
>>+	struct xe_reg upper_reg = RING_TIMESTAMP_UDW(hwe->mmio_base),
>>+		      lower_reg = RING_TIMESTAMP(hwe->mmio_base);
>>  	upper = xe_mmio_read32(mmio, upper_reg);
>>  	do {
>>@@ -155,13 +152,8 @@ query_engine_cycles(struct xe_device *xe,
>>  	if (xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL))
>>  		return -EIO;
>>-	__read_timestamps(gt,
>>-			  RING_TIMESTAMP(hwe->mmio_base),
>>-			  RING_TIMESTAMP_UDW(hwe->mmio_base),
>>-			  &resp.engine_cycles,
>>-			  &resp.cpu_timestamp,
>>-			  &resp.cpu_delta,
>>-			  cpu_clock);
>>+	hwe_read_timestamp(hwe, &resp.engine_cycles, &resp.cpu_timestamp,
>>+			   &resp.cpu_delta, cpu_clock);
>>  	xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);