From: "Kancharla, Sreehari" <sreehari.kancharla@linux•intel.com>
To: Loic Poulain <loic.poulain@linaro•org>
Cc: netdev@vger•kernel.org, kuba@kernel•org, davem@davemloft•net,
johannes@sipsolutions•net, ryazanov.s.a@gmail•com,
m.chetan.kumar@intel•com, chandrashekar.devegowda@intel•com,
linuxwwan@intel•com, chiranjeevi.rapolu@linux•intel.com,
haijun.liu@mediatek•com, ricardo.martinez@linux•intel.com,
andriy.shevchenko@linux•intel.com, dinesh.sharma@intel•com,
ilpo.jarvinen@linux•intel.com, moises.veleta@intel•com,
sreehari.kancharla@intel•com
Subject: Re: [PATCH net-next 2/2] net: wwan: t7xx: Add NAPI support
Date: Mon, 10 Oct 2022 19:52:49 +0530 [thread overview]
Message-ID: <40146bed-6547-f9f0-b7cf-e47f628c4dc8@linux.intel.com> (raw)
In-Reply-To: <CAMZdPi8KdWCke5s03Bvy_4NZcDsgp+jPW5dqvCHyiU2C==tsmw@mail.gmail.com>
Hi Loic,
On 9/12/2022 6:23 PM, Loic Poulain wrote:
> Hi Sreehari,
>
>
> On Fri, 9 Sept 2022 at 18:40, Sreehari Kancharla
> <sreehari.kancharla@linux•intel.com> wrote:
>> From: Haijun Liu <haijun.liu@mediatek•com>
>>
>> Replace the work queue based RX flow with a NAPI implementation
>> Remove rx_thread and dpmaif_rxq_work.
>> Introduce dummy network device. its responsibility is
>> - Binds one NAPI object for each DL HW queue and acts as
>> the agent of all those network devices.
>> - Use NAPI object to poll DL packets.
>> - Helps to dispatch each packet to the network interface.
>>
>> Signed-off-by: Haijun Liu <haijun.liu@mediatek•com>
>> Co-developed-by: Sreehari Kancharla <sreehari.kancharla@linux•intel.com>
>> Signed-off-by: Sreehari Kancharla <sreehari.kancharla@linux•intel.com>
>> Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel•com>
>> Acked-by: Ricardo Martinez <ricardo.martinez@linux•intel.com>
>> Acked-by: M Chetan Kumar <m.chetan.kumar@linux•intel.com>
>> ---
>> drivers/net/wwan/t7xx/t7xx_hif_dpmaif.h | 14 +-
>> drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.c | 220 +++++++--------------
>> drivers/net/wwan/t7xx/t7xx_hif_dpmaif_rx.h | 1 +
>> drivers/net/wwan/t7xx/t7xx_netdev.c | 93 ++++++++-
>> drivers/net/wwan/t7xx/t7xx_netdev.h | 5 +
>> 5 files changed, 167 insertions(+), 166 deletions(-)
>>
>> diff --git a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif.h b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif.h
>> index 1225ca0ed51e..0ce4505e813d 100644
>> --- a/drivers/net/wwan/t7xx/t7xx_hif_dpmaif.h
>> +++ b/drivers/net/wwan/t7xx/t7xx_hif_dpmaif.h
>> @@ -20,6 +20,7 @@
> [...]
>
>> -static void t7xx_dpmaif_rxq_work(struct work_struct *work)
>> +int t7xx_dpmaif_napi_rx_poll(struct napi_struct *napi, const int budget)
>> {
>> - struct dpmaif_rx_queue *rxq = container_of(work, struct dpmaif_rx_queue, dpmaif_rxq_work);
>> - struct dpmaif_ctrl *dpmaif_ctrl = rxq->dpmaif_ctrl;
>> - int ret;
>> + struct dpmaif_rx_queue *rxq = container_of(napi, struct dpmaif_rx_queue, napi);
>> + struct t7xx_pci_dev *t7xx_dev = rxq->dpmaif_ctrl->t7xx_dev;
>> + int ret, once_more = 0, work_done = 0;
>>
>> atomic_set(&rxq->rx_processing, 1);
>> /* Ensure rx_processing is changed to 1 before actually begin RX flow */
>> @@ -917,22 +840,54 @@ static void t7xx_dpmaif_rxq_work(struct work_struct *work)
>>
>> if (!rxq->que_started) {
>> atomic_set(&rxq->rx_processing, 0);
>> - dev_err(dpmaif_ctrl->dev, "Work RXQ: %d has not been started\n", rxq->index);
>> - return;
>> + dev_err(rxq->dpmaif_ctrl->dev, "Work RXQ: %d has not been started\n", rxq->index);
>> + return work_done;
>> }
>>
>> - ret = pm_runtime_resume_and_get(dpmaif_ctrl->dev);
>> - if (ret < 0 && ret != -EACCES)
>> - return;
>> + if (!rxq->sleep_lock_pending) {
>> + ret = pm_runtime_resume_and_get(rxq->dpmaif_ctrl->dev);
> AFAIK, polling is not called in a context allowing you to sleep (e.g.
> performing a synced pm runtime operation).
Device will be in resumed state when NAPI poll is invoked from IRQ context,
but host/driver can trigger RPM suspend to device. so we are using pm_runtime_resume_and_get
here to prevent runtime suspend.
>
>> + if (ret < 0 && ret != -EACCES)
>> + return work_done;
>>
>> - t7xx_pci_disable_sleep(dpmaif_ctrl->t7xx_dev);
>> - if (t7xx_pci_sleep_disable_complete(dpmaif_ctrl->t7xx_dev))
>> - t7xx_dpmaif_do_rx(dpmaif_ctrl, rxq);
>> + t7xx_pci_disable_sleep(t7xx_dev);
>> + }
>>
>> - t7xx_pci_enable_sleep(dpmaif_ctrl->t7xx_dev);
>> - pm_runtime_mark_last_busy(dpmaif_ctrl->dev);
>> - pm_runtime_put_autosuspend(dpmaif_ctrl->dev);
>> + ret = try_wait_for_completion(&t7xx_dev->sleep_lock_acquire);
> The logic seems odd, why not simply scheduling napi polling when you
> are really ready to handle it, i.e when you have awake condition & rx
> ready.
we are using device lock inside the NAPI poll to prevent device to
enter into low power mode when there are pending RX. once packet is
collected we release the device lock so that device can go to low power mode.
>
>> + if (!ret) {
>> + napi_complete_done(napi, work_done);
>> + rxq->sleep_lock_pending = true;
>> + napi_reschedule(napi);
>> + return work_done;
>> + }
>> +
> [...]
>
> Regards,
> Loic
Regards,
Sreehari
next prev parent reply other threads:[~2022-10-10 14:23 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-09 16:34 [PATCH net-next 1/2] net: wwan: t7xx: Use needed_headroom instead of hard_header_len Sreehari Kancharla
2022-09-09 16:35 ` [PATCH net-next 2/2] net: wwan: t7xx: Add NAPI support Sreehari Kancharla
2022-09-12 12:53 ` Loic Poulain
2022-10-10 14:22 ` Kancharla, Sreehari [this message]
2022-10-10 15:21 ` Andy Shevchenko
2022-10-20 14:22 ` Kancharla, Sreehari
2022-09-13 10:45 ` Ilpo Järvinen
2022-09-13 16:53 ` [PATCH net-next 1/2] net: wwan: t7xx: Use needed_headroom instead of hard_header_len Sergey Ryazanov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=40146bed-6547-f9f0-b7cf-e47f628c4dc8@linux.intel.com \
--to=sreehari.kancharla@linux$(echo .)intel.com \
--cc=andriy.shevchenko@linux$(echo .)intel.com \
--cc=chandrashekar.devegowda@intel$(echo .)com \
--cc=chiranjeevi.rapolu@linux$(echo .)intel.com \
--cc=davem@davemloft$(echo .)net \
--cc=dinesh.sharma@intel$(echo .)com \
--cc=haijun.liu@mediatek$(echo .)com \
--cc=ilpo.jarvinen@linux$(echo .)intel.com \
--cc=johannes@sipsolutions$(echo .)net \
--cc=kuba@kernel$(echo .)org \
--cc=linuxwwan@intel$(echo .)com \
--cc=loic.poulain@linaro$(echo .)org \
--cc=m.chetan.kumar@intel$(echo .)com \
--cc=moises.veleta@intel$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=ricardo.martinez@linux$(echo .)intel.com \
--cc=ryazanov.s.a@gmail$(echo .)com \
--cc=sreehari.kancharla@intel$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox