Re: [PATCH 2/3] drm/xe/vf: Return error code from xe_hw_engine_read_timestamp()

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 04.02.2025 19:05, Marcin Bernatowicz wrote:
> Modify xe_hw_engine_read_timestamp() to return an error code instead
> of a u64 timestamp. Return -EOPNOTSUPP when called from Virtual Function
> (VF) mode to avoid reading the inaccessible RING_TIMESTAMP register.
> 
> Update function signature to pass the timestamp via a pointer argument.
> 
> Signed-off-by: Marcin Bernatowicz <marcin.bernatowicz@linux.intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_hw_engine.c | 10 ++++++++--
>  drivers/gpu/drm/xe/xe_hw_engine.h |  2 +-
>  2 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c b/drivers/gpu/drm/xe/xe_hw_engine.c
> index fc447751fe78..ab91c15f3a8b 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> @@ -981,9 +981,15 @@ const char *xe_hw_engine_class_to_str(enum xe_engine_class class)
>  	return NULL;
>  }
>  
> -u64 xe_hw_engine_read_timestamp(struct xe_hw_engine *hwe)
> +int xe_hw_engine_read_timestamp(struct xe_hw_engine *hwe, u64 *gpu_timestamp)
>  {
> -	return xe_mmio_read64_2x32(&hwe->gt->mmio, RING_TIMESTAMP(hwe->mmio_base));
> +	if (IS_SRIOV_VF(gt_to_xe(hwe->gt)))
> +		return -EOPNOTSUPP;

there seems to be 1 caller of this function
is it worth to make change here instead of fixing the caller ?

> +
> +	*gpu_timestamp = xe_mmio_read64_2x32(&hwe->gt->mmio,
> +					     RING_TIMESTAMP(hwe->mmio_base));
> +
> +	return 0;
>  }
>  
>  enum xe_force_wake_domains xe_hw_engine_to_fw_domain(struct xe_hw_engine *hwe)
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.h b/drivers/gpu/drm/xe/xe_hw_engine.h
> index 6b5f9fa2a594..df377da82770 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.h
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.h
> @@ -73,7 +73,7 @@ static inline bool xe_hw_engine_is_valid(struct xe_hw_engine *hwe)
>  }
>  
>  const char *xe_hw_engine_class_to_str(enum xe_engine_class class);
> -u64 xe_hw_engine_read_timestamp(struct xe_hw_engine *hwe);
> +int xe_hw_engine_read_timestamp(struct xe_hw_engine *hwe, u64 *gpu_timestamp);
>  enum xe_force_wake_domains xe_hw_engine_to_fw_domain(struct xe_hw_engine *hwe);
>  
>  void xe_hw_engine_mmio_write32(struct xe_hw_engine *hwe, struct xe_reg reg, u32 val);

and this patch will not compile without 3/3