| Message ID | 1301898405-6999-1-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 5, 2011 at 2:26 AM, Keith Packard <keithp@keithp.com> wrote: > On Mon, 04 Apr 2011 16:29:55 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > >> Yes. I'm saying that that the controller accepts a write to port 0xa0. > > So it's the GMBUS controller itself then, I guess. Weird. > > Let me see if I understand how it used to work and why fixing the GMBUS > reset causes it to break in this case. > I could be missing something here, but aren't i2c addresses 8-bit in this case? 7-bit addr + direction bit, which means 0xa0 isn't valid i2c address, since in Linux the read/write bits are specified separately. I could be wrong though. Dave.
On Tue, 5 Apr 2011 10:19:16 +1000, Dave Airlie <airlied@gmail.com> wrote: > On Tue, Apr 5, 2011 at 2:26 AM, Keith Packard <keithp@keithp.com> wrote: > > On Mon, 04 Apr 2011 16:29:55 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > >> Yes. I'm saying that that the controller accepts a write to port 0xa0. > > > > So it's the GMBUS controller itself then, I guess. Weird. > > > > Let me see if I understand how it used to work and why fixing the GMBUS > > reset causes it to break in this case. > > > > I could be missing something here, but aren't i2c addresses 8-bit in this case? > > 7-bit addr + direction bit, which means 0xa0 isn't valid i2c address, > since in Linux > the read/write bits are specified separately. Could this be is a mis-translation of some X server code? The X server stuck the direction bit in the LSB of the I2C address and required that the drivers shift the register up and add the direction bit manually (yes, a terrible API). This is starting to make a lot more sense now. 0xa0 == 0x50 << 1.
On Mon, 4 Apr 2011 07:26:44 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > Following the fix to reset the GMBUS controller after a NAK, we finally > utilize the 0xa0 probe for a CRT connection. And discover that it is > useless as it simply detects the presence of the controller and not the > actual monitor. Given that we already probe 0x50 prior to performing the > EDID retrieval, we can simply remove the redundant probe to 0xa0. This looks like a revert of 6ec3d0c0e9c0c605696e91048eebaca7b0c36695. I'd rather see it as that than a new patch.
On Mon, 04 Apr 2011 09:26:20 -0700, Keith Packard <keithp@keithp.com> wrote: > On Mon, 04 Apr 2011 16:29:55 +0100, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > > Yes. I'm saying that that the controller accepts a write to port 0xa0. > > So it's the GMBUS controller itself then, I guess. Weird. > > Let me see if I understand how it used to work and why fixing the GMBUS > reset causes it to break in this case. It also requires just the right combination of hardware to reproduce: in my case a 915GM (pre-CRT hotplug detect I think is the critical factor). > In the distant past (pre-GMBUS) > > 1) Some previous DDC transaction would fail, but without GMBUS > this would not break the bus > 2) The 0xA0 transaction would fail as there wasn't anyone > listening on the DDC bus. Not quite. In this case, it fails because the core i2c bitbanging algo doesn't seem to handle a solitary write message and reports EREMOTEIO. Whereas using GMBUS to drive the i2c xfer, the hardware completes the message without reporting an error. > 3) The 0x50 transaction would also fail, again because no-one > was listening > 4) The monitor would be reported as disconnected. > > In the recent past (post-GMBUS): > > 1) Some previous DDC transaction would fail, wedging the GMBUS > 2) The 0xA0 transaction would then fail due to the GMBUS breakage On my test hardware, there happens to be no previous NAK and so it reports "CRT detected via DDC:0xa0" anyway. But a NAK here can only explain how the regressions were only reported after 7f58aabc. > 3) The 0x50 transaction would also fail as the GMBUS was wedged > 4) The VGA port would be reported as disconnected > > With the GMBUS reset: > > 1) Some previous DDC transaction would fail, but the GMBUS would get > reset > 2) The 0xA0 transaction would now succeed. > 3) The VGA port would be reported as connected. Technically as unknown, since although we decided that there was a connection, we could not retrieve an EDID. > Do we have any idea what ports the GMBUS controller is listening > internally for? And, whether this differs from chip to chip? As Dave suggested, using 0xa0 was fubar. And in this case the controller was just presumably accepting a write to 0x50, when I expected it to be NAKed due to no attached slave listening on that address. Of course, in testing, I choose a combination of hardware (915GM and an old VGA panel) that proved impossible to retrieve the EDID for, whether using bitbanging i2c or GMBUS. (I'm seeing the same invalid checksum on a SugarBar, so it is probably not timing related - but I think this is a regression in itself.) That did prevent testing a few of the other cases since even when connected, we could do no better than unknown. So what I am less clear on is how it actually worked if the GMBUS was NAKed, and so proceeded past the 0xa0 probe to the real EDID probe and yet appear to work. -Chris
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c index 8342259..d03fc05 100644 --- a/drivers/gpu/drm/i915/intel_crt.c +++ b/drivers/gpu/drm/i915/intel_crt.c @@ -269,21 +269,6 @@ static bool intel_crt_detect_hotplug(struct drm_connector *connector) return ret; } -static bool intel_crt_ddc_probe(struct drm_i915_private *dev_priv, int ddc_bus) -{ - u8 buf; - struct i2c_msg msgs[] = { - { - .addr = 0xA0, - .flags = 0, - .len = 1, - .buf = &buf, - }, - }; - /* DDC monitor detect: Does it ACK a write to 0xA0? */ - return i2c_transfer(&dev_priv->gmbus[ddc_bus].adapter, msgs, 1) == 1; -} - static bool intel_crt_detect_ddc(struct drm_connector *connector) { struct intel_crt *crt = intel_attached_crt(connector); @@ -293,11 +278,6 @@ static bool intel_crt_detect_ddc(struct drm_connector *connector) if (crt->base.type != INTEL_OUTPUT_ANALOG) return false; - if (intel_crt_ddc_probe(dev_priv, dev_priv->crt_ddc_pin)) { - DRM_DEBUG_KMS("CRT detected via DDC:0xa0\n"); - return true; - } - if (intel_ddc_probe(&crt->base, dev_priv->crt_ddc_pin)) { struct edid *edid; bool is_digital = false;
Following the fix to reset the GMBUS controller after a NAK, we finally utilize the 0xa0 probe for a CRT connection. And discover that it is useless as it simply detects the presence of the controller and not the actual monitor. Given that we already probe 0x50 prior to performing the EDID retrieval, we can simply remove the redundant probe to 0xa0. Reported-and-tested-by: Sitsofe Wheeler <sitsofe@yahoo.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=35904 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_crt.c | 20 -------------------- 1 files changed, 0 insertions(+), 20 deletions(-)