[dpdk-dev] [PATCH 04/28] net/ice/base: read PSM clock frequency from register

Zhang, Qi Z qi.z.zhang at intel.com
Fri Mar 20 08:14:28 CET 2020


Hi Kevin:

> -----Original Message-----
> From: Kevin Traynor <ktraynor at redhat.com>
> Sent: Monday, March 9, 2020 11:45 PM
> To: Zhang, Qi Z <qi.z.zhang at intel.com>; Yang, Qiming
> <qiming.yang at intel.com>; Xing, Beilei <beilei.xing at intel.com>
> Cc: Ye, Xiaolong <xiaolong.ye at intel.com>; dev at dpdk.org; Shelton, Benjamin H
> <benjamin.h.shelton at intel.com>; Stillwell Jr, Paul M
> <paul.m.stillwell.jr at intel.com>
> Subject: Re: [dpdk-dev] [PATCH 04/28] net/ice/base: read PSM clock frequency
> from register
> 
> On 09/03/2020 11:43, Qi Zhang wrote:
> > Read the GLGEN_CLKSTAT_SRC register to determine which PSM clock
> > frequency is selected.  This ensures that the rate limiter profile
> > calculations will be correct.
> >
> 
> This seems to be a fix whereby a default and possibly incorrect frequency was
> used previously. In that case, it should have a Fixes tag (and probably stable tag
> too)

OK, will add fixline
> 
> > Signed-off-by: Ben Shelton <benjamin.h.shelton at intel.com>
> > Signed-off-by: Paul M Stillwell Jr <paul.m.stillwell.jr at intel.com>
> > Signed-off-by: Qi Zhang <qi.z.zhang at intel.com>
> > ---
> >  drivers/net/ice/base/ice_common.c |  1 +
> > drivers/net/ice/base/ice_sched.c  | 59
> > +++++++++++++++++++++++++++++++++------
> >  drivers/net/ice/base/ice_sched.h  |  7 ++++-
> >  drivers/net/ice/base/ice_type.h   |  4 ++-
> >  4 files changed, 61 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/ice/base/ice_common.c
> > b/drivers/net/ice/base/ice_common.c
> > index 786e99d21..9ef1aeef2 100644
> > --- a/drivers/net/ice/base/ice_common.c
> > +++ b/drivers/net/ice/base/ice_common.c
> > @@ -672,6 +672,7 @@ enum ice_status ice_init_hw(struct ice_hw *hw)
> >  			  "Failed to get scheduler allocated resources\n");
> >  		goto err_unroll_alloc;
> >  	}
> > +	ice_sched_get_psm_clk_freq(hw);
> >
> >  	/* Initialize port_info struct with scheduler data */
> >  	status = ice_sched_init_port(hw->port_info);
> > diff --git a/drivers/net/ice/base/ice_sched.c
> > b/drivers/net/ice/base/ice_sched.c
> > index 553fc28ff..26c4ba36f 100644
> > --- a/drivers/net/ice/base/ice_sched.c
> > +++ b/drivers/net/ice/base/ice_sched.c
> > @@ -1369,6 +1369,46 @@ enum ice_status
> > ice_sched_query_res_alloc(struct ice_hw *hw)  }
> >
> >  /**
> > + * ice_sched_get_psm_clk_freq - determine the PSM clock frequency
> > + * @hw: pointer to the HW struct
> > + *
> > + * Determine the PSM clock frequency and store in HW struct  */ void
> > +ice_sched_get_psm_clk_freq(struct ice_hw *hw) {
> > +	u32 val, clk_src;
> > +
> > +	val = rd32(hw, GLGEN_CLKSTAT_SRC);
> > +	clk_src = (val & GLGEN_CLKSTAT_SRC_PSM_CLK_SRC_M) >>
> > +		GLGEN_CLKSTAT_SRC_PSM_CLK_SRC_S;
> > +
> > +#define PSM_CLK_SRC_367_MHZ 0x0
> > +#define PSM_CLK_SRC_416_MHZ 0x1
> > +#define PSM_CLK_SRC_446_MHZ 0x2
> > +#define PSM_CLK_SRC_390_MHZ 0x3
> > +
> > +	switch (clk_src) {
> > +	case PSM_CLK_SRC_367_MHZ:
> > +		hw->psm_clk_freq = ICE_PSM_CLK_367MHZ_IN_HZ;
> > +		break;
> > +	case PSM_CLK_SRC_416_MHZ:
> > +		hw->psm_clk_freq = ICE_PSM_CLK_416MHZ_IN_HZ;
> > +		break;
> > +	case PSM_CLK_SRC_446_MHZ:
> > +		hw->psm_clk_freq = ICE_PSM_CLK_446MHZ_IN_HZ;
> > +		break;
> > +	case PSM_CLK_SRC_390_MHZ:
> > +		hw->psm_clk_freq = ICE_PSM_CLK_390MHZ_IN_HZ;
> > +		break;
> > +	default:
> > +		ice_debug(hw, ICE_DBG_SCHED, "PSM clk_src unexpected %u\n",
> > +			  clk_src);
> > +		/* fall back to a safe default */
> > +		hw->psm_clk_freq = ICE_PSM_CLK_446MHZ_IN_HZ;
> > +	}
> > +}
> > +
> > +/**
> >   * ice_sched_find_node_in_subtree - Find node in part of base node
> subtree
> >   * @hw: pointer to the HW struct
> >   * @base: pointer to the base node
> > @@ -2867,7 +2907,7 @@ ice_sched_update_elem(struct ice_hw *hw, struct
> ice_sched_node *node,
> >   */
> >  static enum ice_status
> >  ice_sched_cfg_node_bw_alloc(struct ice_hw *hw, struct ice_sched_node
> *node,
> > -			    enum ice_rl_type rl_type, u8 bw_alloc)
> > +			    enum ice_rl_type rl_type, u16 bw_alloc)
> 
> This fix looks unrelated to the commit message

Yes, will move this fix to patch that cover all the minor fix.
> 
> >  {
> >  	struct ice_aqc_txsched_elem_data buf;
> >  	struct ice_aqc_txsched_elem *data;
> > @@ -3671,11 +3711,12 @@ ice_cfg_agg_bw_alloc(struct ice_port_info *pi,
> > u32 agg_id, u8 ena_tcmap,
> >
> >  /**
> >   * ice_sched_calc_wakeup - calculate RL profile wakeup parameter
> > + * @hw: pointer to the HW struct
> >   * @bw: bandwidth in Kbps
> >   *
> >   * This function calculates the wakeup parameter of RL profile.
> >   */
> > -static u16 ice_sched_calc_wakeup(s32 bw)
> > +static u16 ice_sched_calc_wakeup(struct ice_hw *hw, s32 bw)
> >  {
> >  	s64 bytes_per_sec, wakeup_int, wakeup_a, wakeup_b, wakeup_f;
> >  	s32 wakeup_f_int;
> > @@ -3683,7 +3724,7 @@ static u16 ice_sched_calc_wakeup(s32 bw)
> >
> >  	/* Get the wakeup integer value */
> >  	bytes_per_sec = DIV_64BIT(((s64)bw * 1000), BITS_PER_BYTE);
> > -	wakeup_int = DIV_64BIT(ICE_RL_PROF_FREQUENCY, bytes_per_sec);
> > +	wakeup_int = DIV_64BIT(hw->psm_clk_freq, bytes_per_sec);
> >  	if (wakeup_int > 63) {
> >  		wakeup = (u16)((1 << 15) | wakeup_int);
> >  	} else {
> > @@ -3692,7 +3733,7 @@ static u16 ice_sched_calc_wakeup(s32 bw)
> >  		 */
> >  		wakeup_b = (s64)ICE_RL_PROF_MULTIPLIER * wakeup_int;
> >  		wakeup_a = DIV_64BIT((s64)ICE_RL_PROF_MULTIPLIER *
> > -				     ICE_RL_PROF_FREQUENCY, bytes_per_sec);
> > +				     hw->psm_clk_freq, bytes_per_sec);
> >
> >  		/* Get Fraction value */
> >  		wakeup_f = wakeup_a - wakeup_b;
> > @@ -3712,13 +3753,15 @@ static u16 ice_sched_calc_wakeup(s32 bw)
> >
> >  /**
> >   * ice_sched_bw_to_rl_profile - convert BW to profile parameters
> > + * @hw: pointer to the HW struct
> >   * @bw: bandwidth in Kbps
> >   * @profile: profile parameters to return
> >   *
> >   * This function converts the BW to profile structure format.
> >   */
> >  static enum ice_status
> > -ice_sched_bw_to_rl_profile(u32 bw, struct ice_aqc_rl_profile_elem
> > *profile)
> > +ice_sched_bw_to_rl_profile(struct ice_hw *hw, u32 bw,
> > +			   struct ice_aqc_rl_profile_elem *profile)
> >  {
> >  	enum ice_status status = ICE_ERR_PARAM;
> >  	s64 bytes_per_sec, ts_rate, mv_tmp;
> > @@ -3738,7 +3781,7 @@ ice_sched_bw_to_rl_profile(u32 bw, struct
> ice_aqc_rl_profile_elem *profile)
> >  	for (i = 0; i < 64; i++) {
> >  		u64 pow_result = BIT_ULL(i);
> >
> > -		ts_rate = DIV_64BIT((s64)ICE_RL_PROF_FREQUENCY,
> > +		ts_rate = DIV_64BIT((s64)hw->psm_clk_freq,
> >  				    pow_result * ICE_RL_PROF_TS_MULTIPLIER);
> >  		if (ts_rate <= 0)
> >  			continue;
> > @@ -3762,7 +3805,7 @@ ice_sched_bw_to_rl_profile(u32 bw, struct
> ice_aqc_rl_profile_elem *profile)
> >  	if (found) {
> >  		u16 wm;
> >
> > -		wm = ice_sched_calc_wakeup(bw);
> > +		wm = ice_sched_calc_wakeup(hw, bw);
> >  		profile->rl_multiply = CPU_TO_LE16(mv);
> >  		profile->wake_up_calc = CPU_TO_LE16(wm);
> >  		profile->rl_encode = CPU_TO_LE16(encode); @@ -3831,7 +3874,7
> @@
> > ice_sched_add_rl_profile(struct ice_port_info *pi,
> >  	if (!rl_prof_elem)
> >  		return NULL;
> >
> > -	status = ice_sched_bw_to_rl_profile(bw, &rl_prof_elem->profile);
> > +	status = ice_sched_bw_to_rl_profile(hw, bw, &rl_prof_elem->profile);
> >  	if (status != ICE_SUCCESS)
> >  		goto exit_add_rl_prof;
> >
> > diff --git a/drivers/net/ice/base/ice_sched.h
> > b/drivers/net/ice/base/ice_sched.h
> > index d6b467477..1a8549931 100644
> > --- a/drivers/net/ice/base/ice_sched.h
> > +++ b/drivers/net/ice/base/ice_sched.h
> > @@ -25,12 +25,16 @@
> >  	((BIT(11) - 1) * 64) /* In Bytes */
> >  #define ICE_MAX_BURST_SIZE_KBYTE_GRANULARITY
> 	ICE_MAX_BURST_SIZE_ALLOWED
> >
> > -#define ICE_RL_PROF_FREQUENCY 446000000  #define
> > ICE_RL_PROF_ACCURACY_BYTES 128  #define ICE_RL_PROF_MULTIPLIER
> 10000
> > #define ICE_RL_PROF_TS_MULTIPLIER 32  #define ICE_RL_PROF_FRACTION
> 512
> >
> > +#define ICE_PSM_CLK_367MHZ_IN_HZ 367647059 #define
> > +ICE_PSM_CLK_416MHZ_IN_HZ 416666667 #define
> ICE_PSM_CLK_446MHZ_IN_HZ
> > +446428571 #define ICE_PSM_CLK_390MHZ_IN_HZ 390625000
> > +
> >  struct rl_profile_params {
> >  	u32 bw;			/* in Kbps */
> >  	u16 rl_multiplier;
> > @@ -83,6 +87,7 @@ ice_aq_query_sched_elems(struct ice_hw *hw, u16
> elems_req,
> >  			 u16 *elems_ret, struct ice_sq_cd *cd);  enum ice_status
> > ice_sched_init_port(struct ice_port_info *pi);  enum ice_status
> > ice_sched_query_res_alloc(struct ice_hw *hw);
> > +void ice_sched_get_psm_clk_freq(struct ice_hw *hw);
> >
> >  /* Functions to cleanup scheduler SW DB */  void
> > ice_sched_clear_port(struct ice_port_info *pi); diff --git
> > a/drivers/net/ice/base/ice_type.h b/drivers/net/ice/base/ice_type.h
> > index 9773a549f..237220ee8 100644
> > --- a/drivers/net/ice/base/ice_type.h
> > +++ b/drivers/net/ice/base/ice_type.h
> > @@ -524,7 +524,7 @@ struct ice_sched_node {
> >  #define ICE_TXSCHED_GET_EIR_BWALLOC(x)	\
> >  	LE16_TO_CPU((x)->info.eir_bw.bw_alloc)
> >
> > -struct ice_sched_rl_profle {
> > +struct ice_sched_rl_profile {
> 
> Is this used somewhere?

The base code is shared by linux/dpdk/windows, the consideration here is
1) we try to make code identical, otherwise a lot of compile option is involved to strip out code for different usage.
2) some code may not be used on specific platform currently, but might be active in future.

Thanks
Qi

> 
> >  	u32 rate; /* In Kbps */
> >  	struct ice_aqc_rl_profile_elem info;  }; @@ -741,6 +741,8 @@ struct
> > ice_hw {
> >  	struct ice_sched_rl_profile **cir_profiles;
> >  	struct ice_sched_rl_profile **eir_profiles;
> >  	struct ice_sched_rl_profile **srl_profiles;
> > +	/* PSM clock frequency for calculating RL profile params */
> > +	u32 psm_clk_freq;
> >  	u64 debug_mask;		/* BITMAP for debug mask */
> >  	enum ice_mac_type mac_type;
> >
> >



More information about the dev mailing list