[PATCH 1/3] drm: make debug levels match in edid failure code.

[PATCH 1/3] drm: make debug levels match in edid failure code.

[Dave Airlie]

From: Dave Airlie <airlied@linux.ie>

this puts the header and followup at the same loglevel as the
hex dump code.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_edid.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0a9357c..3618d29 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -184,9 +184,9 @@ drm_edid_block_valid(u8 *raw_edid)
 
 bad:
 	if (raw_edid) {
-		DRM_ERROR("Raw EDID:\n");
+		printk(KERN_ERR "Raw EDID:\n");
 		print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE, raw_edid, EDID_LENGTH);
-		printk("\n");
+		printk(KERN_ERR "\n");
 	}
 	return 0;
 }
-- 
1.7.5.2

[PATCH 2/3] drm/radeon: workaround a hw bug on some radeon chipsets with all-0 EDIDs.

[Dave Airlie]

From: Dave Airlie <airlied@linux.ie>

Some RS690 chipsets seem to end up with floating connectors, either
a DVI connector isn't actually populated, or an add-in HDMI card
is available but not installed. In this case we seem to get a NULL byte
response for each byte of the i2c transaction, so we detect this
case and if we see it we don't do anymore DDC transactions on this
connector.

I've tested this on my RS690 without the HDMI card installed and
it seems to work fine.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_edid.c                 |   15 +++++++++++++++
 drivers/gpu/drm/radeon/radeon_connectors.c |    7 +++++++
 include/drm/drm_crtc.h                     |    2 ++
 3 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3618d29..0929219 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -258,6 +258,17 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf,
 	return ret == 2 ? 0 : -1;
 }
 
+static bool drm_edid_is_zero(u8 *in_edid, int length)
+{
+	int i;
+	u32 *raw_edid = (u32 *)in_edid;
+
+	for (i = 0; i < length / 4; i++)
+		if (*(raw_edid + i) != 0)
+			return false;
+	return true;
+}
+
 static u8 *
 drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
 {
@@ -273,6 +284,10 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
 			goto out;
 		if (drm_edid_block_valid(block))
 			break;
+		if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
+			connector->null_edid_counter++;
+			goto carp;
+		}
 	}
 	if (i == 4)
 		goto carp;
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 9c2929c..5c3393f 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -836,6 +836,13 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
 		if (!radeon_connector->edid) {
 			DRM_ERROR("%s: probed a monitor but no|invalid EDID\n",
 					drm_get_connector_name(connector));
+			/* rs690 seems to have a problem with connectors not existing and always
+			 * return a block of 0's. If we see this just stop polling on this output */
+			if ((rdev->family == CHIP_RS690) && radeon_connector->base.null_edid_counter) {
+				ret = connector_status_disconnected;
+				DRM_ERROR("%s: detected RS690 floating bus bug, stopping ddc detect\n", drm_get_connector_name(connector));
+				radeon_connector->ddc_bus = NULL;
+			}
 		} else {
 			radeon_connector->use_digital = !!(radeon_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 9573e0c..33d12f8 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -520,6 +520,8 @@ struct drm_connector {
 	uint32_t encoder_ids[DRM_CONNECTOR_MAX_ENCODER];
 	uint32_t force_encoder_id;
 	struct drm_encoder *encoder; /* currently active encoder */
+
+	int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
 };
 
 /**
-- 
1.7.5.2

Re: [PATCH 2/3] drm/radeon: workaround a hw bug on some radeon chipsets with all-0 EDIDs.

[Alex Deucher]

On Tue, Jun 14, 2011 at 2:13 AM, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@linux.ie>
>
> Some RS690 chipsets seem to end up with floating connectors, either
> a DVI connector isn't actually populated, or an add-in HDMI card
> is available but not installed. In this case we seem to get a NULL byte
> response for each byte of the i2c transaction, so we detect this
> case and if we see it we don't do anymore DDC transactions on this
> connector.
>
> I've tested this on my RS690 without the HDMI card installed and
> it seems to work fine.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Just one comment below, but other than that:

Reviewed-by: Alex Deucher <alexdeucher@gmail.com>

> ---
>  drivers/gpu/drm/drm_edid.c                 |   15 +++++++++++++++
>  drivers/gpu/drm/radeon/radeon_connectors.c |    7 +++++++
>  include/drm/drm_crtc.h                     |    2 ++
>  3 files changed, 24 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 3618d29..0929219 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -258,6 +258,17 @@ drm_do_probe_ddc_edid(struct i2c_adapter *adapter, unsigned char *buf,
>        return ret == 2 ? 0 : -1;
>  }
>
> +static bool drm_edid_is_zero(u8 *in_edid, int length)
> +{
> +       int i;
> +       u32 *raw_edid = (u32 *)in_edid;
> +
> +       for (i = 0; i < length / 4; i++)
> +               if (*(raw_edid + i) != 0)
> +                       return false;
> +       return true;
> +}
> +
>  static u8 *
>  drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
>  {
> @@ -273,6 +284,10 @@ drm_do_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
>                        goto out;
>                if (drm_edid_block_valid(block))
>                        break;
> +               if (i == 0 && drm_edid_is_zero(block, EDID_LENGTH)) {
> +                       connector->null_edid_counter++;
> +                       goto carp;
> +               }
>        }
>        if (i == 4)
>                goto carp;
> diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
> index 9c2929c..5c3393f 100644
> --- a/drivers/gpu/drm/radeon/radeon_connectors.c
> +++ b/drivers/gpu/drm/radeon/radeon_connectors.c
> @@ -836,6 +836,13 @@ radeon_dvi_detect(struct drm_connector *connector, bool force)
>                if (!radeon_connector->edid) {
>                        DRM_ERROR("%s: probed a monitor but no|invalid EDID\n",
>                                        drm_get_connector_name(connector));
> +                       /* rs690 seems to have a problem with connectors not existing and always
> +                        * return a block of 0's. If we see this just stop polling on this output */
> +                       if ((rdev->family == CHIP_RS690) && radeon_connector->base.null_edid_counter) {

You may want to extend this to RS740 as well since IIRC they were pin
compatible with RS690 and showed up in a lot of similar setups.

> +                               ret = connector_status_disconnected;
> +                               DRM_ERROR("%s: detected RS690 floating bus bug, stopping ddc detect\n", drm_get_connector_name(connector));
> +                               radeon_connector->ddc_bus = NULL;
> +                       }
>                } else {
>                        radeon_connector->use_digital = !!(radeon_connector->edid->input & DRM_EDID_INPUT_DIGITAL);
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 9573e0c..33d12f8 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -520,6 +520,8 @@ struct drm_connector {
>        uint32_t encoder_ids[DRM_CONNECTOR_MAX_ENCODER];
>        uint32_t force_encoder_id;
>        struct drm_encoder *encoder; /* currently active encoder */
> +
> +       int null_edid_counter; /* needed to workaround some HW bugs where we get all 0s */
>  };
>
>  /**
> --
> 1.7.5.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[PATCH 3/3] drm/radeon/kms: fix dac detect stealing in-use encoder.

[Dave Airlie]

From: Dave Airlie <airlied@linux.ie>

On my RS690 the VGA and TV-out share the same DAC, so when xrandr
is called, tv-out steals the encoder and tries to load detect on it,
however this causes flicker on the VGA.

This blocks this by testing if the encoder is connected to anything
and if so whether its connected to the probing connector.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_encoders.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_encoders.c b/drivers/gpu/drm/radeon/radeon_encoders.c
index 03f124d..eb07e74 100644
--- a/drivers/gpu/drm/radeon/radeon_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_encoders.c
@@ -1971,6 +1971,18 @@ radeon_atom_dac_detect(struct drm_encoder *encoder, struct drm_connector *connec
 	struct radeon_encoder *radeon_encoder = to_radeon_encoder(encoder);
 	struct radeon_connector *radeon_connector = to_radeon_connector(connector);
 	uint32_t bios_0_scratch;
+	struct drm_connector *test_connector;
+
+	/*
+	 * if we are passed a connector and it doesn't match what the encoder
+	 * is connected to do, don't do load detect as it might cause flicker.
+	 */
+	 list_for_each_entry(test_connector, &dev->mode_config.connector_list, head) {
+                if ((test_connector->encoder == encoder) && (test_connector != connector)) {
+			DRM_DEBUG_KMS("load detect failed: encoder in use\n");
+			return connector_status_disconnected;
+		}
+	}
 
 	if (!atombios_dac_load_detect(encoder, connector)) {
 		DRM_DEBUG_KMS("detect returned false \n");
-- 
1.7.5.2

Re: [PATCH 3/3] drm/radeon/kms: fix dac detect stealing in-use encoder.

[Alex Deucher]

On Tue, Jun 14, 2011 at 2:13 AM, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@linux.ie>
>
> On my RS690 the VGA and TV-out share the same DAC, so when xrandr
> is called, tv-out steals the encoder and tries to load detect on it,
> however this causes flicker on the VGA.
>
> This blocks this by testing if the encoder is connected to anything
> and if so whether its connected to the probing connector.

Could we get into a case where we fail to run load detect on both
connectors due to this check?

Alex

Re: [PATCH 1/3] drm: make debug levels match in edid failure code.

[Alex Deucher]

On Tue, Jun 14, 2011 at 2:13 AM, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@linux.ie>
>
> this puts the header and followup at the same loglevel as the
> hex dump code.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Reviewed-by: Alex Deucher <alexdeucher@gmail.com>

> ---
>  drivers/gpu/drm/drm_edid.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 0a9357c..3618d29 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -184,9 +184,9 @@ drm_edid_block_valid(u8 *raw_edid)
>
>  bad:
>        if (raw_edid) {
> -               DRM_ERROR("Raw EDID:\n");
> +               printk(KERN_ERR "Raw EDID:\n");
>                print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE, raw_edid, EDID_LENGTH);
> -               printk("\n");
> +               printk(KERN_ERR "\n");
>        }
>        return 0;
>  }
> --
> 1.7.5.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>

Re: [PATCH 1/3] drm: make debug levels match in edid failure code.

[Tormod Volden]

[-- Attachment #1: Type: text/plain, Size: 1099 bytes --]

On Tue, Jun 14, 2011 at 8:13 AM, Dave Airlie wrote:
> this puts the header and followup at the same loglevel as the
> hex dump code.
...
>  bad:
>        if (raw_edid) {
> -               DRM_ERROR("Raw EDID:\n");
> +               printk(KERN_ERR "Raw EDID:\n");
>                print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE, raw_edid, EDID_LENGTH);
> -               printk("\n");
> +               printk(KERN_ERR "\n");
>        }

It won't work because print_hex_dump_bytes() always uses KERN_DEBUG
anyway. Its first argument is a string prefix so currently the value
of the KERN_ERR macro is printed out:
[drm:drm_edid_block_valid] *ERROR* Raw EDID:
<3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................

We can use print_hex_dump() instead, see attached patch. I even tested
it with "dmesg -r" - I think Linus prefers tested patches at this
point in the cycle :) But it is purely cosmetic and can wait for 3.1
also.

Maybe we eventually should print out in a format like xrandr and Xorg
do, so tools like edid-decode can eat it.

Cheers,
Tormod

[-- Attachment #2: 0001-drm-really-make-debug-levels-match-in-edid-failure-c.patch --]
[-- Type: text/x-patch, Size: 1099 bytes --]

From f64fd288c2d8555df82be884d48787138ee02c3b Mon Sep 17 00:00:00 2001
From: Tormod Volden <debian.tormod@gmail.com>
Date: Fri, 17 Jun 2011 18:45:19 +0200
Subject: [PATCH] drm: really make debug levels match in edid failure code

Also disable the ascii dump and remove the literal printing of the
KERN_ERR macro in the log:

[drm:drm_edid_block_valid] *ERROR* Raw EDID:
<3>00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................

Signed-off-by: Tormod Volden <debian.tormod@gmail.com>
---
 drivers/gpu/drm/drm_edid.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0929219..218f586 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -185,7 +185,8 @@ drm_edid_block_valid(u8 *raw_edid)
 bad:
 	if (raw_edid) {
 		printk(KERN_ERR "Raw EDID:\n");
-		print_hex_dump_bytes(KERN_ERR, DUMP_PREFIX_NONE, raw_edid, EDID_LENGTH);
+		print_hex_dump(KERN_ERR, " \t", DUMP_PREFIX_NONE, 16, 1,
+			       raw_edid, EDID_LENGTH, false);
 		printk(KERN_ERR "\n");
 	}
 	return 0;
-- 
1.7.1


[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel