Re: [PATCH v3] drm/i915/scheduler: add gvt notification for guc submission

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ke, 2017-03-22 at 14:34 +0800, 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@intel.com
> Cc: kevin.tian@intel.com
> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>

<SNIP>

> @@ -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);

Fix indent.

>  	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);

Ditto.

> @@ -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);

Ditto.

> @@ -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)

This needs intel_lr_ prefix now that it's in .h file.

With those changes (make sure context_status_change doesn't become over
character 80 line), this is;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx