Skip to contentSkip to conversation

drm/i915/gvt: add enable_execlists check before enable gvt

PATCH Superseded Chuanxiao Dong 3 participants 7 messages Mar 6, 2017 – Mar 9, 2017

Subsystems (2)
DRM drivers Intel i915

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Chuanxiao Dong

author
#0 lore raw

PATCH 1/1 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 <[email protected]>
---
 drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/gpu/drm/i915/intel_gvt.c +5 -0
index d23c0fc..3799cb3 100644
@@ -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
 	 */
Signature · 6 lines
2.7.4

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#1 lore raw

Hi,

Any comments to this patch?

Thanks
Chuanxiao

Quoted 36 lines

-----Original Message-----
From: intel-gvt-dev [mailto:[email protected]] On Behalf Of Chuanxiao Dong
Sent: Monday, March 6, 2017 1:16 PM
To: [email protected]; [email protected]
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 <[email protected]>
---
 drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/gpu/drm/i915/intel_gvt.c +5 -0
index d23c0fc..3799cb3 100644
@@ -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

Quoted 4 lines

_______________________________________________
intel-gvt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#2 lore raw
Quoted 31 lines · Chuanxiao

Hi,

Any comments to this patch?

Thanks
Chuanxiao

Quoted 24 lines

-----Original Message-----
From: intel-gvt-dev [mailto:[email protected]] On Behalf Of Chuanxiao Dong
Sent: Monday, March 6, 2017 1:16 PM
To: [email protected]; [email protected]
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 <[email protected]>
---
 drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/gpu/drm/i915/intel_gvt.c +0 -0
index d23c0fc..3799cb3 100644
@@ -77,6 +77,11 @@ int intel_gvt_init(struct drm_i915_private *dev_priv)
 		goto bail;
 	}
Quoted 3 lines

drivers/gpu/drm/i915/intel_gvt.c int intel_gvt_init(struct drm_i915_private *dev_priv) · line 82

Quoted 3 lines
drivers/gpu/drm/i915/intel_gvt.c +2 -0
+	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

Signature · 5 lines
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#3 lore raw
Quoted 54 lines

-----Original Message-----
From: intel-gvt-dev [mailto:[email protected]] On Behalf Of Chris Wilson
Sent: Thursday, March 9, 2017 8:45 PM
To: Dong, Chuanxiao <[email protected]>
Cc: [email protected]; [email protected]
Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: add enable_execlists check before enable gvt

Quoted 25 lines · Chuanxiao

Hi,

Any comments to this patch?

Thanks
Chuanxiao

Quoted 18 lines

-----Original Message-----
From: intel-gvt-dev [mailto:[email protected]] On Behalf Of Chuanxiao Dong
Sent: Monday, March 6, 2017 1:16 PM
To: [email protected]; [email protected]
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 <[email protected]>
---
 drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
 1 file changed, 5 insertions(+)
Quoted 6 lines

drivers/gpu/drm/i915/intel_gvt.c int intel_gvt_init(struct drm_i915_private

Quoted 6 lines
drivers/gpu/drm/i915/intel_gvt.c +0 -0
index d23c0fc..3799cb3 100644
@@ -77,6 +77,11 @@ int intel_gvt_init(struct drm_i915_private

*dev_priv)

Quoted 6 lines

drivers/gpu/drm/i915/intel_gvt.c int intel_gvt_init(struct drm_i915_private *dev_priv)

Quoted 6 lines
drivers/gpu/drm/i915/intel_gvt.c +2 -0
 		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.

Quoted 2 lines

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

Quoted 7 lines

--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
intel-gvt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

#4 lore raw
Quoted 54 lines · Chuanxiao
Quoted 54 lines

-----Original Message-----
From: intel-gvt-dev [mailto:[email protected]] On Behalf Of Chris Wilson
Sent: Thursday, March 9, 2017 8:45 PM
To: Dong, Chuanxiao <[email protected]>
Cc: [email protected]; [email protected]
Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: add enable_execlists check before enable gvt

Quoted 25 lines · Chuanxiao

Hi,

Any comments to this patch?

Thanks
Chuanxiao

Quoted 18 lines

-----Original Message-----
From: intel-gvt-dev [mailto:[email protected]] On Behalf Of Chuanxiao Dong
Sent: Monday, March 6, 2017 1:16 PM
To: [email protected]; [email protected]
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 <[email protected]>
---
 drivers/gpu/drm/i915/intel_gvt.c | 5 +++++
 1 file changed, 5 insertions(+)
Quoted 6 lines

drivers/gpu/drm/i915/intel_gvt.c int intel_gvt_init(struct drm_i915_private

Quoted 6 lines
drivers/gpu/drm/i915/intel_gvt.c +0 -0
index d23c0fc..3799cb3 100644
@@ -77,6 +77,11 @@ int intel_gvt_init(struct drm_i915_private

*dev_priv)

Quoted 6 lines

drivers/gpu/drm/i915/intel_gvt.c int intel_gvt_init(struct drm_i915_private *dev_priv)

Quoted 6 lines
drivers/gpu/drm/i915/intel_gvt.c +2 -0
 		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.

Quoted 4 lines
Quoted 2 lines

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.

Quoted 7 lines

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

Signature · 5 lines
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
maintainer
#5 lore raw
Quoted 10 lines · Chris Wilson
Quoted 7 lines · Chuanxiao

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.

Signature · 5 lines
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
author
#6 lore raw
Quoted 22 lines

-----Original Message-----
From: Jani Nikula [mailto:[email protected]]
Sent: Thursday, March 9, 2017 9:48 PM
To: Chris Wilson; Dong, Chuanxiao
Cc: [email protected]; [email protected]
Subject: Re: [Intel-gfx] [PATCH] drm/i915/gvt: add enable_execlists check before enable gvt

Quoted 2 lines · Chris Wilson
Quoted 1 line · Chuanxiao

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?

Quoted 7 lines
Quoted 6 lines

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?

Quoted 7 lines
Quoted 2 lines

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

Quoted 8 lines

BR,
Jani.

--
Jani Nikula, Intel Open Source Technology Center

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx