Re: [Intel-xe] [PATCH v2 19/20] drm/xe/uapi: Crystal Reference Clock updates

["Souza, Jose" <jose.souza@intel.com>]

On Wed, 2023-09-20 at 15:29 -0400, Rodrigo Vivi wrote:
> First of all, let's remove the duplication.
> But also, let's rename it to remove the word 'frequency'
> out of it. In general, the first thing people think of frequency
> is the frequency in which the GTs are operating to execute the
> GPU instructions.
> 
> While this frequency here is a crystal reference clock frequency
> which is the base of everything else, and in this case of this
> uAPI it is used to calculate a better and precise timestamp.
> 
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> Cc: Francois Dugast <francois.dugast@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Francois Dugast <francois.dugast@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_query.c |  5 ++---
>  include/uapi/drm/xe_drm.h     | 10 ++++------
>  2 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c
> index a27b10583f5a..8184df4abde1 100644
> --- a/drivers/gpu/drm/xe/xe_query.c
> +++ b/drivers/gpu/drm/xe/xe_query.c
> @@ -149,7 +149,7 @@ query_cs_cycles(struct xe_device *xe,
>  	if (!hwe)
>  		return -EINVAL;
>  
> -	resp.cs_frequency = gt->info.clock_freq;
> +	resp.cs_reference_clock = gt->info.clock_freq;
>  
>  	xe_device_mem_access_get(xe);
>  	xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
> @@ -167,7 +167,7 @@ query_cs_cycles(struct xe_device *xe,
>  	resp.width = 36;
>  
>  	/* Only write to the output fields of user query */
> -	if (put_user(resp.cs_frequency, &query_ptr->cs_frequency))
> +	if (put_user(resp.cs_reference_clock, &query_ptr->cs_reference_clock))
>  		return -EFAULT;

In my opinion the renames around drm_xe_query_cs_cycles should be done in the patch that implements it.

>  
>  	if (put_user(resp.cpu_timestamp, &query_ptr->cpu_timestamp))
> @@ -380,7 +380,6 @@ static int query_gt_list(struct xe_device *xe, struct drm_xe_device_query *query
>  		else
>  			gt_list->gt_list[id].type = XE_QUERY_GT_TYPE_MAIN;
>  		gt_list->gt_list[id].gt_id = gt->info.id;
> -		gt_list->gt_list[id].clock_freq = gt->info.clock_freq;
>  		if (!IS_DGFX(xe))
>  			gt_list->gt_list[id].native_mem_regions = 0x1;
>  		else
> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> index 45ec20c2765d..5156bac84f20 100644
> --- a/include/uapi/drm/xe_drm.h
> +++ b/include/uapi/drm/xe_drm.h
> @@ -248,8 +248,8 @@ struct drm_xe_query_mem_region {
>   * relevant GPU timestamp. clockid is used to return the specific CPU
>   * timestamp.
>   *
> - * The query returns the command streamer cycles and the frequency that can
> - * be used to calculate the command streamer timestamp. In addition the
> + * The query returns the command streamer cycles and the reference clock that
> + * can be used to calculate the command streamer timestamp. In addition the
>   * query returns a set of cpu timestamps that indicate when the command
>   * streamer cycle count was captured.
>   */
> @@ -266,8 +266,8 @@ struct drm_xe_query_cs_cycles {
>  	 */
>  	__u64 cs_cycles;
>  
> -	/** Frequency of the cs cycles in Hz. */
> -	__u64 cs_frequency;
> +	/** Reference Clock of the cs cycles in Hz. */
> +	__u64 cs_reference_clock;
>  
>  	/**
>  	 * CPU timestamp in ns. The timestamp is captured before reading the
> @@ -381,8 +381,6 @@ struct drm_xe_query_gt {
>  	__u16 type;
>  	/** @gt_id: Unique ID of this GT within the PCI Device */
>  	__u16 gt_id;
> -	/** @clock_freq: A clock frequency for timestamp */
> -	__u32 clock_freq;
>  	/**
>  	 * @native_mem_regions: Bit mask of instances from
>  	 * drm_xe_query_mem_usage that lives on the same GPU/Tile and have