| Message ID | 1468933157-22412-1-git-send-email-david.s.gordon@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19/07/16 13:59, Dave Gordon wrote: > guc_init_doorbell_hw() borrows the (currently single) GuC client to use > in reinitialising ALL the doorbell registers (as the hardware doesn't > reset them when the GuC is reset). As a prerequisite for accommodating > multiple clients, it should only reset doorbells that are supposed to be > disabled, avoiding those that are marked as in use by any client. > > Signed-off-by: Dave Gordon <david.s.gordon@intel.com> > --- > drivers/gpu/drm/i915/i915_guc_submission.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c > index 2112e02..d8402e4 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -699,7 +699,7 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj) > } > > /* > - * Borrow the first client to set up & tear down every doorbell > + * Borrow the first client to set up & tear down each unused doorbell > * in turn, to ensure that all doorbell h/w is (re)initialised. > */ > static void guc_init_doorbell_hw(struct intel_guc *guc) > @@ -715,6 +715,9 @@ static void guc_init_doorbell_hw(struct intel_guc *guc) > i915_reg_t drbreg = GEN8_DRBREGL(i); > u32 value = I915_READ(drbreg); > > + if (test_bit(i, guc->doorbell_bitmap)) > + continue; > + > err = guc_update_doorbell_id(guc, client, i); > > /* Report update failure or unexpectedly active doorbell */ > @@ -733,6 +736,9 @@ static void guc_init_doorbell_hw(struct intel_guc *guc) > i915_reg_t drbreg = GEN8_DRBREGL(i); > u32 value = I915_READ(drbreg); > > + if (test_bit(i, guc->doorbell_bitmap)) > + continue; > + > if (i != db_id && (value & GUC_DOORBELL_ENABLED)) > DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) finally 0x%x\n", > i, drbreg.reg, value); > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
On 19/07/16 15:16, Patchwork wrote: > == Series Details == > > Series: series starting with [1/5] drm/i915/guc: doorbell reset should avoid used doorbells > URL : https://patchwork.freedesktop.org/series/10040/ > State : failure > > == Summary == > > Series 10040v1 Series without cover letter > http://patchwork.freedesktop.org/api/1.0/series/10040/revisions/1/mbox > > Test drv_module_reload_basic: > pass -> SKIP (ro-hsw-i3-4010u) > Test gem_sync: > Subgroup basic-store-each: > fail -> PASS (ro-bdw-i7-5600u) > Test kms_cursor_legacy: > Subgroup basic-cursor-vs-flip: > pass -> FAIL (ro-ilk1-i5-650) Wibble? The test log for this says: + Results for igt@kms_cursor_legacy@basic-flip-vs-cursor + Result: pass + + IGT-Version: 1.15-g4d03467 (x86_64) (Linux: 4.7.0-rc7-gfxbench-RO_Patchwork_1532+ x86_64) + Test requirement not met in function __real_main427, file kms_cursor_legacy.c:448: + Test requirement: !(n >= data.resources->count_crtcs) + Subtest basic-flip-vs-cursor: SUCCESS (1.156s) + + Command /opt/igt/tests/kms_cursor_legacy --run-subtest basic-flip-vs-cursor So whatever that unfulfilled test requirement is, the result should be PASS. > Test kms_pipe_crc_basic: > Subgroup read-crc-pipe-c-frame-sequence: > pass -> DMESG-WARN (fi-hsw-i7-4770k) [ 436.687908] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 122 [ 436.688345] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 122 which looks like https://bugzilla.kernel.org/show_bug.cgi?id=85951 [hsw] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 46 (which was closed as unreproducible) .Dave. > Subgroup suspend-read-crc-pipe-c: > pass -> SKIP (fi-hsw-i7-4770k) > Test prime_vgem: > Subgroup basic-fence-read: > fail -> PASS (ro-byt-n2820) > > fi-hsw-i7-4770k total:243 pass:211 dwarn:1 dfail:0 fail:10 skip:21 > fi-kbl-qkkr total:243 pass:177 dwarn:26 dfail:1 fail:10 skip:29 > fi-skl-i7-6700k total:243 pass:208 dwarn:0 dfail:0 fail:9 skip:26 > fi-snb-i7-2600 total:243 pass:193 dwarn:0 dfail:0 fail:10 skip:40 > ro-bdw-i5-5250u total:244 pass:217 dwarn:4 dfail:0 fail:10 skip:13 > ro-bdw-i7-5557U total:244 pass:219 dwarn:1 dfail:0 fail:9 skip:15 > ro-bdw-i7-5600u total:244 pass:202 dwarn:0 dfail:0 fail:10 skip:32 > ro-bsw-n3050 total:218 pass:173 dwarn:0 dfail:0 fail:2 skip:42 > ro-byt-n2820 total:244 pass:195 dwarn:0 dfail:0 fail:11 skip:38 > ro-hsw-i3-4010u total:244 pass:208 dwarn:0 dfail:0 fail:11 skip:25 > ro-hsw-i7-4770r total:244 pass:209 dwarn:0 dfail:0 fail:11 skip:24 > ro-ilk1-i5-650 total:239 pass:169 dwarn:0 dfail:0 fail:12 skip:58 > ro-ivb-i7-3770 total:244 pass:200 dwarn:0 dfail:0 fail:11 skip:33 > ro-skl3-i5-6260u total:244 pass:221 dwarn:1 dfail:0 fail:10 skip:12 > ro-snb-i7-2620M total:244 pass:190 dwarn:0 dfail:0 fail:12 skip:42 > fi-skl-i5-6260u failed to connect after reboot > > Results at /archive/results/CI_IGT_test/RO_Patchwork_1532/ > > 635555a drm-intel-nightly: 2016y-07m-19d-13h-02m-39s UTC integration manifest > 207ec9d drm/i915/guc: use for_each_engine_id() where appropriate > 5982c52 drm/i915/guc: add engine mask to GuC client & pass to GuC > b1f5991 drm/i915/guc: use a separate GuC client for each engine > 89623f4 drm/i915/guc: refactor guc_init_doorbell_hw() > b9c0a41 drm/i915/guc: doorbell reset should avoid used doorbells >
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 2112e02..d8402e4 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -699,7 +699,7 @@ static void gem_release_guc_obj(struct drm_i915_gem_object *obj) } /* - * Borrow the first client to set up & tear down every doorbell + * Borrow the first client to set up & tear down each unused doorbell * in turn, to ensure that all doorbell h/w is (re)initialised. */ static void guc_init_doorbell_hw(struct intel_guc *guc) @@ -715,6 +715,9 @@ static void guc_init_doorbell_hw(struct intel_guc *guc) i915_reg_t drbreg = GEN8_DRBREGL(i); u32 value = I915_READ(drbreg); + if (test_bit(i, guc->doorbell_bitmap)) + continue; + err = guc_update_doorbell_id(guc, client, i); /* Report update failure or unexpectedly active doorbell */ @@ -733,6 +736,9 @@ static void guc_init_doorbell_hw(struct intel_guc *guc) i915_reg_t drbreg = GEN8_DRBREGL(i); u32 value = I915_READ(drbreg); + if (test_bit(i, guc->doorbell_bitmap)) + continue; + if (i != db_id && (value & GUC_DOORBELL_ENABLED)) DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) finally 0x%x\n", i, drbreg.reg, value);
guc_init_doorbell_hw() borrows the (currently single) GuC client to use in reinitialising ALL the doorbell registers (as the hardware doesn't reset them when the GuC is reset). As a prerequisite for accommodating multiple clients, it should only reset doorbells that are supposed to be disabled, avoiding those that are marked as in use by any client. Signed-off-by: Dave Gordon <david.s.gordon@intel.com> --- drivers/gpu/drm/i915/i915_guc_submission.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)