[v3,1/4] drm/i915/gvt: use cmd to restore in-context mmios to hw for gen9 platform

Submitted by Zhao, Yan Y on April 29, 2019, 8:13 a.m.

Details

Message ID 20190429081309.15428-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 April 29, 2019, 8:13 a.m.
for restore-inhibit context, hardware will not load in-context mmios
(engine context part) to hardware, but hardware will save the mmio
values in hardware back to context image. So, in order to save correct
values of vGPU back to context image, values of vGPU mmios have to be
loaded into hardware first for restore-inhibit context.

In this patch, the mechanism is applied to all gen9 platform.

The reason excluding gen8 platforms is only because of lacking of testing
on those platforms.

v2: update vreg when scanning indirect context for inhibit context for
gen9

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

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
index ab002cfd3cab..5cb59c0b4bbe 100644
--- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
+++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
@@ -896,12 +896,16 @@  static int cmd_reg_handler(struct parser_exec_state *s,
 	}
 
 	/* TODO
-	 * Right now only scan LRI command on KBL and in inhibit context.
-	 * It's good enough to support initializing mmio by lri command in
-	 * vgpu inhibit context on KBL.
+	 * In order to let workload with inhibit context to generate
+	 * correct image data into memory, vregs values will be loaded to
+	 * hw via LRIs in the workload with inhibit context. But as
+	 * indirect context is loaded prior to LRIs in workload, we don't
+	 * want reg values specified in indirect context overwritten by
+	 * LRIs in workloads. So, when scanning an indirect context, we
+	 * update reg values in it into vregs, so LRIs in workload with
+	 * inhibit context will restore with correct values
 	 */
-	if ((IS_KABYLAKE(s->vgpu->gvt->dev_priv)
-		|| IS_COFFEELAKE(s->vgpu->gvt->dev_priv)) &&
+	if (IS_GEN(gvt->dev_priv, 9) &&
 			intel_gvt_mmio_is_in_ctx(gvt, offset) &&
 			!strncmp(cmd, "lri", 3)) {
 		intel_gvt_hypervisor_read_gpa(s->vgpu,
diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c b/drivers/gpu/drm/i915/gvt/mmio_context.c
index e7e14c842be4..a51c51b2c82e 100644
--- a/drivers/gpu/drm/i915/gvt/mmio_context.c
+++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
@@ -469,11 +469,10 @@  static void switch_mmio(struct intel_vgpu *pre,
 			continue;
 		/*
 		 * No need to do save or restore of the mmio which is in context
-		 * state image on kabylake, it's initialized by lri command and
+		 * state image on gen9, it's initialized by lri command and
 		 * save or restore with context together.
 		 */
-		if ((IS_KABYLAKE(dev_priv) || IS_BROXTON(dev_priv)
-			|| IS_COFFEELAKE(dev_priv)) && mmio->in_context)
+		if (IS_GEN(dev_priv, 9) && mmio->in_context)
 			continue;
 
 		// save
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index 8998fa5ab198..1f3ba8efb994 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -299,9 +299,7 @@  static int copy_workload_to_ring_buffer(struct intel_vgpu_workload *workload)
 	void *shadow_ring_buffer_va;
 	u32 *cs;
 
-	if ((IS_KABYLAKE(req->i915) || IS_BROXTON(req->i915)
-		|| IS_COFFEELAKE(req->i915))
-		&& is_inhibit_context(req->hw_context))
+	if (IS_GEN(req->i915, 9) && is_inhibit_context(req->hw_context))
 		intel_vgpu_restore_inhibit_context(vgpu, req);
 
 	/* allocate shadow ring buffer */

Comments


> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Zhenyu Wang
> Sent: Tuesday, April 30, 2019 4:24 PM
> To: Li, Weinan Z <weinan.z.li@intel.com>
> Cc: intel-gvt-dev@lists.freedesktop.org; Zhao, Yan Y <yan.y.zhao@intel.com>
> Subject: Re: [PATCH v3 1/4] drm/i915/gvt: use cmd to restore in-context
> mmios to hw for gen9 platform
> 
> On 2019.04.29 08:38:59 +0000, Li, Weinan Z wrote:
> > > -----Original Message-----
> > > From: Zhao, Yan Y
> > > Sent: Monday, April 29, 2019 4:13 PM
> > > To: intel-gvt-dev@lists.freedesktop.org
> > > Cc: Zhao, Yan Y <yan.y.zhao@intel.com>; Li, Weinan Z
> > > <weinan.z.li@intel.com>
> > > Subject: [PATCH v3 1/4] drm/i915/gvt: use cmd to restore in-context
> > > mmios to hw for gen9 platform
> > >
> > > for restore-inhibit context, hardware will not load in-context mmios
> > > (engine context part) to hardware, but hardware will save the mmio
> > > values in hardware back to context image. So, in order to save
> > > correct values of vGPU back to context image, values of vGPU mmios
> > > have to be loaded into hardware first for restore-inhibit context.
> > >
> > > In this patch, the mechanism is applied to all gen9 platform.
> > >
> > > The reason excluding gen8 platforms is only because of lacking of
> > > testing on those platforms.
> > >
> > > v2: update vreg when scanning indirect context for inhibit context
> > > for
> > > gen9
> > >
> > > Cc: Weinan Li <weinan.z.li@intel.com>
> > > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gvt/cmd_parser.c   | 14 +++++++++-----
> > >  drivers/gpu/drm/i915/gvt/mmio_context.c |  5 ++---
> > >  drivers/gpu/drm/i915/gvt/scheduler.c    |  4 +---
> > >  3 files changed, 12 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > index ab002cfd3cab..5cb59c0b4bbe 100644
> > > --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > > @@ -896,12 +896,16 @@ static int cmd_reg_handler(struct
> > > parser_exec_state *s,
> > >  	}
> > >
> > >  	/* TODO
> > > -	 * Right now only scan LRI command on KBL and in inhibit context.
> > > -	 * It's good enough to support initializing mmio by lri command in
> > > -	 * vgpu inhibit context on KBL.
> > > +	 * In order to let workload with inhibit context to generate
> > > +	 * correct image data into memory, vregs values will be loaded to
> > > +	 * hw via LRIs in the workload with inhibit context. But as
> > > +	 * indirect context is loaded prior to LRIs in workload, we don't
> > > +	 * want reg values specified in indirect context overwritten by
> > > +	 * LRIs in workloads. So, when scanning an indirect context, we
> > > +	 * update reg values in it into vregs, so LRIs in workload with
> > > +	 * inhibit context will restore with correct values
> > >  	 */
> > > -	if ((IS_KABYLAKE(s->vgpu->gvt->dev_priv)
> > > -		|| IS_COFFEELAKE(s->vgpu->gvt->dev_priv)) &&
> > > +	if (IS_GEN(gvt->dev_priv, 9) &&
> > >  			intel_gvt_mmio_is_in_ctx(gvt, offset) &&
> > >  			!strncmp(cmd, "lri", 3)) {
> > >  		intel_gvt_hypervisor_read_gpa(s->vgpu,
> > > diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c
> > > b/drivers/gpu/drm/i915/gvt/mmio_context.c
> > > index e7e14c842be4..a51c51b2c82e 100644
> > > --- a/drivers/gpu/drm/i915/gvt/mmio_context.c
> > > +++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
> > > @@ -469,11 +469,10 @@ static void switch_mmio(struct intel_vgpu
> *pre,
> > >  			continue;
> > >  		/*
> > >  		 * No need to do save or restore of the mmio which is in
> context
> > > -		 * state image on kabylake, it's initialized by lri command and
> > > +		 * state image on gen9, it's initialized by lri command and
> > >  		 * save or restore with context together.
> > >  		 */
> > > -		if ((IS_KABYLAKE(dev_priv) || IS_BROXTON(dev_priv)
> > > -			|| IS_COFFEELAKE(dev_priv)) && mmio->in_context)
> > > +		if (IS_GEN(dev_priv, 9) && mmio->in_context)
> > >  			continue;
> > >
> > >  		// save
> >
> > @@ -392,9 +392,7 @@ static void switch_mocs(struct intel_vgpu *pre,
> struct intel_vgpu *next,
> >                 return;
> >
> >         if (ring_id == RCS0 &&
> > -           (IS_KABYLAKE(dev_priv) ||
> > -            IS_BROXTON(dev_priv) ||
> > -            IS_COFFEELAKE(dev_priv)))
> > +           IS_GEN(dev_priv, 9))
> >                 return;
> > we can ignore the MOCS save/restore if it's RCS, since it's in context MMIO.
> >
> 
> Why it is not skipped on SKL now?
Well. It's a long story. Basically I just skip it on KBL, but do not on the other platforms. Then BXT, CFL platforms enabled, it was added during the enable process.

> 
> > > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
> > > b/drivers/gpu/drm/i915/gvt/scheduler.c
> > > index 8998fa5ab198..1f3ba8efb994 100644
> > > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > > @@ -299,9 +299,7 @@ static int copy_workload_to_ring_buffer(struct
> > > intel_vgpu_workload *workload)
> > >  	void *shadow_ring_buffer_va;
> > >  	u32 *cs;
> > >
> > > -	if ((IS_KABYLAKE(req->i915) || IS_BROXTON(req->i915)
> > > -		|| IS_COFFEELAKE(req->i915))
> > > -		&& is_inhibit_context(req->hw_context))
> > > +	if (IS_GEN(req->i915, 9) && is_inhibit_context(req->hw_context))
> > >  		intel_vgpu_restore_inhibit_context(vgpu, req);
> > >
> > >  	/* allocate shadow ring buffer */
> > > --
> > > 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
On Mon, Apr 29, 2019 at 04:38:59PM +0800, Li, Weinan Z wrote:
> > -----Original Message-----
> > From: Zhao, Yan Y
> > Sent: Monday, April 29, 2019 4:13 PM
> > To: intel-gvt-dev@lists.freedesktop.org
> > Cc: Zhao, Yan Y <yan.y.zhao@intel.com>; Li, Weinan Z <weinan.z.li@intel.com>
> > Subject: [PATCH v3 1/4] drm/i915/gvt: use cmd to restore in-context mmios
> > to hw for gen9 platform
> > 
> > for restore-inhibit context, hardware will not load in-context mmios (engine
> > context part) to hardware, but hardware will save the mmio values in
> > hardware back to context image. So, in order to save correct values of vGPU
> > back to context image, values of vGPU mmios have to be loaded into
> > hardware first for restore-inhibit context.
> > 
> > In this patch, the mechanism is applied to all gen9 platform.
> > 
> > The reason excluding gen8 platforms is only because of lacking of testing on
> > those platforms.
> > 
> > v2: update vreg when scanning indirect context for inhibit context for
> > gen9
> > 
> > Cc: Weinan Li <weinan.z.li@intel.com>
> > Signed-off-by: Yan Zhao <yan.y.zhao@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/cmd_parser.c   | 14 +++++++++-----
> >  drivers/gpu/drm/i915/gvt/mmio_context.c |  5 ++---
> >  drivers/gpu/drm/i915/gvt/scheduler.c    |  4 +---
> >  3 files changed, 12 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > index ab002cfd3cab..5cb59c0b4bbe 100644
> > --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> > @@ -896,12 +896,16 @@ static int cmd_reg_handler(struct
> > parser_exec_state *s,
> >  	}
> > 
> >  	/* TODO
> > -	 * Right now only scan LRI command on KBL and in inhibit context.
> > -	 * It's good enough to support initializing mmio by lri command in
> > -	 * vgpu inhibit context on KBL.
> > +	 * In order to let workload with inhibit context to generate
> > +	 * correct image data into memory, vregs values will be loaded to
> > +	 * hw via LRIs in the workload with inhibit context. But as
> > +	 * indirect context is loaded prior to LRIs in workload, we don't
> > +	 * want reg values specified in indirect context overwritten by
> > +	 * LRIs in workloads. So, when scanning an indirect context, we
> > +	 * update reg values in it into vregs, so LRIs in workload with
> > +	 * inhibit context will restore with correct values
> >  	 */
> > -	if ((IS_KABYLAKE(s->vgpu->gvt->dev_priv)
> > -		|| IS_COFFEELAKE(s->vgpu->gvt->dev_priv)) &&
> > +	if (IS_GEN(gvt->dev_priv, 9) &&
> >  			intel_gvt_mmio_is_in_ctx(gvt, offset) &&
> >  			!strncmp(cmd, "lri", 3)) {
> >  		intel_gvt_hypervisor_read_gpa(s->vgpu,
> > diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c
> > b/drivers/gpu/drm/i915/gvt/mmio_context.c
> > index e7e14c842be4..a51c51b2c82e 100644
> > --- a/drivers/gpu/drm/i915/gvt/mmio_context.c
> > +++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
> > @@ -469,11 +469,10 @@ static void switch_mmio(struct intel_vgpu *pre,
> >  			continue;
> >  		/*
> >  		 * No need to do save or restore of the mmio which is in context
> > -		 * state image on kabylake, it's initialized by lri command and
> > +		 * state image on gen9, it's initialized by lri command and
> >  		 * save or restore with context together.
> >  		 */
> > -		if ((IS_KABYLAKE(dev_priv) || IS_BROXTON(dev_priv)
> > -			|| IS_COFFEELAKE(dev_priv)) && mmio->in_context)
> > +		if (IS_GEN(dev_priv, 9) && mmio->in_context)
> >  			continue;
> > 
> >  		// save
> 
> @@ -392,9 +392,7 @@ static void switch_mocs(struct intel_vgpu *pre, struct intel_vgpu *next,
>                 return;
> 
>         if (ring_id == RCS0 &&
> -           (IS_KABYLAKE(dev_priv) ||
> -            IS_BROXTON(dev_priv) ||
> -            IS_COFFEELAKE(dev_priv)))
> +           IS_GEN(dev_priv, 9))
>                 return;
> we can ignore the MOCS save/restore if it's RCS, since it's in context MMIO.
>
Thanks for this finding. I'll update it :)

> > diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c
> > b/drivers/gpu/drm/i915/gvt/scheduler.c
> > index 8998fa5ab198..1f3ba8efb994 100644
> > --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> > +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> > @@ -299,9 +299,7 @@ static int copy_workload_to_ring_buffer(struct
> > intel_vgpu_workload *workload)
> >  	void *shadow_ring_buffer_va;
> >  	u32 *cs;
> > 
> > -	if ((IS_KABYLAKE(req->i915) || IS_BROXTON(req->i915)
> > -		|| IS_COFFEELAKE(req->i915))
> > -		&& is_inhibit_context(req->hw_context))
> > +	if (IS_GEN(req->i915, 9) && is_inhibit_context(req->hw_context))
> >  		intel_vgpu_restore_inhibit_context(vgpu, req);
> > 
> >  	/* allocate shadow ring buffer */
> > --
> > 2.17.1
>