[Intel-gfx] [PATCH v2] drm/i915: Sanitize GuC client initialization

Chris Wilson chris at chris-wilson.co.uk
Thu Feb 16 07:44:22 UTC 2017


On Wed, Feb 15, 2017 at 06:28:59PM -0800, Daniele Ceraolo Spurio wrote:
> On 14/02/17 05:53, Joonas Lahtinen wrote:
> >-static void guc_disable_doorbell(struct intel_guc *guc,
> >-				 struct i915_guc_client *client)
> >+static int __destroy_doorbell(struct i915_guc_client *client)
> > {
> >-	(void)guc_update_doorbell_id(guc, client, GUC_INVALID_DOORBELL_ID);
> >+	struct guc_doorbell_info *doorbell;
> >
> >-	/* XXX: wait for any interrupts */
> >-	/* XXX: wait for workqueue to drain */
> >+	doorbell = __get_doorbell(client);
> >+	doorbell->db_status = GUC_DOORBELL_DISABLED;
> >+	doorbell->cookie = 0;
> >+
> 
> Not from this patch (but since we're at it...), I did a bit of
> digging and I've found that doorbell release flow requires SW to
> poll the GEN8_DRBREGL(db_id) register after updating
> doorbell->db_status to wait for the GEN8_DRB_VALID bit to go to
> zero. This ensures that any pending events are processed before we
> call into GuC to release the doorbell. Note that GuC will fail the
> DEALLOCATE_DOORBELL action if the bit has not gone to zero yet. This
> is currently not an issue, probably because we use a single doorbell
> and we don't ring it near release time, but the situation could
> change in the future so I believe it is worth to fix it now. I can
> follow up with an additional patch if you want to keep this one as
> refactoring only.

Just keep in mind that we currently do a disable after GPU reset. The guc
may not know what we are talking about :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre


More information about the Intel-gfx mailing list