"Coelho, Luciano" <luciano.coelho@xxxxxxxxx> writes:
> Hi Kalle,
>
> On Wed, 2016-10-26 at 09:32 +0300, Kalle Valo wrote:
>> Luca Coelho <luca@xxxxxxxxx> writes:
>>
>> > From: Liad Kaufman <liad.kaufman@xxxxxxxxx>
>> >
>> > Run DQA flows by default, as long as the FW supports it.
>> >
>> > Signed-off-by: Liad Kaufman <liad.kaufman@xxxxxxxxx>
>> > Signed-off-by: Luca Coelho <luciano.coelho@xxxxxxxxx>
>>
>> What's this DQA mode? A short summary would have been nice why it's
>> useful.
>
> DQA is Dynamic Queue Allocation. We have been working on this for
> quite a while now (since v4.7) and have many patches mentioning that
> are already merged. The first patch is this:
>
> commit 24afba7690e4 ("iwlwifi: mvm: support bss dynamic alloc/dealloc
> of queues")
Yeah, but that's commited over six month ago.
> Its commit message has some explanation and we also have a DOC section
> in our driver that explains it in further details:
>
> https://git.kernel.org/cgit/linux/kernel/git/kvalo/wireless-drivers-next.git/tree/drivers/net/wireless/intel/iwlwifi/mvm/sta.h#n82
And that's too detailed. I was hoping more like an executive summary in
few sentences.
> Since we've been working on this for so long and already use the term
> DQA broadly, we thought it wouldn't be necessary to explain more when
> we are finally enabling it by default. But of course I can change that
> if you prefer.
I guessed I could find more information from the history and I know this
is obvious to your team, but it's not obvious to everyone. The commit
log should always answer the questions "Why?" and this isn't answering
that. For example, I need this information when sending pull requests to
Dave and I'm sure lots of other people find it useful as well,
especially when enabling a new feature.
So I'm not asking for a long essay, something like this would be
adequate:
"Run DQA flows by default, as long as the FW supports it. It's currently
supported on 1234, 3456 and 7654, maybe more in the future. DQA improves
latency when X is used or throughput when Y is disabled. On the downside
it sometimes slows down throughput when using Z but that's still
accetable as it's so rarely used."
But no need to change anything for this commit, but just keep in mind
for the future.
--
Kalle Valo
