[V10 5/8] drm/amd/pm: setup the framework to support Wifi RFI mitigation feature
Lazar, Lijo
Lijo.Lazar at amd.com
Mon Aug 28 02:14:52 UTC 2023
[AMD Official Use Only - General]
> 'j' was initially set as 'num_of_wbrf_ranges - 1'. So, I suppose 'num_of_wbrf_ranges' should be set as 'j' instead of 'j - 1'. Right?
Yes.
Thanks,
Lijo
________________________________
From: Quan, Evan <Evan.Quan at amd.com>
Sent: Monday, August 28, 2023 7:23:55 AM
To: Lazar, Lijo <Lijo.Lazar at amd.com>; lenb at kernel.org <lenb at kernel.org>; johannes at sipsolutions.net <johannes at sipsolutions.net>; davem at davemloft.net <davem at davemloft.net>; edumazet at google.com <edumazet at google.com>; kuba at kernel.org <kuba at kernel.org>; pabeni at redhat.com <pabeni at redhat.com>; Deucher, Alexander <Alexander.Deucher at amd.com>; rafael at kernel.org <rafael at kernel.org>; Limonciello, Mario <Mario.Limonciello at amd.com>
Cc: linux-kernel at vger.kernel.org <linux-kernel at vger.kernel.org>; linux-acpi at vger.kernel.org <linux-acpi at vger.kernel.org>; amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>; dri-devel at lists.freedesktop.org <dri-devel at lists.freedesktop.org>; linux-wireless at vger.kernel.org <linux-wireless at vger.kernel.org>; netdev at vger.kernel.org <netdev at vger.kernel.org>
Subject: RE: [V10 5/8] drm/amd/pm: setup the framework to support Wifi RFI mitigation feature
[AMD Official Use Only - General]
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar at amd.com>
> Sent: Friday, August 25, 2023 10:09 PM
> To: Quan, Evan <Evan.Quan at amd.com>; lenb at kernel.org;
> johannes at sipsolutions.net; davem at davemloft.net; edumazet at google.com;
> kuba at kernel.org; pabeni at redhat.com; Deucher, Alexander
> <Alexander.Deucher at amd.com>; rafael at kernel.org; Limonciello, Mario
> <Mario.Limonciello at amd.com>
> Cc: linux-kernel at vger.kernel.org; linux-acpi at vger.kernel.org; amd-
> gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org; linux-
> wireless at vger.kernel.org; netdev at vger.kernel.org
> Subject: Re: [V10 5/8] drm/amd/pm: setup the framework to support Wifi
> RFI mitigation feature
>
>
>
> On 8/25/2023 2:08 PM, Evan Quan wrote:
> > With WBRF feature supported, as a driver responding to the
> > frequencies, amdgpu driver is able to do shadow pstate switching to
> > mitigate possible interference(between its (G-)DDR memory clocks and
> > local radio module frequency bands used by Wifi 6/6e/7).
> >
> > Signed-off-by: Evan Quan <evan.quan at amd.com>
> > Reviewed-by: Mario Limonciello <mario.limonciello at amd.com>
> > --
> > v1->v2:
> > - update the prompt for feature support(Lijo)
> > v8->v9:
> > - update parameter document for smu_wbrf_event_handler(Simon)
> > v9->v10:
> > - correct the logics for wbrf range sorting(Lijo)
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 +
> > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 17 ++
> > drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 195
> ++++++++++++++++++
> > drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 23 +++
> > drivers/gpu/drm/amd/pm/swsmu/smu_internal.h | 3 +
> > 5 files changed, 240 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index a3b86b86dc47..2bfc9111ab00 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -247,6 +247,8 @@ extern int amdgpu_sg_display;
> >
> > extern int amdgpu_user_partt_mode;
> >
> > +extern int amdgpu_wbrf;
> > +
> > #define AMDGPU_VM_MAX_NUM_CTX 4096
> > #define AMDGPU_SG_THRESHOLD (256*1024*1024)
> > #define AMDGPU_WAIT_IDLE_TIMEOUT_IN_MS 3000
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 0593ef8fe0a6..1c574bd3b60d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -195,6 +195,7 @@ int amdgpu_use_xgmi_p2p = 1;
> > int amdgpu_vcnfw_log;
> > int amdgpu_sg_display = -1; /* auto */
> > int amdgpu_user_partt_mode =
> AMDGPU_AUTO_COMPUTE_PARTITION_MODE;
> > +int amdgpu_wbrf = -1;
> >
> > static void amdgpu_drv_delayed_reset_work_handler(struct work_struct
> > *work);
> >
> > @@ -981,6 +982,22 @@ module_param_named(user_partt_mode,
> amdgpu_user_partt_mode, uint, 0444);
> > module_param(enforce_isolation, bool, 0444);
> > MODULE_PARM_DESC(enforce_isolation, "enforce process isolation
> > between graphics and compute . enforce_isolation = on");
> >
> > +/**
> > + * DOC: wbrf (int)
> > + * Enable Wifi RFI interference mitigation feature.
> > + * Due to electrical and mechanical constraints there may be likely
> > +interference of
> > + * relatively high-powered harmonics of the (G-)DDR memory clocks
> > +with local radio
> > + * module frequency bands used by Wifi 6/6e/7. To mitigate the
> > +possible RFI interference,
> > + * with this feature enabled, PMFW will use either “shadowed P-State”
> > +or “P-State” based
> > + * on active list of frequencies in-use (to be avoided) as part of
> > +initial setting or
> > + * P-state transition. However, there may be potential performance
> > +impact with this
> > + * feature enabled.
> > + * (0 = disabled, 1 = enabled, -1 = auto (default setting, will be
> > +enabled if supported)) */ MODULE_PARM_DESC(wbrf,
> > + "Enable Wifi RFI interference mitigation (0 = disabled, 1 = enabled,
> > +-1 = auto(default)"); module_param_named(wbrf, amdgpu_wbrf, int,
> > +0444);
> > +
> > /* These devices are not supported by amdgpu.
> > * They are supported by the mach64, r128, radeon drivers
> > */
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > index ce41a8309582..bdfd234d1558 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> > @@ -1228,6 +1228,174 @@ static int
> smu_get_thermal_temperature_range(struct smu_context *smu)
> > return ret;
> > }
> >
> > +/**
> > + * smu_wbrf_handle_exclusion_ranges - consume the wbrf exclusion
> > +ranges
> > + *
> > + * @smu: smu_context pointer
> > + *
> > + * Retrieve the wbrf exclusion ranges and send them to PMFW for proper
> handling.
> > + * Returns 0 on success, error on failure.
> > + */
> > +static int smu_wbrf_handle_exclusion_ranges(struct smu_context *smu)
> > +{
> > + struct wbrf_ranges_in_out wbrf_exclusion = {0};
> > + struct exclusion_range *wifi_bands = wbrf_exclusion.band_list;
> > + struct amdgpu_device *adev = smu->adev;
> > + uint32_t num_of_wbrf_ranges = MAX_NUM_OF_WBRF_RANGES;
> > + uint64_t start, end;
> > + int ret, i, j;
> > +
> > + ret = acpi_amd_wbrf_retrieve_exclusions(adev->dev,
> &wbrf_exclusion);
> > + if (ret) {
> > + dev_err(adev->dev, "Failed to retrieve exclusion ranges!\n");
> > + return ret;
> > + }
> > +
> > + /*
> > + * The exclusion ranges array we got might be filled with holes and
> duplicate
> > + * entries. For example:
> > + * {(2400, 2500), (0, 0), (6882, 6962), (2400, 2500), (0, 0), (6117,
> 6189), (0, 0)...}
> > + * We need to do some sortups to eliminate those holes and duplicate
> entries.
> > + * Expected output: {(2400, 2500), (6117, 6189), (6882, 6962), (0,
> 0)...}
> > + */
> > + for (i = 0; i < num_of_wbrf_ranges; i++) {
> > + start = wifi_bands[i].start;
> > + end = wifi_bands[i].end;
> > +
> > + /* get the last valid entry to fill the intermediate hole */
> > + if (!start && !end) {
> > + for (j = num_of_wbrf_ranges - 1; j > i; j--)
> > + if (wifi_bands[j].start &&
> > + wifi_bands[j].end)
> > + break;
> > +
> > + /* no valid entry left */
> > + if (j <= i)
> > + break;
> > +
> > + wifi_bands[i].start = wifi_bands[j].start;
> > + wifi_bands[i].end = wifi_bands[j].end;
>
> start/end variables remain 0. Thus it won't have any effect on the loop below
> which looks for duplicates. start/end need to be reassigned here.
Ah, right. Thanks, I can fix that.
>
> > + wifi_bands[j].start = 0;
> > + wifi_bands[j].end = 0;
> > + num_of_wbrf_ranges--;
>
> Instead of decrementing by 1, this can be kept equal to j - 1 as jth entry is 0
> now.
'j' was initially set as 'num_of_wbrf_ranges - 1'. So, I suppose 'num_of_wbrf_ranges' should be set as 'j' instead of 'j - 1'. Right?
Evan
>
> Thanks,
> Lijo
>
> > + }
> > +
> > + /* eliminate duplicate entries */
> > + for (j = i + 1; j < num_of_wbrf_ranges; j++) {
> > + if ((wifi_bands[j].start == start) &&
> > + (wifi_bands[j].end == end)) {
> > + wifi_bands[j].start = 0;
> > + wifi_bands[j].end = 0;
> > + }
> > + }
> > + }
> > +
> > + /* Send the sorted wifi_bands to PMFW */
> > + ret = smu_set_wbrf_exclusion_ranges(smu, wifi_bands);
> > + /* Give it another chance */
> > + if (unlikely(ret == -EBUSY)) {
> > + mdelay(5);
> > + ret = smu_set_wbrf_exclusion_ranges(smu, wifi_bands);
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * smu_wbrf_event_handler - handle notify events
> > + *
> > + * @nb: notifier block
> > + * @action: event type
> > + * @_arg: event data
> > + *
> > + * Calls relevant amdgpu function in response to wbrf event
> > + * notification from kernel.
> > + */
> > +static int smu_wbrf_event_handler(struct notifier_block *nb,
> > + unsigned long action, void *_arg) {
> > + struct smu_context *smu = container_of(nb, struct smu_context,
> > + wbrf_notifier);
> > +
> > + switch (action) {
> > + case WBRF_CHANGED:
> > + smu_wbrf_handle_exclusion_ranges(smu);
> > + break;
> > + default:
> > + return NOTIFY_DONE;
> > + };
> > +
> > + return NOTIFY_OK;
> > +}
> > +
> > +/**
> > + * smu_wbrf_support_check - check wbrf support
> > + *
> > + * @smu: smu_context pointer
> > + *
> > + * Verifies the ACPI interface whether wbrf is supported.
> > + */
> > +static void smu_wbrf_support_check(struct smu_context *smu) {
> > + struct amdgpu_device *adev = smu->adev;
> > +
> > + smu->wbrf_supported = smu_is_asic_wbrf_supported(smu) &&
> > + !!amdgpu_wbrf &&
> > + acpi_amd_wbrf_supported_consumer(adev->dev);
> > +
> > + if (smu->wbrf_supported)
> > + dev_info(adev->dev, "RF interference mitigation is
> supported\n"); }
> > +
> > +/**
> > + * smu_wbrf_init - init driver wbrf support
> > + *
> > + * @smu: smu_context pointer
> > + *
> > + * Verifies the AMD ACPI interfaces and registers with the wbrf
> > + * notifier chain if wbrf feature is supported.
> > + * Returns 0 on success, error on failure.
> > + */
> > +static int smu_wbrf_init(struct smu_context *smu) {
> > + struct amdgpu_device *adev = smu->adev;
> > + int ret;
> > +
> > + if (!smu->wbrf_supported)
> > + return 0;
> > +
> > + smu->wbrf_notifier.notifier_call = smu_wbrf_event_handler;
> > + ret = acpi_amd_wbrf_register_notifier(&smu->wbrf_notifier);
> > + if (ret)
> > + return ret;
> > +
> > + /*
> > + * Some wifiband exclusion ranges may be already there
> > + * before our driver loaded. To make sure our driver
> > + * is awared of those exclusion ranges.
> > + */
> > + ret = smu_wbrf_handle_exclusion_ranges(smu);
> > + if (ret)
> > + dev_err(adev->dev, "Failed to handle wbrf exclusion
> ranges\n");
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * smu_wbrf_fini - tear down driver wbrf support
> > + *
> > + * @smu: smu_context pointer
> > + *
> > + * Unregisters with the wbrf notifier chain.
> > + */
> > +static void smu_wbrf_fini(struct smu_context *smu) {
> > + if (!smu->wbrf_supported)
> > + return;
> > +
> > + acpi_amd_wbrf_unregister_notifier(&smu->wbrf_notifier);
> > +}
> > +
> > static int smu_smc_hw_setup(struct smu_context *smu)
> > {
> > struct smu_feature *feature = &smu->smu_feature; @@ -1320,6
> > +1488,15 @@ static int smu_smc_hw_setup(struct smu_context *smu)
> > if (ret)
> > return ret;
> >
> > + /* Enable UclkShadow on wbrf supported */
> > + if (smu->wbrf_supported) {
> > + ret = smu_enable_uclk_shadow(smu, true);
> > + if (ret) {
> > + dev_err(adev->dev, "Failed to enable UclkShadow
> feature to support wbrf!\n");
> > + return ret;
> > + }
> > + }
> > +
> > /*
> > * With SCPM enabled, these actions(and relevant messages) are
> > * not needed and permitted.
> > @@ -1416,6 +1593,15 @@ static int smu_smc_hw_setup(struct
> smu_context *smu)
> > */
> > ret = smu_set_min_dcef_deep_sleep(smu,
> > smu->smu_table.boot_values.dcefclk
> / 100);
> > + if (ret) {
> > + dev_err(adev->dev, "Error setting min deepsleep dcefclk\n");
> > + return ret;
> > + }
> > +
> > + /* Init wbrf support. Properly setup the notifier */
> > + ret = smu_wbrf_init(smu);
> > + if (ret)
> > + dev_err(adev->dev, "Error during wbrf init call\n");
> >
> > return ret;
> > }
> > @@ -1471,6 +1657,13 @@ static int smu_hw_init(void *handle)
> > return ret;
> > }
> >
> > + /*
> > + * Check whether wbrf is supported. This needs to be done
> > + * before SMU setup starts since part of SMU configuration
> > + * relies on this.
> > + */
> > + smu_wbrf_support_check(smu);
> > +
> > if (smu->is_apu) {
> > ret = smu_set_gfx_imu_enable(smu);
> > if (ret)
> > @@ -1623,6 +1816,8 @@ static int smu_smc_hw_cleanup(struct
> smu_context *smu)
> > struct amdgpu_device *adev = smu->adev;
> > int ret = 0;
> >
> > + smu_wbrf_fini(smu);
> > +
> > cancel_work_sync(&smu->throttling_logging_work);
> > cancel_work_sync(&smu->interrupt_work);
> >
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> > b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> > index 6e2069dcb6b9..3eb1c72a76f1 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> > @@ -22,6 +22,8 @@
> > #ifndef __AMDGPU_SMU_H__
> > #define __AMDGPU_SMU_H__
> >
> > +#include <linux/acpi_amd_wbrf.h>
> > +
> > #include "amdgpu.h"
> > #include "kgd_pp_interface.h"
> > #include "dm_pp_interface.h"
> > @@ -575,6 +577,10 @@ struct smu_context
> > u32 debug_resp_reg;
> >
> > struct delayed_work swctf_delayed_work;
> > +
> > + /* data structures for wbrf feature support */
> > + bool wbrf_supported;
> > + struct notifier_block wbrf_notifier;
> > };
> >
> > struct i2c_adapter;
> > @@ -1356,6 +1362,23 @@ struct pptable_funcs {
> > * @init_pptable_microcode: Prepare the pptable microcode to upload
> via PSP
> > */
> > int (*init_pptable_microcode)(struct smu_context *smu);
> > +
> > + /**
> > + * @is_asic_wbrf_supported: check whether PMFW supports the wbrf
> feature
> > + */
> > + bool (*is_asic_wbrf_supported)(struct smu_context *smu);
> > +
> > + /**
> > + * @enable_uclk_shadow: Enable the uclk shadow feature on wbrf
> supported
> > + */
> > + int (*enable_uclk_shadow)(struct smu_context *smu,
> > + bool enablement);
> > +
> > + /**
> > + * @set_wbrf_exclusion_ranges: notify SMU the wifi bands occupied
> > + */
> > + int (*set_wbrf_exclusion_ranges)(struct smu_context *smu,
> > + struct exclusion_range
> *exclusion_ranges);
> > };
> >
> > typedef enum {
> > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> > b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> > index ceb13c838067..67d7495ab49e 100644
> > --- a/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_internal.h
> > @@ -97,6 +97,9 @@
> > #define smu_get_default_config_table_settings(smu, config_table)
> smu_ppt_funcs(get_default_config_table_settings, -EOPNOTSUPP,
> smu, config_table)
> > #define smu_set_config_table(smu, config_table)
> smu_ppt_funcs(set_config_table, -EOPNOTSUPP, smu, config_table)
> > #define smu_init_pptable_microcode(smu)
> smu_ppt_funcs(init_pptable_microcode, 0, smu)
> > +#define smu_is_asic_wbrf_supported(smu)
> smu_ppt_funcs(is_asic_wbrf_supported, false, smu)
> > +#define smu_enable_uclk_shadow(smu, enablement)
> smu_ppt_funcs(enable_uclk_shadow, 0, smu, enablement)
> > +#define smu_set_wbrf_exclusion_ranges(smu, exclusion_ranges)
> smu_ppt_funcs(set_wbrf_exclusion_ranges, -EOPNOTSUPP, smu,
> exclusion_ranges)
> >
> > #endif
> > #endif
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20230828/25e8ad10/attachment-0001.htm>
More information about the amd-gfx
mailing list