[PATCH 2/2] drm/i915/gvt: Refine virtual reset function

[Zhenyu Wang]

On 2017.05.22 16:55:39 +0800, Zhenyu Wang wrote:
> On 2017.05.22 16:09:36 +0800, fred gao wrote:
> > during the emulation of virtual reset:
> > 1. only reset the engine related mmio, not display stuff.
> > 2. pvinfo/fence/display are not required to set default
> >    value as well to prevent screen flicking.
> > 
> > this will fix the issue of Guest screen hang while running
> > Force tdr in Linux guest.
> > 
> > v2: only reset the engine related mmio by Zhengyu, Zhiyuan.
> > v3: IMR/Ring mode registers are not save/restored by Changbin.
> > 
> > Reviewed-by: Changbin Du <changbin.du at intel.com>
> > Reviewed-by: Zhenyu Wang <zhenyuw at linux.intel.com>
> > Reviewed-by: Lv, zhiyuan <zhiyuan.lv at intel.com>
> > Reviewed-by: Zhang Yulei <yulei.zhang at intel.com>
> > Signed-off-by: fred gao <fred.gao at intel.com>
> > ---
> >  drivers/gpu/drm/i915/gvt/mmio.c | 25 +++++++++++++++++--------
> >  drivers/gpu/drm/i915/gvt/mmio.h |  2 +-
> >  drivers/gpu/drm/i915/gvt/vgpu.c | 12 ++++++++----
> >  3 files changed, 26 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gvt/mmio.c b/drivers/gpu/drm/i915/gvt/mmio.c
> > index 1ba3bdb..b07be52 100644
> > --- a/drivers/gpu/drm/i915/gvt/mmio.c
> > +++ b/drivers/gpu/drm/i915/gvt/mmio.c
> > @@ -372,20 +372,29 @@ int intel_vgpu_emulate_mmio_write(struct intel_vgpu *vgpu, uint64_t pa,
> >   * @vgpu: a vGPU
> >   *
> >   */
> > -void intel_vgpu_reset_mmio(struct intel_vgpu *vgpu)
> > +void intel_vgpu_reset_mmio(struct intel_vgpu *vgpu, bool dmlr)
> >  {
> >  	struct intel_gvt *gvt = vgpu->gvt;
> >  	const struct intel_gvt_device_info *info = &gvt->device_info;
> > +	void  *mmio = gvt->firmware.mmio;
> >  
> > -	memcpy(vgpu->mmio.vreg, gvt->firmware.mmio, info->mmio_size);
> > -	memcpy(vgpu->mmio.sreg, gvt->firmware.mmio, info->mmio_size);
> > +	if (dmlr) {
> > +		memcpy(vgpu->mmio.vreg, mmio, info->mmio_size);
> > +		memcpy(vgpu->mmio.sreg, mmio, info->mmio_size);
> >  
> > -	vgpu_vreg(vgpu, GEN6_GT_THREAD_STATUS_REG) = 0;
> > +		vgpu_vreg(vgpu, GEN6_GT_THREAD_STATUS_REG) = 0;
> >  
> > -	/* set the bit 0:2(Core C-State ) to C0 */
> > -	vgpu_vreg(vgpu, GEN6_GT_CORE_STATUS) = 0;
> > +		/* set the bit 0:2(Core C-State ) to C0 */
> > +		vgpu_vreg(vgpu, GEN6_GT_CORE_STATUS) = 0;
> > +
> > +		vgpu->mmio.disable_warn_untrack = false;
> > +	} else {
> > +#define GVT_GEN8_MASTER_IRQ		(0x44200)
> > +		/* only reset the engine related, let alone the display mmio */
> > +		memcpy(vgpu->mmio.vreg, mmio, GVT_GEN8_MASTER_IRQ);
> > +		memcpy(vgpu->mmio.sreg, mmio, GVT_GEN8_MASTER_IRQ);
> > +	}
> 
> wow, are you sure?

ok, this is actually an offset, so name it as GVT_GEN8_RESET_OFFSET or sth
and in comment says what it actually points to.

> 
> >  
> > -	vgpu->mmio.disable_warn_untrack = false;
> >  }
> >  
> >  /**
> > @@ -405,7 +414,7 @@ int intel_vgpu_init_mmio(struct intel_vgpu *vgpu)
> >  
> >  	vgpu->mmio.sreg = vgpu->mmio.vreg + info->mmio_size;
> >  
> > -	intel_vgpu_reset_mmio(vgpu);
> > +	intel_vgpu_reset_mmio(vgpu, true);
> >  
> >  	return 0;
> >  }
> > diff --git a/drivers/gpu/drm/i915/gvt/mmio.h b/drivers/gpu/drm/i915/gvt/mmio.h
> > index a3a0270..45169f6 100644
> > --- a/drivers/gpu/drm/i915/gvt/mmio.h
> > +++ b/drivers/gpu/drm/i915/gvt/mmio.h
> > @@ -87,7 +87,7 @@ struct intel_gvt_mmio_info *intel_gvt_find_mmio_info(struct intel_gvt *gvt,
> >  })
> >  
> >  int intel_vgpu_init_mmio(struct intel_vgpu *vgpu);
> > -void intel_vgpu_reset_mmio(struct intel_vgpu *vgpu);
> > +void intel_vgpu_reset_mmio(struct intel_vgpu *vgpu, bool dmlr);
> >  void intel_vgpu_clean_mmio(struct intel_vgpu *vgpu);
> >  
> >  int intel_vgpu_gpa_to_mmio_offset(struct intel_vgpu *vgpu, u64 gpa);
> > diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
> > index 649ef28..d1edebc6 100644
> > --- a/drivers/gpu/drm/i915/gvt/vgpu.c
> > +++ b/drivers/gpu/drm/i915/gvt/vgpu.c
> > @@ -428,13 +428,17 @@ void intel_gvt_reset_vgpu_locked(struct intel_vgpu *vgpu, bool dmlr,
> >  
> >  	/* full GPU reset or device model level reset */
> >  	if (engine_mask == ALL_ENGINES || dmlr) {
> > +
> >  		intel_vgpu_reset_gtt(vgpu, dmlr);
> > -		intel_vgpu_reset_resource(vgpu);
> > -		intel_vgpu_reset_mmio(vgpu);
> > -		populate_pvinfo_page(vgpu);
> > -		intel_vgpu_reset_display(vgpu);
> > +
> > +		if (dmlr)
> > +			intel_vgpu_reset_resource(vgpu);
> > +
> > +		intel_vgpu_reset_mmio(vgpu, dmlr);
> >  
> >  		if (dmlr) {
> > +			populate_pvinfo_page(vgpu);
> > +			intel_vgpu_reset_display(vgpu);
> >  			intel_vgpu_reset_cfg_space(vgpu);
> >  			/* only reset the failsafe mode when dmlr reset */
> >  			vgpu->failsafe = false;

We need detail description to comment on this.

And looks pvinfo populate is kind of confusion, maybe can change
intel_vgpu_reset_gtt() to return a flag to say if need to reset pvinfo
or not, although currently this is to keep current behavior.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20170523/d9e19476/attachment.sig>