[PATCH] drm/i915/guc: Use drm_err instead of pr_err

[PATCH] drm/i915/guc: Use drm_err instead of pr_err

[John.C.Harrison]

From: John Harrison <John.C.Harrison@Intel.com>

Don't use pr_err in places where we have access to a struct_drm.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c      | 10 ++---
 drivers/gpu/drm/i915/gt/uc/selftest_guc.c     | 38 +++++++++----------
 .../drm/i915/gt/uc/selftest_guc_multi_lrc.c   | 10 ++---
 3 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 9361532726d6c..6d1b82da91bd9 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -186,11 +186,11 @@ __uc_fw_auto_select(struct drm_i915_private *i915, struct intel_uc_fw *uc_fw)
 			    fw_blobs[i].rev < fw_blobs[i - 1].rev)
 				continue;
 
-			pr_err("invalid FW blob order: %s r%u comes before %s r%u\n",
-			       intel_platform_name(fw_blobs[i - 1].p),
-			       fw_blobs[i - 1].rev,
-			       intel_platform_name(fw_blobs[i].p),
-			       fw_blobs[i].rev);
+			drm_err(&i915->drm, "invalid FW blob order: %s r%u comes before %s r%u\n",
+				intel_platform_name(fw_blobs[i - 1].p),
+				fw_blobs[i - 1].rev,
+				intel_platform_name(fw_blobs[i].p),
+				fw_blobs[i].rev);
 
 			uc_fw->path = NULL;
 		}
diff --git a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
index 1df71d0796aec..a720f0388e8ce 100644
--- a/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/selftest_guc.c
@@ -62,7 +62,7 @@ static int intel_guc_scrub_ctbs(void *arg)
 		ce = intel_context_create(engine);
 		if (IS_ERR(ce)) {
 			ret = PTR_ERR(ce);
-			pr_err("Failed to create context, %d: %d\n", i, ret);
+			drm_err(&gt->i915->drm, "Failed to create context, %d: %d\n", i, ret);
 			goto err;
 		}
 
@@ -83,7 +83,7 @@ static int intel_guc_scrub_ctbs(void *arg)
 
 		if (IS_ERR(rq)) {
 			ret = PTR_ERR(rq);
-			pr_err("Failed to create request, %d: %d\n", i, ret);
+			drm_err(&gt->i915->drm, "Failed to create request, %d: %d\n", i, ret);
 			goto err;
 		}
 
@@ -93,7 +93,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) {
-			pr_err("Last request failed to complete: %d\n", ret);
+			drm_err(&gt->i915->drm, "Last request failed to complete: %d\n", ret);
 			goto err;
 		}
 		i915_request_put(last[i]);
@@ -110,7 +110,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) {
-		pr_err("GT failed to idle: %d\n", ret);
+		drm_err(&gt->i915->drm, "GT failed to idle: %d\n", ret);
 		goto err;
 	}
 
@@ -150,7 +150,7 @@ static int intel_guc_steal_guc_ids(void *arg)
 
 	ce = kcalloc(GUC_MAX_CONTEXT_ID, sizeof(*ce), GFP_KERNEL);
 	if (!ce) {
-		pr_err("Context array allocation failed\n");
+		drm_err(&gt->i915->drm, "Context array allocation failed\n");
 		return -ENOMEM;
 	}
 
@@ -164,24 +164,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;
-		pr_err("Failed to create context: %d\n", ret);
+		drm_err(&gt->i915->drm, "Failed to create context: %d\n", ret);
 		goto err_wakeref;
 	}
 	ret = igt_spinner_init(&spin, engine->gt);
 	if (ret) {
-		pr_err("Failed to create spinner: %d\n", ret);
+		drm_err(&gt->i915->drm, "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);
-		pr_err("Failed to create spinner request: %d\n", ret);
+		drm_err(&gt->i915->drm, "Failed to create spinner request: %d\n", ret);
 		goto err_contexts;
 	}
 	ret = request_add_spin(spin_rq, &spin);
 	if (ret) {
-		pr_err("Failed to add Spinner request: %d\n", ret);
+		drm_err(&gt->i915->drm, "Failed to add Spinner request: %d\n", ret);
 		goto err_spin_rq;
 	}
 
@@ -191,7 +191,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;
-			pr_err("Failed to create context: %d\n", ret);
+			drm_err(&gt->i915->drm, "Failed to create context: %d\n", ret);
 			goto err_spin_rq;
 		}
 
