| Message ID | 1465577461-35616-7-git-send-email-david.s.gordon@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/06/16 17:51, Dave Gordon wrote: > During a hibernate/resume cycle, the whole system is reset, including > the GuC and the doorbell hardware. Then the system is booted up, drivers > are loaded, etc -- the GuC firmware may be loaded and set running at > this point. But then, the booted kernel is replaced by the hibernated > image, and this resumed kernel will also try to reload the GuC firmware > (which will fail). To recover, we reset the GuC and try again (which > should work). But this GuC reset doesn't also reset the doorbell > hardware, so it can be left in a state inconsistent with that assumed > by the driver and/or the newly-loaded GuC firmware. > > It would be better if the GuC reset also cleared all doorbell state, > but that's not how the hardware currently works; also, the driver cannot > directly reprogram the doorbell hardware (only the GuC can do that). > > So this patch cycles through all doorbells, assigning and releasing each > in turn, so that all the doorbell hardware is left in a consistent > state, no matter how it was programmed by the previously-running kernel > and/or GuC firmware. > > v2: don't use kmap_atomic() now that client page 0 is kept mapped. > > Signed-off-by: Dave Gordon <david.s.gordon@intel.com> > --- > drivers/gpu/drm/i915/i915_guc_submission.c | 44 +++++++++++++++++++++++++++++- > 1 file changed, 43 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c > index 1833bfd..120d2e8 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -694,6 +694,48 @@ static void guc_client_free(struct drm_device *dev, > kfree(client); > } > > +/* > + * Borrow the first client to set up & tear down every doorbell > + * in turn, to ensure that all doorbell h/w is (re)initialised. > + */ > +static void guc_init_doorbell_hw(struct intel_guc *guc) > +{ > + struct drm_i915_private *dev_priv = guc_to_i915(guc); > + struct i915_guc_client *client = guc->execbuf_client; > + uint16_t db_id, i; > + int err; > + > + db_id = client->doorbell_id; > + > + for (i = 0; i < GUC_MAX_DOORBELLS; ++i) { > + i915_reg_t drbreg = GEN8_DRBREGL(i); > + u32 value = I915_READ(drbreg); > + > + err = guc_update_doorbell_id(guc, client, i); > + > + /* Report update failure or unexpectedly active doorbell */ > + if (err || (i != db_id && (value & GUC_DOORBELL_ENABLED))) > + DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) was 0x%x, err %d\n", > + i, drbreg.reg, value, err); > + } > + > + /* Restore to original value */ > + err = guc_update_doorbell_id(guc, client, db_id); > + if (err) > + DRM_ERROR("Failed to restore doorbell to %d, err %d\n", > + db_id, err); > + > + for (i = 0; i < GUC_MAX_DOORBELLS; ++i) { > + i915_reg_t drbreg = GEN8_DRBREGL(i); > + u32 value = I915_READ(drbreg); > + > + 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_client_alloc() - Allocate an i915_guc_client > * @dev: drm device > @@ -959,8 +1001,8 @@ int i915_guc_submission_enable(struct drm_device *dev) > } > > guc->execbuf_client = client; > - > host2guc_sample_forcewake(guc, client); > + guc_init_doorbell_hw(guc); > > return 0; > } > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 1833bfd..120d2e8 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -694,6 +694,48 @@ static void guc_client_free(struct drm_device *dev, kfree(client); } +/* + * Borrow the first client to set up & tear down every doorbell + * in turn, to ensure that all doorbell h/w is (re)initialised. + */ +static void guc_init_doorbell_hw(struct intel_guc *guc) +{ + struct drm_i915_private *dev_priv = guc_to_i915(guc); + struct i915_guc_client *client = guc->execbuf_client; + uint16_t db_id, i; + int err; + + db_id = client->doorbell_id; + + for (i = 0; i < GUC_MAX_DOORBELLS; ++i) { + i915_reg_t drbreg = GEN8_DRBREGL(i); + u32 value = I915_READ(drbreg); + + err = guc_update_doorbell_id(guc, client, i); + + /* Report update failure or unexpectedly active doorbell */ + if (err || (i != db_id && (value & GUC_DOORBELL_ENABLED))) + DRM_DEBUG_DRIVER("Doorbell %d (reg 0x%x) was 0x%x, err %d\n", + i, drbreg.reg, value, err); + } + + /* Restore to original value */ + err = guc_update_doorbell_id(guc, client, db_id); + if (err) + DRM_ERROR("Failed to restore doorbell to %d, err %d\n", + db_id, err); + + for (i = 0; i < GUC_MAX_DOORBELLS; ++i) { + i915_reg_t drbreg = GEN8_DRBREGL(i); + u32 value = I915_READ(drbreg); + + 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_client_alloc() - Allocate an i915_guc_client * @dev: drm device @@ -959,8 +1001,8 @@ int i915_guc_submission_enable(struct drm_device *dev) } guc->execbuf_client = client; - host2guc_sample_forcewake(guc, client); + guc_init_doorbell_hw(guc); return 0; }
During a hibernate/resume cycle, the whole system is reset, including the GuC and the doorbell hardware. Then the system is booted up, drivers are loaded, etc -- the GuC firmware may be loaded and set running at this point. But then, the booted kernel is replaced by the hibernated image, and this resumed kernel will also try to reload the GuC firmware (which will fail). To recover, we reset the GuC and try again (which should work). But this GuC reset doesn't also reset the doorbell hardware, so it can be left in a state inconsistent with that assumed by the driver and/or the newly-loaded GuC firmware. It would be better if the GuC reset also cleared all doorbell state, but that's not how the hardware currently works; also, the driver cannot directly reprogram the doorbell hardware (only the GuC can do that). So this patch cycles through all doorbells, assigning and releasing each in turn, so that all the doorbell hardware is left in a consistent state, no matter how it was programmed by the previously-running kernel and/or GuC firmware. v2: don't use kmap_atomic() now that client page 0 is kept mapped. Signed-off-by: Dave Gordon <david.s.gordon@intel.com> --- drivers/gpu/drm/i915/i915_guc_submission.c | 44 +++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-)