[Intel-gfx] [PATCH v2 1/2] drm/i915/scheduler: add gvt force-single-submission for guc
Dong, Chuanxiao
chuanxiao.dong at intel.com
Sat Apr 1 01:46:43 UTC 2017
> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On
> Behalf Of Chris Wilson
> Sent: Friday, March 31, 2017 10:24 PM
> To: Dong, Chuanxiao <chuanxiao.dong at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; intel-gvt-dev at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
More information about the Intel-gfx
mailing list