[3/7] drm/i915: Always enable execlists on BDW for vgpu
diff mbox

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

Commit Message

Zhiyuan Lv Aug. 20, 2015, 7:45 a.m. UTC
Broadwell hardware supports both ring buffer mode and execlist mode.
When i915 runs inside a VM with Intel GVT-g, we allow execlist mode
only. The reason is that GVT-g does not support the dynamic mode
switch between ring buffer mode and execlist mode when running
multiple virtual machines. Consider that ring buffer mode is legacy
mode, it makes sense to drop it inside virtual machines.

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

Comments

Chris Wilson Aug. 20, 2015, 8:34 a.m. UTC | #1
On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> Broadwell hardware supports both ring buffer mode and execlist mode.
> When i915 runs inside a VM with Intel GVT-g, we allow execlist mode
> only. The reason is that GVT-g does not support the dynamic mode
> switch between ring buffer mode and execlist mode when running
> multiple virtual machines. Consider that ring buffer mode is legacy
> mode, it makes sense to drop it inside virtual machines.

If that is the case, you should query the host as to what mode it is
running.
-Chris
Zhiyuan Lv Aug. 20, 2015, 8:55 a.m. UTC | #2
Hi Chris,

On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > Broadwell hardware supports both ring buffer mode and execlist mode.
> > When i915 runs inside a VM with Intel GVT-g, we allow execlist mode
> > only. The reason is that GVT-g does not support the dynamic mode
> > switch between ring buffer mode and execlist mode when running
> > multiple virtual machines. Consider that ring buffer mode is legacy
> > mode, it makes sense to drop it inside virtual machines.
> 
> If that is the case, you should query the host as to what mode it is
> running.

Thanks for the reply! You mean we query the host mode, then tell the
guest driver inside VM, so that it could use the same mode as host
right? That might be a little complicated, and the only benefit is to
support legacy ring buffer mode ...

And GVT-g host implementation was not well tested with ring buffer mode
since it cannot start windows VM. Probably I should change the commit
message to say explicitly that ring buffer mode is not supported with
GVT-g? Thanks!

Regards,
-Zhiyuan

> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
Chris Wilson Aug. 20, 2015, 9:22 a.m. UTC | #3
On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> Hi Chris,
> 
> On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > Broadwell hardware supports both ring buffer mode and execlist mode.
> > > When i915 runs inside a VM with Intel GVT-g, we allow execlist mode
> > > only. The reason is that GVT-g does not support the dynamic mode
> > > switch between ring buffer mode and execlist mode when running
> > > multiple virtual machines. Consider that ring buffer mode is legacy
> > > mode, it makes sense to drop it inside virtual machines.
> > 
> > If that is the case, you should query the host as to what mode it is
> > running.
> 
> Thanks for the reply! You mean we query the host mode, then tell the
> guest driver inside VM, so that it could use the same mode as host
> right? That might be a little complicated, and the only benefit is to
> support legacy ring buffer mode ...

The only benefit being that the guest works no matter what the host
does?
-Chris
Zhiyuan Lv Aug. 20, 2015, 9:40 a.m. UTC | #4
On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote:
> On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> > Hi Chris,
> > 
> > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > > Broadwell hardware supports both ring buffer mode and execlist mode.
> > > > When i915 runs inside a VM with Intel GVT-g, we allow execlist mode
> > > > only. The reason is that GVT-g does not support the dynamic mode
> > > > switch between ring buffer mode and execlist mode when running
> > > > multiple virtual machines. Consider that ring buffer mode is legacy
> > > > mode, it makes sense to drop it inside virtual machines.
> > > 
> > > If that is the case, you should query the host as to what mode it is
> > > running.
> > 
> > Thanks for the reply! You mean we query the host mode, then tell the
> > guest driver inside VM, so that it could use the same mode as host
> > right? That might be a little complicated, and the only benefit is to
> > support legacy ring buffer mode ...
> 
> The only benefit being that the guest works no matter what the host
> does?

Supporting ring buffer mode may need more work in GVT-g. When we started to
enable BDW support, ring buffer mode used to work but was not well tested. And
the inter-VM switch (all in ringbuffer mode) may be tricker comparing with
driver context switch with "MI_SET_CONTEXT", because we need to switch ring
buffer whereas driver does not. Regarding this, the EXECLIST mode looks
cleaner. In order to support that, we may have to: 1, change more LRI commands
to MMIO in current driver; 2, more testing/debugging of inter-VM context
switch flow.

Based on that, I think we should really make statement that "ring buffer mode"
is not supported by GVT-g on BDW :-)

> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Joonas Lahtinen Aug. 20, 2015, 11:23 a.m. UTC | #5
Hi,

On to, 2015-08-20 at 17:40 +0800, Zhiyuan Lv wrote:
> On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote:
> > On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> > > Hi Chris,
> > > 
> > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > > > Broadwell hardware supports both ring buffer mode and 
> > > > > execlist mode.
> > > > > When i915 runs inside a VM with Intel GVT-g, we allow 
> > > > > execlist mode
> > > > > only. The reason is that GVT-g does not support the dynamic 
> > > > > mode
> > > > > switch between ring buffer mode and execlist mode when 
> > > > > running
> > > > > multiple virtual machines. Consider that ring buffer mode is 
> > > > > legacy
> > > > > mode, it makes sense to drop it inside virtual machines.
> > > > 
> > > > If that is the case, you should query the host as to what mode 
> > > > it is
> > > > running.
> > > 
> > > Thanks for the reply! You mean we query the host mode, then tell 
> > > the
> > > guest driver inside VM, so that it could use the same mode as 
> > > host
> > > right? That might be a little complicated, and the only benefit 
> > > is to
> > > support legacy ring buffer mode ...
> > 
> > The only benefit being that the guest works no matter what the host
> > does?
> 
> Supporting ring buffer mode may need more work in GVT-g. When we 
> started to
> enable BDW support, ring buffer mode used to work but was not well 
> tested. And
> the inter-VM switch (all in ringbuffer mode) may be tricker comparing 
> with
> driver context switch with "MI_SET_CONTEXT", because we need to 
> switch ring
> buffer whereas driver does not. Regarding this, the EXECLIST mode 
> looks
> cleaner. In order to support that, we may have to: 1, change more LRI 
> commands
> to MMIO in current driver; 2, more testing/debugging of inter-VM 
> context
> switch flow.
> 
> Based on that, I think we should really make statement that "ring 
> buffer mode"
> is not supported by GVT-g on BDW :-)
> 

I think just move the vpgu test even before test for GEN9 just making
it return true on intel_vgpu_active(dev) and add a comment that
currently vGPU only supports execlist command submission, and then add
an error early in the init in some appropriate spot if
intel_vgpu_active is true but logical ring context are not available
(which would practically mean GEN < 8).

Does that sound OK?

Regards, Joonas

> > -Chris
> >
Zhiyuan Lv Aug. 21, 2015, 2:24 a.m. UTC | #6
Hi Joonas,

Thanks for the review! And my reply inline.

Regards,
-Zhiyuan

On Thu, Aug 20, 2015 at 02:23:11PM +0300, Joonas Lahtinen wrote:
> Hi,
> 
> On to, 2015-08-20 at 17:40 +0800, Zhiyuan Lv wrote:
> > On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote:
> > > On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> > > > Hi Chris,
> > > > 
> > > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > > > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > > > > Broadwell hardware supports both ring buffer mode and 
> > > > > > execlist mode.
> > > > > > When i915 runs inside a VM with Intel GVT-g, we allow 
> > > > > > execlist mode
> > > > > > only. The reason is that GVT-g does not support the dynamic 
> > > > > > mode
> > > > > > switch between ring buffer mode and execlist mode when 
> > > > > > running
> > > > > > multiple virtual machines. Consider that ring buffer mode is 
> > > > > > legacy
> > > > > > mode, it makes sense to drop it inside virtual machines.
> > > > > 
> > > > > If that is the case, you should query the host as to what mode 
> > > > > it is
> > > > > running.
> > > > 
> > > > Thanks for the reply! You mean we query the host mode, then tell 
> > > > the
> > > > guest driver inside VM, so that it could use the same mode as 
> > > > host
> > > > right? That might be a little complicated, and the only benefit 
> > > > is to
> > > > support legacy ring buffer mode ...
> > > 
> > > The only benefit being that the guest works no matter what the host
> > > does?
> > 
> > Supporting ring buffer mode may need more work in GVT-g. When we 
> > started to
> > enable BDW support, ring buffer mode used to work but was not well 
> > tested. And
> > the inter-VM switch (all in ringbuffer mode) may be tricker comparing 
> > with
> > driver context switch with "MI_SET_CONTEXT", because we need to 
> > switch ring
> > buffer whereas driver does not. Regarding this, the EXECLIST mode 
> > looks
> > cleaner. In order to support that, we may have to: 1, change more LRI 
> > commands
> > to MMIO in current driver; 2, more testing/debugging of inter-VM 
> > context
> > switch flow.
> > 
> > Based on that, I think we should really make statement that "ring 
> > buffer mode"
> > is not supported by GVT-g on BDW :-)
> > 
> 
> I think just move the vpgu test even before test for GEN9 just making
> it return true on intel_vgpu_active(dev) and add a comment that
> currently vGPU only supports execlist command submission, and then add

Will do. Thanks!

> an error early in the init in some appropriate spot if
> intel_vgpu_active is true but logical ring context are not available
> (which would practically mean GEN < 8).
> 
> Does that sound OK?

Ring buffer mode for Haswell with vgpu is still supported. So probably I have
the check like below? Thanks!

@@ -332,6 +332,12 @@ int i915_gem_context_init(struct drm_device *dev)
        if (WARN_ON(dev_priv->ring[RCS].default_context))
                return 0;
 
+       if (intel_vgpu_active(dev)) {
+               if (WARN_ON(HAS_LOGICAL_RING_CONTEXTS(dev) &&
+                           !i915.enable_execlist))
+                       return 0;
+       }
+
        if (i915.enable_execlists) {
                /* NB: intentionally left blank. We will allocate our own
                 * backing objects as we need them, thank you very much */

> 
> Regards, Joonas
> 
> > > -Chris
> > >
Tian, Kevin Aug. 21, 2015, 5:37 a.m. UTC | #7
> From: iGVT-g [mailto:igvt-g-bounces@lists.01.org] On Behalf Of Zhiyuan Lv
> Sent: Thursday, August 20, 2015 5:40 PM
> 
> On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote:
> > On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> > > Hi Chris,
> > >
> > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > > > Broadwell hardware supports both ring buffer mode and execlist mode.
> > > > > When i915 runs inside a VM with Intel GVT-g, we allow execlist mode
> > > > > only. The reason is that GVT-g does not support the dynamic mode
> > > > > switch between ring buffer mode and execlist mode when running
> > > > > multiple virtual machines. Consider that ring buffer mode is legacy
> > > > > mode, it makes sense to drop it inside virtual machines.
> > > >
> > > > If that is the case, you should query the host as to what mode it is
> > > > running.
> > >
> > > Thanks for the reply! You mean we query the host mode, then tell the
> > > guest driver inside VM, so that it could use the same mode as host
> > > right? That might be a little complicated, and the only benefit is to
> > > support legacy ring buffer mode ...
> >
> > The only benefit being that the guest works no matter what the host
> > does?
> 
> Supporting ring buffer mode may need more work in GVT-g. When we started to
> enable BDW support, ring buffer mode used to work but was not well tested. And
> the inter-VM switch (all in ringbuffer mode) may be tricker comparing with
> driver context switch with "MI_SET_CONTEXT", because we need to switch ring
> buffer whereas driver does not. Regarding this, the EXECLIST mode looks
> cleaner. In order to support that, we may have to: 1, change more LRI commands
> to MMIO in current driver; 2, more testing/debugging of inter-VM context
> switch flow.
> 
> Based on that, I think we should really make statement that "ring buffer mode"
> is not supported by GVT-g on BDW :-)
> 

I think the primary reason is that Windows gfx driver only uses execution
list mode. They don't have plan to add back ring buffer mode. So we
have to assume only execution list for all guests powered by GVT-g (being
that dynamic mode switch is not feasible).

Thanks
Kevin
Joonas Lahtinen Aug. 24, 2015, 12:36 p.m. UTC | #8
On pe, 2015-08-21 at 10:24 +0800, Zhiyuan Lv wrote:
> Hi Joonas,
> 
> Thanks for the review! And my reply inline.
> 
> Regards,
> -Zhiyuan
> 
> On Thu, Aug 20, 2015 at 02:23:11PM +0300, Joonas Lahtinen wrote:
> > Hi,
> > 
> > On to, 2015-08-20 at 17:40 +0800, Zhiyuan Lv wrote:
> > > On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote:
> > > > On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> > > > > Hi Chris,
> > > > > 
> > > > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > > > > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > > > > > Broadwell hardware supports both ring buffer mode and 
> > > > > > > execlist mode.
> > > > > > > When i915 runs inside a VM with Intel GVT-g, we allow 
> > > > > > > execlist mode
> > > > > > > only. The reason is that GVT-g does not support the 
> > > > > > > dynamic 
> > > > > > > mode
> > > > > > > switch between ring buffer mode and execlist mode when 
> > > > > > > running
> > > > > > > multiple virtual machines. Consider that ring buffer mode 
> > > > > > > is 
> > > > > > > legacy
> > > > > > > mode, it makes sense to drop it inside virtual machines.
> > > > > > 
> > > > > > If that is the case, you should query the host as to what 
> > > > > > mode 
> > > > > > it is
> > > > > > running.
> > > > > 
> > > > > Thanks for the reply! You mean we query the host mode, then 
> > > > > tell 
> > > > > the
> > > > > guest driver inside VM, so that it could use the same mode as 
> > > > > host
> > > > > right? That might be a little complicated, and the only 
> > > > > benefit 
> > > > > is to
> > > > > support legacy ring buffer mode ...
> > > > 
> > > > The only benefit being that the guest works no matter what the 
> > > > host
> > > > does?
> > > 
> > > Supporting ring buffer mode may need more work in GVT-g. When we 
> > > started to
> > > enable BDW support, ring buffer mode used to work but was not 
> > > well 
> > > tested. And
> > > the inter-VM switch (all in ringbuffer mode) may be tricker 
> > > comparing 
> > > with
> > > driver context switch with "MI_SET_CONTEXT", because we need to 
> > > switch ring
> > > buffer whereas driver does not. Regarding this, the EXECLIST mode 
> > > looks
> > > cleaner. In order to support that, we may have to: 1, change more 
> > > LRI 
> > > commands
> > > to MMIO in current driver; 2, more testing/debugging of inter-VM 
> > > context
> > > switch flow.
> > > 
> > > Based on that, I think we should really make statement that "ring 
> > > buffer mode"
> > > is not supported by GVT-g on BDW :-)
> > > 
> > 
> > I think just move the vpgu test even before test for GEN9 just 
> > making
> > it return true on intel_vgpu_active(dev) and add a comment that
> > currently vGPU only supports execlist command submission, and then 
> > add
> 
> Will do. Thanks!
> 
> > an error early in the init in some appropriate spot if
> > intel_vgpu_active is true but logical ring context are not 
> > available
> > (which would practically mean GEN < 8).
> > 
> > Does that sound OK?
> 
> Ring buffer mode for Haswell with vgpu is still supported. So 
> probably I have
> the check like below? Thanks!
> 

Ah sorry, I missed that.

> @@ -332,6 +332,12 @@ int i915_gem_context_init(struct drm_device 
> *dev)
>         if (WARN_ON(dev_priv->ring[RCS].default_context))
>                 return 0;
>  
> +       if (intel_vgpu_active(dev)) {
> +               if (WARN_ON(HAS_LOGICAL_RING_CONTEXTS(dev) &&
> +                           !i915.enable_execlist))
> +                       return 0;
> +       }
> +

This looks fine to me. Maybe comment might be in place stating that
support is not yet implemented, but could be.

Regards, Joonas

>         if (i915.enable_execlists) {
>                 /* NB: intentionally left blank. We will allocate our 
> own
>                  * backing objects as we need them, thank you very 
> much */
> 
> > 
> > Regards, Joonas
> > 
> > > > -Chris
> > > >
Daniel Vetter Aug. 26, 2015, 8:50 a.m. UTC | #9
On Mon, Aug 24, 2015 at 03:36:46PM +0300, Joonas Lahtinen wrote:
> On pe, 2015-08-21 at 10:24 +0800, Zhiyuan Lv wrote:
> > Hi Joonas,
> > 
> > Thanks for the review! And my reply inline.
> > 
> > Regards,
> > -Zhiyuan
> > 
> > On Thu, Aug 20, 2015 at 02:23:11PM +0300, Joonas Lahtinen wrote:
> > > Hi,
> > > 
> > > On to, 2015-08-20 at 17:40 +0800, Zhiyuan Lv wrote:
> > > > On Thu, Aug 20, 2015 at 10:22:37AM +0100, Chris Wilson wrote:
> > > > > On Thu, Aug 20, 2015 at 04:55:08PM +0800, Zhiyuan Lv wrote:
> > > > > > Hi Chris,
> > > > > > 
> > > > > > On Thu, Aug 20, 2015 at 09:34:05AM +0100, Chris Wilson wrote:
> > > > > > > On Thu, Aug 20, 2015 at 03:45:20PM +0800, Zhiyuan Lv wrote:
> > > > > > > > Broadwell hardware supports both ring buffer mode and 
> > > > > > > > execlist mode.
> > > > > > > > When i915 runs inside a VM with Intel GVT-g, we allow 
> > > > > > > > execlist mode
> > > > > > > > only. The reason is that GVT-g does not support the 
> > > > > > > > dynamic 
> > > > > > > > mode
> > > > > > > > switch between ring buffer mode and execlist mode when 
> > > > > > > > running
> > > > > > > > multiple virtual machines. Consider that ring buffer mode 
> > > > > > > > is 
> > > > > > > > legacy
> > > > > > > > mode, it makes sense to drop it inside virtual machines.
> > > > > > > 
> > > > > > > If that is the case, you should query the host as to what 
> > > > > > > mode 
> > > > > > > it is
> > > > > > > running.
> > > > > > 
> > > > > > Thanks for the reply! You mean we query the host mode, then 
> > > > > > tell 
> > > > > > the
> > > > > > guest driver inside VM, so that it could use the same mode as 
> > > > > > host
> > > > > > right? That might be a little complicated, and the only 
> > > > > > benefit 
> > > > > > is to
> > > > > > support legacy ring buffer mode ...
> > > > > 
> > > > > The only benefit being that the guest works no matter what the 
> > > > > host
> > > > > does?
> > > > 
> > > > Supporting ring buffer mode may need more work in GVT-g. When we 
> > > > started to
> > > > enable BDW support, ring buffer mode used to work but was not 
> > > > well 
> > > > tested. And
> > > > the inter-VM switch (all in ringbuffer mode) may be tricker 
> > > > comparing 
> > > > with
> > > > driver context switch with "MI_SET_CONTEXT", because we need to 
> > > > switch ring
> > > > buffer whereas driver does not. Regarding this, the EXECLIST mode 
> > > > looks
> > > > cleaner. In order to support that, we may have to: 1, change more 
> > > > LRI 
> > > > commands
> > > > to MMIO in current driver; 2, more testing/debugging of inter-VM 
> > > > context
> > > > switch flow.
> > > > 
> > > > Based on that, I think we should really make statement that "ring 
> > > > buffer mode"
> > > > is not supported by GVT-g on BDW :-)
> > > > 
> > > 
> > > I think just move the vpgu test even before test for GEN9 just 
> > > making
> > > it return true on intel_vgpu_active(dev) and add a comment that
> > > currently vGPU only supports execlist command submission, and then 
> > > add
> > 
> > Will do. Thanks!
> > 
> > > an error early in the init in some appropriate spot if
> > > intel_vgpu_active is true but logical ring context are not 
> > > available
> > > (which would practically mean GEN < 8).
> > > 
> > > Does that sound OK?
> > 
> > Ring buffer mode for Haswell with vgpu is still supported. So 
> > probably I have
> > the check like below? Thanks!
> > 
> 
> Ah sorry, I missed that.
> 
> > @@ -332,6 +332,12 @@ int i915_gem_context_init(struct drm_device 
> > *dev)
> >         if (WARN_ON(dev_priv->ring[RCS].default_context))
> >                 return 0;
> >  
> > +       if (intel_vgpu_active(dev)) {
> > +               if (WARN_ON(HAS_LOGICAL_RING_CONTEXTS(dev) &&
> > +                           !i915.enable_execlist))
> > +                       return 0;
> > +       }
> > +
> 
> This looks fine to me. Maybe comment might be in place stating that
> support is not yet implemented, but could be.

You should fail this instead so that i915.ko knows that the render side of
the gpu doesn't work. And maybe just DRM_INFO with a useful informational
notice?

Also same comment here: Don't we need to coordinate this a bit better with
the host?
-Daniel
Zhiyuan Lv Aug. 27, 2015, 2:49 a.m. UTC | #10
Hi Daniel,

On Wed, Aug 26, 2015 at 10:50:23AM +0200, Daniel Vetter wrote:
> > > @@ -332,6 +332,12 @@ int i915_gem_context_init(struct drm_device 
> > > *dev)
> > >         if (WARN_ON(dev_priv->ring[RCS].default_context))
> > >                 return 0;
> > >  
> > > +       if (intel_vgpu_active(dev)) {
> > > +               if (WARN_ON(HAS_LOGICAL_RING_CONTEXTS(dev) &&
> > > +                           !i915.enable_execlist))
> > > +                       return 0;
> > > +       }
> > > +
> > 
> > This looks fine to me. Maybe comment might be in place stating that
> > support is not yet implemented, but could be.
> 
> You should fail this instead so that i915.ko knows that the render side of
> the gpu doesn't work. And maybe just DRM_INFO with a useful informational
> notice?

Thanks for the comments! Will change.

> 
> Also same comment here: Don't we need to coordinate this a bit better with
> the host?

In host side, if driver is running in ring buffer mode, we will let GVT-g
initialization fail, so that guest cannot set vgpu active. Would that be good
enough? Thanks!

> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Daniel Vetter Sept. 2, 2015, 8:06 a.m. UTC | #11
On Thu, Aug 27, 2015 at 10:49:28AM +0800, Zhiyuan Lv wrote:
> Hi Daniel,
> 
> On Wed, Aug 26, 2015 at 10:50:23AM +0200, Daniel Vetter wrote:
> > > > @@ -332,6 +332,12 @@ int i915_gem_context_init(struct drm_device 
> > > > *dev)
> > > >         if (WARN_ON(dev_priv->ring[RCS].default_context))
> > > >                 return 0;
> > > >  
> > > > +       if (intel_vgpu_active(dev)) {
> > > > +               if (WARN_ON(HAS_LOGICAL_RING_CONTEXTS(dev) &&
> > > > +                           !i915.enable_execlist))
> > > > +                       return 0;
> > > > +       }
> > > > +
> > > 
> > > This looks fine to me. Maybe comment might be in place stating that
> > > support is not yet implemented, but could be.
> > 
> > You should fail this instead so that i915.ko knows that the render side of
> > the gpu doesn't work. And maybe just DRM_INFO with a useful informational
> > notice?
> 
> Thanks for the comments! Will change.
> 
> > 
> > Also same comment here: Don't we need to coordinate this a bit better with
> > the host?
> 
> In host side, if driver is running in ring buffer mode, we will let GVT-g
> initialization fail, so that guest cannot set vgpu active. Would that be good
> enough? Thanks!

Yeah that seems sensible.
-Daniel

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2dc8709..39df304 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -239,6 +239,9 @@  int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists
 	if (INTEL_INFO(dev)->gen >= 9)
 		return 1;
 
+	if (HAS_LOGICAL_RING_CONTEXTS(dev) && intel_vgpu_active(dev))
+		return 1;
+
 	if (enable_execlists == 0)
 		return 0;