[PATCH 1/3] drm/i915/guc: Remove stale comment for q_fail

[PATCH 1/3] drm/i915/guc: Remove stale comment for q_fail

[Michal Wajdeczko]

This member was dropped long time ago.

Fixes: 774439e1 ("drm/i915/guc: re-optimise i915_guc_client layout")
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 7618b71..8d52a37 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -59,8 +59,6 @@ struct drm_i915_gem_request;
  *                available in the work queue (note, the queue is shared,
  *                not per-engine). It is OK for this to be nonzero, but
  *                it should not be huge!
- *   q_fail: failed to enqueue a work item. This should never happen,
- *           because we check for space beforehand.
  *   b_fail: failed to ring the doorbell. This should never happen, unless
  *           somehow the hardware misbehaves, or maybe if the GuC firmware
  *           crashes? We probably need to reset the GPU to recover.
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915/guc: Remove failed doorbell stat from debugfs

[Michal Wajdeczko]

This stat is almost always zero unless fatal error occurs,
which should be reported by other means anyway.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        | 1 -
 drivers/gpu/drm/i915/i915_guc_submission.c | 2 --
 drivers/gpu/drm/i915/intel_uc.h            | 4 ----
 3 files changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 8abb939..207175e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2482,7 +2482,6 @@ static void i915_guc_client_info(struct seq_file *m,
 		client->wq_size, client->wq_offset, client->wq_tail);
 
 	seq_printf(m, "\tWork queue full: %u\n", client->no_wq_space);
-	seq_printf(m, "\tFailed doorbell: %u\n", client->b_fail);
 	seq_printf(m, "\tLast submission result: %d\n", client->retcode);
 
 	for_each_engine(engine, dev_priv, id) {
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index b3da056..dc84311 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -615,8 +615,6 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 
 	client->submissions[engine_id] += 1;
 	client->retcode = b_ret;
-	if (b_ret)
-		client->b_fail += 1;
 
 	guc->submissions[engine_id] += 1;
 	guc->last_seqno[engine_id] = rq->global_seqno;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 8d52a37..398a491 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -59,9 +59,6 @@ struct drm_i915_gem_request;
  *                available in the work queue (note, the queue is shared,
  *                not per-engine). It is OK for this to be nonzero, but
  *                it should not be huge!
- *   b_fail: failed to ring the doorbell. This should never happen, unless
- *           somehow the hardware misbehaves, or maybe if the GuC firmware
- *           crashes? We probably need to reset the GPU to recover.
  *   retcode: errno from last guc_submit()
  */
 struct i915_guc_client {
@@ -85,7 +82,6 @@ struct i915_guc_client {
 	uint32_t wq_tail;
 	uint32_t wq_rsvd;
 	uint32_t no_wq_space;
-	uint32_t b_fail;
 	int retcode;
 
 	/* Per-engine counts of GuC submissions */
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915/guc: Remove last submission result from debugfs

[Michal Wajdeczko]

Debugfs does not seems to be a right place to display transient data.
If we want to capture errors, we should log them.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        | 1 -
 drivers/gpu/drm/i915/i915_guc_submission.c | 1 -
 drivers/gpu/drm/i915/intel_uc.h            | 2 --
 3 files changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 207175e..6bdc903 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2482,7 +2482,6 @@ static void i915_guc_client_info(struct seq_file *m,
 		client->wq_size, client->wq_offset, client->wq_tail);
 
 	seq_printf(m, "\tWork queue full: %u\n", client->no_wq_space);
-	seq_printf(m, "\tLast submission result: %d\n", client->retcode);
 
 	for_each_engine(engine, dev_priv, id) {
 		u64 submissions = client->submissions[id];
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index dc84311..ee32fa9 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -614,7 +614,6 @@ static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	b_ret = guc_ring_doorbell(client);
 
 	client->submissions[engine_id] += 1;
-	client->retcode = b_ret;
 
 	guc->submissions[engine_id] += 1;
 	guc->last_seqno[engine_id] = rq->global_seqno;
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 398a491..33c8f77 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -59,7 +59,6 @@ struct drm_i915_gem_request;
  *                available in the work queue (note, the queue is shared,
  *                not per-engine). It is OK for this to be nonzero, but
  *                it should not be huge!
- *   retcode: errno from last guc_submit()
  */
 struct i915_guc_client {
 	struct i915_vma *vma;
@@ -82,7 +81,6 @@ struct i915_guc_client {
 	uint32_t wq_tail;
 	uint32_t wq_rsvd;
 	uint32_t no_wq_space;
-	int retcode;
 
 	/* Per-engine counts of GuC submissions */
 	uint64_t submissions[I915_NUM_ENGINES];
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: warning for series starting with [1/3] drm/i915/guc: Remove stale comment for q_fail

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915/guc: Remove stale comment for q_fail
URL   : https://patchwork.freedesktop.org/series/24622/
State : warning

== Summary ==

Series 24622v1 Series without cover letter
https://patchwork.freedesktop.org/api/1.0/series/24622/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (fi-skl-6700hq)

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:449s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:434s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:588s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:513s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:496s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:491s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:414s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:407s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:420s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:489s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:460s
fi-kbl-7500u     total:278  pass:255  dwarn:5   dfail:0   fail:0   skip:18  time:465s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:462s
fi-skl-6700hq    total:278  pass:260  dwarn:1   dfail:0   fail:0   skip:17  time:569s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:466s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:499s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:440s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:532s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:405s

ab08cb2750e769d074b2f147c8298ccd0cd08340 drm-tip: 2017y-05m-18d-15h-36m-17s UTC integration manifest
e106859 drm/i915/guc: Remove last submission result from debugfs
e75fc1c drm/i915/guc: Remove failed doorbell stat from debugfs
19d608b drm/i915/guc: Remove stale comment for q_fail

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4739/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: warning for series starting with [1/3] drm/i915/guc: Remove stale comment for q_fail

[Michal Wajdeczko]

On Thu, May 18, 2017 at 04:38:01PM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/3] drm/i915/guc: Remove stale comment for q_fail
> URL   : https://patchwork.freedesktop.org/series/24622/
> State : warning
> 
> == Summary ==
> 
> Series 24622v1 Series without cover letter
> https://patchwork.freedesktop.org/api/1.0/series/24622/revisions/1/mbox/
> 
> Test gem_exec_flush:
>         Subgroup basic-batch-kernel-default-uc:
>                 fail       -> PASS       (fi-snb-2600) fdo#100007
> Test kms_flip:
>         Subgroup basic-flip-vs-dpms:
>                 pass       -> DMESG-WARN (fi-skl-6700hq)

Unrelated to guc debugfs changes:

[  396.647050] [drm:intel_dp_start_link_train [i915]] *ERROR* failed to enable link training

Michal

> 
> fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
> 
> fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:449s
> fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:434s
> fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:588s
> fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:513s
> fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:496s
> fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:491s
> fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:414s
> fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:407s
> fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:420s
> fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:489s
> fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:460s
> fi-kbl-7500u     total:278  pass:255  dwarn:5   dfail:0   fail:0   skip:18  time:465s
> fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:462s
> fi-skl-6700hq    total:278  pass:260  dwarn:1   dfail:0   fail:0   skip:17  time:569s
> fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:466s
> fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:499s
> fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:440s
> fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:532s
> fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:405s
> 
> ab08cb2750e769d074b2f147c8298ccd0cd08340 drm-tip: 2017y-05m-18d-15h-36m-17s UTC integration manifest
> e106859 drm/i915/guc: Remove last submission result from debugfs
> e75fc1c drm/i915/guc: Remove failed doorbell stat from debugfs
> 19d608b drm/i915/guc: Remove stale comment for q_fail
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4739/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/guc: Remove last submission result from debugfs

[Chris Wilson]

On Thu, May 18, 2017 at 11:31:04AM +0000, Michal Wajdeczko wrote:
> Debugfs does not seems to be a right place to display transient data.
> If we want to capture errors, we should log them.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

All 3 Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/guc: Remove last submission result from debugfs

[Joonas Lahtinen]

On to, 2017-05-18 at 11:31 +0000, Michal Wajdeczko wrote:
> Debugfs does not seems to be a right place to display transient data.
> If we want to capture errors, we should log them.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

<SNIP>

> @@ -2482,7 +2482,6 @@ static void i915_guc_client_info(struct seq_file *m,
>  		client->wq_size, client->wq_offset, client->wq_tail);
>  
>  	seq_printf(m, "\tWork queue full: %u\n", client->no_wq_space);

This seems like a valid candidate for removal too, the error will be
propagated (although, -EINVAL seems odd, -ENOSPC might make more
sense).

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915/guc: Remove last submission result from debugfs

[Michal Wajdeczko]

On Fri, May 19, 2017 at 11:38:29AM +0300, Joonas Lahtinen wrote:
> On to, 2017-05-18 at 11:31 +0000, Michal Wajdeczko wrote:
> > Debugfs does not seems to be a right place to display transient data.
> > If we want to capture errors, we should log them.
> > 
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> <SNIP>
> 
> > @@ -2482,7 +2482,6 @@ static void i915_guc_client_info(struct seq_file *m,
> >  		client->wq_size, client->wq_offset, client->wq_tail);
> >  
> >  	seq_printf(m, "\tWork queue full: %u\n", client->no_wq_space);
> 
> This seems like a valid candidate for removal too, the error will be
> propagated (although, -EINVAL seems odd, -ENOSPC might make more
> sense).

This removal is included in other patch:

	[PATCH 3/6] drm/i915/guc: Remove GuC wq reservation
	https://patchwork.freedesktop.org/patch/157088/

Michal

> 
> Regards, Joonas
> -- 
> Joonas Lahtinen
> Open Source Technology Center
> Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: warning for series starting with [1/3] drm/i915/guc: Remove stale comment for q_fail

[Chris Wilson]

On Thu, May 18, 2017 at 07:12:37PM +0200, Michal Wajdeczko wrote:
> On Thu, May 18, 2017 at 04:38:01PM +0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: series starting with [1/3] drm/i915/guc: Remove stale comment for q_fail
> > URL   : https://patchwork.freedesktop.org/series/24622/
> > State : warning
> > 
> > == Summary ==
> > 
> > Series 24622v1 Series without cover letter
> > https://patchwork.freedesktop.org/api/1.0/series/24622/revisions/1/mbox/
> > 
> > Test gem_exec_flush:
> >         Subgroup basic-batch-kernel-default-uc:
> >                 fail       -> PASS       (fi-snb-2600) fdo#100007
> > Test kms_flip:
> >         Subgroup basic-flip-vs-dpms:
> >                 pass       -> DMESG-WARN (fi-skl-6700hq)
> 
> Unrelated to guc debugfs changes:
> 
> [  396.647050] [drm:intel_dp_start_link_train [i915]] *ERROR* failed to enable link training

And pushed. Thanks,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915/guc: Remove stale comment for q_fail

[Jani Nikula]

On Thu, 18 May 2017, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
> This member was dropped long time ago.
>
> Fixes: 774439e1 ("drm/i915/guc: re-optimise i915_guc_client layout")

For future reference, please only use Fixes: to indicate actual bugs
fixed. I use the tag to track fixes that need to be backported, stale
comments or typo fixes or whatnot need not, and excessive Fixes: use
leads to noise for me. That said, when in doubt, err on the side of
adding one.

Thanks,
Jani.


> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_uc.h | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 7618b71..8d52a37 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -59,8 +59,6 @@ struct drm_i915_gem_request;
>   *                available in the work queue (note, the queue is shared,
>   *                not per-engine). It is OK for this to be nonzero, but
>   *                it should not be huge!
> - *   q_fail: failed to enqueue a work item. This should never happen,
> - *           because we check for space beforehand.
>   *   b_fail: failed to ring the doorbell. This should never happen, unless
>   *           somehow the hardware misbehaves, or maybe if the GuC firmware
>   *           crashes? We probably need to reset the GPU to recover.

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx