All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
@ 2022-06-23 23:33 ` Vinay Belgaumkar
  0 siblings, 0 replies; 10+ messages in thread
From: Vinay Belgaumkar @ 2022-06-23 23:33 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: Ashutosh Dixit, Vinay Belgaumkar

This test will validate we can achieve actual frequency of RP0. Pcode
grants frequencies based on what GuC is requesting. However, thermal
throttling can limit what is being granted. Add a test to request for
max, but don't fail the test if RP0 is not granted due to throttle
reasons.

Also optimize the selftest by using a common run_test function to avoid
code duplication. Rename the "clamp" tests to vary_max_freq and vary_min_freq.

v2: Fix compile warning
v3: Review comments (Ashutosh). Added a FIXME for the media RP0 case.

Fixes 8ee2c227822e ("drm/i915/guc/slpc: Add SLPC selftest")

Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_slpc.c | 324 ++++++++++++------------
 1 file changed, 157 insertions(+), 167 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
index b768cea5943d..2b8c10b4e38a 100644
--- a/drivers/gpu/drm/i915/gt/selftest_slpc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_slpc.c
@@ -8,6 +8,11 @@
 #define delay_for_h2g() usleep_range(H2G_DELAY, H2G_DELAY + 10000)
 #define FREQUENCY_REQ_UNIT	DIV_ROUND_CLOSEST(GT_FREQUENCY_MULTIPLIER, \
 						  GEN9_FREQ_SCALER)
+enum test_type {
+	VARY_MIN,
+	VARY_MAX,
+	MAX_GRANTED
+};
 
 static int slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 freq)
 {
@@ -36,147 +41,115 @@ static int slpc_set_max_freq(struct intel_guc_slpc *slpc, u32 freq)
 	return ret;
 }
 
-static int live_slpc_clamp_min(void *arg)
+static int vary_max_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps,
+			 u32 *max_act_freq)
 {
-	struct drm_i915_private *i915 = arg;
-	struct intel_gt *gt = to_gt(i915);
-	struct intel_guc_slpc *slpc = &gt->uc.guc.slpc;
-	struct intel_rps *rps = &gt->rps;
-	struct intel_engine_cs *engine;
-	enum intel_engine_id id;
-	struct igt_spinner spin;
-	u32 slpc_min_freq, slpc_max_freq;
+	u32 step, max_freq, req_freq;
+	u32 act_freq;
 	int err = 0;
 
-	if (!intel_uc_uses_guc_slpc(&gt->uc))
-		return 0;
+	/* Go from max to min in 5 steps */
+	step = (slpc->rp0_freq - slpc->min_freq) / NUM_STEPS;
+	*max_act_freq = slpc->min_freq;
+	for (max_freq = slpc->rp0_freq; max_freq > slpc->min_freq;
+				max_freq -= step) {
+		err = slpc_set_max_freq(slpc, max_freq);
+		if (err)
+			break;
 
-	if (igt_spinner_init(&spin, gt))
-		return -ENOMEM;
+		req_freq = intel_rps_read_punit_req_frequency(rps);
 
-	if (intel_guc_slpc_get_max_freq(slpc, &slpc_max_freq)) {
-		pr_err("Could not get SLPC max freq\n");
-		return -EIO;
-	}
+		/* GuC requests freq in multiples of 50/3 MHz */
+		if (req_freq > (max_freq + FREQUENCY_REQ_UNIT)) {
+			pr_err("SWReq is %d, should be at most %d\n", req_freq,
+				max_freq + FREQUENCY_REQ_UNIT);
+			err = -EINVAL;
+		}
 
-	if (intel_guc_slpc_get_min_freq(slpc, &slpc_min_freq)) {
-		pr_err("Could not get SLPC min freq\n");
-		return -EIO;
-	}
+		act_freq =  intel_rps_read_actual_frequency(rps);
+		if (act_freq > *max_act_freq)
+			*max_act_freq = act_freq;
 
-	if (slpc_min_freq == slpc_max_freq) {
-		pr_err("Min/Max are fused to the same value\n");
-		return -EINVAL;
+		if (err)
+			break;
 	}
 
-	intel_gt_pm_wait_for_idle(gt);
-	intel_gt_pm_get(gt);
-	for_each_engine(engine, gt, id) {
-		struct i915_request *rq;
-		u32 step, min_freq, req_freq;
-		u32 act_freq, max_act_freq;
+	return err;
+}
 
-		if (!intel_engine_can_store_dword(engine))
-			continue;
+static int vary_min_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps,
+			 u32 *max_act_freq)
+{
+	u32 step, min_freq, req_freq;
+	u32 act_freq;
+	int err = 0;
 
-		/* Go from min to max in 5 steps */
-		step = (slpc_max_freq - slpc_min_freq) / NUM_STEPS;
-		max_act_freq = slpc_min_freq;
-		for (min_freq = slpc_min_freq; min_freq < slpc_max_freq;
-					min_freq += step) {
-			err = slpc_set_min_freq(slpc, min_freq);
-			if (err)
-				break;
-
-			st_engine_heartbeat_disable(engine);
-
-			rq = igt_spinner_create_request(&spin,
-							engine->kernel_context,
-							MI_NOOP);
-			if (IS_ERR(rq)) {
-				err = PTR_ERR(rq);
-				st_engine_heartbeat_enable(engine);
-				break;
-			}
+	/* Go from min to max in 5 steps */
+	step = (slpc->rp0_freq - slpc->min_freq) / NUM_STEPS;
+	*max_act_freq = slpc->min_freq;
+	for (min_freq = slpc->min_freq; min_freq < slpc->rp0_freq;
+				min_freq += step) {
+		err = slpc_set_min_freq(slpc, min_freq);
+		if (err)
+			break;
 
-			i915_request_add(rq);
+		req_freq = intel_rps_read_punit_req_frequency(rps);
 
-			if (!igt_wait_for_spinner(&spin, rq)) {
-				pr_err("%s: Spinner did not start\n",
-				       engine->name);
-				igt_spinner_end(&spin);
-				st_engine_heartbeat_enable(engine);
-				intel_gt_set_wedged(engine->gt);
-				err = -EIO;
-				break;
-			}
+		/* GuC requests freq in multiples of 50/3 MHz */
+		if (req_freq < (min_freq - FREQUENCY_REQ_UNIT)) {
+			pr_err("SWReq is %d, should be at least %d\n", req_freq,
+				min_freq - FREQUENCY_REQ_UNIT);
+			err = -EINVAL;
+		}
 
-			/* Wait for GuC to detect business and raise
-			 * requested frequency if necessary.
-			 */
-			delay_for_h2g();
+		act_freq =  intel_rps_read_actual_frequency(rps);
+		if (act_freq > *max_act_freq)
+			*max_act_freq = act_freq;
 
-			req_freq = intel_rps_read_punit_req_frequency(rps);
+		if (err)
+			break;
+	}
 
-			/* GuC requests freq in multiples of 50/3 MHz */
-			if (req_freq < (min_freq - FREQUENCY_REQ_UNIT)) {
-				pr_err("SWReq is %d, should be at least %d\n", req_freq,
-				       min_freq - FREQUENCY_REQ_UNIT);
-				igt_spinner_end(&spin);
-				st_engine_heartbeat_enable(engine);
-				err = -EINVAL;
-				break;
-			}
+	return err;
+}
 
-			act_freq =  intel_rps_read_actual_frequency(rps);
-			if (act_freq > max_act_freq)
-				max_act_freq = act_freq;
+static int max_granted_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps, u32 *max_act_freq)
+{
+	struct intel_gt *gt = rps_to_gt(rps);
+	u32 perf_limit_reasons;
+	int err = 0;
 
-			igt_spinner_end(&spin);
-			st_engine_heartbeat_enable(engine);
-		}
+	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
+	if (err)
+		return err;
 
-		pr_info("Max actual frequency for %s was %d\n",
-			engine->name, max_act_freq);
+	*max_act_freq =  intel_rps_read_actual_frequency(rps);
+	if (!(*max_act_freq == slpc->rp0_freq)) {
+		/* Check if there was some throttling by pcode */
+		perf_limit_reasons = intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS);
 
-		/* Actual frequency should rise above min */
-		if (max_act_freq == slpc_min_freq) {
-			pr_err("Actual freq did not rise above min\n");
+		/* If not, this is an error */
+		if (!(perf_limit_reasons && GT0_PERF_LIMIT_REASONS_MASK)) {
+			pr_err("Pcode did not grant max freq\n");
 			err = -EINVAL;
-		}
+		} else {
+			pr_info("Pcode throttled frequency 0x%x\n", perf_limit_reasons);
 
-		if (err)
-			break;
+		}
 	}
 
-	/* Restore min/max frequencies */
-	slpc_set_max_freq(slpc, slpc_max_freq);
-	slpc_set_min_freq(slpc, slpc_min_freq);
-
-	if (igt_flush_test(gt->i915))
-		err = -EIO;
-
-	intel_gt_pm_put(gt);
-	igt_spinner_fini(&spin);
-	intel_gt_pm_wait_for_idle(gt);
-
 	return err;
 }
 
-static int live_slpc_clamp_max(void *arg)
+static int run_test(struct intel_gt *gt, int test_type)
 {
-	struct drm_i915_private *i915 = arg;
-	struct intel_gt *gt = to_gt(i915);
-	struct intel_guc_slpc *slpc;
-	struct intel_rps *rps;
+	struct intel_guc_slpc *slpc = &gt->uc.guc.slpc;
+	struct intel_rps *rps = &gt->rps;
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 	struct igt_spinner spin;
-	int err = 0;
 	u32 slpc_min_freq, slpc_max_freq;
-
-	slpc = &gt->uc.guc.slpc;
-	rps = &gt->rps;
+	int err = 0;
 
 	if (!intel_uc_uses_guc_slpc(&gt->uc))
 		return 0;
@@ -203,105 +176,122 @@ static int live_slpc_clamp_max(void *arg)
 	intel_gt_pm_get(gt);
 	for_each_engine(engine, gt, id) {
 		struct i915_request *rq;
-		u32 max_freq, req_freq;
-		u32 act_freq, max_act_freq;
-		u32 step;
+		u32 max_act_freq;
 
 		if (!intel_engine_can_store_dword(engine))
 			continue;
 
-		/* Go from max to min in 5 steps */
-		step = (slpc_max_freq - slpc_min_freq) / NUM_STEPS;
-		max_act_freq = slpc_min_freq;
-		for (max_freq = slpc_max_freq; max_freq > slpc_min_freq;
-					max_freq -= step) {
-			err = slpc_set_max_freq(slpc, max_freq);
-			if (err)
-				break;
-
-			st_engine_heartbeat_disable(engine);
-
-			rq = igt_spinner_create_request(&spin,
-							engine->kernel_context,
-							MI_NOOP);
-			if (IS_ERR(rq)) {
-				st_engine_heartbeat_enable(engine);
-				err = PTR_ERR(rq);
-				break;
-			}
+		st_engine_heartbeat_disable(engine);
 
-			i915_request_add(rq);
+		rq = igt_spinner_create_request(&spin,
+						engine->kernel_context,
+						MI_NOOP);
+		if (IS_ERR(rq)) {
+			err = PTR_ERR(rq);
+			st_engine_heartbeat_enable(engine);
+			break;
+		}
 
-			if (!igt_wait_for_spinner(&spin, rq)) {
-				pr_err("%s: SLPC spinner did not start\n",
-				       engine->name);
-				igt_spinner_end(&spin);
-				st_engine_heartbeat_enable(engine);
-				intel_gt_set_wedged(engine->gt);
-				err = -EIO;
-				break;
-			}
+		i915_request_add(rq);
+
+		if (!igt_wait_for_spinner(&spin, rq)) {
+			pr_err("%s: Spinner did not start\n",
+			       engine->name);
+			igt_spinner_end(&spin);
+			st_engine_heartbeat_enable(engine);
+			intel_gt_set_wedged(engine->gt);
+			err = -EIO;
+			break;
+		}
 
-			delay_for_h2g();
+		switch (test_type) {
 
-			/* Verify that SWREQ indeed was set to specific value */
-			req_freq = intel_rps_read_punit_req_frequency(rps);
+		case VARY_MIN:
+			err = vary_min_freq(slpc, rps, &max_act_freq);
+			break;
 
-			/* GuC requests freq in multiples of 50/3 MHz */
-			if (req_freq > (max_freq + FREQUENCY_REQ_UNIT)) {
-				pr_err("SWReq is %d, should be at most %d\n", req_freq,
-				       max_freq + FREQUENCY_REQ_UNIT);
+		case VARY_MAX:
+			err = vary_max_freq(slpc, rps, &max_act_freq);
+			break;
+
+		case MAX_GRANTED:
+			/* FIXME: Media engines have a different RP0 */
+			if ((engine->class == VIDEO_DECODE_CLASS) ||
+			    (engine->class == VIDEO_ENHANCEMENT_CLASS)) {
 				igt_spinner_end(&spin);
 				st_engine_heartbeat_enable(engine);
-				err = -EINVAL;
-				break;
+				err = 0;
+				continue;
 			}
 
-			act_freq =  intel_rps_read_actual_frequency(rps);
-			if (act_freq > max_act_freq)
-				max_act_freq = act_freq;
-
-			st_engine_heartbeat_enable(engine);
-			igt_spinner_end(&spin);
-
-			if (err)
-				break;
+			err = max_granted_freq(slpc, rps, &max_act_freq);
+			break;
 		}
 
 		pr_info("Max actual frequency for %s was %d\n",
 			engine->name, max_act_freq);
 
 		/* Actual frequency should rise above min */
-		if (max_act_freq == slpc_min_freq) {
+		if (max_act_freq <= slpc_min_freq) {
 			pr_err("Actual freq did not rise above min\n");
+			pr_err("Perf Limit Reasons: 0x%x\n",
+				intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS));
 			err = -EINVAL;
 		}
 
-		if (igt_flush_test(gt->i915)) {
-			err = -EIO;
-			break;
-		}
+		igt_spinner_end(&spin);
+		st_engine_heartbeat_enable(engine);
 
 		if (err)
 			break;
 	}
 
-	/* Restore min/max freq */
+	/* Restore min/max frequencies */
 	slpc_set_max_freq(slpc, slpc_max_freq);
 	slpc_set_min_freq(slpc, slpc_min_freq);
 
+	if (igt_flush_test(gt->i915))
+		err = -EIO;
+
 	intel_gt_pm_put(gt);
 	igt_spinner_fini(&spin);
 	intel_gt_pm_wait_for_idle(gt);
 
 	return err;
+
+}
+
+static int live_slpc_vary_min(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	struct intel_gt *gt = to_gt(i915);
+
+	return run_test(gt, VARY_MIN);
+}
+
+static int live_slpc_vary_max(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	struct intel_gt *gt = to_gt(i915);
+
+	return run_test(gt, VARY_MAX);
+}
+
+/* check if pcode can grant RP0 */
+static int live_slpc_max_granted(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	struct intel_gt *gt = to_gt(i915);
+
+	return run_test(gt, MAX_GRANTED);
 }
 
 int intel_slpc_live_selftests(struct drm_i915_private *i915)
 {
 	static const struct i915_subtest tests[] = {
-		SUBTEST(live_slpc_clamp_max),
-		SUBTEST(live_slpc_clamp_min),
+		SUBTEST(live_slpc_vary_max),
+		SUBTEST(live_slpc_vary_min),
+		SUBTEST(live_slpc_max_granted),
 	};
 
 	if (intel_gt_is_wedged(to_gt(i915)))
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Intel-gfx] [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
@ 2022-06-23 23:33 ` Vinay Belgaumkar
  0 siblings, 0 replies; 10+ messages in thread
From: Vinay Belgaumkar @ 2022-06-23 23:33 UTC (permalink / raw)
  To: intel-gfx, dri-devel

This test will validate we can achieve actual frequency of RP0. Pcode
grants frequencies based on what GuC is requesting. However, thermal
throttling can limit what is being granted. Add a test to request for
max, but don't fail the test if RP0 is not granted due to throttle
reasons.

Also optimize the selftest by using a common run_test function to avoid
code duplication. Rename the "clamp" tests to vary_max_freq and vary_min_freq.

v2: Fix compile warning
v3: Review comments (Ashutosh). Added a FIXME for the media RP0 case.

Fixes 8ee2c227822e ("drm/i915/guc/slpc: Add SLPC selftest")

Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_slpc.c | 324 ++++++++++++------------
 1 file changed, 157 insertions(+), 167 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
index b768cea5943d..2b8c10b4e38a 100644
--- a/drivers/gpu/drm/i915/gt/selftest_slpc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_slpc.c
@@ -8,6 +8,11 @@
 #define delay_for_h2g() usleep_range(H2G_DELAY, H2G_DELAY + 10000)
 #define FREQUENCY_REQ_UNIT	DIV_ROUND_CLOSEST(GT_FREQUENCY_MULTIPLIER, \
 						  GEN9_FREQ_SCALER)
+enum test_type {
+	VARY_MIN,
+	VARY_MAX,
+	MAX_GRANTED
+};
 
 static int slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 freq)
 {
@@ -36,147 +41,115 @@ static int slpc_set_max_freq(struct intel_guc_slpc *slpc, u32 freq)
 	return ret;
 }
 
-static int live_slpc_clamp_min(void *arg)
+static int vary_max_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps,
+			 u32 *max_act_freq)
 {
-	struct drm_i915_private *i915 = arg;
-	struct intel_gt *gt = to_gt(i915);
-	struct intel_guc_slpc *slpc = &gt->uc.guc.slpc;
-	struct intel_rps *rps = &gt->rps;
-	struct intel_engine_cs *engine;
-	enum intel_engine_id id;
-	struct igt_spinner spin;
-	u32 slpc_min_freq, slpc_max_freq;
+	u32 step, max_freq, req_freq;
+	u32 act_freq;
 	int err = 0;
 
-	if (!intel_uc_uses_guc_slpc(&gt->uc))
-		return 0;
+	/* Go from max to min in 5 steps */
+	step = (slpc->rp0_freq - slpc->min_freq) / NUM_STEPS;
+	*max_act_freq = slpc->min_freq;
+	for (max_freq = slpc->rp0_freq; max_freq > slpc->min_freq;
+				max_freq -= step) {
+		err = slpc_set_max_freq(slpc, max_freq);
+		if (err)
+			break;
 
-	if (igt_spinner_init(&spin, gt))
-		return -ENOMEM;
+		req_freq = intel_rps_read_punit_req_frequency(rps);
 
-	if (intel_guc_slpc_get_max_freq(slpc, &slpc_max_freq)) {
-		pr_err("Could not get SLPC max freq\n");
-		return -EIO;
-	}
+		/* GuC requests freq in multiples of 50/3 MHz */
+		if (req_freq > (max_freq + FREQUENCY_REQ_UNIT)) {
+			pr_err("SWReq is %d, should be at most %d\n", req_freq,
+				max_freq + FREQUENCY_REQ_UNIT);
+			err = -EINVAL;
+		}
 
-	if (intel_guc_slpc_get_min_freq(slpc, &slpc_min_freq)) {
-		pr_err("Could not get SLPC min freq\n");
-		return -EIO;
-	}
+		act_freq =  intel_rps_read_actual_frequency(rps);
+		if (act_freq > *max_act_freq)
+			*max_act_freq = act_freq;
 
-	if (slpc_min_freq == slpc_max_freq) {
-		pr_err("Min/Max are fused to the same value\n");
-		return -EINVAL;
+		if (err)
+			break;
 	}
 
-	intel_gt_pm_wait_for_idle(gt);
-	intel_gt_pm_get(gt);
-	for_each_engine(engine, gt, id) {
-		struct i915_request *rq;
-		u32 step, min_freq, req_freq;
-		u32 act_freq, max_act_freq;
+	return err;
+}
 
-		if (!intel_engine_can_store_dword(engine))
-			continue;
+static int vary_min_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps,
+			 u32 *max_act_freq)
+{
+	u32 step, min_freq, req_freq;
+	u32 act_freq;
+	int err = 0;
 
-		/* Go from min to max in 5 steps */
-		step = (slpc_max_freq - slpc_min_freq) / NUM_STEPS;
-		max_act_freq = slpc_min_freq;
-		for (min_freq = slpc_min_freq; min_freq < slpc_max_freq;
-					min_freq += step) {
-			err = slpc_set_min_freq(slpc, min_freq);
-			if (err)
-				break;
-
-			st_engine_heartbeat_disable(engine);
-
-			rq = igt_spinner_create_request(&spin,
-							engine->kernel_context,
-							MI_NOOP);
-			if (IS_ERR(rq)) {
-				err = PTR_ERR(rq);
-				st_engine_heartbeat_enable(engine);
-				break;
-			}
+	/* Go from min to max in 5 steps */
+	step = (slpc->rp0_freq - slpc->min_freq) / NUM_STEPS;
+	*max_act_freq = slpc->min_freq;
+	for (min_freq = slpc->min_freq; min_freq < slpc->rp0_freq;
+				min_freq += step) {
+		err = slpc_set_min_freq(slpc, min_freq);
+		if (err)
+			break;
 
-			i915_request_add(rq);
+		req_freq = intel_rps_read_punit_req_frequency(rps);
 
-			if (!igt_wait_for_spinner(&spin, rq)) {
-				pr_err("%s: Spinner did not start\n",
-				       engine->name);
-				igt_spinner_end(&spin);
-				st_engine_heartbeat_enable(engine);
-				intel_gt_set_wedged(engine->gt);
-				err = -EIO;
-				break;
-			}
+		/* GuC requests freq in multiples of 50/3 MHz */
+		if (req_freq < (min_freq - FREQUENCY_REQ_UNIT)) {
+			pr_err("SWReq is %d, should be at least %d\n", req_freq,
+				min_freq - FREQUENCY_REQ_UNIT);
+			err = -EINVAL;
+		}
 
-			/* Wait for GuC to detect business and raise
-			 * requested frequency if necessary.
-			 */
-			delay_for_h2g();
+		act_freq =  intel_rps_read_actual_frequency(rps);
+		if (act_freq > *max_act_freq)
+			*max_act_freq = act_freq;
 
-			req_freq = intel_rps_read_punit_req_frequency(rps);
+		if (err)
+			break;
+	}
 
-			/* GuC requests freq in multiples of 50/3 MHz */
-			if (req_freq < (min_freq - FREQUENCY_REQ_UNIT)) {
-				pr_err("SWReq is %d, should be at least %d\n", req_freq,
-				       min_freq - FREQUENCY_REQ_UNIT);
-				igt_spinner_end(&spin);
-				st_engine_heartbeat_enable(engine);
-				err = -EINVAL;
-				break;
-			}
+	return err;
+}
 
-			act_freq =  intel_rps_read_actual_frequency(rps);
-			if (act_freq > max_act_freq)
-				max_act_freq = act_freq;
+static int max_granted_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps, u32 *max_act_freq)
+{
+	struct intel_gt *gt = rps_to_gt(rps);
+	u32 perf_limit_reasons;
+	int err = 0;
 
-			igt_spinner_end(&spin);
-			st_engine_heartbeat_enable(engine);
-		}
+	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
+	if (err)
+		return err;
 
-		pr_info("Max actual frequency for %s was %d\n",
-			engine->name, max_act_freq);
+	*max_act_freq =  intel_rps_read_actual_frequency(rps);
+	if (!(*max_act_freq == slpc->rp0_freq)) {
+		/* Check if there was some throttling by pcode */
+		perf_limit_reasons = intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS);
 
-		/* Actual frequency should rise above min */
-		if (max_act_freq == slpc_min_freq) {
-			pr_err("Actual freq did not rise above min\n");
+		/* If not, this is an error */
+		if (!(perf_limit_reasons && GT0_PERF_LIMIT_REASONS_MASK)) {
+			pr_err("Pcode did not grant max freq\n");
 			err = -EINVAL;
-		}
+		} else {
+			pr_info("Pcode throttled frequency 0x%x\n", perf_limit_reasons);
 
-		if (err)
-			break;
+		}
 	}
 
-	/* Restore min/max frequencies */
-	slpc_set_max_freq(slpc, slpc_max_freq);
-	slpc_set_min_freq(slpc, slpc_min_freq);
-
-	if (igt_flush_test(gt->i915))
-		err = -EIO;
-
-	intel_gt_pm_put(gt);
-	igt_spinner_fini(&spin);
-	intel_gt_pm_wait_for_idle(gt);
-
 	return err;
 }
 
-static int live_slpc_clamp_max(void *arg)
+static int run_test(struct intel_gt *gt, int test_type)
 {
-	struct drm_i915_private *i915 = arg;
-	struct intel_gt *gt = to_gt(i915);
-	struct intel_guc_slpc *slpc;
-	struct intel_rps *rps;
+	struct intel_guc_slpc *slpc = &gt->uc.guc.slpc;
+	struct intel_rps *rps = &gt->rps;
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 	struct igt_spinner spin;
-	int err = 0;
 	u32 slpc_min_freq, slpc_max_freq;
-
-	slpc = &gt->uc.guc.slpc;
-	rps = &gt->rps;
+	int err = 0;
 
 	if (!intel_uc_uses_guc_slpc(&gt->uc))
 		return 0;
@@ -203,105 +176,122 @@ static int live_slpc_clamp_max(void *arg)
 	intel_gt_pm_get(gt);
 	for_each_engine(engine, gt, id) {
 		struct i915_request *rq;
-		u32 max_freq, req_freq;
-		u32 act_freq, max_act_freq;
-		u32 step;
+		u32 max_act_freq;
 
 		if (!intel_engine_can_store_dword(engine))
 			continue;
 
-		/* Go from max to min in 5 steps */
-		step = (slpc_max_freq - slpc_min_freq) / NUM_STEPS;
-		max_act_freq = slpc_min_freq;
-		for (max_freq = slpc_max_freq; max_freq > slpc_min_freq;
-					max_freq -= step) {
-			err = slpc_set_max_freq(slpc, max_freq);
-			if (err)
-				break;
-
-			st_engine_heartbeat_disable(engine);
-
-			rq = igt_spinner_create_request(&spin,
-							engine->kernel_context,
-							MI_NOOP);
-			if (IS_ERR(rq)) {
-				st_engine_heartbeat_enable(engine);
-				err = PTR_ERR(rq);
-				break;
-			}
+		st_engine_heartbeat_disable(engine);
 
-			i915_request_add(rq);
+		rq = igt_spinner_create_request(&spin,
+						engine->kernel_context,
+						MI_NOOP);
+		if (IS_ERR(rq)) {
+			err = PTR_ERR(rq);
+			st_engine_heartbeat_enable(engine);
+			break;
+		}
 
-			if (!igt_wait_for_spinner(&spin, rq)) {
-				pr_err("%s: SLPC spinner did not start\n",
-				       engine->name);
-				igt_spinner_end(&spin);
-				st_engine_heartbeat_enable(engine);
-				intel_gt_set_wedged(engine->gt);
-				err = -EIO;
-				break;
-			}
+		i915_request_add(rq);
+
+		if (!igt_wait_for_spinner(&spin, rq)) {
+			pr_err("%s: Spinner did not start\n",
+			       engine->name);
+			igt_spinner_end(&spin);
+			st_engine_heartbeat_enable(engine);
+			intel_gt_set_wedged(engine->gt);
+			err = -EIO;
+			break;
+		}
 
-			delay_for_h2g();
+		switch (test_type) {
 
-			/* Verify that SWREQ indeed was set to specific value */
-			req_freq = intel_rps_read_punit_req_frequency(rps);
+		case VARY_MIN:
+			err = vary_min_freq(slpc, rps, &max_act_freq);
+			break;
 
-			/* GuC requests freq in multiples of 50/3 MHz */
-			if (req_freq > (max_freq + FREQUENCY_REQ_UNIT)) {
-				pr_err("SWReq is %d, should be at most %d\n", req_freq,
-				       max_freq + FREQUENCY_REQ_UNIT);
+		case VARY_MAX:
+			err = vary_max_freq(slpc, rps, &max_act_freq);
+			break;
+
+		case MAX_GRANTED:
+			/* FIXME: Media engines have a different RP0 */
+			if ((engine->class == VIDEO_DECODE_CLASS) ||
+			    (engine->class == VIDEO_ENHANCEMENT_CLASS)) {
 				igt_spinner_end(&spin);
 				st_engine_heartbeat_enable(engine);
-				err = -EINVAL;
-				break;
+				err = 0;
+				continue;
 			}
 
-			act_freq =  intel_rps_read_actual_frequency(rps);
-			if (act_freq > max_act_freq)
-				max_act_freq = act_freq;
-
-			st_engine_heartbeat_enable(engine);
-			igt_spinner_end(&spin);
-
-			if (err)
-				break;
+			err = max_granted_freq(slpc, rps, &max_act_freq);
+			break;
 		}
 
 		pr_info("Max actual frequency for %s was %d\n",
 			engine->name, max_act_freq);
 
 		/* Actual frequency should rise above min */
-		if (max_act_freq == slpc_min_freq) {
+		if (max_act_freq <= slpc_min_freq) {
 			pr_err("Actual freq did not rise above min\n");
+			pr_err("Perf Limit Reasons: 0x%x\n",
+				intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS));
 			err = -EINVAL;
 		}
 
-		if (igt_flush_test(gt->i915)) {
-			err = -EIO;
-			break;
-		}
+		igt_spinner_end(&spin);
+		st_engine_heartbeat_enable(engine);
 
 		if (err)
 			break;
 	}
 
-	/* Restore min/max freq */
+	/* Restore min/max frequencies */
 	slpc_set_max_freq(slpc, slpc_max_freq);
 	slpc_set_min_freq(slpc, slpc_min_freq);
 
+	if (igt_flush_test(gt->i915))
+		err = -EIO;
+
 	intel_gt_pm_put(gt);
 	igt_spinner_fini(&spin);
 	intel_gt_pm_wait_for_idle(gt);
 
 	return err;
+
+}
+
+static int live_slpc_vary_min(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	struct intel_gt *gt = to_gt(i915);
+
+	return run_test(gt, VARY_MIN);
+}
+
+static int live_slpc_vary_max(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	struct intel_gt *gt = to_gt(i915);
+
+	return run_test(gt, VARY_MAX);
+}
+
+/* check if pcode can grant RP0 */
+static int live_slpc_max_granted(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	struct intel_gt *gt = to_gt(i915);
+
+	return run_test(gt, MAX_GRANTED);
 }
 
 int intel_slpc_live_selftests(struct drm_i915_private *i915)
 {
 	static const struct i915_subtest tests[] = {
-		SUBTEST(live_slpc_clamp_max),
-		SUBTEST(live_slpc_clamp_min),
+		SUBTEST(live_slpc_vary_max),
+		SUBTEST(live_slpc_vary_min),
+		SUBTEST(live_slpc_max_granted),
 	};
 
 	if (intel_gt_is_wedged(to_gt(i915)))
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/guc/slpc: Add a new SLPC selftest (rev3)
  2022-06-23 23:33 ` [Intel-gfx] " Vinay Belgaumkar
  (?)
@ 2022-06-24  0:03 ` Patchwork
  -1 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2022-06-24  0:03 UTC (permalink / raw)
  To: Vinay Belgaumkar; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc/slpc: Add a new SLPC selftest (rev3)
URL   : https://patchwork.freedesktop.org/series/105005/
State : warning

== Summary ==

Error: dim checkpatch failed
4982a0bd3fa6 drm/i915/guc/slpc: Add a new SLPC selftest
-:13: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#13: 
code duplication. Rename the "clamp" tests to vary_max_freq and vary_min_freq.

-:18: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 8ee2c227822e ("drm/i915/guc/slpc: Add SLPC selftest")'
#18: 
Fixes 8ee2c227822e ("drm/i915/guc/slpc: Add SLPC selftest")

-:81: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#81: FILE: drivers/gpu/drm/i915/gt/selftest_slpc.c:65:
+			pr_err("SWReq is %d, should be at most %d\n", req_freq,
+				max_freq + FREQUENCY_REQ_UNIT);

-:161: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#161: FILE: drivers/gpu/drm/i915/gt/selftest_slpc.c:101:
+			pr_err("SWReq is %d, should be at least %d\n", req_freq,
+				min_freq - FREQUENCY_REQ_UNIT);

-:226: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#226: FILE: drivers/gpu/drm/i915/gt/selftest_slpc.c:138:
 
+		}

-:347: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'engine->class == VIDEO_DECODE_CLASS'
#347: FILE: drivers/gpu/drm/i915/gt/selftest_slpc.c:219:
+			if ((engine->class == VIDEO_DECODE_CLASS) ||
+			    (engine->class == VIDEO_ENHANCEMENT_CLASS)) {

-:347: CHECK:UNNECESSARY_PARENTHESES: Unnecessary parentheses around 'engine->class == VIDEO_ENHANCEMENT_CLASS'
#347: FILE: drivers/gpu/drm/i915/gt/selftest_slpc.c:219:
+			if ((engine->class == VIDEO_DECODE_CLASS) ||
+			    (engine->class == VIDEO_ENHANCEMENT_CLASS)) {

-:378: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#378: FILE: drivers/gpu/drm/i915/gt/selftest_slpc.c:238:
+			pr_err("Perf Limit Reasons: 0x%x\n",
+				intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS));

-:407: CHECK:BRACES: Blank lines aren't necessary before a close brace '}'
#407: FILE: drivers/gpu/drm/i915/gt/selftest_slpc.c:262:
+
+}

total: 1 errors, 1 warnings, 7 checks, 415 lines checked



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/guc/slpc: Add a new SLPC selftest (rev3)
  2022-06-23 23:33 ` [Intel-gfx] " Vinay Belgaumkar
  (?)
  (?)
@ 2022-06-24  0:26 ` Patchwork
  -1 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2022-06-24  0:26 UTC (permalink / raw)
  To: Vinay Belgaumkar; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 9183 bytes --]

