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

[Lucas De Marchi]

On Tue, Feb 06, 2024 at 12:11:50PM -0800, John.C.Harrison at Intel.com wrote:
>From: John Harrison <John.C.Harrison at 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 at Intel.com>
>---
> drivers/gpu/drm/xe/xe_gt_freq.c           | 18 +++++++++++--
> drivers/gpu/drm/xe/xe_gt_freq.h           |  4 +++
> drivers/gpu/drm/xe/xe_gt_throttle_sysfs.c | 31 ++++++++---------------
> 3 files changed, 30 insertions(+), 23 deletions(-)
>
>diff --git a/drivers/gpu/drm/xe/xe_gt_freq.c b/drivers/gpu/drm/xe/xe_gt_freq.c
>index e5b0f4ecdbe8..4cf7772c387f 100644
>--- a/drivers/gpu/drm/xe/xe_gt_freq.c
>+++ b/drivers/gpu/drm/xe/xe_gt_freq.c
>@@ -3,15 +3,17 @@
>  * Copyright © 2023 Intel Corporation
>  */
>
>-#include "xe_gt_freq.h"
>-

foo.c should always include foo.h as the first thing.

> #include <linux/kobject.h>
> #include <linux/sysfs.h>
>
> #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_freq.h"
> #include "xe_gt_sysfs.h"
> #include "xe_gt_throttle_sysfs.h"
> #include "xe_guc_pc.h"
>@@ -220,3 +222,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)


any function available to other compilation units should keep the prefix

xe_gt_freq.c ->  xe_gt_freq_XXXXXXXX()

>+{
>+	u32 reg;

this gives the impression you are getting the register offset, not its
value.


>+
>+	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;

does it make sense to share the function to read a register, but then
only return the raw value? What if the register change in the next
platform or what you are reading changes?

Lucas De Marchi

>+}
>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);
>+
> #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
>