| Message ID | 1471596198-30748-20-git-send-email-akash.goel@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On pe, 2016-08-19 at 14:13 +0530, akash.goel@intel.com wrote: > From: Akash Goel <akash.goel@intel.com> > > The GuC log buffer flush work item has to do a register access to send the > ack to GuC and this work item, if not synced before suspend, can potentially > get executed after the GFX device is suspended. This work item function uses > rpm get/put calls around the Hw access, which covers the rpm suspend case > but for system suspend a sync would be required as kernel can potentially > schedule the work items even after some devices, including GFX, have been > put to suspend. But sync has to be done only for the system suspend case, > as sync along with rpm get/put can cause a deadlock for rpm suspend path. > To have the sync, but like a NOOP, for rpm suspend path also this work > item could have been queued from the irq handler only when the device is > runtime active & kept active while that work item is pending or getting > executed but an interrupt can come even after the device is out of use and > so can potentially lead to missing of this work item. > > By marking the workqueue, dedicated for handling GuC log buffer flush > interrupts, as freezable we don't have to bother about flushing of this > work item from the suspend hooks, the pending work item if any will be > either executed before the suspend or scheduled later on resume. This way > the handling of log buffer flush work item can be kept same between system > suspend & rpm suspend. > > Suggested-by: Imre Deak <imre.deak@intel.com> > Cc: Imre Deak <imre.deak@intel.com> > Signed-off-by: Akash Goel <akash.goel@intel.com> Thanks for taking the time, looks good to me: Reviewed-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/i915_guc_submission.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c > index d4a1c84..bb25404 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -1242,8 +1242,19 @@ static int guc_create_log_extras(struct intel_guc *guc) > > /* Need a dedicated wq to process log buffer flush interrupts > * from GuC without much delay so as to avoid any loss of logs. > + * > + * GuC log buffer flush work item has to do register access to > + * send the ack to GuC and this work item, if not synced before > + * suspend, can potentially get executed after the GFX device is > + * suspended. > + * By marking the WQ as freezable, we don't have to bother about > + * flushing of this work item from the suspend hooks, the pending > + * work item if any will be either executed before the suspend > + * or scheduled later on resume. This way the handling of work > + * item can be kept same between system suspend & rpm suspend. > */ > - guc->log.flush_wq = alloc_ordered_workqueue("i915-guc_log", 0); > + guc->log.flush_wq = > + alloc_ordered_workqueue("i915-guc_log", WQ_FREEZABLE); > if (guc->log.flush_wq == NULL) { > DRM_ERROR("Couldn't allocate the wq for GuC logging\n"); > return -ENOMEM;
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index d4a1c84..bb25404 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -1242,8 +1242,19 @@ static int guc_create_log_extras(struct intel_guc *guc) /* Need a dedicated wq to process log buffer flush interrupts * from GuC without much delay so as to avoid any loss of logs. + * + * GuC log buffer flush work item has to do register access to + * send the ack to GuC and this work item, if not synced before + * suspend, can potentially get executed after the GFX device is + * suspended. + * By marking the WQ as freezable, we don't have to bother about + * flushing of this work item from the suspend hooks, the pending + * work item if any will be either executed before the suspend + * or scheduled later on resume. This way the handling of work + * item can be kept same between system suspend & rpm suspend. */ - guc->log.flush_wq = alloc_ordered_workqueue("i915-guc_log", 0); + guc->log.flush_wq = + alloc_ordered_workqueue("i915-guc_log", WQ_FREEZABLE); if (guc->log.flush_wq == NULL) { DRM_ERROR("Couldn't allocate the wq for GuC logging\n"); return -ENOMEM;