Reviewed a long time ago, resending for CI + to dri-devel.
John Harrison (1): drm/i915/uc: Use platform specific defaults for GuC/HuC enabling
drivers/gpu/drm/i915/i915_params.c | 2 +- drivers/gpu/drm/i915/i915_params.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
From: John Harrison John.C.Harrison@Intel.com
The meaning of 'default' for the enable_guc module parameter has been updated to accurately reflect what is supported on current platforms. So start using the defaults instead of forcing everything off. Although, note that right now, the default is for everything to be off anyway. So this is not a change for current platforms.
Signed-off-by: John Harrison John.C.Harrison@Intel.com Signed-off-by: Matthew Brost matthew.brost@intel.com Reviewed-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com --- drivers/gpu/drm/i915/i915_params.c | 2 +- drivers/gpu/drm/i915/i915_params.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 0320878d96b0..e07f4cfea63a 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -160,7 +160,7 @@ i915_param_named_unsafe(edp_vswing, int, 0400, i915_param_named_unsafe(enable_guc, int, 0400, "Enable GuC load for GuC submission and/or HuC load. " "Required functionality can be selected using bitmask values. " - "(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)"); + "(-1=auto [default], 0=disable, 1=GuC submission, 2=HuC load)");
i915_param_named(guc_log_level, int, 0400, "GuC firmware logging level. Requires GuC to be loaded. " diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 4a114a5ad000..f27eceb82c0f 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -59,7 +59,7 @@ struct drm_printer; param(int, disable_power_well, -1, 0400) \ param(int, enable_ips, 1, 0600) \ param(int, invert_brightness, 0, 0600) \ - param(int, enable_guc, 0, 0400) \ + param(int, enable_guc, -1, 0400) \ param(int, guc_log_level, -1, 0400) \ param(char *, guc_firmware_path, NULL, 0400) \ param(char *, huc_firmware_path, NULL, 0400) \
On 6/3/2021 9:48 AM, Matthew Brost wrote:
From: John Harrison John.C.Harrison@Intel.com
The meaning of 'default' for the enable_guc module parameter has been updated to accurately reflect what is supported on current platforms. So start using the defaults instead of forcing everything off. Although, note that right now, the default is for everything to be off anyway. So this is not a change for current platforms.
Signed-off-by: John Harrison John.C.Harrison@Intel.com Signed-off-by: Matthew Brost matthew.brost@intel.com Reviewed-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com
Double checked the CI results and the 2 errors are unrelated. Pushed to gt-next.
Daniele
drivers/gpu/drm/i915/i915_params.c | 2 +- drivers/gpu/drm/i915/i915_params.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 0320878d96b0..e07f4cfea63a 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -160,7 +160,7 @@ i915_param_named_unsafe(edp_vswing, int, 0400, i915_param_named_unsafe(enable_guc, int, 0400, "Enable GuC load for GuC submission and/or HuC load. " "Required functionality can be selected using bitmask values. "
- "(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)");
"(-1=auto [default], 0=disable, 1=GuC submission, 2=HuC load)");
i915_param_named(guc_log_level, int, 0400, "GuC firmware logging level. Requires GuC to be loaded. "
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 4a114a5ad000..f27eceb82c0f 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -59,7 +59,7 @@ struct drm_printer; param(int, disable_power_well, -1, 0400) \ param(int, enable_ips, 1, 0600) \ param(int, invert_brightness, 0, 0600) \
- param(int, enable_guc, 0, 0400) \
- param(int, enable_guc, -1, 0400) \ param(int, guc_log_level, -1, 0400) \ param(char *, guc_firmware_path, NULL, 0400) \ param(char *, huc_firmware_path, NULL, 0400) \
On 03/06/2021 17:48, Matthew Brost wrote:
From: John Harrison John.C.Harrison@Intel.com
The meaning of 'default' for the enable_guc module parameter has been updated to accurately reflect what is supported on current platforms. So start using the defaults instead of forcing everything off. Although, note that right now, the default is for everything to be off anyway. So this is not a change for current platforms.
Signed-off-by: John Harrison John.C.Harrison@Intel.com Signed-off-by: Matthew Brost matthew.brost@intel.com Reviewed-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com
drivers/gpu/drm/i915/i915_params.c | 2 +- drivers/gpu/drm/i915/i915_params.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 0320878d96b0..e07f4cfea63a 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -160,7 +160,7 @@ i915_param_named_unsafe(edp_vswing, int, 0400, i915_param_named_unsafe(enable_guc, int, 0400, "Enable GuC load for GuC submission and/or HuC load. " "Required functionality can be selected using bitmask values. "
- "(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)");
"(-1=auto [default], 0=disable, 1=GuC submission, 2=HuC load)");
i915_param_named(guc_log_level, int, 0400, "GuC firmware logging level. Requires GuC to be loaded. "
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 4a114a5ad000..f27eceb82c0f 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -59,7 +59,7 @@ struct drm_printer; param(int, disable_power_well, -1, 0400) \ param(int, enable_ips, 1, 0600) \ param(int, invert_brightness, 0, 0600) \
- param(int, enable_guc, 0, 0400) \
- param(int, enable_guc, -1, 0400) \ param(int, guc_log_level, -1, 0400) \ param(char *, guc_firmware_path, NULL, 0400) \ param(char *, huc_firmware_path, NULL, 0400) \
What is the BKM to use this with multi-GPU setups? Specifically I have a TGL+DG1 laptop (off the shelf) and want to have GuC with DG1 only. If I pass i915.enable_guc=3 it seems it wants to enable it for TGL as well and wedges the GPU if it can't?
Regards,
Tvrtko
On 4/7/2022 08:49, Tvrtko Ursulin wrote:
On 03/06/2021 17:48, Matthew Brost wrote:
From: John Harrison John.C.Harrison@Intel.com
The meaning of 'default' for the enable_guc module parameter has been updated to accurately reflect what is supported on current platforms. So start using the defaults instead of forcing everything off. Although, note that right now, the default is for everything to be off anyway. So this is not a change for current platforms.
Signed-off-by: John Harrison John.C.Harrison@Intel.com Signed-off-by: Matthew Brost matthew.brost@intel.com Reviewed-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com
drivers/gpu/drm/i915/i915_params.c | 2 +- drivers/gpu/drm/i915/i915_params.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 0320878d96b0..e07f4cfea63a 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -160,7 +160,7 @@ i915_param_named_unsafe(edp_vswing, int, 0400, i915_param_named_unsafe(enable_guc, int, 0400, "Enable GuC load for GuC submission and/or HuC load. " "Required functionality can be selected using bitmask values. " - "(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)"); + "(-1=auto [default], 0=disable, 1=GuC submission, 2=HuC load)"); i915_param_named(guc_log_level, int, 0400, "GuC firmware logging level. Requires GuC to be loaded. " diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 4a114a5ad000..f27eceb82c0f 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -59,7 +59,7 @@ struct drm_printer; param(int, disable_power_well, -1, 0400) \ param(int, enable_ips, 1, 0600) \ param(int, invert_brightness, 0, 0600) \ - param(int, enable_guc, 0, 0400) \ + param(int, enable_guc, -1, 0400) \ param(int, guc_log_level, -1, 0400) \ param(char *, guc_firmware_path, NULL, 0400) \ param(char *, huc_firmware_path, NULL, 0400) \
What is the BKM to use this with multi-GPU setups? Specifically I have a TGL+DG1 laptop (off the shelf) and want to have GuC with DG1 only. If I pass i915.enable_guc=3 it seems it wants to enable it for TGL as well and wedges the GPU if it can't?
I don't think there is one.
Module parameters are driver global and therefore apply to all cards in the system, both discrete and integrated. The '-1' default can and does have different meanings for each card. So in the TGL+DG1 case, TGL should default to execlist and DG1 should already be defaulting to GuC. So the -1 setting should do what you want. But if you did need to override for one specific card only then I think you would need to do that with a code change and rebuild.
John.
Regards,
Tvrtko
On 07/04/2022 21:49, John Harrison wrote:
On 4/7/2022 08:49, Tvrtko Ursulin wrote:
On 03/06/2021 17:48, Matthew Brost wrote:
From: John Harrison John.C.Harrison@Intel.com
The meaning of 'default' for the enable_guc module parameter has been updated to accurately reflect what is supported on current platforms. So start using the defaults instead of forcing everything off. Although, note that right now, the default is for everything to be off anyway. So this is not a change for current platforms.
Signed-off-by: John Harrison John.C.Harrison@Intel.com Signed-off-by: Matthew Brost matthew.brost@intel.com Reviewed-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com
drivers/gpu/drm/i915/i915_params.c | 2 +- drivers/gpu/drm/i915/i915_params.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 0320878d96b0..e07f4cfea63a 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -160,7 +160,7 @@ i915_param_named_unsafe(edp_vswing, int, 0400, i915_param_named_unsafe(enable_guc, int, 0400, "Enable GuC load for GuC submission and/or HuC load. " "Required functionality can be selected using bitmask values. " - "(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)"); + "(-1=auto [default], 0=disable, 1=GuC submission, 2=HuC load)"); i915_param_named(guc_log_level, int, 0400, "GuC firmware logging level. Requires GuC to be loaded. " diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h index 4a114a5ad000..f27eceb82c0f 100644 --- a/drivers/gpu/drm/i915/i915_params.h +++ b/drivers/gpu/drm/i915/i915_params.h @@ -59,7 +59,7 @@ struct drm_printer; param(int, disable_power_well, -1, 0400) \ param(int, enable_ips, 1, 0600) \ param(int, invert_brightness, 0, 0600) \ - param(int, enable_guc, 0, 0400) \ + param(int, enable_guc, -1, 0400) \ param(int, guc_log_level, -1, 0400) \ param(char *, guc_firmware_path, NULL, 0400) \ param(char *, huc_firmware_path, NULL, 0400) \
What is the BKM to use this with multi-GPU setups? Specifically I have a TGL+DG1 laptop (off the shelf) and want to have GuC with DG1 only. If I pass i915.enable_guc=3 it seems it wants to enable it for TGL as well and wedges the GPU if it can't?
I don't think there is one.
Module parameters are driver global and therefore apply to all cards in the system, both discrete and integrated. The '-1' default can and does have different meanings for each card. So in the TGL+DG1 case, TGL should default to execlist and DG1 should already be defaulting to GuC. So the -1 setting should do what you want. But if you did need to override for one specific card only then I think you would need to do that with a code change and rebuild.
You are right, I was on a kernel where DG1 wasn't yet defaulting to GuC hence the confusion when I tried to pass enable_guc=3 that broke TGL as well, but because I had no up to date firmware for it.
We really need per device modparams, as long as we have modparams..
Regards,
Tvrtko
dri-devel@lists.freedesktop.org
-
Daniele Ceraolo Spurio -
John Harrison -
Matthew Brost -
Tvrtko Ursulin