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
> > >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
