[1/2] drm/i915/huc: Introduce enable_huc parameter
diff mbox

Message ID 1481826297-9710-1-git-send-email-anusha.srivatsa@intel.com
State New
Headers show

Commit Message

Srivatsa, Anusha Dec. 15, 2016, 6:24 p.m. UTC
From: Anusha Srivatsa <anusha.srivatsa@intel.com>

Add a new kernel parameter: enable_huc. This parameter
controls HuC functionality. If this parameter
is set to 1, it suggests that HuC functionality is being
used and also that the GuC has to be loaded. GuC has to be
loaded in order to authenticate the HuC.

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Arek <arkadiusz.hiler@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/i915_params.c      | 5 +++++
 drivers/gpu/drm/i915/i915_params.h      | 1 +
 drivers/gpu/drm/i915/intel_huc_loader.c | 4 ++++
 3 files changed, 10 insertions(+)

Comments

Michal Wajdeczko Dec. 15, 2016, 6:31 p.m. UTC | #1
On Thu, Dec 15, 2016 at 10:24:56AM -0800, anushasr wrote:
> From: Anusha Srivatsa <anusha.srivatsa@intel.com>
> 
> Add a new kernel parameter: enable_huc. This parameter
> controls HuC functionality. If this parameter
> is set to 1, it suggests that HuC functionality is being
> used and also that the GuC has to be loaded. GuC has to be
> loaded in order to authenticate the HuC.
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Arek <arkadiusz.hiler@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_params.c      | 5 +++++
>  drivers/gpu/drm/i915/i915_params.h      | 1 +
>  drivers/gpu/drm/i915/intel_huc_loader.c | 4 ++++
>  3 files changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 0e280fb..1d9c306 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -56,6 +56,7 @@ struct i915_params i915 __read_mostly = {
>  	.verbose_state_checks = 1,
>  	.nuclear_pageflip = 0,
>  	.edp_vswing = 0,
> +	.enable_huc = 1,
>  	.enable_guc_loading = 0,
>  	.enable_guc_submission = 0,
>  	.guc_log_level = -1,
> @@ -216,6 +217,10 @@ MODULE_PARM_DESC(edp_vswing,
>  		 "(0=use value from vbt [default], 1=low power swing(200mV),"
>  		 "2=default swing(400mV))");
>  
> +module_param_named(enable_huc, i915.enable_huc, int, 0400);
> +MODULE_PARM_DESC(enable_huc,
> +		"Enable HuC usage. If enabled,load GuC (1:enabled (default), 0:disabled)");
> +
>  module_param_named_unsafe(enable_guc_loading, i915.enable_guc_loading, int, 0400);
>  MODULE_PARM_DESC(enable_guc_loading,
>  		"Enable GuC firmware loading "
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index 8e433de..7b0523b 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -44,6 +44,7 @@ struct i915_params {
>  	int disable_power_well;
>  	int enable_ips;
>  	int invert_brightness;
> +	int enable_huc;
>  	int enable_guc_loading;
>  	int enable_guc_submission;
>  	int guc_log_level;
> diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
> index 8137979..a545f76 100644
> --- a/drivers/gpu/drm/i915/intel_huc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c
> @@ -166,6 +166,10 @@ void intel_huc_init(struct drm_i915_private *dev_priv)
>  	huc_fw->load_status = INTEL_UC_FIRMWARE_NONE;
>  	huc_fw->fw_type = INTEL_UC_FW_TYPE_HUC;
>  
> +	if (!i915.enable_huc)
> +		DRM_ERROR("HuC not enabled. Will not be loaded\n");

Is this really an error ? Maybe DRM_INFO ?
What if value of this param conflicts with HAS_HUC_UCODE ?
Shouldn't we have some 'sanitize' code for this ?
Or maybe we should check this param after HAS_HUC check below ?

Michal

> +		return;
> +
>  	if (!HAS_HUC_UCODE(dev_priv))
>  		return;
>  
> -- 
> 2.7.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Srivatsa, Anusha Dec. 15, 2016, 6:35 p.m. UTC | #2
>-----Original Message-----
>From: Wajdeczko, Michal
>Sent: Thursday, December 15, 2016 10:32 AM
>To: Srivatsa, Anusha <anusha.srivatsa@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Nikula, Jani <jani.nikula@intel.com>
>Subject: Re: [Intel-gfx] [PATCH 1/2] drm/i915/huc: Introduce enable_huc
>parameter
>
>On Thu, Dec 15, 2016 at 10:24:56AM -0800, anushasr wrote:
>> From: Anusha Srivatsa <anusha.srivatsa@intel.com>
>>
>> Add a new kernel parameter: enable_huc. This parameter controls HuC
>> functionality. If this parameter is set to 1, it suggests that HuC
>> functionality is being used and also that the GuC has to be loaded.
>> GuC has to be loaded in order to authenticate the HuC.
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>> Cc: Arek <arkadiusz.hiler@intel.com>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_params.c      | 5 +++++
>>  drivers/gpu/drm/i915/i915_params.h      | 1 +
>>  drivers/gpu/drm/i915/intel_huc_loader.c | 4 ++++
>>  3 files changed, 10 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_params.c
>> b/drivers/gpu/drm/i915/i915_params.c
>> index 0e280fb..1d9c306 100644
>> --- a/drivers/gpu/drm/i915/i915_params.c
>> +++ b/drivers/gpu/drm/i915/i915_params.c
>> @@ -56,6 +56,7 @@ struct i915_params i915 __read_mostly = {
>>  	.verbose_state_checks = 1,
>>  	.nuclear_pageflip = 0,
>>  	.edp_vswing = 0,
>> +	.enable_huc = 1,
>>  	.enable_guc_loading = 0,
>>  	.enable_guc_submission = 0,
>>  	.guc_log_level = -1,
>> @@ -216,6 +217,10 @@ MODULE_PARM_DESC(edp_vswing,
>>  		 "(0=use value from vbt [default], 1=low power swing(200mV),"
>>  		 "2=default swing(400mV))");
>>
>> +module_param_named(enable_huc, i915.enable_huc, int, 0400);
>> +MODULE_PARM_DESC(enable_huc,
>> +		"Enable HuC usage. If enabled,load GuC (1:enabled (default),
>> +0:disabled)");
>> +
>>  module_param_named_unsafe(enable_guc_loading,
>> i915.enable_guc_loading, int, 0400);
>MODULE_PARM_DESC(enable_guc_loading,
>>  		"Enable GuC firmware loading "
>> diff --git a/drivers/gpu/drm/i915/i915_params.h
>> b/drivers/gpu/drm/i915/i915_params.h
>> index 8e433de..7b0523b 100644
>> --- a/drivers/gpu/drm/i915/i915_params.h
>> +++ b/drivers/gpu/drm/i915/i915_params.h
>> @@ -44,6 +44,7 @@ struct i915_params {
>>  	int disable_power_well;
>>  	int enable_ips;
>>  	int invert_brightness;
>> +	int enable_huc;
>>  	int enable_guc_loading;
>>  	int enable_guc_submission;
>>  	int guc_log_level;
>> diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c
>> b/drivers/gpu/drm/i915/intel_huc_loader.c
>> index 8137979..a545f76 100644
>> --- a/drivers/gpu/drm/i915/intel_huc_loader.c
>> +++ b/drivers/gpu/drm/i915/intel_huc_loader.c
>> @@ -166,6 +166,10 @@ void intel_huc_init(struct drm_i915_private *dev_priv)
>>  	huc_fw->load_status = INTEL_UC_FIRMWARE_NONE;
>>  	huc_fw->fw_type = INTEL_UC_FW_TYPE_HUC;
>>
>> +	if (!i915.enable_huc)
>> +		DRM_ERROR("HuC not enabled. Will not be loaded\n");
>
>Is this really an error ? Maybe DRM_INFO ?
>What if value of this param conflicts with HAS_HUC_UCODE ?
>Shouldn't we have some 'sanitize' code for this ?
>Or maybe we should check this param after HAS_HUC check below ?
>
DRM_Info might be a good idea. But I feel this is the right place for this. Since we first check if HuC is there or not, if not there then return immediately. If present then go ahead and do the check below.

Anusha
>Michal
>
>> +		return;
>> +
>>  	if (!HAS_HUC_UCODE(dev_priv))
>>  		return;
>>
>> --
>> 2.7.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 0e280fb..1d9c306 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -56,6 +56,7 @@  struct i915_params i915 __read_mostly = {
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
 	.edp_vswing = 0,
+	.enable_huc = 1,
 	.enable_guc_loading = 0,
 	.enable_guc_submission = 0,
 	.guc_log_level = -1,
@@ -216,6 +217,10 @@  MODULE_PARM_DESC(edp_vswing,
 		 "(0=use value from vbt [default], 1=low power swing(200mV),"
 		 "2=default swing(400mV))");
 
+module_param_named(enable_huc, i915.enable_huc, int, 0400);
+MODULE_PARM_DESC(enable_huc,
+		"Enable HuC usage. If enabled,load GuC (1:enabled (default), 0:disabled)");
+
 module_param_named_unsafe(enable_guc_loading, i915.enable_guc_loading, int, 0400);
 MODULE_PARM_DESC(enable_guc_loading,
 		"Enable GuC firmware loading "
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 8e433de..7b0523b 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -44,6 +44,7 @@  struct i915_params {
 	int disable_power_well;
 	int enable_ips;
 	int invert_brightness;
+	int enable_huc;
 	int enable_guc_loading;
 	int enable_guc_submission;
 	int guc_log_level;
diff --git a/drivers/gpu/drm/i915/intel_huc_loader.c b/drivers/gpu/drm/i915/intel_huc_loader.c
index 8137979..a545f76 100644
--- a/drivers/gpu/drm/i915/intel_huc_loader.c
+++ b/drivers/gpu/drm/i915/intel_huc_loader.c
@@ -166,6 +166,10 @@  void intel_huc_init(struct drm_i915_private *dev_priv)
 	huc_fw->load_status = INTEL_UC_FIRMWARE_NONE;
 	huc_fw->fw_type = INTEL_UC_FW_TYPE_HUC;
 
+	if (!i915.enable_huc)
+		DRM_ERROR("HuC not enabled. Will not be loaded\n");
+		return;
+
 	if (!HAS_HUC_UCODE(dev_priv))
 		return;