[RFC PATCH] Re: KMS:RV635:Radeon 3650HD graphics driver broken on a laptop when connected to a docking station and external display

[Pasi Kärkkäinen]

On Sat, Jun 19, 2010 at 10:24:53PM +0300, Pasi Kärkkäinen wrote:
> On Fri, Jun 18, 2010 at 09:45:22PM +0300, Pasi Kärkkäinen wrote:
> > > > > >> >
> > > > > >> > I wonder if that HDMI-0 is the key to the problem.. I don't have anything connected
> > > > > >> > to the HDMI connector on the laptop. There's no HDMI connector on the dock, it's only in the laptop.
> > > > > >>
> > > > > >> Indeed that is the problem.  On your system the HDMI and DVI ports
> > > > > >> share the same encoder and DDC line so they will both come up as
> > > > > >> connected since the line is shared.  The driver used to check the edid
> > > > > >> when the lines where shared and select HDMI or DVI based on the EDID,
> > > > > >> but perhaps that got broken at some point.
> > > > > >>
> > > > > >
> > > > > > Makes sense. Does some kernel option/parameter affect that?
> > > > > 
> > > > > No.  I'll check the code and see if I can figure out what's going on.
> > > > > 
> > > > 
> > > > Ok, thanks!
> > > > 
> > > 
> > > Hello,
> > > 
> > > I'll be near the dock tomorrow.. So if you have a chance of looking at this, and want me to test something,
> > > I could do it on thursday/friday.
> > > 
> > 
> > I just tried the latest Fedora 13 kernel (2.6.33.5-124.fc13.x86_64) and the latest 
> > Fedora development (rawhide 2.6.34-43.fc14.x86_64) kernel, but the behaviour was the same.
> > 
> > Connecting display to the DVI connector makes the HDMI connector also show the same display, 
> > breaking things..
> > 
> 
> Here's a new log with drm.debug=4.. that's the biggest debug level that still allows me to grab
> the whole dmesg log before it's overwritten by huge amount of debug messages.
> 
> Full dmesg log:
> http://pasik.reaktio.net/fedora/bz593429/both-internal-and-external-displays-active/dmesg-with-drmdebug4.txt
> 

Ok.. I did some debugging, and figured out how to make it work. Patch attached.
I'm not sure if this is the correct method.. someone familiar with the code should review it.


Some debugging information for my system (HP EliteBook 8530p):

> 
> [drm] Radeon Display Connectors
> [drm] Connector 0:
> [drm]   VGA
> [drm]   DDC: 0x7e40 0x7e40 0x7e44 0x7e44 0x7e48 0x7e48 0x7e4c 0x7e4c
> [drm]   Encoders:
> [drm]     CRT1: INTERNAL_KLDSCP_DAC1

connector->devices for VGA is: 1

> [drm] Connector 1:
> [drm]   LVDS
> [drm]   Encoders:
> [drm]     LCD1: INTERNAL_KLDSCP_LVTMA

connector->devices for LVDS is: 2

> [drm] Connector 2:
> [drm]   DVI-D
> [drm]   HPD1
> [drm]   DDC: 0x7e60 0x7e60 0x7e64 0x7e64 0x7e68 0x7e68 0x7e6c 0x7e6c
> [drm]   Encoders:
> [drm]     DFP1: INTERNAL_UNIPHY

connector->devices for DVI-D is: 8

> [drm] Connector 3:
> [drm]   HDMI-A
> [drm]   HPD2
> [drm]   DDC: 0x7e60 0x7e60 0x7e64 0x7e64 0x7e68 0x7e68 0x7e6c 0x7e6c
> [drm]   Encoders:
> [drm]     DFP2: INTERNAL_UNIPHY

connector->devices for HDMI-A is: 128



The code in radeon_connectors.c in radeon_dvi_detect():

	/* multiple connectors on the same encoder with the same ddc line
	 * This tends to be HDMI and DVI on the same encoder with the
	 * same ddc line.  If the edid says HDMI, consider the HDMI port
	 * connected and the DVI port disconnected.  If the edid doesn't
	 * say HDMI, vice versa.
	 */
	if (radeon_connector->shared_ddc && (ret == connector_status_connected)) {
		struct drm_device *dev = connector->dev;
		struct drm_connector *list_connector;
		struct radeon_connector *list_radeon_connector;
		list_for_each_entry(list_connector, &dev->mode_config.connector_list, head) {
			if (connector == list_connector)
				continue;
			list_radeon_connector = to_radeon_connector(list_connector);
			if (radeon_connector->devices == list_radeon_connector->devices) {
				if (drm_detect_hdmi_monitor(radeon_connector->edid)) {
					if (connector->connector_type == DRM_MODE_CONNECTOR_DVID) {
						kfree(radeon_connector->edid);
						radeon_connector->edid = NULL;
						ret = connector_status_disconnected;
					}
				} else {
					if ((connector->connector_type == DRM_MODE_CONNECTOR_HDMIA) ||
					    (connector->connector_type == DRM_MODE_CONNECTOR_HDMIB)) {
						kfree(radeon_connector->edid);
						radeon_connector->edid = NULL;
						ret = connector_status_disconnected;
					}
				}
			}
		}
	}


Ok, so at least on my system the "if (connector == list_connector)" will SKIP the connector
that actually would match in "if (radeon_connector->devices == list_radeon_connector->devices)",
and thus the drm_detect_hdmi_monitor() is never called.

My patch removes that "if (connector == list_connector)" check and fixes the problem on my system.
Is this the correct way to do it? 

radeon-fix-shared_ddc-dvi-hdmi-edid-detection.patch:
	- allow checking against all connectors to be able to properly detect shared_ddc dvi/hdmi displays
	- patch applies to 2.6.33.5, 2.6.34 and 2.6.35-rc3. Tested with Fedora 13 2.6.33.5-124.fc13.x86_64.
	- Signed-off-by: Pasi Kärkkäinen <pasik at iki.fi>


Thanks!

Btw. I still have some other problems related to lid open/closed detection for the LVDS but I'll open another thread about that :) 

-- Pasi