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

Fixes 8ee2c227822e ("drm/i915/guc/slpc: Add SLPC selftest")
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_slpc.c | 323 ++++++++++++------------
 1 file changed, 158 insertions(+), 165 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
index b768cea5943d..099129aae9a5 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,120 @@ 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 step, max_freq, req_freq;
+	u32 act_freq;
 	u32 slpc_min_freq, slpc_max_freq;
 	int err = 0;
 
-	if (!intel_uc_uses_guc_slpc(&gt->uc))
-		return 0;
-
-	if (igt_spinner_init(&spin, gt))
-		return -ENOMEM;
+	slpc_min_freq = slpc->min_freq;
+	slpc_max_freq = slpc->rp0_freq;
 
-	if (intel_guc_slpc_get_max_freq(slpc, &slpc_max_freq)) {
-		pr_err("Could not get SLPC max freq\n");
-		return -EIO;
-	}
-
-	if (intel_guc_slpc_get_min_freq(slpc, &slpc_min_freq)) {
-		pr_err("Could not get SLPC min freq\n");
-		return -EIO;
-	}
-
-	if (slpc_min_freq == slpc_max_freq) {
-		pr_err("Min/Max are fused to the same value\n");
-		return -EINVAL;
-	}
-
-	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;
-
-		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;
 
-		/* 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;
-			}
+		req_freq = intel_rps_read_punit_req_frequency(rps);
 
-			i915_request_add(rq);
+		/* 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 (!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;
-			}
+		act_freq =  intel_rps_read_actual_frequency(rps);
+		if (act_freq > *max_act_freq)
+			*max_act_freq = act_freq;
 
-			/* Wait for GuC to detect business and raise
-			 * requested frequency if necessary.
-			 */
-			delay_for_h2g();
+		if (err)
+			break;
+	}
 
-			req_freq = intel_rps_read_punit_req_frequency(rps);
+	return err;
+}
 
-			/* 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;
-			}
+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;
+	u32 slpc_min_freq, slpc_max_freq;
+	int err = 0;
 
-			act_freq =  intel_rps_read_actual_frequency(rps);
-			if (act_freq > max_act_freq)
-				max_act_freq = act_freq;
+	slpc_min_freq = slpc->min_freq;
+	slpc_max_freq = slpc->rp0_freq;
 
-			igt_spinner_end(&spin);
-			st_engine_heartbeat_enable(engine);
-		}
+	/* 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;
 
-		pr_info("Max actual frequency for %s was %d\n",
-			engine->name, max_act_freq);
+		req_freq = intel_rps_read_punit_req_frequency(rps);
 
-		/* Actual frequency should rise above min */
-		if (max_act_freq == slpc_min_freq) {
-			pr_err("Actual freq did not rise above min\n");
+		/* 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;
 		}
 
+		act_freq =  intel_rps_read_actual_frequency(rps);
+		if (act_freq > *max_act_freq)
+			*max_act_freq = act_freq;
+
 		if (err)
 			break;
 	}
 
-	/* Restore min/max frequencies */
-	slpc_set_max_freq(slpc, slpc_max_freq);
-	slpc_set_min_freq(slpc, slpc_min_freq);
+	return err;
+}
 
-	if (igt_flush_test(gt->i915))
-		err = -EIO;
+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;
 
-	intel_gt_pm_put(gt);
-	igt_spinner_fini(&spin);
-	intel_gt_pm_wait_for_idle(gt);
+	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
+	if (err)
+		return err;
+
+	*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);
+
+		/* 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;
+		}
+	}
 
 	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,69 +181,56 @@ 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;
+
+		case VARY_MAX:
+			err = vary_max_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 MAX_GRANTED:
+			/* 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",
@@ -277,31 +242,59 @@ static int live_slpc_clamp_max(void *arg)
 			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] 16+ messages in thread

* [Intel-gfx] [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
@ 2022-06-10 23:47 ` Vinay Belgaumkar
  0 siblings, 0 replies; 16+ messages in thread
From: Vinay Belgaumkar @ 2022-06-10 23:47 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

Fixes 8ee2c227822e ("drm/i915/guc/slpc: Add SLPC selftest")
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_slpc.c | 323 ++++++++++++------------
 1 file changed, 158 insertions(+), 165 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
index b768cea5943d..099129aae9a5 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,120 @@ 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 step, max_freq, req_freq;
+	u32 act_freq;
 	u32 slpc_min_freq, slpc_max_freq;
 	int err = 0;
 
-	if (!intel_uc_uses_guc_slpc(&gt->uc))
-		return 0;
-
-	if (igt_spinner_init(&spin, gt))
-		return -ENOMEM;
+	slpc_min_freq = slpc->min_freq;
+	slpc_max_freq = slpc->rp0_freq;
 
-	if (intel_guc_slpc_get_max_freq(slpc, &slpc_max_freq)) {
-		pr_err("Could not get SLPC max freq\n");
-		return -EIO;
-	}
-
-	if (intel_guc_slpc_get_min_freq(slpc, &slpc_min_freq)) {
-		pr_err("Could not get SLPC min freq\n");
-		return -EIO;
-	}
-
-	if (slpc_min_freq == slpc_max_freq) {
-		pr_err("Min/Max are fused to the same value\n");
-		return -EINVAL;
-	}
-
-	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;
-
-		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;
 
-		/* 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;
-			}
+		req_freq = intel_rps_read_punit_req_frequency(rps);
 
-			i915_request_add(rq);
+		/* 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 (!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;
-			}
+		act_freq =  intel_rps_read_actual_frequency(rps);
+		if (act_freq > *max_act_freq)
+			*max_act_freq = act_freq;
 
-			/* Wait for GuC to detect business and raise
-			 * requested frequency if necessary.
-			 */
-			delay_for_h2g();
+		if (err)
+			break;
+	}
 
-			req_freq = intel_rps_read_punit_req_frequency(rps);
+	return err;
+}
 
-			/* 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;
-			}
+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;
+	u32 slpc_min_freq, slpc_max_freq;
+	int err = 0;
 
-			act_freq =  intel_rps_read_actual_frequency(rps);
-			if (act_freq > max_act_freq)
-				max_act_freq = act_freq;
+	slpc_min_freq = slpc->min_freq;
+	slpc_max_freq = slpc->rp0_freq;
 
-			igt_spinner_end(&spin);
-			st_engine_heartbeat_enable(engine);
-		}
+	/* 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;
 
-		pr_info("Max actual frequency for %s was %d\n",
-			engine->name, max_act_freq);
+		req_freq = intel_rps_read_punit_req_frequency(rps);
 
-		/* Actual frequency should rise above min */
-		if (max_act_freq == slpc_min_freq) {
-			pr_err("Actual freq did not rise above min\n");
+		/* 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;
 		}
 
+		act_freq =  intel_rps_read_actual_frequency(rps);
+		if (act_freq > *max_act_freq)
+			*max_act_freq = act_freq;
+
 		if (err)
 			break;
 	}
 
-	/* Restore min/max frequencies */
-	slpc_set_max_freq(slpc, slpc_max_freq);
-	slpc_set_min_freq(slpc, slpc_min_freq);
+	return err;
+}
 
-	if (igt_flush_test(gt->i915))
-		err = -EIO;
+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;
 
-	intel_gt_pm_put(gt);
-	igt_spinner_fini(&spin);
-	intel_gt_pm_wait_for_idle(gt);
+	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
+	if (err)
+		return err;
+
+	*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);
+
+		/* 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;
+		}
+	}
 
 	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,69 +181,56 @@ 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;
+
+		case VARY_MAX:
+			err = vary_max_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 MAX_GRANTED:
+			/* 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",
@@ -277,31 +242,59 @@ static int live_slpc_clamp_max(void *arg)
 			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] 16+ messages in thread

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

== Series Details ==

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

== Summary ==

Error: dim checkpatch failed
d128b50efeb4 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.

-:17: 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")'
#17: 
Fixes 8ee2c227822e ("drm/i915/guc/slpc: Add SLPC selftest")

-:42: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#42: FILE: drivers/gpu/drm/i915/gt/selftest_slpc.c:45:
+static int vary_max_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps,
+		  u32 *max_act_freq)

-:122: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#122: FILE: drivers/gpu/drm/i915/gt/selftest_slpc.c:69:
+			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:85:
+static int vary_min_freq(struct intel_guc_slpc *slpc, struct intel_rps *rps,
+		  u32 *max_act_freq)

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

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

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

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

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



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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/guc/slpc: Add a new SLPC selftest (rev2)
  2022-06-10 23:47 ` [Intel-gfx] " Vinay Belgaumkar
  (?)
  (?)
@ 2022-06-13 22:03 ` Patchwork
  -1 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2022-06-13 22:03 UTC (permalink / raw)
  To: Vinay Belgaumkar; +Cc: intel-gfx

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

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_11755 -> Patchwork_105005v2
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (44 -> 42)
------------------------------

  Missing    (2): bat-adlm-1 fi-rkl-11600 

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

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

### CI changes ###

#### Issues hit ####

  * boot:
    - fi-skl-6600u:       [PASS][1] -> [FAIL][2] ([i915#5032])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/fi-skl-6600u/boot.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-skl-6600u/boot.html

  

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@gem:
    - fi-pnv-d510:        NOTRUN -> [DMESG-FAIL][3] ([i915#4528])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-pnv-d510/igt@i915_selftest@live@gem.html

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-g3258:       [PASS][4] -> [INCOMPLETE][5] ([i915#3303] / [i915#4785])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/fi-hsw-g3258/igt@i915_selftest@live@hangcheck.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-hsw-g3258/igt@i915_selftest@live@hangcheck.html
    - fi-bdw-5557u:       NOTRUN -> [INCOMPLETE][6] ([i915#3921])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-bdw-5557u/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@requests:
    - fi-blb-e6850:       [PASS][7] -> [DMESG-FAIL][8] ([i915#4528])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/fi-blb-e6850/igt@i915_selftest@live@requests.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-blb-e6850/igt@i915_selftest@live@requests.html

  * igt@kms_flip@basic-flip-vs-modeset@b-edp1:
    - bat-adlp-4:         [PASS][9] -> [DMESG-WARN][10] ([i915#3576])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/bat-adlp-4/igt@kms_flip@basic-flip-vs-modeset@b-edp1.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/bat-adlp-4/igt@kms_flip@basic-flip-vs-modeset@b-edp1.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@a-edp1:
    - fi-tgl-u2:          [PASS][11] -> [DMESG-WARN][12] ([i915#402])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/fi-tgl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@a-edp1.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-tgl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@a-edp1.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-kbl-guc:         NOTRUN -> [SKIP][13] ([fdo#109271])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-kbl-guc/igt@kms_force_connector_basic@force-load-detect.html

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

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - fi-cfl-8109u:       [DMESG-FAIL][15] ([i915#62]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-cfl-8109u/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@gem_contexts:
    - fi-bdw-5557u:       [INCOMPLETE][17] ([i915#5502]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/fi-bdw-5557u/igt@i915_selftest@live@gem_contexts.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-bdw-5557u/igt@i915_selftest@live@gem_contexts.html

  * igt@i915_selftest@live@gt_heartbeat:
    - {fi-jsl-1}:         [DMESG-FAIL][19] ([i915#5334]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/fi-jsl-1/igt@i915_selftest@live@gt_heartbeat.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-jsl-1/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@late_gt_pm:
    - fi-cfl-8109u:       [DMESG-WARN][21] ([i915#5904]) -> [PASS][22] +29 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-cfl-8109u/igt@i915_selftest@live@late_gt_pm.html

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

  * igt@i915_suspend@basic-s2idle-without-i915:
    - fi-cfl-8109u:       [DMESG-WARN][25] ([i915#5904] / [i915#62]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/fi-cfl-8109u/igt@i915_suspend@basic-s2idle-without-i915.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-cfl-8109u/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@kms_busy@basic@flip:
    - fi-tgl-u2:          [DMESG-WARN][27] ([i915#402]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/fi-tgl-u2/igt@kms_busy@basic@flip.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-tgl-u2/igt@kms_busy@basic@flip.html

  * igt@kms_flip@basic-flip-vs-modeset@a-edp1:
    - {bat-adlp-6}:       [DMESG-WARN][29] ([i915#3576]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/bat-adlp-6/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/bat-adlp-6/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-cfl-8109u:       [DMESG-WARN][31] ([i915#62]) -> [PASS][32] +15 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/fi-cfl-8109u/igt@kms_frontbuffer_tracking@basic.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-cfl-8109u/igt@kms_frontbuffer_tracking@basic.html
    - fi-cml-u2:          [DMESG-WARN][33] ([i915#4269]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.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
  [i915#3303]: https://gitlab.freedesktop.org/drm/intel/issues/3303
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#3921]: https://gitlab.freedesktop.org/drm/intel/issues/3921
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#4269]: https://gitlab.freedesktop.org/drm/intel/issues/4269
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#5032]: https://gitlab.freedesktop.org/drm/intel/issues/5032
  [i915#5270]: https://gitlab.freedesktop.org/drm/intel/issues/5270
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#5502]: https://gitlab.freedesktop.org/drm/intel/issues/5502
  [i915#5763]: https://gitlab.freedesktop.org/drm/intel/issues/5763
  [i915#5904]: https://gitlab.freedesktop.org/drm/intel/issues/5904
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62


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

  * Linux: CI_DRM_11755 -> Patchwork_105005v2

  CI-20190529: 20190529
  CI_DRM_11755: 65b93b94d6bc932ed60bb3fd9d68242db25b1f3b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6522: 5be5a1a1f168a59614101b77385f05f12ec7d30a @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_105005v2: 65b93b94d6bc932ed60bb3fd9d68242db25b1f3b @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

8ae510fa9d0f drm/i915/guc/slpc: Add a new SLPC selftest

== Logs ==

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

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

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

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/guc/slpc: Add a new SLPC selftest (rev2)
  2022-06-10 23:47 ` [Intel-gfx] " Vinay Belgaumkar
                   ` (2 preceding siblings ...)
  (?)
@ 2022-06-14 23:05 ` Patchwork
  -1 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2022-06-14 23:05 UTC (permalink / raw)
  To: Vinay Belgaumkar; +Cc: intel-gfx

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

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_11755_full -> Patchwork_105005v2_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (12 -> 11)
------------------------------

  Missing    (1): shard-dg1 

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

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

### IGT changes ###

#### Suppressed ####

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

  * {igt@kms_cdclk@mode-transition-all-outputs}:
    - shard-iclb:         NOTRUN -> [SKIP][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@kms_cdclk@mode-transition-all-outputs.html

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

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

### CI changes ###

#### Issues hit ####

  * boot:
    - shard-glk:          ([PASS][2], [PASS][3], [PASS][4], [PASS][5], [PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25], [PASS][26]) -> ([PASS][27], [PASS][28], [PASS][29], [PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], [FAIL][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48], [FAIL][49], [PASS][50], [PASS][51]) ([i915#4392])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk1/boot.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk1/boot.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk1/boot.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk2/boot.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk2/boot.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk2/boot.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk3/boot.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk3/boot.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk3/boot.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk4/boot.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk4/boot.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk4/boot.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk5/boot.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk5/boot.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk5/boot.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk6/boot.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk6/boot.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk6/boot.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk7/boot.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk7/boot.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk7/boot.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk8/boot.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk8/boot.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk9/boot.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk9/boot.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk4/boot.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk7/boot.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk7/boot.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk6/boot.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk6/boot.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk6/boot.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk5/boot.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk5/boot.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk4/boot.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk4/boot.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk3/boot.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk3/boot.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk3/boot.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk2/boot.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk2/boot.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk1/boot.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk1/boot.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk1/boot.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk9/boot.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk9/boot.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk9/boot.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk8/boot.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk8/boot.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk8/boot.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk7/boot.html

  
#### Possible fixes ####

  * boot:
    - shard-skl:          ([PASS][52], [PASS][53], [PASS][54], [PASS][55], [PASS][56], [PASS][57], [PASS][58], [PASS][59], [PASS][60], [PASS][61], [PASS][62], [PASS][63], [FAIL][64], [PASS][65], [PASS][66], [PASS][67], [PASS][68], [PASS][69], [PASS][70]) ([i915#5032]) -> ([PASS][71], [PASS][72], [PASS][73], [PASS][74], [PASS][75], [PASS][76], [PASS][77], [PASS][78], [PASS][79], [PASS][80], [PASS][81], [PASS][82], [PASS][83], [PASS][84], [PASS][85], [PASS][86], [PASS][87], [PASS][88], [PASS][89], [PASS][90], [PASS][91], [PASS][92])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl10/boot.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl9/boot.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl9/boot.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl9/boot.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl7/boot.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl7/boot.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl6/boot.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl6/boot.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl5/boot.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl4/boot.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl4/boot.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl4/boot.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl3/boot.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl3/boot.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl2/boot.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl1/boot.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl1/boot.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl1/boot.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl10/boot.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl9/boot.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl9/boot.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl9/boot.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl7/boot.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl7/boot.html
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl6/boot.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl6/boot.html
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl6/boot.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl5/boot.html
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl5/boot.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl4/boot.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl4/boot.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl4/boot.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl3/boot.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl3/boot.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl2/boot.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl1/boot.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl1/boot.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl1/boot.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl10/boot.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl10/boot.html
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl10/boot.html

  

### IGT changes ###

#### Issues hit ####

  * igt@gem_ccs@block-copy-compressed:
    - shard-iclb:         NOTRUN -> [SKIP][93] ([i915#5327])
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb7/igt@gem_ccs@block-copy-compressed.html

  * igt@gem_ctx_persistence@hang:
    - shard-skl:          NOTRUN -> [SKIP][94] ([fdo#109271]) +419 similar issues
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl4/igt@gem_ctx_persistence@hang.html

  * igt@gem_ctx_persistence@smoketest:
    - shard-snb:          NOTRUN -> [SKIP][95] ([fdo#109271] / [i915#1099])
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-snb5/igt@gem_ctx_persistence@smoketest.html

  * igt@gem_eio@unwedge-stress:
    - shard-iclb:         [PASS][96] -> [TIMEOUT][97] ([i915#3070])
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb8/igt@gem_eio@unwedge-stress.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb7/igt@gem_eio@unwedge-stress.html

  * igt@gem_exec_balancer@parallel-bb-first:
    - shard-iclb:         [PASS][98] -> [SKIP][99] ([i915#4525])
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb2/igt@gem_exec_balancer@parallel-bb-first.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb5/igt@gem_exec_balancer@parallel-bb-first.html

  * igt@gem_exec_fair@basic-none@vcs0:
    - shard-kbl:          [PASS][100] -> [FAIL][101] ([i915#2842]) +4 similar issues
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-kbl7/igt@gem_exec_fair@basic-none@vcs0.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-kbl7/igt@gem_exec_fair@basic-none@vcs0.html

  * igt@gem_exec_fair@basic-pace@rcs0:
    - shard-iclb:         [PASS][102] -> [FAIL][103] ([i915#2842])
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb7/igt@gem_exec_fair@basic-pace@rcs0.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb3/igt@gem_exec_fair@basic-pace@rcs0.html

  * igt@gem_exec_params@no-vebox:
    - shard-iclb:         NOTRUN -> [SKIP][104] ([fdo#109283])
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@gem_exec_params@no-vebox.html

  * igt@gem_lmem_swapping@basic:
    - shard-skl:          NOTRUN -> [SKIP][105] ([fdo#109271] / [i915#4613]) +5 similar issues
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl3/igt@gem_lmem_swapping@basic.html

  * igt@gem_lmem_swapping@heavy-verify-random-ccs:
    - shard-apl:          NOTRUN -> [SKIP][106] ([fdo#109271] / [i915#4613])
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-apl3/igt@gem_lmem_swapping@heavy-verify-random-ccs.html

  * igt@gem_lmem_swapping@verify-random:
    - shard-iclb:         NOTRUN -> [SKIP][107] ([i915#4613])
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb7/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_userptr_blits@input-checking:
    - shard-apl:          NOTRUN -> [DMESG-WARN][108] ([i915#4991])
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-apl4/igt@gem_userptr_blits@input-checking.html

  * igt@gem_userptr_blits@invalid-mmap-offset-unsync:
    - shard-iclb:         NOTRUN -> [SKIP][109] ([i915#3297])
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb7/igt@gem_userptr_blits@invalid-mmap-offset-unsync.html

  * igt@gem_workarounds@suspend-resume-fd:
    - shard-kbl:          [PASS][110] -> [DMESG-WARN][111] ([i915#180]) +2 similar issues
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-kbl6/igt@gem_workarounds@suspend-resume-fd.html
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-kbl7/igt@gem_workarounds@suspend-resume-fd.html

  * igt@i915_module_load@load:
    - shard-skl:          NOTRUN -> [SKIP][112] ([fdo#109271] / [i915#6227])
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl4/igt@i915_module_load@load.html

  * igt@i915_pm_dc@dc6-dpms:
    - shard-iclb:         [PASS][113] -> [FAIL][114] ([i915#454])
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb4/igt@i915_pm_dc@dc6-dpms.html
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb3/igt@i915_pm_dc@dc6-dpms.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-skl:          NOTRUN -> [FAIL][115] ([i915#454])
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl4/igt@i915_pm_dc@dc6-psr.html

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-iclb:         NOTRUN -> [WARN][116] ([i915#2684])
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@i915_selftest@mock@requests:
    - shard-skl:          NOTRUN -> [INCOMPLETE][117] ([i915#5183])
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl7/igt@i915_selftest@mock@requests.html

  * igt@kms_big_fb@4-tiled-8bpp-rotate-270:
    - shard-iclb:         NOTRUN -> [SKIP][118] ([i915#5286]) +2 similar issues
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@kms_big_fb@4-tiled-8bpp-rotate-270.html

  * igt@kms_big_fb@x-tiled-64bpp-rotate-270:
    - shard-iclb:         NOTRUN -> [SKIP][119] ([fdo#110725] / [fdo#111614])
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@kms_big_fb@x-tiled-64bpp-rotate-270.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-hflip:
    - shard-apl:          NOTRUN -> [SKIP][120] ([fdo#109271]) +84 similar issues
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-apl8/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-hflip.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip:
    - shard-skl:          NOTRUN -> [FAIL][121] ([i915#3763])
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl7/igt@kms_big_fb@y-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html

  * igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-0-async-flip:
    - shard-skl:          NOTRUN -> [FAIL][122] ([i915#3743]) +1 similar issue
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl10/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-0-async-flip.html

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_gen12_mc_ccs:
    - shard-glk:          NOTRUN -> [SKIP][123] ([fdo#109271] / [i915#3886]) +1 similar issue
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk8/igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc:
    - shard-skl:          NOTRUN -> [SKIP][124] ([fdo#109271] / [i915#1888] / [i915#3886])
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl3/igt@kms_ccs@pipe-a-crc-sprite-planes-basic-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc:
    - shard-apl:          NOTRUN -> [SKIP][125] ([fdo#109271] / [i915#3886])
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-apl8/igt@kms_ccs@pipe-c-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-missing-ccs-buffer-y_tiled_gen12_mc_ccs:
    - shard-iclb:         NOTRUN -> [SKIP][126] ([fdo#109278] / [i915#3886]) +2 similar issues
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb7/igt@kms_ccs@pipe-c-missing-ccs-buffer-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-c-random-ccs-data-y_tiled_gen12_rc_ccs_cc:
    - shard-skl:          NOTRUN -> [SKIP][127] ([fdo#109271] / [i915#3886]) +13 similar issues
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl3/igt@kms_ccs@pipe-c-random-ccs-data-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_chamelium@dp-edid-change-during-suspend:
    - shard-glk:          NOTRUN -> [SKIP][128] ([fdo#109271] / [fdo#111827]) +2 similar issues
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk8/igt@kms_chamelium@dp-edid-change-during-suspend.html

  * igt@kms_chamelium@dp-hpd-storm-disable:
    - shard-skl:          NOTRUN -> [SKIP][129] ([fdo#109271] / [fdo#111827] / [i915#1888])
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl3/igt@kms_chamelium@dp-hpd-storm-disable.html

  * igt@kms_chamelium@hdmi-crc-single:
    - shard-iclb:         NOTRUN -> [SKIP][130] ([fdo#109284] / [fdo#111827]) +5 similar issues
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb7/igt@kms_chamelium@hdmi-crc-single.html

  * igt@kms_chamelium@hdmi-edid-read:
    - shard-snb:          NOTRUN -> [SKIP][131] ([fdo#109271] / [fdo#111827]) +4 similar issues
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-snb5/igt@kms_chamelium@hdmi-edid-read.html

  * igt@kms_chamelium@hdmi-hpd-with-enabled-mode:
    - shard-skl:          NOTRUN -> [SKIP][132] ([fdo#109271] / [fdo#111827]) +23 similar issues
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl3/igt@kms_chamelium@hdmi-hpd-with-enabled-mode.html

  * igt@kms_color_chamelium@pipe-c-ctm-blue-to-red:
    - shard-apl:          NOTRUN -> [SKIP][133] ([fdo#109271] / [fdo#111827]) +2 similar issues
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-apl4/igt@kms_color_chamelium@pipe-c-ctm-blue-to-red.html

  * igt@kms_color_chamelium@pipe-d-ctm-negative:
    - shard-iclb:         NOTRUN -> [SKIP][134] ([fdo#109278] / [fdo#109284] / [fdo#111827])
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@kms_color_chamelium@pipe-d-ctm-negative.html

  * igt@kms_cursor_crc@pipe-c-cursor-512x170-random:
    - shard-iclb:         NOTRUN -> [SKIP][135] ([fdo#109278] / [fdo#109279]) +1 similar issue
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@kms_cursor_crc@pipe-c-cursor-512x170-random.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-skl:          NOTRUN -> [FAIL][136] ([i915#2346] / [i915#533])
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_draw_crc@draw-method-rgb565-render-4tiled:
    - shard-iclb:         NOTRUN -> [SKIP][137] ([i915#5287])
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb7/igt@kms_draw_crc@draw-method-rgb565-render-4tiled.html

  * igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
    - shard-iclb:         NOTRUN -> [SKIP][138] ([fdo#109274])
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible.html

  * igt@kms_flip@flip-vs-expired-vblank@b-edp1:
    - shard-skl:          [PASS][139] -> [FAIL][140] ([i915#2122])
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl6/igt@kms_flip@flip-vs-expired-vblank@b-edp1.html
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl5/igt@kms_flip@flip-vs-expired-vblank@b-edp1.html

  * igt@kms_flip@flip-vs-expired-vblank@c-edp1:
    - shard-skl:          [PASS][141] -> [FAIL][142] ([i915#79])
   [141]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl6/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html
   [142]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl5/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html

  * igt@kms_flip@flip-vs-suspend@b-dp1:
    - shard-apl:          [PASS][143] -> [DMESG-WARN][144] ([i915#180]) +2 similar issues
   [143]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-apl1/igt@kms_flip@flip-vs-suspend@b-dp1.html
   [144]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-apl4/igt@kms_flip@flip-vs-suspend@b-dp1.html

  * igt@kms_flip@flip-vs-suspend@c-dp1:
    - shard-kbl:          [PASS][145] -> [INCOMPLETE][146] ([i915#3614])
   [145]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-kbl1/igt@kms_flip@flip-vs-suspend@c-dp1.html
   [146]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-kbl3/igt@kms_flip@flip-vs-suspend@c-dp1.html

  * igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1:
    - shard-skl:          NOTRUN -> [FAIL][147] ([i915#2122])
   [147]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl2/igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-upscaling:
    - shard-glk:          NOTRUN -> [FAIL][148] ([i915#4911])
   [148]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk8/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-upscaling.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytileccs-to-64bpp-ytile-downscaling:
    - shard-iclb:         [PASS][149] -> [SKIP][150] ([i915#3701])
   [149]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb5/igt@kms_flip_scaled_crc@flip-32bpp-ytileccs-to-64bpp-ytile-downscaling.html
   [150]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-ytileccs-to-64bpp-ytile-downscaling.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytileccs-to-64bpp-ytile-upscaling:
    - shard-glk:          [PASS][151] -> [FAIL][152] ([i915#4911])
   [151]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk7/igt@kms_flip_scaled_crc@flip-32bpp-ytileccs-to-64bpp-ytile-upscaling.html
   [152]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk8/igt@kms_flip_scaled_crc@flip-32bpp-ytileccs-to-64bpp-ytile-upscaling.html

  * igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytile-downscaling:
    - shard-skl:          NOTRUN -> [SKIP][153] ([fdo#109271] / [i915#3701]) +2 similar issues
   [153]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl4/igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytile-downscaling.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-mmap-cpu:
    - shard-iclb:         NOTRUN -> [SKIP][154] ([fdo#109280]) +12 similar issues
   [154]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-mmap-cpu.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-iclb:         NOTRUN -> [SKIP][155] ([i915#3555])
   [155]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_hdr@bpc-switch-suspend@pipe-a-dp-1:
    - shard-kbl:          [PASS][156] -> [FAIL][157] ([i915#1188])
   [156]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-kbl4/igt@kms_hdr@bpc-switch-suspend@pipe-a-dp-1.html
   [157]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-kbl4/igt@kms_hdr@bpc-switch-suspend@pipe-a-dp-1.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-d-frame-sequence:
    - shard-skl:          NOTRUN -> [SKIP][158] ([fdo#109271] / [i915#533]) +3 similar issues
   [158]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl1/igt@kms_pipe_crc_basic@read-crc-pipe-d-frame-sequence.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-apl:          NOTRUN -> [FAIL][159] ([fdo#108145] / [i915#265])
   [159]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-apl3/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-skl:          NOTRUN -> [FAIL][160] ([fdo#108145] / [i915#265])
   [160]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-transparent-fb:
    - shard-apl:          NOTRUN -> [FAIL][161] ([i915#265])
   [161]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-apl4/igt@kms_plane_alpha_blend@pipe-b-alpha-transparent-fb.html

  * igt@kms_plane_cursor@pipe-d-overlay-size-128:
    - shard-iclb:         NOTRUN -> [SKIP][162] ([fdo#109278]) +13 similar issues
   [162]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@kms_plane_cursor@pipe-d-overlay-size-128.html

  * igt@kms_plane_scaling@plane-downscale-with-rotation-factor-0-75@pipe-c-edp-1:
    - shard-iclb:         NOTRUN -> [SKIP][163] ([i915#5176]) +2 similar issues
   [163]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb7/igt@kms_plane_scaling@plane-downscale-with-rotation-factor-0-75@pipe-c-edp-1.html

  * igt@kms_plane_scaling@planes-upscale-20x20-downscale-factor-0-5@pipe-b-edp-1:
    - shard-iclb:         [PASS][164] -> [SKIP][165] ([i915#5235]) +2 similar issues
   [164]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb5/igt@kms_plane_scaling@planes-upscale-20x20-downscale-factor-0-5@pipe-b-edp-1.html
   [165]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb2/igt@kms_plane_scaling@planes-upscale-20x20-downscale-factor-0-5@pipe-b-edp-1.html

  * igt@kms_prime@basic-crc@first-to-second:
    - shard-iclb:         NOTRUN -> [SKIP][166] ([i915#1836])
   [166]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb7/igt@kms_prime@basic-crc@first-to-second.html

  * igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-sf:
    - shard-tglb:         NOTRUN -> [SKIP][167] ([i915#2920])
   [167]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-tglb8/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-sf.html

  * igt@kms_psr2_su@page_flip-nv12:
    - shard-apl:          NOTRUN -> [SKIP][168] ([fdo#109271] / [i915#658])
   [168]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-apl3/igt@kms_psr2_su@page_flip-nv12.html

  * igt@kms_psr2_su@page_flip-xrgb8888:
    - shard-skl:          NOTRUN -> [SKIP][169] ([fdo#109271] / [i915#658]) +2 similar issues
   [169]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl7/igt@kms_psr2_su@page_flip-xrgb8888.html

  * igt@kms_psr@psr2_dpms:
    - shard-iclb:         [PASS][170] -> [SKIP][171] ([fdo#109441]) +1 similar issue
   [170]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb2/igt@kms_psr@psr2_dpms.html
   [171]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb4/igt@kms_psr@psr2_dpms.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         NOTRUN -> [SKIP][172] ([fdo#109441])
   [172]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@kms_psr@psr2_no_drrs.html

  * igt@kms_psr@psr2_sprite_plane_onoff:
    - shard-snb:          NOTRUN -> [SKIP][173] ([fdo#109271]) +78 similar issues
   [173]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-snb5/igt@kms_psr@psr2_sprite_plane_onoff.html

  * igt@kms_psr_stress_test@flip-primary-invalidate-overlay:
    - shard-iclb:         [PASS][174] -> [SKIP][175] ([i915#5519])
   [174]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb1/igt@kms_psr_stress_test@flip-primary-invalidate-overlay.html
   [175]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb1/igt@kms_psr_stress_test@flip-primary-invalidate-overlay.html

  * igt@kms_writeback@writeback-invalid-parameters:
    - shard-iclb:         NOTRUN -> [SKIP][176] ([i915#2437])
   [176]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb7/igt@kms_writeback@writeback-invalid-parameters.html

  * igt@kms_writeback@writeback-pixel-formats:
    - shard-skl:          NOTRUN -> [SKIP][177] ([fdo#109271] / [i915#2437]) +1 similar issue
   [177]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl7/igt@kms_writeback@writeback-pixel-formats.html

  * igt@nouveau_crc@pipe-c-source-outp-complete:
    - shard-iclb:         NOTRUN -> [SKIP][178] ([i915#2530])
   [178]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@nouveau_crc@pipe-c-source-outp-complete.html

  * igt@nouveau_crc@pipe-d-ctx-flip-detection:
    - shard-glk:          NOTRUN -> [SKIP][179] ([fdo#109271]) +28 similar issues
   [179]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk8/igt@nouveau_crc@pipe-d-ctx-flip-detection.html

  * igt@nouveau_crc@pipe-d-source-outp-inactive:
    - shard-iclb:         NOTRUN -> [SKIP][180] ([fdo#109278] / [i915#2530])
   [180]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@nouveau_crc@pipe-d-source-outp-inactive.html

  * igt@prime_nv_api@i915_nv_double_import:
    - shard-iclb:         NOTRUN -> [SKIP][181] ([fdo#109291]) +1 similar issue
   [181]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb8/igt@prime_nv_api@i915_nv_double_import.html

  * igt@sw_sync@sync_merge_same:
    - shard-skl:          NOTRUN -> [FAIL][182] ([i915#6140]) +1 similar issue
   [182]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl4/igt@sw_sync@sync_merge_same.html

  * igt@sysfs_clients@create:
    - shard-iclb:         NOTRUN -> [SKIP][183] ([i915#2994])
   [183]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb7/igt@sysfs_clients@create.html

  * igt@sysfs_clients@split-25:
    - shard-skl:          NOTRUN -> [SKIP][184] ([fdo#109271] / [i915#2994]) +2 similar issues
   [184]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl6/igt@sysfs_clients@split-25.html

  * igt@sysfs_clients@split-50:
    - shard-glk:          NOTRUN -> [SKIP][185] ([fdo#109271] / [i915#2994])
   [185]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk8/igt@sysfs_clients@split-50.html

  
#### Possible fixes ####

  * igt@core_setmaster@master-drop-set-user:
    - shard-iclb:         [INCOMPLETE][186] -> [PASS][187] +1 similar issue
   [186]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb4/igt@core_setmaster@master-drop-set-user.html
   [187]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb7/igt@core_setmaster@master-drop-set-user.html

  * igt@gem_exec_fair@basic-none@vecs0:
    - shard-apl:          [FAIL][188] ([i915#2842]) -> [PASS][189]
   [188]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-apl1/igt@gem_exec_fair@basic-none@vecs0.html
   [189]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-apl6/igt@gem_exec_fair@basic-none@vecs0.html

  * igt@gem_exec_fair@basic-pace-solo@rcs0:
    - shard-kbl:          [FAIL][190] ([i915#2842]) -> [PASS][191]
   [190]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-kbl7/igt@gem_exec_fair@basic-pace-solo@rcs0.html
   [191]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-kbl1/igt@gem_exec_fair@basic-pace-solo@rcs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - shard-iclb:         [FAIL][192] ([i915#2849]) -> [PASS][193]
   [192]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb4/igt@gem_exec_fair@basic-throttle@rcs0.html
   [193]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb3/igt@gem_exec_fair@basic-throttle@rcs0.html

  * igt@gem_workarounds@suspend-resume:
    - shard-apl:          [DMESG-WARN][194] ([i915#180]) -> [PASS][195] +2 similar issues
   [194]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-apl6/igt@gem_workarounds@suspend-resume.html
   [195]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-apl4/igt@gem_workarounds@suspend-resume.html

  * igt@gen9_exec_parse@allowed-all:
    - shard-glk:          [DMESG-WARN][196] ([i915#5566] / [i915#716]) -> [PASS][197]
   [196]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk7/igt@gen9_exec_parse@allowed-all.html
   [197]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk8/igt@gen9_exec_parse@allowed-all.html

  * igt@i915_module_load@reload-with-fault-injection:
    - shard-snb:          [DMESG-WARN][198] ([i915#6201]) -> [PASS][199]
   [198]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-snb7/igt@i915_module_load@reload-with-fault-injection.html
   [199]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-snb5/igt@i915_module_load@reload-with-fault-injection.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip:
    - shard-tglb:         [FAIL][200] ([i915#3743]) -> [PASS][201]
   [200]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-tglb2/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip.html
   [201]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-tglb7/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-glk:          [FAIL][202] ([i915#2346] / [i915#533]) -> [PASS][203]
   [202]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html
   [203]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk3/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_flip@flip-vs-expired-vblank@a-dp1:
    - shard-kbl:          [FAIL][204] ([i915#79]) -> [PASS][205]
   [204]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-kbl7/igt@kms_flip@flip-vs-expired-vblank@a-dp1.html
   [205]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-kbl1/igt@kms_flip@flip-vs-expired-vblank@a-dp1.html

  * igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a2:
    - shard-glk:          [FAIL][206] ([i915#79]) -> [PASS][207]
   [206]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-glk4/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a2.html
   [207]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-glk5/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a2.html

  * igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-16bpp-ytile-downscaling:
    - shard-iclb:         [SKIP][208] ([i915#3701]) -> [PASS][209]
   [208]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb2/igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-16bpp-ytile-downscaling.html
   [209]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb5/igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-16bpp-ytile-downscaling.html

  * igt@kms_psr@psr2_sprite_render:
    - shard-iclb:         [SKIP][210] ([fdo#109441]) -> [PASS][211] +2 similar issues
   [210]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb8/igt@kms_psr@psr2_sprite_render.html
   [211]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb2/igt@kms_psr@psr2_sprite_render.html

  
#### Warnings ####

  * igt@gem_exec_balancer@parallel-ordering:
    - shard-iclb:         [SKIP][212] ([i915#4525]) -> [FAIL][213] ([i915#6117])
   [212]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb5/igt@gem_exec_balancer@parallel-ordering.html
   [213]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb2/igt@gem_exec_balancer@parallel-ordering.html

  * igt@gem_exec_fair@basic-none-rrul@rcs0:
    - shard-iclb:         [FAIL][214] ([i915#2852]) -> [FAIL][215] ([i915#2842])
   [214]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb5/igt@gem_exec_fair@basic-none-rrul@rcs0.html
   [215]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb2/igt@gem_exec_fair@basic-none-rrul@rcs0.html

  * igt@gem_exec_fair@basic-pace@vcs1:
    - shard-tglb:         [FAIL][216] ([i915#2842]) -> [SKIP][217] ([i915#2848]) +4 similar issues
   [216]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-tglb5/igt@gem_exec_fair@basic-pace@vcs1.html
   [217]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-tglb2/igt@gem_exec_fair@basic-pace@vcs1.html

  * igt@kms_big_fb@x-tiled-8bpp-rotate-270:
    - shard-skl:          [SKIP][218] ([fdo#109271] / [i915#1888]) -> [SKIP][219] ([fdo#109271])
   [218]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl2/igt@kms_big_fb@x-tiled-8bpp-rotate-270.html
   [219]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl2/igt@kms_big_fb@x-tiled-8bpp-rotate-270.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-plflip-blt:
    - shard-skl:          [SKIP][220] ([fdo#109271]) -> [SKIP][221] ([fdo#109271] / [i915#1888])
   [220]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-skl4/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-plflip-blt.html
   [221]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-skl10/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-plflip-blt.html

  * igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-sf:
    - shard-iclb:         [SKIP][222] ([i915#2920]) -> [SKIP][223] ([i915#658])
   [222]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb2/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-sf.html
   [223]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb5/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-sf.html

  * igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-fully-sf:
    - shard-iclb:         [SKIP][224] ([i915#658]) -> [SKIP][225] ([i915#2920])
   [224]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb5/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-fully-sf.html
   [225]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb2/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-fully-sf.html

  * igt@kms_psr2_su@page_flip-nv12:
    - shard-iclb:         [SKIP][226] ([fdo#109642] / [fdo#111068] / [i915#658]) -> [FAIL][227] ([i915#5939])
   [226]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-iclb5/igt@kms_psr2_su@page_flip-nv12.html
   [227]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-iclb2/igt@kms_psr2_su@page_flip-nv12.html

  * igt@runner@aborted:
    - shard-kbl:          ([FAIL][228], [FAIL][229]) ([i915#3002] / [i915#4312] / [i915#5257]) -> ([FAIL][230], [FAIL][231], [FAIL][232], [FAIL][233], [FAIL][234]) ([fdo#109271] / [i915#180] / [i915#3002] / [i915#4312] / [i915#5257])
   [228]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-kbl1/igt@runner@aborted.html
   [229]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11755/shard-kbl7/igt@runner@aborted.html
   [230]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-kbl3/igt@runner@aborted.html
   [231]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-kbl7/igt@runner@aborted.html
   [232]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-kbl1/igt@runner@aborted.html
   [233]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-kbl7/igt@runner@aborted.html
   [234]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_105005v2/shard-kbl7/igt@runner@aborted.html

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

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110725]: https://bugs.freedesktop.org/show_bug.cgi?id=110725
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111614]: https://bugs.freedesktop.org/show_bug.cgi?id=111614
  [fdo#111615]: https://bugs.freedesktop.org/show_bug.cgi?id=111615
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1099]: https://gitlab.freedesktop.org/drm/intel/issues/1099
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1836]: https://gitlab.freedesktop.org/drm/intel/issues/1836
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2527]: https://gitlab.freedesktop.org/drm/intel/issues/2527
  [i915#2530]: https://gitlab.freedesktop.org/drm/intel/issues/2530
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#2684]: https://gitlab.freedesktop.org/drm/intel/issues/2684
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2848]: https://gitlab.freedesktop.org/drm/intel/issues/2848
  [i915#2849]: https://gitlab.freedesktop.org/drm/intel/issues/2849
  [i915#2852]: https://gitlab.freedesktop.org/drm/intel/issues/2852
  [i915#2856]: https://gitlab.freedesktop.org/drm/intel/issues/2856
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [i915#2994]: https://gitlab.freedesktop.org/drm/intel/issues/2994
  [i915#3002]: https://gitlab.freedesktop.org/drm/intel/issues/3002
  [i915#3070]: https://gitlab.freedesktop.org/drm/intel/issues/3070
  [i915#3297]: https://gitlab.freedesktop.org/drm/intel/issues/3297
  [i915#3319]: https://gitlab.freedesktop.org/drm/intel/issues/3319
  [i915#3323]: https://gitlab.freedesktop.org/drm/intel/issues/3323
  [i915#3359]: https://gitlab.freedesktop.org/drm/intel/issues/3359
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3614]: https://gitlab.freedesktop.org/drm/intel/issues/3614
  [i915#3689]: https://gitlab.freedesktop.org/drm/intel/issues/3689
  [i915#3701]: https://gitlab.freedesktop.org/drm/intel/issues/3701
  [i915#3743]: https://gitlab.freedesktop.org/drm/intel/issues/3743
  [i915#3763]: https://gitlab.freedesktop.org/drm/intel/issues/3763
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#3987]: https://gitlab.freedesktop.org/drm/intel/issues/3987
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4392]: https://gitlab.freedesktop.org/drm/intel/issues/4392
  [i915#4525]: https://gitlab.freedesktop.org/drm/intel/issues/4525
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4911]: https://gitlab.freedesktop.org/drm/intel/issues/4911
  [i915#4991]: https://gitlab.freedesktop.org/drm/intel/issues/4991
  [i915#5032]: https://gitlab.freedesktop.org/drm/intel/issues/5032
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5183]: https://gitlab.freedesktop.org/drm/intel/issues/5183
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#5257]: https://gitlab.freedesktop.org/drm/intel/issues/5257
  [i915#5286]: https://gitlab.freedesktop.org/drm/intel/issues/5286
  [i915#5287]: https://gitlab.freedesktop.org/drm/intel/issues/5287
  [i915#5288]: https://gitlab.freedesktop.org/drm/intel/issues/5288
  [i915#5327]: https://gitlab.freedesktop.org/drm/intel/issues/5327
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5461]: https://gitlab.freedesktop.org/drm/intel/issues/5461
  [i915#5519]: https://gitlab.freedesktop.org/drm/intel/issues/5519
  [i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
  [i915#5903]: https://gitlab.freedesktop.org/drm/intel/issues/5903
  [i915#5939]: https://gitlab.freedesktop.org/drm/intel/issues/5939
  [i915#6095]: https://gitlab.freedesktop.org/drm/intel/issues/6095
  [i915#6117]: https://gitlab.freedesktop.org/drm/intel/issues/6117
  [i915#6140]: https://gitlab.freedesktop.org/drm/intel/issues/6140
  [i915#6201]: https://gitlab.freedesktop.org/drm/intel/issues/6201
  [i915#6227]: https://gitlab.freedesktop.org/drm/intel/issues/6227
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79


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

  * Linux: CI_DRM_11755 -> Patchwork_105005v2

  CI-20190529: 20190529
  CI_DRM_11755: 65b93b94d6bc932ed60bb3fd9d68242db25b1f3b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6522: 5be5a1a1f168a59614101b77385f05f12ec7d30a @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_105005v2: 65b93b94d6bc932ed60bb3fd9d68242db25b1f3b @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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

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

* Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
  2022-06-10 23:47 ` [Intel-gfx] " Vinay Belgaumkar
                   ` (3 preceding siblings ...)
  (?)
@ 2022-06-22 20:32 ` Dixit, Ashutosh
  2022-06-23 23:21   ` Belgaumkar, Vinay
  -1 siblings, 1 reply; 16+ messages in thread
From: Dixit, Ashutosh @ 2022-06-22 20:32 UTC (permalink / raw)
  To: Vinay Belgaumkar; +Cc: intel-gfx, dri-devel

On Fri, 10 Jun 2022 16:47:12 -0700, Vinay Belgaumkar wrote:
>
> 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.

The refactoring does change the order of operations (changing the freq vs
spawning the spinner) but should be fine I think.

> Rename the "clamp" tests to vary_max_freq and vary_min_freq.

Either is ok, but maybe "clamp" names were ok I think since they verify req
freq is clamped at min/max.

>
> v2: Fix compile warning
>
> Fixes 8ee2c227822e ("drm/i915/guc/slpc: Add SLPC selftest")
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/selftest_slpc.c | 323 ++++++++++++------------
>  1 file changed, 158 insertions(+), 165 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
> index b768cea5943d..099129aae9a5 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,120 @@ 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)

Please run checkpatch, indentation seems off.

>  {
> -	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 step, max_freq, req_freq;
> +	u32 act_freq;
>	u32 slpc_min_freq, slpc_max_freq;
>	int err = 0;
>
> -	if (!intel_uc_uses_guc_slpc(&gt->uc))
> -		return 0;
> -
> -	if (igt_spinner_init(&spin, gt))
> -		return -ENOMEM;
> +	slpc_min_freq = slpc->min_freq;
> +	slpc_max_freq = slpc->rp0_freq;

nit but we don't really need such variables since we don't change their
values, we should just use slpc->min_freq, slpc->rp0_freq directly. I'd
change this in all places in this patch.

>
> -	if (intel_guc_slpc_get_max_freq(slpc, &slpc_max_freq)) {
> -		pr_err("Could not get SLPC max freq\n");
> -		return -EIO;
> -	}
> -
> -	if (intel_guc_slpc_get_min_freq(slpc, &slpc_min_freq)) {
> -		pr_err("Could not get SLPC min freq\n");
> -		return -EIO;

Why do we need these two function calls? Can't we just use slpc->rp0_freq
and slpc->min_freq as we are doing in the vary_min/max_freq() functions
above?

Also, as mentioned below I think here we should just do:

        slpc_set_max_freq(slpc, slpc->rp0_freq);
        slpc_set_min_freq(slpc, slpc->min_freq);

to restore freq to a known state before starting the test (just in case a
previous test changed the values).

> -	}
> -
> -	if (slpc_min_freq == slpc_max_freq) {
> -		pr_err("Min/Max are fused to the same value\n");
> -		return -EINVAL;

What if they are actually equal? I think basically the max/min freq test
loops will just not be entered (so effectively the tests will just
skip). The granted freq test will be fine. So I think we can just delete
this if statement?

(It is showing deleted above in the patch but is in the new code somewhere
too).

> -	}
> -
> -	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;
> -
> -		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;
>
> -		/* 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;
> -			}
> +		req_freq = intel_rps_read_punit_req_frequency(rps);
>
> -			i915_request_add(rq);
> +		/* 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;

Probably a nit but check can be (so should we be checking both high and low
limits?):
		if (req_freq > (max_freq + FREQUENCY_REQ_UNIT) ||
		    req_freq < (slpc_min_freq - FREQUENCY_REQ_UNIT))

Though if we do this we'd need to change the pr_err() or have two separate
if statements. Not sure if it's worth it but thought I'll mention it.

> +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;
> +	u32 slpc_min_freq, slpc_max_freq;
> +	int err = 0;
>
> -			act_freq =  intel_rps_read_actual_frequency(rps);
> -			if (act_freq > max_act_freq)
> -				max_act_freq = act_freq;
> +	slpc_min_freq = slpc->min_freq;
> +	slpc_max_freq = slpc->rp0_freq;
>
> -			igt_spinner_end(&spin);
> -			st_engine_heartbeat_enable(engine);
> -		}
> +	/* 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;
>
> -		pr_info("Max actual frequency for %s was %d\n",
> -			engine->name, max_act_freq);
> +		req_freq = intel_rps_read_punit_req_frequency(rps);
>
> -		/* Actual frequency should rise above min */
> -		if (max_act_freq == slpc_min_freq) {

Nit again. This check is somewhere in the new code but I think a better
check is

		if (max_act_freq <= slpc_min_freq)

just in case the act freq for whatever reason falls below
slpc_min_freq. Even if we know this is impossible (bugs make the impossible
possible).

> -			pr_err("Actual freq did not rise above min\n");
> +		/* 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;

Again nit as above, but check can be:
		if (req_freq < (min_freq - FREQUENCY_REQ_UNIT) ||
		    req_freq > (slpc_max_freq + FREQUENCY_REQ_UNIT)) {

>		}
>
> +		act_freq =  intel_rps_read_actual_frequency(rps);
> +		if (act_freq > *max_act_freq)
> +			*max_act_freq = act_freq;
> +
>		if (err)
>			break;
>	}
>
> -	/* Restore min/max frequencies */
> -	slpc_set_max_freq(slpc, slpc_max_freq);
> -	slpc_set_min_freq(slpc, slpc_min_freq);
> +	return err;
> +}
>
> -	if (igt_flush_test(gt->i915))
> -		err = -EIO;
> +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;
>
> -	intel_gt_pm_put(gt);
> -	igt_spinner_fini(&spin);
> -	intel_gt_pm_wait_for_idle(gt);
> +	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
> +	if (err)
> +		return err;
> +
> +	*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);
> +
> +		/* 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;

Looks incorrect, probably something like:
		if (!(perf_limit_reasons & GT0_PERF_LIMIT_REASONS_MASK))

> +		}
> +	}
>
>	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,69 +181,56 @@ 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;
> +
> +		case VARY_MAX:
> +			err = vary_max_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 MAX_GRANTED:
> +			/* 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;

I think it's preferable to move this media engine code out of the main loop
into max_granted_freq() function if possible (maybe by faking max_act_freq
if needed)?

>			}
>
> -			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",
> @@ -277,31 +242,59 @@ static int live_slpc_clamp_max(void *arg)
>			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);

As mentioned above maybe we should restore at the beginning of the test too
(before the for_each_engine() loop) to start from a known state?

Anyway here maybe get rid of the variables and:

        slpc_set_max_freq(slpc, slpc->rp0_freq);
        slpc_set_min_freq(slpc, slpc->min_freq);

Thanks.
--
Ashutosh

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

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


On 6/22/2022 1:32 PM, Dixit, Ashutosh wrote:
> On Fri, 10 Jun 2022 16:47:12 -0700, Vinay Belgaumkar wrote:
>> 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.
> The refactoring does change the order of operations (changing the freq vs
> spawning the spinner) but should be fine I think.
Yes, we now start the spinner outside the for loop, so that freq changes 
occur quickly. This ensures we don't mess with SLPC algorithm's history 
by frequently restarting the WL in the for loop.
>
>> Rename the "clamp" tests to vary_max_freq and vary_min_freq.
> Either is ok, but maybe "clamp" names were ok I think since they verify req
> freq is clamped at min/max.
True, though clamp usually is associated with limiting, whereas we 
actually increase the min.
>
>> v2: Fix compile warning
>>
>> Fixes 8ee2c227822e ("drm/i915/guc/slpc: Add SLPC selftest")
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/selftest_slpc.c | 323 ++++++++++++------------
>>   1 file changed, 158 insertions(+), 165 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
>> index b768cea5943d..099129aae9a5 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,120 @@ 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)
> Please run checkpatch, indentation seems off.
I had run it. Not sure why this wasn't caught.
>
>>   {
>> -	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 step, max_freq, req_freq;
>> +	u32 act_freq;
>> 	u32 slpc_min_freq, slpc_max_freq;
>> 	int err = 0;
>>
>> -	if (!intel_uc_uses_guc_slpc(&gt->uc))
>> -		return 0;
>> -
>> -	if (igt_spinner_init(&spin, gt))
>> -		return -ENOMEM;
>> +	slpc_min_freq = slpc->min_freq;
>> +	slpc_max_freq = slpc->rp0_freq;
> nit but we don't really need such variables since we don't change their
> values, we should just use slpc->min_freq, slpc->rp0_freq directly. I'd
> change this in all places in this patch.

I will remove it from the sub-functions, but will need to keep the one 
in the main run_test(). We should query SLPC's min and max and then 
restore that at the end of the test. It is possible that SLPC's min is 
different from platform min for certain skus.

>
>> -	if (intel_guc_slpc_get_max_freq(slpc, &slpc_max_freq)) {
>> -		pr_err("Could not get SLPC max freq\n");
>> -		return -EIO;
>> -	}
>> -
>> -	if (intel_guc_slpc_get_min_freq(slpc, &slpc_min_freq)) {
>> -		pr_err("Could not get SLPC min freq\n");
>> -		return -EIO;
> Why do we need these two function calls? Can't we just use slpc->rp0_freq
> and slpc->min_freq as we are doing in the vary_min/max_freq() functions
> above?
Same as above.
>
> Also, as mentioned below I think here we should just do:
>
>          slpc_set_max_freq(slpc, slpc->rp0_freq);
>          slpc_set_min_freq(slpc, slpc->min_freq);
>
> to restore freq to a known state before starting the test (just in case a
> previous test changed the values).
Any test that changes the frequencies should restore them as well.
>
>> -	}
>> -
>> -	if (slpc_min_freq == slpc_max_freq) {
>> -		pr_err("Min/Max are fused to the same value\n");
>> -		return -EINVAL;
> What if they are actually equal? I think basically the max/min freq test
> loops will just not be entered (so effectively the tests will just
> skip). The granted freq test will be fine. So I think we can just delete
> this if statement?
>
> (It is showing deleted above in the patch but is in the new code somewhere
> too).
Actually, we should set it to min/rp0 if this is the case. That change 
will be in a separate patch. This is needed for certain cases.
>
>> -	}
>> -
>> -	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;
>> -
>> -		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;
>>
>> -		/* 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;
>> -			}
>> +		req_freq = intel_rps_read_punit_req_frequency(rps);
>>
>> -			i915_request_add(rq);
>> +		/* 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;
> Probably a nit but check can be (so should we be checking both high and low
> limits?):
> 		if (req_freq > (max_freq + FREQUENCY_REQ_UNIT) ||
> 		    req_freq < (slpc_min_freq - FREQUENCY_REQ_UNIT))
>
> Though if we do this we'd need to change the pr_err() or have two separate
> if statements. Not sure if it's worth it but thought I'll mention it.
We are looking to validate it does not cross the upper limit.
>
>> +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;
>> +	u32 slpc_min_freq, slpc_max_freq;
>> +	int err = 0;
>>
>> -			act_freq =  intel_rps_read_actual_frequency(rps);
>> -			if (act_freq > max_act_freq)
>> -				max_act_freq = act_freq;
>> +	slpc_min_freq = slpc->min_freq;
>> +	slpc_max_freq = slpc->rp0_freq;
>>
>> -			igt_spinner_end(&spin);
>> -			st_engine_heartbeat_enable(engine);
>> -		}
>> +	/* 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;
>>
>> -		pr_info("Max actual frequency for %s was %d\n",
>> -			engine->name, max_act_freq);
>> +		req_freq = intel_rps_read_punit_req_frequency(rps);
>>
>> -		/* Actual frequency should rise above min */
>> -		if (max_act_freq == slpc_min_freq) {
> Nit again. This check is somewhere in the new code but I think a better
> check is
>
> 		if (max_act_freq <= slpc_min_freq)
>
> just in case the act freq for whatever reason falls below
> slpc_min_freq. Even if we know this is impossible (bugs make the impossible
> possible).
sure.
>
>> -			pr_err("Actual freq did not rise above min\n");
>> +		/* 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;
> Again nit as above, but check can be:
> 		if (req_freq < (min_freq - FREQUENCY_REQ_UNIT) ||
> 		    req_freq > (slpc_max_freq + FREQUENCY_REQ_UNIT)) {
It can be higher, we want to validate lower range.
>
>> 		}
>>
>> +		act_freq =  intel_rps_read_actual_frequency(rps);
>> +		if (act_freq > *max_act_freq)
>> +			*max_act_freq = act_freq;
>> +
>> 		if (err)
>> 			break;
>> 	}
>>
>> -	/* Restore min/max frequencies */
>> -	slpc_set_max_freq(slpc, slpc_max_freq);
>> -	slpc_set_min_freq(slpc, slpc_min_freq);
>> +	return err;
>> +}
>>
>> -	if (igt_flush_test(gt->i915))
>> -		err = -EIO;
>> +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;
>>
>> -	intel_gt_pm_put(gt);
>> -	igt_spinner_fini(&spin);
>> -	intel_gt_pm_wait_for_idle(gt);
>> +	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
>> +	if (err)
>> +		return err;
>> +
>> +	*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);
>> +
>> +		/* 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;
> Looks incorrect, probably something like:
> 		if (!(perf_limit_reasons & GT0_PERF_LIMIT_REASONS_MASK))
Hmm, good catch. We should flag error iff there is no throttling and act 
freq does not go to max.
>
>> +		}
>> +	}
>>
>> 	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,69 +181,56 @@ 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;
>> +
>> +		case VARY_MAX:
>> +			err = vary_max_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 MAX_GRANTED:
>> +			/* 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;
> I think it's preferable to move this media engine code out of the main loop
> into max_granted_freq() function if possible (maybe by faking max_act_freq
> if needed)?
All the engine related info is here. I will need to pass it to the 
max_granted_freq() function. Also, faking the act_freq probably 
overkill. I can add a fixme here instead to update when we have a 
reliable way to obtain media RP0 instead.
>
>> 			}
>>
>> -			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",
>> @@ -277,31 +242,59 @@ static int live_slpc_clamp_max(void *arg)
>> 			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);
> As mentioned above maybe we should restore at the beginning of the test too
> (before the for_each_engine() loop) to start from a known state?
>
> Anyway here maybe get rid of the variables and:

This is restoring whatever frequencies SLPC was running with initially. 
Regarding resetting the frequencies to min for every engine loop 
iteration, we are already iterating from min->max inside the for loop, 
so will be duplication.

Thanks for the detailed review,

Vinay.

>
>          slpc_set_max_freq(slpc, slpc->rp0_freq);
>          slpc_set_min_freq(slpc, slpc->min_freq);
>
> Thanks.
> --
> Ashutosh

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

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

On Thu, 23 Jun 2022 16:21:46 -0700, Belgaumkar, Vinay wrote:
> On 6/22/2022 1:32 PM, Dixit, Ashutosh wrote:
> > On Fri, 10 Jun 2022 16:47:12 -0700, Vinay Belgaumkar wrote:
> >> 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.
> > The refactoring does change the order of operations (changing the freq vs
> > spawning the spinner) but should be fine I think.
> Yes, we now start the spinner outside the for loop, so that freq changes
> occur quickly. This ensures we don't mess with SLPC algorithm's history by
> frequently restarting the WL in the for loop.
> >
> >> Rename the "clamp" tests to vary_max_freq and vary_min_freq.
> > Either is ok, but maybe "clamp" names were ok I think since they verify req
> > freq is clamped at min/max.
> True, though clamp usually is associated with limiting, whereas we actually
> increase the min.
> >
> >> v2: Fix compile warning
> >>
> >> Fixes 8ee2c227822e ("drm/i915/guc/slpc: Add SLPC selftest")
> >> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/gt/selftest_slpc.c | 323 ++++++++++++------------
> >>   1 file changed, 158 insertions(+), 165 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
> >> index b768cea5943d..099129aae9a5 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,120 @@ 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)
> > Please run checkpatch, indentation seems off.
> I had run it. Not sure why this wasn't caught.

Need to use 'checkpatch --strict'.

> >
> >>   {
> >> -	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 step, max_freq, req_freq;
> >> +	u32 act_freq;
> >>	u32 slpc_min_freq, slpc_max_freq;
> >>	int err = 0;
> >>
> >> -	if (!intel_uc_uses_guc_slpc(&gt->uc))
> >> -		return 0;
> >> -
> >> -	if (igt_spinner_init(&spin, gt))
> >> -		return -ENOMEM;
> >> +	slpc_min_freq = slpc->min_freq;
> >> +	slpc_max_freq = slpc->rp0_freq;
> > nit but we don't really need such variables since we don't change their
> > values, we should just use slpc->min_freq, slpc->rp0_freq directly. I'd
> > change this in all places in this patch.
>
> I will remove it from the sub-functions, but will need to keep the one in
> the main run_test(). We should query SLPC's min and max and then restore
> that at the end of the test. It is possible that SLPC's min is different
> from platform min for certain skus.

Sorry, I am not following. The tests are varying freq between platform min
to platform max correct? And platform min can be different from slpc min?
So why don't the tests start at slpc min rather than platform min? Can't
this return error?

And shouldn't slpc->min set to the real slpc min rather than to the
platform min when slpc initializes (in intel_guc_slpc_enable() or
slpc_get_rp_values())? (I am assuming the issue is only for the min and not
the max but not sure).

So I'd expect everywhere a consistent set of freq's be used, in run_test()
and the actual vary_min/max_freq tests and also in the main driver.

> >
> >> -	if (intel_guc_slpc_get_max_freq(slpc, &slpc_max_freq)) {
> >> -		pr_err("Could not get SLPC max freq\n");
> >> -		return -EIO;
> >> -	}
> >> -
> >> -	if (intel_guc_slpc_get_min_freq(slpc, &slpc_min_freq)) {
> >> -		pr_err("Could not get SLPC min freq\n");
> >> -		return -EIO;
> > Why do we need these two function calls? Can't we just use slpc->rp0_freq
> > and slpc->min_freq as we are doing in the vary_min/max_freq() functions
> > above?
> Same as above.
> >
> > Also, as mentioned below I think here we should just do:
> >
> >          slpc_set_max_freq(slpc, slpc->rp0_freq);
> >          slpc_set_min_freq(slpc, slpc->min_freq);
> >
> > to restore freq to a known state before starting the test (just in case a
> > previous test changed the values).
> Any test that changes the frequencies should restore them as well.

I was saying restore the freq's *before* starting the tests as well to
start from a known state.

> >
> >> -	}
> >> -
> >> -	if (slpc_min_freq == slpc_max_freq) {
> >> -		pr_err("Min/Max are fused to the same value\n");
> >> -		return -EINVAL;
> > What if they are actually equal? I think basically the max/min freq test
> > loops will just not be entered (so effectively the tests will just
> > skip). The granted freq test will be fine. So I think we can just delete
> > this if statement?
> >
> > (It is showing deleted above in the patch but is in the new code somewhere
> > too).
> Actually, we should set it to min/rp0 if this is the case. That change will
> be in a separate patch. This is needed for certain cases.

I don't see why it's needed as I said, can you explain? Set what to min/rp0?

> >
> >> -	}
> >> -
> >> -	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;
> >> -
> >> -		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;
> >>
> >> -		/* 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;
> >> -			}
> >> +		req_freq = intel_rps_read_punit_req_frequency(rps);
> >>
> >> -			i915_request_add(rq);
> >> +		/* 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;
> > Probably a nit but check can be (so should we be checking both high and low
> > limits?):
> >		if (req_freq > (max_freq + FREQUENCY_REQ_UNIT) ||
> >		    req_freq < (slpc_min_freq - FREQUENCY_REQ_UNIT))
> >
> > Though if we do this we'd need to change the pr_err() or have two separate
> > if statements. Not sure if it's worth it but thought I'll mention it.
> We are looking to validate it does not cross the upper limit.

OK.

> >
> >> +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;
> >> +	u32 slpc_min_freq, slpc_max_freq;
> >> +	int err = 0;
> >>
> >> -			act_freq =  intel_rps_read_actual_frequency(rps);
> >> -			if (act_freq > max_act_freq)
> >> -				max_act_freq = act_freq;
> >> +	slpc_min_freq = slpc->min_freq;
> >> +	slpc_max_freq = slpc->rp0_freq;
> >>
> >> -			igt_spinner_end(&spin);
> >> -			st_engine_heartbeat_enable(engine);
> >> -		}
> >> +	/* 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;
> >>
> >> -		pr_info("Max actual frequency for %s was %d\n",
> >> -			engine->name, max_act_freq);
> >> +		req_freq = intel_rps_read_punit_req_frequency(rps);
> >>
> >> -		/* Actual frequency should rise above min */
> >> -		if (max_act_freq == slpc_min_freq) {
> > Nit again. This check is somewhere in the new code but I think a better
> > check is
> >
> >		if (max_act_freq <= slpc_min_freq)
> >
> > just in case the act freq for whatever reason falls below
> > slpc_min_freq. Even if we know this is impossible (bugs make the impossible
> > possible).
> sure.
> >
> >> -			pr_err("Actual freq did not rise above min\n");
> >> +		/* 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;
> > Again nit as above, but check can be:
> >		if (req_freq < (min_freq - FREQUENCY_REQ_UNIT) ||
> >		    req_freq > (slpc_max_freq + FREQUENCY_REQ_UNIT)) {
> It can be higher, we want to validate lower range.

OK.

> >
> >>		}
> >>
> >> +		act_freq =  intel_rps_read_actual_frequency(rps);
> >> +		if (act_freq > *max_act_freq)
> >> +			*max_act_freq = act_freq;
> >> +
> >>		if (err)
> >>			break;
> >>	}
> >>
> >> -	/* Restore min/max frequencies */
> >> -	slpc_set_max_freq(slpc, slpc_max_freq);
> >> -	slpc_set_min_freq(slpc, slpc_min_freq);
> >> +	return err;
> >> +}
> >>
> >> -	if (igt_flush_test(gt->i915))
> >> -		err = -EIO;
> >> +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;
> >>
> >> -	intel_gt_pm_put(gt);
> >> -	igt_spinner_fini(&spin);
> >> -	intel_gt_pm_wait_for_idle(gt);
> >> +	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
> >> +	if (err)
> >> +		return err;
> >> +
> >> +	*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);
> >> +
> >> +		/* 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;
> > Looks incorrect, probably something like:
> >		if (!(perf_limit_reasons & GT0_PERF_LIMIT_REASONS_MASK))
> Hmm, good catch. We should flag error iff there is no throttling and act
> freq does not go to max.
> >
> >> +		}
> >> +	}
> >>
> >>	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,69 +181,56 @@ 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;
> >> +
> >> +		case VARY_MAX:
> >> +			err = vary_max_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 MAX_GRANTED:
> >> +			/* 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;
> > I think it's preferable to move this media engine code out of the main loop
> > into max_granted_freq() function if possible (maybe by faking max_act_freq
> > if needed)?
> All the engine related info is here. I will need to pass it to the
> max_granted_freq() function.  Also, faking the act_freq probably
> overkill. I can add a fixme here instead to update when we have a
> reliable way to obtain media RP0 instead.

OK let's leave as is, no need for FIXME, just leave the comment as before.

> >
> >>			}
> >>
> >> -			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",
> >> @@ -277,31 +242,59 @@ static int live_slpc_clamp_max(void *arg)
> >>			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);
> > As mentioned above maybe we should restore at the beginning of the test too
> > (before the for_each_engine() loop) to start from a known state?
> >
> > Anyway here maybe get rid of the variables and:
>
> This is restoring whatever frequencies SLPC was running with
> initially. Regarding resetting the frequencies to min for every engine loop
> iteration, we are already iterating from min->max inside the for loop, so
> will be duplication.

I didn't say reset frequencies to min for every engine loop iteration, I
said "before the for_each_engine() loop". Same as above: "I was saying
restore the freq's *before* starting the tests as well to start from a
known state".

Thanks.
--
Ashutosh

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

* Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
  2022-06-25  3:59     ` Dixit, Ashutosh
@ 2022-06-27 22:52       ` Belgaumkar, Vinay
  0 siblings, 0 replies; 16+ 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:21:46 -0700, Belgaumkar, Vinay wrote:
>> On 6/22/2022 1:32 PM, Dixit, Ashutosh wrote:
>>> On Fri, 10 Jun 2022 16:47:12 -0700, Vinay Belgaumkar wrote:
>>>> 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.
>>> The refactoring does change the order of operations (changing the freq vs
>>> spawning the spinner) but should be fine I think.
>> Yes, we now start the spinner outside the for loop, so that freq changes
>> occur quickly. This ensures we don't mess with SLPC algorithm's history by
>> frequently restarting the WL in the for loop.
>>>> Rename the "clamp" tests to vary_max_freq and vary_min_freq.
>>> Either is ok, but maybe "clamp" names were ok I think since they verify req
>>> freq is clamped at min/max.
>> True, though clamp usually is associated with limiting, whereas we actually
>> increase the min.
>>>> v2: Fix compile warning
>>>>
>>>> Fixes 8ee2c227822e ("drm/i915/guc/slpc: Add SLPC selftest")
>>>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/gt/selftest_slpc.c | 323 ++++++++++++------------
>>>>    1 file changed, 158 insertions(+), 165 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
>>>> index b768cea5943d..099129aae9a5 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,120 @@ 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)
>>> Please run checkpatch, indentation seems off.
>> I had run it. Not sure why this wasn't caught.
> Need to use 'checkpatch --strict'.
ok.
>
>>>>    {
>>>> -	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 step, max_freq, req_freq;
>>>> +	u32 act_freq;
>>>> 	u32 slpc_min_freq, slpc_max_freq;
>>>> 	int err = 0;
>>>>
>>>> -	if (!intel_uc_uses_guc_slpc(&gt->uc))
>>>> -		return 0;
>>>> -
>>>> -	if (igt_spinner_init(&spin, gt))
>>>> -		return -ENOMEM;
>>>> +	slpc_min_freq = slpc->min_freq;
>>>> +	slpc_max_freq = slpc->rp0_freq;
>>> nit but we don't really need such variables since we don't change their
>>> values, we should just use slpc->min_freq, slpc->rp0_freq directly. I'd
>>> change this in all places in this patch.
>> I will remove it from the sub-functions, but will need to keep the one in
>> the main run_test(). We should query SLPC's min and max and then restore
>> that at the end of the test. It is possible that SLPC's min is different
>> from platform min for certain skus.
> Sorry, I am not following. The tests are varying freq between platform min
> to platform max correct? And platform min can be different from slpc min?
> So why don't the tests start at slpc min rather than platform min? Can't
> this return error?
Will start the tests from platform min -> platform max, that way we 
remain consistent.
>
> And shouldn't slpc->min set to the real slpc min rather than to the
> platform min when slpc initializes (in intel_guc_slpc_enable() or
> slpc_get_rp_values())? (I am assuming the issue is only for the min and not
> the max but not sure).
Certain conditions may result in SLPC setting the min to a different 
value. We can worry about that in a different patch.
>
> So I'd expect everywhere a consistent set of freq's be used, in run_test()
> and the actual vary_min/max_freq tests and also in the main driver.
Agree.
>
>>>> -	if (intel_guc_slpc_get_max_freq(slpc, &slpc_max_freq)) {
>>>> -		pr_err("Could not get SLPC max freq\n");
>>>> -		return -EIO;
>>>> -	}
>>>> -
>>>> -	if (intel_guc_slpc_get_min_freq(slpc, &slpc_min_freq)) {
>>>> -		pr_err("Could not get SLPC min freq\n");
>>>> -		return -EIO;
>>> Why do we need these two function calls? Can't we just use slpc->rp0_freq
>>> and slpc->min_freq as we are doing in the vary_min/max_freq() functions
>>> above?
>> Same as above.
>>> Also, as mentioned below I think here we should just do:
>>>
>>>           slpc_set_max_freq(slpc, slpc->rp0_freq);
>>>           slpc_set_min_freq(slpc, slpc->min_freq);
>>>
>>> to restore freq to a known state before starting the test (just in case a
>>> previous test changed the values).
>> Any test that changes the frequencies should restore them as well.
> I was saying restore the freq's *before* starting the tests as well to
> start from a known state.

Ok, so 2 things here-

- A test should not alter the state of the system - we need to save the 
slpc_min and slpc_max at the beginning and restore them at the end.

- A test should start from a known state -We can set min/max to RPn/RP0. 
Also, what if RPn = RP0? This could be a bug in the fuse registers, so 
we need to flag an error.

>
>>>> -	}
>>>> -
>>>> -	if (slpc_min_freq == slpc_max_freq) {
>>>> -		pr_err("Min/Max are fused to the same value\n");
>>>> -		return -EINVAL;
>>> What if they are actually equal? I think basically the max/min freq test
>>> loops will just not be entered (so effectively the tests will just
>>> skip). The granted freq test will be fine. So I think we can just delete
>>> this if statement?
>>>
>>> (It is showing deleted above in the patch but is in the new code somewhere
>>> too).
>> Actually, we should set it to min/rp0 if this is the case. That change will
>> be in a separate patch. This is needed for certain cases.
> I don't see why it's needed as I said, can you explain? Set what to min/rp0?
Above discussion should resolve this as well.
>
>>>> -	}
>>>> -
>>>> -	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;
>>>> -
>>>> -		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;
>>>>
>>>> -		/* 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;
>>>> -			}
>>>> +		req_freq = intel_rps_read_punit_req_frequency(rps);
>>>>
>>>> -			i915_request_add(rq);
>>>> +		/* 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;
>>> Probably a nit but check can be (so should we be checking both high and low
>>> limits?):
>>> 		if (req_freq > (max_freq + FREQUENCY_REQ_UNIT) ||
>>> 		    req_freq < (slpc_min_freq - FREQUENCY_REQ_UNIT))
>>>
>>> Though if we do this we'd need to change the pr_err() or have two separate
>>> if statements. Not sure if it's worth it but thought I'll mention it.
>> We are looking to validate it does not cross the upper limit.
> OK.
>
>>>> +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;
>>>> +	u32 slpc_min_freq, slpc_max_freq;
>>>> +	int err = 0;
>>>>
>>>> -			act_freq =  intel_rps_read_actual_frequency(rps);
>>>> -			if (act_freq > max_act_freq)
>>>> -				max_act_freq = act_freq;
>>>> +	slpc_min_freq = slpc->min_freq;
>>>> +	slpc_max_freq = slpc->rp0_freq;
>>>>
>>>> -			igt_spinner_end(&spin);
>>>> -			st_engine_heartbeat_enable(engine);
>>>> -		}
>>>> +	/* 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;
>>>>
>>>> -		pr_info("Max actual frequency for %s was %d\n",
>>>> -			engine->name, max_act_freq);
>>>> +		req_freq = intel_rps_read_punit_req_frequency(rps);
>>>>
>>>> -		/* Actual frequency should rise above min */
>>>> -		if (max_act_freq == slpc_min_freq) {
>>> Nit again. This check is somewhere in the new code but I think a better
>>> check is
>>>
>>> 		if (max_act_freq <= slpc_min_freq)
>>>
>>> just in case the act freq for whatever reason falls below
>>> slpc_min_freq. Even if we know this is impossible (bugs make the impossible
>>> possible).
>> sure.
>>>> -			pr_err("Actual freq did not rise above min\n");
>>>> +		/* 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;
>>> Again nit as above, but check can be:
>>> 		if (req_freq < (min_freq - FREQUENCY_REQ_UNIT) ||
>>> 		    req_freq > (slpc_max_freq + FREQUENCY_REQ_UNIT)) {
>> It can be higher, we want to validate lower range.
> OK.
>
>>>> 		}
>>>>
>>>> +		act_freq =  intel_rps_read_actual_frequency(rps);
>>>> +		if (act_freq > *max_act_freq)
>>>> +			*max_act_freq = act_freq;
>>>> +
>>>> 		if (err)
>>>> 			break;
>>>> 	}
>>>>
>>>> -	/* Restore min/max frequencies */
>>>> -	slpc_set_max_freq(slpc, slpc_max_freq);
>>>> -	slpc_set_min_freq(slpc, slpc_min_freq);
>>>> +	return err;
>>>> +}
>>>>
>>>> -	if (igt_flush_test(gt->i915))
>>>> -		err = -EIO;
>>>> +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;
>>>>
>>>> -	intel_gt_pm_put(gt);
>>>> -	igt_spinner_fini(&spin);
>>>> -	intel_gt_pm_wait_for_idle(gt);
>>>> +	err = slpc_set_min_freq(slpc, slpc->rp0_freq);
>>>> +	if (err)
>>>> +		return err;
>>>> +
>>>> +	*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);
>>>> +
>>>> +		/* 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;
>>> Looks incorrect, probably something like:
>>> 		if (!(perf_limit_reasons & GT0_PERF_LIMIT_REASONS_MASK))
>> Hmm, good catch. We should flag error iff there is no throttling and act
>> freq does not go to max.
>>>> +		}
>>>> +	}
>>>>
>>>> 	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,69 +181,56 @@ 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;
>>>> +
>>>> +		case VARY_MAX:
>>>> +			err = vary_max_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 MAX_GRANTED:
>>>> +			/* 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;
>>> I think it's preferable to move this media engine code out of the main loop
>>> into max_granted_freq() function if possible (maybe by faking max_act_freq
>>> if needed)?
>> All the engine related info is here. I will need to pass it to the
>> max_granted_freq() function.  Also, faking the act_freq probably
>> overkill. I can add a fixme here instead to update when we have a
>> reliable way to obtain media RP0 instead.
> OK let's leave as is, no need for FIXME, just leave the comment as before.
ok.
>
>>>> 			}
>>>>
>>>> -			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",
>>>> @@ -277,31 +242,59 @@ static int live_slpc_clamp_max(void *arg)
>>>> 			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);
>>> As mentioned above maybe we should restore at the beginning of the test too
>>> (before the for_each_engine() loop) to start from a known state?
>>>
>>> Anyway here maybe get rid of the variables and:
>> This is restoring whatever frequencies SLPC was running with
>> initially. Regarding resetting the frequencies to min for every engine loop
>> iteration, we are already iterating from min->max inside the for loop, so
>> will be duplication.
> I didn't say reset frequencies to min for every engine loop iteration, I
> said "before the for_each_engine() loop". Same as above: "I was saying
> restore the freq's *before* starting the tests as well to start from a
> known state".

Every test will work off a known state - from RPn->RP0

Thanks,

Vinay.

>
> Thanks.
> --
> Ashutosh

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

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

On Mon, 27 Jun 2022 16:03:46 -0700, Vinay Belgaumkar wrote:
>
>		/* 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;
>		}

Maybe to be clear, we should combine these two pr_err's into a single
pr_err.

In any case this patch is:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

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

* [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
@ 2022-06-27 23:03 Vinay Belgaumkar
  2022-06-28  4:48 ` Dixit, Ashutosh
  0 siblings, 1 reply; 16+ messages in thread
From: Vinay Belgaumkar @ 2022-06-27 23:03 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.
v4: Checkpatch (strict) fixes, remove FIXME and other comments (Ashutosh)

Fixes commit 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 | 323 ++++++++++++------------
 1 file changed, 155 insertions(+), 168 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
index b768cea5943d..ac29691e0b1a 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,114 @@ 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;
@@ -194,7 +166,7 @@ static int live_slpc_clamp_max(void *arg)
 		return -EIO;
 	}
 
-	if (slpc_min_freq == slpc_max_freq) {
+	if (slpc->min_freq == slpc->rp0_freq) {
 		pr_err("Min/Max are fused to the same value\n");
 		return -EINVAL;
 	}
@@ -203,93 +175,82 @@ 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);
 
-			delay_for_h2g();
+		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;
+		}
 
-			/* Verify that SWREQ indeed was set to specific value */
-			req_freq = intel_rps_read_punit_req_frequency(rps);
+		switch (test_type) {
+		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:
+			/* 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);
@@ -297,11 +258,37 @@ static int live_slpc_clamp_max(void *arg)
 	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] 16+ messages in thread

* Re: [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
  2022-06-25  3:59 ` Dixit, Ashutosh
  2022-06-27 22:52   ` Belgaumkar, Vinay
@ 2022-06-27 23:02   ` Belgaumkar, Vinay
  1 sibling, 0 replies; 16+ 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] 16+ messages in thread

* Re: [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
  2022-06-25  3:59 ` Dixit, Ashutosh
@ 2022-06-27 22:52   ` Belgaumkar, Vinay
  2022-06-27 23:02   ` Belgaumkar, Vinay
  1 sibling, 0 replies; 16+ 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] 16+ messages in thread

* Re: [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
  2022-06-23 23:33 Vinay Belgaumkar
@ 2022-06-25  3:59 ` Dixit, Ashutosh
  2022-06-27 22:52   ` Belgaumkar, Vinay
  2022-06-27 23:02   ` Belgaumkar, Vinay
  0 siblings, 2 replies; 16+ 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] 16+ messages in thread

* [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
@ 2022-06-23 23:33 Vinay Belgaumkar
  2022-06-25  3:59 ` Dixit, Ashutosh
  0 siblings, 1 reply; 16+ 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] 16+ messages in thread

* [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest
@ 2022-06-10 21:18 Vinay Belgaumkar
  0 siblings, 0 replies; 16+ messages in thread
From: Vinay Belgaumkar @ 2022-06-10 21:18 UTC (permalink / raw)
  To: intel-gfx, dri-devel; +Cc: 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.

Fixes 8ee2c227822e ("drm/i915/guc/slpc: Add SLPC selftest")
Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_slpc.c | 322 ++++++++++++------------
 1 file changed, 157 insertions(+), 165 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
index b768cea5943d..c361a63530c3 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,119 @@ static int slpc_set_max_freq(struct intel_guc_slpc *slpc, u32 freq)
 	return ret;
 }
 
-static int live_slpc_clamp_min(void *arg)
+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 step, max_freq, req_freq;
+	u32 act_freq;
 	u32 slpc_min_freq, slpc_max_freq;
 	int err = 0;
 
-	if (!intel_uc_uses_guc_slpc(&gt->uc))
-		return 0;
+	slpc_min_freq = slpc->min_freq;
+	slpc_max_freq = slpc->rp0_freq;
 
-	if (igt_spinner_init(&spin, gt))
-		return -ENOMEM;
+	/* 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;
 
-	if (intel_guc_slpc_get_max_freq(slpc, &slpc_max_freq)) {
-		pr_err("Could not get SLPC max freq\n");
-		return -EIO;
-	}
+		req_freq = intel_rps_read_punit_req_frequency(rps);
 
-	if (intel_guc_slpc_get_min_freq(slpc, &slpc_min_freq)) {
-		pr_err("Could not get SLPC min 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 (slpc_min_freq == slpc_max_freq) {
-		pr_err("Min/Max are fused to the same value\n");
-		return -EINVAL;
+		act_freq =  intel_rps_read_actual_frequency(rps);
+		if (act_freq > *max_act_freq)
+			*max_act_freq = act_freq;
+
+		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;
+}
+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;
+	u32 slpc_min_freq, slpc_max_freq;
+	int err = 0;
 
-		if (!intel_engine_can_store_dword(engine))
-			continue;
+	slpc_min_freq = slpc->min_freq;
+	slpc_max_freq = slpc->rp0_freq;
 
-		/* 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_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;
 
-			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;
+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;
 		}
-
-		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,69 +180,56 @@ 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;
+
+		case VARY_MAX:
+			err = vary_max_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 MAX_GRANTED:
+			/* 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",
@@ -277,31 +241,59 @@ static int live_slpc_clamp_max(void *arg)
 			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] 16+ messages in thread

end of thread, other threads:[~2022-06-28  4:48 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-10 23:47 [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest Vinay Belgaumkar
2022-06-10 23:47 ` [Intel-gfx] " Vinay Belgaumkar
2022-06-13 21:40 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/guc/slpc: Add a new SLPC selftest (rev2) Patchwork
2022-06-13 22:03 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-06-14 23:05 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2022-06-22 20:32 ` [Intel-gfx] [PATCH] drm/i915/guc/slpc: Add a new SLPC selftest Dixit, Ashutosh
2022-06-23 23:21   ` Belgaumkar, Vinay
2022-06-25  3:59     ` Dixit, Ashutosh
2022-06-27 22:52       ` Belgaumkar, Vinay
  -- strict thread matches above, loose matches on Subject: below --
2022-06-27 23:03 Vinay Belgaumkar
2022-06-28  4:48 ` Dixit, Ashutosh
2022-06-23 23:33 Vinay Belgaumkar
2022-06-25  3:59 ` Dixit, Ashutosh
2022-06-27 22:52   ` Belgaumkar, Vinay
2022-06-27 23:02   ` Belgaumkar, Vinay
2022-06-10 21:18 Vinay Belgaumkar

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.