[2/2] drm/i915/gvt: optimization for save-inhibit context

Submitted by Zhao, Yan Y on June 3, 2019, 4:57 a.m.

Details

Message ID 20190603045757.31090-1-yan.y.zhao@intel.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Intel GVT devel

Not browsing as part of any series.

Commit Message

Zhao, Yan Y June 3, 2019, 4:57 a.m.
sometimes, linux guest would send preempt contexts which are both
restore inhibit context and save inhibit context.
E.g. glmark2 sends 30 preempt contexts every sec on average.

For this kind of context,
1. no need to load mmios in save-restore list back to hardware
2. no need to copy context mmios back to guest

Cc: Weinan Li <weinan.z.li@intel.com>
Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
---
 drivers/gpu/drm/i915/gvt/mmio_context.c |  8 ++++
 drivers/gpu/drm/i915/gvt/mmio_context.h |  5 +++
 drivers/gpu/drm/i915/gvt/scheduler.c    | 49 ++++++++++++++-----------
 3 files changed, 41 insertions(+), 21 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c b/drivers/gpu/drm/i915/gvt/mmio_context.c
index 0c3e2f21e28c..9b9630f4297b 100644
--- a/drivers/gpu/drm/i915/gvt/mmio_context.c
+++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
@@ -445,6 +445,14 @@  bool is_restore_inhibit_context(struct intel_context *ce)
 	return IS_RESTORE_INHIBIT(reg_state[CTX_CONTEXT_CONTROL_VAL]);
 }
 
+bool is_save_inhibit_context(struct intel_context *ce)
+{
+	const u32 *reg_state = ce->lrc_reg_state;
+
+	return IS_SAVE_INHIBIT(reg_state[CTX_CONTEXT_CONTROL_VAL]);
+}
+
+
 /* Switch ring mmio values (context). */
 static void switch_mmio(struct intel_vgpu *pre,
 			struct intel_vgpu *next,
diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.h b/drivers/gpu/drm/i915/gvt/mmio_context.h
index 08e3a775fae7..daffd2d3d023 100644
--- a/drivers/gpu/drm/i915/gvt/mmio_context.h
+++ b/drivers/gpu/drm/i915/gvt/mmio_context.h
@@ -50,6 +50,7 @@  void intel_gvt_switch_mmio(struct intel_vgpu *pre,
 void intel_gvt_init_engine_mmio_context(struct intel_gvt *gvt);
 
 bool is_restore_inhibit_context(struct intel_context *ce);
+bool is_save_inhibit_context(struct intel_context *ce);
 
 int intel_vgpu_restore_inhibit_context(struct intel_vgpu *vgpu,
 				       struct i915_request *req);
@@ -57,4 +58,8 @@  int intel_vgpu_restore_inhibit_context(struct intel_vgpu *vgpu,
 	(_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT) == \
 	((a) & _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT)))
 
+#define IS_SAVE_INHIBIT(a)	\
+	(_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT) == \
+	((a) & _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT)))
+
 #endif
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index 5c2087600442..9fb69bf37bc1 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -313,7 +313,9 @@  static int copy_workload_to_ring_buffer(struct intel_vgpu_workload *workload)
 	u32 *cs;
 	int err;
 
-	if (IS_GEN(req->i915, 9) && is_restore_inhibit_context(req->hw_context))
+	if (IS_GEN(req->i915, 9) &&
+			is_restore_inhibit_context(req->hw_context) &&
+			!is_save_inhibit_context(req->hw_context))
 		intel_vgpu_restore_inhibit_context(vgpu, req);
 
 	/*
@@ -804,6 +806,31 @@  static void update_guest_context(struct intel_vgpu_workload *workload)
 	gvt_dbg_sched("ring id %d workload lrca %x\n", rq->engine->id,
 		      workload->ctx_desc.lrca);
 
+	page = i915_gem_object_get_page(ctx_obj, LRC_STATE_PN);
+	shadow_ring_context = kmap(page);
+	if (IS_SAVE_INHIBIT(shadow_ring_context->ctx_ctrl.val)) {
+		kunmap(page);
+		return;
+	}
+
+#define COPY_REG(name) \
+	intel_gvt_hypervisor_write_gpa(vgpu, workload->ring_context_gpa + \
+		RING_CTX_OFF(name.val), &shadow_ring_context->name.val, 4)
+
+	COPY_REG(ctx_ctrl);
+	COPY_REG(ctx_timestamp);
+
+#undef COPY_REG
+
+	intel_gvt_hypervisor_write_gpa(vgpu,
+			workload->ring_context_gpa +
+			sizeof(*shadow_ring_context),
+			(void *)shadow_ring_context +
+			sizeof(*shadow_ring_context),
+			I915_GTT_PAGE_SIZE - sizeof(*shadow_ring_context));
+
+	kunmap(page);
+
 	context_page_num = rq->engine->context_size;
 	context_page_num = context_page_num >> PAGE_SHIFT;
 
@@ -832,26 +859,6 @@  static void update_guest_context(struct intel_vgpu_workload *workload)
 	intel_gvt_hypervisor_write_gpa(vgpu, workload->ring_context_gpa +
 		RING_CTX_OFF(ring_header.val), &workload->rb_tail, 4);
 
-	page = i915_gem_object_get_page(ctx_obj, LRC_STATE_PN);
-	shadow_ring_context = kmap(page);
-
-#define COPY_REG(name) \
-	intel_gvt_hypervisor_write_gpa(vgpu, workload->ring_context_gpa + \
-		RING_CTX_OFF(name.val), &shadow_ring_context->name.val, 4)
-
-	COPY_REG(ctx_ctrl);
-	COPY_REG(ctx_timestamp);
-
-#undef COPY_REG
-
-	intel_gvt_hypervisor_write_gpa(vgpu,
-			workload->ring_context_gpa +
-			sizeof(*shadow_ring_context),
-			(void *)shadow_ring_context +
-			sizeof(*shadow_ring_context),
-			I915_GTT_PAGE_SIZE - sizeof(*shadow_ring_context));
-
-	kunmap(page);
 }
 
 void intel_vgpu_clean_workloads(struct intel_vgpu *vgpu,

Comments


On Tue, Jun 04, 2019 at 03:50:45PM +0800, Zhenyu Wang wrote:
> On 2019.06.03 00:57:57 -0400, Yan Zhao wrote:
> > sometimes, linux guest would send preempt contexts which are both
> > restore inhibit context and save inhibit context.
> > E.g. glmark2 sends 30 preempt contexts every sec on average.
> > 
> > For this kind of context,
> > 1. no need to load mmios in save-restore list back to hardware
> > 2. no need to copy context mmios back to guest
> > 
> > Cc: Weinan Li <weinan.z.li@intel.com>
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/mmio_context.c |  8 ++++
> >  drivers/gpu/drm/i915/gvt/mmio_context.h |  5 +++
> >  drivers/gpu/drm/i915/gvt/scheduler.c    | 49 ++++++++++++++-----------
> >  3 files changed, 41 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c b/drivers/gpu/drm/i915/gvt/mmio_context.c
> > index 0c3e2f21e28c..9b9630f4297b 100644
> > --- a/drivers/gpu/drm/i915/gvt/mmio_context.c
> > +++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
> > @@ -445,6 +445,14 @@ bool is_restore_inhibit_context(struct intel_context *ce)
> >  	return IS_RESTORE_INHIBIT(reg_state[CTX_CONTEXT_CONTROL_VAL]);
> >  }
> >  
> > +bool is_save_inhibit_context(struct intel_context *ce)
> > +{
> > +	const u32 *reg_state = ce->lrc_reg_state;
> > +
> > +	return IS_SAVE_INHIBIT(reg_state[CTX_CONTEXT_CONTROL_VAL]);
> > +}
> > +
> > +
> >  /* Switch ring mmio values (context). */
> >  static void switch_mmio(struct intel_vgpu *pre,
> >  			struct intel_vgpu *next,
> > diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.h b/drivers/gpu/drm/i915/gvt/mmio_context.h
> > index 08e3a775fae7..daffd2d3d023 100644
> > --- a/drivers/gpu/drm/i915/gvt/mmio_context.h
> > +++ b/drivers/gpu/drm/i915/gvt/mmio_context.h
> > @@ -50,6 +50,7 @@ void intel_gvt_switch_mmio(struct intel_vgpu *pre,
> >  void intel_gvt_init_engine_mmio_context(struct intel_gvt *gvt);
> >  
> >  bool is_restore_inhibit_context(struct intel_context *ce);
> > +bool is_save_inhibit_context(struct intel_context *ce);
> >  
> >  int intel_vgpu_restore_inhibit_context(struct intel_vgpu *vgpu,
> >  				       struct i915_request *req);
> > @@ -57,4 +58,8 @@ int intel_vgpu_restore_inhibit_context(struct intel_vgpu *vgpu,
> >  	(_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT) == \
> >  	((a) & _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT)))
> >  
> > +#define IS_SAVE_INHIBIT(a)	\
> > +	(_MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT) == \
> > +	((a) & _MASKED_BIT_ENABLE(CTX_CTRL_ENGINE_CTX_SAVE_INHIBIT)))
> > +
> >  #endif
> > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> > index 5c2087600442..9fb69bf37bc1 100644
> > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > @@ -313,7 +313,9 @@ static int copy_workload_to_ring_buffer(struct intel_vgpu_workload *workload)
> >  	u32 *cs;
> >  	int err;
> >  
> > -	if (IS_GEN(req->i915, 9) && is_restore_inhibit_context(req->hw_context))
> > +	if (IS_GEN(req->i915, 9) &&
> > +			is_restore_inhibit_context(req->hw_context) &&
> > +			!is_save_inhibit_context(req->hw_context))
> >  		intel_vgpu_restore_inhibit_context(vgpu, req);
> >
> 
> From our discussion, even for save inhibit context we should still keep to restore
> mmio state in restore inhibit case, to make sure context submission is under expected
> vGPU state. Others look fine to me.

OK. Agree with you, though in reality it's normally not the case for guest to send workload in a save-inhibit context.
Will update this patch.
Thank you:)


> >  	/*
> > @@ -804,6 +806,31 @@ static void update_guest_context(struct intel_vgpu_workload *workload)
> >  	gvt_dbg_sched("ring id %d workload lrca %x\n", rq->engine->id,
> >  		      workload->ctx_desc.lrca);
> >  
> > +	page = i915_gem_object_get_page(ctx_obj, LRC_STATE_PN);
> > +	shadow_ring_context = kmap(page);
> > +	if (IS_SAVE_INHIBIT(shadow_ring_context->ctx_ctrl.val)) {
> > +		kunmap(page);
> > +		return;
> > +	}
> > +
> > +#define COPY_REG(name) \
> > +	intel_gvt_hypervisor_write_gpa(vgpu, workload->ring_context_gpa + \
> > +		RING_CTX_OFF(name.val), &shadow_ring_context->name.val, 4)
> > +
> > +	COPY_REG(ctx_ctrl);
> > +	COPY_REG(ctx_timestamp);
> > +
> > +#undef COPY_REG
> > +
> > +	intel_gvt_hypervisor_write_gpa(vgpu,
> > +			workload->ring_context_gpa +
> > +			sizeof(*shadow_ring_context),
> > +			(void *)shadow_ring_context +
> > +			sizeof(*shadow_ring_context),
> > +			I915_GTT_PAGE_SIZE - sizeof(*shadow_ring_context));
> > +
> > +	kunmap(page);
> > +
> >  	context_page_num = rq->engine->context_size;
> >  	context_page_num = context_page_num >> PAGE_SHIFT;
> >  
> > @@ -832,26 +859,6 @@ static void update_guest_context(struct intel_vgpu_workload *workload)
> >  	intel_gvt_hypervisor_write_gpa(vgpu, workload->ring_context_gpa +
> >  		RING_CTX_OFF(ring_header.val), &workload->rb_tail, 4);
> >  
> > -	page = i915_gem_object_get_page(ctx_obj, LRC_STATE_PN);
> > -	shadow_ring_context = kmap(page);
> > -
> > -#define COPY_REG(name) \
> > -	intel_gvt_hypervisor_write_gpa(vgpu, workload->ring_context_gpa + \
> > -		RING_CTX_OFF(name.val), &shadow_ring_context->name.val, 4)
> > -
> > -	COPY_REG(ctx_ctrl);
> > -	COPY_REG(ctx_timestamp);
> > -
> > -#undef COPY_REG
> > -
> > -	intel_gvt_hypervisor_write_gpa(vgpu,
> > -			workload->ring_context_gpa +
> > -			sizeof(*shadow_ring_context),
> > -			(void *)shadow_ring_context +
> > -			sizeof(*shadow_ring_context),
> > -			I915_GTT_PAGE_SIZE - sizeof(*shadow_ring_context));
> > -
> > -	kunmap(page);
> >  }
> >  
> >  void intel_vgpu_clean_workloads(struct intel_vgpu *vgpu,
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > intel-gvt-dev mailing list
> > intel-gvt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
> 
> -- 
> Open Source Technology Center, Intel ltd.
> 
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827