[dpdk-dev] [PATCH v9 04/10] ethdev: add simple power management API

Ajit Khaparde ajit.khaparde at broadcom.com
Wed Oct 28 04:24:31 CET 2020


On Tue, Oct 27, 2020 at 4:29 PM Ananyev, Konstantin
<konstantin.ananyev at intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Thomas Monjalon <thomas at monjalon.net>
> > Sent: Tuesday, October 27, 2020 6:31 PM
> > To: Ma, Liang J <liang.j.ma at intel.com>; Ananyev, Konstantin <konstantin.ananyev at intel.com>
> > Cc: dev at dpdk.org; Burakov, Anatoly <anatoly.burakov at intel.com>; viktorin at rehivetech.com; Zhang, Qi Z <qi.z.zhang at intel.com>;
> > ruifeng.wang at arm.com; Xing, Beilei <beilei.xing at intel.com>; Guo, Jia <jia.guo at intel.com>; Yang, Qiming <qiming.yang at intel.com>;
> > Wang, Haiyue <haiyue.wang at intel.com>; Richardson, Bruce <bruce.richardson at intel.com>; Hunt, David <david.hunt at intel.com>;
> > jerinjacobk at gmail.com; nhorman at tuxdriver.com; McDaniel, Timothy <timothy.mcdaniel at intel.com>; Eads, Gage
> > <gage.eads at intel.com>; drc at linux.vnet.ibm.com; Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>; Yigit, Ferruh
> > <ferruh.yigit at intel.com>; jerinj at marvell.com; hemant.agrawal at nxp.com; viacheslavo at nvidia.com; matan at nvidia.com;
> > ajit.khaparde at broadcom.com; rahul.lakkireddy at chelsio.com; johndale at cisco.com; xavier.huwei at huawei.com; shahafs at nvidia.com;
> > sthemmin at microsoft.com; g.singh at nxp.com; rmody at marvell.com; maxime.coquelin at redhat.com; david.marchand at 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.
>


More information about the dev mailing list