Re: [PATCH 1/2] drm/i915/huc: Introduce enable_huc parameter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




>-----Original Message-----
>From: Wajdeczko, Michal
>Sent: Thursday, December 15, 2016 10:32 AM
>To: Srivatsa, Anusha <anusha.srivatsa@xxxxxxxxx>
>Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Nikula, Jani <jani.nikula@xxxxxxxxx>
>Subject: Re:  [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@xxxxxxxxx>
>>
>> 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@xxxxxxxxx>
>> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
>> Cc: Arek <arkadiusz.hiler@xxxxxxxxx>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@xxxxxxxxx>
>> ---
>>  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@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux