[Intel-gfx] [PATCH 4/6] drm/i915/guc: More debug print updates - GuC selftests

John Harrison john.c.harrison at intel.com
Fri Feb 3 17:43:42 UTC 2023


On 2/3/2023 01:54, Michal Wajdeczko wrote:
> On 03.02.2023 01:11, John.C.Harrison at Intel.com wrote:
>> From: John Harrison <John.C.Harrison at Intel.com>
>>
>> Update a bunch more debug prints to use the new GT based scheme.
>>
>> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/uc/selftest_guc.c     | 35 ++++++++++---------
>>   .../drm/i915/gt/uc/selftest_guc_hangcheck.c   | 23 ++++++------
>>   .../drm/i915/gt/uc/selftest_guc_multi_lrc.c   | 11 +++---
>>   3 files changed, 36 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
>> index e28518fe8b908..6cc1e9c7a47d6 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
>> @@ -3,6 +3,7 @@
>>    * Copyright �� 2021 Intel Corporation
>>    */
>>   
>> +#include "intel_guc_print.h"
> You've included right headers but then you mostly used gt_xxx() macros
> instead of guc_xxx() ones which IMHO are way more appropriate as we are
> in GuC selftests
Yeah, forgot to go back and change the include.

I started with a blanked update to guc_ instead of drm_ but only then 
realised that the tests do not actually do anything so low level as to 
require access the guc object. Which means having to jump through hoops 
to get to a guc object purely for the purpose of using the guc debug 
functions. Yes, it is a GuC specific selftest but there is no GuC 
specific code in the file and that is really the criteria used for the 
debug print decision.

John.

>
> Michal
>
>>   #include "selftests/igt_spinner.h"
>>   #include "selftests/intel_scheduler_helpers.h"
>>   
>> @@ -65,7 +66,7 @@ static int intel_guc_scrub_ctbs(void *arg)
>>   		ce = intel_context_create(engine);
>>   		if (IS_ERR(ce)) {
>>   			ret = PTR_ERR(ce);
>> -			drm_err(&gt->i915->drm, "Failed to create context, %d: %d\n", i, ret);
>> +			gt_err(gt, "Failed to create context, %d: %d\n", i, ret);
>>   			goto err;
>>   		}
>>   
>> @@ -86,7 +87,7 @@ static int intel_guc_scrub_ctbs(void *arg)
>>   
>>   		if (IS_ERR(rq)) {
>>   			ret = PTR_ERR(rq);
>> -			drm_err(&gt->i915->drm, "Failed to create request, %d: %d\n", i, ret);
>> +			gt_err(gt, "Failed to create request, %d: %d\n", i, ret);
>>   			goto err;
>>   		}
>>   
>> @@ -96,7 +97,7 @@ static int intel_guc_scrub_ctbs(void *arg)
>>   	for (i = 0; i < 3; ++i) {
>>   		ret = i915_request_wait(last[i], 0, HZ);
>>   		if (ret < 0) {
>> -			drm_err(&gt->i915->drm, "Last request failed to complete: %d\n", ret);
>> +			gt_err(gt, "Last request failed to complete: %d\n", ret);
>>   			goto err;
>>   		}
>>   		i915_request_put(last[i]);
>> @@ -113,7 +114,7 @@ static int intel_guc_scrub_ctbs(void *arg)
>>   	/* GT will not idle if G2H are lost */
>>   	ret = intel_gt_wait_for_idle(gt, HZ);
>>   	if (ret < 0) {
>> -		drm_err(&gt->i915->drm, "GT failed to idle: %d\n", ret);
>> +		gt_err(gt, "GT failed to idle: %d\n", ret);
>>   		goto err;
>>   	}
>>   
>> @@ -153,7 +154,7 @@ static int intel_guc_steal_guc_ids(void *arg)
>>   
>>   	ce = kcalloc(GUC_MAX_CONTEXT_ID, sizeof(*ce), GFP_KERNEL);
>>   	if (!ce) {
>> -		drm_err(&gt->i915->drm, "Context array allocation failed\n");
>> +		guc_err(guc, "Context array allocation failed\n");
>>   		return -ENOMEM;
>>   	}
>>   
>> @@ -167,24 +168,24 @@ static int intel_guc_steal_guc_ids(void *arg)
>>   	if (IS_ERR(ce[context_index])) {
>>   		ret = PTR_ERR(ce[context_index]);
>>   		ce[context_index] = NULL;
>> -		drm_err(&gt->i915->drm, "Failed to create context: %d\n", ret);
>> +		guc_err(guc, "Failed to create context: %d\n", ret);
>>   		goto err_wakeref;
>>   	}
>>   	ret = igt_spinner_init(&spin, engine->gt);
>>   	if (ret) {
>> -		drm_err(&gt->i915->drm, "Failed to create spinner: %d\n", ret);
>> +		guc_err(guc, "Failed to create spinner: %d\n", ret);
>>   		goto err_contexts;
>>   	}
>>   	spin_rq = igt_spinner_create_request(&spin, ce[context_index],
>>   					     MI_ARB_CHECK);
>>   	if (IS_ERR(spin_rq)) {
>>   		ret = PTR_ERR(spin_rq);
>> -		drm_err(&gt->i915->drm, "Failed to create spinner request: %d\n", ret);
>> +		guc_err(guc, "Failed to create spinner request: %d\n", ret);
>>   		goto err_contexts;
>>   	}
>>   	ret = request_add_spin(spin_rq, &spin);
>>   	if (ret) {
>> -		drm_err(&gt->i915->drm, "Failed to add Spinner request: %d\n", ret);
>> +		guc_err(guc, "Failed to add Spinner request: %d\n", ret);
>>   		goto err_spin_rq;
>>   	}
>>   
>> @@ -194,7 +195,7 @@ static int intel_guc_steal_guc_ids(void *arg)
>>   		if (IS_ERR(ce[context_index])) {
>>   			ret = PTR_ERR(ce[context_index--]);
>>   			ce[context_index] = NULL;
>> -			drm_err(&gt->i915->drm, "Failed to create context: %d\n", ret);
>> +			guc_err(guc, "Failed to create context: %d\n", ret);
>>   			goto err_spin_rq;
>>   		}
>>   
>> @@ -203,7 +204,7 @@ static int intel_guc_steal_guc_ids(void *arg)
>>   			ret = PTR_ERR(rq);
>>   			rq = NULL;
>>   			if (ret != -EAGAIN) {
>> -				drm_err(&gt->i915->drm, "Failed to create request, %d: %d\n",
>> +				guc_err(guc, "Failed to create request, %d: %d\n",
>>   					context_index, ret);
>>   				goto err_spin_rq;
>>   			}
>> @@ -218,7 +219,7 @@ static int intel_guc_steal_guc_ids(void *arg)
>>   	igt_spinner_end(&spin);
>>   	ret = intel_selftest_wait_for_rq(spin_rq);
>>   	if (ret) {
>> -		drm_err(&gt->i915->drm, "Spin request failed to complete: %d\n", ret);
>> +		guc_err(guc, "Spin request failed to complete: %d\n", ret);
>>   		i915_request_put(last);
>>   		goto err_spin_rq;
>>   	}
>> @@ -230,7 +231,7 @@ static int intel_guc_steal_guc_ids(void *arg)
>>   	ret = i915_request_wait(last, 0, HZ * 30);
>>   	i915_request_put(last);
>>   	if (ret < 0) {
>> -		drm_err(&gt->i915->drm, "Last request failed to complete: %d\n", ret);
>> +		guc_err(guc, "Last request failed to complete: %d\n", ret);
>>   		goto err_spin_rq;
>>   	}
>>   
>> @@ -238,7 +239,7 @@ static int intel_guc_steal_guc_ids(void *arg)
>>   	rq = nop_user_request(ce[context_index], NULL);
>>   	if (IS_ERR(rq)) {
>>   		ret = PTR_ERR(rq);
>> -		drm_err(&gt->i915->drm, "Failed to steal guc_id, %d: %d\n", context_index, ret);
>> +		guc_err(guc, "Failed to steal guc_id, %d: %d\n", context_index, ret);
>>   		goto err_spin_rq;
>>   	}
>>   
>> @@ -246,20 +247,20 @@ static int intel_guc_steal_guc_ids(void *arg)
>>   	ret = i915_request_wait(rq, 0, HZ);
>>   	i915_request_put(rq);
>>   	if (ret < 0) {
>> -		drm_err(&gt->i915->drm, "Request with stolen guc_id failed to complete: %d\n", ret);
>> +		guc_err(guc, "Request with stolen guc_id failed to complete: %d\n", ret);
>>   		goto err_spin_rq;
>>   	}
>>   
>>   	/* Wait for idle */
>>   	ret = intel_gt_wait_for_idle(gt, HZ * 30);
>>   	if (ret < 0) {
>> -		drm_err(&gt->i915->drm, "GT failed to idle: %d\n", ret);
>> +		guc_err(guc, "GT failed to idle: %d\n", ret);
>>   		goto err_spin_rq;
>>   	}
>>   
>>   	/* Verify a guc_id was stolen */
>>   	if (guc->number_guc_id_stolen == number_guc_id_stolen) {
>> -		drm_err(&gt->i915->drm, "No guc_id was stolen");
>> +		guc_err(guc, "No guc_id was stolen");
>>   		ret = -EINVAL;
>>   	} else {
>>   		ret = 0;
>> diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc_hangcheck.c b/drivers/gpu/drm/i915/gt/uc/selftest_guc_hangcheck.c
>> index d91b58f704039..fffe95ac15c4e 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/selftest_guc_hangcheck.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/selftest_guc_hangcheck.c
>> @@ -3,6 +3,7 @@
>>    * Copyright © 2022 Intel Corporation
>>    */
>>   
>> +#include "intel_guc_print.h"
>>   #include "selftests/igt_spinner.h"
>>   #include "selftests/igt_reset.h"
>>   #include "selftests/intel_scheduler_helpers.h"
>> @@ -45,7 +46,7 @@ static int intel_hang_guc(void *arg)
>>   
>>   	ctx = kernel_context(gt->i915, NULL);
>>   	if (IS_ERR(ctx)) {
>> -		drm_err(&gt->i915->drm, "Failed get kernel context: %ld\n", PTR_ERR(ctx));
>> +		gt_err(gt, "Failed get kernel context: %ld\n", PTR_ERR(ctx));
>>   		return PTR_ERR(ctx);
>>   	}
>>   
>> @@ -54,7 +55,7 @@ static int intel_hang_guc(void *arg)
>>   	ce = intel_context_create(engine);
>>   	if (IS_ERR(ce)) {
>>   		ret = PTR_ERR(ce);
>> -		drm_err(&gt->i915->drm, "Failed to create spinner request: %d\n", ret);
>> +		gt_err(gt, "Failed to create spinner request: %d\n", ret);
>>   		goto err;
>>   	}
>>   
>> @@ -63,13 +64,13 @@ static int intel_hang_guc(void *arg)
>>   	old_beat = engine->props.heartbeat_interval_ms;
>>   	ret = intel_engine_set_heartbeat(engine, BEAT_INTERVAL);
>>   	if (ret) {
>> -		drm_err(&gt->i915->drm, "Failed to boost heatbeat interval: %d\n", ret);
>> +		gt_err(gt, "Failed to boost heatbeat interval: %d\n", ret);
>>   		goto err;
>>   	}
>>   
>>   	ret = igt_spinner_init(&spin, engine->gt);
>>   	if (ret) {
>> -		drm_err(&gt->i915->drm, "Failed to create spinner: %d\n", ret);
>> +		gt_err(gt, "Failed to create spinner: %d\n", ret);
>>   		goto err;
>>   	}
>>   
>> @@ -77,28 +78,28 @@ static int intel_hang_guc(void *arg)
>>   	intel_context_put(ce);
>>   	if (IS_ERR(rq)) {
>>   		ret = PTR_ERR(rq);
>> -		drm_err(&gt->i915->drm, "Failed to create spinner request: %d\n", ret);
>> +		gt_err(gt, "Failed to create spinner request: %d\n", ret);
>>   		goto err_spin;
>>   	}
>>   
>>   	ret = request_add_spin(rq, &spin);
>>   	if (ret) {
>>   		i915_request_put(rq);
>> -		drm_err(&gt->i915->drm, "Failed to add Spinner request: %d\n", ret);
>> +		gt_err(gt, "Failed to add Spinner request: %d\n", ret);
>>   		goto err_spin;
>>   	}
>>   
>>   	ret = intel_reset_guc(gt);
>>   	if (ret) {
>>   		i915_request_put(rq);
>> -		drm_err(&gt->i915->drm, "Failed to reset GuC, ret = %d\n", ret);
>> +		gt_err(gt, "Failed to reset GuC, ret = %d\n", ret);
>>   		goto err_spin;
>>   	}
>>   
>>   	guc_status = intel_uncore_read(gt->uncore, GUC_STATUS);
>>   	if (!(guc_status & GS_MIA_IN_RESET)) {
>>   		i915_request_put(rq);
>> -		drm_err(&gt->i915->drm, "GuC failed to reset: status = 0x%08X\n", guc_status);
>> +		gt_err(gt, "Failed to reset GuC: status = 0x%08X\n", guc_status);
>>   		ret = -EIO;
>>   		goto err_spin;
>>   	}
>> @@ -107,12 +108,12 @@ static int intel_hang_guc(void *arg)
>>   	ret = intel_selftest_wait_for_rq(rq);
>>   	i915_request_put(rq);
>>   	if (ret) {
>> -		drm_err(&gt->i915->drm, "Request failed to complete: %d\n", ret);
>> +		gt_err(gt, "Request failed to complete: %d\n", ret);
>>   		goto err_spin;
>>   	}
>>   
>>   	if (i915_reset_count(global) == reset_count) {
>> -		drm_err(&gt->i915->drm, "Failed to record a GPU reset\n");
>> +		gt_err(gt, "Failed to record a GPU reset\n");
>>   		ret = -EINVAL;
>>   		goto err_spin;
>>   	}
>> @@ -132,7 +133,7 @@ static int intel_hang_guc(void *arg)
>>   		ret = intel_selftest_wait_for_rq(rq);
>>   		i915_request_put(rq);
>>   		if (ret) {
>> -			drm_err(&gt->i915->drm, "No-op failed to complete: %d\n", ret);
>> +			gt_err(gt, "No-op failed to complete: %d\n", ret);
>>   			goto err;
>>   		}
>>   	}
>> diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c b/drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c
>> index d17982c36d256..0e64be0918ae5 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c
>> @@ -3,6 +3,7 @@
>>    * Copyright �� 2019 Intel Corporation
>>    */
>>   
>> +#include "intel_guc_print.h"
>>   #include "selftests/igt_spinner.h"
>>   #include "selftests/igt_reset.h"
>>   #include "selftests/intel_scheduler_helpers.h"
>> @@ -115,30 +116,30 @@ static int __intel_guc_multi_lrc_basic(struct intel_gt *gt, unsigned int class)
>>   
>>   	parent = multi_lrc_create_parent(gt, class, 0);
>>   	if (IS_ERR(parent)) {
>> -		drm_err(&gt->i915->drm, "Failed creating contexts: %ld", PTR_ERR(parent));
>> +		gt_err(gt, "Failed creating contexts: %ld\n", PTR_ERR(parent));
>>   		return PTR_ERR(parent);
>>   	} else if (!parent) {
>> -		drm_dbg(&gt->i915->drm, "Not enough engines in class: %d", class);
>> +		gt_dbg(gt, "Not enough engines in class: %d\n", class);
>>   		return 0;
>>   	}
>>   
>>   	rq = multi_lrc_nop_request(parent);
>>   	if (IS_ERR(rq)) {
>>   		ret = PTR_ERR(rq);
>> -		drm_err(&gt->i915->drm, "Failed creating requests: %d", ret);
>> +		gt_err(gt, "Failed creating requests: %d\n", ret);
>>   		goto out;
>>   	}
>>   
>>   	ret = intel_selftest_wait_for_rq(rq);
>>   	if (ret)
>> -		drm_err(&gt->i915->drm, "Failed waiting on request: %d", ret);
>> +		gt_err(gt, "Failed waiting on request: %d\n", ret);
>>   
>>   	i915_request_put(rq);
>>   
>>   	if (ret >= 0) {
>>   		ret = intel_gt_wait_for_idle(gt, HZ * 5);
>>   		if (ret < 0)
>> -			drm_err(&gt->i915->drm, "GT failed to idle: %d\n", ret);
>> +			gt_err(gt, "GT failed to idle: %d\n", ret);
>>   	}
>>   
>>   out:



More information about the dri-devel mailing list