[v12,01/17] drm/i915/guc/slpc: Add SLPC control to enable_guc modparam
diff mbox

Message ID 1522398722-12161-2-git-send-email-sagar.a.kamble@intel.com
State New
Headers show

Commit Message

sagar.a.kamble@intel.com March 30, 2018, 8:31 a.m. UTC
From: Tom O'Rourke <Tom.O'Rourke@intel.com>

GuC is currently being used for submission and HuC authentication.
Choices can be configured through enable_guc modparam. GuC SLPC is GT
Power and Performance management feature in GuC. Add another option to
enable_guc modparam to control SLPC.

v1: Add early call to sanitize enable_guc_slpc in intel_guc_ucode_init
    Remove sanitize enable_guc_slpc call before firmware version check
    is performed. (ChrisW)
    Version check is added in next patch and that will be done as part
    of slpc_enable_sanitize function in the next patch. (Sagar) Updated
    slpc option sanitize function call for platforms without GuC support.
    This was caught by CI BAT.

v2: Changed parameter to dev_priv for HAS_SLPC macro. (David)
    Code indentation based on checkpatch.

v3: Rebase.

v4: Moved sanitization of SLPC option post GuC load.

v5: Removed function intel_slpc_enabled. Planning to rely only on kernel
    parameter. Moved sanitization prior to GuC load to use the parameter
    during SLPC state setup during to GuC load. (Sagar)

v6: Commit message update. Rebase.

v7: Moved SLPC option sanitization to intel_uc_sanitize_options.

v8: Clearing SLPC option on GuC load failure. Change moved from later
    patch. (Sagar)

v9: s/enable_slpc/enable_guc_slpc. Rebase w.r.t modparam change.

v10: Rebase. Separate modparam is not needed now that we maintain all
     options in single param enable_guc.

Suggested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Cc: Jeff McGee <jeff.mcgee@intel.com>
---
 drivers/gpu/drm/i915/i915_params.c |  5 +++--
 drivers/gpu/drm/i915/i915_params.h |  1 +
 drivers/gpu/drm/i915/intel_uc.c    | 23 +++++++++++++++++++----
 drivers/gpu/drm/i915/intel_uc.h    |  6 ++++++
 4 files changed, 29 insertions(+), 6 deletions(-)

Comments

Michal Wajdeczko March 30, 2018, 12:37 p.m. UTC | #1
On Fri, 30 Mar 2018 10:31:46 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>
> GuC is currently being used for submission and HuC authentication.
> Choices can be configured through enable_guc modparam. GuC SLPC is GT
> Power and Performance management feature in GuC. Add another option to
> enable_guc modparam to control SLPC.
>
> v1: Add early call to sanitize enable_guc_slpc in intel_guc_ucode_init
>     Remove sanitize enable_guc_slpc call before firmware version check
>     is performed. (ChrisW)
>     Version check is added in next patch and that will be done as part
>     of slpc_enable_sanitize function in the next patch. (Sagar) Updated
>     slpc option sanitize function call for platforms without GuC support.
>     This was caught by CI BAT.
>
> v2: Changed parameter to dev_priv for HAS_SLPC macro. (David)
>     Code indentation based on checkpatch.
>
> v3: Rebase.
>
> v4: Moved sanitization of SLPC option post GuC load.
>
> v5: Removed function intel_slpc_enabled. Planning to rely only on kernel
>     parameter. Moved sanitization prior to GuC load to use the parameter
>     during SLPC state setup during to GuC load. (Sagar)
>
> v6: Commit message update. Rebase.
>
> v7: Moved SLPC option sanitization to intel_uc_sanitize_options.
>
> v8: Clearing SLPC option on GuC load failure. Change moved from later
>     patch. (Sagar)
>
> v9: s/enable_slpc/enable_guc_slpc. Rebase w.r.t modparam change.
>
> v10: Rebase. Separate modparam is not needed now that we maintain all
>      options in single param enable_guc.
>
> Suggested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> Cc: Jeff McGee <jeff.mcgee@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_params.c |  5 +++--
>  drivers/gpu/drm/i915/i915_params.h |  1 +
>  drivers/gpu/drm/i915/intel_uc.c    | 23 +++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_uc.h    |  6 ++++++
>  4 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_params.c  
> b/drivers/gpu/drm/i915/i915_params.c
> index 08108ce..40b799b 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -150,9 +150,10 @@ i915_param_named_unsafe(edp_vswing, int, 0400,
>  	"2=default swing(400mV))");
> i915_param_named_unsafe(enable_guc, int, 0400,
> -	"Enable GuC load for GuC submission and/or HuC load. "
> +	"Enable GuC load for GuC submission and/or HuC load and/or GuC SLPC. "
>  	"Required functionality can be selected using bitmask values. "
> -	"(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)");
> +	"(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load, "
> +	"4=GuC SLPC)");

Maybe to avoid later surprise, we should explicitly say that:

+	"4=GuC SLPC [requires GuC submission])");

> 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 c963603..2484925 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -32,6 +32,7 @@ struct drm_printer;
> #define ENABLE_GUC_SUBMISSION		BIT(0)
>  #define ENABLE_GUC_LOAD_HUC		BIT(1)
> +#define ENABLE_GUC_SLPC			BIT(2)
> #define I915_PARAMS_FOR_EACH(param) \
>  	param(char *, vbt_firmware, NULL) \
> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> b/drivers/gpu/drm/i915/intel_uc.c
> index 1cffaf7..0e4a97f 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -56,9 +56,15 @@ static int __get_platform_enable_guc(struct  
> drm_i915_private *dev_priv)
>  	struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
>  	int enable_guc = 0;
> -	/* Default is to enable GuC/HuC if we know their firmwares */
> -	if (intel_uc_fw_is_selected(guc_fw))
> +	/*
> +	 * Default is to enable GuC submission/SLPC/HuC if we know their
> +	 * firmwares
> +	 */
> +	if (intel_uc_fw_is_selected(guc_fw)) {
>  		enable_guc |= ENABLE_GUC_SUBMISSION;
> +		enable_guc |= ENABLE_GUC_SLPC;
> +	}
> +
>  	if (intel_uc_fw_is_selected(huc_fw))
>  		enable_guc |= ENABLE_GUC_LOAD_HUC;
> @@ -110,10 +116,11 @@ static void sanitize_options_early(struct  
> drm_i915_private *dev_priv)
>  	if (i915_modparams.enable_guc < 0)
>  		i915_modparams.enable_guc = __get_platform_enable_guc(dev_priv);
> -	DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n",
> +	DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s slpc:%s)\n",
>  			 i915_modparams.enable_guc,
>  			 yesno(intel_uc_is_using_guc_submission()),
> -			 yesno(intel_uc_is_using_huc()));
> +			 yesno(intel_uc_is_using_huc()),
> +			 yesno(intel_uc_is_using_guc_slpc()));
> 	/* Verify GuC firmware availability */
>  	if (intel_uc_is_using_guc() && !intel_uc_fw_is_selected(guc_fw)) {
> @@ -123,6 +130,14 @@ static void sanitize_options_early(struct  
> drm_i915_private *dev_priv)
>  					      "no GuC firmware");
>  	}
> +	/* Verify GuC submission and SLPC dependency */
> +	if (intel_uc_is_using_guc_slpc() &&
> +	    !intel_uc_is_using_guc_submission()) {
> +		DRM_WARN("Incompatible option detected: %s=%d, "
> +			 "GuC SLPC enabled without enabling GuC submission!\n",
> +			 "enable_guc", i915_modparams.enable_guc);

If this is unsupported variant, then maybe we should clear slpc bit:

	i915_modparams.enable_guc &= ~ENABLE_GUC_SLPC;

> +	}
> +
>  	/* Verify HuC firmware availability */
>  	if (intel_uc_is_using_huc() && !intel_uc_fw_is_selected(huc_fw)) {
>  		DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
> diff --git a/drivers/gpu/drm/i915/intel_uc.h  
> b/drivers/gpu/drm/i915/intel_uc.h
> index 25d73ad..76139d3 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -59,4 +59,10 @@ static inline bool intel_uc_is_using_huc(void)
>  	return i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC;
>  }
> +static inline bool intel_uc_is_using_guc_slpc(void)
> +{
> +	GEM_BUG_ON(i915_modparams.enable_guc < 0);
> +	return i915_modparams.enable_guc & ENABLE_GUC_SLPC;
> +}
> +
>  #endif

