| Message ID | 1607598951-2340-2-git-send-email-loic.poulain@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,1/3] bus: mhi: core: Add helper API to return number of free TREs | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | warning | Series does not have a cover letter |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 84 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
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 >
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
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
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 --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;
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(-)