> -----Original Message----- > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@xxxxxxxxxxxxxxxxxxxxx] On > Behalf Of Chris Wilson > Sent: Friday, March 31, 2017 10:24 PM > To: Dong, Chuanxiao <chuanxiao.dong@xxxxxxxxx> > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; intel-gvt-dev@xxxxxxxxxxxxxxxxxxxxx > 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx