| Message ID | 1460049678-21918-6-git-send-email-david.s.gordon@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
LGTM. Reviewed-by: Alex Dai <yu.dai@intel.com> On 04/07/2016 10:21 AM, Dave Gordon wrote: > When resetting and reloading the GuC, the GuC submission management code > also needs to destroy and recreate the GuC client(s). Currently this is > done by a separate call from the GuC loader, but really, it's just an > internal detail of the submission code. So here we remove the call from > the loader (which is too late, really, because the GuC has already been > reloaded at this point) and put it into guc_submission_init() instead. > This means that any preexisting client is destroyed *before* the GuC > (re)load and then recreated after, iff the firmware was successfully > loaded. If the GuC reload fails, we don't recreate the client, so fallback > to execlists mode (if active) won't leak the client object (previously, > the now-unusable client would have been left allocated, and leaked if > the driver were unloaded). > > Signed-off-by: Dave Gordon <david.s.gordon@intel.com> > --- > drivers/gpu/drm/i915/i915_guc_submission.c | 6 ++++-- > drivers/gpu/drm/i915/intel_guc_loader.c | 3 --- > 2 files changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c > index f466eab..a8717f7 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -981,6 +981,10 @@ int i915_guc_submission_init(struct drm_device *dev) > const size_t gemsize = round_up(poolsize, PAGE_SIZE); > struct intel_guc *guc = &dev_priv->guc; > > + /* Wipe bitmap & delete client in case of reinitialisation */ > + bitmap_clear(guc->doorbell_bitmap, 0, GUC_MAX_DOORBELLS); > + i915_guc_submission_disable(dev); > + > if (!i915.enable_guc_submission) > return 0; /* not enabled */ > > @@ -992,9 +996,7 @@ int i915_guc_submission_init(struct drm_device *dev) > return -ENOMEM; > > ida_init(&guc->ctx_ids); > - > guc_create_log(guc); > - > guc_create_ads(guc); > > return 0; > diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c > index 876e5da..3e14a9a 100644 > --- a/drivers/gpu/drm/i915/intel_guc_loader.c > +++ b/drivers/gpu/drm/i915/intel_guc_loader.c > @@ -470,9 +470,6 @@ int intel_guc_ucode_load(struct drm_device *dev) > intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); > > if (i915.enable_guc_submission) { > - /* The execbuf_client will be recreated. Release it first. */ > - i915_guc_submission_disable(dev); > - > err = i915_guc_submission_enable(dev); > if (err) > goto fail;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index f466eab..a8717f7 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -981,6 +981,10 @@ int i915_guc_submission_init(struct drm_device *dev) const size_t gemsize = round_up(poolsize, PAGE_SIZE); struct intel_guc *guc = &dev_priv->guc; + /* Wipe bitmap & delete client in case of reinitialisation */ + bitmap_clear(guc->doorbell_bitmap, 0, GUC_MAX_DOORBELLS); + i915_guc_submission_disable(dev); + if (!i915.enable_guc_submission) return 0; /* not enabled */ @@ -992,9 +996,7 @@ int i915_guc_submission_init(struct drm_device *dev) return -ENOMEM; ida_init(&guc->ctx_ids); - guc_create_log(guc); - guc_create_ads(guc); return 0; diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c index 876e5da..3e14a9a 100644 --- a/drivers/gpu/drm/i915/intel_guc_loader.c +++ b/drivers/gpu/drm/i915/intel_guc_loader.c @@ -470,9 +470,6 @@ int intel_guc_ucode_load(struct drm_device *dev) intel_guc_fw_status_repr(guc_fw->guc_fw_load_status)); if (i915.enable_guc_submission) { - /* The execbuf_client will be recreated. Release it first. */ - i915_guc_submission_disable(dev); - err = i915_guc_submission_enable(dev); if (err) goto fail;
When resetting and reloading the GuC, the GuC submission management code also needs to destroy and recreate the GuC client(s). Currently this is done by a separate call from the GuC loader, but really, it's just an internal detail of the submission code. So here we remove the call from the loader (which is too late, really, because the GuC has already been reloaded at this point) and put it into guc_submission_init() instead. This means that any preexisting client is destroyed *before* the GuC (re)load and then recreated after, iff the firmware was successfully loaded. If the GuC reload fails, we don't recreate the client, so fallback to execlists mode (if active) won't leak the client object (previously, the now-unusable client would have been left allocated, and leaked if the driver were unloaded). Signed-off-by: Dave Gordon <david.s.gordon@intel.com> --- drivers/gpu/drm/i915/i915_guc_submission.c | 6 ++++-- drivers/gpu/drm/i915/intel_guc_loader.c | 3 --- 2 files changed, 4 insertions(+), 5 deletions(-)