In intel_uc_init_hw() we print summary, so maybe add there:

	dev_info(dev_priv->drm.dev, "GuC SLPC %s\n",
		 enableddisabled(USES_GUC_SLPC(dev_priv)));

Then we can move USES_GUC_SLPC() definition from patch 2 to 1.

With all that,

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
sagar.a.kamble@intel.com March 30, 2018, 3:26 p.m. UTC | #2
Thanks for the review. Will update with all suggestions in the next rev.

On 3/30/2018 6:07 PM, Michal Wajdeczko wrote:
> On Fri, 30 Mar 2018 10:31:46 +0200, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
>
>> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>>
>> GuC is currently being used for submission and HuC authentication.
>> Choices can be configured through enable_guc modparam. GuC SLPC is GT
>> Power and Performance management feature in GuC. Add another option to
>> enable_guc modparam to control SLPC.
>>
>> v1: Add early call to sanitize enable_guc_slpc in intel_guc_ucode_init
>>     Remove sanitize enable_guc_slpc call before firmware version check
>>     is performed. (ChrisW)
>>     Version check is added in next patch and that will be done as part
>>     of slpc_enable_sanitize function in the next patch. (Sagar) Updated
>>     slpc option sanitize function call for platforms without GuC 
>> support.
>>     This was caught by CI BAT.
>>
>> v2: Changed parameter to dev_priv for HAS_SLPC macro. (David)
>>     Code indentation based on checkpatch.
>>
>> v3: Rebase.
>>
>> v4: Moved sanitization of SLPC option post GuC load.
>>
>> v5: Removed function intel_slpc_enabled. Planning to rely only on kernel
>>     parameter. Moved sanitization prior to GuC load to use the parameter
>>     during SLPC state setup during to GuC load. (Sagar)
>>
>> v6: Commit message update. Rebase.
>>
>> v7: Moved SLPC option sanitization to intel_uc_sanitize_options.
>>
>> v8: Clearing SLPC option on GuC load failure. Change moved from later
>>     patch. (Sagar)
>>
>> v9: s/enable_slpc/enable_guc_slpc. Rebase w.r.t modparam change.
>>
>> v10: Rebase. Separate modparam is not needed now that we maintain all
>>      options in single param enable_guc.
>>
>> Suggested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>> Cc: Jeff McGee <jeff.mcgee@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_params.c |  5 +++--
>>  drivers/gpu/drm/i915/i915_params.h |  1 +
>>  drivers/gpu/drm/i915/intel_uc.c    | 23 +++++++++++++++++++----
>>  drivers/gpu/drm/i915/intel_uc.h    |  6 ++++++
>>  4 files changed, 29 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_params.c 
>> b/drivers/gpu/drm/i915/i915_params.c
>> index 08108ce..40b799b 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -150,9 +150,10 @@ i915_param_named_unsafe(edp_vswing, int, 0400,
>>      "2=default swing(400mV))");
>> i915_param_named_unsafe(enable_guc, int, 0400,
>> -    "Enable GuC load for GuC submission and/or HuC load. "
>> +    "Enable GuC load for GuC submission and/or HuC load and/or GuC 
>> SLPC. "
>>      "Required functionality can be selected using bitmask values. "
>> -    "(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)");
>> +    "(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load, "
>> +    "4=GuC SLPC)");
>
> Maybe to avoid later surprise, we should explicitly say that:
>
> +    "4=GuC SLPC [requires GuC submission])");
>
>> 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 c963603..2484925 100644
>> --- a/drivers/gpu/drm/i915/i915_params.h
>> +++ b/drivers/gpu/drm/i915/i915_params.h
>> @@ -32,6 +32,7 @@ struct drm_printer;
>> #define ENABLE_GUC_SUBMISSION        BIT(0)
>>  #define ENABLE_GUC_LOAD_HUC        BIT(1)
>> +#define ENABLE_GUC_SLPC            BIT(2)
>> #define I915_PARAMS_FOR_EACH(param) \
>>      param(char *, vbt_firmware, NULL) \
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 1cffaf7..0e4a97f 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -56,9 +56,15 @@ static int __get_platform_enable_guc(struct 
>> drm_i915_private *dev_priv)
>>      struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
>>      int enable_guc = 0;
>> -    /* Default is to enable GuC/HuC if we know their firmwares */
>> -    if (intel_uc_fw_is_selected(guc_fw))
>> +    /*
>> +     * Default is to enable GuC submission/SLPC/HuC if we know their
>> +     * firmwares
>> +     */
>> +    if (intel_uc_fw_is_selected(guc_fw)) {
>>          enable_guc |= ENABLE_GUC_SUBMISSION;
>> +        enable_guc |= ENABLE_GUC_SLPC;
>> +    }
>> +
>>      if (intel_uc_fw_is_selected(huc_fw))
>>          enable_guc |= ENABLE_GUC_LOAD_HUC;
>> @@ -110,10 +116,11 @@ static void sanitize_options_early(struct 
>> drm_i915_private *dev_priv)
>>      if (i915_modparams.enable_guc < 0)
>>          i915_modparams.enable_guc = 
>> __get_platform_enable_guc(dev_priv);
>> -    DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n",
>> +    DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s slpc:%s)\n",
>>               i915_modparams.enable_guc,
>>               yesno(intel_uc_is_using_guc_submission()),
>> -             yesno(intel_uc_is_using_huc()));
>> +             yesno(intel_uc_is_using_huc()),
>> +             yesno(intel_uc_is_using_guc_slpc()));
>>     /* Verify GuC firmware availability */
>>      if (intel_uc_is_using_guc() && !intel_uc_fw_is_selected(guc_fw)) {
>> @@ -123,6 +130,14 @@ static void sanitize_options_early(struct 
>> drm_i915_private *dev_priv)
>>                            "no GuC firmware");
>>      }
>> +    /* Verify GuC submission and SLPC dependency */
>> +    if (intel_uc_is_using_guc_slpc() &&
>> +        !intel_uc_is_using_guc_submission()) {
>> +        DRM_WARN("Incompatible option detected: %s=%d, "
>> +             "GuC SLPC enabled without enabling GuC submission!\n",
>> +             "enable_guc", i915_modparams.enable_guc);
>
> If this is unsupported variant, then maybe we should clear slpc bit:
>
>     i915_modparams.enable_guc &= ~ENABLE_GUC_SLPC;
>
>> +    }
>> +
>>      /* Verify HuC firmware availability */
>>      if (intel_uc_is_using_huc() && !intel_uc_fw_is_selected(huc_fw)) {
>>          DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
>> diff --git a/drivers/gpu/drm/i915/intel_uc.h 
>> b/drivers/gpu/drm/i915/intel_uc.h
>> index 25d73ad..76139d3 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.h
>> +++ b/drivers/gpu/drm/i915/intel_uc.h
>> @@ -59,4 +59,10 @@ static inline bool intel_uc_is_using_huc(void)
>>      return i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC;
>>  }
>> +static inline bool intel_uc_is_using_guc_slpc(void)
>> +{
>> +    GEM_BUG_ON(i915_modparams.enable_guc < 0);
>> +    return i915_modparams.enable_guc & ENABLE_GUC_SLPC;
>> +}
>> +
>>  #endif
>
> In intel_uc_init_hw() we print summary, so maybe add there:
>
>     dev_info(dev_priv->drm.dev, "GuC SLPC %s\n",
>          enableddisabled(USES_GUC_SLPC(dev_priv)));
>
> Then we can move USES_GUC_SLPC() definition from patch 2 to 1.
>
> With all that,
>
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 08108ce..40b799b 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -150,9 +150,10 @@  i915_param_named_unsafe(edp_vswing, int, 0400,
 	"2=default swing(400mV))");
 
 i915_param_named_unsafe(enable_guc, int, 0400,
-	"Enable GuC load for GuC submission and/or HuC load. "
+	"Enable GuC load for GuC submission and/or HuC load and/or GuC SLPC. "
 	"Required functionality can be selected using bitmask values. "
-	"(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load)");
+	"(-1=auto, 0=disable [default], 1=GuC submission, 2=HuC load, "
+	"4=GuC SLPC)");
 
 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 c963603..2484925 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -32,6 +32,7 @@  struct drm_printer;
 
 #define ENABLE_GUC_SUBMISSION		BIT(0)
 #define ENABLE_GUC_LOAD_HUC		BIT(1)
+#define ENABLE_GUC_SLPC			BIT(2)
 
 #define I915_PARAMS_FOR_EACH(param) \
 	param(char *, vbt_firmware, NULL) \
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 1cffaf7..0e4a97f 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -56,9 +56,15 @@  static int __get_platform_enable_guc(struct drm_i915_private *dev_priv)
 	struct intel_uc_fw *huc_fw = &dev_priv->huc.fw;
 	int enable_guc = 0;
 
-	/* Default is to enable GuC/HuC if we know their firmwares */
-	if (intel_uc_fw_is_selected(guc_fw))
+	/*
+	 * Default is to enable GuC submission/SLPC/HuC if we know their
+	 * firmwares
+	 */
+	if (intel_uc_fw_is_selected(guc_fw)) {
 		enable_guc |= ENABLE_GUC_SUBMISSION;
+		enable_guc |= ENABLE_GUC_SLPC;
+	}
+
 	if (intel_uc_fw_is_selected(huc_fw))
 		enable_guc |= ENABLE_GUC_LOAD_HUC;
 
@@ -110,10 +116,11 @@  static void sanitize_options_early(struct drm_i915_private *dev_priv)
 	if (i915_modparams.enable_guc < 0)
 		i915_modparams.enable_guc = __get_platform_enable_guc(dev_priv);
 
-	DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s)\n",
+	DRM_DEBUG_DRIVER("enable_guc=%d (submission:%s huc:%s slpc:%s)\n",
 			 i915_modparams.enable_guc,
 			 yesno(intel_uc_is_using_guc_submission()),
-			 yesno(intel_uc_is_using_huc()));
+			 yesno(intel_uc_is_using_huc()),
+			 yesno(intel_uc_is_using_guc_slpc()));
 
 	/* Verify GuC firmware availability */
 	if (intel_uc_is_using_guc() && !intel_uc_fw_is_selected(guc_fw)) {
@@ -123,6 +130,14 @@  static void sanitize_options_early(struct drm_i915_private *dev_priv)
 					      "no GuC firmware");
 	}
 
+	/* Verify GuC submission and SLPC dependency */
+	if (intel_uc_is_using_guc_slpc() &&
+	    !intel_uc_is_using_guc_submission()) {
+		DRM_WARN("Incompatible option detected: %s=%d, "
+			 "GuC SLPC enabled without enabling GuC submission!\n",
+			 "enable_guc", i915_modparams.enable_guc);
+	}
+
 	/* Verify HuC firmware availability */
 	if (intel_uc_is_using_huc() && !intel_uc_fw_is_selected(huc_fw)) {
 		DRM_WARN("Incompatible option detected: %s=%d, %s!\n",
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 25d73ad..76139d3 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -59,4 +59,10 @@  static inline bool intel_uc_is_using_huc(void)
 	return i915_modparams.enable_guc & ENABLE_GUC_LOAD_HUC;
 }
 
+static inline bool intel_uc_is_using_guc_slpc(void)
+{
+	GEM_BUG_ON(i915_modparams.enable_guc < 0);
+	return i915_modparams.enable_guc & ENABLE_GUC_SLPC;
+}
+
 #endif