[RFC] Add the BMPS timer to enter/exit BMPS mode when running
Eugene Krasnikov
k.eugene.e at gmail.com
Thu Jul 25 11:24:12 EDT 2013
Hi Yanbo,
Timer can lead to random entering bmps mode and that is more difficult
to track. That's why i think whenever wcn can enter bmps it must enter
bmps. Also with this patch people who are using old FW will have a low
tput. So my suggestion as a first step to implement a driver data flow
control. How does that sound?
2013/7/25 YanBo <dreamfly281 at gmail.com>:
> 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
--
Best regards,
Eugene
More information about the wcn36xx
mailing list