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

[Li, Weinan Z]

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 
Cc: Chris Wilson 




@@ -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)
   >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


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Joonas Lahtinen]

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 
> > > Cc: Chris Wilson 
> > 
> > 
> > 
> > > @@ -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)
> > >   
> > > >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
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Chris Wilson]

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 
> > Cc: Chris Wilson 
> 
> 
> 
> > @@ -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)
> >   
> > >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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Joonas Lahtinen]

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 
> Cc: Chris Wilson 



> @@ -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)
>   >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
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Weinan Li]

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 
Cc: Chris Wilson 
---
 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(-)

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)
>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)));
-- 
1.9.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx