diff mbox

[1/5] drm/i915/guc: Remove obsolete comments and remove unused variable

Message ID 20170914083216.10192-1-michal.winiarski@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michał Winiarski Sept. 14, 2017, 8:32 a.m. UTC
Originally removed in:
c1adab970348 ("drm/i915/guc: Remove failed doorbell stat from debugfs")
f1448a62a103 ("drm/i915/guc: Remove last submission result from debugfs")

Were accidentally restored in:
925344ccc91d ("BackMerge tag 'v4.12-rc5' into drm-next")

We can also remove unused variable and replace it with a WARN.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 3 +--
 drivers/gpu/drm/i915/intel_uc.h            | 4 ----
 2 files changed, 1 insertion(+), 6 deletions(-)

Comments

Michał Winiarski Sept. 14, 2017, 10:28 a.m. UTC | #1
On Thu, Sep 14, 2017 at 09:04:57AM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
> URL   : https://patchwork.freedesktop.org/series/30345/
> State : failure
> 
> == Summary ==
> 
> Series 30345v1 series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
> https://patchwork.freedesktop.org/api/1.0/series/30345/revisions/1/mbox/
> 
> Test chamelium:
>         Subgroup dp-crc-fast:
>                 fail       -> PASS       (fi-kbl-7500u) fdo#102514
> Test debugfs_test:
>         Subgroup read_all_entries:
>                 pass       -> SKIP       (fi-glk-2a)
> Test drv_hangman:
>         Subgroup error-state-basic:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_busy:
>         Subgroup basic-busy-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-hang-default:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_close_race:
>         Subgroup basic-process:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-threads:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_cpu_reloc:
>         Subgroup basic:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_cs_tlb:
>         Subgroup basic-default:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_ctx_create:
>         Subgroup basic:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-files:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_ctx_exec:
>         Subgroup basic:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_ctx_switch:
>         Subgroup basic-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-default-heavy:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_exec_basic:
>         Subgroup basic-blt:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-bsd:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-render:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-vebox:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup gtt-blt:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup gtt-bsd:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup gtt-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup gtt-render:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup gtt-vebox:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup readonly-blt:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup readonly-bsd:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup readonly-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup readonly-render:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup readonly-vebox:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_exec_create:
>         Subgroup basic:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_exec_fence:
>         Subgroup basic-busy-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-wait-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-await-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup await-hang-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup nb-await-default:
>                 pass       -> SKIP       (fi-glk-2a)
> Test gem_exec_flush:
>         Subgroup basic-batch-kernel-default-uc:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-batch-kernel-default-wb:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-uc-pro-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-uc-prw-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-uc-ro-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-uc-rw-default:
>                 pass       -> SKIP       (fi-glk-2a)
>         Subgroup basic-uc-set-default:
> WARNING: Long output truncated
> fi-pnv-d510 failed to connect after reboot
> 
> b21142588a9a16f74988f6b524953d4382be4a81 drm-tip: 2017y-09m-14d-08h-23m-57s UTC integration manifest
> 04fa653656cf HAX Enable GuC Submission for CI
> 8473b76dbe42 drm/i915/guc: Cleanup adding GuC work items
> 6800bd8067ca drm/i915/guc: Simplify GuC doorbell logic
> 158ad955fb26 drm/i915/guc: Submit GuC workitems containing coalesced requests
> ef09cda09694 drm/i915/guc: Remove obsolete comments and remove unused variable

I think the only suspicious thing in the results, is the GPU hang in gem_sync.
However, I'm also able to reproduce it locally without the series (with GuC
submission enabled).

-Michał

> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5693/
Chris Wilson Sept. 14, 2017, 10:40 a.m. UTC | #2
Quoting Michał Winiarski (2017-09-14 11:28:11)
> On Thu, Sep 14, 2017 at 09:04:57AM +0000, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
> > URL   : https://patchwork.freedesktop.org/series/30345/
> > State : failure
> > 
> > == Summary ==
> > 
> > Series 30345v1 series starting with [1/5] drm/i915/guc: Remove obsolete comments and remove unused variable
> > https://patchwork.freedesktop.org/api/1.0/series/30345/revisions/1/mbox/
> > 
> > Test chamelium:
> >         Subgroup dp-crc-fast:
> >                 fail       -> PASS       (fi-kbl-7500u) fdo#102514
> > Test debugfs_test:
> >         Subgroup read_all_entries:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test drv_hangman:
> >         Subgroup error-state-basic:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_busy:
> >         Subgroup basic-busy-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-hang-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_close_race:
> >         Subgroup basic-process:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-threads:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_cpu_reloc:
> >         Subgroup basic:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_cs_tlb:
> >         Subgroup basic-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_ctx_create:
> >         Subgroup basic:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-files:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_ctx_exec:
> >         Subgroup basic:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_ctx_switch:
> >         Subgroup basic-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-default-heavy:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_exec_basic:
> >         Subgroup basic-blt:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-bsd:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-render:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-vebox:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup gtt-blt:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup gtt-bsd:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup gtt-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup gtt-render:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup gtt-vebox:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup readonly-blt:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup readonly-bsd:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup readonly-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup readonly-render:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup readonly-vebox:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_exec_create:
> >         Subgroup basic:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_exec_fence:
> >         Subgroup basic-busy-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-wait-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-await-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup await-hang-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup nb-await-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> > Test gem_exec_flush:
> >         Subgroup basic-batch-kernel-default-uc:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-batch-kernel-default-wb:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-uc-pro-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-uc-prw-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-uc-ro-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-uc-rw-default:
> >                 pass       -> SKIP       (fi-glk-2a)
> >         Subgroup basic-uc-set-default:
> > WARNING: Long output truncated
> > fi-pnv-d510 failed to connect after reboot
> > 
> > b21142588a9a16f74988f6b524953d4382be4a81 drm-tip: 2017y-09m-14d-08h-23m-57s UTC integration manifest
> > 04fa653656cf HAX Enable GuC Submission for CI
> > 8473b76dbe42 drm/i915/guc: Cleanup adding GuC work items
> > 6800bd8067ca drm/i915/guc: Simplify GuC doorbell logic
> > 158ad955fb26 drm/i915/guc: Submit GuC workitems containing coalesced requests
> > ef09cda09694 drm/i915/guc: Remove obsolete comments and remove unused variable
> 
> I think the only suspicious thing in the results, is the GPU hang in gem_sync.
> However, I'm also able to reproduce it locally without the series (with GuC
> submission enabled).

And it's been fluctuating between different versions of this series as
well (without an obvious correlation to the series/versions).
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index b28677e5a4f2..c180ff1423fd 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -601,7 +601,6 @@  static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	struct intel_guc *guc = &rq->i915->guc;
 	struct i915_guc_client *client = guc->execbuf_client;
 	unsigned long flags;
-	int b_ret;
 
 	/* WA to flush out the pending GMADR writes to ring buffer. */
 	if (i915_vma_is_map_and_fenceable(rq->ring->vma))
@@ -610,7 +609,7 @@  static void __i915_guc_submit(struct drm_i915_gem_request *rq)
 	spin_lock_irqsave(&client->wq_lock, flags);
 
 	guc_wq_item_append(client, rq);
-	b_ret = guc_ring_doorbell(client);
+	WARN_ON(guc_ring_doorbell(client));
 
 	client->submissions[engine_id] += 1;
 
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 22ae52b17b0f..69daf4c01cd0 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -59,10 +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 {
 	struct i915_vma *vma;