diff mbox series

[v2,2/3] net: mhi: Get RX queue size from MHI core

Message ID 1607598951-2340-2-git-send-email-loic.poulain@linaro.org (mailing list archive)
State Superseded
Headers show
Series [v2,1/3] bus: mhi: core: Add helper API to return number of free TREs | expand

Commit Message

Loic Poulain Dec. 10, 2020, 11:15 a.m. UTC
The RX queue size can be determined at runtime by retrieving the
number of available transfer descriptors.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 v2: Fixed commit message typo

 drivers/net/mhi_net.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Manivannan Sadhasivam Dec. 11, 2020, 5:38 a.m. UTC | #1
On Thu, Dec 10, 2020 at 12:15:50PM +0100, Loic Poulain wrote:
> The RX queue size can be determined at runtime by retrieving the
> number of available transfer descriptors.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>  v2: Fixed commit message typo
> 
>  drivers/net/mhi_net.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/mhi_net.c b/drivers/net/mhi_net.c
> index 8e72d94..0333e07 100644
> --- a/drivers/net/mhi_net.c
> +++ b/drivers/net/mhi_net.c
> @@ -256,9 +256,6 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
>  	mhi_netdev->mdev = mhi_dev;
>  	SET_NETDEV_DEV(ndev, &mhi_dev->dev);
>  
> -	/* All MHI net channels have 128 ring elements (at least for now) */
> -	mhi_netdev->rx_queue_sz = 128;
> -
>  	INIT_DELAYED_WORK(&mhi_netdev->rx_refill, mhi_net_rx_refill_work);
>  	u64_stats_init(&mhi_netdev->stats.rx_syncp);
>  	u64_stats_init(&mhi_netdev->stats.tx_syncp);
> @@ -268,6 +265,9 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
>  	if (err)
>  		goto out_err;
>  
> +	/* Number of transfer descriptors determines size of the queue */
> +	mhi_netdev->rx_queue_sz = mhi_get_free_desc_count(mhi_dev, DMA_FROM_DEVICE);
> +

This value is not static right? You might need to fetch the count in
mhi_net_rx_refill_work().

Thanks,
Mani

>  	err = register_netdev(ndev);
>  	if (err)
>  		goto out_err;
> -- 
> 2.7.4
>
Loic Poulain Dec. 11, 2020, 9:40 a.m. UTC | #2
Hi Mani,

On Fri, 11 Dec 2020 at 06:38, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Thu, Dec 10, 2020 at 12:15:50PM +0100, Loic Poulain wrote:
> > The RX queue size can be determined at runtime by retrieving the
> > number of available transfer descriptors.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> > ---
> >  v2: Fixed commit message typo
> >
> >  drivers/net/mhi_net.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
[...]
> > -
> >       INIT_DELAYED_WORK(&mhi_netdev->rx_refill, mhi_net_rx_refill_work);
> >       u64_stats_init(&mhi_netdev->stats.rx_syncp);
> >       u64_stats_init(&mhi_netdev->stats.tx_syncp);
> > @@ -268,6 +265,9 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
> >       if (err)
> >               goto out_err;
> >
> > +     /* Number of transfer descriptors determines size of the queue */
> > +     mhi_netdev->rx_queue_sz = mhi_get_free_desc_count(mhi_dev, DMA_FROM_DEVICE);
> > +
>
> This value is not static right? You might need to fetch the count in
> mhi_net_rx_refill_work().

It is, actually here driver is just looking for the total queue size,
which is the number of descriptors at init time. This total queue size
is used later to determine the level of MHI queue occupancy rate.

Regards,
Loic
Manivannan Sadhasivam Dec. 11, 2020, 10:15 a.m. UTC | #3
On Fri, Dec 11, 2020 at 10:40:13AM +0100, Loic Poulain wrote:
> Hi Mani,
> 
> On Fri, 11 Dec 2020 at 06:38, Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Thu, Dec 10, 2020 at 12:15:50PM +0100, Loic Poulain wrote:
> > > The RX queue size can be determined at runtime by retrieving the
> > > number of available transfer descriptors.
> > >
> > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> > > ---
> > >  v2: Fixed commit message typo
> > >
> > >  drivers/net/mhi_net.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> [...]
> > > -
> > >       INIT_DELAYED_WORK(&mhi_netdev->rx_refill, mhi_net_rx_refill_work);
> > >       u64_stats_init(&mhi_netdev->stats.rx_syncp);
> > >       u64_stats_init(&mhi_netdev->stats.tx_syncp);
> > > @@ -268,6 +265,9 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
> > >       if (err)
> > >               goto out_err;
> > >
> > > +     /* Number of transfer descriptors determines size of the queue */
> > > +     mhi_netdev->rx_queue_sz = mhi_get_free_desc_count(mhi_dev, DMA_FROM_DEVICE);
> > > +
> >
> > This value is not static right? You might need to fetch the count in
> > mhi_net_rx_refill_work().
> 
> It is, actually here driver is just looking for the total queue size,
> which is the number of descriptors at init time. This total queue size
> is used later to determine the level of MHI queue occupancy rate.
> 

Right but what if the size got increased in runtime (recycled etc...), we won't
be fully utilizing the ring.

Thanks,
Mani

> Regards,
> Loic
Loic Poulain Dec. 11, 2020, 10:40 a.m. UTC | #4
On Fri, 11 Dec 2020 at 11:15, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> On Fri, Dec 11, 2020 at 10:40:13AM +0100, Loic Poulain wrote:
> > Hi Mani,
> >
> > On Fri, 11 Dec 2020 at 06:38, Manivannan Sadhasivam
> > <manivannan.sadhasivam@linaro.org> wrote:
> > >
> > > On Thu, Dec 10, 2020 at 12:15:50PM +0100, Loic Poulain wrote:
> > > > The RX queue size can be determined at runtime by retrieving the
> > > > number of available transfer descriptors.
> > > >
> > > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> > > > ---
> > > >  v2: Fixed commit message typo
> > > >
> > > >  drivers/net/mhi_net.c | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > [...]
> > > > -
> > > >       INIT_DELAYED_WORK(&mhi_netdev->rx_refill, mhi_net_rx_refill_work);
> > > >       u64_stats_init(&mhi_netdev->stats.rx_syncp);
> > > >       u64_stats_init(&mhi_netdev->stats.tx_syncp);
> > > > @@ -268,6 +265,9 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
> > > >       if (err)
> > > >               goto out_err;
> > > >
> > > > +     /* Number of transfer descriptors determines size of the queue */
> > > > +     mhi_netdev->rx_queue_sz = mhi_get_free_desc_count(mhi_dev, DMA_FROM_DEVICE);
> > > > +
> > >
> > > This value is not static right? You might need to fetch the count in
> > > mhi_net_rx_refill_work().
> >
> > It is, actually here driver is just looking for the total queue size,
> > which is the number of descriptors at init time. This total queue size
> > is used later to determine the level of MHI queue occupancy rate.
> >
>
> Right but what if the size got increased in runtime (recycled etc...), we won't
> be fully utilizing the ring.

The queue size can not be more than the initial size, which is the
number of elements in the rings (e.g. 128). At runtime, the driver
calls again mhi_get_free_desc_count() in DL callback to determine the
current number available slots (e.g. 32). If this value is higher than
a certain limit (e.g 128 /2), then we start refilling the MHI RX queue
with fresh buffers to prevent buffer starvation.

Regards,
Loic
diff mbox series

Patch

diff --git a/drivers/net/mhi_net.c b/drivers/net/mhi_net.c
index 8e72d94..0333e07 100644
--- a/drivers/net/mhi_net.c
+++ b/drivers/net/mhi_net.c
@@ -256,9 +256,6 @@  static int mhi_net_probe(struct mhi_device *mhi_dev,
 	mhi_netdev->mdev = mhi_dev;
 	SET_NETDEV_DEV(ndev, &mhi_dev->dev);
 
-	/* All MHI net channels have 128 ring elements (at least for now) */
-	mhi_netdev->rx_queue_sz = 128;
-
 	INIT_DELAYED_WORK(&mhi_netdev->rx_refill, mhi_net_rx_refill_work);
 	u64_stats_init(&mhi_netdev->stats.rx_syncp);
 	u64_stats_init(&mhi_netdev->stats.tx_syncp);
@@ -268,6 +265,9 @@  static int mhi_net_probe(struct mhi_device *mhi_dev,
 	if (err)
 		goto out_err;
 
+	/* Number of transfer descriptors determines size of the queue */
+	mhi_netdev->rx_queue_sz = mhi_get_free_desc_count(mhi_dev, DMA_FROM_DEVICE);
+
 	err = register_netdev(ndev);
 	if (err)
 		goto out_err;