<div dir="ltr"><br><br>On Tue, Oct 10, 2023 at 9:51 AM Ferruh Yigit <<a href="mailto:ferruh.yigit@amd.com">ferruh.yigit@amd.com</a>> wrote:<br>><br>> On 10/5/2023 9:52 PM, Ed Czeck wrote:<br>> > features and function have been removed from FPGA firmware<br>> ><br>><br>> I am always a little confused how you manage the deployment, if a<br>> customer requires RQ pacing, how you manage it, at least should it be<br>> documented in driver documentation that RQ pacing supported before<br>> v23.11, or something like that?<br>> If this doesn't make sense for your deployment model, scratch it, this<br><div>> is just a reminder if it is useful.</div><div><br></div><div><div>Our deployment needs to balance the DPDK  release, our FPGA firmware, our (not yet</div><div>published) DPDKpatches and external FPGA-IP firmware from AMD (Xilinx) and Intel </div><div>(Altera).  We have safety code to ensure that these fall into a valid alignment. We also </div><div>try to maintain SW/FPGA compatibility and evolve without breaking things unnecessarily. </div><div>Our releases follow DPDK's and we update other tools as they are released.</div><br><div>For RQ pacing, it was an internal feature needed for older Xilinx PCIE IP, with a</div><div>narrow exposure via our PMD.  The Xilinx IP no longer requires this module, our</div><div>firmware no longer includes it, and the PMD can drop.  It was not user controllable </div><div>nor an advertised feature.<br></div><div><br></div></div>><br>><br>> > Signed-off-by: Ed Czeck <<a href="mailto:ed.czeck@atomicrules.com">ed.czeck@atomicrules.com</a>><br>> > ---<br>> >  drivers/net/ark/ark_ethdev.c | 62 ++++++++------------------------<br>> >  drivers/net/ark/ark_global.h |  3 --<br>> >  drivers/net/ark/ark_rqp.c    | 70 ------------------------------------<br>> >  drivers/net/ark/ark_rqp.h    | 58 ------------------------------<br>> >  drivers/net/ark/meson.build  |  1 -<br>> >  5 files changed, 15 insertions(+), 179 deletions(-)<br>> >  delete mode 100644 drivers/net/ark/ark_rqp.c<br>> >  delete mode 100644 drivers/net/ark/ark_rqp.h<br>> ><br>> > diff --git a/drivers/net/ark/ark_ethdev.c b/drivers/net/ark/ark_ethdev.c<br>> > index 90d3c8abe6..306121ba31 100644<br>> > --- a/drivers/net/ark/ark_ethdev.c<br>> > +++ b/drivers/net/ark/ark_ethdev.c<br>> > @@ -17,7 +17,6 @@<br>> >  #include "ark_mpu.h"<br>> >  #include "ark_ddm.h"<br>> >  #include "ark_udm.h"<br>> > -#include "ark_rqp.h"<br>> >  #include "ark_pktdir.h"<br>> >  #include "ark_pktgen.h"<br>> >  #include "ark_pktchkr.h"<br>> > @@ -107,36 +106,32 @@ static const struct rte_pci_id pci_id_ark_map[] = {<br>> >   * This structure is used to statically define the capabilities<br>> >   * of supported devices.<br>> >   * Capabilities:<br>> > - *  rqpacing -<br>> > - * Some HW variants require that PCIe read-requests be correctly throttled.<br>> > - * This is called "rqpacing" and has to do with credit and flow control<br>> > - * on certain Arkville implementations.<br>> > + *    isvf -- defined for function id that are virtual<br>> >   */<br>> >  struct ark_caps {<br>> > -     bool rqpacing;<br>> >       bool isvf;<br>> >  };<br>> >  struct ark_dev_caps {<br>> >       uint32_t  device_id;<br>> >       struct ark_caps  caps;<br>> >  };<br>> > -#define SET_DEV_CAPS(id, rqp, vf)                    \<br>> > -     {id, {.rqpacing = rqp, .isvf = vf} }<br>> > +#define SET_DEV_CAPS(id, vf)                 \<br>> > +     {id, {.isvf = vf} }<br>> ><br>> >  static const struct ark_dev_caps<br>> >  ark_device_caps[] = {<br>> > -                  SET_DEV_CAPS(0x100d, true, false),<br>> > -                  SET_DEV_CAPS(0x100e, true, false),<br>> > -                  SET_DEV_CAPS(0x100f, true, false),<br>> > -                  SET_DEV_CAPS(0x1010, false, false),<br>> > -                  SET_DEV_CAPS(0x1017, true, false),<br>> > -                  SET_DEV_CAPS(0x1018, true, false),<br>> > -                  SET_DEV_CAPS(0x1019, true, false),<br>> > -                  SET_DEV_CAPS(0x101a, true, false),<br>> > -                  SET_DEV_CAPS(0x101b, true, false),<br>> > -                  SET_DEV_CAPS(0x101c, true, true),<br>> > -                  SET_DEV_CAPS(0x101e, false, false),<br>> > -                  SET_DEV_CAPS(0x101f, false, false),<br>> > +                  SET_DEV_CAPS(0x100d, false),<br>> > +                  SET_DEV_CAPS(0x100e, false),<br>> > +                  SET_DEV_CAPS(0x100f, false),<br>> > +                  SET_DEV_CAPS(0x1010, false),<br>> > +                  SET_DEV_CAPS(0x1017, false),<br>> > +                  SET_DEV_CAPS(0x1018, false),<br>> > +                  SET_DEV_CAPS(0x1019, false),<br>> > +                  SET_DEV_CAPS(0x101a, false),<br>> > +                  SET_DEV_CAPS(0x101b, false),<br>> > +                  SET_DEV_CAPS(0x101c, true),<br>> > +                  SET_DEV_CAPS(0x101e, false),<br>> > +                  SET_DEV_CAPS(0x101f, false),<br>> >                    {.device_id = 0,}<br>> >  };<br>> ><br>> > @@ -301,9 +296,6 @@ eth_ark_dev_init(struct rte_eth_dev *dev)<br>> >       int port_count = 1;<br>> >       int p;<br>> >       uint16_t num_queues;<br>> > -     bool rqpacing = false;<br>> > -<br>> > -     ark->eth_dev = dev;<br>> ><br>><br>> Above "ark->eth_dev" assignment doesn't look directly related with RQ<br><div>> pacing, I just want to double check if it is removed intentionally?</div><div><br></div><div>This change is in error.   Thanks for catching it.  New patch to follow.</div><div><br></div>><br>></div>