On Tue, 17 Oct 2023, Ma Jun wrote:

> From: Evan Quan <quanlia...@hotmail.com>
> 
> Fulfill the SMU13.0.7 support for Wifi RFI mitigation feature.
> 
> Signed-off-by: Evan Quan <quanlia...@hotmail.com>
> Reviewed-by: Mario Limonciello <mario.limoncie...@amd.com>
> Signed-off-by: Ma Jun <jun....@amd.com>
> --
> v10->v11:
>   - downgrade the prompt level on message failure(Lijo)
> ---
>  .../drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c  | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c 
> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> index 62f2886ab4df..c5736fb3cf6d 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> @@ -126,6 +126,7 @@ static struct cmn2asic_msg_mapping 
> smu_v13_0_7_message_map[SMU_MSG_MAX_COUNT] =
>       MSG_MAP(AllowGpo,                       PPSMC_MSG_SetGpoAllow,          
>  0),
>       MSG_MAP(GetPptLimit,                    PPSMC_MSG_GetPptLimit,          
>        0),
>       MSG_MAP(NotifyPowerSource,              PPSMC_MSG_NotifyPowerSource,    
>        0),
> +     MSG_MAP(EnableUCLKShadow,               PPSMC_MSG_EnableUCLKShadow,     
>        0),
>  };
>  
>  static struct cmn2asic_mapping smu_v13_0_7_clk_map[SMU_CLK_COUNT] = {
> @@ -207,6 +208,7 @@ static struct cmn2asic_mapping 
> smu_v13_0_7_table_map[SMU_TABLE_COUNT] = {
>       TAB_MAP(ACTIVITY_MONITOR_COEFF),
>       [SMU_TABLE_COMBO_PPTABLE] = {1, TABLE_COMBO_PPTABLE},
>       TAB_MAP(OVERDRIVE),
> +     TAB_MAP(WIFIBAND),
>  };
>  
>  static struct cmn2asic_mapping 
> smu_v13_0_7_pwr_src_map[SMU_POWER_SOURCE_COUNT] = {
> @@ -503,6 +505,9 @@ static int smu_v13_0_7_tables_init(struct smu_context 
> *smu)
>                      AMDGPU_GEM_DOMAIN_VRAM);
>       SMU_TABLE_INIT(tables, SMU_TABLE_COMBO_PPTABLE, 
> MP0_MP1_DATA_REGION_SIZE_COMBOPPTABLE,
>                       PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM);
> +     SMU_TABLE_INIT(tables, SMU_TABLE_WIFIBAND,
> +                    sizeof(WifiBandEntryTable_t), PAGE_SIZE,
> +                    AMDGPU_GEM_DOMAIN_VRAM);
>  
>       smu_table->metrics_table = kzalloc(sizeof(SmuMetricsExternal_t), 
> GFP_KERNEL);
>       if (!smu_table->metrics_table)
> @@ -2179,6 +2184,57 @@ static int smu_v13_0_7_set_df_cstate(struct 
> smu_context *smu,
>                                              NULL);
>  }
>  
> +static bool smu_v13_0_7_wbrf_support_check(struct smu_context *smu)
> +{
> +     return smu->smc_fw_version > 0x00524600;
> +}
> +
> +static int smu_v13_0_7_set_wbrf_exclusion_ranges(struct smu_context *smu,
> +                                              struct freq_band_range 
> *exclusion_ranges)
> +{
> +     WifiBandEntryTable_t wifi_bands;
> +     int valid_entries = 0;
> +     int ret, i;
> +
> +     memset(&wifi_bands, 0, sizeof(wifi_bands));
> +     for (i = 0; i < ARRAY_SIZE(wifi_bands.WifiBandEntry); i++) {
> +             if (!exclusion_ranges[i].start &&
> +                 !exclusion_ranges[i].end)

After having seen this construct nth time, I think you should have a 
static inline function for this check with a proper name.

> +                     break;
> +
> +             /* PMFW expects the inputs to be in Mhz unit */
> +             wifi_bands.WifiBandEntry[valid_entries].LowFreq =
> +                     DIV_ROUND_DOWN_ULL(exclusion_ranges[i].start, 
> HZ_IN_MHZ);
> +             wifi_bands.WifiBandEntry[valid_entries++].HighFreq =
> +                     DIV_ROUND_UP_ULL(exclusion_ranges[i].end, HZ_IN_MHZ);
> +     }
> +     wifi_bands.WifiBandEntryNum = valid_entries;
> +
> +     /*
> +      * Per confirm with PMFW team, WifiBandEntryNum = 0 is a valid setting.
> +      * Considering the scenarios below:
> +      * - At first the wifi device adds an exclusion range e.g. (2400,2500) 
> to
> +      *   BIOS and our driver gets notified. We will set WifiBandEntryNum = 1
> +      *   and pass the WifiBandEntry (2400, 2500) to PMFW.
> +      *
> +      * - Later the wifi device removes the wifiband list added above and
> +      *   our driver gets notified again. At this time, driver will set
> +      *   WifiBandEntryNum = 0 and pass an empty WifiBandEntry list to PMFW.
> +      *   - PMFW may still need to do some uclk shadow update(e.g. switching
> +      *     from shadow clock back to primary clock) on receiving this.
> +      */
> +
> +     ret = smu_cmn_update_table(smu,
> +                                SMU_TABLE_WIFIBAND,
> +                                0,
> +                                (void *)(&wifi_bands),
> +                                true);
> +     if (ret)
> +             dev_warn(smu->adev->dev, "Failed to set wifiband!");
> +
> +     return ret;
> +}

Is this whole function duplicate of the one in the other file? Don't 
duplicate code like this but create reusable functions properly.

-- 
 i.

> +
>  static const struct pptable_funcs smu_v13_0_7_ppt_funcs = {
>       .get_allowed_feature_mask = smu_v13_0_7_get_allowed_feature_mask,
>       .set_default_dpm_table = smu_v13_0_7_set_default_dpm_table,
> @@ -2247,6 +2303,9 @@ static const struct pptable_funcs smu_v13_0_7_ppt_funcs 
> = {
>       .set_mp1_state = smu_v13_0_7_set_mp1_state,
>       .set_df_cstate = smu_v13_0_7_set_df_cstate,
>       .gpo_control = smu_v13_0_gpo_control,
> +     .is_asic_wbrf_supported = smu_v13_0_7_wbrf_support_check,
> +     .enable_uclk_shadow = smu_v13_0_enable_uclk_shadow,
> +     .set_wbrf_exclusion_ranges = smu_v13_0_7_set_wbrf_exclusion_ranges,
>  };
>  
>  void smu_v13_0_7_set_ppt_funcs(struct smu_context *smu)
> 

Reply via email to