Inside NIC RX interrupt is needed for single RX descriptor write back. The fix is to correct the wrong configuration of register 'I40E_QINT_RQCTL'. In addition, several code style fixes are added. Note that interrupt will be inside NIC only, that means it will never be reported outside NIC hardware. Helin Zhang (2): i40e: code style fix i40e: fix of configuring inside NIC RX interrupt lib/librte_pmd_i40e/i40e_ethdev.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) -- 1.8.1.4
Add several code style fixes. Signed-off-by: Helin Zhang <helin.zhang@intel.com> --- lib/librte_pmd_i40e/i40e_ethdev.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c index e1f581a..20c99a4 100644 --- a/lib/librte_pmd_i40e/i40e_ethdev.c +++ b/lib/librte_pmd_i40e/i40e_ethdev.c @@ -580,7 +580,8 @@ i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi) uint32_t val; struct i40e_hw *hw = I40E_VSI_TO_HW(vsi); uint16_t msix_vect = vsi->msix_intr; - uint16_t interval = i40e_calc_itr_interval(RTE_LIBRTE_I40E_ITR_INTERVAL); + uint16_t interval = + i40e_calc_itr_interval(RTE_LIBRTE_I40E_ITR_INTERVAL); int i; for (i = 0; i < vsi->nb_qps; i++) @@ -603,25 +604,26 @@ i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi) /* Write first RX queue to Link list register as the head element */ if (vsi->type != I40E_VSI_SRIOV) { I40E_WRITE_REG(hw, I40E_PFINT_LNKLSTN(msix_vect - 1), - (vsi->base_queue << I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | + (vsi->base_queue << + I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | (0x0 << I40E_PFINT_LNKLSTN_FIRSTQ_TYPE_SHIFT)); I40E_WRITE_REG(hw, I40E_PFINT_ITRN(I40E_ITR_INDEX_DEFAULT, - msix_vect - 1), interval); + msix_vect - 1), interval); /* Disable auto-mask on enabling of all none-zero interrupt */ I40E_WRITE_REG(hw, I40E_GLINT_CTL, - I40E_GLINT_CTL_DIS_AUTOMASK_N_MASK); - } - else { + I40E_GLINT_CTL_DIS_AUTOMASK_N_MASK); + } else { uint32_t reg; + /* num_msix_vectors_vf needs to minus irq0 */ reg = (hw->func_caps.num_msix_vectors_vf - 1) * vsi->user_param + (msix_vect - 1); - I40E_WRITE_REG(hw, I40E_VPINT_LNKLSTN(reg), - (vsi->base_queue << I40E_VPINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | - (0x0 << I40E_VPINT_LNKLSTN_FIRSTQ_TYPE_SHIFT)); + I40E_WRITE_REG(hw, I40E_VPINT_LNKLSTN(reg), (vsi->base_queue << + I40E_VPINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | + (0x0 << I40E_VPINT_LNKLSTN_FIRSTQ_TYPE_SHIFT)); } I40E_WRITE_FLUSH(hw); -- 1.8.1.4
Inside NIC RX interrupt is needed for single RX descriptor write back. The fix is to correct the wrong configuration of register 'I40E_QINT_RQCTL'. Note that interrupt will be inside NIC only, that means it will never be reported outside NIC hardware. Signed-off-by: Helin Zhang <helin.zhang@intel.com> --- lib/librte_pmd_i40e/i40e_ethdev.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c index 20c99a4..4614c3d 100644 --- a/lib/librte_pmd_i40e/i40e_ethdev.c +++ b/lib/librte_pmd_i40e/i40e_ethdev.c @@ -580,8 +580,6 @@ i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi) uint32_t val; struct i40e_hw *hw = I40E_VSI_TO_HW(vsi); uint16_t msix_vect = vsi->msix_intr; - uint16_t interval = - i40e_calc_itr_interval(RTE_LIBRTE_I40E_ITR_INTERVAL); int i; for (i = 0; i < vsi->nb_qps; i++) @@ -590,7 +588,7 @@ i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi) /* Bind all RX queues to allocated MSIX interrupt */ for (i = 0; i < vsi->nb_qps; i++) { val = (msix_vect << I40E_QINT_RQCTL_MSIX_INDX_SHIFT) | - (interval << I40E_QINT_RQCTL_ITR_INDX_SHIFT) | + I40E_QINT_RQCTL_ITR_INDX_MASK | ((vsi->base_queue + i + 1) << I40E_QINT_RQCTL_NEXTQ_INDX_SHIFT) | (0 << I40E_QINT_RQCTL_NEXTQ_TYPE_SHIFT) | @@ -603,6 +601,9 @@ i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi) /* Write first RX queue to Link list register as the head element */ if (vsi->type != I40E_VSI_SRIOV) { + uint16_t interval = + i40e_calc_itr_interval(RTE_LIBRTE_I40E_ITR_INTERVAL); + I40E_WRITE_REG(hw, I40E_PFINT_LNKLSTN(msix_vect - 1), (vsi->base_queue << I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | -- 1.8.1.4
> -----Original Message-----
> From: Zhang, Helin
> Sent: Wednesday, October 29, 2014 11:43 AM
> To: dev@dpdk.org
> Cc: Cao, Waterman; Chen, Jing D; Zhang, Helin
> Subject: [PATCH 0/2] fix of configuring inside NIC RX interrupt
>
> Inside NIC RX interrupt is needed for single RX descriptor
> write back. The fix is to correct the wrong configuration
> of register 'I40E_QINT_RQCTL'. In addition, several code
> style fixes are added.
> Note that interrupt will be inside NIC only, that means it
> will never be reported outside NIC hardware.
>
> Helin Zhang (2):
> i40e: code style fix
> i40e: fix of configuring inside NIC RX interrupt
>
> lib/librte_pmd_i40e/i40e_ethdev.c | 23 +++++++++++++----------
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> --
> 1.8.1.4
Acked-by: Jing Chen <jing.d.chen@intel.com>
10/29/2014 11:44 AM, Helin Zhang : > Add several code style fixes. > > Signed-off-by: Helin Zhang <helin.zhang@intel.com> > --- > lib/librte_pmd_i40e/i40e_ethdev.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c > index e1f581a..20c99a4 100644 > --- a/lib/librte_pmd_i40e/i40e_ethdev.c > +++ b/lib/librte_pmd_i40e/i40e_ethdev.c > @@ -580,7 +580,8 @@ i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi) > uint32_t val; > struct i40e_hw *hw = I40E_VSI_TO_HW(vsi); > uint16_t msix_vect = vsi->msix_intr; > - uint16_t interval = i40e_calc_itr_interval(RTE_LIBRTE_I40E_ITR_INTERVAL); > + uint16_t interval = > + i40e_calc_itr_interval(RTE_LIBRTE_I40E_ITR_INTERVAL); > int i; > > for (i = 0; i < vsi->nb_qps; i++) > @@ -603,25 +604,26 @@ i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi) > /* Write first RX queue to Link list register as the head element */ > if (vsi->type != I40E_VSI_SRIOV) { > I40E_WRITE_REG(hw, I40E_PFINT_LNKLSTN(msix_vect - 1), > - (vsi->base_queue << I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | > + (vsi->base_queue << ^^^^^^^^^^^^^^^^^^ > + I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | Here why so many indentations? or just my mail agent's display issue, if it does pls ignor. anyway, it is not a beautiful style in my mind :) I40E_WRITE_REG(hw, I40E_PFINT_LNKLSTN(msix_vect - 1), - (vsi->base_queue << I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | + (vsi->base_queue << + I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | BTW, for coding style, I think we can follow the linux kernel, for kernel indentations, it always use the format like: n"Tab"+m"space", 0 <= m <=7. Another example is qemu, it is all "space" without "tab". > (0x0 << I40E_PFINT_LNKLSTN_FIRSTQ_TYPE_SHIFT)); > > I40E_WRITE_REG(hw, I40E_PFINT_ITRN(I40E_ITR_INDEX_DEFAULT, > - msix_vect - 1), interval); > + msix_vect - 1), interval); > > /* Disable auto-mask on enabling of all none-zero interrupt */ > I40E_WRITE_REG(hw, I40E_GLINT_CTL, > - I40E_GLINT_CTL_DIS_AUTOMASK_N_MASK); > - } > - else { > + I40E_GLINT_CTL_DIS_AUTOMASK_N_MASK); > + } else { > uint32_t reg; > + > /* num_msix_vectors_vf needs to minus irq0 */ > reg = (hw->func_caps.num_msix_vectors_vf - 1) * > vsi->user_param + (msix_vect - 1); > > - I40E_WRITE_REG(hw, I40E_VPINT_LNKLSTN(reg), > - (vsi->base_queue << I40E_VPINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | > - (0x0 << I40E_VPINT_LNKLSTN_FIRSTQ_TYPE_SHIFT)); > + I40E_WRITE_REG(hw, I40E_VPINT_LNKLSTN(reg), (vsi->base_queue << > + I40E_VPINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | > + (0x0 << I40E_VPINT_LNKLSTN_FIRSTQ_TYPE_SHIFT)); Here the same I think. Thanks, Michael > } > > I40E_WRITE_FLUSH(hw);
Hi Michael > -----Original Message----- > From: Qiu, Michael > Sent: Wednesday, October 29, 2014 2:11 PM > To: Zhang, Helin; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 1/2] i40e: code style fix > > 10/29/2014 11:44 AM, Helin Zhang : > > Add several code style fixes. > > > > Signed-off-by: Helin Zhang <helin.zhang@intel.com> > > --- > > lib/librte_pmd_i40e/i40e_ethdev.c | 20 +++++++++++--------- > > 1 file changed, 11 insertions(+), 9 deletions(-) > > > > diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c > > b/lib/librte_pmd_i40e/i40e_ethdev.c > > index e1f581a..20c99a4 100644 > > --- a/lib/librte_pmd_i40e/i40e_ethdev.c > > +++ b/lib/librte_pmd_i40e/i40e_ethdev.c > > @@ -580,7 +580,8 @@ i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi) > > uint32_t val; > > struct i40e_hw *hw = I40E_VSI_TO_HW(vsi); > > uint16_t msix_vect = vsi->msix_intr; > > - uint16_t interval = i40e_calc_itr_interval(RTE_LIBRTE_I40E_ITR_INTERVAL); > > + uint16_t interval = > > + i40e_calc_itr_interval(RTE_LIBRTE_I40E_ITR_INTERVAL); > > int i; > > > > for (i = 0; i < vsi->nb_qps; i++) > > @@ -603,25 +604,26 @@ i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi) > > /* Write first RX queue to Link list register as the head element */ > > if (vsi->type != I40E_VSI_SRIOV) { > > I40E_WRITE_REG(hw, I40E_PFINT_LNKLSTN(msix_vect - 1), > > - (vsi->base_queue << I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) > | > > + (vsi->base_queue << > > ^^^^^^^^^^^^^^^^^^ > > + I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | > > Here why so many indentations? or just my mail agent's display issue, if it does > pls ignor. anyway, it is not a beautiful style in my mind :) I do not know why. The right patch can be downloaded directly from dpdk.org. The email editor may not show the correct format. > > I40E_WRITE_REG(hw, I40E_PFINT_LNKLSTN(msix_vect - 1), > - (vsi->base_queue << I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) > | > + (vsi->base_queue << > + I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | > > BTW, for coding style, I think we can follow the linux kernel, for kernel > indentations, it always use the format like: n"Tab"+m"space", 0 <= m <=7. > Another example is qemu, it is all "space" without "tab". Yes, we have DPDK style, tab or tab + space. Space only is not allowed. Checkpatch.pl can check it for us. > > > (0x0 << I40E_PFINT_LNKLSTN_FIRSTQ_TYPE_SHIFT)); > > > > I40E_WRITE_REG(hw, I40E_PFINT_ITRN(I40E_ITR_INDEX_DEFAULT, > > - msix_vect - 1), interval); > > + msix_vect - 1), interval); > > > > /* Disable auto-mask on enabling of all none-zero interrupt */ > > I40E_WRITE_REG(hw, I40E_GLINT_CTL, > > - I40E_GLINT_CTL_DIS_AUTOMASK_N_MASK); > > - } > > - else { > > + I40E_GLINT_CTL_DIS_AUTOMASK_N_MASK); > > + } else { > > uint32_t reg; > > + > > /* num_msix_vectors_vf needs to minus irq0 */ > > reg = (hw->func_caps.num_msix_vectors_vf - 1) * > > vsi->user_param + (msix_vect - 1); > > > > - I40E_WRITE_REG(hw, I40E_VPINT_LNKLSTN(reg), > > - (vsi->base_queue << I40E_VPINT_LNKLSTN_FIRSTQ_INDX_SHIFT) > | > > - (0x0 << I40E_VPINT_LNKLSTN_FIRSTQ_TYPE_SHIFT)); > > + I40E_WRITE_REG(hw, I40E_VPINT_LNKLSTN(reg), (vsi->base_queue > << > > + I40E_VPINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | > > + (0x0 << I40E_VPINT_LNKLSTN_FIRSTQ_TYPE_SHIFT)); > > Here the same I think. > > Thanks, > Michael > > } > > > > I40E_WRITE_FLUSH(hw); Regards, Helin
10/29/2014 2:26 PM, Zhang, Helin: > Hi Michael > >> -----Original Message----- >> From: Qiu, Michael >> Sent: Wednesday, October 29, 2014 2:11 PM >> To: Zhang, Helin; dev@dpdk.org >> Subject: Re: [dpdk-dev] [PATCH 1/2] i40e: code style fix >> >> 10/29/2014 11:44 AM, Helin Zhang : >>> Add several code style fixes. >>> >>> Signed-off-by: Helin Zhang <helin.zhang@intel.com> >>> --- >>> lib/librte_pmd_i40e/i40e_ethdev.c | 20 +++++++++++--------- >>> 1 file changed, 11 insertions(+), 9 deletions(-) >>> >>> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c >>> b/lib/librte_pmd_i40e/i40e_ethdev.c >>> index e1f581a..20c99a4 100644 >>> --- a/lib/librte_pmd_i40e/i40e_ethdev.c >>> +++ b/lib/librte_pmd_i40e/i40e_ethdev.c >>> @@ -580,7 +580,8 @@ i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi) >>> uint32_t val; >>> struct i40e_hw *hw = I40E_VSI_TO_HW(vsi); >>> uint16_t msix_vect = vsi->msix_intr; >>> - uint16_t interval = i40e_calc_itr_interval(RTE_LIBRTE_I40E_ITR_INTERVAL); >>> + uint16_t interval = >>> + i40e_calc_itr_interval(RTE_LIBRTE_I40E_ITR_INTERVAL); >>> int i; >>> >>> for (i = 0; i < vsi->nb_qps; i++) >>> @@ -603,25 +604,26 @@ i40e_vsi_queues_bind_intr(struct i40e_vsi *vsi) >>> /* Write first RX queue to Link list register as the head element */ >>> if (vsi->type != I40E_VSI_SRIOV) { >>> I40E_WRITE_REG(hw, I40E_PFINT_LNKLSTN(msix_vect - 1), >>> - (vsi->base_queue << I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) >> | >>> + (vsi->base_queue << >> ^^^^^^^^^^^^^^^^^^ >>> + I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | >> Here why so many indentations? or just my mail agent's display issue, if it does >> pls ignor. anyway, it is not a beautiful style in my mind :) > I do not know why. The right patch can be downloaded directly from dpdk.org. The email editor may not show the correct format. OK, if so, pls ignore my comment:) Thanks, Michael > >> I40E_WRITE_REG(hw, I40E_PFINT_LNKLSTN(msix_vect - 1), >> - (vsi->base_queue << I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) >> | >> + (vsi->base_queue << >> + I40E_PFINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | >> >> BTW, for coding style, I think we can follow the linux kernel, for kernel >> indentations, it always use the format like: n"Tab"+m"space", 0 <= m <=7. >> Another example is qemu, it is all "space" without "tab". > Yes, we have DPDK style, tab or tab + space. Space only is not allowed. Checkpatch.pl can check it for us. > >>> (0x0 << I40E_PFINT_LNKLSTN_FIRSTQ_TYPE_SHIFT)); >>> >>> I40E_WRITE_REG(hw, I40E_PFINT_ITRN(I40E_ITR_INDEX_DEFAULT, >>> - msix_vect - 1), interval); >>> + msix_vect - 1), interval); >>> >>> /* Disable auto-mask on enabling of all none-zero interrupt */ >>> I40E_WRITE_REG(hw, I40E_GLINT_CTL, >>> - I40E_GLINT_CTL_DIS_AUTOMASK_N_MASK); >>> - } >>> - else { >>> + I40E_GLINT_CTL_DIS_AUTOMASK_N_MASK); >>> + } else { >>> uint32_t reg; >>> + >>> /* num_msix_vectors_vf needs to minus irq0 */ >>> reg = (hw->func_caps.num_msix_vectors_vf - 1) * >>> vsi->user_param + (msix_vect - 1); >>> >>> - I40E_WRITE_REG(hw, I40E_VPINT_LNKLSTN(reg), >>> - (vsi->base_queue << I40E_VPINT_LNKLSTN_FIRSTQ_INDX_SHIFT) >> | >>> - (0x0 << I40E_VPINT_LNKLSTN_FIRSTQ_TYPE_SHIFT)); >>> + I40E_WRITE_REG(hw, I40E_VPINT_LNKLSTN(reg), (vsi->base_queue >> << >>> + I40E_VPINT_LNKLSTN_FIRSTQ_INDX_SHIFT) | >>> + (0x0 << I40E_VPINT_LNKLSTN_FIRSTQ_TYPE_SHIFT)); >> Here the same I think. >> >> Thanks, >> Michael >>> } >>> >>> I40E_WRITE_FLUSH(hw); > Regards, > Helin >
2014-10-29 05:00, Chen, Jing D:
> From: Zhang, Helin
> > Inside NIC RX interrupt is needed for single RX descriptor
> > write back. The fix is to correct the wrong configuration
> > of register 'I40E_QINT_RQCTL'. In addition, several code
> > style fixes are added.
> > Note that interrupt will be inside NIC only, that means it
> > will never be reported outside NIC hardware.
> >
> > Helin Zhang (2):
> > i40e: code style fix
> > i40e: fix of configuring inside NIC RX interrupt
>
> Acked-by: Jing Chen <jing.d.chen@intel.com>
Applied
About commit log of the second patch:
- title of a fix should show which feature is fixed, not how it is fixed
- content of the commit log should explain what was broken
Thanks
--
Thomas