[next,S70,04/12] i40e: factor out queue control from i40e_vsi_control_(tx|rx)

Message ID 20170413084555.6962-4-alice.michael@intel.com
State Accepted
Delegated to: Jeff Kirsher
Headers show

Commit Message

Alice Michael April 13, 2017, 8:45 a.m.
From: Jacob Keller <jacob.e.keller@intel.com>

A future patch will need to be able to handle controlling queues without
waiting until all VSIs are handled. Factor out the direct queue
modification so that we can easily re-use this code. The result is also
a bit easier to read since we don't embed multiple single-letter loop
counters.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Change-ID: Id923cbfa43127b1c24d8ed4f809b1012c736d9ac
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 139 ++++++++++++++++++----------
 1 file changed, 89 insertions(+), 50 deletions(-)

Comments

Keller, Jacob E April 13, 2017, 5:22 p.m. | #1
ACK

Thanks,
Jake

> -----Original Message-----
> From: Michael, Alice
> Sent: Thursday, April 13, 2017 1:46 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-lan@lists.osuosl.org
> Cc: Keller, Jacob E <jacob.e.keller@intel.com>
> Subject: [next PATCH S70 04/12] i40e: factor out queue control from
> i40e_vsi_control_(tx|rx)
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> A future patch will need to be able to handle controlling queues without
> waiting until all VSIs are handled. Factor out the direct queue
> modification so that we can easily re-use this code. The result is also
> a bit easier to read since we don't embed multiple single-letter loop
> counters.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-ID: Id923cbfa43127b1c24d8ed4f809b1012c736d9ac
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 139 ++++++++++++++++++-------
> ---
>  1 file changed, 89 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
> b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index 1c06693..ebf2ad4 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -3919,6 +3919,8 @@ static void i40e_netpoll(struct net_device *netdev)
>  }
>  #endif
> 
> +#define I40E_QTX_ENA_WAIT_COUNT 50
> +
>  /**
>   * i40e_pf_txq_wait - Wait for a PF's Tx queue to be enabled or disabled
>   * @pf: the PF being configured
> @@ -3949,6 +3951,50 @@ static int i40e_pf_txq_wait(struct i40e_pf *pf, int pf_q,
> bool enable)
>  }
> 
>  /**
> + * i40e_control_tx_q - Start or stop a particular Tx queue
> + * @pf: the PF structure
> + * @pf_q: the PF queue to configure
> + * @enable: start or stop the queue
> + *
> + * This function enables or disables a single queue. Note that any delay
> + * required after the operation is expected to be handled by the caller of
> + * this function.
> + **/
> +static void i40e_control_tx_q(struct i40e_pf *pf, int pf_q, bool enable)
> +{
> +	struct i40e_hw *hw = &pf->hw;
> +	u32 tx_reg;
> +	int i;
> +
> +	/* warn the TX unit of coming changes */
> +	i40e_pre_tx_queue_cfg(&pf->hw, pf_q, enable);
> +	if (!enable)
> +		usleep_range(10, 20);
> +
> +	for (i = 0; i < I40E_QTX_ENA_WAIT_COUNT; i++) {
> +		tx_reg = rd32(hw, I40E_QTX_ENA(pf_q));
> +		if (((tx_reg >> I40E_QTX_ENA_QENA_REQ_SHIFT) & 1) ==
> +		    ((tx_reg >> I40E_QTX_ENA_QENA_STAT_SHIFT) & 1))
> +			break;
> +		usleep_range(1000, 2000);
> +	}
> +
> +	/* Skip if the queue is already in the requested state */
> +	if (enable == !!(tx_reg & I40E_QTX_ENA_QENA_STAT_MASK))
> +		return;
> +
> +	/* turn on/off the queue */
> +	if (enable) {
> +		wr32(hw, I40E_QTX_HEAD(pf_q), 0);
> +		tx_reg |= I40E_QTX_ENA_QENA_REQ_MASK;
> +	} else {
> +		tx_reg &= ~I40E_QTX_ENA_QENA_REQ_MASK;
> +	}
> +
> +	wr32(hw, I40E_QTX_ENA(pf_q), tx_reg);
> +}
> +
> +/**
>   * i40e_vsi_control_tx - Start or stop a VSI's rings
>   * @vsi: the VSI being configured
>   * @enable: start or stop the rings
> @@ -3956,39 +4002,13 @@ static int i40e_pf_txq_wait(struct i40e_pf *pf, int
> pf_q, bool enable)
>  static int i40e_vsi_control_tx(struct i40e_vsi *vsi, bool enable)
>  {
>  	struct i40e_pf *pf = vsi->back;
> -	struct i40e_hw *hw = &pf->hw;
> -	int i, j, pf_q, ret = 0;
> -	u32 tx_reg;
> +	int i, pf_q, ret = 0;
> 
>  	pf_q = vsi->base_queue;
>  	for (i = 0; i < vsi->num_queue_pairs; i++, pf_q++) {
> +		i40e_control_tx_q(pf, pf_q, enable);
> 
> -		/* warn the TX unit of coming changes */
> -		i40e_pre_tx_queue_cfg(&pf->hw, pf_q, enable);
> -		if (!enable)
> -			usleep_range(10, 20);
> -
> -		for (j = 0; j < 50; j++) {
> -			tx_reg = rd32(hw, I40E_QTX_ENA(pf_q));
> -			if (((tx_reg >> I40E_QTX_ENA_QENA_REQ_SHIFT) & 1) ==
> -			    ((tx_reg >> I40E_QTX_ENA_QENA_STAT_SHIFT) & 1))
> -				break;
> -			usleep_range(1000, 2000);
> -		}
> -		/* Skip if the queue is already in the requested state */
> -		if (enable == !!(tx_reg & I40E_QTX_ENA_QENA_STAT_MASK))
> -			continue;
> -
> -		/* turn on/off the queue */
> -		if (enable) {
> -			wr32(hw, I40E_QTX_HEAD(pf_q), 0);
> -			tx_reg |= I40E_QTX_ENA_QENA_REQ_MASK;
> -		} else {
> -			tx_reg &= ~I40E_QTX_ENA_QENA_REQ_MASK;
> -		}
> -
> -		wr32(hw, I40E_QTX_ENA(pf_q), tx_reg);
> -		/* No waiting for the Tx queue to disable */
> +		/* Don't wait to disable when port Tx is suspended */
>  		if (!enable && test_bit(__I40E_PORT_TX_SUSPENDED, &pf-
> >state))
>  			continue;
> 
> @@ -4035,6 +4055,43 @@ static int i40e_pf_rxq_wait(struct i40e_pf *pf, int pf_q,
> bool enable)
>  }
> 
>  /**
> + * i40e_control_rx_q - Start or stop a particular Rx queue
> + * @pf: the PF structure
> + * @pf_q: the PF queue to configure
> + * @enable: start or stop the queue
> + *
> + * This function enables or disables a single queue. Note that any delay
> + * required after the operation is expected to be handled by the caller of
> + * this function.
> + **/
> +static void i40e_control_rx_q(struct i40e_pf *pf, int pf_q, bool enable)
> +{
> +	struct i40e_hw *hw = &pf->hw;
> +	u32 rx_reg;
> +	int i;
> +
> +	for (i = 0; i < I40E_QTX_ENA_WAIT_COUNT; i++) {
> +		rx_reg = rd32(hw, I40E_QRX_ENA(pf_q));
> +		if (((rx_reg >> I40E_QRX_ENA_QENA_REQ_SHIFT) & 1) ==
> +		    ((rx_reg >> I40E_QRX_ENA_QENA_STAT_SHIFT) & 1))
> +			break;
> +		usleep_range(1000, 2000);
> +	}
> +
> +	/* Skip if the queue is already in the requested state */
> +	if (enable == !!(rx_reg & I40E_QRX_ENA_QENA_STAT_MASK))
> +		return;
> +
> +	/* turn on/off the queue */
> +	if (enable)
> +		rx_reg |= I40E_QRX_ENA_QENA_REQ_MASK;
> +	else
> +		rx_reg &= ~I40E_QRX_ENA_QENA_REQ_MASK;
> +
> +	wr32(hw, I40E_QRX_ENA(pf_q), rx_reg);
> +}
> +
> +/**
>   * i40e_vsi_control_rx - Start or stop a VSI's rings
>   * @vsi: the VSI being configured
>   * @enable: start or stop the rings
> @@ -4042,31 +4099,13 @@ static int i40e_pf_rxq_wait(struct i40e_pf *pf, int
> pf_q, bool enable)
>  static int i40e_vsi_control_rx(struct i40e_vsi *vsi, bool enable)
>  {
>  	struct i40e_pf *pf = vsi->back;
> -	struct i40e_hw *hw = &pf->hw;
> -	int i, j, pf_q, ret = 0;
> -	u32 rx_reg;
> +	int i, pf_q, ret = 0;
> 
>  	pf_q = vsi->base_queue;
>  	for (i = 0; i < vsi->num_queue_pairs; i++, pf_q++) {
> -		for (j = 0; j < 50; j++) {
> -			rx_reg = rd32(hw, I40E_QRX_ENA(pf_q));
> -			if (((rx_reg >> I40E_QRX_ENA_QENA_REQ_SHIFT) & 1) ==
> -			    ((rx_reg >> I40E_QRX_ENA_QENA_STAT_SHIFT) & 1))
> -				break;
> -			usleep_range(1000, 2000);
> -		}
> +		i40e_control_rx_q(pf, pf_q, enable);
> 
> -		/* Skip if the queue is already in the requested state */
> -		if (enable == !!(rx_reg & I40E_QRX_ENA_QENA_STAT_MASK))
> -			continue;
> -
> -		/* turn on/off the queue */
> -		if (enable)
> -			rx_reg |= I40E_QRX_ENA_QENA_REQ_MASK;
> -		else
> -			rx_reg &= ~I40E_QRX_ENA_QENA_REQ_MASK;
> -		wr32(hw, I40E_QRX_ENA(pf_q), rx_reg);
> -		/* No waiting for the Tx queue to disable */
> +		/* Don't wait to disable when port Tx is suspended */
>  		if (!enable && test_bit(__I40E_PORT_TX_SUSPENDED, &pf-
> >state))
>  			continue;
> 
> --
> 2.9.3
Bowers, AndrewX April 14, 2017, 9:47 p.m. | #2
> -----Original Message-----
> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On
> Behalf Of Alice Michael
> Sent: Thursday, April 13, 2017 1:46 AM
> To: Michael, Alice <alice.michael@intel.com>; intel-wired-
> lan@lists.osuosl.org
> Subject: [Intel-wired-lan] [next PATCH S70 04/12] i40e: factor out queue
> control from i40e_vsi_control_(tx|rx)
> 
> From: Jacob Keller <jacob.e.keller@intel.com>
> 
> A future patch will need to be able to handle controlling queues without
> waiting until all VSIs are handled. Factor out the direct queue modification so
> that we can easily re-use this code. The result is also a bit easier to read since
> we don't embed multiple single-letter loop counters.
> 
> Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
> Change-ID: Id923cbfa43127b1c24d8ed4f809b1012c736d9ac
> ---
>  drivers/net/ethernet/intel/i40e/i40e_main.c | 139 ++++++++++++++++++--
> --------
>  1 file changed, 89 insertions(+), 50 deletions(-)

Tested-by: Andrew Bowers <andrewx.bowers@intel.com>

Patch

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 1c06693..ebf2ad4 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -3919,6 +3919,8 @@  static void i40e_netpoll(struct net_device *netdev)
 }
 #endif
 
+#define I40E_QTX_ENA_WAIT_COUNT 50
+
 /**
  * i40e_pf_txq_wait - Wait for a PF's Tx queue to be enabled or disabled
  * @pf: the PF being configured
@@ -3949,6 +3951,50 @@  static int i40e_pf_txq_wait(struct i40e_pf *pf, int pf_q, bool enable)
 }
 
 /**
+ * i40e_control_tx_q - Start or stop a particular Tx queue
+ * @pf: the PF structure
+ * @pf_q: the PF queue to configure
+ * @enable: start or stop the queue
+ *
+ * This function enables or disables a single queue. Note that any delay
+ * required after the operation is expected to be handled by the caller of
+ * this function.
+ **/
+static void i40e_control_tx_q(struct i40e_pf *pf, int pf_q, bool enable)
+{
+	struct i40e_hw *hw = &pf->hw;
+	u32 tx_reg;
+	int i;
+
+	/* warn the TX unit of coming changes */
+	i40e_pre_tx_queue_cfg(&pf->hw, pf_q, enable);
+	if (!enable)
+		usleep_range(10, 20);
+
+	for (i = 0; i < I40E_QTX_ENA_WAIT_COUNT; i++) {
+		tx_reg = rd32(hw, I40E_QTX_ENA(pf_q));
+		if (((tx_reg >> I40E_QTX_ENA_QENA_REQ_SHIFT) & 1) ==
+		    ((tx_reg >> I40E_QTX_ENA_QENA_STAT_SHIFT) & 1))
+			break;
+		usleep_range(1000, 2000);
+	}
+
+	/* Skip if the queue is already in the requested state */
+	if (enable == !!(tx_reg & I40E_QTX_ENA_QENA_STAT_MASK))
+		return;
+
+	/* turn on/off the queue */
+	if (enable) {
+		wr32(hw, I40E_QTX_HEAD(pf_q), 0);
+		tx_reg |= I40E_QTX_ENA_QENA_REQ_MASK;
+	} else {
+		tx_reg &= ~I40E_QTX_ENA_QENA_REQ_MASK;
+	}
+
+	wr32(hw, I40E_QTX_ENA(pf_q), tx_reg);
+}
+
+/**
  * i40e_vsi_control_tx - Start or stop a VSI's rings
  * @vsi: the VSI being configured
  * @enable: start or stop the rings
@@ -3956,39 +4002,13 @@  static int i40e_pf_txq_wait(struct i40e_pf *pf, int pf_q, bool enable)
 static int i40e_vsi_control_tx(struct i40e_vsi *vsi, bool enable)
 {
 	struct i40e_pf *pf = vsi->back;
-	struct i40e_hw *hw = &pf->hw;
-	int i, j, pf_q, ret = 0;
-	u32 tx_reg;
+	int i, pf_q, ret = 0;
 
 	pf_q = vsi->base_queue;
 	for (i = 0; i < vsi->num_queue_pairs; i++, pf_q++) {
+		i40e_control_tx_q(pf, pf_q, enable);
 
-		/* warn the TX unit of coming changes */
-		i40e_pre_tx_queue_cfg(&pf->hw, pf_q, enable);
-		if (!enable)
-			usleep_range(10, 20);
-
-		for (j = 0; j < 50; j++) {
-			tx_reg = rd32(hw, I40E_QTX_ENA(pf_q));
-			if (((tx_reg >> I40E_QTX_ENA_QENA_REQ_SHIFT) & 1) ==
-			    ((tx_reg >> I40E_QTX_ENA_QENA_STAT_SHIFT) & 1))
-				break;
-			usleep_range(1000, 2000);
-		}
-		/* Skip if the queue is already in the requested state */
-		if (enable == !!(tx_reg & I40E_QTX_ENA_QENA_STAT_MASK))
-			continue;
-
-		/* turn on/off the queue */
-		if (enable) {
-			wr32(hw, I40E_QTX_HEAD(pf_q), 0);
-			tx_reg |= I40E_QTX_ENA_QENA_REQ_MASK;
-		} else {
-			tx_reg &= ~I40E_QTX_ENA_QENA_REQ_MASK;
-		}
-
-		wr32(hw, I40E_QTX_ENA(pf_q), tx_reg);
-		/* No waiting for the Tx queue to disable */
+		/* Don't wait to disable when port Tx is suspended */
 		if (!enable && test_bit(__I40E_PORT_TX_SUSPENDED, &pf->state))
 			continue;
 
@@ -4035,6 +4055,43 @@  static int i40e_pf_rxq_wait(struct i40e_pf *pf, int pf_q, bool enable)
 }
 
 /**
+ * i40e_control_rx_q - Start or stop a particular Rx queue
+ * @pf: the PF structure
+ * @pf_q: the PF queue to configure
+ * @enable: start or stop the queue
+ *
+ * This function enables or disables a single queue. Note that any delay
+ * required after the operation is expected to be handled by the caller of
+ * this function.
+ **/
+static void i40e_control_rx_q(struct i40e_pf *pf, int pf_q, bool enable)
+{
+	struct i40e_hw *hw = &pf->hw;
+	u32 rx_reg;
+	int i;
+
+	for (i = 0; i < I40E_QTX_ENA_WAIT_COUNT; i++) {
+		rx_reg = rd32(hw, I40E_QRX_ENA(pf_q));
+		if (((rx_reg >> I40E_QRX_ENA_QENA_REQ_SHIFT) & 1) ==
+		    ((rx_reg >> I40E_QRX_ENA_QENA_STAT_SHIFT) & 1))
+			break;
+		usleep_range(1000, 2000);
+	}
+
+	/* Skip if the queue is already in the requested state */
+	if (enable == !!(rx_reg & I40E_QRX_ENA_QENA_STAT_MASK))
+		return;
+
+	/* turn on/off the queue */
+	if (enable)
+		rx_reg |= I40E_QRX_ENA_QENA_REQ_MASK;
+	else
+		rx_reg &= ~I40E_QRX_ENA_QENA_REQ_MASK;
+
+	wr32(hw, I40E_QRX_ENA(pf_q), rx_reg);
+}
+
+/**
  * i40e_vsi_control_rx - Start or stop a VSI's rings
  * @vsi: the VSI being configured
  * @enable: start or stop the rings
@@ -4042,31 +4099,13 @@  static int i40e_pf_rxq_wait(struct i40e_pf *pf, int pf_q, bool enable)
 static int i40e_vsi_control_rx(struct i40e_vsi *vsi, bool enable)
 {
 	struct i40e_pf *pf = vsi->back;
-	struct i40e_hw *hw = &pf->hw;
-	int i, j, pf_q, ret = 0;
-	u32 rx_reg;
+	int i, pf_q, ret = 0;
 
 	pf_q = vsi->base_queue;
 	for (i = 0; i < vsi->num_queue_pairs; i++, pf_q++) {
-		for (j = 0; j < 50; j++) {
-			rx_reg = rd32(hw, I40E_QRX_ENA(pf_q));
-			if (((rx_reg >> I40E_QRX_ENA_QENA_REQ_SHIFT) & 1) ==
-			    ((rx_reg >> I40E_QRX_ENA_QENA_STAT_SHIFT) & 1))
-				break;
-			usleep_range(1000, 2000);
-		}
+		i40e_control_rx_q(pf, pf_q, enable);
 
-		/* Skip if the queue is already in the requested state */
-		if (enable == !!(rx_reg & I40E_QRX_ENA_QENA_STAT_MASK))
-			continue;
-
-		/* turn on/off the queue */
-		if (enable)
-			rx_reg |= I40E_QRX_ENA_QENA_REQ_MASK;
-		else
-			rx_reg &= ~I40E_QRX_ENA_QENA_REQ_MASK;
-		wr32(hw, I40E_QRX_ENA(pf_q), rx_reg);
-		/* No waiting for the Tx queue to disable */
+		/* Don't wait to disable when port Tx is suspended */
 		if (!enable && test_bit(__I40E_PORT_TX_SUSPENDED, &pf->state))
 			continue;