[PATCH v2] drm/i915/gvt: Add runtime_pm get/put to proctect MMIO accessing

[Zhenyu Wang]

On 2017.05.27 13:27:33 +0800, Chuanxiao Dong wrote:
> In some cases, GVT-g is accessing MMIO without holding runtime_pm
> and this patch can add the runtime_pm get/put to make sure when
> accessing MMIO the i915 HW is really powered on.
> 
> v2:
> - make the runtime pm protection smaller granularity. (Zhenyu)
> 
> Cc: Zhenyu Wang <zhenyuw at linux.intel.com>
> Signed-off-by: Chuanxiao Dong <chuanxiao.dong at intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/gtt.c      |  2 ++
>  drivers/gpu/drm/i915/gvt/handlers.c | 11 +++++++++++
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
> index c6f0077..13bfe09 100644
> --- a/drivers/gpu/drm/i915/gvt/gtt.c
> +++ b/drivers/gpu/drm/i915/gvt/gtt.c
> @@ -251,8 +251,10 @@ static void write_pte64(struct drm_i915_private *dev_priv,
>  
>  	writeq(pte, addr);
>  
> +	intel_runtime_pm_get(dev_priv);
>  	I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
>  	POSTING_READ(GFX_FLSH_CNTL_GEN6);
> +	intel_runtime_pm_put(dev_priv);
>  }

Could we handle this invalidate explicitly instead of hiding in write pte?
And invalidate/flush only after one set of ggtt operations end? otherwise
I'm afraid this is overkill. And we can remove the posting read as well.

>  
>  static inline struct intel_gvt_gtt_entry *gtt_get_entry64(void *pt,
> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
> index 7a28f46..3c8086d 100644
> --- a/drivers/gpu/drm/i915/gvt/handlers.c
> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
> @@ -209,6 +209,7 @@ static int fence_mmio_read(struct intel_vgpu *vgpu, unsigned int off,
>  static int fence_mmio_write(struct intel_vgpu *vgpu, unsigned int off,
>  		void *p_data, unsigned int bytes)
>  {
> +	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
>  	unsigned int fence_num = offset_to_fence_num(off);
>  	int ret;
>  
> @@ -217,8 +218,10 @@ static int fence_mmio_write(struct intel_vgpu *vgpu, unsigned int off,
>  		return ret;
>  	write_vreg(vgpu, off, p_data, bytes);
>  
> +	intel_runtime_pm_get(dev_priv);
>  	intel_vgpu_write_fence(vgpu, fence_num,
>  			vgpu_vreg64(vgpu, fence_num_to_offset(fence_num)));
> +	intel_runtime_pm_put(dev_priv);
>  	return 0;
>  }

Better like to have something like mmio_hw_access_pre/post() to cover this
as macro or inline function.

>  
> @@ -1265,7 +1268,9 @@ static int gen9_trtte_write(struct intel_vgpu *vgpu, unsigned int offset,
>  	}
>  	write_vreg(vgpu, offset, p_data, bytes);
>  	/* TRTTE is not per-context */
> +	intel_runtime_pm_get(dev_priv);
>  	I915_WRITE(_MMIO(offset), vgpu_vreg(vgpu, offset));
> +	intel_runtime_pm_put(dev_priv);
>  
>  	return 0;
>  }
> @@ -1278,7 +1283,9 @@ static int gen9_trtt_chicken_write(struct intel_vgpu *vgpu, unsigned int offset,
>  
>  	if (val & 1) {
>  		/* unblock hw logic */
> +		intel_runtime_pm_get(dev_priv);
>  		I915_WRITE(_MMIO(offset), val);
> +		intel_runtime_pm_put(dev_priv);
>  	}
>  	write_vreg(vgpu, offset, p_data, bytes);
>  	return 0;
> @@ -1415,7 +1422,9 @@ static int ring_timestamp_mmio_read(struct intel_vgpu *vgpu,
>  {
>  	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
>  
> +	intel_runtime_pm_get(dev_priv);
>  	vgpu_vreg(vgpu, offset) = I915_READ(_MMIO(offset));
> +	intel_runtime_pm_put(dev_priv);
>  	return intel_vgpu_default_mmio_read(vgpu, offset, p_data, bytes);
>  }
>  
> @@ -1424,7 +1433,9 @@ static int instdone_mmio_read(struct intel_vgpu *vgpu,
>  {
>  	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
>  
> +	intel_runtime_pm_get(dev_priv);
>  	vgpu_vreg(vgpu, offset) = I915_READ(_MMIO(offset));
> +	intel_runtime_pm_put(dev_priv);
>  	return intel_vgpu_default_mmio_read(vgpu, offset, p_data, bytes);
>  }
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at 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
-------------- 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/20170531/b3016747/attachment-0001.sig>