> -----Original Message----- > From: Tvrtko Ursulin [mailto:tvrtko.ursulin@xxxxxxxxxxxxxxx] > Sent: Thursday, March 23, 2017 5:52 PM > To: Dong, Chuanxiao; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH v3] drm/i915/scheduler: add gvt notification > for guc submission > > > On 22/03/2017 06:34, Chuanxiao Dong wrote: > > GVT request needs a manual mmio load/restore. Before GuC submit a > > request, send notification to gvt for mmio loading. And after the GuC > > finished this GVT request, notify gvt again for mmio restore. This > > follows the usage when using execlists submission. > > > > v2: use context_status_change instead of > execlists_context_status_change > > for better understanding (ZhengXiao) > > v3: remove the comment as it is obvious and not friendly to > > the caller (Kevin) > > > > Cc: xiao.zheng@xxxxxxxxx > > Cc: kevin.tian@xxxxxxxxx > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/i915_guc_submission.c | 3 +++ > > drivers/gpu/drm/i915/intel_lrc.c | 21 +++------------------ > > drivers/gpu/drm/i915/intel_lrc.h | 13 +++++++++++++ > > 3 files changed, 19 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c > > b/drivers/gpu/drm/i915/i915_guc_submission.c > > index 055467a..0195547 100644 > > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > > @@ -520,6 +520,8 @@ static void __i915_guc_submit(struct > drm_i915_gem_request *rq) > > unsigned long flags; > > int b_ret; > > > > + context_status_change(rq, INTEL_CONTEXT_SCHEDULE_IN); > > + > > /* WA to flush out the pending GMADR writes to ring buffer. */ > > if (i915_vma_is_map_and_fenceable(rq->ring->vma)) > > POSTING_READ_FW(GUC_STATUS); > > @@ -634,6 +636,7 @@ static void i915_guc_irq_handler(unsigned long data) > > rq = port[0].request; > > while (rq && i915_gem_request_completed(rq)) { > > trace_i915_gem_request_out(rq); > > + context_status_change(rq, > INTEL_CONTEXT_SCHEDULE_OUT); > > Does GVT care that the context will still be active on the GPU for a small > window after this notification? (User interrupt happens before context > complete, which GuC hides from the driver.) Actually GVT cares. GVT driver will check the context status register to make sure the status is ACTIVE_IDLE in this notification before manually doing the context switch out for the GuC submission case. Thanks Chuanxiao > > Regards, > > Tvrtko > > > i915_gem_request_put(rq); > > port[0].request = port[1].request; > > port[1].request = NULL; > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > b/drivers/gpu/drm/i915/intel_lrc.c > > index eec1e71..24c69b5 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -295,21 +295,6 @@ uint64_t intel_lr_context_descriptor(struct > i915_gem_context *ctx, > > return ctx->engine[engine->id].lrc_desc; } > > > > -static inline void > > -execlists_context_status_change(struct drm_i915_gem_request *rq, > > - unsigned long status) > > -{ > > - /* > > - * Only used when GVT-g is enabled now. When GVT-g is disabled, > > - * The compiler should eliminate this function as dead-code. > > - */ > > - if (!IS_ENABLED(CONFIG_DRM_I915_GVT)) > > - return; > > - > > - atomic_notifier_call_chain(&rq->engine->context_status_notifier, > > - status, rq); > > -} > > - > > static void > > execlists_update_context_pdps(struct i915_hw_ppgtt *ppgtt, u32 > > *reg_state) { @@ -350,7 +335,7 @@ static void > > execlists_submit_ports(struct intel_engine_cs *engine) > > > > GEM_BUG_ON(port[0].count > 1); > > if (!port[0].count) > > - execlists_context_status_change(port[0].request, > > + context_status_change(port[0].request, > > > INTEL_CONTEXT_SCHEDULE_IN); > > desc[0] = execlists_update_context(port[0].request); > > GEM_DEBUG_EXEC(port[0].context_id = upper_32_bits(desc[0])); > @@ > > -358,7 +343,7 @@ static void execlists_submit_ports(struct > > intel_engine_cs *engine) > > > > if (port[1].request) { > > GEM_BUG_ON(port[1].count); > > - execlists_context_status_change(port[1].request, > > + context_status_change(port[1].request, > > > INTEL_CONTEXT_SCHEDULE_IN); > > desc[1] = execlists_update_context(port[1].request); > > GEM_DEBUG_EXEC(port[1].context_id = > upper_32_bits(desc[1])); @@ > > -581,7 +566,7 @@ static void intel_lrc_irq_handler(unsigned long data) > > if (--port[0].count == 0) { > > GEM_BUG_ON(status & > GEN8_CTX_STATUS_PREEMPTED); > > > GEM_BUG_ON(!i915_gem_request_completed(port[0].request)); > > - > execlists_context_status_change(port[0].request, > > + context_status_change(port[0].request, > > > INTEL_CONTEXT_SCHEDULE_OUT); > > > > > trace_i915_gem_request_out(port[0].request); > > diff --git a/drivers/gpu/drm/i915/intel_lrc.h > > b/drivers/gpu/drm/i915/intel_lrc.h > > index e8015e7..51e1be9 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.h > > +++ b/drivers/gpu/drm/i915/intel_lrc.h > > @@ -87,5 +87,18 @@ uint64_t intel_lr_context_descriptor(struct > > i915_gem_context *ctx, > > /* Execlists */ > > int intel_sanitize_enable_execlists(struct drm_i915_private *dev_priv, > > int enable_execlists); > > +static inline void > > +context_status_change(struct drm_i915_gem_request *rq, unsigned long > > +status) { > > + /* > > + * Only used when GVT-g is enabled now. When GVT-g is disabled, > > + * The compiler should eliminate this function as dead-code. > > + */ > > + if (!IS_ENABLED(CONFIG_DRM_I915_GVT)) > > + return; > > + > > + atomic_notifier_call_chain(&rq->engine->context_status_notifier, > > + status, rq); > > +} > > > > #endif /* _INTEL_LRC_H_ */ > > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
