[PATCH] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[PATCH] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Chuanxiao Dong]

Currently GVT-g cannot work properly when host GuC submission
is enabled, so disable GVT in this case.

Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
 drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
index e1ab643..0e1ff48 100644
--- a/drivers/gpu/drm/i915/intel_gvt.c
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -84,6 +84,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
 		goto bail;
 	}
 
+	if (i915.enable_guc_submission) {
+		DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to enabled GuC submission [i915.enable_guc_submission module parameter]\n");
+		goto bail;
+	}
+
 	/*
 	 * We're not in host or fail to find a MPT module, disable GVT-g
 	 */
-- 
2.7.4

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

✓ Fi.CI.BAT: success for drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Patchwork]

== Series Details ==

Series: drm/i915/gvt: disable GVT-g if host GuC submission is enabled
URL   : https://patchwork.freedesktop.org/series/23796/
State : success

== Summary ==

Series 23796v1 drm/i915/gvt: disable GVT-g if host GuC submission is enabled
https://patchwork.freedesktop.org/api/1.0/series/23796/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-snb-2600) fdo#100215

fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:434s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:424s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:575s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:509s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:543s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:484s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:481s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:406s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:405s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:418s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:481s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:484s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:459s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:565s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:457s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:567s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:457s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:499s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:430s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:530s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time:407s

0d90375e6d38f986224e4dfce1674c5b093590cc drm-tip: 2017y-05m-02d-09h-11m-43s UTC integration manifest
94abe09 drm/i915/gvt: disable GVT-g if host GuC submission is enabled

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4591/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1099 bytes --]

On 2017.05.02 16:58:31 +0800, Chuanxiao Dong wrote:
> Currently GVT-g cannot work properly when host GuC submission
> is enabled, so disable GVT in this case.
> 
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
> index e1ab643..0e1ff48 100644
> --- a/drivers/gpu/drm/i915/intel_gvt.c
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -84,6 +84,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
>  		goto bail;
>  	}
>  
> +	if (i915.enable_guc_submission) {
> +		DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to enabled GuC submission [i915.enable_guc_submission module parameter]\n");
> +		goto bail;
> +	}
> +
>  	/*
>  	 * We're not in host or fail to find a MPT module, disable GVT-g
>  	 */
> -- 

Applied, thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Joonas Lahtinen]

+ Daniel

On ke, 2017-05-03 at 16:36 +0800, Zhenyu Wang wrote:
> On 2017.05.02 16:58:31 +0800, Chuanxiao Dong wrote:
> > 
> > Currently GVT-g cannot work properly when host GuC submission
> > is enabled, so disable GVT in this case.
> > 
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>

<SNIP>

> > @@ -84,6 +84,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
> >  		goto bail;
> >  	}
> >  
> > +	if (i915.enable_guc_submission) {
> > +		DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to enabled GuC submission [i915.enable_guc_submission module parameter]\n");

Guest module parameter is not the correct way of detetecting if host
has GuC submission enabled. And even if it was, the message is overly
verbose (and it'll be incorrect once i915.enable_guc_submission
defaults to something else than zero).

> > +		goto bail;
> > +	}
> > +
> >  	/*
> >  	 * We're not in host or fail to find a MPT module, disable GVT-g
> >  	 */
> > -- 
> 
> Applied, thanks!

The original patch should've included at least some Cc's, or wait being
merged through drm-tip as it's not int drm/i915/gvt directory at all
(unlike the message states).

The patch should be reverted for being incorrect.

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: [PATCH] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Dong, Chuanxiao]

> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Friday, May 5, 2017 4:55 PM
> To: Zhenyu Wang <zhenyuw@linux.intel.com>; Dong, Chuanxiao
> <chuanxiao.dong@intel.com>; Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: disable GVT-g if host GuC
> submission is enabled
> 
> + Daniel
> 
> On ke, 2017-05-03 at 16:36 +0800, Zhenyu Wang wrote:
> > On 2017.05.02 16:58:31 +0800, Chuanxiao Dong wrote:
> > >
> > > Currently GVT-g cannot work properly when host GuC submission is
> > > enabled, so disable GVT in this case.
> > >
> > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> 
> <SNIP>
> 
> > > @@ -84,6 +84,11 @@ int intel_gvt_init(struct drm_i915_private
> *dev_priv)
> > >  		goto bail;
> > >  	}
> > >
> > > +	if (i915.enable_guc_submission) {
> > > +		DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to
> > > +enabled GuC submission [i915.enable_guc_submission module
> > > +parameter]\n");
> 
> Guest module parameter is not the correct way of detetecting if host has
> GuC submission enabled. And even if it was, the message is overly verbose
> (and it'll be incorrect once i915.enable_guc_submission defaults to
> something else than zero).

Hi Joonas,

How about just to say " DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to enabled GuC submission\n");"

Thanks
Chuanxiao

> 
> > > +		goto bail;
> > > +	}
> > > +
> > >  	/*
> > >  	 * We're not in host or fail to find a MPT module, disable GVT-g
> > >  	 */
> > > --
> >
> > Applied, thanks!
> 
> The original patch should've included at least some Cc's, or wait being
> merged through drm-tip as it's not int drm/i915/gvt directory at all (unlike
> the message states).
> 
> The patch should be reverted for being incorrect.
> 
> 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: [PATCH] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Chris Wilson]

On Fri, May 05, 2017 at 11:55:14AM +0300, Joonas Lahtinen wrote:
> + Daniel
> 
> On ke, 2017-05-03 at 16:36 +0800, Zhenyu Wang wrote:
> > On 2017.05.02 16:58:31 +0800, Chuanxiao Dong wrote:
> > > 
> > > Currently GVT-g cannot work properly when host GuC submission
> > > is enabled, so disable GVT in this case.
> > > 
> > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> 
> <SNIP>
> 
> > > @@ -84,6 +84,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
> > >  		goto bail;
> > >  	}
> > >  
> > > +	if (i915.enable_guc_submission) {
> > > +		DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to enabled GuC submission [i915.enable_guc_submission module parameter]\n");
> 
> Guest module parameter is not the correct way of detetecting if host
> has GuC submission enabled. And even if it was, the message is overly
> verbose (and it'll be incorrect once i915.enable_guc_submission
> defaults to something else than zero).

It needs to be verbose because it is a message to the user, it should
tell them what broke, what that will likely mean to them and if possible
how to rectify. Even then we know they won't read the entirety of the
message but at least it gives us a starting point for the inevitable
bugs. We should always aim for clarity and avoid too much jargon in
DRM_INFO+.

In this case, you could argue that i915.enable_guc_submission is an
unsafe parameter set to off by default and so the combination of gvt +
guc is pure user error and they can keep both pieces.

Ideally we wouldn't use i915.enable_guc_submission at all, but gvt
should be disabled upon enabling guc -- since the combination is currently
inoperable. But again, this is just a user error and we can just -EIO
the driver load...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Dong, Chuanxiao]

> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Friday, May 5, 2017 5:11 PM
> To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>; Dong, Chuanxiao
> <chuanxiao.dong@intel.com>; Daniel Vetter <daniel.vetter@ffwll.ch>; intel-
> gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: disable GVT-g if host GuC
> submission is enabled
> 
> On Fri, May 05, 2017 at 11:55:14AM +0300, Joonas Lahtinen wrote:
> > + Daniel
> >
> > On ke, 2017-05-03 at 16:36 +0800, Zhenyu Wang wrote:
> > > On 2017.05.02 16:58:31 +0800, Chuanxiao Dong wrote:
> > > >
> > > > Currently GVT-g cannot work properly when host GuC submission is
> > > > enabled, so disable GVT in this case.
> > > >
> > > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> >
> > <SNIP>
> >
> > > > @@ -84,6 +84,11 @@ int intel_gvt_init(struct drm_i915_private
> *dev_priv)
> > > >  		goto bail;
> > > >  	}
> > > >
> > > > +	if (i915.enable_guc_submission) {
> > > > +		DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to
> > > > +enabled GuC submission [i915.enable_guc_submission module
> > > > +parameter]\n");
> >
> > Guest module parameter is not the correct way of detetecting if host
> > has GuC submission enabled. And even if it was, the message is overly
> > verbose (and it'll be incorrect once i915.enable_guc_submission
> > defaults to something else than zero).
> 
> It needs to be verbose because it is a message to the user, it should tell them
> what broke, what that will likely mean to them and if possible how to rectify.
> Even then we know they won't read the entirety of the message but at least
> it gives us a starting point for the inevitable bugs. We should always aim for
> clarity and avoid too much jargon in DRM_INFO+.
> 
> In this case, you could argue that i915.enable_guc_submission is an unsafe
> parameter set to off by default and so the combination of gvt + guc is pure
> user error and they can keep both pieces.
> 
> Ideally we wouldn't use i915.enable_guc_submission at all, but gvt should be
> disabled upon enabling guc -- since the combination is currently inoperable.
> But again, this is just a user error and we can just -EIO the driver load...
> -Chris

Hi Chris/Joonas,

Do you agree to use i915.enable_guc_submission for telling gvt to disable itself?

Thanks
Chuanxiao

> 
> --
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Joonas Lahtinen]

On ma, 2017-05-08 at 10:30 +0000, Dong, Chuanxiao wrote:

<SNIP>

> > > > > +	if (i915.enable_guc_submission) {
> > > > > +		DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to
> > > > > +enabled GuC submission [i915.enable_guc_submission module
> > > > > +parameter]\n");

<SNIP>

Something along the lines: "Graphics virtualization is not yet
supported with GuC" would be most informative to user.

> > > 
> > It needs to be verbose because it is a message to the user, it should tell them
> > what broke, what that will likely mean to them and if possible how to rectify.
> > Even then we know they won't read the entirety of the message but at least
> > it gives us a starting point for the inevitable bugs. We should always aim for
> > clarity and avoid too much jargon in DRM_INFO+.
> > 
> > In this case, you could argue that i915.enable_guc_submission is an unsafe
> > parameter set to off by default and so the combination of gvt + guc is pure
> > user error and they can keep both pieces.
> > 
> > Ideally we wouldn't use i915.enable_guc_submission at all, but gvt should be
> > disabled upon enabling guc -- since the combination is currently inoperable.
> > But again, this is just a user error and we can just -EIO the driver load...

I agree that currently when GuC submission is not enabled by default,
enabling it could result in -EIO on driver load if GVT is enabled
simultaneously.

When GuC gets enabled by default, it would be preferable for GVT team
to have support ready, or enable_gvt probably needs to become an
_unsafe param.

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: [PATCH] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 1619 bytes --]

On 2017.05.05 11:55:14 +0300, Joonas Lahtinen wrote:
> + Daniel
> 
> On ke, 2017-05-03 at 16:36 +0800, Zhenyu Wang wrote:
> > On 2017.05.02 16:58:31 +0800, Chuanxiao Dong wrote:
> > > 
> > > Currently GVT-g cannot work properly when host GuC submission
> > > is enabled, so disable GVT in this case.
> > > 
> > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> 
> <SNIP>
> 
> > > @@ -84,6 +84,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
> > >  		goto bail;
> > >  	}
> > >  
> > > +	if (i915.enable_guc_submission) {
> > > +		DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to enabled GuC submission [i915.enable_guc_submission module parameter]\n");
> 
> Guest module parameter is not the correct way of detetecting if host
> has GuC submission enabled. And even if it was, the message is overly
> verbose (and it'll be incorrect once i915.enable_guc_submission
> defaults to something else than zero).
> 
> > > +		goto bail;
> > > +	}
> > > +
> > >  	/*
> > >  	 * We're not in host or fail to find a MPT module, disable GVT-g
> > >  	 */
> > > -- 
> > 
> > Applied, thanks!
> 
> The original patch should've included at least some Cc's, or wait being
> merged through drm-tip as it's not int drm/i915/gvt directory at all
> (unlike the message states).
> 
> The patch should be reverted for being incorrect.
> 

ok, as this is not sent for any gvt pull, will drop this in gvt tree.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCH v2] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Chuanxiao Dong]

Currently GVT-g cannot work properly when host GuC submission
is enabled, so disable GVT in this case.

v2: update the user message (Joonas)

Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
---
 drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
index e1ab643..d85742c 100644
--- a/drivers/gpu/drm/i915/intel_gvt.c
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -84,6 +84,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
 		goto bail;
 	}
 
+	if (i915.enable_guc_submission) {
+		DRM_INFO("GPU guest virtualisation [GVT-g] disabled as Graphics virtualization is not yet supported with GuC submission [i915.enable_guc_submission module parameter]\n");
+		goto bail;
+	}
+
 	/*
 	 * We're not in host or fail to find a MPT module, disable GVT-g
 	 */
-- 
2.7.4

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

Re: [PATCH v2] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Joonas Lahtinen]

On ti, 2017-05-09 at 18:11 +0800, Chuanxiao Dong wrote:
> Currently GVT-g cannot work properly when host GuC submission
> is enabled, so disable GVT in this case.
> 
> v2: update the user message (Joonas)
> 
> Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
> index e1ab643..d85742c 100644
> --- a/drivers/gpu/drm/i915/intel_gvt.c
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -84,6 +84,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
>  		goto bail;
>  	}
>  
> +	if (i915.enable_guc_submission) {
> +		DRM_INFO("GPU guest virtualisation [GVT-g] disabled as Graphics virtualization is not yet supported with GuC submission [i915.enable_guc_submission module parameter]\n");
> +		goto bail;
> +	}

As discussed earlier, driver loading should fail with -EIO when
incompatible options are specified.

Also, the message is now even more cryptic, just:

	DRM_INFO("Graphics virtualization is not yet supported with GuC submission");

Should do.

Please Cc: the people who've previously commented on the patch to make
it easier to spot new versions.

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: [PATCH v2] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Dong, Chuanxiao]

> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Wednesday, May 10, 2017 8:48 PM
> To: Dong, Chuanxiao <chuanxiao.dong@intel.com>; intel-gvt-
> dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/gvt: disable GVT-g if host GuC
> submission is enabled
> 
> On ti, 2017-05-09 at 18:11 +0800, Chuanxiao Dong wrote:
> > Currently GVT-g cannot work properly when host GuC submission is
> > enabled, so disable GVT in this case.
> >
> > v2: update the user message (Joonas)
> >
> > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_gvt.c
> > b/drivers/gpu/drm/i915/intel_gvt.c
> > index e1ab643..d85742c 100644
> > --- a/drivers/gpu/drm/i915/intel_gvt.c
> > +++ b/drivers/gpu/drm/i915/intel_gvt.c
> > @@ -84,6 +84,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
> >  		goto bail;
> >  	}
> >
> > +	if (i915.enable_guc_submission) {
> > +		DRM_INFO("GPU guest virtualisation [GVT-g] disabled as
> Graphics virtualization is not yet supported with GuC submission
> [i915.enable_guc_submission module parameter]\n");
> > +		goto bail;
> > +	}
> 
> As discussed earlier, driver loading should fail with -EIO when incompatible
> options are specified.

Sorry for not getting why should fail with -EIO? By looking into the source, Intel_gvt_init is part of i915 driver loading, and fail with -EIO will make i915 driver failed to load. 
The loading of intel gvt is in intel_gvt_init, and "goto bail" can skip gvt loading which should be the purpose of disabling gvt when incompatible options are specified, so it doesn't rely on the return value of intel_gvt_init.

> 
> Also, the message is now even more cryptic, just:
> 
> 	DRM_INFO("Graphics virtualization is not yet supported with GuC
> submission");

Ah...It is my misunderstanding of your last comment. Will fix this.

> 
> Should do.
> 
> Please Cc: the people who've previously commented on the patch to make it
> easier to spot new versions.

Sorry for missing this.

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

Re: [PATCH v2] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Joonas Lahtinen]

On to, 2017-05-11 at 02:33 +0000, Dong, Chuanxiao wrote:
> 
> > 
> > -----Original Message-----
> > From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> > Sent: Wednesday, May 10, 2017 8:48 PM
> > To: Dong, Chuanxiao <chuanxiao.dong@intel.com>; intel-gvt-
> > dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/gvt: disable GVT-g if host GuC
> > submission is enabled
> > 
> > On ti, 2017-05-09 at 18:11 +0800, Chuanxiao Dong wrote:
> > > 
> > > Currently GVT-g cannot work properly when host GuC submission is
> > > enabled, so disable GVT in this case.
> > > 
> > > v2: update the user message (Joonas)
> > > 
> > > > > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > > > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_gvt.c
> > > b/drivers/gpu/drm/i915/intel_gvt.c
> > > index e1ab643..d85742c 100644
> > > --- a/drivers/gpu/drm/i915/intel_gvt.c
> > > +++ b/drivers/gpu/drm/i915/intel_gvt.c
> > > @@ -84,6 +84,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
> > >  		goto bail;
> > >  	}
> > > 
> > > +	if (i915.enable_guc_submission) {
> > > +		DRM_INFO("GPU guest virtualisation [GVT-g] disabled as
> > Graphics virtualization is not yet supported with GuC submission
> > [i915.enable_guc_submission module parameter]\n");
> > > 
> > > +		goto bail;
> > > +	}
> > 
> > As discussed earlier, driver loading should fail with -EIO when incompatible
> > options are specified.
> 
> Sorry for not getting why should fail with -EIO? By looking into the
> source, Intel_gvt_init is part of i915 driver loading, and fail with
> -EIO will make i915 driver failed to load. 

Yes, the user has specified an unsafe kernel configuration option,
enable_guc_submission and the driver can rightfully stop loading if
enable_gvt option was passed in the same command line.

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: [PATCH v2] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Dong, Chuanxiao]

> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Thursday, May 11, 2017 8:50 PM
> To: Dong, Chuanxiao <chuanxiao.dong@intel.com>; intel-gvt-
> dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/gvt: disable GVT-g if host GuC
> submission is enabled
> 
> On to, 2017-05-11 at 02:33 +0000, Dong, Chuanxiao wrote:
> >
> > >
> > > -----Original Message-----
> > > From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> > > Sent: Wednesday, May 10, 2017 8:48 PM
> > > To: Dong, Chuanxiao <chuanxiao.dong@intel.com>; intel-gvt-
> > > dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/gvt: disable GVT-g if
> > > host GuC submission is enabled
> > >
> > > On ti, 2017-05-09 at 18:11 +0800, Chuanxiao Dong wrote:
> > > >
> > > > Currently GVT-g cannot work properly when host GuC submission is
> > > > enabled, so disable GVT in this case.
> > > >
> > > > v2: update the user message (Joonas)
> > > >
> > > > > > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > > > > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
> > > >  1 file changed, 5 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_gvt.c
> > > > b/drivers/gpu/drm/i915/intel_gvt.c
> > > > index e1ab643..d85742c 100644
> > > > --- a/drivers/gpu/drm/i915/intel_gvt.c
> > > > +++ b/drivers/gpu/drm/i915/intel_gvt.c
> > > > @@ -84,6 +84,11 @@ int intel_gvt_init(struct drm_i915_private
> *dev_priv)
> > > >  		goto bail;
> > > >  	}
> > > >
> > > > +	if (i915.enable_guc_submission) {
> > > > +		DRM_INFO("GPU guest virtualisation [GVT-g] disabled as
> > > Graphics virtualization is not yet supported with GuC submission
> > > [i915.enable_guc_submission module parameter]\n");
> > > >
> > > > +		goto bail;
> > > > +	}
> > >
> > > As discussed earlier, driver loading should fail with -EIO when
> > > incompatible options are specified.
> >
> > Sorry for not getting why should fail with -EIO? By looking into the
> > source, Intel_gvt_init is part of i915 driver loading, and fail with
> > -EIO will make i915 driver failed to load.
> 
> Yes, the user has specified an unsafe kernel configuration option,
> enable_guc_submission and the driver can rightfully stop loading if
> enable_gvt option was passed in the same command line.
> 
Thanks Joonas. If to fail with -EIO, how about for the other two checks: " if (!is_supported_device(dev_priv))" and " if (!i915.enable_execlists)"?  Currently these two cases are failed with 0 instead of -EIO. Looks like should also fail with -EIO?

For the gvt init failure case, it is also failed with 0 right now, which means that even gvt init is failed, i915 loading will be continue instead of failed due to gvt. Right now we also want to fail with -EIO, right?

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

Re: [PATCH v2] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Dong, Chuanxiao]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Dong, Chuanxiao
> Sent: Thursday, May 11, 2017 9:38 PM
> To: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>; intel-gvt-
> dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Subject: RE: [Intel-gfx] [PATCH v2] drm/i915/gvt: disable GVT-g if host GuC
> submission is enabled
> 
> 
> 
> > -----Original Message-----
> > From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> > Sent: Thursday, May 11, 2017 8:50 PM
> > To: Dong, Chuanxiao <chuanxiao.dong@intel.com>; intel-gvt-
> > dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/gvt: disable GVT-g if
> > host GuC submission is enabled
> >
> > On to, 2017-05-11 at 02:33 +0000, Dong, Chuanxiao wrote:
> > >
> > > >
> > > > -----Original Message-----
> > > > From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> > > > Sent: Wednesday, May 10, 2017 8:48 PM
> > > > To: Dong, Chuanxiao <chuanxiao.dong@intel.com>; intel-gvt-
> > > > dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> > > > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/gvt: disable GVT-g if
> > > > host GuC submission is enabled
> > > >
> > > > On ti, 2017-05-09 at 18:11 +0800, Chuanxiao Dong wrote:
> > > > >
> > > > > Currently GVT-g cannot work properly when host GuC submission is
> > > > > enabled, so disable GVT in this case.
> > > > >
> > > > > v2: update the user message (Joonas)
> > > > >
> > > > > > > > Cc: Zhenyu Wang <zhenyuw@linux.intel.com>
> > > > > > > > Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
> > > > >  1 file changed, 5 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_gvt.c
> > > > > b/drivers/gpu/drm/i915/intel_gvt.c
> > > > > index e1ab643..d85742c 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_gvt.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_gvt.c
> > > > > @@ -84,6 +84,11 @@ int intel_gvt_init(struct drm_i915_private
> > *dev_priv)
> > > > >  		goto bail;
> > > > >  	}
> > > > >
> > > > > +	if (i915.enable_guc_submission) {
> > > > > +		DRM_INFO("GPU guest virtualisation [GVT-g]
> disabled as
> > > > Graphics virtualization is not yet supported with GuC submission
> > > > [i915.enable_guc_submission module parameter]\n");
> > > > >
> > > > > +		goto bail;
> > > > > +	}
> > > >
> > > > As discussed earlier, driver loading should fail with -EIO when
> > > > incompatible options are specified.
> > >
> > > Sorry for not getting why should fail with -EIO? By looking into the
> > > source, Intel_gvt_init is part of i915 driver loading, and fail with
> > > -EIO will make i915 driver failed to load.
> >
> > Yes, the user has specified an unsafe kernel configuration option,
> > enable_guc_submission and the driver can rightfully stop loading if
> > enable_gvt option was passed in the same command line.
> >
> Thanks Joonas. If to fail with -EIO, how about for the other two checks: " if
> (!is_supported_device(dev_priv))" and " if (!i915.enable_execlists)"?
> Currently these two cases are failed with 0 instead of -EIO. Looks like should
> also fail with -EIO?
> 
> For the gvt init failure case, it is also failed with 0 right now, which means
> that even gvt init is failed, i915 loading will be continue instead of failed due
> to gvt. Right now we also want to fail with -EIO, right?
> 
Hi Joonas,

Do you have any suggestion on how to handle the above two cases? Or you only want to fail with -EIO for the guc case, and keep the original for the others?

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

Re: [PATCH v2] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Joonas Lahtinen]

On ti, 2017-05-16 at 07:53 +0000, Dong, Chuanxiao wrote:
> > 
> > Thanks Joonas. If to fail with -EIO, how about for the other two checks: " if
> > (!is_supported_device(dev_priv))" and " if (!i915.enable_execlists)"?
> > Currently these two cases are failed with 0 instead of -EIO. Looks like should
> > also fail with -EIO?

!is_supported_device check should probably be done in parameter
sanitization phase, it should not result in EIO, because enable_gvt is
not marked as an unsafe option.

enable_execlists on the other hand is a an _unsafe option and should
result in -EIO.

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: [PATCH v2] drm/i915/gvt: disable GVT-g if host GuC submission is enabled

[Dong, Chuanxiao]

> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Thursday, May 18, 2017 6:37 PM
> To: Dong, Chuanxiao <chuanxiao.dong@intel.com>; intel-gvt-
> dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v2] drm/i915/gvt: disable GVT-g if host GuC
> submission is enabled
> 
> On ti, 2017-05-16 at 07:53 +0000, Dong, Chuanxiao wrote:
> > >
> > > Thanks Joonas. If to fail with -EIO, how about for the other two
> > > checks: " if (!is_supported_device(dev_priv))" and " if
> (!i915.enable_execlists)"?
> > > Currently these two cases are failed with 0 instead of -EIO. Looks
> > > like should also fail with -EIO?
> 
> !is_supported_device check should probably be done in parameter
> sanitization phase, it should not result in EIO, because enable_gvt is not
> marked as an unsafe option.
> 
> enable_execlists on the other hand is a an _unsafe option and should result
> in -EIO.
> 
Thanks Joonas for the suggestion. Looks like we need two more patches here, one for add a sanitize API for GVT so it can be called by intel_sanitize_options in sanitization phase. Another patch is to return -EIO if !i915.enable_execlists. Will send out these two patches first.

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

✓ Fi.CI.BAT: success for drm/i915/gvt: disable GVT-g if host GuC submission is enabled (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/gvt: disable GVT-g if host GuC submission is enabled (rev2)
URL   : https://patchwork.freedesktop.org/series/23796/
State : success

== Summary ==

Series 23796v2 drm/i915/gvt: disable GVT-g if host GuC submission is enabled
https://patchwork.freedesktop.org/api/1.0/series/23796/revisions/2/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                dmesg-warn -> PASS       (fi-byt-j1900) fdo#100652

fdo#100652 https://bugs.freedesktop.org/show_bug.cgi?id=100652

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:431s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:427s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:584s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:509s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:539s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:490s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:487s
fi-elk-e7500     total:278  pass:229  dwarn:0   dfail:0   fail:0   skip:49  time:417s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:410s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:407s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:413s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:493s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:471s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:457s
fi-kbl-7560u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:563s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:450s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:571s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:457s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:498s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:429s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:529s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:398s

f558b185c57202d90bdb9059f3d446956cbae133 drm-tip: 2017y-05m-08d-16h-35m-28s UTC integration manifest
b8e1220 drm/i915/gvt: disable GVT-g if host GuC submission is enabled

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4647/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx