Re: [PATCH v6 8/9] drm/i915: Support LRC context single submission

Joonas Lahtinen
2016-06-03T09:47:32Z

On to, 2016-06-02 at 12:36 -0400, Zhi Wang wrote:
> This patch introduces the support of LRC context signle submission.

"single"

> As GVT context may come from different guests, which requires different

"require"

> configuration of render registers. It can't be combined into a dual ELSP
> submission combo.
>
> Only GVT-g will create this kinds of GEM context currently.
>
> v6:
> - Make GVT code as dead code when !CONFIG_DRM_I915_GVT. (Chris)
>
> v5:
>
> - Only compile this feature when CONFIG_DRM_I915_GVT=y. (Tvrtko)
>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_lrc.c | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index dee72d3..92d01e3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -882,6 +882,7 @@ struct i915_gem_context {
>   u32 lrc_ring_buffer_size;
>   u32 lrc_addressing_mode_bits;
>   bool enable_lrc_status_change_notification;
> + bool enable_lrc_single_submission;
>
>   struct list_head link;
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 96d20c8..ce707ea 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -446,6 +446,21 @@ static void execlists_context_unqueue(struct intel_engine_cs *engine)
>   i915_gem_request_unreference(req0);
>   req0 = cursor;
>   } else {
> + /* Compiler will do the dead-code elimination */
> + if (IS_ENABLED(CONFIG_DRM_I915_GVT)) {
> + /*
> +  * req0 (after merged) ctx requires single
> +  * submission, stop picking
> +  */
> + if (req0->ctx->enable_lrc_single_submission)
> + break;
> + /*
> +  * req0 ctx doesn't require single submission,
> +  * but next req ctx requires, stop picking
> +  */
> + if (cursor->ctx->enable_lrc_single_submission)
> + break;
> + }

I remember discussing this on the F2F, we will be aware of the VM IDs
at this point, so we could use that criterion, instead of just
disabling it pessimistically. Is there some reason we could not do it?

Check would be rather simple;

#if IS_ENABLED(CONFIG_DRM_I915_GVT)
if (req0->gvt.vmid != cursor->gvt.vmid)
break;
#endif

I'm not sure if it will be worth the #if when vmid would be zero in
DOM0 always.

Regards, Joonas

>   req1 = cursor;
>   WARN_ON(req1->elsp_submitted);
>   break;
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation