[Intel-gfx] [PATCH] drm/i915/guc: Removed unused GuC parameters.

[Michel Thierry]

On 28/02/18 12:26, Michel Thierry wrote:
> On 28/02/18 10:42, Piotr Piórkowski wrote:
>> In the i915 driver, there is a function, intel_guc_init_params(),
>> which initializes the GuC parameter block which is passed into
>> the GuC. There is parameter GUC_CTL_DEVICE_INFO with values
>> GfxGtType and GfxCoreFamily unused by GuC.
>>
>> This patch remove GUC_CTL_DEVICE_INFO with GfxGtType and
>> GfxCoreFamily parameters and also unnecessary functions
>> get_gt_type() and get_core_family().
>>
> 
> Hi,
> 
> Looking at the fw code, you're partially right, GfxGtType is ignored... 
> but GfxCoreFamily isn't.
> 

Unless whoever wrote the fw was smart enough to forget to call the 
function that is reading GfxCoreFamily... I didn't count on that.

So yes, guc couldn't care less about what value we put in GfxCoreFamily.

Then I would also remove the defines from intel_guc_fwif.h:

--#define GUC_CORE_FAMILY_GEN9            12
--#define GUC_CORE_FAMILY_UNKNOWN         0x7fffffff

-Michel

> If you don't pass a known GfxCoreFamily, SLPC will be disabled (enabling 
> SLPC is in some manager's wish list). Also it seems nobody remembered to 
> add GUC_CORE_FAMILY_GEN10 for CNL.
> 
>> Signed-off-by: Piotr Piórkowski <piotr.piorkowski at intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_params.h    |  2 +-
>>   drivers/gpu/drm/i915/intel_guc.c      | 24 ------------------------
>>   drivers/gpu/drm/i915/intel_guc_fwif.h |  4 ----
>>   drivers/gpu/drm/i915/intel_uc.c       |  2 ++
>>   4 files changed, 3 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_params.h 
>> b/drivers/gpu/drm/i915/i915_params.h
>> index 430f5f9d0ff4..3deae1e22974 100644
>> --- a/drivers/gpu/drm/i915/i915_params.h
>> +++ b/drivers/gpu/drm/i915/i915_params.h
>> @@ -47,7 +47,7 @@ struct drm_printer;
>>       param(int, disable_power_well, -1) \
>>       param(int, enable_ips, 1) \
>>       param(int, invert_brightness, 0) \
>> -    param(int, enable_guc, 0) \
>> +    param(int, enable_guc, -1) \
> 
> This shouldn't be part of your patch, enable guc submission in a 2nd 
> patch, for example: [PATCH 2/2] HAX: Enable GuC submission for CI
> 
>>       param(int, guc_log_level, 0) \
>>       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 21140ccd7a97..5f6d84251830 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>> @@ -200,26 +200,6 @@ void intel_guc_fini(struct intel_guc *guc)
>>       guc_shared_data_destroy(guc);
>>   }
>> -static u32 get_gt_type(struct drm_i915_private *dev_priv)
>> -{
>> -    /* XXX: GT type based on PCI device ID? field seems unused by fw */
>> -    return 0;
>> -}
>> -
>> -static u32 get_core_family(struct drm_i915_private *dev_priv)
>> -{
>> -    u32 gen = INTEL_GEN(dev_priv);
>> -
>> -    switch (gen) {
>> -    case 9:
>> -        return GUC_CORE_FAMILY_GEN9;
>> -
>> -    default:
>> -        MISSING_CASE(gen);
>> -        return GUC_CORE_FAMILY_UNKNOWN;
>> -    }
>> -}
>> -
>>   static u32 get_log_verbosity_flags(void)
>>   {
>>       if (i915_modparams.guc_log_level > 0) {
>> @@ -246,10 +226,6 @@ void intel_guc_init_params(struct intel_guc *guc)
>>       memset(params, 0, sizeof(params));
>> -    params[GUC_CTL_DEVICE_INFO] |=
>> -        (get_gt_type(dev_priv) << GUC_CTL_GT_TYPE_SHIFT) |
>> -        (get_core_family(dev_priv) << GUC_CTL_CORE_FAMILY_SHIFT);
>> -
>>       /*
>>        * GuC ARAT increment is 10 ns. GuC default scheduler quantum is 
>> one
>>        * second. This ARAR is calculated by:
>> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
>> b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> index 6a10aa6f04d3..0f381de44722 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
>> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> @@ -81,10 +81,6 @@
>>   #define GUC_CTL_ARAT_HIGH        1
>>   #define GUC_CTL_ARAT_LOW        2
>> -#define GUC_CTL_DEVICE_INFO        3
>> -#define   GUC_CTL_GT_TYPE_SHIFT        0
>> -#define   GUC_CTL_CORE_FAMILY_SHIFT    7
>> -
>>   #define GUC_CTL_LOG_PARAMS        4
>>   #define   GUC_LOG_VALID            (1 << 0)
>>   #define   GUC_LOG_NOTIFY_ON_HALF_FULL    (1 << 1)
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 9f1bac6398fb..b48056fb769d 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -63,6 +63,8 @@ static int __get_platform_enable_guc(struct 
>> drm_i915_private *dev_priv)
>>           enable_guc |= ENABLE_GUC_LOAD_HUC;
>>       /* Any platform specific fine-tuning can be done here */
>> +    if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
>> +        enable_guc = 0;
> 
> This is also part of [PATCH 2/2] HAX: Enable GuC submission for CI
> 
>>       return enable_guc;
>>   }
>>
> 
> -Michel