[PATCH v2] drm/i915/gvt: Add runtime_pm get/put to proctect MMIO accessing
Dong, Chuanxiao
chuanxiao.dong at intel.com
Wed May 31 05:12:58 UTC 2017
> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On
> Behalf Of Zhenyu Wang
> Sent: Wednesday, May 31, 2017 12:27 PM
> To: Dong, Chuanxiao <chuanxiao.dong at intel.com>
> Cc: intel-gvt-dev at lists.freedesktop.org
> Subject: Re: [PATCH v2] drm/i915/gvt: Add runtime_pm get/put to proctect
> MMIO accessing
>
> 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.
Good suggestion. Would like to send separate patch to add invalidate/flush API to handle pte writing case.
>
> >
> > 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.
Sure. Will handle this in a new series.
Thanks
Chuanxiao
More information about the intel-gvt-dev
mailing list