| Message ID | 1475759659-16616-1-git-send-email-michal.winiarski@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 06, 2016 at 03:14:19PM +0200, Michał Winiarski wrote: > We're reserving space in the workqueue early during request allocation. > This can be problematic if we get interrupted later in the process, > because we can end up in a state where GuC workqueue is seemingly full, > while the HW is idle and no work has been submitted. > Let's delay the reservation to avoid such scenario. > > Testcase: igt/gem_ringfill/*-interruptible > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97978 > Cc: Michel Thierry <michel.thierry@intel.com> > Cc: Jeff Mcgee <jeff.mcgee@intel.com> > Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 +++++++++++ > drivers/gpu/drm/i915/intel_lrc.c | 11 ----------- > 2 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index e88786e..37204db 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1529,6 +1529,17 @@ execbuf_submit(struct i915_execbuffer_params *params, > if (ret) > return ret; > > + if (i915.enable_guc_submission) { No. This logic does not deserve to be at this level. Please try fixing the underlying problem, which would be that we don't cancel the reserved space. -Chris
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index e88786e..37204db 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1529,6 +1529,17 @@ execbuf_submit(struct i915_execbuffer_params *params, if (ret) return ret; + if (i915.enable_guc_submission) { + /* + * Check that the GuC has space for the request before + * going any further, as the i915_add_request() call + * later on mustn't fail ... + */ + ret = i915_guc_wq_reserve(params->request); + if (ret) + return ret; + } + trace_i915_gem_ring_dispatch(params->request, params->dispatch_flags); i915_gem_execbuffer_move_to_active(vmas, params->request); diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 936f6f6..cc8fd03 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -626,17 +626,6 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request request->ring = ce->ring; - if (i915.enable_guc_submission) { - /* - * Check that the GuC has space for the request before - * going any further, as the i915_add_request() call - * later on mustn't fail ... - */ - ret = i915_guc_wq_reserve(request); - if (ret) - return ret; - } - ret = intel_lr_context_pin(request->ctx, engine); if (ret) return ret;
We're reserving space in the workqueue early during request allocation. This can be problematic if we get interrupted later in the process, because we can end up in a state where GuC workqueue is seemingly full, while the HW is idle and no work has been submitted. Let's delay the reservation to avoid such scenario. Testcase: igt/gem_ringfill/*-interruptible Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97978 Cc: Michel Thierry <michel.thierry@intel.com> Cc: Jeff Mcgee <jeff.mcgee@intel.com> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com> --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 11 +++++++++++ drivers/gpu/drm/i915/intel_lrc.c | 11 ----------- 2 files changed, 11 insertions(+), 11 deletions(-)