| Message ID | 1506751032-7250-3-git-send-email-weinan.z.li@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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)));
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(-)