[2/7] drm/i915: Enable full ppgtt for vgpu
diff mbox

Message ID 1440056724-26976-3-git-send-email-zhiyuan.lv@intel.com
State New
Headers show

Commit Message

Zhiyuan Lv Aug. 20, 2015, 7:45 a.m. UTC
The full ppgtt is supported in Intel GVT-g device model. So the
restriction can be removed.

Signed-off-by: Zhiyuan Lv <zhiyuan.lv@intel.com>
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ---
 1 file changed, 3 deletions(-)

Comments

Joonas Lahtinen Aug. 20, 2015, 10:57 a.m. UTC | #1
On to, 2015-08-20 at 15:45 +0800, Zhiyuan Lv wrote:
> The full ppgtt is supported in Intel GVT-g device model. So the
> restriction can be removed.
> 
> Signed-off-by: Zhiyuan Lv <zhiyuan.lv@intel.com>
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index ed10e77..823005c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -108,9 +108,6 @@ static int sanitize_enable_ppgtt(struct 
> drm_device *dev, int enable_ppgtt)
>  	has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6;
>  	has_full_ppgtt = INTEL_INFO(dev)->gen >= 7;
>  
> -	if (intel_vgpu_active(dev))
> -		has_full_ppgtt = false; /* emulation is too hard */
> -
>  	/*
>  	 * We don't allow disabling PPGTT for gen9+ as it's a 
> requirement for
>  	 * execlists, the sole mechanism available to submit work.
Daniel Vetter Aug. 26, 2015, 8:47 a.m. UTC | #2
On Thu, Aug 20, 2015 at 01:57:13PM +0300, Joonas Lahtinen wrote:
> On to, 2015-08-20 at 15:45 +0800, Zhiyuan Lv wrote:
> > The full ppgtt is supported in Intel GVT-g device model. So the
> > restriction can be removed.
> > 
> > Signed-off-by: Zhiyuan Lv <zhiyuan.lv@intel.com>
> > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index ed10e77..823005c 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -108,9 +108,6 @@ static int sanitize_enable_ppgtt(struct 
> > drm_device *dev, int enable_ppgtt)
> >  	has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6;
> >  	has_full_ppgtt = INTEL_INFO(dev)->gen >= 7;
> >  
> > -	if (intel_vgpu_active(dev))
> > -		has_full_ppgtt = false; /* emulation is too hard */

Don't we need a feature check for the virtual gpu here? Or at least a
platform check? Seems like the backwards/forwards compat story isn't too
thought out yet here. Note that the kernel of the host and the guest might
not be the same at all, much less the kvm part.
-Daniel

> > -
> >  	/*
> >  	 * We don't allow disabling PPGTT for gen9+ as it's a 
> > requirement for
> >  	 * execlists, the sole mechanism available to submit work.
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Zhiyuan Lv Aug. 27, 2015, 2:28 a.m. UTC | #3
Hi Danie,

On Wed, Aug 26, 2015 at 10:47:37AM +0200, Daniel Vetter wrote:
> On Thu, Aug 20, 2015 at 01:57:13PM +0300, Joonas Lahtinen wrote:
> > On to, 2015-08-20 at 15:45 +0800, Zhiyuan Lv wrote:
> > > The full ppgtt is supported in Intel GVT-g device model. So the
> > > restriction can be removed.
> > > 
> > > Signed-off-by: Zhiyuan Lv <zhiyuan.lv@intel.com>
> > > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> > 
> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > 
> > > ---
> > >  drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ---
> > >  1 file changed, 3 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> > > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index ed10e77..823005c 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -108,9 +108,6 @@ static int sanitize_enable_ppgtt(struct 
> > > drm_device *dev, int enable_ppgtt)
> > >  	has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6;
> > >  	has_full_ppgtt = INTEL_INFO(dev)->gen >= 7;
> > >  
> > > -	if (intel_vgpu_active(dev))
> > > -		has_full_ppgtt = false; /* emulation is too hard */
> 
> Don't we need a feature check for the virtual gpu here? Or at least a
> platform check? Seems like the backwards/forwards compat story isn't too
> thought out yet here. Note that the kernel of the host and the guest might
> not be the same at all, much less the kvm part.

Yeah, backwards/forwards compatibility is not considered, since we are
just to start the upstream of iGVT-g host changes. Right now if people
uses new enough iGVT-g code (off-tree now), both HSW and BDW should
work with PPGTT.

So new host with both old guest or new guest are working. The only
thing impacted is old host with new guest kernel. In order to keep it
work, I can change the code like:

+	if (intel_vgpu_active(dev))
+		has_full_ppgtt = !IS_HASWELL(dev);

Any comments? Thanks for the review!

Regards,
-Zhiyuan

> -Daniel
> 
> > > -
> > >  	/*
> > >  	 * We don't allow disabling PPGTT for gen9+ as it's a 
> > > requirement for
> > >  	 * execlists, the sole mechanism available to submit work.
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Daniel Vetter Sept. 2, 2015, 8:06 a.m. UTC | #4
On Thu, Aug 27, 2015 at 10:28:49AM +0800, Zhiyuan Lv wrote:
> Hi Danie,
> 
> On Wed, Aug 26, 2015 at 10:47:37AM +0200, Daniel Vetter wrote:
> > On Thu, Aug 20, 2015 at 01:57:13PM +0300, Joonas Lahtinen wrote:
> > > On to, 2015-08-20 at 15:45 +0800, Zhiyuan Lv wrote:
> > > > The full ppgtt is supported in Intel GVT-g device model. So the
> > > > restriction can be removed.
> > > > 
> > > > Signed-off-by: Zhiyuan Lv <zhiyuan.lv@intel.com>
> > > > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> > > 
> > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > > 
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_gem_gtt.c | 3 ---
> > > >  1 file changed, 3 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c 
> > > > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > > index ed10e77..823005c 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > > @@ -108,9 +108,6 @@ static int sanitize_enable_ppgtt(struct 
> > > > drm_device *dev, int enable_ppgtt)
> > > >  	has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6;
> > > >  	has_full_ppgtt = INTEL_INFO(dev)->gen >= 7;
> > > >  
> > > > -	if (intel_vgpu_active(dev))
> > > > -		has_full_ppgtt = false; /* emulation is too hard */
> > 
> > Don't we need a feature check for the virtual gpu here? Or at least a
> > platform check? Seems like the backwards/forwards compat story isn't too
> > thought out yet here. Note that the kernel of the host and the guest might
> > not be the same at all, much less the kvm part.
> 
> Yeah, backwards/forwards compatibility is not considered, since we are
> just to start the upstream of iGVT-g host changes. Right now if people
> uses new enough iGVT-g code (off-tree now), both HSW and BDW should
> work with PPGTT.
> 
> So new host with both old guest or new guest are working. The only
> thing impacted is old host with new guest kernel. In order to keep it
> work, I can change the code like:
> 
> +	if (intel_vgpu_active(dev))
> +		has_full_ppgtt = !IS_HASWELL(dev);
> 
> Any comments? Thanks for the review!

In the end that's your call (well until we have users screaming that a
kernel upgrade breaks their xengt setup, then you have to keep backwards
compat). But since it's a requirement from Linus Torvalds for upstream I
highly suggest you start thinking/testing for backwards compat (both on
host and guest side when the other part is older) right away. Otherwise
trying to shoehorn a good backwards compat scheme later on could be really
painful.
-Daniel

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index ed10e77..823005c 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -108,9 +108,6 @@  static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt)
 	has_aliasing_ppgtt = INTEL_INFO(dev)->gen >= 6;
 	has_full_ppgtt = INTEL_INFO(dev)->gen >= 7;
 
-	if (intel_vgpu_active(dev))
-		has_full_ppgtt = false; /* emulation is too hard */
-
 	/*
 	 * We don't allow disabling PPGTT for gen9+ as it's a requirement for
 	 * execlists, the sole mechanism available to submit work.