Re: [Intel-gfx] [RFC PATCH] drm/i915: make context status notifier head be per engine

[Du, Changbin]

hi, is there any comments for this? I'd like hear chris and daniel's
comments. :)

Thanks,
Changbin

On Thu, Mar 02, 2017 at 07:23:05PM +0800, changbin...@intel.com wrote:
> From: Changbin Du 
> 
> hi, Daniel, Chris and All,
> As you know, GVTg introduced the context status notifier to schedule
> the GVTg workload. At that time, the notifier is bound to GVTg context
> only, so GVTg is not aware of host workloads.
> 
> Now we are going to improve GVTg's guest workload scheduler policy,
> and add Guc emulation support for new Gen graphics. Both these two
> features require acknowledgment for all contexts running on hardware.
> (But will not alter host workload.) So here try to make some change.
> 
> The change is easy:
>   1. Move the context status notifier head from i915_gem_context to
>  intel_engine_cs. Which means there is a notifier head per engine
>  instead of per context. Execlist driver still call notifier for
>  each context sched-in/out events of current engine.
>   2. At GVTg side, it binds a notifier_block for each physical engine
>  at GVTg initialization period. Then GVTg can hear all context
>  status events.
> 
> In this RFC patch, GVTg do nothing for host context event, but later
> will add a function there. But in any case, the notifier callback is
> a noop if this is no active vGPU.
> 
> Since intel_gvt_init() is called at early initialization stage and
> require the status notifier head has been initiated, I initiate it in
> intel_engine_setup(). I am not sure if this is the right place, please
> correct me if wrong. Thank you! Look forward to your feedback. :)
> 
> Signed-off-by: Changbin Du 


signature.asc
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [RFC PATCH] drm/i915: make context status notifier head be per engine

[changbin . du]

From: Changbin Du 

hi, Daniel, Chris and All,
As you know, GVTg introduced the context status notifier to schedule
the GVTg workload. At that time, the notifier is bound to GVTg context
only, so GVTg is not aware of host workloads.

Now we are going to improve GVTg's guest workload scheduler policy,
and add Guc emulation support for new Gen graphics. Both these two
features require acknowledgment for all contexts running on hardware.
(But will not alter host workload.) So here try to make some change.

The change is easy:
  1. Move the context status notifier head from i915_gem_context to
 intel_engine_cs. Which means there is a notifier head per engine
 instead of per context. Execlist driver still call notifier for
 each context sched-in/out events of current engine.
  2. At GVTg side, it binds a notifier_block for each physical engine
 at GVTg initialization period. Then GVTg can hear all context
 status events.

In this RFC patch, GVTg do nothing for host context event, but later
will add a function there. But in any case, the notifier callback is
a noop if this is no active vGPU.

Since intel_gvt_init() is called at early initialization stage and
require the status notifier head has been initiated, I initiate it in
intel_engine_setup(). I am not sure if this is the right place, please
correct me if wrong. Thank you! Look forward to your feedback. :)

Signed-off-by: Changbin Du 
---
 drivers/gpu/drm/i915/gvt/gvt.h  |  2 +-
 drivers/gpu/drm/i915/gvt/scheduler.c| 46 ++---
 drivers/gpu/drm/i915/i915_gem_context.c |  1 -
 drivers/gpu/drm/i915/i915_gem_context.h |  3 ---
 drivers/gpu/drm/i915/intel_engine_cs.c  |  2 ++
 drivers/gpu/drm/i915/intel_lrc.c|  3 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.h |  3 +++
 7 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index a41b322..6c1ec7d6 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -162,7 +162,6 @@ struct intel_vgpu {
atomic_t running_workload_num;
DECLARE_BITMAP(tlb_handle_pending, I915_NUM_ENGINES);
struct i915_gem_context *shadow_ctx;
-   struct notifier_block shadow_ctx_notifier_block;
 
 #if IS_ENABLED(CONFIG_DRM_I915_GVT_KVMGT)
struct {
@@ -233,6 +232,7 @@ struct intel_gvt {
struct intel_gvt_gtt gtt;
struct intel_gvt_opregion opregion;
struct intel_gvt_workload_scheduler scheduler;
+   struct notifier_block shadow_ctx_notifier_block[I915_NUM_ENGINES];
DECLARE_HASHTABLE(cmd_table, GVT_CMD_HASH_BITS);
struct intel_vgpu_type *types;
unsigned int num_types;
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c 
b/drivers/gpu/drm/i915/gvt/scheduler.c
index e355a82..1cb90b8 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -127,15 +127,14 @@ static int populate_shadow_context(struct 
intel_vgpu_workload *workload)
return 0;
 }
 
+
 static int shadow_context_status_change(struct notifier_block *nb,
unsigned long action, void *data)
 {
-   struct intel_vgpu *vgpu = container_of(nb,
-   struct intel_vgpu, shadow_ctx_notifier_block);
-   struct drm_i915_gem_request *req =
-   (struct drm_i915_gem_request *)data;
-   struct intel_gvt_workload_scheduler *scheduler =
-   >gvt->scheduler;
+   struct drm_i915_gem_request *req = (struct drm_i915_gem_request *)data;
+   struct intel_gvt *gvt = container_of(nb, struct intel_gvt,
+   shadow_ctx_notifier_block[req->engine->id]);
+   struct intel_gvt_workload_scheduler *scheduler = >scheduler;
struct intel_vgpu_workload *workload =
scheduler->current_workload[req->engine->id];
 
@@ -496,15 +495,16 @@ void intel_gvt_wait_vgpu_idle(struct intel_vgpu *vgpu)
 void intel_gvt_clean_workload_scheduler(struct intel_gvt *gvt)
 {
struct intel_gvt_workload_scheduler *scheduler = >scheduler;
-   int i;
+   struct intel_engine_cs *engine;
+   enum intel_engine_id i;
 
gvt_dbg_core("clean workload scheduler\n");
 
-   for (i = 0; i < I915_NUM_ENGINES; i++) {
-   if (scheduler->thread[i]) {
-   kthread_stop(scheduler->thread[i]);
-   scheduler->thread[i] = NULL;
-   }
+   for_each_engine(engine, gvt->dev_priv, i) {
+   atomic_notifier_chain_unregister(
+   >context_status_notifier,
+   >shadow_ctx_notifier_block[i]);
+   kthread_stop(scheduler->thread[i]);
}
 }
 
@@ -512,18 +512,15 @@ int intel_gvt_init_workload_scheduler(struct intel_gvt 
*gvt)
 {
struct intel_gvt_workload_scheduler *scheduler = >scheduler;
struct