[v2,2/2] drm/i915: enable to read CSB and CSB write pointer from HWSP in GVT-g VM

Submitted by Weinan Li on Sept. 30, 2017, 5:57 a.m.

Details

Message ID 1506751032-7250-3-git-send-email-weinan.z.li@intel.com
State New
Headers show
Series "enable virtual HWSP in GVT-g" ( rev: 1 ) in Intel GFX

Not browsing as part of any series.

Commit Message

Weinan Li Sept. 30, 2017, 5:57 a.m.
Let GVT-g VM read the CSB and CSB write pointer from virtual HWSP, not all
the host support this feature, need to check the BIT(3) of caps in PVINFO.

Signed-off-by: Weinan Li <weinan.z.li@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_vgpu.c       |  5 +++++
 drivers/gpu/drm/i915/i915_vgpu.h       |  1 +
 drivers/gpu/drm/i915/intel_engine_cs.c | 11 +++++++----
 drivers/gpu/drm/i915/intel_lrc.c       |  7 ++++++-
 4 files changed, 19 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 5fe9f3f..6f713c5 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -86,6 +86,11 @@  bool intel_vgpu_has_full_48bit_ppgtt(struct drm_i915_private *dev_priv)
 	return dev_priv->vgpu.caps & VGT_CAPS_FULL_48BIT_PPGTT;
 }
 
+bool intel_vgpu_has_hwsp_emulation(struct drm_i915_private *dev_priv)
+{
+	return dev_priv->vgpu.caps & VGT_CAPS_HWSP_EMULATION;
+}
+
 struct _balloon_info_ {
 	/*
 	 * There are up to 2 regions per mappable/unmappable graphic
diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
index b72bd29..cec0ec1 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.h
+++ b/drivers/gpu/drm/i915/i915_vgpu.h
@@ -29,6 +29,7 @@ 
 void i915_check_vgpu(struct drm_i915_private *dev_priv);
 
 bool intel_vgpu_has_full_48bit_ppgtt(struct drm_i915_private *dev_priv);
+bool intel_vgpu_has_hwsp_emulation(struct drm_i915_private *dev_priv);
 
 int intel_vgt_balloon(struct drm_i915_private *dev_priv);
 void intel_vgt_deballoon(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index a28e2a8..58945ef 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -23,6 +23,7 @@ 
  */
 
 #include "i915_drv.h"
+#include "i915_vgpu.h"
 #include "intel_ringbuffer.h"
 #include "intel_lrc.h"
 
@@ -384,10 +385,6 @@  static void intel_engine_init_timeline(struct intel_engine_cs *engine)
 
 static bool csb_force_mmio(struct drm_i915_private *i915)
 {
-	/* GVT emulation depends upon intercepting CSB mmio */
-	if (intel_vgpu_active(i915))
-		return true;
-
 	/*
 	 * IOMMU adds unpredictable latency causing the CSB write (from the
 	 * GPU into the HWSP) to only be visible some time after the interrupt
@@ -396,6 +393,12 @@  static bool csb_force_mmio(struct drm_i915_private *i915)
 	if (intel_vtd_active())
 		return true;
 
+	/* GVT emulation depends upon host kernel implementation, check
+	 * support capbility by reading PV INFO before access HWSP.
+	 */
+	if (intel_vgpu_active(i915) && !intel_vgpu_has_hwsp_emulation(i915))
+		return true;
+
 	return false;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 7d6da13..2313d0a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -722,7 +722,12 @@  static void intel_lrc_irq_handler(unsigned long data)
 			&engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
 		unsigned int head, tail;
 
-		/* However GVT emulation depends upon intercepting CSB mmio */
+		/* However GVT-g emulation depends upon host kernel
+		 * implementation, need to check support capbility by reading PV
+		 * INFO before access HWSP. Beside from this, another special
+		 * configuration may also need to force use mmio, like IOMMU
+		 * enabled.
+		 */
 		if (unlikely(execlists->csb_use_mmio)) {
 			buf = (u32 * __force)
 				(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0)));

Comments

On Sat, 2017-09-30 at 13:57 +0800, Weinan Li wrote:
> Let GVT-g VM read the CSB and CSB write pointer from virtual HWSP, not all
> the host support this feature, need to check the BIT(3) of caps in PVINFO.
> 
> Signed-off-by: Weinan Li <weinan.z.li@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> @@ -396,6 +393,12 @@ static bool csb_force_mmio(struct drm_i915_private *i915)
>  	if (intel_vtd_active())
>  		return true;
>  
> +	/* GVT emulation depends upon host kernel implementation, check
> +	 * support capbility by reading PV INFO before access HWSP.
> +	 */

The comment can be dropped completely, the code is self-descriptive.

> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -722,7 +722,12 @@ static void intel_lrc_irq_handler(unsigned long data)
>  			&engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
>  		unsigned int head, tail;
>  
> -		/* However GVT emulation depends upon intercepting CSB mmio */
> +		/* However GVT-g emulation depends upon host kernel
> +		 * implementation, need to check support capbility by reading PV
> +		 * INFO before access HWSP. Beside from this, another special
> +		 * configuration may also need to force use mmio, like IOMMU
> +		 * enabled.
> +		 */

s/capbility/capability/ and please rephrase this to be a kerneldoc for
csb_use_mmio at the declaration.

>  		if (unlikely(execlists->csb_use_mmio)) {
>  			buf = (u32 * __force)
>  				(dev_priv->regs + i915_mmio_reg_offset(RING_CONTEXT_STATUS_BUF_LO(engine, 0)));

Regards, Joonas
Quoting Joonas Lahtinen (2017-10-02 11:03:30)
> On Sat, 2017-09-30 at 13:57 +0800, Weinan Li wrote:
> > Let GVT-g VM read the CSB and CSB write pointer from virtual HWSP, not all
> > the host support this feature, need to check the BIT(3) of caps in PVINFO.
> > 
> > Signed-off-by: Weinan Li <weinan.z.li@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> 
> <SNIP>
> 
> > @@ -396,6 +393,12 @@ static bool csb_force_mmio(struct drm_i915_private *i915)
> >       if (intel_vtd_active())
> >               return true;
> >  
> > +     /* GVT emulation depends upon host kernel implementation, check
> > +      * support capbility by reading PV INFO before access HWSP.
> > +      */
> 
> The comment can be dropped completely, the code is self-descriptive.
> 
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -722,7 +722,12 @@ static void intel_lrc_irq_handler(unsigned long data)
> >                       &engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
> >               unsigned int head, tail;
> >  
> > -             /* However GVT emulation depends upon intercepting CSB mmio */
> > +             /* However GVT-g emulation depends upon host kernel
> > +              * implementation, need to check support capbility by reading PV
> > +              * INFO before access HWSP. Beside from this, another special
> > +              * configuration may also need to force use mmio, like IOMMU
> > +              * enabled.
> > +              */
> 
> s/capbility/capability/ and please rephrase this to be a kerneldoc for
> csb_use_mmio at the declaration.

This is not a description of how to use the function or even on how
csb_use_mmio work, this is why we want certain logic paths. Just a regular
old comment.
-Chris
On Mon, 2017-10-02 at 11:23 +0100, Chris Wilson wrote:
> Quoting Joonas Lahtinen (2017-10-02 11:03:30)
> > On Sat, 2017-09-30 at 13:57 +0800, Weinan Li wrote:
> > > Let GVT-g VM read the CSB and CSB write pointer from virtual HWSP, not all
> > > the host support this feature, need to check the BIT(3) of caps in PVINFO.
> > > 
> > > Signed-off-by: Weinan Li <weinan.z.li@intel.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > <SNIP>
> > 
> > > @@ -396,6 +393,12 @@ static bool csb_force_mmio(struct drm_i915_private *i915)
> > >       if (intel_vtd_active())
> > >               return true;
> > >  
> > > +     /* GVT emulation depends upon host kernel implementation, check
> > > +      * support capbility by reading PV INFO before access HWSP.
> > > +      */
> > 
> > The comment can be dropped completely, the code is self-descriptive.
> > 
> > > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > > @@ -722,7 +722,12 @@ static void intel_lrc_irq_handler(unsigned long data)
> > >                       &engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
> > >               unsigned int head, tail;
> > >  
> > > -             /* However GVT emulation depends upon intercepting CSB mmio */
> > > +             /* However GVT-g emulation depends upon host kernel
> > > +              * implementation, need to check support capbility by reading PV
> > > +              * INFO before access HWSP. Beside from this, another special
> > > +              * configuration may also need to force use mmio, like IOMMU
> > > +              * enabled.
> > > +              */
> > 
> > s/capbility/capability/ and please rephrase this to be a kerneldoc for
> > csb_use_mmio at the declaration.
> 
> This is not a description of how to use the function or even on how
> csb_use_mmio work, this is why we want certain logic paths. Just a regular
> old comment.

That's why I asked to "rephrase" :) Anyway, it seems like there already
is kerneldoc for the csb_use_mmio, so this comment can be dropped.

Regards, Joonas
On 10/2/2017 7:37 PM, Joonas Lahtinen wrote:
> On Mon, 2017-10-02 at 11:23 +0100, Chris Wilson wrote:
>> Quoting Joonas Lahtinen (2017-10-02 11:03:30)
>>> On Sat, 2017-09-30 at 13:57 +0800, Weinan Li wrote:
>>>> Let GVT-g VM read the CSB and CSB write pointer from virtual HWSP, not all
>>>> the host support this feature, need to check the BIT(3) of caps in PVINFO.
>>>>
>>>> Signed-off-by: Weinan Li <weinan.z.li@intel.com>
>>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> <SNIP>
>>>
>>>> @@ -396,6 +393,12 @@ static bool csb_force_mmio(struct drm_i915_private *i915)
>>>>        if (intel_vtd_active())
>>>>                return true;
>>>>   
>>>> +     /* GVT emulation depends upon host kernel implementation, check
>>>> +      * support capbility by reading PV INFO before access HWSP.
>>>> +      */
>>> The comment can be dropped completely, the code is self-descriptive.
will remove it in next version.
>>>
>>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>>> @@ -722,7 +722,12 @@ static void intel_lrc_irq_handler(unsigned long data)
>>>>                        &engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
>>>>                unsigned int head, tail;
>>>>   
>>>> -             /* However GVT emulation depends upon intercepting CSB mmio */
>>>> +             /* However GVT-g emulation depends upon host kernel
>>>> +              * implementation, need to check support capbility by reading PV
>>>> +              * INFO before access HWSP. Beside from this, another special
>>>> +              * configuration may also need to force use mmio, like IOMMU
>>>> +              * enabled.
>>>> +              */
>>> s/capbility/capability/ and please rephrase this to be a kerneldoc for
>>> csb_use_mmio at the declaration.
>> This is not a description of how to use the function or even on how
>> csb_use_mmio work, this is why we want certain logic paths. Just a regular
>> old comment.
> That's why I asked to "rephrase" :) Anyway, it seems like there already
> is kerneldoc for the csb_use_mmio, so this comment can be dropped.
will remove this comment in next version.
> Regards, Joonas