| Message ID | 20180214230216.2001-1-kevans@FreeBSD.org |
|---|---|
| Headers | show |
| Series | Add fix for Pine64 gigabit throughput issues | expand |
Hi, thanks for picking this up! (CC:ing Icenowy, who was engaged in a Linux fix for this issue last year [2][3]. It's Chinese New Year though, so not sure how quickly she will answer). On 14/02/18 23:02, kevans@FreeBSD.org wrote: > The Pine64 has a known issue on gigabit links (see [1]); some boards suffer > significant packet loss on Gigabit links. Do you have a faulty board at hand? What is the actual effect in U-Boot? IIRC the bug "just" causes a slower connection in Gigabit mode, I am not sure we care so much in U-Boot? Or is it actually packages dropped, which is much more annoying without TCP covering up for this? > This patch sets the magical bits > in CONFREG on the RTL8211E PHY to turn off the internal delay and do some other > undocumented stuff. So if I remember the discussion correctly, this workaround affects the performance of the "good" boards. Have you checked this? There was a discussion last year [2][3] about how to fix this in Linux, which definitely involved some DT property (ideally in the PHY node). This would allow people to turn this on and off depending on their particular board. I am not sure this discussion lead anywhere, though, it might be a good idea to warm this up again. Cheers, Andre. [2] https://marc.info/?l=devicetree&m=149281711105621 [3] https://marc.info/?l=linux-netdev&m=150337466923103 > [1] https://forum.pine64.org/showthread.php?tid=835&pid=19704#pid19704 > > Kyle Evans (2): > net: phy: Add PHY_RTL8211E_PINE64_GIGABIT_FIX for realtek phys > Configs: Use the newly added PHY_RTL8211E_PINE64_GIGABIT_FIX > > configs/pine64_plus_defconfig | 2 ++ > drivers/net/phy/Kconfig | 10 ++++++++++ > drivers/net/phy/realtek.c | 34 ++++++++++++++++++++++++++++++++++ > 3 files changed, 46 insertions(+) >
On Thu, Feb 15, 2018 at 3:32 PM, André Przywara <andre.przywara@arm.com> wrote: > Hi, > > thanks for picking this up! > > (CC:ing Icenowy, who was engaged in a Linux fix for this issue last year > [2][3]. It's Chinese New Year though, so not sure how quickly she will > answer). > > On 14/02/18 23:02, kevans@FreeBSD.org wrote: >> The Pine64 has a known issue on gigabit links (see [1]); some boards suffer >> significant packet loss on Gigabit links. > > Do you have a faulty board at hand? What is the actual effect in U-Boot? > IIRC the bug "just" causes a slower connection in Gigabit mode, I am not > sure we care so much in U-Boot? Or is it actually packages dropped, > which is much more annoying without TCP covering up for this? I do. =) My board in particular sees 60-70% packet loss without this, making netboot incredibly unreliable. > >> This patch sets the magical bits >> in CONFREG on the RTL8211E PHY to turn off the internal delay and do some other >> undocumented stuff. > > So if I remember the discussion correctly, this workaround affects the > performance of the "good" boards. Have you checked this? > There was a discussion last year [2][3] about how to fix this in Linux, > which definitely involved some DT property (ideally in the PHY node). > This would allow people to turn this on and off depending on their > particular board. > I am not sure this discussion lead anywhere, though, it might be a good > idea to warm this up again. > Unfortunately, I do not have a good board to test with, so I've no idea what it ends up doing to performance. for them. I think rgmii-txid is actually not right for this. As an aside, I've pulled this value from [1]. The value they ended up going with (as seen in this patch and in [1]) turns off both TXID and RXID (IIRC, from the realtek documentation for this PHY), leaving us with just plain ol' "rgmii" with no internal delay at all. [1] also applies it unconditionally for pine64. [1] https://github.com/longsleep/linux-pine64/commit/ffe3ca5be7682bbeb0fdadede29acd4a3c888015
On Thu, Feb 15, 2018 at 9:32 PM, André Przywara <andre.przywara@arm.com> wrote: > Hi, > > thanks for picking this up! > > (CC:ing Icenowy, who was engaged in a Linux fix for this issue last year > [2][3]. It's Chinese New Year though, so not sure how quickly she will > answer). > > On 14/02/18 23:02, kevans@FreeBSD.org wrote: >> The Pine64 has a known issue on gigabit links (see [1]); some boards suffer >> significant packet loss on Gigabit links. > > Do you have a faulty board at hand? What is the actual effect in U-Boot? > IIRC the bug "just" causes a slower connection in Gigabit mode, I am not > sure we care so much in U-Boot? Or is it actually packages dropped, > which is much more annoying without TCP covering up for this? > >> This patch sets the magical bits >> in CONFREG on the RTL8211E PHY to turn off the internal delay and do some other >> undocumented stuff. > > So if I remember the discussion correctly, this workaround affects the > performance of the "good" boards. Have you checked this? > There was a discussion last year [2][3] about how to fix this in Linux, > which definitely involved some DT property (ideally in the PHY node). > This would allow people to turn this on and off depending on their > particular board. So I have a "bad" board, the fix upstream took it from "basically not working at all" to vaguely but not with great speed be able to transfer data. I've just tested this patch series on that board and was getting around 7MBps on Linux too. So presumably what ever bits that are being tweaked by this patch in u-boot aren't reset when the linux driver loads and it improved performance on Linux for me too. From a netboot (untested) I would think this would be useful too. Either way for the series: Tested-by: Peter Robinson <pbrobinson@gmail.com> > [2] https://marc.info/?l=devicetree&m=149281711105621 > [3] https://marc.info/?l=linux-netdev&m=150337466923103 > >> [1] https://forum.pine64.org/showthread.php?tid=835&pid=19704#pid19704 >> >> Kyle Evans (2): >> net: phy: Add PHY_RTL8211E_PINE64_GIGABIT_FIX for realtek phys >> Configs: Use the newly added PHY_RTL8211E_PINE64_GIGABIT_FIX >> >> configs/pine64_plus_defconfig | 2 ++ >> drivers/net/phy/Kconfig | 10 ++++++++++ >> drivers/net/phy/realtek.c | 34 ++++++++++++++++++++++++++++++++++ >> 3 files changed, 46 insertions(+) >> > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > https://lists.denx.de/listinfo/u-boot
On Wed, Feb 14, 2018 at 5:02 PM, <kevans@freebsd.org> wrote: > The Pine64 has a known issue on gigabit links (see [1]); some boards suffer > significant packet loss on Gigabit links. This patch sets the magical bits > in CONFREG on the RTL8211E PHY to turn off the internal delay and do some other > undocumented stuff. > > [1] https://forum.pine64.org/showthread.php?tid=835&pid=19704#pid19704 > > Kyle Evans (2): > net: phy: Add PHY_RTL8211E_PINE64_GIGABIT_FIX for realtek phys > Configs: Use the newly added PHY_RTL8211E_PINE64_GIGABIT_FIX > > configs/pine64_plus_defconfig | 2 ++ > drivers/net/phy/Kconfig | 10 ++++++++++ > drivers/net/phy/realtek.c | 34 ++++++++++++++++++++++++++++++++++ > 3 files changed, 46 insertions(+) > Hi, I'd like to dredge this one up again, since it's been a couple months and I still run this patch locally. =) It does hurt measured performance, but for the broken set of boards this is absolutely necessary to function on a Gigabit link and it doesn't look like this broken set is really a minority given the number of complaints I've seen. Thanks, Kyle Evans
On Wed, Apr 18, 2018 at 2:16 PM, Kyle Evans <kevans@freebsd.org> wrote: > On Wed, Feb 14, 2018 at 5:02 PM, <kevans@freebsd.org> wrote: >> The Pine64 has a known issue on gigabit links (see [1]); some boards suffer >> significant packet loss on Gigabit links. This patch sets the magical bits >> in CONFREG on the RTL8211E PHY to turn off the internal delay and do some other >> undocumented stuff. >> >> [1] https://forum.pine64.org/showthread.php?tid=835&pid=19704#pid19704 >> >> Kyle Evans (2): >> net: phy: Add PHY_RTL8211E_PINE64_GIGABIT_FIX for realtek phys >> Configs: Use the newly added PHY_RTL8211E_PINE64_GIGABIT_FIX >> >> configs/pine64_plus_defconfig | 2 ++ >> drivers/net/phy/Kconfig | 10 ++++++++++ >> drivers/net/phy/realtek.c | 34 ++++++++++++++++++++++++++++++++++ >> 3 files changed, 46 insertions(+) >> > > Hi, > > I'd like to dredge this one up again, since it's been a couple months > and I still run this patch locally. =) > > It does hurt measured performance, but for the broken set of boards > this is absolutely necessary to function on a Gigabit link and it > doesn't look like this broken set is really a minority given the > number of complaints I've seen. I see it upstream in master: http://git.denx.de/?p=u-boot.git;a=commit;h=dfa1a74045c930ec3935f748b0969f9d76352e13 http://git.denx.de/?p=u-boot.git;a=commit;h=66526e70381dbaad58533cfbd7bce07c668205c6 What else is needed?
On Wed, Apr 18, 2018 at 8:55 AM, Peter Robinson <pbrobinson@gmail.com> wrote: > On Wed, Apr 18, 2018 at 2:16 PM, Kyle Evans <kevans@freebsd.org> wrote: >> On Wed, Feb 14, 2018 at 5:02 PM, <kevans@freebsd.org> wrote: >>> The Pine64 has a known issue on gigabit links (see [1]); some boards suffer >>> significant packet loss on Gigabit links. This patch sets the magical bits >>> in CONFREG on the RTL8211E PHY to turn off the internal delay and do some other >>> undocumented stuff. >>> >>> [1] https://forum.pine64.org/showthread.php?tid=835&pid=19704#pid19704 >>> >>> Kyle Evans (2): >>> net: phy: Add PHY_RTL8211E_PINE64_GIGABIT_FIX for realtek phys >>> Configs: Use the newly added PHY_RTL8211E_PINE64_GIGABIT_FIX >>> >>> configs/pine64_plus_defconfig | 2 ++ >>> drivers/net/phy/Kconfig | 10 ++++++++++ >>> drivers/net/phy/realtek.c | 34 ++++++++++++++++++++++++++++++++++ >>> 3 files changed, 46 insertions(+) >>> >> >> Hi, >> >> I'd like to dredge this one up again, since it's been a couple months >> and I still run this patch locally. =) >> >> It does hurt measured performance, but for the broken set of boards >> this is absolutely necessary to function on a Gigabit link and it >> doesn't look like this broken set is really a minority given the >> number of complaints I've seen. > > I see it upstream in master: > http://git.denx.de/?p=u-boot.git;a=commit;h=dfa1a74045c930ec3935f748b0969f9d76352e13 > http://git.denx.de/?p=u-boot.git;a=commit;h=66526e70381dbaad58533cfbd7bce07c668205c6 > > What else is needed? Ah, sorry about this, then. =) Clearly I need to grow a new pair of eyeballs and stop browsing out-of-date source trees.
On Wed, Apr 18, 2018 at 2:58 PM, Kyle Evans <kevans@freebsd.org> wrote: > On Wed, Apr 18, 2018 at 8:55 AM, Peter Robinson <pbrobinson@gmail.com> wrote: >> On Wed, Apr 18, 2018 at 2:16 PM, Kyle Evans <kevans@freebsd.org> wrote: >>> On Wed, Feb 14, 2018 at 5:02 PM, <kevans@freebsd.org> wrote: >>>> The Pine64 has a known issue on gigabit links (see [1]); some boards suffer >>>> significant packet loss on Gigabit links. This patch sets the magical bits >>>> in CONFREG on the RTL8211E PHY to turn off the internal delay and do some other >>>> undocumented stuff. >>>> >>>> [1] https://forum.pine64.org/showthread.php?tid=835&pid=19704#pid19704 >>>> >>>> Kyle Evans (2): >>>> net: phy: Add PHY_RTL8211E_PINE64_GIGABIT_FIX for realtek phys >>>> Configs: Use the newly added PHY_RTL8211E_PINE64_GIGABIT_FIX >>>> >>>> configs/pine64_plus_defconfig | 2 ++ >>>> drivers/net/phy/Kconfig | 10 ++++++++++ >>>> drivers/net/phy/realtek.c | 34 ++++++++++++++++++++++++++++++++++ >>>> 3 files changed, 46 insertions(+) >>>> >>> >>> Hi, >>> >>> I'd like to dredge this one up again, since it's been a couple months >>> and I still run this patch locally. =) >>> >>> It does hurt measured performance, but for the broken set of boards >>> this is absolutely necessary to function on a Gigabit link and it >>> doesn't look like this broken set is really a minority given the >>> number of complaints I've seen. >> >> I see it upstream in master: >> http://git.denx.de/?p=u-boot.git;a=commit;h=dfa1a74045c930ec3935f748b0969f9d76352e13 >> http://git.denx.de/?p=u-boot.git;a=commit;h=66526e70381dbaad58533cfbd7bce07c668205c6 >> >> What else is needed? > > Ah, sorry about this, then. =) Clearly I need to grow a new pair of > eyeballs and stop browsing out-of-date source trees. was in at least 2018.05-rc2