== Series Details ==

Series: drm/i915/guc/slpc: Add a new SLPC selftest (rev3)
URL   : https://patchwork.freedesktop.org/series/105005/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11800 -> Patchwork_105005v3
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_105005v3 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_105005v3, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/index.html

Participating hosts (38 -> 38)
------------------------------

  Additional (2): fi-hsw-4770 fi-bxt-dsi 
  Missing    (2): fi-icl-u2 fi-bdw-samus 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_105005v3:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size:
    - fi-bsw-kefka:       [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11800/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions-varying-size.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_selftest@live@gt_lrc:
    - {bat-adln-1}:       NOTRUN -> [DMESG-FAIL][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/bat-adln-1/igt@i915_selftest@live@gt_lrc.html

  
Known issues
------------

  Here are the changes found in Patchwork_105005v3 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-bxt-dsi:         NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#2190])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-bxt-dsi/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@verify-random:
    - fi-bxt-dsi:         NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4613]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-bxt-dsi/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_tiled_blits@basic:
    - fi-bxt-dsi:         NOTRUN -> [SKIP][6] ([fdo#109271]) +12 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-bxt-dsi/igt@gem_tiled_blits@basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-hsw-4770:        NOTRUN -> [SKIP][7] ([fdo#109271] / [i915#3012])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-hsw-4770/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-4770:        NOTRUN -> [INCOMPLETE][8] ([i915#4785])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
    - bat-dg1-6:          [PASS][9] -> [DMESG-FAIL][10] ([i915#4494] / [i915#4957])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11800/bat-dg1-6/igt@i915_selftest@live@hangcheck.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/bat-dg1-6/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@requests:
    - fi-pnv-d510:        [PASS][11] -> [DMESG-FAIL][12] ([i915#4528])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11800/fi-pnv-d510/igt@i915_selftest@live@requests.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-pnv-d510/igt@i915_selftest@live@requests.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - fi-hsw-4770:        NOTRUN -> [SKIP][13] ([fdo#109271]) +9 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-hsw-4770/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-bsw-kefka:       NOTRUN -> [SKIP][14] ([fdo#109271] / [fdo#111827])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-bsw-kefka/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-hsw-4770:        NOTRUN -> [SKIP][15] ([fdo#109271] / [fdo#111827]) +7 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-hsw-4770/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-bxt-dsi:         NOTRUN -> [SKIP][16] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-bxt-dsi/igt@kms_chamelium@hdmi-edid-read.html

  * igt@kms_psr@sprite_plane_onoff:
    - fi-hsw-4770:        NOTRUN -> [SKIP][17] ([fdo#109271] / [i915#1072]) +3 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-hsw-4770/igt@kms_psr@sprite_plane_onoff.html

  * igt@runner@aborted:
    - fi-hsw-4770:        NOTRUN -> [FAIL][18] ([fdo#109271] / [i915#4312] / [i915#5594] / [i915#6246])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-hsw-4770/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload:
    - {bat-adln-1}:       [DMESG-WARN][19] -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11800/bat-adln-1/igt@i915_module_load@reload.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/bat-adln-1/igt@i915_module_load@reload.html

  * igt@i915_selftest@live@execlists:
    - fi-bsw-kefka:       [INCOMPLETE][21] ([i915#2940]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11800/fi-bsw-kefka/igt@i915_selftest@live@execlists.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-bsw-kefka/igt@i915_selftest@live@execlists.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions:
    - fi-bsw-kefka:       [FAIL][23] -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11800/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor@atomic-transitions.html

  * igt@kms_force_connector_basic@force-connector-state:
    - {bat-adln-1}:       [DMESG-WARN][25] ([i915#3576]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11800/bat-adln-1/igt@kms_force_connector_basic@force-connector-state.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/bat-adln-1/igt@kms_force_connector_basic@force-connector-state.html

  
#### Warnings ####

  * igt@i915_selftest@live@hangcheck:
    - fi-bdw-5557u:       [INCOMPLETE][27] ([i915#3921] / [i915#6105]) -> [INCOMPLETE][28] ([i915#3921])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11800/fi-bdw-5557u/igt@i915_selftest@live@hangcheck.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/fi-bdw-5557u/igt@i915_selftest@live@hangcheck.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2940]: https://gitlab.freedesktop.org/drm/intel/issues/2940
  [i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957
  [i915#5594]: https://gitlab.freedesktop.org/drm/intel/issues/5594
  [i915#6105]: https://gitlab.freedesktop.org/drm/intel/issues/6105
  [i915#6246]: https://gitlab.freedesktop.org/drm/intel/issues/6246


Build changes
-------------

  * Linux: CI_DRM_11800 -> Patchwork_105005v3

  CI-20190529: 20190529
  CI_DRM_11800: 21de2c24999d9ecc5d2d51fa5e68727a64b621f7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6541: 02153f109bd422d93cfce7f5aa9d7b0e22fab13c @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_105005v3: 21de2c24999d9ecc5d2d51fa5e68727a64b621f7 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

078c4d078919 drm/i915/guc/slpc: Add a new SLPC selftest

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v3/index.html

[-- Attachment #2: Type: text/html, Size: 11093 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
  2022-06-23 23:33 ` [Intel-gfx] " Vinay Belgaumkar
@ 2022-06-25  3:59   ` Dixit, Ashutosh
  -1 siblings, 0 replies; 10+ messages in thread
From: Dixit, Ashutosh @ 2022-06-25  3:59 UTC (permalink / raw)
  To: Vinay Belgaumkar; +Cc: intel-gfx, dri-devel

On Thu, 23 Jun 2022 16:33:20 -0700, Vinay Belgaumkar wrote:
>
> +static int max_granted_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps, u32 *max_act_freq)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 perf_limit_reasons;
> +	int err = 0;
>
> -			igt_spinner_end(&spin);
> -			st_engine_heartbeat_enable(engine);
> -		}
> +	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
> +	if (err)
> +		return err;
>
> -		pr_info("Max actual frequency for %s was %d\n",
> -			engine->name, max_act_freq);
> +	*max_act_freq =  intel_rps_read_actual_frequency(rps);
> +	if (!(*max_act_freq == slpc->rp0_freq)) {

nit but '*max_act_freq != slpc->rp0_freq'


> +		/* Check if there was some throttling by pcode */
> +		perf_limit_reasons = intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS);
>
> -		/* Actual frequency should rise above min */
> -		if (max_act_freq == slpc_min_freq) {
> -			pr_err("Actual freq did not rise above min\n");
> +		/* If not, this is an error */
> +		if (!(perf_limit_reasons && GT0_PERF_LIMIT_REASONS_MASK)) {

Still wrong, should be & not &&

> +			pr_err("Pcode did not grant max freq\n");
>			err = -EINVAL;
> -		}
> +		} else {
> +			pr_info("Pcode throttled frequency 0x%x\n", perf_limit_reasons);

Another question, why are we using pr_err/info here rather than
drm_err/info? pr_err/info is ok for mock selftests since there is no drm
device but that is not the case here, I think this is done in other
selftests too but maybe fix this as well if we are making so many changes
here? Anyway can do later too.

So let's settle issues in v2 thread first.

Thanks.
--
Ashutosh

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
@ 2022-06-25  3:59   ` Dixit, Ashutosh
  0 siblings, 0 replies; 10+ messages in thread
From: Dixit, Ashutosh @ 2022-06-25  3:59 UTC (permalink / raw)
  To: Vinay Belgaumkar; +Cc: intel-gfx, dri-devel

On Thu, 23 Jun 2022 16:33:20 -0700, Vinay Belgaumkar wrote:
>
> +static int max_granted_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps, u32 *max_act_freq)
> +{
> +	struct intel_gt *gt = rps_to_gt(rps);
> +	u32 perf_limit_reasons;
> +	int err = 0;
>
> -			igt_spinner_end(&spin);
> -			st_engine_heartbeat_enable(engine);
> -		}
> +	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
> +	if (err)
> +		return err;
>
> -		pr_info("Max actual frequency for %s was %d\n",
> -			engine->name, max_act_freq);
> +	*max_act_freq =  intel_rps_read_actual_frequency(rps);
> +	if (!(*max_act_freq == slpc->rp0_freq)) {

nit but '*max_act_freq != slpc->rp0_freq'


> +		/* Check if there was some throttling by pcode */
> +		perf_limit_reasons = intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS);
>
> -		/* Actual frequency should rise above min */
> -		if (max_act_freq == slpc_min_freq) {
> -			pr_err("Actual freq did not rise above min\n");
> +		/* If not, this is an error */
> +		if (!(perf_limit_reasons && GT0_PERF_LIMIT_REASONS_MASK)) {

Still wrong, should be & not &&

> +			pr_err("Pcode did not grant max freq\n");
>			err = -EINVAL;
> -		}
> +		} else {
> +			pr_info("Pcode throttled frequency 0x%x\n", perf_limit_reasons);

Another question, why are we using pr_err/info here rather than
drm_err/info? pr_err/info is ok for mock selftests since there is no drm
device but that is not the case here, I think this is done in other
selftests too but maybe fix this as well if we are making so many changes
here? Anyway can do later too.

So let's settle issues in v2 thread first.

Thanks.
--
Ashutosh

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
  2022-06-25  3:59   ` [Intel-gfx] " Dixit, Ashutosh
@ 2022-06-27 22:52     ` Belgaumkar, Vinay
  -1 siblings, 0 replies; 10+ messages in thread
From: Belgaumkar, Vinay @ 2022-06-27 22:52 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: intel-gfx, dri-devel


On 6/24/2022 8:59 PM, Dixit, Ashutosh wrote:
> On Thu, 23 Jun 2022 16:33:20 -0700, Vinay Belgaumkar wrote:
>> +static int max_granted_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps, u32 *max_act_freq)
>> +{
>> +	struct intel_gt *gt = rps_to_gt(rps);
>> +	u32 perf_limit_reasons;
>> +	int err = 0;
>>
>> -			igt_spinner_end(&spin);
>> -			st_engine_heartbeat_enable(engine);
>> -		}
>> +	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
>> +	if (err)
>> +		return err;
>>
>> -		pr_info("Max actual frequency for %s was %d\n",
>> -			engine->name, max_act_freq);
>> +	*max_act_freq =  intel_rps_read_actual_frequency(rps);
>> +	if (!(*max_act_freq == slpc->rp0_freq)) {
> nit but '*max_act_freq != slpc->rp0_freq'
ok.
>
>
>> +		/* Check if there was some throttling by pcode */
>> +		perf_limit_reasons = intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS);
>>
>> -		/* Actual frequency should rise above min */
>> -		if (max_act_freq == slpc_min_freq) {
>> -			pr_err("Actual freq did not rise above min\n");
>> +		/* If not, this is an error */
>> +		if (!(perf_limit_reasons && GT0_PERF_LIMIT_REASONS_MASK)) {
> Still wrong, should be & not &&
yup, third time's the charm.
>
>> +			pr_err("Pcode did not grant max freq\n");
>> 			err = -EINVAL;
>> -		}
>> +		} else {
>> +			pr_info("Pcode throttled frequency 0x%x\n", perf_limit_reasons);
> Another question, why are we using pr_err/info here rather than
> drm_err/info? pr_err/info is ok for mock selftests since there is no drm
> device but that is not the case here, I think this is done in other
> selftests too but maybe fix this as well if we are making so many changes
> here? Anyway can do later too.
>
> So let's settle issues in v2 thread first.

Thanks,

Vinay.

>
> Thanks.
> --
> Ashutosh

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
@ 2022-06-27 22:52     ` Belgaumkar, Vinay
  0 siblings, 0 replies; 10+ messages in thread
From: Belgaumkar, Vinay @ 2022-06-27 22:52 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: intel-gfx, dri-devel


On 6/24/2022 8:59 PM, Dixit, Ashutosh wrote:
> On Thu, 23 Jun 2022 16:33:20 -0700, Vinay Belgaumkar wrote:
>> +static int max_granted_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps, u32 *max_act_freq)
>> +{
>> +	struct intel_gt *gt = rps_to_gt(rps);
>> +	u32 perf_limit_reasons;
>> +	int err = 0;
>>
>> -			igt_spinner_end(&spin);
>> -			st_engine_heartbeat_enable(engine);
>> -		}
>> +	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
>> +	if (err)
>> +		return err;
>>
>> -		pr_info("Max actual frequency for %s was %d\n",
>> -			engine->name, max_act_freq);
>> +	*max_act_freq =  intel_rps_read_actual_frequency(rps);
>> +	if (!(*max_act_freq == slpc->rp0_freq)) {
> nit but '*max_act_freq != slpc->rp0_freq'
ok.
>
>
>> +		/* Check if there was some throttling by pcode */
>> +		perf_limit_reasons = intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS);
>>
>> -		/* Actual frequency should rise above min */
>> -		if (max_act_freq == slpc_min_freq) {
>> -			pr_err("Actual freq did not rise above min\n");
>> +		/* If not, this is an error */
>> +		if (!(perf_limit_reasons && GT0_PERF_LIMIT_REASONS_MASK)) {
> Still wrong, should be & not &&
yup, third time's the charm.
>
>> +			pr_err("Pcode did not grant max freq\n");
>> 			err = -EINVAL;
>> -		}
>> +		} else {
>> +			pr_info("Pcode throttled frequency 0x%x\n", perf_limit_reasons);
> Another question, why are we using pr_err/info here rather than
> drm_err/info? pr_err/info is ok for mock selftests since there is no drm
> device but that is not the case here, I think this is done in other
> selftests too but maybe fix this as well if we are making so many changes
> here? Anyway can do later too.
>
> So let's settle issues in v2 thread first.

Thanks,

Vinay.

>
> Thanks.
> --
> Ashutosh

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
  2022-06-25  3:59   ` [Intel-gfx] " Dixit, Ashutosh
@ 2022-06-27 23:02     ` Belgaumkar, Vinay
  -1 siblings, 0 replies; 10+ messages in thread
From: Belgaumkar, Vinay @ 2022-06-27 23:02 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: intel-gfx, dri-devel


On 6/24/2022 8:59 PM, Dixit, Ashutosh wrote:
> On Thu, 23 Jun 2022 16:33:20 -0700, Vinay Belgaumkar wrote:
>> +static int max_granted_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps, u32 *max_act_freq)
>> +{
>> +	struct intel_gt *gt = rps_to_gt(rps);
>> +	u32 perf_limit_reasons;
>> +	int err = 0;
>>
>> -			igt_spinner_end(&spin);
>> -			st_engine_heartbeat_enable(engine);
>> -		}
>> +	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
>> +	if (err)
>> +		return err;
>>
>> -		pr_info("Max actual frequency for %s was %d\n",
>> -			engine->name, max_act_freq);
>> +	*max_act_freq =  intel_rps_read_actual_frequency(rps);
>> +	if (!(*max_act_freq == slpc->rp0_freq)) {
> nit but '*max_act_freq != slpc->rp0_freq'
>
>
>> +		/* Check if there was some throttling by pcode */
>> +		perf_limit_reasons = intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS);
>>
>> -		/* Actual frequency should rise above min */
>> -		if (max_act_freq == slpc_min_freq) {
>> -			pr_err("Actual freq did not rise above min\n");
>> +		/* If not, this is an error */
>> +		if (!(perf_limit_reasons && GT0_PERF_LIMIT_REASONS_MASK)) {
> Still wrong, should be & not &&
>
>> +			pr_err("Pcode did not grant max freq\n");
>> 			err = -EINVAL;
>> -		}
>> +		} else {
>> +			pr_info("Pcode throttled frequency 0x%x\n", perf_limit_reasons);
> Another question, why are we using pr_err/info here rather than
> drm_err/info? pr_err/info is ok for mock selftests since there is no drm
> device but that is not the case here, I think this is done in other
> selftests too but maybe fix this as well if we are making so many changes
> here? Anyway can do later too.

Yup, will send a separate patch to change them to drm_err/info.

Thanks,

Vinay.

>
> So let's settle issues in v2 thread first.
>
> Thanks.
> --
> Ashutosh

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
@ 2022-06-27 23:02     ` Belgaumkar, Vinay
  0 siblings, 0 replies; 10+ messages in thread
From: Belgaumkar, Vinay @ 2022-06-27 23:02 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: intel-gfx, dri-devel


On 6/24/2022 8:59 PM, Dixit, Ashutosh wrote:
> On Thu, 23 Jun 2022 16:33:20 -0700, Vinay Belgaumkar wrote:
>> +static int max_granted_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps, u32 *max_act_freq)
>> +{
>> +	struct intel_gt *gt = rps_to_gt(rps);
>> +	u32 perf_limit_reasons;
>> +	int err = 0;
>>
>> -			igt_spinner_end(&spin);
>> -			st_engine_heartbeat_enable(engine);
>> -		}
>> +	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
>> +	if (err)
>> +		return err;
>>
>> -		pr_info("Max actual frequency for %s was %d\n",
>> -			engine->name, max_act_freq);
>> +	*max_act_freq =  intel_rps_read_actual_frequency(rps);
>> +	if (!(*max_act_freq == slpc->rp0_freq)) {
> nit but '*max_act_freq != slpc->rp0_freq'
>
>
>> +		/* Check if there was some throttling by pcode */
>> +		perf_limit_reasons = intel_uncore_read(gt->uncore, GT0_PERF_LIMIT_REASONS);
>>
>> -		/* Actual frequency should rise above min */
>> -		if (max_act_freq == slpc_min_freq) {
>> -			pr_err("Actual freq did not rise above min\n");
>> +		/* If not, this is an error */
>> +		if (!(perf_limit_reasons && GT0_PERF_LIMIT_REASONS_MASK)) {
> Still wrong, should be & not &&
>
>> +			pr_err("Pcode did not grant max freq\n");
>> 			err = -EINVAL;
>> -		}
>> +		} else {
>> +			pr_info("Pcode throttled frequency 0x%x\n", perf_limit_reasons);
> Another question, why are we using pr_err/info here rather than
> drm_err/info? pr_err/info is ok for mock selftests since there is no drm
> device but that is not the case here, I think this is done in other
> selftests too but maybe fix this as well if we are making so many changes
> here? Anyway can do later too.

Yup, will send a separate patch to change them to drm_err/info.

Thanks,

Vinay.

>
> So let's settle issues in v2 thread first.
>
> Thanks.
> --
> Ashutosh

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-06-27 23:02 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-23 23:33 [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest Vinay Belgaumkar
2022-06-23 23:33 ` [Intel-gfx] " Vinay Belgaumkar
2022-06-24  0:03 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/guc/slpc: Add a new SLPC selftest (rev3) Patchwork
2022-06-24  0:26 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2022-06-25  3:59 ` [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest Dixit, Ashutosh
2022-06-25  3:59   ` [Intel-gfx] " Dixit, Ashutosh
2022-06-27 22:52   ` Belgaumkar, Vinay
2022-06-27 22:52     ` [Intel-gfx] " Belgaumkar, Vinay
2022-06-27 23:02   ` Belgaumkar, Vinay
2022-06-27 23:02     ` [Intel-gfx] " Belgaumkar, Vinay

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.