From: Luis R. Rodriguez on
On Mon, Jul 26, 2010 at 3:21 PM, Matthew Garrett <mjg59(a)srcf.ucam.org> wrote:
> On Mon, Jul 26, 2010 at 03:15:32PM -0700, Luis R. Rodriguez wrote:
>> On Mon, Jul 26, 2010 at 2:25 PM, Matthew Garrett <mjg59(a)srcf.ucam.org> wrote:
>> > This may need to be done on a chip by chip basis. Take a look at
>> > http://www.atheros.cz/inffile.php?inf=68&bit=32&atheros=AR5002G&system=4
>> > and some of the other inf files on that site to see which devices
>> > provide the PciASPMOptIn flag - those should support ASPM states even if
>> > they're pre-1.1 devices.
>>
>> I rather we not bother with these, lets simply follow the kernel's
>> lead here for its rule matching.
>
> Sorry? The idea is to indicate which chips support ASPM even though
> they're pre-PCIe 1.1. If all Atheros parts work fine with L1 then that
> makes things much easier, but it would be good to know the correct set
> of chips that are broken with L0s.

What I meant was that the PCI config space would already have L1
enabled if L1 worked, so I don't see why we would need to nitpick out
specifics here. All Atheros PCIE chips should work with L1. The advise
given is to disable L0s though. I believe AR2425 would be one which
likely had L0s enabled but requires it to be disabled. Not sure of
others. But this is why I am saying this can be done globally for all
ath5k chipsets.

Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Luis R. Rodriguez on
On Mon, Jul 26, 2010 at 3:24 PM, Matthew Garrett <mjg59(a)srcf.ucam.org> wrote:
> On Mon, Jul 26, 2010 at 03:20:23PM -0700, Luis R. Rodriguez wrote:
>> On Mon, Jul 26, 2010 at 2:14 PM, Matthew Garrett <mjg59(a)srcf.ucam.org> wrote:
>> > On Mon, Jul 26, 2010 at 02:06:51PM -0700, Luis R. Rodriguez wrote:
>> >
>> >> No, ASPM must be enabled by the Systems Integrator through the BIOS, there are
>> >> other settings that have to be taken care of like modifying some PCI entrance and
>> >> exit latency timers, the number of FTS packets we send to exit L0s, amongst
>> >> other things. If a user selectively enables L1 but the BIOS had it disabled on
>> >> the device it may not work correctly.
>> >
>> > That's really the job of the driver.
>>
>> The problem is that sometimes tweaks need to be done on the PCI
>> controller/root complex, not the PCIE device/endpoint. Today these
>> sort of changes *are* handled by the respective  systems
>> integrator/BIOS team and varies depending on the root complex used.
>> Atheros does not handle these at all in the driver.
>
> Really? Your Windows drivers declare that they support ASPM for some
> parts, which will trigger Vista and later to program L0s and L1
> (depending on system config) without further reference to the driver. If
> we need hooks in the kernel for drivers to provide further feedback to
> make sure this works correctly then we can certainly provide them...

Sure, agreed, I'm just pointing out today there are other changes made
and even Microsoft does not get involved with them, so it is the same
for us. However I do agree if the kernel can expose API to tweak
things then great.

>> > If the ASPM policy is set to
>> > powersave, the fadt doesn't indicate that ASPM should be disabled and
>> > the bus's _OSC method grants full control then the kernel will enable
>> > whatever combination of L states meet the latency constraints. If the
>> > hardware has additional constraints then the hardware-specific driver
>> > needs to handle them.
>>
>> This makes sense but Is there an API for this?
>
> Right now, no. What do you need?

Not sure, let me check internally and see if I can get the exact details.

>> > We don't rely on the BIOS to set up ASPM states. Nor does Windows.
>>
>> Understood, but today some tweaks seem to be done on the BIOS
>> depending on the endpoint / root complex.
>
> The current situation is that we won't modify anything that the BIOS has
> done, other than the link and clock pm bits. So if the BIOS has done
> something for us, we won't (currently) step on it.

Sure, understood.

Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Matthew Garrett on
On Mon, Jul 26, 2010 at 03:31:28PM -0700, Luis R. Rodriguez wrote:
> On Mon, Jul 26, 2010 at 3:29 PM, Matthew Garrett <mjg59(a)srcf.ucam.org> wrote:
> > On Mon, Jul 26, 2010 at 03:26:37PM -0700, Luis R. Rodriguez wrote:
> >
> >> What I meant was that the PCI config space would already have L1
> >> enabled if L1 worked, so I don't see why we would need to nitpick out
> >> specifics here. All Atheros PCIE chips should work with L1. The advise
> >> given is to disable L0s though. I believe AR2425 would be one which
> >> likely had L0s enabled but requires it to be disabled. Not sure of
> >> others. But this is why I am saying this can be done globally for all
> >> ath5k chipsets.
> >
> > If L1 is set but the chip is pre-PCIe 1.1 then we'll disable L1 unless
> > the driver tells us that it's functional. The .inf from the Windows
> > driver seemed to suggest that only a subset of the chips re-enabled L1
> > there, but if it's ok in general then that's a straightforward one-line
> > patch.
>
> But why can't we just rely on what the device already has on its PCI
> config space and only ensure to disable L0s?

Because we globally disable ASPM on pre-1.1 devices, because that's what
Windows does. It makes it easier for us to figure out what level of
support we can expect from different hardware revisions.

--
Matthew Garrett | mjg59(a)srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Luis R. Rodriguez on
On Mon, Jul 26, 2010 at 3:33 PM, Matthew Garrett <mjg59(a)srcf.ucam.org> wrote:
> On Mon, Jul 26, 2010 at 03:31:28PM -0700, Luis R. Rodriguez wrote:
>> On Mon, Jul 26, 2010 at 3:29 PM, Matthew Garrett <mjg59(a)srcf.ucam.org> wrote:
>> > On Mon, Jul 26, 2010 at 03:26:37PM -0700, Luis R. Rodriguez wrote:
>> >
>> >> What I meant was that the PCI config space would already have L1
>> >> enabled if L1 worked, so I don't see why we would need to nitpick out
>> >> specifics here. All Atheros PCIE chips should work with L1. The advise
>> >> given is to disable L0s though. I believe AR2425 would be one which
>> >> likely had L0s enabled but requires it to be disabled. Not sure of
>> >> others. But this is why I am saying this can be done globally for all
>> >> ath5k chipsets.
>> >
>> > If L1 is set but the chip is pre-PCIe 1.1 then we'll disable L1 unless
>> > the driver tells us that it's functional. The .inf from the Windows
>> > driver seemed to suggest that only a subset of the chips re-enabled L1
>> > there, but if it's ok in general then that's a straightforward one-line
>> > patch.
>>
>> But why can't we just rely on what the device already has on its PCI
>> config space and only ensure to disable L0s?
>
> Because we globally disable ASPM on pre-1.1 devices, because that's what
> Windows does. It makes it easier for us to figure out what level of
> support we can expect from different hardware revisions.

I see.. thanks Mathew... in that case since L1 works on all devices we
could just force enable L1 for all PCIE devices. What do you think?

Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Luis R. Rodriguez on
On Mon, Jul 26, 2010 at 01:13:22PM -0700, Luis R. Rodriguez wrote:
> On Sat, Jun 19, 2010 at 08:32:44AM -0700, Maxim Levitsky wrote:
> > On Sat, 2010-06-19 at 16:02 +0300, Maxim Levitsky wrote:
> > > On Sat, 2010-06-19 at 08:38 -0400, Bob Copeland wrote:
> > > > On Sat, Jun 19, 2010 at 10:49:34AM +0300, Maxim Levitsky wrote:
> > > > > How this patch?
> > > >
> > > > Looks fine to me. Some nitpicking below but feel free to add my
> > > >
> > > > Acked-by: Bob Copeland <me(a)bobcopeland.com>
> > > >
> > Done.
> >
> > Best regards,
> > Maxim Levitsky
> >
> > ---
> >
> > commit 616afa397b3e843f2aba06be12a30e72dfff7740
> > Author: Maxim Levitsky <maximlevitsky(a)gmail.com>
> > Date: Thu Jun 17 23:21:42 2010 +0300
> >
> > ath5k: disable ASPM
> >
> > Atheros card on Acer Aspire One (AOA150, Atheros Communications Inc. AR5001
> > Wireless Network Adapter [168c:001c] (rev 01)) doesn't work well with ASPM
> > enabled. With ASPM ath5k will eventually stall on heavy traffic with often
> > 'unsupported jumbo' warnings appearing. Disabling ASPM L0s in ath5k fixes
> > these problems.
> > Also card sends a storm of RXORN interrupts even though medium is idle.
> >
> > Reproduced with pcie_aspm=force and by using 'nc < /dev/zero > /dev/null' at
> > both ends (usually stalls within seconds).
> >
> > Unfortunately BIOS enables ASPM on this card by default on these machines
> > This means that, problem shows up (less often) without pcie_aspm=force too.
> > Therefore to benefit from this fix you need to _enable_ CONFIG_PCIEASPM
> >
> >
> > All credit for this patch goes to Jussi Kivilinna <jussi.kivilinna(a)mbnet.fi>
> > for finding and fixing this bug.
> >
> > Based on patch that is
> > From: Jussi Kivilinna <jussi.kivilinna(a)mbnet.fi>
> >
> >
> > Signed-off-by: Maxim Levitsky <maximlevitsky(a)gmail.com>
> > Acked-by: Bob Copeland <me(a)bobcopeland.com>
>
> Acked-by: Luis R. Rodriguez <lrodriguez(a)atheros.com>

So NACK now, to really fix this correctly its best to instead disable L0s but
to force enable also L1 if the device is a PCI Express device. All Atheros legacy
devices (ath5k) should work correctly with L1. Kernels with CONFIG_PCIEASPM
would end up disabling even L1 for pre PCI 1.1 devices.

Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/