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

[Matt Roper]

On Thu, Oct 10, 2024 at 08:56:17PM -0700, 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.
> 
> Reviewed-by: Sai Teja Pottumuttu <sai.teja.pottumuttu at intel.com>
> 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 07093af0b32e3..69307ff4146a1 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,
> +		   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);

Not specifically related to this patch, but is there any reason why
we're not using xe_mmio_read64_2x32() to read the timestamp?  It already
has rollover logic specifically to handle timestamp reads like this.

Anyway, this patch looks fine.

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>

>  	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);
>  
> -- 
> 2.46.2
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation