diff mbox

[v9,6/8] drm/i915/guc : Introducing enable_guc module parameter

Message ID 1510358798-21566-7-git-send-email-sujaritha.sundaresan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sundaresan, Sujaritha Nov. 11, 2017, 12:06 a.m. UTC
Replacing enable_guc_submission with enable_guc modparam.
In effect enable_guc is replacing enable_guc_loading and
enable_guc_submission.

Suggested by : Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  2 +-
 drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
 drivers/gpu/drm/i915/i915_irq.c         |  2 +-
 drivers/gpu/drm/i915/i915_params.c      |  7 ++++---
 drivers/gpu/drm/i915/i915_params.h      |  2 +-
 drivers/gpu/drm/i915/intel_guc.c        |  2 +-
 drivers/gpu/drm/i915/intel_uc.c         | 35 +++++++++++++++++----------------
 7 files changed, 27 insertions(+), 25 deletions(-)

Comments

Michal Wajdeczko Nov. 12, 2017, 4:52 p.m. UTC | #1
On Sat, 11 Nov 2017 01:06:36 +0100, Sujaritha Sundaresan  
<sujaritha.sundaresan@intel.com> wrote:

> Replacing enable_guc_submission with enable_guc modparam.
> In effect enable_guc is replacing enable_guc_loading and
> enable_guc_submission.

Maybe it will be better if we replace guc_loading and guc_submission
modparams with single guc_enable in one patch, as now you are touching
almost the same places again...

>
> Suggested by : Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  2 +-
>  drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
>  drivers/gpu/drm/i915/i915_irq.c         |  2 +-
>  drivers/gpu/drm/i915/i915_params.c      |  7 ++++---
>  drivers/gpu/drm/i915/i915_params.h      |  2 +-
>  drivers/gpu/drm/i915/intel_guc.c        |  2 +-
>  drivers/gpu/drm/i915/intel_uc.c         | 35  
> +++++++++++++++++----------------
>  7 files changed, 27 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h  
> b/drivers/gpu/drm/i915/i915_drv.h
> index 798fa8a..ad73cee 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3225,7 +3225,7 @@ static inline unsigned int  
> i915_sg_segment_size(void)
>  #define HAS_HUC_UCODE(dev_priv)	((dev_priv)->huc.fw.path != NULL)
> #define NEEDS_GUC_FW(dev_priv) \
> -		(HAS_GUC(dev_priv) && i915_modparams.enable_guc_submission)
> +		(HAS_GUC(dev_priv) && i915_modparams.enable_guc)
> #define HAS_RESOURCE_STREAMER(dev_priv)  
> ((dev_priv)->info.has_resource_streamer)
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c  
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index 6a819c0..43210df 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -409,7 +409,7 @@ struct i915_gem_context *
>  	i915_gem_context_set_closed(ctx); /* not user accessible */
>  	i915_gem_context_clear_bannable(ctx);
>  	i915_gem_context_set_force_single_submission(ctx);
> -	if (!i915_modparams.enable_guc_submission)
> +	if (!i915_modparams.enable_guc)
>  		ctx->ring_size = 512 * PAGE_SIZE; /* Max ring buffer size */
> 	GEM_BUG_ON(i915_gem_context_is_kernel(ctx));
> diff --git a/drivers/gpu/drm/i915/i915_irq.c  
> b/drivers/gpu/drm/i915/i915_irq.c
> index a414bca..693b345 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1400,7 +1400,7 @@ static void snb_gt_irq_handler(struct  
> drm_i915_private *dev_priv,
> 	if (iir & (GT_RENDER_USER_INTERRUPT << test_shift)) {
>  		notify_ring(engine);
> -		tasklet |= i915_modparams.enable_guc_submission;
> +		tasklet |= i915_modparams.enable_guc;
>  	}
> 	if (tasklet)
> diff --git a/drivers/gpu/drm/i915/i915_params.c  
> b/drivers/gpu/drm/i915/i915_params.c
> index 1c25f45..51cf6bd 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -162,9 +162,10 @@ struct i915_params i915_modparams __read_mostly = {
>  	"(0=use value from vbt [default], 1=low power swing(200mV),"
>  	"2=default swing(400mV))");
> -i915_param_named_unsafe(enable_guc_submission, int, 0400,
> -	"Enable GuC submission "
> -	"(-1=auto, 0=never [default], 1=if available, 2=required)");
> +i915_param_named_unsafe(enable_guc, int, 0400,
> +	"Enable GuC submission and loading "
> +	"(-1=auto [default], 0=No GuC or HuC, 1=Load & use GuC, HuC on the  
> side"
> +	" 2=Load GuC only for HuC)");

What about direct description:

"Enable GuC"
-1=auto [default]
  0=disable GuC loading
  1=enable GuC submission
  2=enable HuC
  3=enable GuC submission and HuC

> i915_param_named(guc_log_level, int, 0400,
>  	"GuC firmware logging level (-1:disabled (default), 0-3:enabled)");
> diff --git a/drivers/gpu/drm/i915/i915_params.h  
> b/drivers/gpu/drm/i915/i915_params.h
> index 9e1e231..7bf4dce 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -44,7 +44,7 @@
>  	param(int, disable_power_well, -1) \
>  	param(int, enable_ips, 1) \
>  	param(int, invert_brightness, 0) \
> -	param(int, enable_guc_submission, 0) \
> +	param(int, enable_guc, -1) \
>  	param(int, guc_log_level, -1) \
>  	param(char *, guc_firmware_path, NULL) \
>  	param(char *, huc_firmware_path, NULL) \
> diff --git a/drivers/gpu/drm/i915/intel_guc.c  
> b/drivers/gpu/drm/i915/intel_guc.c
> index 823d0c2..629ef5d 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -128,7 +128,7 @@ void intel_guc_init_params(struct intel_guc *guc)
>  	}
> 	/* If GuC submission is enabled, set up additional parameters here */
> -	if (i915_modparams.enable_guc_submission) {
> +	if (i915_modparams.enable_guc) {
>  		u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
>  		u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
>  		u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> b/drivers/gpu/drm/i915/intel_uc.c
> index 320165a..980de7a 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -51,33 +51,34 @@ void intel_uc_sanitize_options(struct  
> drm_i915_private *dev_priv)
>  {
>  	/* Verify Hardware version */
>  	if (!HAS_GUC(dev_priv)) {
> -		if (i915_modparams.enable_guc_submission > 0)
> +		if (i915_modparams.enable_guc > 0)
> -			DRM_INFO("Ignoring option %s - no hardware",  
> "enable_guc_submission");
> +			DRM_INFO("Ignoring option %s - no hardware", "enable_guc");
> -		i915_modparams.enable_guc_submission = 0;
> +		i915_modparams.enable_guc = 0;
>  		return;
>  	}
> 	/* Verify Firmware version */
>  	if (!HAS_HUC_UCODE(dev_priv)) {
> -		if (i915_modparams.enable_guc_submission > 0) {
> +		if (i915_modparams.enable_guc > 0) {
> -			DRM_INFO("Ignoring option %s - no firmware",  
> "enable_guc_submission");
> +			DRM_INFO("Ignoring option %s - no firmware", "enable_guc");
> -			i915_modparams.enable_guc_submission = 0;
> +			i915_modparams.enable_guc = 0;
>  		return;
>  		}
>  	
> -		if (i915_modparams.enable_guc_submission < 0) {
> -			i915_modparams.enable_guc_submission = 0;
> -		return;
> +		if (i915_modparams.enable_guc < 0) {
> +			i915_modparams.enable_guc = 0;
> +			return;
>  		}
>  	}
> +
> 	/*
>  	 * A negative value means "use platform default" (enabled if we have
>  	 * survived to get here)
>  	 */
> -	if (i915_modparams.enable_guc_submission == 1)
> -		i915_modparams.enable_guc_submission = HAS_GUC(dev_priv);
> +	if (i915_modparams.enable_guc < 0)
> +		i915_modparams.enable_guc = 1;
>  }
> void intel_uc_init_early(struct drm_i915_private *dev_priv)
> @@ -169,7 +170,7 @@ int intel_uc_init_hw(struct drm_i915_private  
> *dev_priv)
>  	/* We need to notify the guc whenever we change the GGTT */
>  	i915_ggtt_enable_guc(dev_priv);
> -	if (i915_modparams.enable_guc_submission) {
> +	if (i915_modparams.enable_guc) {
>  		/*
>  		 * This is stuff we need to have available at fw load time
>  		 * if we are planning to enable submission later
> @@ -219,7 +220,7 @@ int intel_uc_init_hw(struct drm_i915_private  
> *dev_priv)
>  		goto err_log_capture;
> 	intel_huc_auth(&dev_priv->huc);
> -	if (i915_modparams.enable_guc_submission) {
> +	if (i915_modparams.enable_guc) {
>  		if (i915_modparams.guc_log_level >= 0)
>  			gen9_enable_guc_interrupts(dev_priv);
> @@ -229,7 +230,7 @@ int intel_uc_init_hw(struct drm_i915_private  
> *dev_priv)
>  	}
> 	dev_info(dev_priv->drm.dev, "GuC %s (firmware %s [version %u.%u])\n",
> -		 i915_modparams.enable_guc_submission ? "submission enabled" :
> +		 i915_modparams.enable_guc ? "submission enabled" :
>  							"loaded",
>  		 guc->fw.path,
>  		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
> @@ -251,15 +252,15 @@ int intel_uc_init_hw(struct drm_i915_private  
> *dev_priv)
>  err_log_capture:
>  	guc_capture_load_err_log(guc);
>  err_submission:
> -	if (i915_modparams.enable_guc_submission)
> +	if (i915_modparams.enable_guc)
>  		intel_guc_submission_fini(guc);
>  err_guc:
>  	i915_ggtt_disable_guc(dev_priv);
> -	if (i915_modparams.enable_guc_submission > 1) {
> +	if (i915_modparams.enable_guc > 1) {
>  		DRM_ERROR("GuC init failed. Firmware loading disabled.\n");
>  		ret = -EIO;
> -	} else if (i915_modparams.enable_guc_submission == 1) {
> +	} else if (i915_modparams.enable_guc == 1) {
>  		DRM_NOTE("Falling back from GuC submission to execlist mode\n");
>  		ret = 0;
>  	} else {
> @@ -276,12 +277,12 @@ void intel_uc_fini_hw(struct drm_i915_private  
> *dev_priv)
>  	if (!NEEDS_GUC_FW(dev_priv))
>  		return;
> -	if (i915_modparams.enable_guc_submission)
> +	if (i915_modparams.enable_guc)
>  		intel_guc_submission_disable(&dev_priv->guc);
> 	guc_disable_communication(&dev_priv->guc);
> -	if (i915_modparams.enable_guc_submission) {
> +	if (i915_modparams.enable_guc) {
>  		gen9_disable_guc_interrupts(dev_priv);
>  		intel_guc_submission_fini(&dev_priv->guc);
>  	}
Sundaresan, Sujaritha Nov. 22, 2017, 9:54 p.m. UTC | #2
On 11/12/2017 08:52 AM, Michal Wajdeczko wrote:
> On Sat, 11 Nov 2017 01:06:36 +0100, Sujaritha Sundaresan 
> <sujaritha.sundaresan@intel.com> wrote:
>
>> Replacing enable_guc_submission with enable_guc modparam.
>> In effect enable_guc is replacing enable_guc_loading and
>> enable_guc_submission.
>
> Maybe it will be better if we replace guc_loading and guc_submission
> modparams with single guc_enable in one patch, as now you are touching
> almost the same places again...
>
I will merge this with removing enable_guc_loading :)
>>
>> Suggested by : Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h         |  2 +-
>>  drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
>>  drivers/gpu/drm/i915/i915_irq.c         |  2 +-
>>  drivers/gpu/drm/i915/i915_params.c      |  7 ++++---
>>  drivers/gpu/drm/i915/i915_params.h      |  2 +-
>>  drivers/gpu/drm/i915/intel_guc.c        |  2 +-
>>  drivers/gpu/drm/i915/intel_uc.c         | 35 
>> +++++++++++++++++----------------
>>  7 files changed, 27 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 798fa8a..ad73cee 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3225,7 +3225,7 @@ static inline unsigned int 
>> i915_sg_segment_size(void)
>>  #define HAS_HUC_UCODE(dev_priv)    ((dev_priv)->huc.fw.path != NULL)
>> #define NEEDS_GUC_FW(dev_priv) \
>> -        (HAS_GUC(dev_priv) && i915_modparams.enable_guc_submission)
>> +        (HAS_GUC(dev_priv) && i915_modparams.enable_guc)
>> #define HAS_RESOURCE_STREAMER(dev_priv) 
>> ((dev_priv)->info.has_resource_streamer)
>> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
>> b/drivers/gpu/drm/i915/i915_gem_context.c
>> index 6a819c0..43210df 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>> @@ -409,7 +409,7 @@ struct i915_gem_context *
>>      i915_gem_context_set_closed(ctx); /* not user accessible */
>>      i915_gem_context_clear_bannable(ctx);
>>      i915_gem_context_set_force_single_submission(ctx);
>> -    if (!i915_modparams.enable_guc_submission)
>> +    if (!i915_modparams.enable_guc)
>>          ctx->ring_size = 512 * PAGE_SIZE; /* Max ring buffer size */
>>     GEM_BUG_ON(i915_gem_context_is_kernel(ctx));
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c 
>> b/drivers/gpu/drm/i915/i915_irq.c
>> index a414bca..693b345 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -1400,7 +1400,7 @@ static void snb_gt_irq_handler(struct 
>> drm_i915_private *dev_priv,
>>     if (iir & (GT_RENDER_USER_INTERRUPT << test_shift)) {
>>          notify_ring(engine);
>> -        tasklet |= i915_modparams.enable_guc_submission;
>> +        tasklet |= i915_modparams.enable_guc;
>>      }
>>     if (tasklet)
>> diff --git a/drivers/gpu/drm/i915/i915_params.c 
>> b/drivers/gpu/drm/i915/i915_params.c
>> index 1c25f45..51cf6bd 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -162,9 +162,10 @@ struct i915_params i915_modparams __read_mostly = {
>>      "(0=use value from vbt [default], 1=low power swing(200mV),"
>>      "2=default swing(400mV))");
>> -i915_param_named_unsafe(enable_guc_submission, int, 0400,
>> -    "Enable GuC submission "
>> -    "(-1=auto, 0=never [default], 1=if available, 2=required)");
>> +i915_param_named_unsafe(enable_guc, int, 0400,
>> +    "Enable GuC submission and loading "
>> +    "(-1=auto [default], 0=No GuC or HuC, 1=Load & use GuC, HuC on 
>> the side"
>> +    " 2=Load GuC only for HuC)");
>
> What about direct description:
>
> "Enable GuC"
> -1=auto [default]
>  0=disable GuC loading
>  1=enable GuC submission
>  2=enable HuC
>  3=enable GuC submission and HuC
>
>> i915_param_named(guc_log_level, int, 0400,
>>      "GuC firmware logging level (-1:disabled (default), 0-3:enabled)");
>> diff --git a/drivers/gpu/drm/i915/i915_params.h 
>> b/drivers/gpu/drm/i915/i915_params.h
>> index 9e1e231..7bf4dce 100644
>> --- a/drivers/gpu/drm/i915/i915_params.h
>> +++ b/drivers/gpu/drm/i915/i915_params.h
>> @@ -44,7 +44,7 @@
>>      param(int, disable_power_well, -1) \
>>      param(int, enable_ips, 1) \
>>      param(int, invert_brightness, 0) \
>> -    param(int, enable_guc_submission, 0) \
>> +    param(int, enable_guc, -1) \
>>      param(int, guc_log_level, -1) \
>>      param(char *, guc_firmware_path, NULL) \
>>      param(char *, huc_firmware_path, NULL) \
>> diff --git a/drivers/gpu/drm/i915/intel_guc.c 
>> b/drivers/gpu/drm/i915/intel_guc.c
>> index 823d0c2..629ef5d 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>> @@ -128,7 +128,7 @@ void intel_guc_init_params(struct intel_guc *guc)
>>      }
>>     /* If GuC submission is enabled, set up additional parameters 
>> here */
>> -    if (i915_modparams.enable_guc_submission) {
>> +    if (i915_modparams.enable_guc) {
>>          u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
>>          u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
>>          u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 320165a..980de7a 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -51,33 +51,34 @@ void intel_uc_sanitize_options(struct 
>> drm_i915_private *dev_priv)
>>  {
>>      /* Verify Hardware version */
>>      if (!HAS_GUC(dev_priv)) {
>> -        if (i915_modparams.enable_guc_submission > 0)
>> +        if (i915_modparams.enable_guc > 0)
>> -            DRM_INFO("Ignoring option %s - no hardware", 
>> "enable_guc_submission");
>> +            DRM_INFO("Ignoring option %s - no hardware", "enable_guc");
>> -        i915_modparams.enable_guc_submission = 0;
>> +        i915_modparams.enable_guc = 0;
>>          return;
>>      }
>>     /* Verify Firmware version */
>>      if (!HAS_HUC_UCODE(dev_priv)) {
>> -        if (i915_modparams.enable_guc_submission > 0) {
>> +        if (i915_modparams.enable_guc > 0) {
>> -            DRM_INFO("Ignoring option %s - no firmware", 
>> "enable_guc_submission");
>> +            DRM_INFO("Ignoring option %s - no firmware", "enable_guc");
>> -            i915_modparams.enable_guc_submission = 0;
>> +            i915_modparams.enable_guc = 0;
>>          return;
>>          }
>>
>> -        if (i915_modparams.enable_guc_submission < 0) {
>> -            i915_modparams.enable_guc_submission = 0;
>> -        return;
>> +        if (i915_modparams.enable_guc < 0) {
>> +            i915_modparams.enable_guc = 0;
>> +            return;
>>          }
>>      }
>> +
>>     /*
>>       * A negative value means "use platform default" (enabled if we 
>> have
>>       * survived to get here)
>>       */
>> -    if (i915_modparams.enable_guc_submission == 1)
>> -        i915_modparams.enable_guc_submission = HAS_GUC(dev_priv);
>> +    if (i915_modparams.enable_guc < 0)
>> +        i915_modparams.enable_guc = 1;
>>  }
>> void intel_uc_init_early(struct drm_i915_private *dev_priv)
>> @@ -169,7 +170,7 @@ int intel_uc_init_hw(struct drm_i915_private 
>> *dev_priv)
>>      /* We need to notify the guc whenever we change the GGTT */
>>      i915_ggtt_enable_guc(dev_priv);
>> -    if (i915_modparams.enable_guc_submission) {
>> +    if (i915_modparams.enable_guc) {
>>          /*
>>           * This is stuff we need to have available at fw load time
>>           * if we are planning to enable submission later
>> @@ -219,7 +220,7 @@ int intel_uc_init_hw(struct drm_i915_private 
>> *dev_priv)
>>          goto err_log_capture;
>>     intel_huc_auth(&dev_priv->huc);
>> -    if (i915_modparams.enable_guc_submission) {
>> +    if (i915_modparams.enable_guc) {
>>          if (i915_modparams.guc_log_level >= 0)
>>              gen9_enable_guc_interrupts(dev_priv);
>> @@ -229,7 +230,7 @@ int intel_uc_init_hw(struct drm_i915_private 
>> *dev_priv)
>>      }
>>     dev_info(dev_priv->drm.dev, "GuC %s (firmware %s [version 
>> %u.%u])\n",
>> -         i915_modparams.enable_guc_submission ? "submission enabled" :
>> +         i915_modparams.enable_guc ? "submission enabled" :
>>                              "loaded",
>>           guc->fw.path,
>>           guc->fw.major_ver_found, guc->fw.minor_ver_found);
>> @@ -251,15 +252,15 @@ int intel_uc_init_hw(struct drm_i915_private 
>> *dev_priv)
>>  err_log_capture:
>>      guc_capture_load_err_log(guc);
>>  err_submission:
>> -    if (i915_modparams.enable_guc_submission)
>> +    if (i915_modparams.enable_guc)
>>          intel_guc_submission_fini(guc);
>>  err_guc:
>>      i915_ggtt_disable_guc(dev_priv);
>> -    if (i915_modparams.enable_guc_submission > 1) {
>> +    if (i915_modparams.enable_guc > 1) {
>>          DRM_ERROR("GuC init failed. Firmware loading disabled.\n");
>>          ret = -EIO;
>> -    } else if (i915_modparams.enable_guc_submission == 1) {
>> +    } else if (i915_modparams.enable_guc == 1) {
>>          DRM_NOTE("Falling back from GuC submission to execlist 
>> mode\n");
>>          ret = 0;
>>      } else {
>> @@ -276,12 +277,12 @@ void intel_uc_fini_hw(struct drm_i915_private 
>> *dev_priv)
>>      if (!NEEDS_GUC_FW(dev_priv))
>>          return;
>> -    if (i915_modparams.enable_guc_submission)
>> +    if (i915_modparams.enable_guc)
>>          intel_guc_submission_disable(&dev_priv->guc);
>>     guc_disable_communication(&dev_priv->guc);
>> -    if (i915_modparams.enable_guc_submission) {
>> +    if (i915_modparams.enable_guc) {
>>          gen9_disable_guc_interrupts(dev_priv);
>>          intel_guc_submission_fini(&dev_priv->guc);
>>      }

Thanks for the review.

Regards,
Sujaritha
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 798fa8a..ad73cee 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3225,7 +3225,7 @@  static inline unsigned int i915_sg_segment_size(void)
 #define HAS_HUC_UCODE(dev_priv)	((dev_priv)->huc.fw.path != NULL)
 
 #define NEEDS_GUC_FW(dev_priv) \
-		(HAS_GUC(dev_priv) && i915_modparams.enable_guc_submission)
+		(HAS_GUC(dev_priv) && i915_modparams.enable_guc)
 
 #define HAS_RESOURCE_STREAMER(dev_priv) ((dev_priv)->info.has_resource_streamer)
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 6a819c0..43210df 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -409,7 +409,7 @@  struct i915_gem_context *
 	i915_gem_context_set_closed(ctx); /* not user accessible */
 	i915_gem_context_clear_bannable(ctx);
 	i915_gem_context_set_force_single_submission(ctx);
-	if (!i915_modparams.enable_guc_submission)
+	if (!i915_modparams.enable_guc)
 		ctx->ring_size = 512 * PAGE_SIZE; /* Max ring buffer size */
 
 	GEM_BUG_ON(i915_gem_context_is_kernel(ctx));
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a414bca..693b345 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1400,7 +1400,7 @@  static void snb_gt_irq_handler(struct drm_i915_private *dev_priv,
 
 	if (iir & (GT_RENDER_USER_INTERRUPT << test_shift)) {
 		notify_ring(engine);
-		tasklet |= i915_modparams.enable_guc_submission;
+		tasklet |= i915_modparams.enable_guc;
 	}
 
 	if (tasklet)
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 1c25f45..51cf6bd 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -162,9 +162,10 @@  struct i915_params i915_modparams __read_mostly = {
 	"(0=use value from vbt [default], 1=low power swing(200mV),"
 	"2=default swing(400mV))");
 
-i915_param_named_unsafe(enable_guc_submission, int, 0400,
-	"Enable GuC submission "
-	"(-1=auto, 0=never [default], 1=if available, 2=required)");
+i915_param_named_unsafe(enable_guc, int, 0400,
+	"Enable GuC submission and loading "
+	"(-1=auto [default], 0=No GuC or HuC, 1=Load & use GuC, HuC on the side"
+	" 2=Load GuC only for HuC)");
 
 i915_param_named(guc_log_level, int, 0400,
 	"GuC firmware logging level (-1:disabled (default), 0-3:enabled)");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 9e1e231..7bf4dce 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -44,7 +44,7 @@ 
 	param(int, disable_power_well, -1) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
-	param(int, enable_guc_submission, 0) \
+	param(int, enable_guc, -1) \
 	param(int, guc_log_level, -1) \
 	param(char *, guc_firmware_path, NULL) \
 	param(char *, huc_firmware_path, NULL) \
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 823d0c2..629ef5d 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -128,7 +128,7 @@  void intel_guc_init_params(struct intel_guc *guc)
 	}
 
 	/* If GuC submission is enabled, set up additional parameters here */
-	if (i915_modparams.enable_guc_submission) {
+	if (i915_modparams.enable_guc) {
 		u32 ads = guc_ggtt_offset(guc->ads_vma) >> PAGE_SHIFT;
 		u32 pgs = guc_ggtt_offset(dev_priv->guc.stage_desc_pool);
 		u32 ctx_in_16 = GUC_MAX_STAGE_DESCRIPTORS / 16;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 320165a..980de7a 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -51,33 +51,34 @@  void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
 {
 	/* Verify Hardware version */
 	if (!HAS_GUC(dev_priv)) {
-		if (i915_modparams.enable_guc_submission > 0)
+		if (i915_modparams.enable_guc > 0)
-			DRM_INFO("Ignoring option %s - no hardware", "enable_guc_submission");
+			DRM_INFO("Ignoring option %s - no hardware", "enable_guc");
-		i915_modparams.enable_guc_submission = 0;
+		i915_modparams.enable_guc = 0;
 		return;
 	}
 
 	/* Verify Firmware version */
 	if (!HAS_HUC_UCODE(dev_priv)) {
-		if (i915_modparams.enable_guc_submission > 0) {
+		if (i915_modparams.enable_guc > 0) {
-			DRM_INFO("Ignoring option %s - no firmware", "enable_guc_submission");
+			DRM_INFO("Ignoring option %s - no firmware", "enable_guc");
-			i915_modparams.enable_guc_submission = 0;
+			i915_modparams.enable_guc = 0;
 		return;
 		}
 	
-		if (i915_modparams.enable_guc_submission < 0) {
-			i915_modparams.enable_guc_submission = 0;
-		return;
+		if (i915_modparams.enable_guc < 0) {
+			i915_modparams.enable_guc = 0;
+			return;
 		}
 	}
+
 
 	/*
 	 * A negative value means "use platform default" (enabled if we have
 	 * survived to get here)
 	 */
 
-	if (i915_modparams.enable_guc_submission == 1)
-		i915_modparams.enable_guc_submission = HAS_GUC(dev_priv);
+	if (i915_modparams.enable_guc < 0)
+		i915_modparams.enable_guc = 1;
 }
 
 void intel_uc_init_early(struct drm_i915_private *dev_priv)
@@ -169,7 +170,7 @@  int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	/* We need to notify the guc whenever we change the GGTT */
 	i915_ggtt_enable_guc(dev_priv);
 
-	if (i915_modparams.enable_guc_submission) {
+	if (i915_modparams.enable_guc) {
 		/*
 		 * This is stuff we need to have available at fw load time
 		 * if we are planning to enable submission later
@@ -219,7 +220,7 @@  int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 		goto err_log_capture;
 
 	intel_huc_auth(&dev_priv->huc);
-	if (i915_modparams.enable_guc_submission) {
+	if (i915_modparams.enable_guc) {
 		if (i915_modparams.guc_log_level >= 0)
 			gen9_enable_guc_interrupts(dev_priv);
 
@@ -229,7 +230,7 @@  int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	}
 
 	dev_info(dev_priv->drm.dev, "GuC %s (firmware %s [version %u.%u])\n",
-		 i915_modparams.enable_guc_submission ? "submission enabled" :
+		 i915_modparams.enable_guc ? "submission enabled" :
 							"loaded",
 		 guc->fw.path,
 		 guc->fw.major_ver_found, guc->fw.minor_ver_found);
@@ -251,15 +252,15 @@  int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 err_log_capture:
 	guc_capture_load_err_log(guc);
 err_submission:
-	if (i915_modparams.enable_guc_submission)
+	if (i915_modparams.enable_guc)
 		intel_guc_submission_fini(guc);
 err_guc:
 	i915_ggtt_disable_guc(dev_priv);
 
-	if (i915_modparams.enable_guc_submission > 1) {
+	if (i915_modparams.enable_guc > 1) {
 		DRM_ERROR("GuC init failed. Firmware loading disabled.\n");
 		ret = -EIO;
-	} else if (i915_modparams.enable_guc_submission == 1) {
+	} else if (i915_modparams.enable_guc == 1) {
 		DRM_NOTE("Falling back from GuC submission to execlist mode\n");
 		ret = 0;
 	} else {
@@ -276,12 +277,12 @@  void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 	if (!NEEDS_GUC_FW(dev_priv))
 		return;
 
-	if (i915_modparams.enable_guc_submission)
+	if (i915_modparams.enable_guc)
 		intel_guc_submission_disable(&dev_priv->guc);
 
 	guc_disable_communication(&dev_priv->guc);
 
-	if (i915_modparams.enable_guc_submission) {
+	if (i915_modparams.enable_guc) {
 		gen9_disable_guc_interrupts(dev_priv);
 		intel_guc_submission_fini(&dev_priv->guc);
 	}