@@ -200,8 +200,8 @@ static int intel_guc_steal_guc_ids(void *arg)
 			ret = PTR_ERR(rq);
 			rq = NULL;
 			if (ret != -EAGAIN) {
-				pr_err("Failed to create request, %d: %d\n",
-				       context_index, ret);
+				drm_err(&gt->i915->drm, "Failed to create request, %d: %d\n",
+					context_index, ret);
 				goto err_spin_rq;
 			}
 		} else {
@@ -215,7 +215,7 @@ static int intel_guc_steal_guc_ids(void *arg)
 	igt_spinner_end(&spin);
 	ret = intel_selftest_wait_for_rq(spin_rq);
 	if (ret) {
-		pr_err("Spin request failed to complete: %d\n", ret);
+		drm_err(&gt->i915->drm, "Spin request failed to complete: %d\n", ret);
 		i915_request_put(last);
 		goto err_spin_rq;
 	}
@@ -227,7 +227,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) {
-		pr_err("Last request failed to complete: %d\n", ret);
+		drm_err(&gt->i915->drm, "Last request failed to complete: %d\n", ret);
 		goto err_spin_rq;
 	}
 
@@ -235,7 +235,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);
-		pr_err("Failed to steal guc_id, %d: %d\n", context_index, ret);
+		drm_err(&gt->i915->drm, "Failed to steal guc_id, %d: %d\n", context_index, ret);
 		goto err_spin_rq;
 	}
 
@@ -243,21 +243,21 @@ static int intel_guc_steal_guc_ids(void *arg)
 	ret = i915_request_wait(rq, 0, HZ);
 	i915_request_put(rq);
 	if (ret < 0) {
-		pr_err("Request with stolen guc_id failed to complete: %d\n",
-		       ret);
+		drm_err(&gt->i915->drm, "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) {
-		pr_err("GT failed to idle: %d\n", ret);
+		drm_err(&gt->i915->drm, "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) {
-		pr_err("No guc_id was stolen");
+		drm_err(&gt->i915->drm, "No guc_id was stolen");
 		ret = -EINVAL;
 	} else {
 		ret = 0;
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 812220a43df81..b2e1d50a49d8e 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
@@ -115,30 +115,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)) {
-		pr_err("Failed creating contexts: %ld", PTR_ERR(parent));
+		drm_err(&gt->i915->drm, "Failed creating contexts: %ld", PTR_ERR(parent));
 		return PTR_ERR(parent);
 	} else if (!parent) {
-		pr_debug("Not enough engines in class: %d", class);
+		drm_debug(&gt->i915->drm, "Not enough engines in class: %d", class);
 		return 0;
 	}
 
 	rq = multi_lrc_nop_request(parent);
 	if (IS_ERR(rq)) {
 		ret = PTR_ERR(rq);
-		pr_err("Failed creating requests: %d", ret);
+		drm_err(&gt->i915->drm, "Failed creating requests: %d", ret);
 		goto out;
 	}
 
 	ret = intel_selftest_wait_for_rq(rq);
 	if (ret)
-		pr_err("Failed waiting on request: %d", ret);
+		drm_err(&gt->i915->drm, "Failed waiting on request: %d", ret);
 
 	i915_request_put(rq);
 
 	if (ret >= 0) {
 		ret = intel_gt_wait_for_idle(gt, HZ * 5);
 		if (ret < 0)
-			pr_err("GT failed to idle: %d\n", ret);
+			drm_err(&gt->i915->drm, "GT failed to idle: %d\n", ret);
 	}
 
 out:
-- 
2.36.0

Re: [PATCH] drm/i915/guc: Use drm_err instead of pr_err

[Dixit, Ashutosh]

On Tue, 07 Jun 2022 14:51:03 -0700, John.C.Harrison@Intel.com wrote:
>
> From: John Harrison <John.C.Harrison@Intel.com>
>
> Don't use pr_err in places where we have access to a struct_drm.

Seem to be many more pr_err's in selftests. Is there a reason why drm_err's
cannot be used in selftests (especially those using an i915 device)?
Thanks.

Re: [PATCH] drm/i915/guc: Use drm_err instead of pr_err

[John Harrison]

On 6/7/2022 15:07, Dixit, Ashutosh wrote:
> On Tue, 07 Jun 2022 14:51:03 -0700, John.C.Harrison@Intel.com wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> Don't use pr_err in places where we have access to a struct_drm.
> Seem to be many more pr_err's in selftests. Is there a reason why drm_err's
> cannot be used in selftests (especially those using an i915 device)?
> Thanks.
I figured I'd start small and just do the gt/uc ones to being with as 
those are the ones that affect me.

It sounds like the only reason to use pr_err is in the mock selftests 
where there is no easy access to a DRM structure. For everything else, 
there is no reason that I am aware of.

John.

Re: [PATCH] drm/i915/guc: Use drm_err instead of pr_err

[Dixit, Ashutosh]

On Tue, 07 Jun 2022 15:23:17 -0700, John Harrison wrote:
>
> On 6/7/2022 15:07, Dixit, Ashutosh wrote:
> > On Tue, 07 Jun 2022 14:51:03 -0700, John.C.Harrison@Intel.com wrote:
> >> From: John Harrison <John.C.Harrison@Intel.com>
> >>
> >> Don't use pr_err in places where we have access to a struct_drm.
> > Seem to be many more pr_err's in selftests. Is there a reason why drm_err's
> > cannot be used in selftests (especially those using an i915 device)?
> > Thanks.
> I figured I'd start small and just do the gt/uc ones to being with as those
> are the ones that affect me.
>
> It sounds like the only reason to use pr_err is in the mock selftests where
> there is no easy access to a DRM structure. For everything else, there is
> no reason that I am aware of.

Fair enough:

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

Re: [PATCH] drm/i915/guc: Use drm_err instead of pr_err

[kernel test robot]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v5.19-rc1]
[also build test ERROR on next-20220610]
[cannot apply to drm-intel/for-linux-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/John-C-Harrison-Intel-com/drm-i915-guc-Use-drm_err-instead-of-pr_err/20220608-055315
base:    f2906aa863381afb0015a9eb7fefad885d4e5a56
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20220611/202206111316.zDpQcjgJ-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-3) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/66426324c5bb1a53dd401b7d781c3c9c58f163d5
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review John-C-Harrison-Intel-com/drm-i915-guc-Use-drm_err-instead-of-pr_err/20220608-055315
        git checkout 66426324c5bb1a53dd401b7d781c3c9c58f163d5
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:4956:
   drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c: In function '__intel_guc_multi_lrc_basic':
>> drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c:121:17: error: implicit declaration of function 'drm_debug'; did you mean 'pr_debug'? [-Werror=implicit-function-declaration]
     121 |                 drm_debug(&gt->i915->drm, "Not enough engines in class: %d", class);
         |                 ^~~~~~~~~
         |                 pr_debug
   cc1: all warnings being treated as errors


vim +121 drivers/gpu/drm/i915/gt/uc/selftest_guc_multi_lrc.c

   109	
   110	static int __intel_guc_multi_lrc_basic(struct intel_gt *gt, unsigned int class)
   111	{
   112		struct intel_context *parent;
   113		struct i915_request *rq;
   114		int ret;
   115	
   116		parent = multi_lrc_create_parent(gt, class, 0);
   117		if (IS_ERR(parent)) {
   118			drm_err(&gt->i915->drm, "Failed creating contexts: %ld", PTR_ERR(parent));
   119			return PTR_ERR(parent);
   120		} else if (!parent) {
 > 121			drm_debug(&gt->i915->drm, "Not enough engines in class: %d", class);
   122			return 0;
   123		}
   124	
   125		rq = multi_lrc_nop_request(parent);
   126		if (IS_ERR(rq)) {
   127			ret = PTR_ERR(rq);
   128			drm_err(&gt->i915->drm, "Failed creating requests: %d", ret);
   129			goto out;
   130		}
   131	
   132		ret = intel_selftest_wait_for_rq(rq);
   133		if (ret)
   134			drm_err(&gt->i915->drm, "Failed waiting on request: %d", ret);
   135	
   136		i915_request_put(rq);
   137	
   138		if (ret >= 0) {
   139			ret = intel_gt_wait_for_idle(gt, HZ * 5);
   140			if (ret < 0)
   141				drm_err(&gt->i915->drm, "GT failed to idle: %d\n", ret);
   142		}
   143	
   144	out:
   145		multi_lrc_context_unpin(parent);
   146		multi_lrc_context_put(parent);
   147		return ret;
   148	}
   149	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp