Bug 29512

Summary: [PATCH]radeon rejects EDID with RV350 when hw_i2c=1
Product: Drivers Reporter: Chris Rankin (rankincj)
Component: Video(DRI - non Intel)Assignee: drivers_video-dri
Status: RESOLVED CODE_FIX    
Severity: normal CC: alan, alexdeucher
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 2.6.37.1 Subsystem:
Regression: No Bisected commit-id:
Attachments: dmesg output from RV350 on single transient success with hw_i2c=1
dmesg output showing I2C errors with rv100 and DVI monitor
Fixes hw_i2c for RV350 and RV280 cards.

Description Chris Rankin 2011-02-20 02:43:33 UTC
Trying to boot 2.6.37.1 with KMS, hw_i2c=1 and RV350 card fails to initialise monitor. This is what dmesg reports:

[drm] Initialized drm 1.1.0 20060810
agpgart-intel 0000:00:00.0: Intel E7505 Chipset
agpgart-intel 0000:00:00.0: AGP aperture is 256M @ 0xc0000000
[drm] radeon defaulting to kernel modesetting.
[drm] radeon kernel modesetting enabled.
radeon 0000:01:00.0: PCI INT A -> GSI 16 (level, low) -> IRQ 16
[drm] initializing kernel modesetting (RV350 0x1002:0x4153).
[drm] register mmio base: 0xFF8F0000
[drm] register mmio size: 65536
agpgart-intel 0000:00:00.0: AGP 3.0 bridge
agpgart-intel 0000:00:00.0: putting AGP V3 device into 8x mode
radeon 0000:01:00.0: putting AGP V3 device into 8x mode
radeon 0000:01:00.0: GTT: 256M 0xC0000000 - 0xCFFFFFFF
[drm] Generation 2 PCI interface, using max accessible memory
radeon 0000:01:00.0: VRAM: 256M 0x00000000E0000000 - 0x00000000EFFFFFFF (256M used)
[drm] radeon: irq initialized.
[drm] Detected VRAM RAM=256M, BAR=256M
[drm] RAM width 128bits DDR
[TTM] Zone  kernel: Available graphics memory: 439026 kiB.
[TTM] Zone highmem: Available graphics memory: 1032672 kiB.
[TTM] Initializing pool allocator.
[drm] radeon: 256M of VRAM memory ready
[drm] radeon: 256M of GTT memory ready.
[drm] radeon: 1 quad pipes, 1 Z pipes initialized.
radeon 0000:01:00.0: WB disabled
[drm] Loading R300 Microcode
[drm] radeon: ring at 0x00000000C0001000
[drm] ring test succeeded in 1 usecs
[drm] radeon: ib pool ready.
[drm] ib test succeeded in 0 usecs
[drm] Radeon Display Connectors
[drm] Connector 0:
[drm]   VGA
[drm]   DDC: 0x60 0x60 0x60 0x60 0x60 0x60 0x60 0x60
[drm]   Encoders:
[drm]     CRT1: INTERNAL_DAC1
[drm] Connector 1:
[drm]   DVI-I
[drm]   HPD1
[drm]   DDC: 0x64 0x64 0x64 0x64 0x64 0x64 0x64 0x64
[drm]   Encoders:
[drm]     CRT2: INTERNAL_DAC2
[drm]     DFP1: INTERNAL_TMDS1
[drm] Connector 2:
[drm]   S-video
[drm]   Encoders:
[drm]     TV1: INTERNAL_DAC2
[drm:drm_edid_block_valid] *ERROR* Raw EDID:
<3>00 00 ff ff ff 1e 3e 9a 02 08 01 ee 1b ea 05 57  ......>........W
<3>9d 12 54 54 ff ff ff ff ff ff ff ff ff ff ff ff  ..TT............
<3>ff ff ff ff ff a7 a7 ff ff ff ff ff ff ff ff ff  ................
<3>ff ff ff ff ff ff ff ff ff ff ff 6b 6b ff ff ff  ...........kk...
<3>ff ff ff ff ff ff ff ff ff ff ff ff ff ff 80 80  ................
<3>ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
<3>ff 81 81 ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
<3>ff ff ff ff ff ff ff 80 80 4f 01 01 01 01 01 01  .........O......

[drm:radeon_dvi_detect] *ERROR* DVI-I-1: probed a monitor but no|invalid EDID
No connectors reported connected with modes
[drm] Cannot find any crtc or sizes - going 1024x768
[drm] fb mappable at 0xE0040000
[drm] vram apper at 0xE0000000
[drm] size 3145728
[drm] fb depth is 24
[drm]    pitch is 4096
Console: switching to colour frame buffer device 128x48
fb0: radeondrmfb frame buffer device
drm: registered panic notifier
[drm] Initialized radeon 2.7.0 20080528 for 0000:01:00.0 on minor 0

The monitor is connected via DVI to a KVM box. However, another PC with a RV790 card that is connected to the exact same KVM box and monitor is reading the EDID just fine.
Comment 1 Alex Deucher 2011-02-21 16:06:32 UTC
hw i2c was only implemented for r1xx-5xx asics; support for newer asics was never added.  The prescale value is not calculated reliably on certain pre-r5xx asics which lead to unreliable operation.
Comment 2 Chris Rankin 2011-02-22 13:23:37 UTC
(In reply to comment #1)
> The prescale value is not calculated reliably on certain pre-r5xx
> asics which lead to unreliable operation.

I since tested the hw_i2c parameter with my rv280, and it fails here too. However, in this particular case I was able to "tweak" the algorithm so that it did work:

--- radeon_i2c.c.orig	2011-02-22 13:07:13.000000000 +0000
+++ radeon_i2c.c	2011-02-22 13:12:55.000000000 +0000
@@ -235,7 +235,7 @@
 		i2c_clock = 60;
 		nm = (sclk * 10) / (i2c_clock * 4);
 		for (loop = 1; loop < 255; loop++) {
-			if ((nm / loop) < loop)
+			if ((nm / loop) <= loop)
 				break;
 		}
 		n = loop - 1;

The working values were: sclk=25000, n=31, m=30. When I tried this exact same patch with my RV350, it did work *once*. However, I have not been able to repeat that success.

Can you point me to some context for this algorithm, please?
Comment 3 Chris Rankin 2011-02-22 13:34:21 UTC
Created attachment 48662 [details]
dmesg output from RV350 on single transient success with hw_i2c=1

This dmesg output from my serial console shows that changing the "< loop" to "<= loop" did work on one occasion. However, it didn't last for long, and had started spitting out EDID errors again by the time I rebooted.
Comment 4 Alex Deucher 2011-02-22 16:58:14 UTC
The hw i2c clock is derived from the engine clock on r1xx-r5xx asics.  The prescale scales it down from the engine clock to something useful for i2c.  I'm not sure exactly how the prescale is derived beyond the algorithms.
Comment 5 Chris Rankin 2011-02-22 19:40:23 UTC
So the correct prescale value is really "whatever magic number happens to make it work"? Because the algorithm wouldn't need tweaking if it were truly definitive. Does it have a basis in any released documentation, and which asics is it currently known to work for, please?
Comment 6 Alex Deucher 2011-02-22 19:55:46 UTC
HW i2c works on every card family I tested (rv100, rv280, rv380, rv410, r430, rv515, rv530) when I wrote the code.  The only chips that have reported problems are the oldest ones (the pre-rv380 chips; r100-rv350).  There isn't any documentation that I know of any more for how to calculate the prescale on these older asics; I dug up some old r100 sample code with the algo in it and it worked for me on the chips in question, but internally at AMD, no one has looked at this stuff in 8-10 years so info about it is pretty scarce.

You might try adjusting the i2c_clock value as well; it could be that some chips preferred slightly faster/slower hw i2c clocks.
Comment 7 Chris Rankin 2011-02-22 21:31:04 UTC
(In reply to comment #6)
> The only chips that have reported problems are the oldest ones
> (the pre-rv380 chips; r100-rv350).

No coincidence that these are all the asics that calculate the prescale value using the for-loop algorithm, eh ;-)? (I have tested hw_i2c with my M66GL and it's fine, BTW.)

So it looks like "the trick" is to find the value of n such that ((n - 1) | (n << 8)) "works". It looks like n = 31 for my rv280, but not quite for my RV350. Both these cards have sclk = 25000, and so this algorithm would imply that there should exist a single value of n that works for both these cards too, correct?
Comment 8 Alex Deucher 2011-02-22 22:29:53 UTC
(In reply to comment #7)
> 
> No coincidence that these are all the asics that calculate the prescale value
> using the for-loop algorithm, eh ;-)? (I have tested hw_i2c with my M66GL and
> it's fine, BTW.)
> 
> So it looks like "the trick" is to find the value of n such that ((n - 1) |
> (n
> << 8)) "works". It looks like n = 31 for my rv280, but not quite for my
> RV350.
> Both these cards have sclk = 25000, and so this algorithm would imply that
> there should exist a single value of n that works for both these cards too,
> correct?

The algo I found was for r100 IIRC, but it happened to work for me on r1xx, r2xx, and r3xx, so I went with it.  I wasn't able to find any info specially about r2xx or r3xx.  It's possible that different asics need slightly different algos.

If you look at the rv380-rv410 and r5xx algos, they are actually pretty similar to the loop one, only they use a a fixed value of 128 for m and 100 for the i2c_clock. e.g.,

diff --git a/drivers/gpu/drm/radeon/radeon_i2c.c b/drivers/gpu/drm/radeon/radeon_i2c.c
index ded2a45..734b1ef 100644
--- a/drivers/gpu/drm/radeon/radeon_i2c.c
+++ b/drivers/gpu/drm/radeon/radeon_i2c.c
@@ -248,7 +248,10 @@ static u32 radeon_get_i2c_prescale(struct radeon_device *rdev)
        case CHIP_R420:
        case CHIP_R423:
        case CHIP_RV410:
-               prescale = (((sclk * 10)/(4 * 128 * 100) + 1) << 8) + 128;
+               i2c_clock = 100;
+               m = 128;
+               n = ((sclk * 10) / (4 * 128 * i2c_clock)) + 1;
+               prescale = m | (n << 8);
                break;
        case CHIP_RS600:
        case CHIP_RS690:
@@ -261,11 +264,17 @@ static u32 radeon_get_i2c_prescale(struct radeon_device *rdev)
        case CHIP_RV560:
        case CHIP_RV570:
        case CHIP_R580:
-               i2c_clock = 50;
-               if (rdev->family == CHIP_R520)
-                       prescale = (127 << 8) + ((sclk * 10) / (4 * 127 * i2c_clock));
-               else
-                       prescale = (((sclk * 10)/(4 * 128 * 100) + 1) << 8) + 128;
+               if (rdev->family == CHIP_R520) {
+                       i2c_clock = 50;
+                       m = (sclk * 10) / (4 * 127 * i2c_clock);
+                       n = 127;
+                       prescale = m | (n << 8);
+               } else {
+                       i2c_clock = 100;
+                       m = 128;
+                       n = ((sclk * 10) / (4 * 128 * i2c_clock)) + 1;
+                       prescale = m | (n << 8);
+               }
                break;
        case CHIP_R600:
        case CHIP_RV610:
Comment 9 Chris Rankin 2011-02-22 22:59:44 UTC
I've just tested my RV350 and discovered that hw_i2c *seems* to work for 11 <= n <= 30. IOW, I get a picture on my DVI monitor without any scary errors in the dmesg log.
Comment 10 Chris Rankin 2011-02-23 11:11:08 UTC
For my RV280, hw_i2c works for 12 <= n <= 31. I still have my rv100 to test as well.
Comment 11 Chris Rankin 2011-02-23 22:20:14 UTC
My rv100 has a sclk value of 13300, and I have (so far) been unable to find a value of n that will enable HW I2C.
Comment 12 Chris Rankin 2011-02-26 20:46:00 UTC
Created attachment 49322 [details]
dmesg output showing I2C errors with rv100 and DVI monitor

Actually, it looks like 2.6.37.2 has deeper issues with rv100 and I2C, beyond merely hw_i2c. Device i2c-0/name is:

Radeon i2c bit bus DVI_DDC
Comment 13 Alan 2012-08-16 11:01:32 UTC
Is this still a problem ?
Comment 14 Chris Rankin 2012-08-16 22:04:05 UTC
(In reply to comment #13)
> Is this still a problem ?

Yes, but I have been patching my kernel locally so that it works for my RV280 and RV350 cards. (I never did manage to fix it for my rv100, though).

Without the patch, enabling hw_i2c results in a lot of this:

[   44.954542] Raw EDID:
[   44.956826]      00 ff ff ff ff ff ff 00 ff ff ff ff ff ff ff ff
[   44.962576]      ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[   44.968328]      ff ff ff ff 00 ff ff 1e ff ff 6d 6d ff ff 3e 3e
[   44.974080]      ff ff ff 4e 4e ff ff ff 9a 9a ff ff ff f7 f7 ff
[   44.979830]      ff ff 02 02 ff ff 00 00 ff ff ff ff ff ff ff 08
[   44.985580]      08 ff ff 10 10 ff ff ff ff 01 01 ff ff ff 03 03
[   44.991330]      ff ff ff ee ee ff ff ff 2b 2b ff ff ff ff 1b 1b
[   44.997080]      ff ff ff ff ff ff ff 78 78 ff ff ea ff ff c6 ff
[   45.002836] radeon 0000:01:00.0: DVI-I-1: EDID block 0 invalid.
[   45.008765] [drm:radeon_dvi_detect] *ERROR* DVI-I-1: probed a monitor but no|invalid EDID

And no monitor output, of course.
Comment 15 Chris Rankin 2012-08-16 22:07:26 UTC
Created attachment 77871 [details]
Fixes hw_i2c for RV350 and RV280 cards.

Testing whether this fixes any other cases would be good, although I can confirm that it *doesn't* fix the rv100 case.

Signed-off-by: Chris Rankin <rankincj@yahoo.com>