[PATCH] drm/i915/gvt: add enable_execlists check before enable gvt

[PATCH] drm/i915/gvt: add enable_execlists check before enable gvt

[Chuanxiao Dong]

The GVT-g needs execlists to be enabled otherwise gvt should be
disabled. Add a check for enable_execlists before enabling gvt.

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 d23c0fc..3799cb3 100644
--- a/drivers/gpu/drm/i915/intel_gvt.c
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -77,6 +77,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
 		goto bail;
 	}
 
+	if (!i915.enable_execlists) {
+		DRM_DEBUG_DRIVER("Execlists unsupported, GVT-g is disabled\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] drm/i915/gvt: add enable_execlists check before enable gvt

[Dong, Chuanxiao]

Hi,

Any comments to this patch?

Thanks
Chuanxiao

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Chuanxiao Dong
> Sent: Monday, March 6, 2017 1:16 PM
> To: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> Subject: [PATCH] drm/i915/gvt: add enable_execlists check before enable gvt
> 
> The GVT-g needs execlists to be enabled otherwise gvt should be disabled.
> Add a check for enable_execlists before enabling gvt.
> 
> 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 d23c0fc..3799cb3 100644
> --- a/drivers/gpu/drm/i915/intel_gvt.c
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -77,6 +77,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
>  		goto bail;
>  	}
> 
> +	if (!i915.enable_execlists) {
> +		DRM_DEBUG_DRIVER("Execlists unsupported, GVT-g is
> disabled\n");
> +		goto bail;
> +	}
> +
>  	/*
>  	 * We're not in host or fail to find a MPT module, disable GVT-g
>  	 */
> --
> 2.7.4
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/gvt: add enable_execlists check before enable gvt

[Chris Wilson]

On Thu, Mar 09, 2017 at 12:32:18PM +0000, Dong, Chuanxiao wrote:
> Hi,
> 
> Any comments to this patch?
> 
> Thanks
> Chuanxiao
> 
> > -----Original Message-----
> > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> > Behalf Of Chuanxiao Dong
> > Sent: Monday, March 6, 2017 1:16 PM
> > To: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> > Subject: [PATCH] drm/i915/gvt: add enable_execlists check before enable gvt
> > 
> > The GVT-g needs execlists to be enabled otherwise gvt should be disabled.
> > Add a check for enable_execlists before enabling gvt.
> > 
> > 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 d23c0fc..3799cb3 100644
> > --- a/drivers/gpu/drm/i915/intel_gvt.c
> > +++ b/drivers/gpu/drm/i915/intel_gvt.c
> > @@ -77,6 +77,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
> >  		goto bail;
> >  	}
> > 
> > +	if (!i915.enable_execlists) {
> > +		DRM_DEBUG_DRIVER("Execlists unsupported, GVT-g is
> > disabled\n");

Should be DRM_INFO() in response to the user action, and should be
treated as a user visible string.

DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to disabled execlist submission [i915.enable_execlists module parameter]\n");
-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: add enable_execlists check before enable gvt

[Dong, Chuanxiao]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> Behalf Of Chris Wilson
> Sent: Thursday, March 9, 2017 8:45 PM
> To: Dong, Chuanxiao <chuanxiao.dong@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: add enable_execlists check
> before enable gvt
> 
> On Thu, Mar 09, 2017 at 12:32:18PM +0000, Dong, Chuanxiao wrote:
> > Hi,
> >
> > Any comments to this patch?
> >
> > Thanks
> > Chuanxiao
> >
> > > -----Original Message-----
> > > From: intel-gvt-dev
> > > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of
> > > Chuanxiao Dong
> > > Sent: Monday, March 6, 2017 1:16 PM
> > > To: intel-gfx@lists.freedesktop.org;
> > > intel-gvt-dev@lists.freedesktop.org
> > > Subject: [PATCH] drm/i915/gvt: add enable_execlists check before
> > > enable gvt
> > >
> > > The GVT-g needs execlists to be enabled otherwise gvt should be disabled.
> > > Add a check for enable_execlists before enabling gvt.
> > >
> > > 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 d23c0fc..3799cb3 100644
> > > --- a/drivers/gpu/drm/i915/intel_gvt.c
> > > +++ b/drivers/gpu/drm/i915/intel_gvt.c
> > > @@ -77,6 +77,11 @@ int intel_gvt_init(struct drm_i915_private
> *dev_priv)
> > >  		goto bail;
> > >  	}
> > >
> > > +	if (!i915.enable_execlists) {
> > > +		DRM_DEBUG_DRIVER("Execlists unsupported, GVT-g is
> > > disabled\n");
> 
> Should be DRM_INFO() in response to the user action, and should be treated
> as a user visible string.
> 
> DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to disabled
> execlist submission [i915.enable_execlists module parameter]\n"); -Chris

Thanks Chris for the comments. Use "DRM_DEBUG_DRIVER" is just following the same print level with the other fail case in intel_gvt_init(). Anyway, I will take your suggestion to use DRM_INFO instead.

The message printed is longer than 80 characters, and checkpatch tool also complain with " quoted string split across lines" by splitting to multiple lines. So should I use the message print like below?
                
DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to ");
DRM_INFO("disabled execlist submission ");
DRM_INFO("[i915.enable_execlists module parameter]\n");

Or any better idea?

Thanks
Chuanxiao

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

Re: [PATCH] drm/i915/gvt: add enable_execlists check before enable gvt

[Chris Wilson]

On Thu, Mar 09, 2017 at 01:03:17PM +0000, Dong, Chuanxiao wrote:
> 
> 
> > -----Original Message-----
> > From: intel-gvt-dev [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On
> > Behalf Of Chris Wilson
> > Sent: Thursday, March 9, 2017 8:45 PM
> > To: Dong, Chuanxiao <chuanxiao.dong@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: add enable_execlists check
> > before enable gvt
> > 
> > On Thu, Mar 09, 2017 at 12:32:18PM +0000, Dong, Chuanxiao wrote:
> > > Hi,
> > >
> > > Any comments to this patch?
> > >
> > > Thanks
> > > Chuanxiao
> > >
> > > > -----Original Message-----
> > > > From: intel-gvt-dev
> > > > [mailto:intel-gvt-dev-bounces@lists.freedesktop.org] On Behalf Of
> > > > Chuanxiao Dong
> > > > Sent: Monday, March 6, 2017 1:16 PM
> > > > To: intel-gfx@lists.freedesktop.org;
> > > > intel-gvt-dev@lists.freedesktop.org
> > > > Subject: [PATCH] drm/i915/gvt: add enable_execlists check before
> > > > enable gvt
> > > >
> > > > The GVT-g needs execlists to be enabled otherwise gvt should be disabled.
> > > > Add a check for enable_execlists before enabling gvt.
> > > >
> > > > 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 d23c0fc..3799cb3 100644
> > > > --- a/drivers/gpu/drm/i915/intel_gvt.c
> > > > +++ b/drivers/gpu/drm/i915/intel_gvt.c
> > > > @@ -77,6 +77,11 @@ int intel_gvt_init(struct drm_i915_private
> > *dev_priv)
> > > >  		goto bail;
> > > >  	}
> > > >
> > > > +	if (!i915.enable_execlists) {
> > > > +		DRM_DEBUG_DRIVER("Execlists unsupported, GVT-g is
> > > > disabled\n");
> > 
> > Should be DRM_INFO() in response to the user action, and should be treated
> > as a user visible string.
> > 
> > DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to disabled
> > execlist submission [i915.enable_execlists module parameter]\n"); -Chris
> 
> Thanks Chris for the comments. Use "DRM_DEBUG_DRIVER" is just following the same print level with the other fail case in intel_gvt_init(). Anyway, I will take your suggestion to use DRM_INFO instead.

Just a challenge to identify something that may be in response to user
action, typically module parameters or quirks, that may either have
intended or unintended side-effects.

> The message printed is longer than 80 characters, and checkpatch tool also complain with " quoted string split across lines" by splitting to multiple lines. So should I use the message print like below?
>                 
> DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to ");
> DRM_INFO("disabled execlist submission ");
> DRM_INFO("[i915.enable_execlists module parameter]\n");
> 
> Or any better idea?

Just ignore checkpatch. Either split the line or not, that's up to you :)
-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: add enable_execlists check before enable gvt

[Jani Nikula]

On Thu, 09 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, Mar 09, 2017 at 01:03:17PM +0000, Dong, Chuanxiao wrote:
>> The message printed is longer than 80 characters, and checkpatch tool also complain with " quoted string split across lines" by splitting to multiple lines. So should I use the message print like below?
>>                 
>> DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to ");
>> DRM_INFO("disabled execlist submission ");
>> DRM_INFO("[i915.enable_execlists module parameter]\n");
>> 
>> Or any better idea?
>
> Just ignore checkpatch. Either split the line or not, that's up to you :)

Btw ignoring checkpatch is not general advise. Please do always look at
checkpatch results, it does give helpful feedback on silly mistakes. But
just don't regard it as the final authority on kernel style, use your
own discretion. In this case, checkpatch is just silly.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/gvt: add enable_execlists check before enable gvt

[Dong, Chuanxiao]

> -----Original Message-----
> From: Jani Nikula [mailto:jani.nikula@linux.intel.com]
> Sent: Thursday, March 9, 2017 9:48 PM
> To: Chris Wilson; Dong, Chuanxiao
> Cc: intel-gfx@lists.freedesktop.org; intel-gvt-dev@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: add enable_execlists check
> before enable gvt
> 
> On Thu, 09 Mar 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > On Thu, Mar 09, 2017 at 01:03:17PM +0000, Dong, Chuanxiao wrote:
> >> The message printed is longer than 80 characters, and checkpatch tool
> also complain with " quoted string split across lines" by splitting to multiple
> lines. So should I use the message print like below?
> >>
> >> DRM_INFO("GPU guest virtualisation [GVT-g] disabled due to ");
> >> DRM_INFO("disabled execlist submission ");
> >> DRM_INFO("[i915.enable_execlists module parameter]\n");
> >>
> >> Or any better idea?
> >
> > Just ignore checkpatch. Either split the line or not, that's up to you
> > :)
> 
> Btw ignoring checkpatch is not general advise. Please do always look at
> checkpatch results, it does give helpful feedback on silly mistakes. But just
> don't regard it as the final authority on kernel style, use your own discretion.
> In this case, checkpatch is just silly.

I see. Thanks Chris and Jani for the guide. :) Will send out the v2 with the suggested message.

Thanks
Chuanxiao

> 
> 
> BR,
> Jani.
> 
> 
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx