On Tue, Oct 27, 2020 at 4:29 PM Ananyev, Konstantin <konstantin.anan...@intel.com> wrote: > > > > > -----Original Message----- > > From: Thomas Monjalon <tho...@monjalon.net> > > Sent: Tuesday, October 27, 2020 6:31 PM > > To: Ma, Liang J <liang.j...@intel.com>; Ananyev, Konstantin > > <konstantin.anan...@intel.com> > > Cc: dev@dpdk.org; Burakov, Anatoly <anatoly.bura...@intel.com>; > > vikto...@rehivetech.com; Zhang, Qi Z <qi.z.zh...@intel.com>; > > ruifeng.w...@arm.com; Xing, Beilei <beilei.x...@intel.com>; Guo, Jia > > <jia....@intel.com>; Yang, Qiming <qiming.y...@intel.com>; > > Wang, Haiyue <haiyue.w...@intel.com>; Richardson, Bruce > > <bruce.richard...@intel.com>; Hunt, David <david.h...@intel.com>; > > jerinjac...@gmail.com; nhor...@tuxdriver.com; McDaniel, Timothy > > <timothy.mcdan...@intel.com>; Eads, Gage > > <gage.e...@intel.com>; d...@linux.vnet.ibm.com; Andrew Rybchenko > > <andrew.rybche...@oktetlabs.ru>; Yigit, Ferruh > > <ferruh.yi...@intel.com>; jer...@marvell.com; hemant.agra...@nxp.com; > > viachesl...@nvidia.com; ma...@nvidia.com; > > ajit.khapa...@broadcom.com; rahul.lakkire...@chelsio.com; > > johnd...@cisco.com; xavier.hu...@huawei.com; shah...@nvidia.com; > > sthem...@microsoft.com; g.si...@nxp.com; rm...@marvell.com; > > maxime.coque...@redhat.com; david.march...@redhat.com > > Subject: Re: [dpdk-dev] [PATCH v9 04/10] ethdev: add simple power > > management API > > > > 27/10/2020 18:43, Ananyev, Konstantin: > > > > 27/10/2020 12:15, Liang, Ma: > > > > > > > --- a/lib/librte_ethdev/rte_ethdev.h > > > > > > > +++ b/lib/librte_ethdev/rte_ethdev.h > > > > > > > +/** > > > > > > > + * Retrieve the wake up address for the receive queue. > > > > > > > > > > > > I guess how this function should be used, > > > > > > but a bit more explanations would not hurt here. > > > > > agree > > > > > > > + * > > > > > > > + * @param port_id > > > > > > > + * The port identifier of the Ethernet device. > > > > > > > + * @param queue_id > > > > > > > + * The Rx queue on the Ethernet device for which information > > > > > > > will be > > > > > > > + * retrieved. > > > > > > > + * @param wake_addr > > > > > > > + * The pointer to the address which will be monitored. > > > > > > > > > > > > This function does not make the address monitored, right? > > > > > This function only get the target wakeup address. that does not > > > > > monitor this address. > > > > > > > > > > > > > + * @param expected > > > > > > > + * The pointer to value to be expected when descriptor is set. > > > > > > > > > > > > Not sure we should restrict it to a "descriptor". > > > > > actully that is not limited to a descriptor, any writeback content > > > > > should work. > > > > > > > > > > > > Expecting a value or some bits looks too much restrictive. > > > > > > I understand it probably fits well for Intel NICs, > > > > > > but in the general case, we can imagine that any change > > > > > > in a byte array could be a wake up signal. > > > > > > > > > > this parameter doesn not limited user how to use it. > > > > > In fact, current design can support any bits change within 64 bits > > > > > content. > > > > > > > > How the driver can specify that any value change should be monitored? > > > > I understand that it is only a value/mask pair, > > > > it does not give room for "any value". > > > > > > As I can read the code, value=0, mask=0 will provide you with 'any value'. > > > Though it would mean that rte_power_monitor() will *always* go into sleep, > > > so not sure what will be there any practical usage for such case. > > > > I think what is missing is to allow waking up when the value > > of a byte array is changing, without specifiying any value. > > > I think it will always wakeup on any write to wait_addr. > What you control with value/mask pair - when we should go to sleep. > In other words: > ret = rte_eth_get_wake_addr(port, queue, &wait_addr, &value, &mask, ....); > > mask==0: always go to sleep, wakeup at any store to wait_addr. > mask!=0: go to sleep only if (*wait_addr & mask) == value, wakeup at any > store to wait_addr. I did not get this impression on reading it first time. Till you put it this way. The comment "if the masked value is already matching, abort" stumped me.
> > Liang, Anatoly - feel free to correct me here, if I missed something. >
