[RFC] Add the BMPS timer to enter/exit BMPS mode when running

Thu Jul 25 07:46:24 EDT 2013

On Thu, Jul 25, 2013 at 7:06 PM, Eugene Krasnikov <k.eugene.e at gmail.com> wrote:
> Hi Yanbo,
>
> I am still concerned regarding this peace of code:
>>> +{
>>> +       struct wcn36xx *wcn = container_of(work, struct wcn36xx,
>>> +                                          bmps_work.work);
>>> +
>>> +       /* Enter or Exit BMPS depend on the currently condition */
>>> +       wcn36xx_pmc_enter_bmps_state(wcn);
>>> +       wcn36xx_pmc_exit_bmps_state(wcn);
>>> +
>>> +       ieee80211_queue_delayed_work(wcn->hw, &wcn->bmps_work,
>>> +                                    msecs_to_jiffies(BMPS_MEASURE_TIMER_DEFAULT));
>>> +}
>
> If some external component want's to enter full power mode then wcn
> should enter full power mode until this external component will ask
> wcn to exit full power mode. So i do not see any reasons in the timer
> BMPS_MEASURE_TIMER_DEFAULT? Could you please explain in more details
> why do you think we need this timer?

Ener_full_power case is a bit easy, let's consider enter bmps case firstly.

Indeed the timer mechanism is not better than call directly enter_bmps
from practicality layer.
But just image in the code, there are maybe several or even more point
or even which may need enter full power mode,
They maybe trigger in user event, or packet info, or interrupt context
which reply from FW response,
And we need call each enter_bmps at that point and judge all the
condition, even worse maybe in some point the enter_bmps can not be
called
directly call maybe involve deadlock, difference context for workqueue
etc , and the caller must consider these condition before directly
call it.
Does the way what we put them in a timer and manager them at one point
will be more easily?

For the enter full power, it also a bit easy to maintain enter point
in one place (the timer) than directly call them at anywhere it need,
but frankly speak
may not too much benefit at that moment, but more for keep the
enter/exit bmps mechanism consistent.

BR /Yanbo

>
>
> 2013/7/25 Eugene Krasnikov <k.eugene.e at gmail.com>:
>> Hi Yanbo,
>>
>> Do you know starting which FW version data flow monitoring is moved to
>> FW? I want just to be sure that after bringing this patch in it will
>> not break tput on my device.
>>
>> 2013/7/24 Eugene Krasnikov <k.eugene.e at gmail.com>:
>>> Just FYI in future we will need to implement BSS_CHANGED_PS so upper
>>> layers can control whether driver can shut down radio or not. E.g.
>>> today during DHCP DORA upper layers will notify bottom half to exit
>>> bmps using BSS_CHANGED_PS flag.
>>>
>>> 2013/7/24 Eugene Krasnikov <k.eugene.e at gmail.com>:
>>>> Do you know from which FW version data flow monitor was moved to FW?
>>>> Because i have an old FW version so I will need it in driver anyway.
>>>>
>>>> 2013/7/24 YanBo <dreamfly281 at gmail.com>:
>>>>> On Wed, Jul 24, 2013 at 8:57 PM, Eugene Krasnikov <k.eugene.e at gmail.com> wrote:
>>>>>>> +static void wcn36xx_pmc_bmps_measure_work(struct work_struct *work)
>>>>>>> +{
>>>>>>> +       struct wcn36xx *wcn = container_of(work, struct wcn36xx,
>>>>>>> +                                          bmps_work.work);
>>>>>>> +
>>>>>>> +       /* Enter or Exit BMPS depend on the currently condition */
>>>>>>> +       wcn36xx_pmc_enter_bmps_state(wcn);
>>>>>>> +       wcn36xx_pmc_exit_bmps_state(wcn);
>>>>>>> +
>>>>>>> +       ieee80211_queue_delayed_work(wcn->hw, &wcn->bmps_work,
>>>>>>> +                                    msecs_to_jiffies(BMPS_MEASURE_TIMER_DEFAULT));
>>>>>>> +}
>>>>>>
>>>>>> What is the purpose to queue the work again? Does it mean that wcn36xx
>>>>>> will enter and exit bmps every BMPS_MEASURE_TIMER_DEFAULT seconds?
>>>>> it will make sure the work queue running like a timer, cause for every
>>>>> time period, they may be some event will
>>>>> trigger the enter full power mode event, the enter/exit bmps function
>>>>> used to check whether the condition is triggered or not.
>>>>>>
>>>>>> I also thought you gonna implement a patch that will check if there is
>>>>>> a high data flow then exit bmps . But apparently this patch is not
>>>>>> doing this.
>>>>>>
>>>>>
>>>>> That is also I intended to implemented, but as I get the newest
>>>>> update, the monitor high/lower data flow function has been moved into
>>>>> firmware,
>>>>> the firmware will do such work, and in the host driver, there is still
>>>>> some scenario need be protected to avoid into BMPS, like currently the
>>>>> DHCP process(to be confirm?) some BTC cases. in such case, they can
>>>>> all the  wcn36xx_vote_enter_full_power to protect such scenario
>>>>>>
>>>>>> 2013/7/24 Olof Johansson <dev at skyshaper.net>:
>>>>>>> Hello Yanbo.
>>>>>>>
>>>>>>> No, I mean this parameter:
>>>>>>> https://www.codeaurora.org/cgit/quic/la/platform/vendor/qcom-opensource/wlan/prima/tree/firmware_bin/WCNSS_qcom_cfg.ini?h=b2g/jb_mr1_rb2.17#n339
>>>>>>>
>>>>>>> Cheers
>>>>>>> --
>>>>>>> Olof
>>>>>>>
>>>>>>> On Wed, Jul 24, 2013 at 4:15 AM, YanBo <dreamfly281 at gmail.com> wrote:
>>>>>>>> Hi Olof,
>>>>>>>>
>>>>>>>> What the specify configuration name you mentioned, do you mean
>>>>>>>> dynamic_dtim? if so, it is used for dynamic adjust the
>>>>>>>> DTIM value but not conflict with BMPS  timer
>>>>>>>>
>>>>>>>> BR /Yanbo
>>>>>>>>
>>>>>>>> On Wed, Jul 24, 2013 at 12:59 AM, Olof Johansson <dev at skyshaper.net> wrote:
>>>>>>>>> On Tue, Jul 23, 2013 at 6:46 PM, Eugene Krasnikov <k.eugene.e at gmail.com> wrote:
>>>>>>>>>> @Olof: I am not sure  what do you mean under DYNAMIC_PS but there is
>>>>>>>>>> one configuration parameters that is responsible for how time between
>>>>>>>>>> last data packet received from AP and NULL packet sent from STA to AP
>>>>>>>>>> with PS bit set.
>>>>>>>>>
>>>>>>>>> There is a CFG parameter (sent at start or update config smd packets)
>>>>>>>>> which is called DYNAMIC_PS which corresponds to the value of data
>>>>>>>>> inactivity timeout in the prima .ini file. This value adjusts how long
>>>>>>>>> the firmware is staying awake after sending or receiving a packet
>>>>>>>>> before going back to BMPS mode. This value we adjusted several times
>>>>>>>>> to trim throughput as well as btcoex issues.
>>>>>>>>>
>>>>>>>>> At least that is how it was explained to me by QC :)
>>>>>>>>>
>>>>>>>>> Cheers
>>>>>>>>> --
>>>>>>>>> Olof
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Best regards,
>>>>>> Eugene
>>>>
>>>>
>>>>
>>>> --
>>>> Best regards,
>>>> Eugene
>>>
>>>
>>>
>>> --
>>> Best regards,
>>> Eugene
>>
>>
>>
>> --
>> Best regards,
>> Eugene
>
>
>
> --
> Best regards,
> Eugene