Re: [PATCH v2 1/2] drm/xe: Make read_perf_limit_reasons globally accessible

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Feb 12, 2024 at 04:34:25PM -0800, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
> 
> Other driver code beyond the sysfs interface wants to know about
> throttling. So move the query function out of sysfs.
> 
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> ---
>  drivers/gpu/drm/xe/xe_gt_freq.c           | 15 +++++++++++
>  drivers/gpu/drm/xe/xe_gt_freq.h           |  4 +++
>  drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c | 31 ++++++++---------------
>  3 files changed, 29 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_gt_freq.c b/drivers/gpu/drm/xe/xe_gt_freq.c
> index e5b0f4ecdbe8..51645a24009e 100644
> --- a/drivers/gpu/drm/xe/xe_gt_freq.c
> +++ b/drivers/gpu/drm/xe/xe_gt_freq.c
> @@ -11,7 +11,10 @@
>  #include <drm/drm_managed.h>
>  #include <drm/drm_print.h>
>  
> +#include "regs/xe_gt_regs.h"
>  #include "xe_device_types.h"
> +#include "xe_mmio.h"
> +#include "xe_gt.h"
>  #include "xe_gt_sysfs.h"
>  #include "xe_gt_throttle_sysfs.h"
>  #include "xe_guc_pc.h"
> @@ -220,3 +223,15 @@ void xe_gt_freq_init(struct xe_gt *gt)
>  
>  	xe_gt_throttle_sysfs_init(gt);
>  }
> +
> +u32 xe_read_perf_limit_reasons(struct xe_gt *gt)
> +{
> +	u32 reg;
> +
> +	if (xe_gt_is_media_type(gt))
> +		reg = xe_mmio_read32(gt, MTL_MEDIA_PERF_LIMIT_REASONS);
> +	else
> +		reg = xe_mmio_read32(gt, GT0_PERF_LIMIT_REASONS);
> +
> +	return reg;
> +}
> diff --git a/drivers/gpu/drm/xe/xe_gt_freq.h b/drivers/gpu/drm/xe/xe_gt_freq.h
> index f3fe3c90491a..89be518b4967 100644
> --- a/drivers/gpu/drm/xe/xe_gt_freq.h
> +++ b/drivers/gpu/drm/xe/xe_gt_freq.h
> @@ -6,8 +6,12 @@
>  #ifndef _XE_GT_FREQ_H_
>  #define _XE_GT_FREQ_H_
>  
> +#include <linux/types.h>
> +
>  struct xe_gt;
>  
>  void xe_gt_freq_init(struct xe_gt *gt);
>  
> +u32 xe_read_perf_limit_reasons(struct xe_gt *gt);

this component is xe_gt_freq, not xe_ nor xe_read or xe_read_perf.
Please use the right namespace prefixes.

But besides the namespace, I don't believe that these limits
belong to xe_gt_freq anyway... why are you taking that from the
throttle reasons and moving here?

What about removing the 'sysfs' name from the xe_gt_throttle
component and making that to export a function
xe_gt_throttle_get_limit_reasons(struct xe_gt *gt)
?

Thanks,
Rodrigo.

> +
>  #endif
> diff --git a/drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c b/drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c
> index 63d640591a52..89d9f89962ad 100644
> --- a/drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c
> @@ -8,6 +8,7 @@
>  #include <regs/xe_gt_regs.h>
>  #include "xe_device.h"
>  #include "xe_gt.h"
> +#include "xe_gt_freq.h"
>  #include "xe_gt_sysfs.h"
>  #include "xe_gt_throttle_sysfs.h"
>  #include "xe_mmio.h"
> @@ -34,77 +35,65 @@ dev_to_gt(struct device *dev)
>  	return kobj_to_gt(dev->kobj.parent);
>  }
>  
> -static u32 read_perf_limit_reasons(struct xe_gt *gt)
> -{
> -	u32 reg;
> -
> -	if (xe_gt_is_media_type(gt))
> -		reg = xe_mmio_read32(gt, MTL_MEDIA_PERF_LIMIT_REASONS);
> -	else
> -		reg = xe_mmio_read32(gt, GT0_PERF_LIMIT_REASONS);
> -
> -	return reg;
> -}
> -
>  static u32 read_status(struct xe_gt *gt)
>  {
> -	u32 status = read_perf_limit_reasons(gt) & GT0_PERF_LIMIT_REASONS_MASK;
> +	u32 status = xe_read_perf_limit_reasons(gt) & GT0_PERF_LIMIT_REASONS_MASK;
>  
>  	return status;
>  }
>  
>  static u32 read_reason_pl1(struct xe_gt *gt)
>  {
> -	u32 pl1 = read_perf_limit_reasons(gt) & POWER_LIMIT_1_MASK;
> +	u32 pl1 = xe_read_perf_limit_reasons(gt) & POWER_LIMIT_1_MASK;
>  
>  	return pl1;
>  }
>  
>  static u32 read_reason_pl2(struct xe_gt *gt)
>  {
> -	u32 pl2 = read_perf_limit_reasons(gt) & POWER_LIMIT_2_MASK;
> +	u32 pl2 = xe_read_perf_limit_reasons(gt) & POWER_LIMIT_2_MASK;
>  
>  	return pl2;
>  }
>  
>  static u32 read_reason_pl4(struct xe_gt *gt)
>  {
> -	u32 pl4 = read_perf_limit_reasons(gt) & POWER_LIMIT_4_MASK;
> +	u32 pl4 = xe_read_perf_limit_reasons(gt) & POWER_LIMIT_4_MASK;
>  
>  	return pl4;
>  }
>  
>  static u32 read_reason_thermal(struct xe_gt *gt)
>  {
> -	u32 thermal = read_perf_limit_reasons(gt) & THERMAL_LIMIT_MASK;
> +	u32 thermal = xe_read_perf_limit_reasons(gt) & THERMAL_LIMIT_MASK;
>  
>  	return thermal;
>  }
>  
>  static u32 read_reason_prochot(struct xe_gt *gt)
>  {
> -	u32 prochot = read_perf_limit_reasons(gt) & PROCHOT_MASK;
> +	u32 prochot = xe_read_perf_limit_reasons(gt) & PROCHOT_MASK;
>  
>  	return prochot;
>  }
>  
>  static u32 read_reason_ratl(struct xe_gt *gt)
>  {
> -	u32 ratl = read_perf_limit_reasons(gt) & RATL_MASK;
> +	u32 ratl = xe_read_perf_limit_reasons(gt) & RATL_MASK;
>  
>  	return ratl;
>  }
>  
>  static u32 read_reason_vr_thermalert(struct xe_gt *gt)
>  {
> -	u32 thermalert = read_perf_limit_reasons(gt) & VR_THERMALERT_MASK;
> +	u32 thermalert = xe_read_perf_limit_reasons(gt) & VR_THERMALERT_MASK;
>  
>  	return thermalert;
>  }
>  
>  static u32 read_reason_vr_tdc(struct xe_gt *gt)
>  {
> -	u32 tdc = read_perf_limit_reasons(gt) & VR_TDC_MASK;
> +	u32 tdc = xe_read_perf_limit_reasons(gt) & VR_TDC_MASK;
>  
>  	return tdc;
>  }
> -- 
> 2.43.0
>