diff mbox series

[v2,1/1] drm/i915/guc/slpc: Add selftest for slpc tile-tile interaction

Message ID 20221028095058.3624647-2-riana.tauro@intel.com (mailing list archive)
State New, archived
Headers show
Series Add selftest for slpc tile interaction | expand

Commit Message

Riana Tauro Oct. 28, 2022, 9:50 a.m. UTC 
Run a workload on tiles simultaneously by requesting for RP0 frequency.
Pcode can however limit the frequency being granted due to throttling
reasons. This test fails if there is any throttling

v2: Fix build error

Signed-off-by: Riana Tauro <riana.tauro@intel.com>
---
 drivers/gpu/drm/i915/gt/selftest_slpc.c | 63 +++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

Comments

Tvrtko Ursulin Oct. 28, 2022, 10:23 a.m. UTC | #1 
Hi,

I peeked inside from curiosity and was pleasantly surprise to see 
kthread_work is used! Some comments below.

On 28/10/2022 10:50, Riana Tauro wrote:
> Run a workload on tiles simultaneously by requesting for RP0 frequency.
> Pcode can however limit the frequency being granted due to throttling
> reasons. This test fails if there is any throttling
> 
> v2: Fix build error
> 
> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/selftest_slpc.c | 63 +++++++++++++++++++++++++
>   1 file changed, 63 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
> index 82ec95a299f6..d19486772f5a 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_slpc.c
> @@ -13,6 +13,14 @@ enum test_type {
>   	VARY_MAX,
>   	MAX_GRANTED,
>   	SLPC_POWER,
> +	TILE_INTERACTION,
> +};
> +
> +struct slpc_thread {
> +	struct kthread_worker *worker;
> +	struct kthread_work work;
> +	struct intel_gt *gt;
> +	int result;
>   };
>   
>   static int slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 freq)
> @@ -310,6 +318,7 @@ static int run_test(struct intel_gt *gt, int test_type)
>   			break;
>   
>   		case MAX_GRANTED:
> +		case TILE_INTERACTION:
>   			/* Media engines have a different RP0 */
>   			if (engine->class == VIDEO_DECODE_CLASS ||
>   			    engine->class == VIDEO_ENHANCEMENT_CLASS) {
> @@ -325,6 +334,7 @@ static int run_test(struct intel_gt *gt, int test_type)
>   		case SLPC_POWER:
>   			err = slpc_power(gt, engine);
>   			break;
> +

Noise.

>   		}
>   
>   		if (test_type != SLPC_POWER) {
> @@ -426,6 +436,58 @@ static int live_slpc_power(void *arg)
>   	return ret;
>   }
>   
> +static void slpc_spinner_thread(struct kthread_work *work)
> +{
> +	struct slpc_thread *thread = container_of(work, typeof(*thread), work);
> +
> +	thread->result = run_test(thread->gt, TILE_INTERACTION);
> +}
> +
> +static int live_slpc_tile_interaction(void *arg)
> +{
> +	struct drm_i915_private *i915 = arg;
> +	struct intel_gt *gt;
> +	struct slpc_thread *threads;
> +	int i = 0, ret = 0;
> +
> +	threads = kcalloc(I915_MAX_GT, sizeof(*threads), GFP_KERNEL);
> +	if (!threads)
> +		return -ENOMEM;
> +
> +	for_each_gt(gt, i915, i) {
> +		pr_info("Running on GT: %d\n", gt->info.id);

Not sure logging these makes sense since test runs on all tiles in 
parallel anyway.

> +		threads[i].worker = kthread_create_worker(0, "igt/slpc_parallel:%d", gt->info.id);
> +
> +		if (IS_ERR(threads[i].worker)) {
> +			ret = PTR_ERR(threads[i].worker);
> +			break;
> +		}
> +
> +		threads[i].gt = gt;
> +		threads[i].result = 0;

No need to zero result due kcalloc and runs do not repeat.

> +		kthread_init_work(&threads[i].work, slpc_spinner_thread);
> +		kthread_queue_work(threads[i].worker, &threads[i].work);
> +	}
> +
> +	for_each_gt(gt, i915, i) {
> +		int status;
> +
> +		if (!threads[i].worker)
> +			continue;

Could be ERR_PTR by the look of it so it would crash below. Either gate 
on threads[i].gt or use IS_ERR_OR_NULL.

Regards,

Tvrtko

> +
> +		kthread_flush_work(&threads[i].work);
> +		status = READ_ONCE(threads[i].result);
> +		if (status && !ret) {
> +			pr_err("%s GT %d failed ", __func__, gt->info.id);
> +			ret = status;
> +		}
> +		kthread_destroy_worker(threads[i].worker);
> +	}
> +
> +	kfree(threads);
> +	return ret;
> +}
> +
>   int intel_slpc_live_selftests(struct drm_i915_private *i915)
>   {
>   	static const struct i915_subtest tests[] = {
> @@ -433,6 +495,7 @@ int intel_slpc_live_selftests(struct drm_i915_private *i915)
>   		SUBTEST(live_slpc_vary_min),
>   		SUBTEST(live_slpc_max_granted),
>   		SUBTEST(live_slpc_power),
> +		SUBTEST(live_slpc_tile_interaction),
>   	};
>   
>   	struct intel_gt *gt;
Riana Tauro Oct. 31, 2022, 4:38 a.m. UTC | #2 
On 10/28/2022 3:53 PM, Tvrtko Ursulin wrote:
> 
> Hi,
> 
> I peeked inside from curiosity and was pleasantly surprise to see 
> kthread_work is used! Some comments below.
> 
> On 28/10/2022 10:50, Riana Tauro wrote:
>> Run a workload on tiles simultaneously by requesting for RP0 frequency.
>> Pcode can however limit the frequency being granted due to throttling
>> reasons. This test fails if there is any throttling
>>
>> v2: Fix build error
>>
>> Signed-off-by: Riana Tauro <riana.tauro@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/selftest_slpc.c | 63 +++++++++++++++++++++++++
>>   1 file changed, 63 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c 
>> b/drivers/gpu/drm/i915/gt/selftest_slpc.c
>> index 82ec95a299f6..d19486772f5a 100644
>> --- a/drivers/gpu/drm/i915/gt/selftest_slpc.c
>> +++ b/drivers/gpu/drm/i915/gt/selftest_slpc.c
>> @@ -13,6 +13,14 @@ enum test_type {
>>       VARY_MAX,
>>       MAX_GRANTED,
>>       SLPC_POWER,
>> +    TILE_INTERACTION,
>> +};
>> +
>> +struct slpc_thread {
>> +    struct kthread_worker *worker;
>> +    struct kthread_work work;
>> +    struct intel_gt *gt;
>> +    int result;
>>   };
>>   static int slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 freq)
>> @@ -310,6 +318,7 @@ static int run_test(struct intel_gt *gt, int 
>> test_type)
>>               break;
>>           case MAX_GRANTED:
>> +        case TILE_INTERACTION:
>>               /* Media engines have a different RP0 */
>>               if (engine->class == VIDEO_DECODE_CLASS ||
>>                   engine->class == VIDEO_ENHANCEMENT_CLASS) {
>> @@ -325,6 +334,7 @@ static int run_test(struct intel_gt *gt, int 
>> test_type)
>>           case SLPC_POWER:
>>               err = slpc_power(gt, engine);
>>               break;
>> +
> 
> Noise.
> 
>>           }
>>           if (test_type != SLPC_POWER) {
>> @@ -426,6 +436,58 @@ static int live_slpc_power(void *arg)
>>       return ret;
>>   }
>> +static void slpc_spinner_thread(struct kthread_work *work)
>> +{
>> +    struct slpc_thread *thread = container_of(work, typeof(*thread), 
>> work);
>> +
>> +    thread->result = run_test(thread->gt, TILE_INTERACTION);
>> +}
>> +
>> +static int live_slpc_tile_interaction(void *arg)
>> +{
>> +    struct drm_i915_private *i915 = arg;
>> +    struct intel_gt *gt;
>> +    struct slpc_thread *threads;
>> +    int i = 0, ret = 0;
>> +
>> +    threads = kcalloc(I915_MAX_GT, sizeof(*threads), GFP_KERNEL);
>> +    if (!threads)
>> +        return -ENOMEM;
>> +
>> +    for_each_gt(gt, i915, i) {
>> +        pr_info("Running on GT: %d\n", gt->info.id);
> 
> Not sure logging these makes sense since test runs on all tiles in 
> parallel anyway.
> 
>> +        threads[i].worker = kthread_create_worker(0, 
>> "igt/slpc_parallel:%d", gt->info.id);
>> +
>> +        if (IS_ERR(threads[i].worker)) {
>> +            ret = PTR_ERR(threads[i].worker);
>> +            break;
>> +        }
>> +
>> +        threads[i].gt = gt;
>> +        threads[i].result = 0;
> 
> No need to zero result due kcalloc and runs do not repeat.
> 
>> +        kthread_init_work(&threads[i].work, slpc_spinner_thread);
>> +        kthread_queue_work(threads[i].worker, &threads[i].work);
>> +    }
>> +
>> +    for_each_gt(gt, i915, i) {
>> +        int status;
>> +
>> +        if (!threads[i].worker)
>> +            continue;
> 
> Could be ERR_PTR by the look of it so it would crash below. Either gate 
> on threads[i].gt or use IS_ERR_OR_NULL.
> 
> Regards,
> 
> Tvrtko

Thank you Tvrtko for the review comments.
Will fix these.

Thanks
Riana

> 
>> +
>> +        kthread_flush_work(&threads[i].work);
>> +        status = READ_ONCE(threads[i].result);
>> +        if (status && !ret) {
>> +            pr_err("%s GT %d failed ", __func__, gt->info.id);
>> +            ret = status;
>> +        }
>> +        kthread_destroy_worker(threads[i].worker);
>> +    }
>> +
>> +    kfree(threads);
>> +    return ret;
>> +}
>> +
>>   int intel_slpc_live_selftests(struct drm_i915_private *i915)
>>   {
>>       static const struct i915_subtest tests[] = {
>> @@ -433,6 +495,7 @@ int intel_slpc_live_selftests(struct 
>> drm_i915_private *i915)
>>           SUBTEST(live_slpc_vary_min),
>>           SUBTEST(live_slpc_max_granted),
>>           SUBTEST(live_slpc_power),
>> +        SUBTEST(live_slpc_tile_interaction),
>>       };
>>       struct intel_gt *gt;
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/selftest_slpc.c b/drivers/gpu/drm/i915/gt/selftest_slpc.c
index 82ec95a299f6..d19486772f5a 100644
--- a/drivers/gpu/drm/i915/gt/selftest_slpc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_slpc.c
@@ -13,6 +13,14 @@  enum test_type {
 	VARY_MAX,
 	MAX_GRANTED,
 	SLPC_POWER,
+	TILE_INTERACTION,
+};
+
+struct slpc_thread {
+	struct kthread_worker *worker;
+	struct kthread_work work;
+	struct intel_gt *gt;
+	int result;
 };
 
 static int slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 freq)
@@ -310,6 +318,7 @@  static int run_test(struct intel_gt *gt, int test_type)
 			break;
 
 		case MAX_GRANTED:
+		case TILE_INTERACTION:
 			/* Media engines have a different RP0 */
 			if (engine->class == VIDEO_DECODE_CLASS ||
 			    engine->class == VIDEO_ENHANCEMENT_CLASS) {
@@ -325,6 +334,7 @@  static int run_test(struct intel_gt *gt, int test_type)
 		case SLPC_POWER:
 			err = slpc_power(gt, engine);
 			break;
+
 		}
 
 		if (test_type != SLPC_POWER) {
@@ -426,6 +436,58 @@  static int live_slpc_power(void *arg)
 	return ret;
 }
 
+static void slpc_spinner_thread(struct kthread_work *work)
+{
+	struct slpc_thread *thread = container_of(work, typeof(*thread), work);
+
+	thread->result = run_test(thread->gt, TILE_INTERACTION);
+}
+
+static int live_slpc_tile_interaction(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	struct intel_gt *gt;
+	struct slpc_thread *threads;
+	int i = 0, ret = 0;
+
+	threads = kcalloc(I915_MAX_GT, sizeof(*threads), GFP_KERNEL);
+	if (!threads)
+		return -ENOMEM;
+
+	for_each_gt(gt, i915, i) {
+		pr_info("Running on GT: %d\n", gt->info.id);
+		threads[i].worker = kthread_create_worker(0, "igt/slpc_parallel:%d", gt->info.id);
+
+		if (IS_ERR(threads[i].worker)) {
+			ret = PTR_ERR(threads[i].worker);
+			break;
+		}
+
+		threads[i].gt = gt;
+		threads[i].result = 0;
+		kthread_init_work(&threads[i].work, slpc_spinner_thread);
+		kthread_queue_work(threads[i].worker, &threads[i].work);
+	}
+
+	for_each_gt(gt, i915, i) {
+		int status;
+
+		if (!threads[i].worker)
+			continue;
+
+		kthread_flush_work(&threads[i].work);
+		status = READ_ONCE(threads[i].result);
+		if (status && !ret) {
+			pr_err("%s GT %d failed ", __func__, gt->info.id);
+			ret = status;
+		}
+		kthread_destroy_worker(threads[i].worker);
+	}
+
+	kfree(threads);
+	return ret;
+}
+
 int intel_slpc_live_selftests(struct drm_i915_private *i915)
 {
 	static const struct i915_subtest tests[] = {
@@ -433,6 +495,7 @@  int intel_slpc_live_selftests(struct drm_i915_private *i915)
 		SUBTEST(live_slpc_vary_min),
 		SUBTEST(live_slpc_max_granted),
 		SUBTEST(live_slpc_power),
+		SUBTEST(live_slpc_tile_interaction),
 	};
 
 	struct intel_gt *gt;