| Message ID | 1490693921-28692-2-git-send-email-chuanxiao.dong@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 28, 2017 at 05:38:40PM +0800, Chuanxiao Dong wrote: > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index dd0e9d587..951540f 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -377,24 +377,6 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) > writel(lower_32_bits(desc[0]), elsp); > } > > -static bool ctx_single_port_submission(const struct i915_gem_context *ctx) > -{ > - return (IS_ENABLED(CONFIG_DRM_I915_GVT) && > - i915_gem_context_force_single_submission(ctx)); > -} > - > -static bool can_merge_ctx(const struct i915_gem_context *prev, > - const struct i915_gem_context *next) > -{ > - if (prev != next) > - return false; > - > - if (ctx_single_port_submission(prev)) > - return false; > - > - return true; > -} > - > static void execlists_dequeue(struct intel_engine_cs *engine) > { > struct drm_i915_gem_request *last; > @@ -462,7 +444,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine) > * request, and so we never need to tell the hardware about > * the first. > */ > - if (last && !can_merge_ctx(cursor->ctx, last->ctx)) { > + if (last && ((last->ctx != cursor->ctx) || > + intel_gvt_context_single_port_submit(last->ctx))) { Which is easier to understand the original code or the replacement? Bonus points for sticking to coding style. -Chris
> -----Original Message----- > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On > Behalf Of Chris Wilson > Sent: Friday, March 31, 2017 10:24 PM > To: Dong, Chuanxiao <chuanxiao.dong@intel.com> > Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org > Subject: Re: [PATCH v2 1/2] drm/i915/scheduler: add gvt force-single- > submission for guc > > On Tue, Mar 28, 2017 at 05:38:40PM +0800, Chuanxiao Dong wrote: > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > > b/drivers/gpu/drm/i915/intel_lrc.c > > index dd0e9d587..951540f 100644 > > --- a/drivers/gpu/drm/i915/intel_lrc.c > > +++ b/drivers/gpu/drm/i915/intel_lrc.c > > @@ -377,24 +377,6 @@ static void execlists_submit_ports(struct > intel_engine_cs *engine) > > writel(lower_32_bits(desc[0]), elsp); } > > > > -static bool ctx_single_port_submission(const struct i915_gem_context > > *ctx) -{ > > - return (IS_ENABLED(CONFIG_DRM_I915_GVT) && > > - i915_gem_context_force_single_submission(ctx)); > > -} > > - > > -static bool can_merge_ctx(const struct i915_gem_context *prev, > > - const struct i915_gem_context *next) > > -{ > > - if (prev != next) > > - return false; > > - > > - if (ctx_single_port_submission(prev)) > > - return false; > > - > > - return true; > > -} > > - > > static void execlists_dequeue(struct intel_engine_cs *engine) { > > struct drm_i915_gem_request *last; > > @@ -462,7 +444,8 @@ static void execlists_dequeue(struct > intel_engine_cs *engine) > > * request, and so we never need to tell the hardware about > > * the first. > > */ > > - if (last && !can_merge_ctx(cursor->ctx, last->ctx)) { > > + if (last && ((last->ctx != cursor->ctx) || > > + intel_gvt_context_single_port_submit(last->ctx))) { > > Which is easier to understand the original code or the replacement? > Bonus points for sticking to coding style. I was thinking to use the original code but didn't find a good place to define can_merge_ctx(). The function of can_merge_ctx() is normal to all i915 gem context, seems i915_gem_context.h is a good place. But as can_merge_ctx() will call intel_gvt_context_single_port_submit() which is defined in intel_gvt.h, and intel_gvt_context_single_port_submit() will call the function defined in i915_gem_context.h as well, that will build a dependency between i915_gem_context.h and intel_gvt.h which seems not sense. So just use the above replacement to make it simple. Can we use this replacement? Or keep the original function? Thanks Chuanxiao > -Chris > > -- > Chris Wilson, Intel Open Source Technology Centre > _______________________________________________ > intel-gvt-dev mailing list > intel-gvt-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index 991e76e..58087630 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -680,10 +680,15 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine) struct drm_i915_gem_request *rq = rb_entry(rb, typeof(*rq), priotree.node); - if (last && rq->ctx != last->ctx) { + if (last && ((last->ctx != rq->ctx) || + intel_gvt_context_single_port_submit(last->ctx))) { if (port != engine->execlist_port) break; + if (intel_gvt_context_single_port_submit(last->ctx) || + intel_gvt_context_single_port_submit(rq->ctx)) + break; + i915_gem_request_assign(&port->request, last); nested_enable_signaling(last); port++; diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h index 25df2d6..c0dcd66 100644 --- a/drivers/gpu/drm/i915/intel_gvt.h +++ b/drivers/gpu/drm/i915/intel_gvt.h @@ -32,6 +32,12 @@ void intel_gvt_cleanup(struct drm_i915_private *dev_priv); int intel_gvt_init_device(struct drm_i915_private *dev_priv); void intel_gvt_clean_device(struct drm_i915_private *dev_priv); int intel_gvt_init_host(void); + +static inline bool +intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx) +{ + return i915_gem_context_force_single_submission(ctx); +} #else static inline int intel_gvt_init(struct drm_i915_private *dev_priv) { @@ -40,6 +46,11 @@ static inline int intel_gvt_init(struct drm_i915_private *dev_priv) static inline void intel_gvt_cleanup(struct drm_i915_private *dev_priv) { } +static inline bool +intel_gvt_context_single_port_submit(const struct i915_gem_context *ctx) +{ + return false; +} #endif #endif /* _INTEL_GVT_H_ */ diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index dd0e9d587..951540f 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -377,24 +377,6 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) writel(lower_32_bits(desc[0]), elsp); } -static bool ctx_single_port_submission(const struct i915_gem_context *ctx) -{ - return (IS_ENABLED(CONFIG_DRM_I915_GVT) && - i915_gem_context_force_single_submission(ctx)); -} - -static bool can_merge_ctx(const struct i915_gem_context *prev, - const struct i915_gem_context *next) -{ - if (prev != next) - return false; - - if (ctx_single_port_submission(prev)) - return false; - - return true; -} - static void execlists_dequeue(struct intel_engine_cs *engine) { struct drm_i915_gem_request *last; @@ -462,7 +444,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine) * request, and so we never need to tell the hardware about * the first. */ - if (last && !can_merge_ctx(cursor->ctx, last->ctx)) { + if (last && ((last->ctx != cursor->ctx) || + intel_gvt_context_single_port_submit(last->ctx))) { /* If we are on the second port and cannot combine * this request with the last, then we are done. */ @@ -475,8 +458,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine) * context (even though a different request) to * the second port. */ - if (ctx_single_port_submission(last->ctx) || - ctx_single_port_submission(cursor->ctx)) + if (intel_gvt_context_single_port_submit(last->ctx) || + intel_gvt_context_single_port_submit(cursor->ctx)) break; GEM_BUG_ON(last->ctx == cursor->ctx);
GVT needs single submission and cannot allow merge. So when GuC submitting a GVT request, the next one should be submitted to guc later until the previous one is completed. This is following the usage when using execlist mode submission. v2: make force-single-submission specific to gvt (Chris) Cc: chris@chris-wilson.co.uk Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com> --- drivers/gpu/drm/i915/i915_guc_submission.c | 7 ++++++- drivers/gpu/drm/i915/intel_gvt.h | 11 +++++++++++ drivers/gpu/drm/i915/intel_lrc.c | 25 ++++--------------------- 3 files changed, 21 insertions(+), 22 deletions(-)