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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->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(>->i915->drm, "GT failed to idle: %d\n", ret); }
out:
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.
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.
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
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... base: f2906aa863381afb0015a9eb7fefad885d4e5a56 config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20220611/202206111316.zDpQcjgJ-lkp@i...) 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/66426324c5bb1a53dd401b7d781c3c... 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(>->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(>->i915->drm, "Failed creating contexts: %ld", PTR_ERR(parent)); 119 return PTR_ERR(parent); 120 } else if (!parent) {
121 drm_debug(>->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(>->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(>->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(>->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
dri-devel@lists.freedesktop.org