DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 906D0208E7
MIME-Version: 1.0
References: <1542163848-837-1-git-send-email-wgong@codeaurora.org>
 <20181115002836.GA71934@google.com> <c94346b26a6d4b11a045a176ca854051@aptaiexm02f.ap.qualcomm.com>
 <20181115184333.GA87504@google.com> <87va4x8q2c.fsf@codeaurora.org>
In-Reply-To: <87va4x8q2c.fsf@codeaurora.org>
From:   =?UTF-8?Q?Micha=C5=82_Kazior?= <kazikcz@gmail.com>
Date:   Fri, 16 Nov 2018 08:56:27 +0100
Message-ID: <CABvG-CVAnwkiKVJik0PdsmRxF62kKv2N+aRKNq=nbopoExLvDA@mail.gmail.com>
Subject: Re: [PATCH] ath10k: support PCIe enter L1 state
To:     kvalo@codeaurora.org
Cc:     briannorris@chromium.org,
        linux-wireless <linux-wireless@vger.kernel.org>,
        wgong@qti.qualcomm.com, ath10k@lists.infradead.org,
        Wen Gong <wgong@codeaurora.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Fri, 16 Nov 2018 at 08:00, Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Brian Norris <briannorris@chromium.org> writes:
> > On Thu, Nov 15, 2018 at 06:38:25AM +0000, Wen Gong wrote:
> >> >
> >> > Is there some reason L1 was disabled in the first place? Was it know=
n to be
> >> > unreliable?
> >>
> >> Hi Brian,
> >> It is a BUG for power, and it is not considered this BUG before.
> >> So this change will fix the bug.
> >
> > I understand that the existing behavior is suboptimal for power, but on
> > the other hand, code that goes out of its way to *clear* the L1 flag
> > doesn't just pop up out of nowhere. Somebody clearly wrote that! If it
> > just meant "we didn't verify L1 at first", then maybe it's fine to
> > enable it unconditionally and see what happens, but if it meant "we
> > tried L1 on some old chip XXXX and it caused problems", then it would b=
e
> > nice to know what those problems were.
> >
> > Or maybe that is hard to figure out, given there's no public git histor=
y
> > tracking the original code, and we just need to try it out.
>
> Yeah, it seems L1 was disabled already on the first ath10k commit:
>
> 5e3dd157d7e70 (Kalle Valo                2013-06-12 20:52:10 +0300 2404) =
       pcie_config_flags &=3D ~PCIE_CONFIG_FLAG_ENABLE_L1;
>
> I don't remember anymore why but my guess is that the proprietary driver
> at the time didn't support it with QCA998X. Or maybe QCA988X doesn't
> even support L1? Michal, do you remember?

Proprietary driver has it ifdef-ed to enable/disable.

Older driver enabled it only for some station-only target/product so
by default QCA988X would build with L1 flag masked out. It made sense
to be conservative and change as little as possible to avoid random
bugs and breakage - so the logic was inherited minus the build-time
ifdef.

However 10.4 driver seems to enable it unconditionally. I'm not sure
if this depends on target firmware in any way or if some other host
driver or bus settings need to be pre-set before L1 can be expected to
work reliably.

I guess there's no way other than testing it out.


Micha=C5=82