Subject: [PATCH] rtl8187b: do not do per packet TX AGC

The code for rtl8187 does not do per packet TX AGC. Resetting the per
packet TX AGC for rtl8187b appears to increase its overall TX power.
This allows the device to associate and a connection be established
using APs a little further away.

Tested-by: Thadeu Lima de Souza Cascardo <[email protected]>
Signed-off-by: Thadeu Lima de Souza Cascardo <[email protected]>
Cc: [email protected]
Cc: Larry Finger <[email protected]>
Cc: Rogerio Luz Coelho <[email protected]>
Cc: Herton Ronaldo Krzesinski <[email protected]>
Cc: Hin-Tak Leung <[email protected]>
---
drivers/net/wireless/rtl818x/rtl8187_dev.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rtl818x/rtl8187_dev.c b/drivers/net/wireless/rtl818x/rtl8187_dev.c
index 38fa824..6e26149 100644
--- a/drivers/net/wireless/rtl818x/rtl8187_dev.c
+++ b/drivers/net/wireless/rtl818x/rtl8187_dev.c
@@ -775,10 +775,6 @@ static int rtl8187b_init_hw(struct ieee80211_hw *dev)
reg = rtl818x_ioread8(priv, &priv->map->CW_CONF);
reg |= RTL818X_CW_CONF_PERPACKET_RETRY_SHIFT;
rtl818x_iowrite8(priv, &priv->map->CW_CONF, reg);
- reg = rtl818x_ioread8(priv, &priv->map->TX_AGC_CTL);
- reg |= RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT |
- RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT;
- rtl818x_iowrite8(priv, &priv->map->TX_AGC_CTL, reg);

rtl818x_iowrite16_idx(priv, (__le16 *)0xFFE0, 0x0FFF, 1);

@@ -929,6 +925,12 @@ static int rtl8187_start(struct ieee80211_hw *dev)
priv->rx_conf = reg;
rtl818x_iowrite32(priv, &priv->map->RX_CONF, reg);

+ reg = rtl818x_ioread8(priv, &priv->map->TX_AGC_CTL);
+ reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT;
+ reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT;
+ reg &= ~RTL818X_TX_AGC_CTL_FEEDBACK_ANT;
+ rtl818x_iowrite8(priv, &priv->map->TX_AGC_CTL, reg);
+
rtl818x_iowrite32(priv, &priv->map->TX_CONF,
RTL818X_TX_CONF_HW_SEQNUM |
RTL818X_TX_CONF_DISREQQSIZE |
--
1.7.1



Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On Mon, Aug 30, 2010 at 09:48:57AM -0500, Larry Finger wrote:
> On 08/30/2010 09:12 AM, John W. Linville wrote:
> > On Sat, Aug 28, 2010 at 04:32:48PM -0500, Larry Finger wrote:
> >> On 08/28/2010 12:54 AM, Thadeu Lima de Souza Cascardo wrote:
> >>> The code for rtl8187 does not do per packet TX AGC. Resetting the per
> >>> packet TX AGC for rtl8187b appears to increase its overall TX power.
> >>> This allows the device to associate and a connection be established
> >>> using APs a little further away.
> >>>
> >>> Tested-by: Thadeu Lima de Souza Cascardo <[email protected]>
> >>> Signed-off-by: Thadeu Lima de Souza Cascardo <[email protected]>
> >>> Cc: [email protected]
> >>> Cc: Larry Finger <[email protected]>
> >>> Cc: Rogerio Luz Coelho <[email protected]>
> >>> Cc: Herton Ronaldo Krzesinski <[email protected]>
> >>> Cc: Hin-Tak Leung <[email protected]>
> >>> ---
> >>
> >> I'm a little confused here. The subject says not to set the per packet TX AGC,
> >> while the submission text says that setting it helps. I don't have access to any
> >> documents that describe this register, but I expect that clearing
> >> RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT would disable that feature. As a result
> >> the code change seems to match the subject.
> >>
> >> I have not yet tested this code, but I do note that all the Realtek drivers for
> >> the RTL8187B set this bit.
> >>
> >> Until testing, I withhold judgment on the actual code change, but NACK for the
> >> inconsistency.
> >
> > Perhaps the word "clearing" rather than "resetting" would be less confusing?
>

Thanks, John, for the clarification. Clearing is much better, really.

> I see the source of my confusion. I am in the habit of using lower case to
> indicate the driver and upper case for the device. I would rewrite the commit
> code to say "The code for the RTL8187L does not do per packet TX AGC. Clearing
> the per packet TX AGC for RTL8187B ..."
>

I will use your rewriting. Thanks.

> I have now tested the patch. For my RTL8187B device, I get the following:
>
> Distance from AP Indicated rate Signal Measured TX rate
> 2m 18 Mb/s -17 dBm 10 Mb/s
> 10m(original) 11 Mb/s -48 dBm 2.6 Mb/s
> 10m(with patch) 11 Mb/s -43 dBm 4.1 Mb/s
>
> The measured TX rates were obtained using tcpperf. At a distance of 2 m, the
> results were the same with and without the patch. At 10 m, the patch went
> through 2 exterior walls with drywall, insulation, and stucco. I'm not sure that
> the increase from 2.6 to 4.1 Mb/s is reproducible. About all I can say is that
> the change did not hurt the performance of the device, which is already pretty bad.
>
> If the OP can post numbers documenting the improvement, I have no objection to
> this patch, even though every Realtek driver that I have seen turns "per packet
> TX AGC" on for the RTL8187B and turns it off for the RTL8187L.
>

Hum. Did you take a look at the version I've pointed out in the last
message, rtl8187B_linux_26.1056.1112.2009.release? It does not clear
PERPACKET_GAIN in the same function that it does for non-RTL8187B
devices. But it does in what should be the code for RTL8225Z2, in the
function r8180_rtl8225z2.c:InitializeExtraRegsOn8185.

I did not measure the distances exactly here, while doing my tests. But
I could move the AP further away by about 5 or 6 meters. The AP
indicated a -84dB signal while receiving injected packages. It was a
rt73usb device. File transmission through scp was not working that
further away, but I could still get an improvement of about 3 to 4
meters of distance.

I am still going to do some more testing today in a customer's facility.
And I would like very much that Rogério, who has reported some problems
too, did some testing.

> Larry

Best regards,
Cascardo.


Attachments:
(No filename) (3.82 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2010-08-30 14:14:53

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On Sat, Aug 28, 2010 at 04:32:48PM -0500, Larry Finger wrote:
> On 08/28/2010 12:54 AM, Thadeu Lima de Souza Cascardo wrote:
> > The code for rtl8187 does not do per packet TX AGC. Resetting the per
> > packet TX AGC for rtl8187b appears to increase its overall TX power.
> > This allows the device to associate and a connection be established
> > using APs a little further away.
> >
> > Tested-by: Thadeu Lima de Souza Cascardo <[email protected]>
> > Signed-off-by: Thadeu Lima de Souza Cascardo <[email protected]>
> > Cc: [email protected]
> > Cc: Larry Finger <[email protected]>
> > Cc: Rogerio Luz Coelho <[email protected]>
> > Cc: Herton Ronaldo Krzesinski <[email protected]>
> > Cc: Hin-Tak Leung <[email protected]>
> > ---
>
> I'm a little confused here. The subject says not to set the per packet TX AGC,
> while the submission text says that setting it helps. I don't have access to any
> documents that describe this register, but I expect that clearing
> RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT would disable that feature. As a result
> the code change seems to match the subject.
>
> I have not yet tested this code, but I do note that all the Realtek drivers for
> the RTL8187B set this bit.
>
> Until testing, I withhold judgment on the actual code change, but NACK for the
> inconsistency.

Perhaps the word "clearing" rather than "resetting" would be less confusing?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On Sat, Aug 28, 2010 at 04:32:48PM -0500, Larry Finger wrote:
> On 08/28/2010 12:54 AM, Thadeu Lima de Souza Cascardo wrote:
> > The code for rtl8187 does not do per packet TX AGC. Resetting the per
> > packet TX AGC for rtl8187b appears to increase its overall TX power.
> > This allows the device to associate and a connection be established
> > using APs a little further away.
> >
> > Tested-by: Thadeu Lima de Souza Cascardo <[email protected]>
> > Signed-off-by: Thadeu Lima de Souza Cascardo <[email protected]>
> > Cc: [email protected]
> > Cc: Larry Finger <[email protected]>
> > Cc: Rogerio Luz Coelho <[email protected]>
> > Cc: Herton Ronaldo Krzesinski <[email protected]>
> > Cc: Hin-Tak Leung <[email protected]>
> > ---
>
> I'm a little confused here. The subject says not to set the per packet TX AGC,
> while the submission text says that setting it helps. I don't have access to any
> documents that describe this register, but I expect that clearing
> RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT would disable that feature. As a result
> the code change seems to match the subject.

Hello, Larry.

I meant rtl8187 versus rtl8187b, since the driver does that distinction.
You may note that in function rtl8187_start, if priv->is_rtl8187b fails,
it will reset RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT, just before
writing to TX_CONF. That's why I've decided to do it too before writing
in TX_CONF in the priv->is_rtl8187b true case.

And I meant resetting and "setting to zero". Perhaps, I may clarify that
too.

>
> I have not yet tested this code, but I do note that all the Realtek drivers for
> the RTL8187B set this bit.
>

Well, rtl8187B_linux_26.1056.1112.2009.release does reset it to zero
too. And that's what has hit me into trying it.

> Until testing, I withhold judgment on the actual code change, but NACK for the
> inconsistency.
>

I will send another version, clarifying the commit message. Thanks for
the comments.

Regards,
Cascardo.

> Larry
>


Attachments:
(No filename) (1.98 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2010-08-30 14:47:46

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On 08/30/2010 09:12 AM, John W. Linville wrote:
> On Sat, Aug 28, 2010 at 04:32:48PM -0500, Larry Finger wrote:
>> On 08/28/2010 12:54 AM, Thadeu Lima de Souza Cascardo wrote:
>>> The code for rtl8187 does not do per packet TX AGC. Resetting the per
>>> packet TX AGC for rtl8187b appears to increase its overall TX power.
>>> This allows the device to associate and a connection be established
>>> using APs a little further away.
>>>
>>> Tested-by: Thadeu Lima de Souza Cascardo <[email protected]>
>>> Signed-off-by: Thadeu Lima de Souza Cascardo <[email protected]>
>>> Cc: [email protected]
>>> Cc: Larry Finger <[email protected]>
>>> Cc: Rogerio Luz Coelho <[email protected]>
>>> Cc: Herton Ronaldo Krzesinski <[email protected]>
>>> Cc: Hin-Tak Leung <[email protected]>
>>> ---
>>
>> I'm a little confused here. The subject says not to set the per packet TX AGC,
>> while the submission text says that setting it helps. I don't have access to any
>> documents that describe this register, but I expect that clearing
>> RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT would disable that feature. As a result
>> the code change seems to match the subject.
>>
>> I have not yet tested this code, but I do note that all the Realtek drivers for
>> the RTL8187B set this bit.
>>
>> Until testing, I withhold judgment on the actual code change, but NACK for the
>> inconsistency.
>
> Perhaps the word "clearing" rather than "resetting" would be less confusing?

I see the source of my confusion. I am in the habit of using lower case to
indicate the driver and upper case for the device. I would rewrite the commit
code to say "The code for the RTL8187L does not do per packet TX AGC. Clearing
the per packet TX AGC for RTL8187B ..."

I have now tested the patch. For my RTL8187B device, I get the following:

Distance from AP Indicated rate Signal Measured TX rate
2m 18 Mb/s -17 dBm 10 Mb/s
10m(original) 11 Mb/s -48 dBm 2.6 Mb/s
10m(with patch) 11 Mb/s -43 dBm 4.1 Mb/s

The measured TX rates were obtained using tcpperf. At a distance of 2 m, the
results were the same with and without the patch. At 10 m, the patch went
through 2 exterior walls with drywall, insulation, and stucco. I'm not sure that
the increase from 2.6 to 4.1 Mb/s is reproducible. About all I can say is that
the change did not hurt the performance of the device, which is already pretty bad.

If the OP can post numbers documenting the improvement, I have no objection to
this patch, even though every Realtek driver that I have seen turns "per packet
TX AGC" on for the RTL8187B and turns it off for the RTL8187L.

Larry

2010-08-28 21:31:40

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On 08/28/2010 12:54 AM, Thadeu Lima de Souza Cascardo wrote:
> The code for rtl8187 does not do per packet TX AGC. Resetting the per
> packet TX AGC for rtl8187b appears to increase its overall TX power.
> This allows the device to associate and a connection be established
> using APs a little further away.
>
> Tested-by: Thadeu Lima de Souza Cascardo <[email protected]>
> Signed-off-by: Thadeu Lima de Souza Cascardo <[email protected]>
> Cc: [email protected]
> Cc: Larry Finger <[email protected]>
> Cc: Rogerio Luz Coelho <[email protected]>
> Cc: Herton Ronaldo Krzesinski <[email protected]>
> Cc: Hin-Tak Leung <[email protected]>
> ---

I'm a little confused here. The subject says not to set the per packet TX AGC,
while the submission text says that setting it helps. I don't have access to any
documents that describe this register, but I expect that clearing
RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT would disable that feature. As a result
the code change seems to match the subject.

I have not yet tested this code, but I do note that all the Realtek drivers for
the RTL8187B set this bit.

Until testing, I withhold judgment on the actual code change, but NACK for the
inconsistency.

Larry


2010-09-16 18:44:47

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On Mon, Aug 30, 2010 at 12:12:04PM -0300, Thadeu Lima de Souza Cascardo wrote:

> > Until testing, I withhold judgment on the actual code change, but NACK for the
> > inconsistency.
> >
>
> I will send another version, clarifying the commit message. Thanks for
> the comments.

Should I still be expecting to see this?

--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-10-30 00:00:07

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On 10/29/2010 05:31 PM, seno wrote:
> I noticed that the connection is fast for a short time after booting (10 mbit @
> 10 meters), but a few seconds after the download on speedtest.net started
> (meaning the network gets under load), speed drops to 1 mbit and never recovers
> to faster speed again; somehow like an "unrecoverable fallback mechanism"...?

Is your device overheating?

Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On Tue, Oct 26, 2010 at 01:25:03PM -0500, Larry Finger wrote:
> On 10/26/2010 01:03 PM, seno wrote:
> > I tried that patch on compat-wireless-2010-10-25 (Kernel 2.6.33.7).
> >
> > Performance improves minimal, but thousands of errors occur in iwconfig output.
> > (Tx excessive retries, Invalid misc, Missed beacon).
> >
> > The router also shows lots of errors
> > (for Received (RX) / Transmitted (TX) packets)
>
> I no longer remember the details of why this patch was abandoned, but I suspect
> you found the answer.
>

It was because of the bad wording in the commit description. I was
expecting some more testing from my client before posting it again with
the wording fixed. They've been testing it and there is enough
improvement they require the patch. I don't get the thousands of errors
in iwconfig output. I can try to reproduce it here if I get more
details.

Thanks,
Cascardo.

> Larry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


Attachments:
(No filename) (1.08 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2010-10-29 22:31:31

by seno

[permalink] [raw]
Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

Thadeu Lima de Souza Cascardo <cascardo@...> writes:

> Perhaps, the problem is related to using compat-wireless. Have you tried
> using the module without the patch, using this same compat-wireless
> version in the same linux version you are using?

Cascardo, you were right :-) Your patch did not cause the problem.
I get those errors also with an unpatched compat-wireless-2010-10-25 driver.

With a patched compat-wireless-2.6.33.6 there occur no errors.

The router reports a better signal quality compared to the unpatched module, but
I can not really recognize a better throughput - usually still max 1 mbit @ 10
meters.

I noticed that the connection is fast for a short time after booting (10 mbit @
10 meters), but a few seconds after the download on speedtest.net started
(meaning the network gets under load), speed drops to 1 mbit and never recovers
to faster speed again; somehow like an "unrecoverable fallback mechanism"...?


Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On Thu, Oct 28, 2010 at 11:21:47PM +0000, seno wrote:
> Herton Ronaldo Krzesinski <herton@...> writes:
>
> > The excessive retries etc. should be another issue. The patch from Thadeu
> > really improves the driver.
>
> Well, I tried once more today, maybe I'm doing something wrong?
>
> - I run "./scripts/driver-select rtl818x" in compat-wireless-2010-10-28
> - then "patch -p1 rtl8187_dev.c rtl8187b-do-not-do-per-packet-TX-AGC.patch"
> - then "make"
> - then "sudo make install"
> - then reboot
>
> "dmesg |grep compat" gives "Compat-wireless backport release: compat-
> wireless-2010-10-22-5-g69fdc09" so the new module is used.
>
> On speedtest.net, max speed is ~ 1mbit (on 10 mbit cable internet @10 meters
> distance)
> The router reports an improved "Signal Quality", but iwconfig now shows:
>
> "Rx invalid nwid:0 Rx invalid crypt:0 Rx invalid frag:0"
> "Tx excessive retries:2589 Invalid misc:17686 Missed beacon:0"
>
> Herton: If I make something wrong, can you please post your rtl8187.ko.gz
> (Mandriva 2010.1 / 2.6.33.7-desktop-2mnb x86_64)?
>
> Thank you.

Hello, seno.

Perhaps, the problem is related to using compat-wireless. Have you tried
using the module without the patch, using this same compat-wireless
version in the same linux version you are using?

Regards,
Cascardo.


Attachments:
(No filename) (1.28 kB)
signature.asc (836.00 B)
Digital signature
Download all attachments

2010-10-30 02:12:32

by seno

[permalink] [raw]
Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

Larry Finger <Larry.Finger@...> writes:

> Is your device overheating?

I don't think so.

I reduced Tx-Power to 20 dBm (wireless regulatory domain DE).
That doesn't make any difference to 27 dBm (CA) or even 30 dBm (BO).
(Beside that: it performs fine in w7)

In contrast to the unchanged measured performance, web browsing feels 'snappier'
with the patched driver (websites load faster although speed peek never top > 140
kByte/s)


2010-10-26 18:24:49

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On 10/26/2010 01:03 PM, seno wrote:
> I tried that patch on compat-wireless-2010-10-25 (Kernel 2.6.33.7).
>
> Performance improves minimal, but thousands of errors occur in iwconfig output.
> (Tx excessive retries, Invalid misc, Missed beacon).
>
> The router also shows lots of errors
> (for Received (RX) / Transmitted (TX) packets)

I no longer remember the details of why this patch was abandoned, but I suspect
you found the answer.

Larry

2010-10-26 18:03:43

by seno

[permalink] [raw]
Subject: Re:[PATCH] rtl8187b: do not do per packet TX AGC

I tried that patch on compat-wireless-2010-10-25 (Kernel 2.6.33.7).

Performance improves minimal, but thousands of errors occur in iwconfig output.
(Tx excessive retries, Invalid misc, Missed beacon).

The router also shows lots of errors
(for Received (RX) / Transmitted (TX) packets)


Subject: [PATCH] rtl8187b: do not do per packet TX AGC

Clearing the per packet TX AGC for the RTL8187B device appears to
increase its overall TX power. This allows the device to associate and a
connection to be established using APs a little further away.

This is in accordance to what is done for RTL8187L devices and also what
Realtek drivers do.

Tested-by: Thadeu Lima de Souza Cascardo <[email protected]>
Signed-off-by: Thadeu Lima de Souza Cascardo <[email protected]>
Cc: [email protected]
Cc: Larry Finger <[email protected]>
Cc: Rogerio Luz Coelho <[email protected]>
Cc: Herton Ronaldo Krzesinski <[email protected]>
Cc: Hin-Tak Leung <[email protected]>
Cc: seno <[email protected]>
---
drivers/net/wireless/rtl818x/rtl8187_dev.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rtl818x/rtl8187_dev.c b/drivers/net/wireless/rtl818x/rtl8187_dev.c
index 38fa824..6e26149 100644
--- a/drivers/net/wireless/rtl818x/rtl8187_dev.c
+++ b/drivers/net/wireless/rtl818x/rtl8187_dev.c
@@ -775,10 +775,6 @@ static int rtl8187b_init_hw(struct ieee80211_hw *dev)
reg = rtl818x_ioread8(priv, &priv->map->CW_CONF);
reg |= RTL818X_CW_CONF_PERPACKET_RETRY_SHIFT;
rtl818x_iowrite8(priv, &priv->map->CW_CONF, reg);
- reg = rtl818x_ioread8(priv, &priv->map->TX_AGC_CTL);
- reg |= RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT |
- RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT;
- rtl818x_iowrite8(priv, &priv->map->TX_AGC_CTL, reg);

rtl818x_iowrite16_idx(priv, (__le16 *)0xFFE0, 0x0FFF, 1);

@@ -929,6 +925,12 @@ static int rtl8187_start(struct ieee80211_hw *dev)
priv->rx_conf = reg;
rtl818x_iowrite32(priv, &priv->map->RX_CONF, reg);

+ reg = rtl818x_ioread8(priv, &priv->map->TX_AGC_CTL);
+ reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT;
+ reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT;
+ reg &= ~RTL818X_TX_AGC_CTL_FEEDBACK_ANT;
+ rtl818x_iowrite8(priv, &priv->map->TX_AGC_CTL, reg);
+
rtl818x_iowrite32(priv, &priv->map->TX_CONF,
RTL818X_TX_CONF_HW_SEQNUM |
RTL818X_TX_CONF_DISREQQSIZE |
--
1.7.1


2010-10-28 23:21:59

by seno

[permalink] [raw]
Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

Herton Ronaldo Krzesinski <herton@...> writes:

> The excessive retries etc. should be another issue. The patch from Thadeu
> really improves the driver.

Well, I tried once more today, maybe I'm doing something wrong?

- I run "./scripts/driver-select rtl818x" in compat-wireless-2010-10-28
- then "patch -p1 rtl8187_dev.c rtl8187b-do-not-do-per-packet-TX-AGC.patch"
- then "make"
- then "sudo make install"
- then reboot

"dmesg |grep compat" gives "Compat-wireless backport release: compat-
wireless-2010-10-22-5-g69fdc09" so the new module is used.

On speedtest.net, max speed is ~ 1mbit (on 10 mbit cable internet @10 meters
distance)
The router reports an improved "Signal Quality", but iwconfig now shows:

"Rx invalid nwid:0 Rx invalid crypt:0 Rx invalid frag:0"
"Tx excessive retries:2589 Invalid misc:17686 Missed beacon:0"

Herton: If I make something wrong, can you please post your rtl8187.ko.gz
(Mandriva 2010.1 / 2.6.33.7-desktop-2mnb x86_64)?

Thank you.


Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On Tue, 26 Oct 2010 16:53:42 -0200
Thadeu Lima de Souza Cascardo <[email protected]> wrote:
> On Tue, Oct 26, 2010 at 01:25:03PM -0500, Larry Finger wrote:
> > On 10/26/2010 01:03 PM, seno wrote:
> > > I tried that patch on compat-wireless-2010-10-25 (Kernel 2.6.33.7).
> > >
> > > Performance improves minimal, but thousands of errors occur in iwconfig output.
> > > (Tx excessive retries, Invalid misc, Missed beacon).
> > >
> > > The router also shows lots of errors
> > > (for Received (RX) / Transmitted (TX) packets)
> >
> > I no longer remember the details of why this patch was abandoned, but I suspect
> > you found the answer.
> >
>
> It was because of the bad wording in the commit description. I was
> expecting some more testing from my client before posting it again with
> the wording fixed. They've been testing it and there is enough
> improvement they require the patch. I don't get the thousands of errors
> in iwconfig output. I can try to reproduce it here if I get more
> details.

The excessive retries etc. should be another issue. The patch from Thadeu
really improves the driver. In last days I got back a RTL8187B device for
testing, and for me it also improved the range and throughput doing some
scps or with iperf. And looking at newer realtek drivers for 8187B, they
also started to clear the flags. In fact, while doing a printk to see the
contents of the register after chip reset, it's already cleared, but I don't
see any problem to make sure we already have all *TX_AGC_CTL* flags cleared.
Also for 8180 and 8187L they are cleared. Unfortunately we don't have details
(documentation) on what these flags are exactly supposed to do...

But I really think the patch should go in, Thadeu can you post it again with
commit description fixed?

>
> Thanks,
> Cascardo.
>
> > Larry
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html

--
[]'s
Herton

Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On Thu, 28 Oct 2010 20:01:00 -0200
Thadeu Lima de Souza Cascardo <[email protected]> wrote:

> Clearing the per packet TX AGC for the RTL8187B device appears to
> increase its overall TX power. This allows the device to associate and a
> connection to be established using APs a little further away.
>
> This is in accordance to what is done for RTL8187L devices and also what
> Realtek drivers do.
>
> Tested-by: Thadeu Lima de Souza Cascardo <[email protected]>
> Signed-off-by: Thadeu Lima de Souza Cascardo <[email protected]>
> Cc: [email protected]
> Cc: Larry Finger <[email protected]>
> Cc: Rogerio Luz Coelho <[email protected]>
> Cc: Herton Ronaldo Krzesinski <[email protected]>
> Cc: Hin-Tak Leung <[email protected]>
> Cc: seno <[email protected]>

Tested-by: Herton Ronaldo Krzesinski <[email protected]>

> ---
> drivers/net/wireless/rtl818x/rtl8187_dev.c | 10 ++++++----
> 1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/rtl818x/rtl8187_dev.c b/drivers/net/wireless/rtl818x/rtl8187_dev.c
> index 38fa824..6e26149 100644
> --- a/drivers/net/wireless/rtl818x/rtl8187_dev.c
> +++ b/drivers/net/wireless/rtl818x/rtl8187_dev.c
> @@ -775,10 +775,6 @@ static int rtl8187b_init_hw(struct ieee80211_hw *dev)
> reg = rtl818x_ioread8(priv, &priv->map->CW_CONF);
> reg |= RTL818X_CW_CONF_PERPACKET_RETRY_SHIFT;
> rtl818x_iowrite8(priv, &priv->map->CW_CONF, reg);
> - reg = rtl818x_ioread8(priv, &priv->map->TX_AGC_CTL);
> - reg |= RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT |
> - RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT;
> - rtl818x_iowrite8(priv, &priv->map->TX_AGC_CTL, reg);
>
> rtl818x_iowrite16_idx(priv, (__le16 *)0xFFE0, 0x0FFF, 1);
>
> @@ -929,6 +925,12 @@ static int rtl8187_start(struct ieee80211_hw *dev)
> priv->rx_conf = reg;
> rtl818x_iowrite32(priv, &priv->map->RX_CONF, reg);
>
> + reg = rtl818x_ioread8(priv, &priv->map->TX_AGC_CTL);
> + reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_GAIN_SHIFT;
> + reg &= ~RTL818X_TX_AGC_CTL_PERPACKET_ANTSEL_SHIFT;
> + reg &= ~RTL818X_TX_AGC_CTL_FEEDBACK_ANT;
> + rtl818x_iowrite8(priv, &priv->map->TX_AGC_CTL, reg);
> +
> rtl818x_iowrite32(priv, &priv->map->TX_CONF,
> RTL818X_TX_CONF_HW_SEQNUM |
> RTL818X_TX_CONF_DISREQQSIZE |
> --
> 1.7.1
>


--
[]'s
Herton

2010-11-03 03:16:15

by Rogerio Luz Coelho

[permalink] [raw]
Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

Ok I thought I got something wrong when I reviewed the thread I was
reading, this hole thread was in my trash folder (who would figure)

Will post results

Rogerio

2010/11/1 Larry Finger <[email protected]>:
> On 10/31/2010 11:39 PM, Herton Ronaldo Krzesinski wrote:
>> I have seen the throughput issue, but on my tests it wasn't so bad.
>> Usually max I got on testing was 24M while close to the AP, and it goes
>> into lower speeds when going more far away, but takes a good number of
>> meters to go down to 1M.
>>
>> Anyway of course something isn't right. I started reviewing realtek GPL
>> code (their ieee80211 drivers) and code in rtl8187 in the kernel, doing
>> many tests and verifying things, trying to understand and check some
>> register writes etc. (hard with missing doc or other info).
>>
>> I did some cleanups, and discovered two bad things in the code. After
>> the fixes, I can get normal rates (up to 54M close to the AP) without
>> issue. I'll soon post the patch series to be included in
>> wireless-testing, for now just for test I attach them to be easier (hope
>> it isn't stripped by ML, and yes they start on 0003 through 0011 :P),
>> check if with it you get better results.
>>
>> The patches that matter (shown by my tests) are:
>> 0010-rtl8187-remove-uneeded-setting-of-anaparam-write.patch
>> 0011-rtl8187-restore-anaparam-registers-after-reset-with-.patch
>>
>> but they depend on some previous patches in the series (better apply
>> all), and I diffed on a tree with Thadeu's patch applied already. There
>> is more cleanups and checking to do, but I probably will submit this
>> initial series and later come with more ones, as with this initial
>> series the throughput issue should be solved.
>
> Good work. As you noted, patches 3 - 9 did not make any difference in the
> transmit throughput. From a distance of 2 m from the AP, my 8187B yielded up to
> 11.4 Mb/s and an indicated rate of 24 M. Applying patch 10 raised those numbers
> to 17.5 Mb/s and 48 M. Adding patch 11 raised them again to 26.9 Mb/s and 54 M,
> thus getting full throughput for the 8187B and a transmit rate increase of 2.4X.
>
> As expected, these changes had no affect on the RTL8187L. Note: The 8187L gets
> values of 23.0 Mb/s and 54M - there may be a little performance gain to be
> obtained from the driver.
>
> You should submit these patches now, and feel free to add my "Acked-by:" to them.
>
> Larry
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

2010-11-01 16:24:21

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On 10/31/2010 11:39 PM, Herton Ronaldo Krzesinski wrote:
> I have seen the throughput issue, but on my tests it wasn't so bad.
> Usually max I got on testing was 24M while close to the AP, and it goes
> into lower speeds when going more far away, but takes a good number of
> meters to go down to 1M.
>
> Anyway of course something isn't right. I started reviewing realtek GPL
> code (their ieee80211 drivers) and code in rtl8187 in the kernel, doing
> many tests and verifying things, trying to understand and check some
> register writes etc. (hard with missing doc or other info).
>
> I did some cleanups, and discovered two bad things in the code. After
> the fixes, I can get normal rates (up to 54M close to the AP) without
> issue. I'll soon post the patch series to be included in
> wireless-testing, for now just for test I attach them to be easier (hope
> it isn't stripped by ML, and yes they start on 0003 through 0011 :P),
> check if with it you get better results.
>
> The patches that matter (shown by my tests) are:
> 0010-rtl8187-remove-uneeded-setting-of-anaparam-write.patch
> 0011-rtl8187-restore-anaparam-registers-after-reset-with-.patch
>
> but they depend on some previous patches in the series (better apply
> all), and I diffed on a tree with Thadeu's patch applied already. There
> is more cleanups and checking to do, but I probably will submit this
> initial series and later come with more ones, as with this initial
> series the throughput issue should be solved.

Good work. As you noted, patches 3 - 9 did not make any difference in the
transmit throughput. From a distance of 2 m from the AP, my 8187B yielded up to
11.4 Mb/s and an indicated rate of 24 M. Applying patch 10 raised those numbers
to 17.5 Mb/s and 48 M. Adding patch 11 raised them again to 26.9 Mb/s and 54 M,
thus getting full throughput for the 8187B and a transmit rate increase of 2.4X.

As expected, these changes had no affect on the RTL8187L. Note: The 8187L gets
values of 23.0 Mb/s and 54M - there may be a little performance gain to be
obtained from the driver.

You should submit these patches now, and feel free to add my "Acked-by:" to them.

Larry

Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On 29-10-2010 20:31, seno wrote:
> Thadeu Lima de Souza Cascardo<cascardo@...> writes:
>
>> Perhaps, the problem is related to using compat-wireless. Have you tried
>> using the module without the patch, using this same compat-wireless
>> version in the same linux version you are using?
>
> Cascardo, you were right :-) Your patch did not cause the problem.
> I get those errors also with an unpatched compat-wireless-2010-10-25 driver.
>
> With a patched compat-wireless-2.6.33.6 there occur no errors.
>
> The router reports a better signal quality compared to the unpatched module, but
> I can not really recognize a better throughput - usually still max 1 mbit @ 10
> meters.
>
> I noticed that the connection is fast for a short time after booting (10 mbit @
> 10 meters), but a few seconds after the download on speedtest.net started
> (meaning the network gets under load), speed drops to 1 mbit and never recovers
> to faster speed again; somehow like an "unrecoverable fallback mechanism"...?

I have seen the throughput issue, but on my tests it wasn't so bad.
Usually max I got on testing was 24M while close to the AP, and it goes
into lower speeds when going more far away, but takes a good number of
meters to go down to 1M.

Anyway of course something isn't right. I started reviewing realtek GPL
code (their ieee80211 drivers) and code in rtl8187 in the kernel, doing
many tests and verifying things, trying to understand and check some
register writes etc. (hard with missing doc or other info).

I did some cleanups, and discovered two bad things in the code. After
the fixes, I can get normal rates (up to 54M close to the AP) without
issue. I'll soon post the patch series to be included in
wireless-testing, for now just for test I attach them to be easier (hope
it isn't stripped by ML, and yes they start on 0003 through 0011 :P),
check if with it you get better results.

The patches that matter (shown by my tests) are:
0010-rtl8187-remove-uneeded-setting-of-anaparam-write.patch
0011-rtl8187-restore-anaparam-registers-after-reset-with-.patch

but they depend on some previous patches in the series (better apply
all), and I diffed on a tree with Thadeu's patch applied already. There
is more cleanups and checking to do, but I probably will submit this
initial series and later come with more ones, as with this initial
series the throughput issue should be solved.

>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
[]'s
Herton


Attachments:
0003-rtl8187-remove-redundant-initialization-of-ARFR.patch (2.13 kB)
0004-rtl8187-remove-setting-of-beacon-atim-registers-from.patch (1.20 kB)
0005-rtl8187-fix-wrong-register-initialization-in-8187B.patch (2.33 kB)
0006-rtl8187-avoid-redundant-write-to-register-FF72.patch (1.66 kB)
0007-rtl8187-move-pll-reset-at-start-out-of-ANAPARAM-writ.patch (2.07 kB)
0008-rtl8187-don-t-set-RTL818X_CONFIG3_GNT_SELECT-flag-on.patch (1.44 kB)
0009-rtl8187-consolidate-anaparam-on-off-write-sequences.patch (6.44 kB)
0010-rtl8187-remove-uneeded-setting-of-anaparam-write.patch (1.55 kB)
0011-rtl8187-restore-anaparam-registers-after-reset-with-.patch (1.26 kB)
Download all attachments

2010-11-04 14:23:10

by Hin-Tak Leung

[permalink] [raw]
Subject: Re: [PATCH] rtl8187b: do not do per packet TX AGC

On Mon, Nov 1, 2010 at 4:24 PM, Larry Finger <[email protected]> wrote:
> On 10/31/2010 11:39 PM, Herton Ronaldo Krzesinski wrote:
>> I have seen the throughput issue, but on my tests it wasn't so bad.
>> Usually max I got on testing was 24M while close to the AP, and it goes
>> into lower speeds when going more far away, but takes a good number of
>> meters to go down to 1M.
>>
>> Anyway of course something isn't right. I started reviewing realtek GPL
>> code (their ieee80211 drivers) and code in rtl8187 in the kernel, doing
>> many tests and verifying things, trying to understand and check some
>> register writes etc. (hard with missing doc or other info).
>>
>> I did some cleanups, and discovered two bad things in the code. After
>> the fixes, I can get normal rates (up to 54M close to the AP) without
>> issue. I'll soon post the patch series to be included in
>> wireless-testing, for now just for test I attach them to be easier (hope
>> it isn't stripped by ML, and yes they start on 0003 through 0011 :P),
>> check if with it you get better results.
>>
>> The patches that matter (shown by my tests) are:
>> 0010-rtl8187-remove-uneeded-setting-of-anaparam-write.patch
>> 0011-rtl8187-restore-anaparam-registers-after-reset-with-.patch
>>
>> but they depend on some previous patches in the series (better apply
>> all), and I diffed on a tree with Thadeu's patch applied already. There
>> is more cleanups and checking to do, but I probably will submit this
>> initial series and later come with more ones, as with this initial
>> series the throughput issue should be solved.
>
> Good work. As you noted, patches 3 - 9 did not make any difference in the
> transmit throughput. From a distance of 2 m from the AP, my 8187B yielded up to
> 11.4 Mb/s and an indicated rate of 24 M. Applying patch 10 raised those numbers
> to 17.5 Mb/s and 48 M. Adding patch 11 raised them again to 26.9 Mb/s and 54 M,
> thus getting full throughput for the 8187B and a transmit rate increase of 2.4X.
>
> As expected, these changes had no affect on the RTL8187L. Note: The 8187L gets
> values of 23.0 Mb/s and 54M - there may be a little performance gain to be
> obtained from the driver.
>
> You should submit these patches now, and feel free to add my "Acked-by:" to them.
>
> Larry
>

Acked-by: Hin-Tak Leung <[email protected]>

Just to say I am following the thread from time to time. My AP machine
died (almost a year ago now...) so just haven't been using wireless
much lately.

HT