| Message ID | 20220704123252.2962020-4-michal.wilczynski@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ice: Support 5 layer tx scheduler topology | expand |
On 7/4/2022 5:32 AM, Michal Wilczynski wrote: > Introduce support for tx scheduler topology change, based on user > selection, from default 9-layer to 5-layer. In order for switch to be > successful there is a new NVM(version 3.20 or older) and DDP package(OS > Package 1.3.29 or older). > > Enable 5-layer topology switch in init path of the driver. To accomplish > that upload of the DDP package needs to be delayed, until change in Tx > topology is finished. To trigger the Tx change user selection should be > changed in NVM using devlink. Then the platform should be rebooted. > > Example way of switching NVM settings in devlink: > > Show: > devlink dev param show pci/0000:4b:00.0 > name txbalancing type driver-specific > values: > cmode permanent value true > > Set: > devlink dev param set pci/0000:4b:00.0 name txbalancing value true cmode > permanent Did I miss the devlink implementation? > Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> > --- <snip> > +static int ice_init_tx_topology(struct ice_hw *hw, > + const struct firmware *firmware) > +{ > + u8 num_tx_sched_layers = hw->num_tx_sched_layers; > + struct ice_pf *pf = hw->back; > + struct device *dev; > + u8 *buf_copy; > + int err; > + > + dev = ice_pf_to_dev(pf); > + /* ice_cfg_tx_topo buf argument is not a constant, > + * so we have to make a copy > + */ > + buf_copy = kmemdup(firmware->data, firmware->size, GFP_KERNEL); > + > + err = ice_cfg_tx_topo(hw, buf_copy, firmware->size); > + if (!err) { > + if (hw->num_tx_sched_layers > num_tx_sched_layers) > + dev_info(dev, "Transmit balancing feature disabled\n"); > + else > + dev_info(dev, "Transmit balancing feature enabled\n"); > + > + /* if there was a change in topology ice_cfg_tx_topo triggered > + * a CORER and we need to re-init hw. > + */ > + ice_deinit_hw(hw); > + err = ice_init_hw(hw); > + > + /* in this case we're not allowing safe mode */ > + devm_kfree(ice_hw_to_dev(hw), buf_copy); This should be a kfree now. > + > + return err; > + > + } else if (err == -EIO) { > + dev_info(dev, "DDP package does not support transmit balancing feature - please update to the latest DDP package and try again\n"); > + } > + > + kfree(buf_copy); > + > + return 0; > +} > +
On 7/12/2022 2:25 AM, Tony Nguyen wrote: > > > On 7/4/2022 5:32 AM, Michal Wilczynski wrote: >> Introduce support for tx scheduler topology change, based on user >> selection, from default 9-layer to 5-layer. In order for switch to be >> successful there is a new NVM(version 3.20 or older) and DDP package(OS >> Package 1.3.29 or older). >> >> Enable 5-layer topology switch in init path of the driver. To accomplish >> that upload of the DDP package needs to be delayed, until change in Tx >> topology is finished. To trigger the Tx change user selection should be >> changed in NVM using devlink. Then the platform should be rebooted. >> >> Example way of switching NVM settings in devlink: >> >> Show: >> devlink dev param show pci/0000:4b:00.0 >> name txbalancing type driver-specific >> values: >> cmode permanent value true >> >> Set: >> devlink dev param set pci/0000:4b:00.0 name txbalancing value true cmode >> permanent > > Did I miss the devlink implementation? It's not upstreamed yet, however I had to provide some example on how this might be configured. There was a review comment about that. I tested it by using epct with OOT driver first. Obviously I can't provide this method for upstream community. > >> Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> >> --- > > <snip> Can't figure out what's wrong here ? Signed-off seems to be okay. > > >> +static int ice_init_tx_topology(struct ice_hw *hw, >> + const struct firmware *firmware) >> +{ >> + u8 num_tx_sched_layers = hw->num_tx_sched_layers; >> + struct ice_pf *pf = hw->back; >> + struct device *dev; >> + u8 *buf_copy; >> + int err; >> + >> + dev = ice_pf_to_dev(pf); >> + /* ice_cfg_tx_topo buf argument is not a constant, >> + * so we have to make a copy >> + */ >> + buf_copy = kmemdup(firmware->data, firmware->size, GFP_KERNEL); >> + >> + err = ice_cfg_tx_topo(hw, buf_copy, firmware->size); >> + if (!err) { >> + if (hw->num_tx_sched_layers > num_tx_sched_layers) >> + dev_info(dev, "Transmit balancing feature disabled\n"); >> + else >> + dev_info(dev, "Transmit balancing feature enabled\n"); >> + >> + /* if there was a change in topology ice_cfg_tx_topo triggered >> + * a CORER and we need to re-init hw. >> + */ >> + ice_deinit_hw(hw); >> + err = ice_init_hw(hw); >> + >> + /* in this case we're not allowing safe mode */ >> + devm_kfree(ice_hw_to_dev(hw), buf_copy); > > This should be a kfree now. yep thanks > >> + >> + return err; >> + >> + } else if (err == -EIO) { >> + dev_info(dev, "DDP package does not support transmit >> balancing feature - please update to the latest DDP package and try >> again\n"); >> + } >> + >> + kfree(buf_copy); >> + >> + return 0; >> +} >> +
On 7/12/2022 12:51 AM, Wilczynski, Michal wrote: > > > On 7/12/2022 2:25 AM, Tony Nguyen wrote: >> >> >> On 7/4/2022 5:32 AM, Michal Wilczynski wrote: >>> Introduce support for tx scheduler topology change, based on user >>> selection, from default 9-layer to 5-layer. In order for switch to be >>> successful there is a new NVM(version 3.20 or older) and DDP package(OS >>> Package 1.3.29 or older). >>> >>> Enable 5-layer topology switch in init path of the driver. To accomplish >>> that upload of the DDP package needs to be delayed, until change in Tx >>> topology is finished. To trigger the Tx change user selection should be >>> changed in NVM using devlink. Then the platform should be rebooted. >>> >>> Example way of switching NVM settings in devlink: >>> >>> Show: >>> devlink dev param show pci/0000:4b:00.0 >>> name txbalancing type driver-specific >>> values: >>> cmode permanent value true >>> >>> Set: >>> devlink dev param set pci/0000:4b:00.0 name txbalancing value true cmode >>> permanent >> >> Did I miss the devlink implementation? > > It's not upstreamed yet, however I had to provide some example on how > this might be > configured. There was a review comment about that. I tested it by using > epct with OOT > driver first. Obviously I can't provide this method for upstream community. As there is a plan to send the devlink implementation soon, these should all be put together into a series that has the functionality as well as the ability to get/set it. If it is too large for a series, provide some method to view the other portions of code such as RFC patches or point to a repo that contains it. I don't believe functionality that would, currently, require installing the OOT driver to change modes is acceptable.
Hi, I created a new version that includes Lukasz commit with devlink param implementation. Thanks, Michał On 7/14/2022 10:31 PM, Tony Nguyen wrote: > > > On 7/12/2022 12:51 AM, Wilczynski, Michal wrote: >> >> >> On 7/12/2022 2:25 AM, Tony Nguyen wrote: >>> >>> >>> On 7/4/2022 5:32 AM, Michal Wilczynski wrote: >>>> Introduce support for tx scheduler topology change, based on user >>>> selection, from default 9-layer to 5-layer. In order for switch to be >>>> successful there is a new NVM(version 3.20 or older) and DDP >>>> package(OS >>>> Package 1.3.29 or older). >>>> >>>> Enable 5-layer topology switch in init path of the driver. To >>>> accomplish >>>> that upload of the DDP package needs to be delayed, until change in Tx >>>> topology is finished. To trigger the Tx change user selection >>>> should be >>>> changed in NVM using devlink. Then the platform should be rebooted. >>>> >>>> Example way of switching NVM settings in devlink: >>>> >>>> Show: >>>> devlink dev param show pci/0000:4b:00.0 >>>> name txbalancing type driver-specific >>>> values: >>>> cmode permanent value true >>>> >>>> Set: >>>> devlink dev param set pci/0000:4b:00.0 name txbalancing value true >>>> cmode >>>> permanent >>> >>> Did I miss the devlink implementation? >> >> It's not upstreamed yet, however I had to provide some example on how >> this might be >> configured. There was a review comment about that. I tested it by >> using epct with OOT >> driver first. Obviously I can't provide this method for upstream >> community. > > As there is a plan to send the devlink implementation soon, these > should all be put together into a series that has the functionality as > well as the ability to get/set it. If it is too large for a series, > provide some method to view the other portions of code such as RFC > patches or point to a repo that contains it. > > I don't believe functionality that would, currently, require > installing the OOT driver to change modes is acceptable.
diff --git a/drivers/net/ethernet/intel/ice/ice_common.c b/drivers/net/ethernet/intel/ice/ice_common.c index 8b65e2bfb160..167f9d5c345a 100644 --- a/drivers/net/ethernet/intel/ice/ice_common.c +++ b/drivers/net/ethernet/intel/ice/ice_common.c @@ -1535,6 +1535,8 @@ ice_aq_send_cmd(struct ice_hw *hw, struct ice_aq_desc *desc, void *buf, case ice_aqc_opc_set_port_params: case ice_aqc_opc_get_vlan_mode_parameters: case ice_aqc_opc_set_vlan_mode_parameters: + case ice_aqc_opc_set_tx_topo: + case ice_aqc_opc_get_tx_topo: case ice_aqc_opc_add_recipe: case ice_aqc_opc_recipe_to_profile: case ice_aqc_opc_get_recipe: diff --git a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c index 7c82f05621e3..02c7f3d2c027 100644 --- a/drivers/net/ethernet/intel/ice/ice_flex_pipe.c +++ b/drivers/net/ethernet/intel/ice/ice_flex_pipe.c @@ -1952,7 +1952,8 @@ int ice_cfg_tx_topo(struct ice_hw *hw, u8 *buf, u32 len) /* acquire global lock to make sure that set topology issued * by one PF */ - status = ice_acquire_global_cfg_lock(hw, ICE_RES_WRITE); + status = ice_acquire_res(hw, ICE_GLOBAL_CFG_LOCK_RES_ID, ICE_RES_WRITE, + ICE_GLOBAL_CFG_LOCK_TIMEOUT); if (status) { ice_debug(hw, ICE_DBG_INIT, "Failed to acquire global lock\n"); return status; diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index c1ac2f746714..1c975b034f6b 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -4453,11 +4453,11 @@ static char *ice_get_opt_fw_name(struct ice_pf *pf) /** * ice_request_fw - Device initialization routine * @pf: pointer to the PF instance + * @firmware: double pointer to firmware struct */ -static void ice_request_fw(struct ice_pf *pf) +static int ice_request_fw(struct ice_pf *pf, const struct firmware **firmware) { char *opt_fw_filename = ice_get_opt_fw_name(pf); - const struct firmware *firmware = NULL; struct device *dev = ice_pf_to_dev(pf); int err = 0; @@ -4466,29 +4466,98 @@ static void ice_request_fw(struct ice_pf *pf) * and warning messages for other errors. */ if (opt_fw_filename) { - err = firmware_request_nowarn(&firmware, opt_fw_filename, dev); - if (err) { - kfree(opt_fw_filename); - goto dflt_pkg_load; - } - - /* request for firmware was successful. Download to device */ - ice_load_pkg(firmware, pf); + err = firmware_request_nowarn(firmware, opt_fw_filename, dev); kfree(opt_fw_filename); - release_firmware(firmware); - return; + if (!err) + return err; } -dflt_pkg_load: - err = request_firmware(&firmware, ICE_DDP_PKG_FILE, dev); - if (err) { + err = request_firmware(firmware, ICE_DDP_PKG_FILE, dev); + if (err) dev_err(dev, "The DDP package file was not found or could not be read. Entering Safe Mode\n"); - return; + + return err; +} + +/** + * ice_init_tx_topology - performs Tx topology initialization + * @hw: pointer to the hardware structure + * @firmware: pointer to firmware structure + */ +static int ice_init_tx_topology(struct ice_hw *hw, + const struct firmware *firmware) +{ + u8 num_tx_sched_layers = hw->num_tx_sched_layers; + struct ice_pf *pf = hw->back; + struct device *dev; + u8 *buf_copy; + int err; + + dev = ice_pf_to_dev(pf); + /* ice_cfg_tx_topo buf argument is not a constant, + * so we have to make a copy + */ + buf_copy = kmemdup(firmware->data, firmware->size, GFP_KERNEL); + + err = ice_cfg_tx_topo(hw, buf_copy, firmware->size); + if (!err) { + if (hw->num_tx_sched_layers > num_tx_sched_layers) + dev_info(dev, "Transmit balancing feature disabled\n"); + else + dev_info(dev, "Transmit balancing feature enabled\n"); + + /* if there was a change in topology ice_cfg_tx_topo triggered + * a CORER and we need to re-init hw. + */ + ice_deinit_hw(hw); + err = ice_init_hw(hw); + + /* in this case we're not allowing safe mode */ + devm_kfree(ice_hw_to_dev(hw), buf_copy); + + return err; + + } else if (err == -EIO) { + dev_info(dev, "DDP package does not support transmit balancing feature - please update to the latest DDP package and try again\n"); + } + + kfree(buf_copy); + + return 0; +} + +/** + * ice_init_ddp_config - DDP related configuration + * @hw: pointer to the hardware structure + * @pf: pointer to pf structure + * + * This function loads DDP file from the disk, then initializes tx + * topology. At the end DDP package is loaded on the card. + */ +static int ice_init_ddp_config(struct ice_hw *hw, struct ice_pf *pf) +{ + struct device *dev = ice_pf_to_dev(pf); + const struct firmware *firmware = NULL; + int err; + + err = ice_request_fw(pf, &firmware); + if (err) + /* we can still operate in safe mode if DDP package load fails */ + return 0; + + err = ice_init_tx_topology(hw, firmware); + if (err) { + dev_err(dev, "ice_init_hw during change of tx topology failed: %d\n", + err); + release_firmware(firmware); + return err; } - /* request for firmware was successful. Download to device */ + /* Download firmware to device */ ice_load_pkg(firmware, pf); release_firmware(firmware); + + return 0; } /** @@ -4641,9 +4710,15 @@ ice_probe(struct pci_dev *pdev, const struct pci_device_id __always_unused *ent) ice_init_feature_support(pf); - ice_request_fw(pf); + err = ice_init_ddp_config(hw, pf); + + /* during topology change ice_init_hw may fail */ + if (err) { + err = -EIO; + goto err_exit_unroll; + } - /* if ice_request_fw fails, ICE_FLAG_ADV_FEATURES bit won't be + /* if ice_init_ddp_config fails, ICE_FLAG_ADV_FEATURES bit won't be * set in pf->state, which will cause ice_is_safe_mode to return * true */
Introduce support for tx scheduler topology change, based on user selection, from default 9-layer to 5-layer. In order for switch to be successful there is a new NVM(version 3.20 or older) and DDP package(OS Package 1.3.29 or older). Enable 5-layer topology switch in init path of the driver. To accomplish that upload of the DDP package needs to be delayed, until change in Tx topology is finished. To trigger the Tx change user selection should be changed in NVM using devlink. Then the platform should be rebooted. Example way of switching NVM settings in devlink: Show: devlink dev param show pci/0000:4b:00.0 name txbalancing type driver-specific values: cmode permanent value true Set: devlink dev param set pci/0000:4b:00.0 name txbalancing value true cmode permanent Signed-off-by: Michal Wilczynski <michal.wilczynski@intel.com> --- drivers/net/ethernet/intel/ice/ice_common.c | 2 + .../net/ethernet/intel/ice/ice_flex_pipe.c | 3 +- drivers/net/ethernet/intel/ice/ice_main.c | 113 +++++++++++++++--- 3 files changed, 98 insertions(+), 20 deletions(-)