[20/90] drm/radeon: Extended DDC Probing for Connectors with Improperly Wired DDC Lines (here: Asus M2A-VM HDMI)

From: Greg KH
Date: Sun Aug 14 2011 - 12:06:50 EST


3.0-stable review patch. If anyone has any objections, please let us know.

------------------

From: Thomas Reim <reimth@xxxxxxxxx>

commit e384fab8c6f3ca88600bcb2ebdf0eb2f90864fab upstream.

Some integrated ATI Radeon chipset implementations with add-on HDMI card
(e. g. Asus M2A-VM HDMI) indicate the availability of a DDC even
when the add-on card is not plugged in or HDMI is disabled in BIOS setup.
In this case, drm_get_edid() and drm_edid_block_valid() periodically
dump data and kernel errors into system log files and onto terminals.
For these connectors DDC probing is extended by a check for a correct
EDID header. Only in case a valid EDID header is also found, the
(HDMI or DVI) connector will be used by the Radeon driver. This prevents
the kernel driver from useless flooding of logs and terminal sessions with
EDID dumps and error messages.
This patch adds a flag 'requires_extended_probe' to the radeon_connector
structure. In function radeon_connector_needs_extended_probe() this flag
can be set on a chipset family/vendor/connector type specific basis.
In addition, function radeon_ddc_probe() has been adapted to perform
extended DDC probing if required by the connector's flag.
Requires function drm_edid_header_is_valid() in DRM module provided by
[PATCH] drm: Separate EDID Header Check from EDID Block Check.

Tested for kernel 2.6.35, 2.6.38 and 3.0 on Asus M2A-VM HDMI board

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=668196
BugLink: http://bugs.launchpad.net/bugs/7228066

Signed-off-by: Thomas Reim <reimth@xxxxxxxxx>
Reviewed-by: Alex Deucher <alexdeucher@xxxxxxxxx>
Acked-by: Stephen Michaels <Stephen.Micheals@xxxxxxxxx>
Signed-off-by: Dave Airlie <airlied@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>

---
drivers/gpu/drm/radeon/radeon_connectors.c | 45 +++++++++++++++++++++++++++--
drivers/gpu/drm/radeon/radeon_display.c | 9 +++++
drivers/gpu/drm/radeon/radeon_i2c.c | 32 +++++++++++++++-----
drivers/gpu/drm/radeon/radeon_mode.h | 6 +++
4 files changed, 80 insertions(+), 12 deletions(-)

--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -430,6 +430,36 @@ int radeon_connector_set_property(struct
return 0;
}

+/*
+ * Some integrated ATI Radeon chipset implementations (e. g.
+ * Asus M2A-VM HDMI) may indicate the availability of a DDC,
+ * even when there's no monitor connected. For these connectors
+ * following DDC probe extension will be applied: check also for the
+ * availability of EDID with at least a correct EDID header. Only then,
+ * DDC is assumed to be available. This prevents drm_get_edid() and
+ * drm_edid_block_valid() from periodically dumping data and kernel
+ * errors into the logs and onto the terminal.
+ */
+static bool radeon_connector_needs_extended_probe(struct radeon_device *dev,
+ uint32_t supported_device,
+ int connector_type)
+{
+ /* Asus M2A-VM HDMI board sends data to i2c bus even,
+ * if HDMI add-on card is not plugged in or HDMI is disabled in
+ * BIOS. Valid DDC can only be assumed, if also a valid EDID header
+ * can be retrieved via i2c bus during DDC probe */
+ if ((dev->pdev->device == 0x791e) &&
+ (dev->pdev->subsystem_vendor == 0x1043) &&
+ (dev->pdev->subsystem_device == 0x826d)) {
+ if ((connector_type == DRM_MODE_CONNECTOR_HDMIA) &&
+ (supported_device == ATOM_DEVICE_DFP2_SUPPORT))
+ return true;
+ }
+
+ /* Default: no EDID header probe required for DDC probing */
+ return false;
+}
+
static void radeon_fixup_lvds_native_mode(struct drm_encoder *encoder,
struct drm_connector *connector)
{
@@ -661,7 +691,8 @@ radeon_vga_detect(struct drm_connector *
ret = connector_status_disconnected;

if (radeon_connector->ddc_bus)
- dret = radeon_ddc_probe(radeon_connector);
+ dret = radeon_ddc_probe(radeon_connector,
+ radeon_connector->requires_extended_probe);
if (dret) {
if (radeon_connector->edid) {
kfree(radeon_connector->edid);
@@ -833,7 +864,8 @@ radeon_dvi_detect(struct drm_connector *
bool dret = false;

if (radeon_connector->ddc_bus)
- dret = radeon_ddc_probe(radeon_connector);
+ dret = radeon_ddc_probe(radeon_connector,
+ radeon_connector->requires_extended_probe);
if (dret) {
if (radeon_connector->edid) {
kfree(radeon_connector->edid);
@@ -1251,7 +1283,8 @@ radeon_dp_detect(struct drm_connector *c
if (radeon_dp_getdpcd(radeon_connector))
ret = connector_status_connected;
} else {
- if (radeon_ddc_probe(radeon_connector))
+ if (radeon_ddc_probe(radeon_connector,
+ radeon_connector->requires_extended_probe))
ret = connector_status_connected;
}
}
@@ -1406,6 +1439,9 @@ radeon_add_atom_connector(struct drm_dev
radeon_connector->shared_ddc = shared_ddc;
radeon_connector->connector_object_id = connector_object_id;
radeon_connector->hpd = *hpd;
+ radeon_connector->requires_extended_probe =
+ radeon_connector_needs_extended_probe(rdev, supported_device,
+ connector_type);
radeon_connector->router = *router;
if (router->ddc_valid || router->cd_valid) {
radeon_connector->router_bus = radeon_i2c_lookup(rdev, &router->i2c_info);
@@ -1752,6 +1788,9 @@ radeon_add_legacy_connector(struct drm_d
radeon_connector->devices = supported_device;
radeon_connector->connector_object_id = connector_object_id;
radeon_connector->hpd = *hpd;
+ radeon_connector->requires_extended_probe =
+ radeon_connector_needs_extended_probe(rdev, supported_device,
+ connector_type);
switch (connector_type) {
case DRM_MODE_CONNECTOR_VGA:
drm_connector_init(dev, &radeon_connector->base, &radeon_vga_connector_funcs, connector_type);
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -777,8 +777,17 @@ static int radeon_ddc_dump(struct drm_co
if (!radeon_connector->ddc_bus)
return -1;
edid = drm_get_edid(connector, &radeon_connector->ddc_bus->adapter);
+ /* Log EDID retrieval status here. In particular with regard to
+ * connectors with requires_extended_probe flag set, that will prevent
+ * function radeon_dvi_detect() to fetch EDID on this connector,
+ * as long as there is no valid EDID header found */
if (edid) {
+ DRM_INFO("Radeon display connector %s: Found valid EDID",
+ drm_get_connector_name(connector));
kfree(edid);
+ } else {
+ DRM_INFO("Radeon display connector %s: No monitor connected or invalid EDID",
+ drm_get_connector_name(connector));
}
return ret;
}
--- a/drivers/gpu/drm/radeon/radeon_i2c.c
+++ b/drivers/gpu/drm/radeon/radeon_i2c.c
@@ -32,17 +32,17 @@
* radeon_ddc_probe
*
*/
-bool radeon_ddc_probe(struct radeon_connector *radeon_connector)
+bool radeon_ddc_probe(struct radeon_connector *radeon_connector, bool requires_extended_probe)
{
- u8 out_buf[] = { 0x0, 0x0};
- u8 buf[2];
+ u8 out = 0x0;
+ u8 buf[8];
int ret;
struct i2c_msg msgs[] = {
{
.addr = 0x50,
.flags = 0,
.len = 1,
- .buf = out_buf,
+ .buf = &out,
},
{
.addr = 0x50,
@@ -52,15 +52,31 @@ bool radeon_ddc_probe(struct radeon_conn
}
};

+ /* Read 8 bytes from i2c for extended probe of EDID header */
+ if (requires_extended_probe)
+ msgs[1].len = 8;
+
/* on hw with routers, select right port */
if (radeon_connector->router.ddc_valid)
radeon_router_select_ddc_port(radeon_connector);

ret = i2c_transfer(&radeon_connector->ddc_bus->adapter, msgs, 2);
- if (ret == 2)
- return true;
-
- return false;
+ if (ret != 2)
+ /* Couldn't find an accessible DDC on this connector */
+ return false;
+ if (requires_extended_probe) {
+ /* Probe also for valid EDID header
+ * EDID header starts with:
+ * 0x00,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0x00.
+ * Only the first 6 bytes must be valid as
+ * drm_edid_block_valid() can fix the last 2 bytes */
+ if (drm_edid_header_is_valid(buf) < 6) {
+ /* Couldn't find an accessible EDID on this
+ * connector */
+ return false;
+ }
+ }
+ return true;
}

/* bit banging i2c */
--- a/drivers/gpu/drm/radeon/radeon_mode.h
+++ b/drivers/gpu/drm/radeon/radeon_mode.h
@@ -438,6 +438,9 @@ struct radeon_connector {
struct radeon_i2c_chan *ddc_bus;
/* some systems have an hdmi and vga port with a shared ddc line */
bool shared_ddc;
+ /* for some Radeon chip families we apply an additional EDID header
+ check as part of the DDC probe */
+ bool requires_extended_probe;
bool use_digital;
/* we need to mind the EDID between detect
and get modes due to analog/digital/tvencoder */
@@ -514,7 +517,8 @@ extern void radeon_i2c_put_byte(struct r
u8 val);
extern void radeon_router_select_ddc_port(struct radeon_connector *radeon_connector);
extern void radeon_router_select_cd_port(struct radeon_connector *radeon_connector);
-extern bool radeon_ddc_probe(struct radeon_connector *radeon_connector);
+extern bool radeon_ddc_probe(struct radeon_connector *radeon_connector,
+ bool requires_extended_probe);
extern int radeon_ddc_get_modes(struct radeon_connector *radeon_connector);

extern struct drm_encoder *radeon_best_encoder(struct drm_connector *connector);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/