All of lore.kernel.org
 help / color / mirror / Atom feed
* Enhancing EDID quirk functionality
@ 2012-04-19 19:16 Ian Pilcher
  2012-04-24  9:07 ` Lars-Peter Clausen
  0 siblings, 1 reply; 32+ messages in thread
From: Ian Pilcher @ 2012-04-19 19:16 UTC (permalink / raw)
  To: dri-devel

Greetings all!

I recently discovered that my nice 1900x1200 display is horribly
confused by the InfoFrame functionality that was added to the nouveau
driver in Linux 3.3.  Additional testing has shown that it has the same
problem with the i915 driver and NVIDIA's proprietary driver.

NVIDIA's Windows 7 driver does not exhibit the problem, but it appears
that the problem was seen a few years ago on Windows Vista.  See this
Red Hat Bugzilla for all the gory details:

  https://bugzilla.redhat.com/show_bug.cgi?id=806091

Ben Skeggs suggested that the proper way to proceed is an EDID quirk
that will disable InfoFrames by causing drm_detect_hdmi_monitor() return
false.  I'm thinking of EDID_QUIRK_DISABLE_HDMI as the symbolic name for
this quirk.

After looking through drm_edid.c and noticing that my display appears to
be reporting non-existent audio functionality, I believe that quirk
which disables HDMI audio functionality (by causing
drm_detect_monitor_audio() to return false) might also be useful.

Before I start turning things off, however, I think it's really
important to provide users with a way to override these quirks.  (Who's
to say there isn't a variant of my display out there somewhere with
exactly the same EDID that actually does have speakers?)  And wouldn't
it be extremely useful for users to be able to add EDID quirks on the
kernel command line -- for testing/workaround purposes?

I'm thinking of a syntax like:

  drm.edid_quirk=ACR:44358:0x01,ACR:2423:0x20

This would be parsed into a separate quirks list (edid_user_quirk_list?)
which could add new quirks or override built-in quirks (by changing
edid_get_quirks()).

Parsing the quirks will be a bit of a pain.  I'm new to writing kernel
code, so pointers to any appropriate helper functions or examples of
parsing somewhat complex strings in-kernel would be appreciated.

The other prerequisite, IMO, is for the kernel to log the displays that
it detects, so that users know what the first two fields of a quirk
should be.  (Currently, the easiest place to get this is the X log.)  I
tried adding this logging to drm_get_edid(), but the results were overly
verbose; apparently that function gets called fairly frequently.  Is
there a better place to log a newly detected display?

So my specific questions at this time are:

1.  Any objections to the functionality or approach that I've outlined
    above?

2.  How should I go about parsing a "quirk string"?  (Pointers to helper
    functions and/or examples of similar parsing requested.)

3.  What is the best place at which to log newly detected displays?

Thanks!

-- 
========================================================================
Ian Pilcher                                         arequipeno@gmail.com
"If you're going to shift my paradigm ... at least buy me dinner first."
========================================================================

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Enhancing EDID quirk functionality
  2012-04-19 19:16 Enhancing EDID quirk functionality Ian Pilcher
@ 2012-04-24  9:07 ` Lars-Peter Clausen
  2012-04-24 19:00   ` Ian Pilcher
  0 siblings, 1 reply; 32+ messages in thread
From: Lars-Peter Clausen @ 2012-04-24  9:07 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: dri-devel

On 04/19/2012 09:16 PM, Ian Pilcher wrote:
> Greetings all!
> 
> I recently discovered that my nice 1900x1200 display is horribly
> confused by the InfoFrame functionality that was added to the nouveau
> driver in Linux 3.3.  Additional testing has shown that it has the same
> problem with the i915 driver and NVIDIA's proprietary driver.
> 

I just had a similar issue with a different driver and remembered your post

If the S bits in the infoframe are 0 the display may under- or overscan the
the image (Although the spec says it should behave the same if no infoframe
is present). If it is set to 2 the display should underscan the image, so
I'd be interested to see if the following patch changes the displays
behavior for you.


--- a/drivers/gpu/drm/nouveau/nouveau_hdmi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_hdmi.c
@@ -147,7 +147,7 @@ static void
 nouveau_hdmi_video_infoframe(struct drm_encoder *encoder,
 			     struct drm_display_mode *mode)
 {
-	const u8 Y = 0, A = 0, B = 0, S = 0, C = 0, M = 0, R = 0;
+	const u8 Y = 0, A = 0, B = 0, S = 2, C = 0, M = 0, R = 0;
 	const u8 ITC = 0, EC = 0, Q = 0, SC = 0, VIC = 0, PR = 0;
 	const u8 bar_top = 0, bar_bottom = 0, bar_left = 0, bar_right = 0;
 	u8 frame[20];

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Enhancing EDID quirk functionality
  2012-04-24  9:07 ` Lars-Peter Clausen
@ 2012-04-24 19:00   ` Ian Pilcher
  2012-05-03 18:01     ` Ian Pilcher
  0 siblings, 1 reply; 32+ messages in thread
From: Ian Pilcher @ 2012-04-24 19:00 UTC (permalink / raw)
  To: dri-devel

On 04/24/2012 04:07 AM, Lars-Peter Clausen wrote:
> I just had a similar issue with a different driver and remembered your post
>
> If the S bits in the infoframe are 0 the display may under- or overscan the
> the image (Although the spec says it should behave the same if no infoframe
> is present). If it is set to 2 the display should underscan the image, so
> I'd be interested to see if the following patch changes the displays
> behavior for you.
>
>
> --- a/drivers/gpu/drm/nouveau/nouveau_hdmi.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_hdmi.c
> @@ -147,7 +147,7 @@ static void
>   nouveau_hdmi_video_infoframe(struct drm_encoder *encoder,
>   			     struct drm_display_mode *mode)
>   {
> -	const u8 Y = 0, A = 0, B = 0, S = 0, C = 0, M = 0, R = 0;
> +	const u8 Y = 0, A = 0, B = 0, S = 2, C = 0, M = 0, R = 0;
>   	const u8 ITC = 0, EC = 0, Q = 0, SC = 0, VIC = 0, PR = 0;
>   	const u8 bar_top = 0, bar_bottom = 0, bar_left = 0, bar_right = 0;
>   	u8 frame[20];

No change.

BTW, I've been grinding ahead with the EDID quirk infrastructure
changes that I discussed in my previous note.  I'd prefer not to waste
huge amounts of my less-than-copious free time, so please speak up if
you have any objections.

(The preceding paragraph is not specifically directed at Lars-Peter.)

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Enhancing EDID quirk functionality
  2012-04-24 19:00   ` Ian Pilcher
@ 2012-05-03 18:01     ` Ian Pilcher
  2012-05-03 19:42       ` Adam Jackson
  0 siblings, 1 reply; 32+ messages in thread
From: Ian Pilcher @ 2012-05-03 18:01 UTC (permalink / raw)
  To: dri-devel

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

I just attached this patch to
https://bugzilla.redhat.com/show_bug.cgi?id=806091, along with the
following comments:

This is the "first draft" of an EDID quirk-based approach to solving
this problem.  I intend to perform a bit of cleanup and break it up
into a series of separate patches, but I'm posting it here for any
comments about the approach.

The patch does the following:

* Changes the vendor field of struct edid_quirk to an array, rather
  than a pointer.  (This has already been sent to the dri-devel list.)
* Adds two new quirks EDID_QUIRK_DISABLE_INFOFRAMES and
  EDID_QUIRK_NO_AUDIO. This first quirk causes drm_detect_hdmi_monitor
  to return false; the second   causes drm_detect_monitor_audio to
  return false.
* Logs the EDID vendor and model of connected monitors.
* Adds an edid_quirks parameter to the drm module, for user-defined
  quirks.

With this patch, my display works when I add
drm.edid_quirks=GSM:0x563f:0x180 to my kernel command line.  (0x80,
EDID_QUIRK_DISABLE_INFOFRAMES, makes it work with the nouveau driver;
0x100, EDID_QUIRK_NO_AUDIO, makes it work with the i915 driver.  I.e.,
the i915 driver is sending audio InfoFrames even when
drm_detect_hdmi_monitor returns false; bug?)

Thoughts?

-- 
========================================================================
Ian Pilcher                                         arequipeno@gmail.com
"If you're going to shift my paradigm ... at least buy me dinner first."
========================================================================

[-- Attachment #2: edid_quirks_all.patch --]
[-- Type: text/x-patch, Size: 9610 bytes --]

diff -ur linux-3.3/drivers/gpu/drm/drm_drv.c linux-3.3-working/drivers/gpu/drm/drm_drv.c
--- linux-3.3/drivers/gpu/drm/drm_drv.c	2012-03-18 18:15:34.000000000 -0500
+++ linux-3.3-working/drivers/gpu/drm/drm_drv.c	2012-04-27 21:30:07.535452184 -0500
@@ -280,6 +280,8 @@
 		ret = -1;
 		goto err_p3;
 	}
+	
+	drm_edid_xtra_quirks_parse();
 
 	DRM_INFO("Initialized %s %d.%d.%d %s\n",
 		 CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
@@ -304,6 +306,8 @@
 
 	idr_remove_all(&drm_minors_idr);
 	idr_destroy(&drm_minors_idr);
+	
+	drm_edid_xtra_quirks_free();
 }
 
 module_init(drm_core_init);
diff -ur linux-3.3/drivers/gpu/drm/drm_edid.c linux-3.3-working/drivers/gpu/drm/drm_edid.c
--- linux-3.3/drivers/gpu/drm/drm_edid.c	2012-03-18 18:15:34.000000000 -0500
+++ linux-3.3-working/drivers/gpu/drm/drm_edid.c	2012-04-28 01:11:51.316839743 -0500
@@ -66,6 +66,10 @@
 #define EDID_QUIRK_FIRST_DETAILED_PREFERRED	(1 << 5)
 /* use +hsync +vsync for detailed mode */
 #define EDID_QUIRK_DETAILED_SYNC_PP		(1 << 6)
+/* Display is confused by InfoFrames; don't send any. */
+#define EDID_QUIRK_DISABLE_INFOFRAMES		(1 << 7)
+/* Display doesn't have any audio output */
+#define EDID_QUIRK_NO_AUDIO			(1 << 8)
 
 struct detailed_mode_closure {
 	struct drm_connector *connector;
@@ -81,7 +85,7 @@
 #define LEVEL_CVT	3
 
 static struct edid_quirk {
-	char *vendor;
+	char vendor[4];
 	int product_id;
 	u32 quirks;
 } edid_quirk_list[] = {
@@ -122,13 +126,100 @@
 	{ "SAM", 638, EDID_QUIRK_PREFER_LARGE_60 },
 };
 
+/* Parsed extra quirks from the edid_quirks module parameter */
+static struct edid_quirk *edid_xtra_quirk_list;
+static int edid_num_xtra_quirks;
+
+/*
+ * Free the extra EDID quirks list.
+ */
+void drm_edid_xtra_quirks_free(void)
+{
+	kfree(edid_xtra_quirk_list);
+	edid_xtra_quirk_list = NULL;
+	edid_num_xtra_quirks = 0;
+}
+
+/*
+ * Parse the EDID quirk at s and store it a q. Returns 1 on success, 0 on error.
+ */
+static int drm_edid_parse_quirk(char *s, struct edid_quirk *q)
+{
+	char *c;
+
+	if (sscanf(s, "%3s:%i:%i",
+		   q->vendor, &q->product_id, &q->quirks) == 3) {
+		DRM_DEBUG("Parsed EDID quirk: "
+			  "mfr: %s, product: %d, quirks: %u\n",
+			  q->vendor, q->product_id, q->quirks);
+		return 1;
+	} else {
+		if ((c = strchr(s, ',')) != NULL) {
+			*c = 0;
+		}
+		printk(KERN_WARNING "Invalid EDID quirk: '%s'\n", s);
+		if (c != NULL) {
+			*c = ',';
+		}
+		return 0;
+	}
+}
+
+/*
+ * Parse drm_edid_xtra_quirks and create the extra EDID quirks list.
+ */
+void drm_edid_xtra_quirks_parse(void)
+{
+	int num_quirks;
+	char *c;
+	if (drm_edid_xtra_quirks == NULL) {
+		edid_xtra_quirk_list = NULL;
+		edid_num_xtra_quirks = 0;
+		return;
+	}
+	
+	/* Number of (potential) quirks = number of commas in param + 1 */
+	num_quirks = 1;
+	c = drm_edid_xtra_quirks;
+	while ((c = strchr(c, ',')) != NULL) {
+		++num_quirks;
+		++c;
+	}
+
+	if (num_quirks > DRM_EDID_XTRA_QUIRKS_MAX) {
+		printk(KERN_WARNING "Number of additional EDID quirks limited "
+		       "to " __stringify(DRM_EDID_XTRA_QUIRKS_MAX) "\n");
+		num_quirks = DRM_EDID_XTRA_QUIRKS_MAX;
+	}
+	
+	edid_xtra_quirk_list = kmalloc(sizeof(struct edid_quirk) * num_quirks,
+				       GFP_KERNEL);
+	if (edid_xtra_quirk_list == NULL) {
+		printk(KERN_WARNING "Failed to allocate memory for additional "
+		       "EDID quirks\n");
+		return;
+	}
+	
+	edid_num_xtra_quirks = 0;
+	c = drm_edid_xtra_quirks;
+	while (1) {
+		edid_num_xtra_quirks += drm_edid_parse_quirk(c,
+				&edid_xtra_quirk_list[edid_num_xtra_quirks]);
+		if (edid_num_xtra_quirks == num_quirks ||
+						(c = strchr(c, ',')) == NULL) {
+			break;
+		}
+		++c;
+	}
+}			
+			
 /*** DDC fetch and block validation ***/
 
 static const u8 edid_header[] = {
 	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
 };
 
- /*
+/*
  * Sanity check the header of the base EDID block.  Return 8 if the header
  * is perfect, down to 0 if it's totally wrong.
  */
@@ -366,6 +457,24 @@
 }
 
 /**
+ * edid_vendor_string - decodes EDID vendor field
+ * @edid: EDID from which to extract the vendor field
+ * @edid_vendor: buffer in which to store the vendor string
+ * 
+ * Returns a pointer to @edid_vendor
+ */
+static char *edid_vendor_string(const struct edid *edid, char edid_vendor[4])
+{
+	edid_vendor[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
+	edid_vendor[1] = (((edid->mfg_id[0] & 0x3) << 3) |
+			  ((edid->mfg_id[1] & 0xe0) >> 5)) + '@';
+	edid_vendor[2] = (edid->mfg_id[1] & 0x1f) + '@';
+	edid_vendor[3] = 0;
+	
+	return edid_vendor;
+}
+
+/**
  * drm_get_edid - get EDID data, if available
  * @connector: connector we're probing
  * @adapter: i2c adapter to use for DDC
@@ -378,15 +487,31 @@
 struct edid *drm_get_edid(struct drm_connector *connector,
 			  struct i2c_adapter *adapter)
 {
+	const struct edid *old_edid;
+	char edid_vendor[4];
 	struct edid *edid = NULL;
-
+	
 	if (drm_probe_ddc(adapter))
 		edid = (struct edid *)drm_do_get_edid(connector, adapter);
+	
+	/* Log if something has changed */
+	if (edid != NULL) {
+		old_edid = (struct edid *)connector->display_info.raw_edid;
+		/* memcmp call compares the mfg_id, prod_code, and serial
+		 * fields of the two (packed) EDID structures for equality */
+		if (!old_edid || memcmp(&edid->mfg_id, &old_edid->mfg_id, 8)) {
+			DRM_INFO("Detected display on connector %s: "
+				 "mfr: %s, product: %04hx, serial: %d\n",
+				 drm_get_connector_name(connector),
+				 edid_vendor_string(edid, edid_vendor),
+				 le16_to_cpu(EDID_PRODUCT_ID(edid)),
+				 le32_to_cpu(edid->serial));
+		}
+	}
 
 	connector->display_info.raw_edid = (char *)edid;
 
 	return edid;
-
 }
 EXPORT_SYMBOL(drm_get_edid);
 
@@ -401,13 +526,9 @@
  */
 static bool edid_vendor(struct edid *edid, char *vendor)
 {
-	char edid_vendor[3];
-
-	edid_vendor[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
-	edid_vendor[1] = (((edid->mfg_id[0] & 0x3) << 3) |
-			  ((edid->mfg_id[1] & 0xe0) >> 5)) + '@';
-	edid_vendor[2] = (edid->mfg_id[1] & 0x1f) + '@';
+	char edid_vendor[4];
 
+	edid_vendor_string(edid, edid_vendor);
 	return !strncmp(edid_vendor, vendor, 3);
 }
 
@@ -420,17 +541,34 @@
 static u32 edid_get_quirks(struct edid *edid)
 {
 	struct edid_quirk *quirk;
+	u32 quirks;
 	int i;
+	
+	quirks = 0;
 
+	/* Check the built-in quirks first */
 	for (i = 0; i < ARRAY_SIZE(edid_quirk_list); i++) {
 		quirk = &edid_quirk_list[i];
 
 		if (edid_vendor(edid, quirk->vendor) &&
-		    (EDID_PRODUCT_ID(edid) == quirk->product_id))
-			return quirk->quirks;
+		    (EDID_PRODUCT_ID(edid) == quirk->product_id)) {
+			quirks = quirk->quirks;
+			break;
+		}
+	}
+	
+	/* Extra quirks can override built-in ones */
+	for (i = 0; i < edid_num_xtra_quirks; ++i) {
+		quirk = &edid_xtra_quirk_list[i];
+		
+		if (edid_vendor(edid, quirk->vendor) &&
+		    EDID_PRODUCT_ID(edid) == quirk->product_id) {
+			quirks = quirk->quirks;
+			break;
+		}
 	}
 
-	return 0;
+	return quirks;
 }
 
 #define MODE_SIZE(m) ((m)->hdisplay * (m)->vdisplay)
@@ -1562,6 +1700,11 @@
 	int i, hdmi_id;
 	int start_offset, end_offset;
 	bool is_hdmi = false;
+	
+	if (edid_get_quirks(edid) & EDID_QUIRK_DISABLE_INFOFRAMES) {
+		DRM_DEBUG_KMS("Disabling HDMI InfoFrames due to EDID quirk\n");
+		goto end;
+	}
 
 	edid_ext = drm_find_cea_extension(edid);
 	if (!edid_ext)
@@ -1610,6 +1753,11 @@
 	int i, j;
 	bool has_audio = false;
 	int start_offset, end_offset;
+	
+	if (edid_get_quirks(edid) & EDID_QUIRK_NO_AUDIO) {
+		DRM_DEBUG_KMS("Disabling HDMI audio due to EDID quirk\n");
+		goto end;
+	}
 
 	edid_ext = drm_find_cea_extension(edid);
 	if (!edid_ext)
diff -ur linux-3.3/drivers/gpu/drm/drm_stub.c linux-3.3-working/drivers/gpu/drm/drm_stub.c
--- linux-3.3/drivers/gpu/drm/drm_stub.c	2012-03-18 18:15:34.000000000 -0500
+++ linux-3.3-working/drivers/gpu/drm/drm_stub.c	2012-04-24 13:32:35.394132568 -0500
@@ -46,16 +46,22 @@
 unsigned int drm_timestamp_precision = 20;  /* Default to 20 usecs. */
 EXPORT_SYMBOL(drm_timestamp_precision);
 
+char *drm_edid_xtra_quirks = NULL;
+EXPORT_SYMBOL(drm_edid_xtra_quirks);
+
 MODULE_AUTHOR(CORE_AUTHOR);
 MODULE_DESCRIPTION(CORE_DESC);
 MODULE_LICENSE("GPL and additional rights");
 MODULE_PARM_DESC(debug, "Enable debug output");
 MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable [msecs]");
 MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps [usecs]");
+MODULE_PARM_DESC(edid_quirks, "Additional EDID quirks [up to "
+		 __stringify(DRM_EDID_XTRA_QUIRKS_MAX) "]");
 
 module_param_named(debug, drm_debug, int, 0600);
 module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600);
 module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600);
+module_param_named(edid_quirks, drm_edid_xtra_quirks, charp, 0600);
 
 struct idr drm_minors_idr;
 
diff -ur linux-3.3/include/drm/drmP.h linux-3.3-working/include/drm/drmP.h
--- linux-3.3/include/drm/drmP.h	2012-03-18 18:15:34.000000000 -0500
+++ linux-3.3-working/include/drm/drmP.h	2012-04-27 16:09:12.043494619 -0500
@@ -1468,6 +1468,7 @@
 
 extern unsigned int drm_vblank_offdelay;
 extern unsigned int drm_timestamp_precision;
+extern char *drm_edid_xtra_quirks;
 
 extern struct class *drm_class;
 extern struct proc_dir_entry *drm_proc_root;
@@ -1552,6 +1553,12 @@
 void drm_gem_vm_close(struct vm_area_struct *vma);
 int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
 
+				/* EDID support (drm_edid.c) */
+/* This gets stringified in drm_stub.c and drm_edid.c; don't add parentheses */
+#define DRM_EDID_XTRA_QUIRKS_MAX	10
+void drm_edid_xtra_quirks_parse(void);
+void drm_edid_xtra_quirks_free(void);
+
 #include "drm_global.h"
 
 static inline void

[-- 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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Enhancing EDID quirk functionality
  2012-05-03 18:01     ` Ian Pilcher
@ 2012-05-03 19:42       ` Adam Jackson
  2012-05-07 19:50         ` Ian Pilcher
  0 siblings, 1 reply; 32+ messages in thread
From: Adam Jackson @ 2012-05-03 19:42 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: dri-devel

On 5/3/12 2:01 PM, Ian Pilcher wrote:

> The patch does the following:

This looks good, thank you for taking it on.

> * Changes the vendor field of struct edid_quirk to an array, rather
>    than a pointer.  (This has already been sent to the dri-devel list.)
> * Adds two new quirks EDID_QUIRK_DISABLE_INFOFRAMES and
>    EDID_QUIRK_NO_AUDIO. This first quirk causes drm_detect_hdmi_monitor
>    to return false; the second   causes drm_detect_monitor_audio to
>    return false.
> * Logs the EDID vendor and model of connected monitors.
> * Adds an edid_quirks parameter to the drm module, for user-defined
>    quirks.

I'd like to see documentation for the bit values of the quirks as well. 
And, ideally, this would also have some runtime API for manipulating the 
quirk list, so that way you can test new quirks without needing a reboot 
cycle.

To close the loop all the way on that I'd also want to be able to scrape 
the quirk list back out from that API, but that's not completely clean 
right now.  We're being a little cavalier with the quirk list as it 
stands because we don't differentiate among phy layers, and I can easily 
imagine a monitor that needs a quirk on DVI but where the same quirk on 
the same monitors' VGA would break it.  I don't think this has caused 
problems yet, but.

> With this patch, my display works when I add
> drm.edid_quirks=GSM:0x563f:0x180 to my kernel command line.  (0x80,
> EDID_QUIRK_DISABLE_INFOFRAMES, makes it work with the nouveau driver;
> 0x100, EDID_QUIRK_NO_AUDIO, makes it work with the i915 driver.  I.e.,
> the i915 driver is sending audio InfoFrames even when
> drm_detect_hdmi_monitor returns false; bug?)

InfoFrames are not valid for non-HDMI sinks, so yes, I'd call that a bug.

Your original bug report is a little strange, particularly:

https://bugzilla.redhat.com/attachment.cgi?id=572790

Where the EDID for DP-1 appears to be truncated: the "extension" field 
(second byte from the end) is 1 as you'd expect for an HDMI monitor, but 
there's no extension block.  How big of a file do you get from 
/sys/class/drm/*/edid for that port?

- ajax

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Enhancing EDID quirk functionality
  2012-05-03 19:42       ` Adam Jackson
@ 2012-05-07 19:50         ` Ian Pilcher
  2012-05-07 21:38           ` Adam Jackson
  0 siblings, 1 reply; 32+ messages in thread
From: Ian Pilcher @ 2012-05-07 19:50 UTC (permalink / raw)
  To: Adam Jackson; +Cc: dri-devel

On 05/03/2012 02:42 PM, Adam Jackson wrote:
> This looks good, thank you for taking it on.

It was either that or give up on my big display, so ... you're welcome.

> I'd like to see documentation for the bit values of the quirks as well.
> And, ideally, this would also have some runtime API for manipulating the
> quirk list, so that way you can test new quirks without needing a reboot
> cycle.

I agree that the bit values should be documented.  I'm not sure where
that documentation should go, however, since I can't find any
documentation of the existing drm module parameters.  Tell me where it
should go, and I'll happily write the doc.

I also agree that it would be nice to be able to manipulate the quirk
list at runtime, and I did think about trying to enable that.  I held
off for a couple of reasons:

1) I'm a total noob at kernel code, so things like in-kernel locking,
   sysfs, memory management, etc., that would be required for a more
   dynamic API are all new to me.

   That said, I'm more that willing to give it a go, if I can get some
   guidance on those (and similar) topics.

2) I'm not sure how a runtime API should work.  The simplest possibility
   is to just take a string, parse it, and overwrite the old extra
   quirk list with the new list.  The downside to this is that all of
   the existing extra quirks need to be repeated to change a single
   quirk.

> To close the loop all the way on that I'd also want to be able to scrape
> the quirk list back out from that API, but that's not completely clean
> right now.

Sound like a couple of sysfs files to me, one for the built-in quirks
and one for the extra quirks -- maybe one quirk per line?  See my
comments about the sysfs API above.

> We're being a little cavalier with the quirk list as it
> stands because we don't differentiate among phy layers, and I can easily
> imagine a monitor that needs a quirk on DVI but where the same quirk on
> the same monitors' VGA would break it.  I don't think this has caused
> problems yet, but.

Now you're above my pay grade.  What little I've read discovered about
the way DisplayPort, HDMI, VGA, and DVI play together makes me think
this is a nightmare best deferred, hopefully forever.

> InfoFrames are not valid for non-HDMI sinks, so yes, I'd call that a bug.

That's pretty much what I figured.

> Where the EDID for DP-1 appears to be truncated: the "extension" field
> (second byte from the end) is 1 as you'd expect for an HDMI monitor, but
> there's no extension block.  How big of a file do you get from
> /sys/class/drm/*/edid for that port?

The EDID data in sysfs is 256 bytes, which I believe means that it does
include the extension block.

I just tried connecting an HDMI TV to my laptop, and I saw the same
behavior -- 256-byte edid file in sysfs, but "xrandr --verbose" only
shows 128 bytes.  When I attach the same TV to my workstation with Intel
"HD 2000" graphics, "xrandr --verbose" shows all 256 bytes of EDID data.

So it appears that the full data is being read by both systems, but the
behavior of xrandr (or presumably whatever API xrandr uses to get the
EDID data that it displays) differs between the two drivers.  Fun.

Thanks!

-- 
========================================================================
Ian Pilcher                                         arequipeno@gmail.com
"If you're going to shift my paradigm ... at least buy me dinner first."
========================================================================

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Enhancing EDID quirk functionality
  2012-05-07 19:50         ` Ian Pilcher
@ 2012-05-07 21:38           ` Adam Jackson
  2012-08-10  4:23             ` Ian Pilcher
  0 siblings, 1 reply; 32+ messages in thread
From: Adam Jackson @ 2012-05-07 21:38 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 4898 bytes --]

On Mon, 2012-05-07 at 14:50 -0500, Ian Pilcher wrote:
> On 05/03/2012 02:42 PM, Adam Jackson wrote:
> > I'd like to see documentation for the bit values of the quirks as well.
> > And, ideally, this would also have some runtime API for manipulating the
> > quirk list, so that way you can test new quirks without needing a reboot
> > cycle.

I should point out that I don't think any of my objections are
necessarily blockers to getting your work merged.

> I agree that the bit values should be documented.  I'm not sure where
> that documentation should go, however, since I can't find any
> documentation of the existing drm module parameters.  Tell me where it
> should go, and I'll happily write the doc.

Well the standard place is Documentation/kernel-parameters.txt.  Sadly
it appears to only document one drm parameter at the moment...

But there's also Documentation/EDID/ now.

> I also agree that it would be nice to be able to manipulate the quirk
> list at runtime, and I did think about trying to enable that.  I held
> off for a couple of reasons:
> 
> 1) I'm a total noob at kernel code, so things like in-kernel locking,
>    sysfs, memory management, etc., that would be required for a more
>    dynamic API are all new to me.
> 
>    That said, I'm more that willing to give it a go, if I can get some
>    guidance on those (and similar) topics.
>
> 2) I'm not sure how a runtime API should work.  The simplest possibility
>    is to just take a string, parse it, and overwrite the old extra
>    quirk list with the new list.  The downside to this is that all of
>    the existing extra quirks need to be repeated to change a single
>    quirk.

Files in sysfs are probably the least unnatural thing.  It's weird to
express this as an ioctl since the quirk list is global drm state and
the device nodes are, well, per-device.

You can copypasta much of that from the existing stuff in drm_sysfs.c, I
bet.  You'll need to hang the control file off the toplevel class, like
the 'version' file does, and you'll probably need to introduce a new
mutex around the quirk list to handle userspace changing it racing with
the driver reading it.

As far as interface, I'd probably say something like:

# echo "GSM:0x563f:0x180" > /sys/class/drm/edid_quirks

You'd match on the first two fields and then simply clobber the value.
If you don't find an existing match you add a new one.  If you find a
match and the new value is 0, you can delete it.  cat'ing the file gives
you a list of all programmed quirks.  If you wanted a shorthand for
"delete all" you could maybe make a 0-length write do it?  Like:

# echo "" > /sys/class/drm/edid_quirks

> > To close the loop all the way on that I'd also want to be able to scrape
> > the quirk list back out from that API, but that's not completely clean
> > right now.
> 
> Sound like a couple of sysfs files to me, one for the built-in quirks
> and one for the extra quirks -- maybe one quirk per line?  See my
> comments about the sysfs API above.
> 
> > We're being a little cavalier with the quirk list as it
> > stands because we don't differentiate among phy layers, and I can easily
> > imagine a monitor that needs a quirk on DVI but where the same quirk on
> > the same monitors' VGA would break it.  I don't think this has caused
> > problems yet, but.
> 
> Now you're above my pay grade.  What little I've read discovered about
> the way DisplayPort, HDMI, VGA, and DVI play together makes me think
> this is a nightmare best deferred, hopefully forever.

Yeah, I wouldn't worry about it, just noting it for posterity.  We
should try to keep track of this for future quirks but I'm not going to
lose sleep over what we've got.

> > Where the EDID for DP-1 appears to be truncated: the "extension" field
> > (second byte from the end) is 1 as you'd expect for an HDMI monitor, but
> > there's no extension block.  How big of a file do you get from
> > /sys/class/drm/*/edid for that port?
> 
> The EDID data in sysfs is 256 bytes, which I believe means that it does
> include the extension block.
> 
> I just tried connecting an HDMI TV to my laptop, and I saw the same
> behavior -- 256-byte edid file in sysfs, but "xrandr --verbose" only
> shows 128 bytes.  When I attach the same TV to my workstation with Intel
> "HD 2000" graphics, "xrandr --verbose" shows all 256 bytes of EDID data.
> 
> So it appears that the full data is being read by both systems, but the
> behavior of xrandr (or presumably whatever API xrandr uses to get the
> EDID data that it displays) differs between the two drivers.  Fun.

Well thankfully that's just the X nouveau driver being broken, and
shouldn't affect actual output setup.  I could have sworn I'd fixed
nouveau for this before.  I'll send the fix along, thanks for catching
it.

- ajax

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: Enhancing EDID quirk functionality
  2012-05-07 21:38           ` Adam Jackson
@ 2012-08-10  4:23             ` Ian Pilcher
  2012-08-10  4:23               ` [PATCH] drm: EDID quirk improvements Ian Pilcher
  0 siblings, 1 reply; 32+ messages in thread
From: Ian Pilcher @ 2012-08-10  4:23 UTC (permalink / raw)
  To: ajax; +Cc: dri-devel

Well, it took me 4 months to find the time, but I finally had a
crack addressing the ideas you mentioned in your last response.

I believe that the patch in the following note does everything
that you wanted, and hopefully it's all straightforward enough
that it doesn't require a lot of explanation.

I have tested this on both 32- and 64-bit x86 systems, but I
don't have access to any big-endian hardware, so an extra-
careful look at the parts where I swizzle the bytes in the EDID
display ID data is probably warranted.

Please let me know if you have any questions or concerns.

Thanks!

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH] drm: EDID quirk improvements
  2012-08-10  4:23             ` Ian Pilcher
@ 2012-08-10  4:23               ` Ian Pilcher
  2012-08-10 18:44                 ` Ian Pilcher
  0 siblings, 1 reply; 32+ messages in thread
From: Ian Pilcher @ 2012-08-10  4:23 UTC (permalink / raw)
  To: ajax; +Cc: Ian Pilcher, dri-devel

Add ability for user to add or remove EDID quirks, via module
parameter or sysfs.  Also add two new quirk flags --
EDID_QUIRK_DISABLE_INFOFRAMES and EDID_QUIRK_NO_AUDIO -- and adds
a quirk for the LG L246WP display.  Document module parameter and
sysfs interface.
---
 Documentation/EDID/edid_quirks.txt | 161 ++++++++++++
 drivers/gpu/drm/drm_drv.c          |   2 +
 drivers/gpu/drm/drm_edid.c         | 524 +++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/drm_stub.c         |   5 +
 drivers/gpu/drm/drm_sysfs.c        |  19 ++
 include/drm/drmP.h                 |  10 +
 include/drm/drm_edid.h             |  13 +-
 7 files changed, 673 insertions(+), 61 deletions(-)
 create mode 100644 Documentation/EDID/edid_quirks.txt

diff --git a/Documentation/EDID/edid_quirks.txt b/Documentation/EDID/edid_quirks.txt
new file mode 100644
index 0000000..07a0087
--- /dev/null
+++ b/Documentation/EDID/edid_quirks.txt
@@ -0,0 +1,161 @@
+                                  EDID Quirks
+                                 =============
+                       Ian Pilcher <arequipeno@gmail.com>
+                                 August 8, 2012
+
+
+    "EDID blocks out in the wild have a variety of bugs"
+        -- from drivers/gpu/drm/drm_edid.c
+
+
+Overview
+========
+
+EDID quirks provide a mechanism for working around display hardware with buggy
+EDID data.
+
+An individual EDID quirk maps a display type (identified by its EDID
+manufacturer ID and product code[1]) to a set of flags. For example, the current
+list of quirks built into the kernel is:
+
+    ACR:0xad46:0x00000001
+    API:0x7602:0x00000001
+    ACR:0x0977:0x00000020
+    MAX:0x05ec:0x00000001
+    MAX:0x077e:0x00000001
+    EPI:0xe780:0x00000002
+    EPI:0x2028:0x00000001
+    FCM:0x3520:0x0000000c
+    LPL:0x0000:0x00000010
+    LPL:0x2a00:0x00000010
+    PHL:0xe014:0x00000020
+    PTS:0x02fd:0x00000020
+    SAM:0x021d:0x00000040
+    SAM:0x0254:0x00000001
+    SAM:0x027e:0x00000001
+    VSC:0x139c:0x00000080
+    GSM:0x563f:0x00000300
+
+The first field of each quirk is the manufacturer ID, the second field is the
+product code, and the third field is the quirk flags.
+
+NOTE: All of the manufacturer IDs above are displayed as 3-character strings,
+    because they are conformant IDs that have been properly encoded:
+
+    - The most-significant bit of the encoded ID is 0
+    - They only contain ASCII characters in the range A-Z
+
+    IDs that do not conform to these rules are displayed as "raw" hexadecimal
+    values.
+
+The current quirk flags are:
+
+    /* First detailed mode wrong, use largest 60Hz mode */
+    #define EDID_QUIRK_PREFER_LARGE_60                  0x00000001
+
+    /* Reported 135MHz pixel clock is too high, needs adjustment */
+    #define EDID_QUIRK_135_CLOCK_TOO_HIGH               0x00000002
+
+    /* Prefer the largest mode at 75 Hz */
+    #define EDID_QUIRK_PREFER_LARGE_75                  0x00000004
+
+    /* Detail timing is in cm not mm */
+    #define EDID_QUIRK_DETAILED_IN_CM                   0x00000008
+
+    /* Detailed timing descriptors have bogus size values, so just take the
+     * maximum size and use that.
+     */
+    #define EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE        0x00000010
+
+    /* Monitor forgot to set the first detailed is preferred bit. */
+    #define EDID_QUIRK_FIRST_DETAILED_PREFERRED         0x00000020
+
+    /* use +hsync +vsync for detailed mode */
+    #define EDID_QUIRK_DETAILED_SYNC_PP                 0x00000040
+
+    /* Force reduced-blanking timings for detailed modes */
+    #define EDID_QUIRK_FORCE_REDUCED_BLANKING           0x00000080
+
+    /* Display is confused by InfoFrames; don't sent any */
+    #define EDID_QUIRK_DISABLE_INFOFRAMES               0x00000100
+
+    /* Display doesn't have any audio output */
+    #define EDID_QUIRK_NO_AUDIO                         0x00000200
+
+
+sysfs interface
+===============
+
+The current EDID quirk list can be read from /sys/class/drm/edid_quirks.
+
+The number of total "slots" in the list can be read from
+/sys/class/drm/edid_quirks_size.  This total includes both occupied slots (i.e.
+the current list) and any slots available for additional quirks.  The number of
+available slots can be calculated by subtracting the number of quirks in the
+current list from the total number of slots.
+
+If a slot is available, an additional quirk can be added to the list by writing
+it to edid_quirks:
+
+    # echo FOO:0xffff:0x100 > /sys/class/drm/edid_quirks
+
+Manufacturer IDs can also be specified numerically.  (This is the only way to
+specify a nonconformant ID.) This command is equivalent to the previous one:
+
+    # echo 0x19ef:0xffff:0x100 > /sys/class/drm/edid_quirks
+
+Numeric values can also be specified in decimal or octal formats; a number that
+begins with a 0 is assumed to be octal:
+
+    # echo FOO:65535:0400 > /sys/class/drm/edid_quirks
+
+An existing quirk can be replaced by writing a new set of flags:
+
+    # echo FOO:0xffff:0x200 > /sys/class/drm/edid_quirks
+
+A quirk can be deleted from the list by writing an empty flag set (0). This
+makes the list slot occupied by that quirk available.
+
+    # echo FOO:0xffff:0 > /sys/class/drm/edid_quirks
+
+Writing the a single period (.) clears the entire quirk list:
+
+    # echo . > /sys/class/drm/edid_quirks
+
+Multiple changes to the list can be specified in a comma (or newline) separated
+list. For example, the following command clears all of the existing quirks in
+the list and adds 3 new quirks:
+
+    # echo .,FOO:0xffff:0x100,BAR:0x1111:0x001,BAZ:0x2222:0x002 > \
+            /sys/class/drm/edid_quirks
+
+Note however, that any error (an incorrectly formatted quirk or an attempt to
+add a quirk when no slot is available) will abort processing of any further
+changes, potentially making it difficult to determine exactly which change
+caused the error and what changes were made. For this reason, making changes
+one at a time is recommended, particularly if the changes are being made by a
+script or program.
+
+
+Module parameter
+================
+
+The EDID quirk list can also be modified via the edid_quirks module parameter
+(drm.edid_quirks on the kernel command line). The effect of setting this
+parameter is identical to the effect of writing its value to
+/sys/class/drm/edid_quirks, with one important difference. When an error is
+encountered during module parameter parsing or processing, any remaining quirks
+in the parameter string will still be processed. (It is hoped that this approach
+maximizes the probability of producing a working display.)
+
+
+Follow-up
+=========
+
+If you encounter a display that requires an additional EDID quirk in order to
+function properly, please report it to the direct rendering development mailing
+list <dri-devel@lists.freedesktop.org>.
+
+
+[1] See http://en.wikipedia.org/wiki/Extended_display_identification_data for a
+    description of the manufacturer ID and product code fields.
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 9238de4..7fe39e0 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -276,6 +276,8 @@ static int __init drm_core_init(void)
 		goto err_p3;
 	}
 
+	drm_edid_quirks_param_process();
+
 	DRM_INFO("Initialized %s %d.%d.%d %s\n",
 		 CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
 	return 0;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a8743c3..6a5280a 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -31,6 +31,7 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/ctype.h>
 #include "drmP.h"
 #include "drm_edid.h"
 #include "drm_edid_modes.h"
@@ -68,6 +69,15 @@
 #define EDID_QUIRK_DETAILED_SYNC_PP		(1 << 6)
 /* Force reduced-blanking timings for detailed modes */
 #define EDID_QUIRK_FORCE_REDUCED_BLANKING	(1 << 7)
+/* Display is confused by InfoFrames; don't send any */
+#define EDID_QUIRK_DISABLE_INFOFRAMES		(1 << 8)
+/* Display doesn't have any audio output */
+#define EDID_QUIRK_NO_AUDIO			(1 << 9)
+
+/*
+ * When adding additional quirk flags, please update
+ * Documentation/EDID/edid_quirks.txt.
+ */
 
 struct detailed_mode_closure {
 	struct drm_connector *connector;
@@ -82,51 +92,457 @@ struct detailed_mode_closure {
 #define LEVEL_GTF2	2
 #define LEVEL_CVT	3
 
-static struct edid_quirk {
-	char vendor[4];
-	int product_id;
-	u32 quirks;
-} edid_quirk_list[] = {
+union edid_quirk {
+	struct {
+		union edid_display_id display_id;
+		u32 quirks;
+	} __attribute__((packed)) s;
+	u64 u;
+};
+
+#define EDID_MFG_ID(c1, c2, c3)		cpu_to_be16(			\
+						(c1 & 0x1f) << 10 |	\
+						(c2 & 0x1f) << 5 |	\
+						(c3 & 0x1f)		\
+					)
+
+#define EDID_QUIRK_LIST_SIZE	24
+
+union edid_quirk edid_quirk_list[EDID_QUIRK_LIST_SIZE] = {
+
 	/* Acer AL1706 */
-	{ "ACR", 44358, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('A', 'C', 'R'), cpu_to_le16(44358) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 	/* Acer F51 */
-	{ "API", 0x7602, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('A', 'P', 'I'), cpu_to_le16(0x7602) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 	/* Unknown Acer */
-	{ "ACR", 2423, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+	{ { { { EDID_MFG_ID('A', 'C', 'R'), cpu_to_le16(2423) } },
+		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
 
 	/* Belinea 10 15 55 */
-	{ "MAX", 1516, EDID_QUIRK_PREFER_LARGE_60 },
-	{ "MAX", 0x77e, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('M', 'A', 'X'), cpu_to_le16(1516) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
+	{ { { { EDID_MFG_ID('M', 'A', 'X'), cpu_to_le16(0x77e) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 
 	/* Envision Peripherals, Inc. EN-7100e */
-	{ "EPI", 59264, EDID_QUIRK_135_CLOCK_TOO_HIGH },
+	{ { { { EDID_MFG_ID('E', 'P', 'I'), cpu_to_le16(59264) } },
+		EDID_QUIRK_135_CLOCK_TOO_HIGH } },
 	/* Envision EN2028 */
-	{ "EPI", 8232, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('E', 'P', 'I'), cpu_to_le16(8232) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 
 	/* Funai Electronics PM36B */
-	{ "FCM", 13600, EDID_QUIRK_PREFER_LARGE_75 |
-	  EDID_QUIRK_DETAILED_IN_CM },
+	{ { { { EDID_MFG_ID('F', 'C', 'M'), cpu_to_le16(13600) } },
+		EDID_QUIRK_PREFER_LARGE_75 | EDID_QUIRK_DETAILED_IN_CM } },
 
 	/* LG Philips LCD LP154W01-A5 */
-	{ "LPL", 0, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE },
-	{ "LPL", 0x2a00, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE },
+	{ { { { EDID_MFG_ID('L', 'P', 'L'), cpu_to_le16(0) } },
+		EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE } },
+	{ { { { EDID_MFG_ID('L', 'P', 'L'), cpu_to_le16(0x2a00) } },
+		EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE } },
 
 	/* Philips 107p5 CRT */
-	{ "PHL", 57364, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+	{ { { { EDID_MFG_ID('P', 'H', 'L'), cpu_to_le16(57364) } },
+		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
 
 	/* Proview AY765C */
-	{ "PTS", 765, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+	{ { { { EDID_MFG_ID('P', 'T', 'S'), cpu_to_le16(765) } },
+		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
 
 	/* Samsung SyncMaster 205BW.  Note: irony */
-	{ "SAM", 541, EDID_QUIRK_DETAILED_SYNC_PP },
+	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(541) } },
+		EDID_QUIRK_DETAILED_SYNC_PP } },
 	/* Samsung SyncMaster 22[5-6]BW */
-	{ "SAM", 596, EDID_QUIRK_PREFER_LARGE_60 },
-	{ "SAM", 638, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(596) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
+	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(638) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 
 	/* ViewSonic VA2026w */
-	{ "VSC", 5020, EDID_QUIRK_FORCE_REDUCED_BLANKING },
+	{ { { { EDID_MFG_ID('V', 'S', 'C'), cpu_to_le16(5020) } },
+		EDID_QUIRK_FORCE_REDUCED_BLANKING } },
+
+	/* LG L246WP */
+	{ { { { EDID_MFG_ID('G', 'S', 'M'), cpu_to_le16(0x563f) } },
+		EDID_QUIRK_DISABLE_INFOFRAMES | EDID_QUIRK_NO_AUDIO } },
+
+	/*
+	 * When adding built-in quirks, please adjust EDID_QUIRK_LIST_SIZE to
+	 * provide some room for user-supplied quirks.
+	 */
 };
 
+DEFINE_MUTEX(edid_quirk_list_mutex);
+
+/**
+ * drm_edid_mfg_format - format an "encoded" EDID manufacturer ID for printing
+ * @mfg_id: the encoded manufacturer ID
+ * @buf: destination buffer for the formated manufacturer ID (minimum 7 bytes)
+ * @strip: if non-zero, the returned pointer will skip any leading spaces
+ *
+ * An EDID manufacturer ID is supposed to consist of 3 capital letters (A-Z).
+ * Each letter is stored as a 5-bit value between 1 and 26, taking up 15 bits of
+ * the 16-bit ID. The remaining bit should always be 0. If display manufacturers
+ * always did things correctly, however, EDID quirks wouldn't be required in
+ * the first place. This function does the following:
+ *
+ * - Broken IDs are printed in hexadecimal (0xffff).
+ * - "Correct" IDs are formatted as a 3-letter ID string, preceded by 3 spaces;
+ *   the spaces ensure that both output formats are the same length.
+ *
+ * Thus, a formatted manufacturer ID is always 6 characters long (not including
+ * the terminating 0).
+ *
+ * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
+ * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
+ * been formatted as a 3-letter string, a pointer to the first non-space
+ * character (@buf + 3) is returned.
+ */
+static const char *drm_edid_mfg_format(__be16 mfg_id, char *buf, int strip)
+{
+	u16 id = be16_to_cpu(mfg_id);
+
+	if (id & 0x8000)
+		goto bad_id;
+
+	buf[3] = ((id & 0x7c00) >> 10) + '@';
+	if (!isupper(buf[3]))
+		goto bad_id;
+
+	buf[4] = ((id & 0x03e0) >> 5) + '@';
+	if (!isupper(buf[4]))
+		goto bad_id;
+
+	buf[5] = (id & 0x001f) + '@';
+	if (!isupper(buf[5]))
+		goto bad_id;
+
+	memset(buf, ' ', 3);
+	buf[6] = 0;
+
+	return strip ? (buf + 3) : buf;
+
+bad_id:
+	sprintf(buf, "0x%04hx", id);
+	return buf;
+}
+
+#define EDID_MFG_BUF_SIZE		7
+
+/**
+ * drm_edid_display_id_format - format an EDID "display ID" (manufacturer ID
+ *				and product code) for printing
+ * @display_id: the display ID
+ * @buf: destination buffer for the formatted display ID (minimum 14 bytes)
+ * @strip: if non-zero, the returned pointer will skip any leading spaces
+ *
+ * A formatted display ID is always 13 characters long (not including the
+ * terminating 0).
+ *
+ * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
+ * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
+ * been formatted as a 3-letter string, a pointer to the first non-space
+ * character (@buf + 3) is returned.
+ */
+static const char *drm_edid_display_id_format(union edid_display_id display_id,
+					      char *buf, int strip)
+{
+	const char *s;
+
+	s = drm_edid_mfg_format(display_id.s.mfg_id, buf, strip);
+	sprintf(buf + EDID_MFG_BUF_SIZE - 1, ":0x%04hx",
+		le16_to_cpu(display_id.s.prod_code));
+
+	return s;
+}
+
+#define EDID_DISPLAY_ID_BUF_SIZE	(EDID_MFG_BUF_SIZE + 7)
+
+/**
+ * drm_edid_quirk_format - format an EDID quirk for printing
+ * @quirk: the quirk
+ * @buf: destination buffer for the formatted quirk (minimum 25 bytes)
+ * @strip: if non-zero, the returned pointer will skip any leading spaces
+ *
+ * A formatted EDID quirk is always 24 characters long (not including the
+ * terminating 0).
+ *
+ * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
+ * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
+ * been formatted as a 3-letter string, a pointer to the first non-space
+ * character (@buf + 3) is returned.
+ */
+static const char *drm_edid_quirk_format(const union edid_quirk *quirk,
+					 char *buf, int strip)
+{
+	const char *s;
+
+	s = drm_edid_display_id_format(quirk->s.display_id, buf, strip);
+	sprintf(buf + EDID_DISPLAY_ID_BUF_SIZE - 1, ":0x%08x", quirk->s.quirks);
+
+	return s;
+}
+
+#define EDID_QUIRK_BUF_SIZE		(EDID_DISPLAY_ID_BUF_SIZE + 11)
+
+/**
+ * drm_edid_quirk_parse - parse an EDID quirk
+ * @s: string containing the quirk to be parsed
+ * @quirk: destination for parsed quirk
+ *
+ * Returns 0 on success, < 0 (currently -EINVAL) on error.
+ */
+static int drm_edid_quirk_parse(const char *s, union edid_quirk *quirk)
+{
+	char buf[EDID_QUIRK_BUF_SIZE];
+	s32 mfg;
+	s32 product;
+	s64 quirks;
+	char *c;
+
+	if (sscanf(s, "%i:%i:%lli", &mfg, &product, &quirks) == 3) {
+		if (mfg < 0 || mfg > 0xffff)
+			goto error;
+		quirk->s.display_id.s.mfg_id = cpu_to_be16((u16)mfg);
+	} else {
+		if (sscanf(s, "%3s:%i:%lli", buf, &product, &quirks) != 3 ||
+				!isupper(buf[0]) ||
+				!isupper(buf[1]) ||
+				!isupper(buf[2]))
+			goto error;
+		quirk->s.display_id.s.mfg_id =
+				EDID_MFG_ID(buf[0], buf[1], buf[2]);
+	}
+
+	if (product < 0 || product > 0xffff ||
+			quirks < 0 || quirks > 0xffffffffLL)
+		goto error;
+
+	quirk->s.display_id.s.prod_code = cpu_to_le16((u16)product);
+	quirk->s.quirks = (u32)quirks;
+
+	DRM_DEBUG("Successfully parsed EDID quirk: %s\n",
+		  drm_edid_quirk_format(quirk, buf, 1));
+
+	return 0;
+
+error:
+	c = strpbrk(s, ",\n");
+	if (c == NULL) {
+		printk(KERN_WARNING "Invalid EDID quirk: '%s'\n", s);
+	} else {
+		printk(KERN_WARNING "Invalid EDID quirk: '%.*s'\n",
+		      (int)(c - s), s);
+	}
+
+	return -EINVAL;
+}
+
+/**
+ * drm_edid_quirk_find_by_id - find the EDID quirk matching a display ID
+ * @display_id: the display ID to match
+ *
+ * Caller MUST hold edid_quirk_list_mutex.
+ *
+ * Returns a pointer to the matching quirk list entry, NULL if no such entry
+ * exists.
+ */
+static union edid_quirk *drm_edid_quirk_find_by_id(union edid_display_id id)
+{
+	union edid_quirk *q = edid_quirk_list;
+
+	do {
+		if (q->s.display_id.u == id.u && q->s.quirks != 0)
+			return q;
+	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
+
+	return NULL;
+}
+
+/**
+ * drm_edid_quirk_find_slot - find an empty slot in the EDID quirk list
+ *
+ * Caller MUST hold edid_quirk_list_mutex.
+ *
+ * Returns a pointer to the first empty slot, NULL if no empty slots exist.
+ */
+static union edid_quirk *drm_edid_quirk_find_empty(void)
+{
+	union edid_quirk *q = edid_quirk_list;
+
+	do {
+		if (q->s.quirks == 0)
+			return q;
+	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
+
+	return NULL;
+}
+
+/**
+ * drm_edid_quirk_process - process a newly parsed EDID quirk
+ * @quirk: the quirk to be processed
+ *
+ * Depending on the newly parsed quirk and the contents of the quirks list, this
+ * function will clear the quirks list, remove a single quirk from the list, add
+ * a new quirk to the list, or replace an existing quirk.
+ *
+ * Returns 0 on success, < 0 on error (-ENOSPC if there is no free slot for a
+ * new quirk). Note that trying to remove a quirk that isn't present is not
+ * considered an error.
+ */
+static int drm_edid_quirk_process(const union edid_quirk *quirk)
+{
+	char buf[EDID_QUIRK_BUF_SIZE];
+	union edid_quirk *q;
+	int res = 0;
+
+	mutex_lock(&edid_quirk_list_mutex);
+
+	if (quirk->s.quirks == 0) {
+		DRM_INFO("Removing EDID quirk for display %s\n",
+			 drm_edid_display_id_format(quirk->s.display_id,
+						    buf, 1));
+		q = drm_edid_quirk_find_by_id(quirk->s.display_id);
+		if (q == NULL) {
+			printk(KERN_WARNING "No quirk found for display %s\n",
+			       drm_edid_display_id_format(quirk->s.display_id,
+							  buf, 1));
+		} else {
+			q->u = 0;
+		}
+	} else {
+		DRM_INFO("Adding EDID quirk: %s\n",
+			 drm_edid_quirk_format(quirk, buf, 1));
+		q = drm_edid_quirk_find_by_id(quirk->s.display_id);
+		if (q == NULL) {
+			q = drm_edid_quirk_find_empty();
+			if (q == NULL) {
+				printk(KERN_WARNING
+				       "No free slot in EDID quirk list\n");
+				res = -ENOSPC;
+			} else {
+				q->u = quirk->u;
+			}
+		} else {
+			DRM_INFO("Replacing existing quirk: %s\n",
+				 drm_edid_quirk_format(q, buf, 1));
+			q->s.quirks = quirk->s.quirks;
+		}
+	}
+
+	mutex_unlock(&edid_quirk_list_mutex);
+
+	return res;
+}
+
+/**
+ * drm_edid_quirks_process - parse and process a comma separated list of EDID
+ *			     quirks
+ * @s: string containing the quirks to be processed
+ * @strict: if non-zero, any parsing or processing error aborts further
+ *	    processing
+ *
+ * Returns 0 on success, < 0 if any error is encountered.  (If multiple errors
+ * occur when strict is set to 0, the last error encountered is returned.)
+ */
+static int drm_edid_quirks_process(const char *s, int strict)
+{
+	union edid_quirk quirk;
+	int res = 0;
+
+	do {
+
+		if (*s == '.') {
+			DRM_INFO("Clearing EDID quirk list\n");
+			mutex_lock(&edid_quirk_list_mutex);
+			memset(edid_quirk_list, 0, sizeof edid_quirk_list);
+			mutex_unlock(&edid_quirk_list_mutex);
+		} else {
+			res = drm_edid_quirk_parse(s, &quirk);
+			if (res != 0) {
+				if (strict)
+					goto error;
+				continue;
+			}
+
+			res = drm_edid_quirk_process(&quirk);
+			if (res != 0) {
+				if (strict)
+					goto error;
+			}
+		}
+
+		s = strpbrk(s, ",\n");
+
+	} while (s != NULL && *(++s) != 0);
+
+	return res;
+
+error:
+	printk(KERN_WARNING "Aborting EDID quirk parsing\n");
+	return res;
+}
+
+/**
+ * drm_edid_quirks_param_process - process the edid_quirks module parameter
+ */
+void drm_edid_quirks_param_process(void)
+{
+	if (drm_edid_quirks != NULL)
+		drm_edid_quirks_process(drm_edid_quirks, 0);
+}
+
+/**
+ * drm_edid_quirks_size_show - show the size of the EDID quirk list in sysfs
+ * @buf: destination buffer (PAGE_SIZE bytes)
+ */
+ssize_t drm_edid_quirks_size_show(struct class *class,
+				  struct class_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%zu\n", ARRAY_SIZE(edid_quirk_list));
+}
+
+/**
+ * drm_edid_quirks_show - show the contents of the EDID quirk list in sysfs
+ * @buf: destination buffer (PAGE_SIZE bytes)
+ */
+ssize_t drm_edid_quirks_show(struct class *class, struct class_attribute *attr,
+			     char *buf)
+{
+	const union edid_quirk *q = edid_quirk_list;
+	ssize_t count = 0;
+
+	BUILD_BUG_ON(ARRAY_SIZE(edid_quirk_list) >
+				PAGE_SIZE / EDID_QUIRK_BUF_SIZE);
+
+	mutex_lock(&edid_quirk_list_mutex);
+
+	do {
+		if (q->s.quirks != 0) {
+			drm_edid_quirk_format(q, buf + count, 0);
+			(buf + count)[EDID_QUIRK_BUF_SIZE - 1] = '\n';
+			count += EDID_QUIRK_BUF_SIZE;
+		}
+	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
+
+	mutex_unlock(&edid_quirk_list_mutex);
+
+	return count;
+}
+
+ssize_t drm_edid_quirks_store(struct class *class, struct class_attribute *attr,
+			      const char *buf, size_t count)
+{
+	int res;
+
+	res = drm_edid_quirks_process(buf, 1);
+	if (res != 0)
+		return res;
+
+	return count;
+}
 /*** DDC fetch and block validation ***/
 
 static const u8 edid_header[] = {
@@ -409,25 +825,6 @@ EXPORT_SYMBOL(drm_get_edid);
 /*** EDID parsing ***/
 
 /**
- * edid_vendor - match a string against EDID's obfuscated vendor field
- * @edid: EDID to match
- * @vendor: vendor string
- *
- * Returns true if @vendor is in @edid, false otherwise
- */
-static bool edid_vendor(struct edid *edid, char *vendor)
-{
-	char edid_vendor[3];
-
-	edid_vendor[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
-	edid_vendor[1] = (((edid->mfg_id[0] & 0x3) << 3) |
-			  ((edid->mfg_id[1] & 0xe0) >> 5)) + '@';
-	edid_vendor[2] = (edid->mfg_id[1] & 0x1f) + '@';
-
-	return !strncmp(edid_vendor, vendor, 3);
-}
-
-/**
  * edid_get_quirks - return quirk flags for a given EDID
  * @edid: EDID to process
  *
@@ -435,18 +832,18 @@ static bool edid_vendor(struct edid *edid, char *vendor)
  */
 static u32 edid_get_quirks(struct edid *edid)
 {
-	struct edid_quirk *quirk;
-	int i;
+	union edid_quirk *q;
+	u32 quirks = 0;
 
-	for (i = 0; i < ARRAY_SIZE(edid_quirk_list); i++) {
-		quirk = &edid_quirk_list[i];
+	mutex_lock(&edid_quirk_list_mutex);
 
-		if (edid_vendor(edid, quirk->vendor) &&
-		    (EDID_PRODUCT_ID(edid) == quirk->product_id))
-			return quirk->quirks;
-	}
+	q = drm_edid_quirk_find_by_id(edid->display_id);
+	if (q != NULL)
+		quirks = q->s.quirks;
 
-	return 0;
+	mutex_unlock(&edid_quirk_list_mutex);
+
+	return quirks;
 }
 
 #define MODE_SIZE(m) ((m)->hdisplay * (m)->vdisplay)
@@ -1162,7 +1559,7 @@ do_inferred_modes(struct detailed_timing *timing, void *c)
 	closure->modes += drm_dmt_modes_for_range(closure->connector,
 						  closure->edid,
 						  timing);
-	
+
 	if (!version_greater(closure->edid, 1, 1))
 		return; /* GTF not defined yet */
 
@@ -1399,7 +1796,7 @@ do_cvt_mode(struct detailed_timing *timing, void *c)
 
 static int
 add_cvt_modes(struct drm_connector *connector, struct edid *edid)
-{	
+{
 	struct detailed_mode_closure closure = {
 		connector, edid, 0, 0, 0
 	};
@@ -1615,15 +2012,12 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 
 	eld[0] = 2 << 3;		/* ELD version: 2 */
 
-	eld[16] = edid->mfg_id[0];
-	eld[17] = edid->mfg_id[1];
-	eld[18] = edid->prod_code[0];
-	eld[19] = edid->prod_code[1];
+	*(u32 *)(&eld[16]) = edid->display_id.u;
 
 	if (cea[1] >= 3)
 		for (db = cea + 4; db < cea + cea[2]; db += dbl + 1) {
 			dbl = db[0] & 0x1f;
-			
+
 			switch ((db[0] & 0xe0) >> 5) {
 			case AUDIO_BLOCK:
 				/* Audio Data Block, contains SADs */
@@ -1723,6 +2117,14 @@ bool drm_detect_hdmi_monitor(struct edid *edid)
 	int i, hdmi_id;
 	int start_offset, end_offset;
 	bool is_hdmi = false;
+	char buf[EDID_DISPLAY_ID_BUF_SIZE];
+
+	if (edid_get_quirks(edid) & EDID_QUIRK_DISABLE_INFOFRAMES) {
+		DRM_INFO("Disabling HDMI InfoFrames on display %s "
+			 "due to EDID quirk\n",
+			 drm_edid_display_id_format(edid->display_id, buf, 1));
+		goto end;
+	}
 
 	edid_ext = drm_find_cea_extension(edid);
 	if (!edid_ext)
@@ -1771,6 +2173,14 @@ bool drm_detect_monitor_audio(struct edid *edid)
 	int i, j;
 	bool has_audio = false;
 	int start_offset, end_offset;
+	char buf[EDID_DISPLAY_ID_BUF_SIZE];
+
+	if (edid_get_quirks(edid) & EDID_QUIRK_NO_AUDIO) {
+		DRM_INFO("Disabling HDMI audio on display %s "
+			 "due to EDID quirk\n",
+			 drm_edid_display_id_format(edid->display_id, buf, 1));
+		goto end;
+	}
 
 	edid_ext = drm_find_cea_extension(edid);
 	if (!edid_ext)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 21bcd4a..1885fc9 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -46,16 +46,21 @@ EXPORT_SYMBOL(drm_vblank_offdelay);
 unsigned int drm_timestamp_precision = 20;  /* Default to 20 usecs. */
 EXPORT_SYMBOL(drm_timestamp_precision);
 
+char *drm_edid_quirks = NULL;
+EXPORT_SYMBOL(drm_edid_quirks);
+
 MODULE_AUTHOR(CORE_AUTHOR);
 MODULE_DESCRIPTION(CORE_DESC);
 MODULE_LICENSE("GPL and additional rights");
 MODULE_PARM_DESC(debug, "Enable debug output");
 MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable [msecs]");
 MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps [usecs]");
+MODULE_PARM_DESC(edid_quirks, "See Documentation/EDID/edid_quirks.txt");
 
 module_param_named(debug, drm_debug, int, 0600);
 module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600);
 module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600);
+module_param_named(edid_quirks, drm_edid_quirks, charp, 0400);
 
 struct idr drm_minors_idr;
 
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 45ac8d6..84dc365 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -84,6 +84,11 @@ static CLASS_ATTR_STRING(version, S_IRUGO,
 		__stringify(CORE_PATCHLEVEL) " "
 		CORE_DATE);
 
+static CLASS_ATTR(edid_quirks_size, 0400, drm_edid_quirks_size_show, 0);
+
+static CLASS_ATTR(edid_quirks, 0600, drm_edid_quirks_show,
+		  drm_edid_quirks_store);
+
 /**
  * drm_sysfs_create - create a struct drm_sysfs_class structure
  * @owner: pointer to the module that is to "own" this struct drm_sysfs_class
@@ -113,10 +118,22 @@ struct class *drm_sysfs_create(struct module *owner, char *name)
 	if (err)
 		goto err_out_class;
 
+	err = class_create_file(class, &class_attr_edid_quirks_size);
+	if (err)
+		goto err_out_version;
+
+	err = class_create_file(class, &class_attr_edid_quirks);
+	if (err)
+		goto err_out_quirks_size;
+
 	class->devnode = drm_devnode;
 
 	return class;
 
+err_out_quirks_size:
+	class_remove_file(class, &class_attr_edid_quirks_size);
+err_out_version:
+	class_remove_file(class, &class_attr_version.attr);
 err_out_class:
 	class_destroy(class);
 err_out:
@@ -132,6 +149,8 @@ void drm_sysfs_destroy(void)
 {
 	if ((drm_class == NULL) || (IS_ERR(drm_class)))
 		return;
+	class_remove_file(drm_class, &class_attr_edid_quirks);
+	class_remove_file(drm_class, &class_attr_edid_quirks_size);
 	class_remove_file(drm_class, &class_attr_version.attr);
 	class_destroy(drm_class);
 	drm_class = NULL;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d6b67bb..c947f3e 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1501,6 +1501,7 @@ extern unsigned int drm_debug;
 
 extern unsigned int drm_vblank_offdelay;
 extern unsigned int drm_timestamp_precision;
+extern char *drm_edid_quirks;
 
 extern struct class *drm_class;
 extern struct proc_dir_entry *drm_proc_root;
@@ -1612,6 +1613,15 @@ void drm_gem_vm_open(struct vm_area_struct *vma);
 void drm_gem_vm_close(struct vm_area_struct *vma);
 int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
 
+					/* EDID support (drm_edid.c) */
+void drm_edid_quirks_param_process(void);
+ssize_t drm_edid_quirks_size_show(struct class *class,
+				  struct class_attribute *attr, char *buf);
+ssize_t drm_edid_quirks_show(struct class *class, struct class_attribute *attr,
+			     char *buf);
+ssize_t drm_edid_quirks_store(struct class *class, struct class_attribute *attr,
+			      const char *buf, size_t count);
+
 #include "drm_global.h"
 
 static inline void
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 0cac551..713229b 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -202,11 +202,18 @@ struct detailed_timing {
 #define DRM_EDID_FEATURE_PM_SUSPEND       (1 << 6)
 #define DRM_EDID_FEATURE_PM_STANDBY       (1 << 7)
 
+union edid_display_id {
+	struct {
+		__be16 mfg_id;
+		__le16 prod_code;
+	} __attribute__((packed)) s;
+	u32 u;
+};
+
 struct edid {
 	u8 header[8];
 	/* Vendor & product info */
-	u8 mfg_id[2];
-	u8 prod_code[2];
+	union edid_display_id display_id;
 	u32 serial; /* FIXME: byte order */
 	u8 mfg_week;
 	u8 mfg_year;
@@ -242,8 +249,6 @@ struct edid {
 	u8 checksum;
 } __attribute__((packed));
 
-#define EDID_PRODUCT_ID(e) ((e)->prod_code[0] | ((e)->prod_code[1] << 8))
-
 struct drm_encoder;
 struct drm_connector;
 struct drm_display_mode;
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH] drm: EDID quirk improvements
  2012-08-10  4:23               ` [PATCH] drm: EDID quirk improvements Ian Pilcher
@ 2012-08-10 18:44                 ` Ian Pilcher
  2012-08-10 18:44                   ` Ian Pilcher
  0 siblings, 1 reply; 32+ messages in thread
From: Ian Pilcher @ 2012-08-10 18:44 UTC (permalink / raw)
  To: ajax; +Cc: dri-devel

OK, so using a period as the "magic" character to clear the list
turns out to be a disastrously bad idea.  (It works great everywhere
except on the kernel command line.)

New patch coming that uses at sign (@) instead.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH] drm: EDID quirk improvements
  2012-08-10 18:44                 ` Ian Pilcher
@ 2012-08-10 18:44                   ` Ian Pilcher
  2012-08-11  8:31                     ` Paul Menzel
  2012-08-11 19:37                     ` Paul Menzel
  0 siblings, 2 replies; 32+ messages in thread
From: Ian Pilcher @ 2012-08-10 18:44 UTC (permalink / raw)
  To: ajax; +Cc: Ian Pilcher, dri-devel

Add ability for user to add or remove EDID quirks, via module
parameter or sysfs.  Also add two new quirk flags --
EDID_QUIRK_DISABLE_INFOFRAMES and EDID_QUIRK_NO_AUDIO -- and adds
a quirk for the LG L246WP display.  Document module parameter and
sysfs interface.
---
 Documentation/EDID/edid_quirks.txt | 161 +++++++++++
 drivers/gpu/drm/drm_drv.c          |   2 +
 drivers/gpu/drm/drm_edid.c         | 527 +++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/drm_stub.c         |   5 +
 drivers/gpu/drm/drm_sysfs.c        |  19 ++
 include/drm/drmP.h                 |  10 +
 include/drm/drm_edid.h             |  13 +-
 7 files changed, 676 insertions(+), 61 deletions(-)
 create mode 100644 Documentation/EDID/edid_quirks.txt

diff --git a/Documentation/EDID/edid_quirks.txt b/Documentation/EDID/edid_quirks.txt
new file mode 100644
index 0000000..256ded0
--- /dev/null
+++ b/Documentation/EDID/edid_quirks.txt
@@ -0,0 +1,161 @@
+                                  EDID Quirks
+                                 =============
+                       Ian Pilcher <arequipeno@gmail.com>
+                                 August 8, 2012
+
+
+    "EDID blocks out in the wild have a variety of bugs"
+        -- from drivers/gpu/drm/drm_edid.c
+
+
+Overview
+========
+
+EDID quirks provide a mechanism for working around display hardware with buggy
+EDID data.
+
+An individual EDID quirk maps a display type (identified by its EDID
+manufacturer ID and product code[1]) to a set of flags. For example, the current
+list of quirks built into the kernel is:
+
+    ACR:0xad46:0x00000001
+    API:0x7602:0x00000001
+    ACR:0x0977:0x00000020
+    MAX:0x05ec:0x00000001
+    MAX:0x077e:0x00000001
+    EPI:0xe780:0x00000002
+    EPI:0x2028:0x00000001
+    FCM:0x3520:0x0000000c
+    LPL:0x0000:0x00000010
+    LPL:0x2a00:0x00000010
+    PHL:0xe014:0x00000020
+    PTS:0x02fd:0x00000020
+    SAM:0x021d:0x00000040
+    SAM:0x0254:0x00000001
+    SAM:0x027e:0x00000001
+    VSC:0x139c:0x00000080
+    GSM:0x563f:0x00000300
+
+The first field of each quirk is the manufacturer ID, the second field is the
+product code, and the third field is the quirk flags.
+
+NOTE: All of the manufacturer IDs above are displayed as 3-character strings,
+    because they are conformant IDs that have been properly encoded:
+
+    - The most-significant bit of the encoded ID is 0
+    - They only contain ASCII characters in the range A-Z
+
+    IDs that do not conform to these rules are displayed as "raw" hexadecimal
+    values.
+
+The current quirk flags are:
+
+    /* First detailed mode wrong, use largest 60Hz mode */
+    #define EDID_QUIRK_PREFER_LARGE_60                  0x00000001
+
+    /* Reported 135MHz pixel clock is too high, needs adjustment */
+    #define EDID_QUIRK_135_CLOCK_TOO_HIGH               0x00000002
+
+    /* Prefer the largest mode at 75 Hz */
+    #define EDID_QUIRK_PREFER_LARGE_75                  0x00000004
+
+    /* Detail timing is in cm not mm */
+    #define EDID_QUIRK_DETAILED_IN_CM                   0x00000008
+
+    /* Detailed timing descriptors have bogus size values, so just take the
+     * maximum size and use that.
+     */
+    #define EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE        0x00000010
+
+    /* Monitor forgot to set the first detailed is preferred bit. */
+    #define EDID_QUIRK_FIRST_DETAILED_PREFERRED         0x00000020
+
+    /* use +hsync +vsync for detailed mode */
+    #define EDID_QUIRK_DETAILED_SYNC_PP                 0x00000040
+
+    /* Force reduced-blanking timings for detailed modes */
+    #define EDID_QUIRK_FORCE_REDUCED_BLANKING           0x00000080
+
+    /* Display is confused by InfoFrames; don't sent any */
+    #define EDID_QUIRK_DISABLE_INFOFRAMES               0x00000100
+
+    /* Display doesn't have any audio output */
+    #define EDID_QUIRK_NO_AUDIO                         0x00000200
+
+
+sysfs interface
+===============
+
+The current EDID quirk list can be read from /sys/class/drm/edid_quirks.
+
+The number of total "slots" in the list can be read from
+/sys/class/drm/edid_quirks_size.  This total includes both occupied slots (i.e.
+the current list) and any slots available for additional quirks.  The number of
+available slots can be calculated by subtracting the number of quirks in the
+current list from the total number of slots.
+
+If a slot is available, an additional quirk can be added to the list by writing
+it to edid_quirks:
+
+    # echo FOO:0xffff:0x100 > /sys/class/drm/edid_quirks
+
+Manufacturer IDs can also be specified numerically.  (This is the only way to
+specify a nonconformant ID.) This command is equivalent to the previous one:
+
+    # echo 0x19ef:0xffff:0x100 > /sys/class/drm/edid_quirks
+
+Numeric values can also be specified in decimal or octal formats; a number that
+begins with a 0 is assumed to be octal:
+
+    # echo FOO:65535:0400 > /sys/class/drm/edid_quirks
+
+An existing quirk can be replaced by writing a new set of flags:
+
+    # echo FOO:0xffff:0x200 > /sys/class/drm/edid_quirks
+
+A quirk can be deleted from the list by writing an empty flag set (0). This
+makes the list slot occupied by that quirk available.
+
+    # echo FOO:0xffff:0 > /sys/class/drm/edid_quirks
+
+Writing an "at symbol" (@) clears the entire quirk list:
+
+    # echo @ > /sys/class/drm/edid_quirks
+
+Multiple changes to the list can be specified in a comma (or newline) separated
+list. For example, the following command clears all of the existing quirks in
+the list and adds 3 new quirks:
+
+    # echo @,FOO:0xffff:0x100,BAR:0x1111:0x001,BAZ:0x2222:0x002 > \
+            /sys/class/drm/edid_quirks
+
+Note however, that any error (an incorrectly formatted quirk or an attempt to
+add a quirk when no slot is available) will abort processing of any further
+changes, potentially making it difficult to determine exactly which change
+caused the error and what changes were made. For this reason, making changes
+one at a time is recommended, particularly if the changes are being made by a
+script or program.
+
+
+Module parameter
+================
+
+The EDID quirk list can also be modified via the edid_quirks module parameter
+(drm.edid_quirks on the kernel command line). The effect of setting this
+parameter is identical to the effect of writing its value to
+/sys/class/drm/edid_quirks, with one important difference. When an error is
+encountered during module parameter parsing or processing, any remaining quirks
+in the parameter string will still be processed. (It is hoped that this approach
+maximizes the probability of producing a working display.)
+
+
+Follow-up
+=========
+
+If you encounter a display that requires an additional EDID quirk in order to
+function properly, please report it to the direct rendering development mailing
+list <dri-devel@lists.freedesktop.org>.
+
+
+[1] See http://en.wikipedia.org/wiki/Extended_display_identification_data for a
+    description of the manufacturer ID and product code fields.
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 9238de4..7fe39e0 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -276,6 +276,8 @@ static int __init drm_core_init(void)
 		goto err_p3;
 	}
 
+	drm_edid_quirks_param_process();
+
 	DRM_INFO("Initialized %s %d.%d.%d %s\n",
 		 CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
 	return 0;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a8743c3..398c361 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -31,6 +31,7 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/ctype.h>
 #include "drmP.h"
 #include "drm_edid.h"
 #include "drm_edid_modes.h"
@@ -68,6 +69,15 @@
 #define EDID_QUIRK_DETAILED_SYNC_PP		(1 << 6)
 /* Force reduced-blanking timings for detailed modes */
 #define EDID_QUIRK_FORCE_REDUCED_BLANKING	(1 << 7)
+/* Display is confused by InfoFrames; don't sent any */
+#define EDID_QUIRK_DISABLE_INFOFRAMES		(1 << 8)
+/* Display doesn't have any audio output */
+#define EDID_QUIRK_NO_AUDIO			(1 << 9)
+
+/*
+ * When adding additional quirk flags, please update
+ * Documentation/EDID/edid_quirks.txt.
+ */
 
 struct detailed_mode_closure {
 	struct drm_connector *connector;
@@ -82,51 +92,460 @@ struct detailed_mode_closure {
 #define LEVEL_GTF2	2
 #define LEVEL_CVT	3
 
-static struct edid_quirk {
-	char vendor[4];
-	int product_id;
-	u32 quirks;
-} edid_quirk_list[] = {
+union edid_quirk {
+	struct {
+		union edid_display_id display_id;
+		u32 quirks;
+	} __attribute__((packed)) s;
+	u64 u;
+};
+
+#define EDID_MFG_ID(c1, c2, c3)		cpu_to_be16(			\
+						(c1 & 0x1f) << 10 |	\
+						(c2 & 0x1f) << 5 |	\
+						(c3 & 0x1f)		\
+					)
+
+#define EDID_QUIRK_LIST_SIZE	24
+
+union edid_quirk edid_quirk_list[EDID_QUIRK_LIST_SIZE] = {
+
 	/* Acer AL1706 */
-	{ "ACR", 44358, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('A', 'C', 'R'), cpu_to_le16(44358) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 	/* Acer F51 */
-	{ "API", 0x7602, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('A', 'P', 'I'), cpu_to_le16(0x7602) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 	/* Unknown Acer */
-	{ "ACR", 2423, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+	{ { { { EDID_MFG_ID('A', 'C', 'R'), cpu_to_le16(2423) } },
+		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
 
 	/* Belinea 10 15 55 */
-	{ "MAX", 1516, EDID_QUIRK_PREFER_LARGE_60 },
-	{ "MAX", 0x77e, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('M', 'A', 'X'), cpu_to_le16(1516) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
+	{ { { { EDID_MFG_ID('M', 'A', 'X'), cpu_to_le16(0x77e) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 
 	/* Envision Peripherals, Inc. EN-7100e */
-	{ "EPI", 59264, EDID_QUIRK_135_CLOCK_TOO_HIGH },
+	{ { { { EDID_MFG_ID('E', 'P', 'I'), cpu_to_le16(59264) } },
+		EDID_QUIRK_135_CLOCK_TOO_HIGH } },
 	/* Envision EN2028 */
-	{ "EPI", 8232, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('E', 'P', 'I'), cpu_to_le16(8232) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 
 	/* Funai Electronics PM36B */
-	{ "FCM", 13600, EDID_QUIRK_PREFER_LARGE_75 |
-	  EDID_QUIRK_DETAILED_IN_CM },
+	{ { { { EDID_MFG_ID('F', 'C', 'M'), cpu_to_le16(13600) } },
+		EDID_QUIRK_PREFER_LARGE_75 | EDID_QUIRK_DETAILED_IN_CM } },
 
 	/* LG Philips LCD LP154W01-A5 */
-	{ "LPL", 0, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE },
-	{ "LPL", 0x2a00, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE },
+	{ { { { EDID_MFG_ID('L', 'P', 'L'), cpu_to_le16(0) } },
+		EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE } },
+	{ { { { EDID_MFG_ID('L', 'P', 'L'), cpu_to_le16(0x2a00) } },
+		EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE } },
 
 	/* Philips 107p5 CRT */
-	{ "PHL", 57364, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+	{ { { { EDID_MFG_ID('P', 'H', 'L'), cpu_to_le16(57364) } },
+		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
 
 	/* Proview AY765C */
-	{ "PTS", 765, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+	{ { { { EDID_MFG_ID('P', 'T', 'S'), cpu_to_le16(765) } },
+		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
 
 	/* Samsung SyncMaster 205BW.  Note: irony */
-	{ "SAM", 541, EDID_QUIRK_DETAILED_SYNC_PP },
+	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(541) } },
+		EDID_QUIRK_DETAILED_SYNC_PP } },
 	/* Samsung SyncMaster 22[5-6]BW */
-	{ "SAM", 596, EDID_QUIRK_PREFER_LARGE_60 },
-	{ "SAM", 638, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(596) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
+	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(638) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 
 	/* ViewSonic VA2026w */
-	{ "VSC", 5020, EDID_QUIRK_FORCE_REDUCED_BLANKING },
+	{ { { { EDID_MFG_ID('V', 'S', 'C'), cpu_to_le16(5020) } },
+		EDID_QUIRK_FORCE_REDUCED_BLANKING } },
+
+	/* LG L246WP */
+	{ { { { EDID_MFG_ID('G', 'S', 'M'), cpu_to_le16(0x563f) } },
+		EDID_QUIRK_DISABLE_INFOFRAMES | EDID_QUIRK_NO_AUDIO } },
+
+	/*
+	 * When adding built-in quirks, please adjust EDID_QUIRK_LIST_SIZE to
+	 * provide some room for user-supplied quirks.
+	 */
 };
 
+DEFINE_MUTEX(edid_quirk_list_mutex);
+
+/**
+ * drm_edid_mfg_format - format an "encoded" EDID manufacturer ID for printing
+ * @mfg_id: the encoded manufacturer ID
+ * @buf: destination buffer for the formated manufacturer ID (minimum 7 bytes)
+ * @strip: if non-zero, the returned pointer will skip any leading spaces
+ *
+ * An EDID manufacturer ID is supposed to consist of 3 capital letters (A-Z).
+ * Each letter is stored as a 5-bit value between 1 and 26, taking up 15 bits of
+ * the 16-bit ID. The remaining bit should always be 0. If display manufacturers
+ * always did things correctly, however, EDID quirks wouldn't be required in
+ * the first place. This function does the following:
+ *
+ * - Broken IDs are printed in hexadecimal (0xffff).
+ * - "Correct" IDs are formatted as a 3-letter ID string, preceded by 3 spaces;
+ *   the spaces ensure that both output formats are the same length.
+ *
+ * Thus, a formatted manufacturer ID is always 6 characters long (not including
+ * the terminating 0).
+ *
+ * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
+ * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
+ * been formatted as a 3-letter string, a pointer to the first non-space
+ * character (@buf + 3) is returned.
+ */
+static const char *drm_edid_mfg_format(__be16 mfg_id, char *buf, int strip)
+{
+	u16 id = be16_to_cpu(mfg_id);
+
+	if (id & 0x8000)
+		goto bad_id;
+
+	buf[3] = ((id & 0x7c00) >> 10) + '@';
+	if (!isupper(buf[3]))
+		goto bad_id;
+
+	buf[4] = ((id & 0x03e0) >> 5) + '@';
+	if (!isupper(buf[4]))
+		goto bad_id;
+
+	buf[5] = (id & 0x001f) + '@';
+	if (!isupper(buf[5]))
+		goto bad_id;
+
+	memset(buf, ' ', 3);
+	buf[6] = 0;
+
+	return strip ? (buf + 3) : buf;
+
+bad_id:
+	sprintf(buf, "0x%04hx", id);
+	return buf;
+}
+
+#define EDID_MFG_BUF_SIZE		7
+
+/**
+ * drm_edid_display_id_format - format an EDID "display ID" (manufacturer ID
+ * 				and product code) for printing
+ * @display_id: the display ID
+ * @buf: destination buffer for the formatted display ID (minimum 14 bytes)
+ * @strip: if non-zero, the returned pointer will skip any leading spaces
+ *
+ * A formatted display ID is always 13 characters long (not including the
+ * terminating 0).
+ *
+ * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
+ * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
+ * been formatted as a 3-letter string, a pointer to the first non-space
+ * character (@buf + 3) is returned.
+ */
+static const char *drm_edid_display_id_format(union edid_display_id display_id,
+					      char *buf, int strip)
+{
+	const char *s;
+
+	s = drm_edid_mfg_format(display_id.s.mfg_id, buf, strip);
+	sprintf(buf + EDID_MFG_BUF_SIZE - 1, ":0x%04hx",
+		le16_to_cpu(display_id.s.prod_code));
+
+	return s;
+}
+
+#define EDID_DISPLAY_ID_BUF_SIZE	(EDID_MFG_BUF_SIZE + 7)
+
+/**
+ * drm_edid_quirk_format - format an EDID quirk for printing
+ * @quirk: the quirk
+ * @buf: destination buffer for the formatted quirk (minimum 25 bytes)
+ * @strip: if non-zero, the returned pointer will skip any leading spaces
+ *
+ * A formatted EDID quirk is always 24 characters long (not including the
+ * terminating 0).
+ *
+ * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
+ * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
+ * been formatted as a 3-letter string, a pointer to the first non-space
+ * character (@buf + 3) is returned.
+ */
+static const char *drm_edid_quirk_format(const union edid_quirk *quirk,
+					 char *buf, int strip)
+{
+	const char *s;
+
+	s = drm_edid_display_id_format(quirk->s.display_id, buf, strip);
+	sprintf(buf + EDID_DISPLAY_ID_BUF_SIZE - 1, ":0x%08x", quirk->s.quirks);
+
+	return s;
+}
+
+#define EDID_QUIRK_BUF_SIZE		(EDID_DISPLAY_ID_BUF_SIZE + 11)
+
+/**
+ * drm_edid_quirk_parse - parse an EDID quirk
+ * @s: string containing the quirk to be parsed
+ * @quirk: destination for parsed quirk
+ *
+ * Returns 0 on success, < 0 (currently -EINVAL) on error.
+ */
+static int drm_edid_quirk_parse(const char *s, union edid_quirk *quirk)
+{
+	char buf[EDID_QUIRK_BUF_SIZE];
+	s32 mfg;
+	s32 product;
+	s64 quirks;
+	char *c;
+
+	if (sscanf(s, "%i:%i:%lli", &mfg, &product, &quirks) == 3) {
+		if (mfg < 0 || mfg > 0xffff)
+			goto error;
+		quirk->s.display_id.s.mfg_id = cpu_to_be16((u16)mfg);
+	} else {
+		if (sscanf(s, "%3s:%i:%lli", buf, &product, &quirks) != 3 ||
+				!isupper(buf[0]) ||
+				!isupper(buf[1]) ||
+				!isupper(buf[2]))
+			goto error;
+		quirk->s.display_id.s.mfg_id =
+				EDID_MFG_ID(buf[0], buf[1], buf[2]);
+	}
+
+	if (product < 0 || product > 0xffff ||
+			quirks < 0 || quirks > 0xffffffffLL)
+		goto error;
+
+	quirk->s.display_id.s.prod_code = cpu_to_le16((u16)product);
+	quirk->s.quirks = (u32)quirks;
+
+	DRM_DEBUG("Successfully parsed EDID quirk: %s\n",
+		  drm_edid_quirk_format(quirk, buf, 1));
+
+	return 0;
+
+error:
+	c = strpbrk(s, ",\n");
+	if (c == NULL) {
+		printk(KERN_WARNING "Invalid EDID quirk: '%s'\n", s);
+	} else {
+		printk(KERN_WARNING "Invalid EDID quirk: '%.*s'\n",
+		      (int)(c - s), s);
+	}
+
+	return -EINVAL;
+}
+
+/**
+ * drm_edid_quirk_find_by_id - find the EDID quirk matching a display ID
+ * @display_id: the display ID to match
+ *
+ * Caller MUST hold edid_quirk_list_mutex.
+ *
+ * Returns a pointer to the matching quirk list entry, NULL if no such entry
+ * exists.
+ */
+static union edid_quirk *drm_edid_quirk_find_by_id(union edid_display_id id)
+{
+	union edid_quirk *q = edid_quirk_list;
+
+	do {
+		if (q->s.display_id.u == id.u && q->s.quirks != 0)
+			return q;
+	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
+
+	return NULL;
+}
+
+/**
+ * drm_edid_quirk_find_slot - find an empty slot in the EDID quirk list
+ *
+ * Caller MUST hold edid_quirk_list_mutex.
+ *
+ * Returns a pointer to the first empty slot, NULL if no empty slots exist.
+ */
+static union edid_quirk *drm_edid_quirk_find_empty(void)
+{
+	union edid_quirk *q = edid_quirk_list;
+
+	do {
+		if (q->s.quirks == 0)
+			return q;
+	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
+
+	return NULL;
+}
+
+/**
+ * drm_edid_quirk_process - process a newly parsed EDID quirk
+ * @quirk: the quirk to be processed
+ *
+ * Depending on the newly parsed quirk and the contents of the quirks list, this
+ * function will add, remove, or replace a quirk.
+ *
+ * Returns 0 on success, < 0 on error (-ENOSPC if there is no free slot for a
+ * new quirk). Note that trying to remove a quirk that isn't present is not
+ * considered an error.
+ */
+static int drm_edid_quirk_process(const union edid_quirk *quirk)
+{
+	char buf[EDID_QUIRK_BUF_SIZE];
+	union edid_quirk *q;
+	int res = 0;
+
+	mutex_lock(&edid_quirk_list_mutex);
+
+	if (quirk->s.quirks == 0) {
+		DRM_INFO("Removing EDID quirk for display %s\n",
+			 drm_edid_display_id_format(quirk->s.display_id,
+						    buf, 1));
+		q = drm_edid_quirk_find_by_id(quirk->s.display_id);
+		if (q == NULL) {
+			printk(KERN_WARNING "No quirk found for display %s\n",
+			       drm_edid_display_id_format(quirk->s.display_id,
+							  buf, 1));
+		} else {
+			q->u = 0;
+		}
+	} else {
+		DRM_INFO("Adding EDID quirk: %s\n",
+			 drm_edid_quirk_format(quirk, buf, 1));
+		q = drm_edid_quirk_find_by_id(quirk->s.display_id);
+		if (q == NULL) {
+			q = drm_edid_quirk_find_empty();
+			if (q == NULL) {
+				printk(KERN_WARNING
+				       "No free slot in EDID quirk list\n");
+				res = -ENOSPC;
+			} else {
+				q->u = quirk->u;
+			}
+		} else {
+			DRM_INFO("Replacing existing quirk: %s\n",
+				 drm_edid_quirk_format(q, buf, 1));
+			q->s.quirks = quirk->s.quirks;
+		}
+	}
+
+	mutex_unlock(&edid_quirk_list_mutex);
+
+	return res;
+}
+
+/**
+ * drm_edid_quirks_process - parse and process a comma separated list of EDID
+ * 			     quirks
+ * @s: string containing the quirks to be processed
+ * @strict: if non-zero, any parsing or processing error aborts further
+ * 	    processing
+ *
+ * Returns 0 on success, < 0 if any error is encountered.  (If multiple errors
+ * occur when strict is set to 0, the last error encountered is returned.)
+ */
+static int drm_edid_quirks_process(const char *s, int strict)
+{
+	union edid_quirk quirk;
+	int res = 0;
+
+	do {
+
+		if (*s == '@') {
+			DRM_INFO("Clearing EDID quirk list\n");
+			mutex_lock(&edid_quirk_list_mutex);
+			memset(edid_quirk_list, 0, sizeof edid_quirk_list);
+			mutex_unlock(&edid_quirk_list_mutex);
+		} else {
+			res = drm_edid_quirk_parse(s, &quirk);
+			if (res != 0) {
+				if (strict)
+					goto error;
+				continue;
+			}
+
+			res = drm_edid_quirk_process(&quirk);
+			if (res != 0) {
+				if (strict)
+					goto error;
+			}
+		}
+
+		s = strpbrk(s, ",\n");
+
+	} while (s != NULL && *(++s) != 0);
+
+	return res;
+
+error:
+	printk(KERN_WARNING "Aborting EDID quirk parsing\n");
+	return res;
+}
+
+/**
+ * drm_edid_quirks_param_process - process the edid_quirks module parameter
+ */
+void drm_edid_quirks_param_process(void)
+{
+	if (drm_edid_quirks != NULL)
+		drm_edid_quirks_process(drm_edid_quirks, 0);
+}
+
+/**
+ * drm_edid_quirks_size_show - show the size of the EDID quirk list in sysfs
+ * @buf: destination buffer (PAGE_SIZE bytes)
+ */
+ssize_t drm_edid_quirks_size_show(struct class *class,
+				  struct class_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%zu\n", ARRAY_SIZE(edid_quirk_list));
+}
+
+/**
+ * drm_edid_quirks_show - show the contents of the EDID quirk list in sysfs
+ * @buf: destination buffer (PAGE_SIZE bytes)
+ */
+ssize_t drm_edid_quirks_show(struct class *class, struct class_attribute *attr,
+			     char *buf)
+{
+	const union edid_quirk *q = edid_quirk_list;
+	ssize_t count = 0;
+
+	BUILD_BUG_ON(ARRAY_SIZE(edid_quirk_list) >
+				PAGE_SIZE / EDID_QUIRK_BUF_SIZE);
+
+	mutex_lock(&edid_quirk_list_mutex);
+
+	do {
+		if (q->s.quirks != 0) {
+			drm_edid_quirk_format(q, buf + count, 0);
+			(buf + count)[EDID_QUIRK_BUF_SIZE - 1] = '\n';
+			count += EDID_QUIRK_BUF_SIZE;
+		}
+	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
+
+	mutex_unlock(&edid_quirk_list_mutex);
+
+	return count;
+}
+
+/**
+ * drm_edid_quirks_store - parse and process EDID quirkl list changes written
+ *			   to sysfs attribute
+ */
+ssize_t drm_edid_quirks_store(struct class *class, struct class_attribute *attr,
+			      const char *buf, size_t count)
+{
+	int res;
+
+	res = drm_edid_quirks_process(buf, 1);
+	if (res != 0)
+		return res;
+
+	return count;
+}
 /*** DDC fetch and block validation ***/
 
 static const u8 edid_header[] = {
@@ -409,25 +828,6 @@ EXPORT_SYMBOL(drm_get_edid);
 /*** EDID parsing ***/
 
 /**
- * edid_vendor - match a string against EDID's obfuscated vendor field
- * @edid: EDID to match
- * @vendor: vendor string
- *
- * Returns true if @vendor is in @edid, false otherwise
- */
-static bool edid_vendor(struct edid *edid, char *vendor)
-{
-	char edid_vendor[3];
-
-	edid_vendor[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
-	edid_vendor[1] = (((edid->mfg_id[0] & 0x3) << 3) |
-			  ((edid->mfg_id[1] & 0xe0) >> 5)) + '@';
-	edid_vendor[2] = (edid->mfg_id[1] & 0x1f) + '@';
-
-	return !strncmp(edid_vendor, vendor, 3);
-}
-
-/**
  * edid_get_quirks - return quirk flags for a given EDID
  * @edid: EDID to process
  *
@@ -435,18 +835,18 @@ static bool edid_vendor(struct edid *edid, char *vendor)
  */
 static u32 edid_get_quirks(struct edid *edid)
 {
-	struct edid_quirk *quirk;
-	int i;
+	union edid_quirk *q;
+	u32 quirks = 0;
 
-	for (i = 0; i < ARRAY_SIZE(edid_quirk_list); i++) {
-		quirk = &edid_quirk_list[i];
+	mutex_lock(&edid_quirk_list_mutex);
 
-		if (edid_vendor(edid, quirk->vendor) &&
-		    (EDID_PRODUCT_ID(edid) == quirk->product_id))
-			return quirk->quirks;
-	}
+	q = drm_edid_quirk_find_by_id(edid->display_id);
+	if (q != NULL)
+		quirks = q->s.quirks;
 
-	return 0;
+	mutex_unlock(&edid_quirk_list_mutex);
+
+	return quirks;
 }
 
 #define MODE_SIZE(m) ((m)->hdisplay * (m)->vdisplay)
@@ -1162,7 +1562,7 @@ do_inferred_modes(struct detailed_timing *timing, void *c)
 	closure->modes += drm_dmt_modes_for_range(closure->connector,
 						  closure->edid,
 						  timing);
-	
+
 	if (!version_greater(closure->edid, 1, 1))
 		return; /* GTF not defined yet */
 
@@ -1399,7 +1799,7 @@ do_cvt_mode(struct detailed_timing *timing, void *c)
 
 static int
 add_cvt_modes(struct drm_connector *connector, struct edid *edid)
-{	
+{
 	struct detailed_mode_closure closure = {
 		connector, edid, 0, 0, 0
 	};
@@ -1615,15 +2015,12 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 
 	eld[0] = 2 << 3;		/* ELD version: 2 */
 
-	eld[16] = edid->mfg_id[0];
-	eld[17] = edid->mfg_id[1];
-	eld[18] = edid->prod_code[0];
-	eld[19] = edid->prod_code[1];
+	*(u32 *)(&eld[16]) = edid->display_id.u;
 
 	if (cea[1] >= 3)
 		for (db = cea + 4; db < cea + cea[2]; db += dbl + 1) {
 			dbl = db[0] & 0x1f;
-			
+
 			switch ((db[0] & 0xe0) >> 5) {
 			case AUDIO_BLOCK:
 				/* Audio Data Block, contains SADs */
@@ -1723,6 +2120,14 @@ bool drm_detect_hdmi_monitor(struct edid *edid)
 	int i, hdmi_id;
 	int start_offset, end_offset;
 	bool is_hdmi = false;
+	char buf[EDID_DISPLAY_ID_BUF_SIZE];
+
+	if (edid_get_quirks(edid) & EDID_QUIRK_DISABLE_INFOFRAMES) {
+		DRM_INFO("Disabling HDMI InfoFrames on display %s "
+			 "due to EDID quirk\n",
+			 drm_edid_display_id_format(edid->display_id, buf, 1));
+		goto end;
+	}
 
 	edid_ext = drm_find_cea_extension(edid);
 	if (!edid_ext)
@@ -1771,6 +2176,14 @@ bool drm_detect_monitor_audio(struct edid *edid)
 	int i, j;
 	bool has_audio = false;
 	int start_offset, end_offset;
+	char buf[EDID_DISPLAY_ID_BUF_SIZE];
+
+	if (edid_get_quirks(edid) & EDID_QUIRK_NO_AUDIO) {
+		DRM_INFO("Disabling HDMI audio on display %s "
+			 "due to EDID quirk\n",
+			 drm_edid_display_id_format(edid->display_id, buf, 1));
+		goto end;
+	}
 
 	edid_ext = drm_find_cea_extension(edid);
 	if (!edid_ext)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 21bcd4a..1885fc9 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -46,16 +46,21 @@ EXPORT_SYMBOL(drm_vblank_offdelay);
 unsigned int drm_timestamp_precision = 20;  /* Default to 20 usecs. */
 EXPORT_SYMBOL(drm_timestamp_precision);
 
+char *drm_edid_quirks = NULL;
+EXPORT_SYMBOL(drm_edid_quirks);
+
 MODULE_AUTHOR(CORE_AUTHOR);
 MODULE_DESCRIPTION(CORE_DESC);
 MODULE_LICENSE("GPL and additional rights");
 MODULE_PARM_DESC(debug, "Enable debug output");
 MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable [msecs]");
 MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps [usecs]");
+MODULE_PARM_DESC(edid_quirks, "See Documentation/EDID/edid_quirks.txt");
 
 module_param_named(debug, drm_debug, int, 0600);
 module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600);
 module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600);
+module_param_named(edid_quirks, drm_edid_quirks, charp, 0400);
 
 struct idr drm_minors_idr;
 
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 45ac8d6..84dc365 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -84,6 +84,11 @@ static CLASS_ATTR_STRING(version, S_IRUGO,
 		__stringify(CORE_PATCHLEVEL) " "
 		CORE_DATE);
 
+static CLASS_ATTR(edid_quirks_size, 0400, drm_edid_quirks_size_show, 0);
+
+static CLASS_ATTR(edid_quirks, 0600, drm_edid_quirks_show,
+		  drm_edid_quirks_store);
+
 /**
  * drm_sysfs_create - create a struct drm_sysfs_class structure
  * @owner: pointer to the module that is to "own" this struct drm_sysfs_class
@@ -113,10 +118,22 @@ struct class *drm_sysfs_create(struct module *owner, char *name)
 	if (err)
 		goto err_out_class;
 
+	err = class_create_file(class, &class_attr_edid_quirks_size);
+	if (err)
+		goto err_out_version;
+
+	err = class_create_file(class, &class_attr_edid_quirks);
+	if (err)
+		goto err_out_quirks_size;
+
 	class->devnode = drm_devnode;
 
 	return class;
 
+err_out_quirks_size:
+	class_remove_file(class, &class_attr_edid_quirks_size);
+err_out_version:
+	class_remove_file(class, &class_attr_version.attr);
 err_out_class:
 	class_destroy(class);
 err_out:
@@ -132,6 +149,8 @@ void drm_sysfs_destroy(void)
 {
 	if ((drm_class == NULL) || (IS_ERR(drm_class)))
 		return;
+	class_remove_file(drm_class, &class_attr_edid_quirks);
+	class_remove_file(drm_class, &class_attr_edid_quirks_size);
 	class_remove_file(drm_class, &class_attr_version.attr);
 	class_destroy(drm_class);
 	drm_class = NULL;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d6b67bb..c947f3e 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1501,6 +1501,7 @@ extern unsigned int drm_debug;
 
 extern unsigned int drm_vblank_offdelay;
 extern unsigned int drm_timestamp_precision;
+extern char *drm_edid_quirks;
 
 extern struct class *drm_class;
 extern struct proc_dir_entry *drm_proc_root;
@@ -1612,6 +1613,15 @@ void drm_gem_vm_open(struct vm_area_struct *vma);
 void drm_gem_vm_close(struct vm_area_struct *vma);
 int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
 
+					/* EDID support (drm_edid.c) */
+void drm_edid_quirks_param_process(void);
+ssize_t drm_edid_quirks_size_show(struct class *class,
+				  struct class_attribute *attr, char *buf);
+ssize_t drm_edid_quirks_show(struct class *class, struct class_attribute *attr,
+			     char *buf);
+ssize_t drm_edid_quirks_store(struct class *class, struct class_attribute *attr,
+			      const char *buf, size_t count);
+
 #include "drm_global.h"
 
 static inline void
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 0cac551..713229b 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -202,11 +202,18 @@ struct detailed_timing {
 #define DRM_EDID_FEATURE_PM_SUSPEND       (1 << 6)
 #define DRM_EDID_FEATURE_PM_STANDBY       (1 << 7)
 
+union edid_display_id {
+	struct {
+		__be16 mfg_id;
+		__le16 prod_code;
+	} __attribute__((packed)) s;
+	u32 u;
+};
+
 struct edid {
 	u8 header[8];
 	/* Vendor & product info */
-	u8 mfg_id[2];
-	u8 prod_code[2];
+	union edid_display_id display_id;
 	u32 serial; /* FIXME: byte order */
 	u8 mfg_week;
 	u8 mfg_year;
@@ -242,8 +249,6 @@ struct edid {
 	u8 checksum;
 } __attribute__((packed));
 
-#define EDID_PRODUCT_ID(e) ((e)->prod_code[0] | ((e)->prod_code[1] << 8))
-
 struct drm_encoder;
 struct drm_connector;
 struct drm_display_mode;
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH] drm: EDID quirk improvements
  2012-08-10 18:44                   ` Ian Pilcher
@ 2012-08-11  8:31                     ` Paul Menzel
  2012-08-11 15:38                       ` Ian Pilcher
  2012-08-11 19:37                     ` Paul Menzel
  1 sibling, 1 reply; 32+ messages in thread
From: Paul Menzel @ 2012-08-11  8:31 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 34854 bytes --]

Dear Ian,


thank you for your patch! The best thing is, I just asked Adam on
#intel-gfx the other day, if it was possible to have infrastructure to
test quirks without having to patch and build the Linux kernel. Nice!

As a side note, could you also mention the patch iteration in the tag,
that means [PATCH vN] so that I know what is the latest version. That
would be great.


Am Freitag, den 10.08.2012, 13:44 -0500 schrieb Ian Pilcher:
> Add ability for user to add or remove EDID quirks, via module
> parameter or sysfs.  Also add two new quirk flags --
> EDID_QUIRK_DISABLE_INFOFRAMES and EDID_QUIRK_NO_AUDIO -- and adds
> a quirk for the LG L246WP display.

I would submit adding the new quirk flags and the LG quirk as separate
patches. If it is not too much work, it would be great if you could
split them up.

> Document module parameter and sysfs interface.
> ---
>  Documentation/EDID/edid_quirks.txt | 161 +++++++++++
>  drivers/gpu/drm/drm_drv.c          |   2 +
>  drivers/gpu/drm/drm_edid.c         | 527 +++++++++++++++++++++++++++++++++----
>  drivers/gpu/drm/drm_stub.c         |   5 +
>  drivers/gpu/drm/drm_sysfs.c        |  19 ++
>  include/drm/drmP.h                 |  10 +
>  include/drm/drm_edid.h             |  13 +-
>  7 files changed, 676 insertions(+), 61 deletions(-)
>  create mode 100644 Documentation/EDID/edid_quirks.txt
> 
> diff --git a/Documentation/EDID/edid_quirks.txt b/Documentation/EDID/edid_quirks.txt
> new file mode 100644
> index 0000000..256ded0
> --- /dev/null
> +++ b/Documentation/EDID/edid_quirks.txt
> @@ -0,0 +1,161 @@
> +                                  EDID Quirks
> +                                 =============
> +                       Ian Pilcher <arequipeno@gmail.com>
> +                                 August 8, 2012
> +
> +
> +    "EDID blocks out in the wild have a variety of bugs"
> +        -- from drivers/gpu/drm/drm_edid.c
> +
> +
> +Overview
> +========
> +
> +EDID quirks provide a mechanism for working around display hardware with buggy
> +EDID data.
> +
> +An individual EDID quirk maps a display type (identified by its EDID
> +manufacturer ID and product code[1]) to a set of flags. For example, the current
> +list of quirks built into the kernel is:
> +
> +    ACR:0xad46:0x00000001
> +    API:0x7602:0x00000001
> +    ACR:0x0977:0x00000020
> +    MAX:0x05ec:0x00000001
> +    MAX:0x077e:0x00000001
> +    EPI:0xe780:0x00000002
> +    EPI:0x2028:0x00000001
> +    FCM:0x3520:0x0000000c
> +    LPL:0x0000:0x00000010
> +    LPL:0x2a00:0x00000010
> +    PHL:0xe014:0x00000020
> +    PTS:0x02fd:0x00000020
> +    SAM:0x021d:0x00000040
> +    SAM:0x0254:0x00000001
> +    SAM:0x027e:0x00000001
> +    VSC:0x139c:0x00000080
> +    GSM:0x563f:0x00000300
> +
> +The first field of each quirk is the manufacturer ID, the second field is the
> +product code, and the third field is the quirk flags.
> +
> +NOTE: All of the manufacturer IDs above are displayed as 3-character strings,
> +    because they are conformant IDs that have been properly encoded:
> +
> +    - The most-significant bit of the encoded ID is 0
> +    - They only contain ASCII characters in the range A-Z
> +
> +    IDs that do not conform to these rules are displayed as "raw" hexadecimal
> +    values.
> +
> +The current quirk flags are:
> +
> +    /* First detailed mode wrong, use largest 60Hz mode */
> +    #define EDID_QUIRK_PREFER_LARGE_60                  0x00000001
> +
> +    /* Reported 135MHz pixel clock is too high, needs adjustment */
> +    #define EDID_QUIRK_135_CLOCK_TOO_HIGH               0x00000002
> +
> +    /* Prefer the largest mode at 75 Hz */
> +    #define EDID_QUIRK_PREFER_LARGE_75                  0x00000004
> +
> +    /* Detail timing is in cm not mm */
> +    #define EDID_QUIRK_DETAILED_IN_CM                   0x00000008
> +
> +    /* Detailed timing descriptors have bogus size values, so just take the
> +     * maximum size and use that.
> +     */
> +    #define EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE        0x00000010
> +
> +    /* Monitor forgot to set the first detailed is preferred bit. */
> +    #define EDID_QUIRK_FIRST_DETAILED_PREFERRED         0x00000020
> +
> +    /* use +hsync +vsync for detailed mode */
> +    #define EDID_QUIRK_DETAILED_SYNC_PP                 0x00000040
> +
> +    /* Force reduced-blanking timings for detailed modes */
> +    #define EDID_QUIRK_FORCE_REDUCED_BLANKING           0x00000080

You even added this one from the following commit.

        commit bc42aabc6a01b92b0f961d65671564e0e1cd7592
        Author: Adam Jackson <ajax@redhat.com>
        Date:   Wed May 23 16:26:54 2012 -0400
        
            drm/edid/quirks: ViewSonic VA2026w

I am going to need that quirk. Great!

> +
> +    /* Display is confused by InfoFrames; don't sent any */
> +    #define EDID_QUIRK_DISABLE_INFOFRAMES               0x00000100
> +
> +    /* Display doesn't have any audio output */
> +    #define EDID_QUIRK_NO_AUDIO                         0x00000200
> +
> +
> +sysfs interface
> +===============
> +
> +The current EDID quirk list can be read from /sys/class/drm/edid_quirks.
> +
> +The number of total "slots" in the list can be read from
> +/sys/class/drm/edid_quirks_size.  This total includes both occupied slots (i.e.
> +the current list) and any slots available for additional quirks.  The number of
> +available slots can be calculated by subtracting the number of quirks in the
> +current list from the total number of slots.
> +
> +If a slot is available, an additional quirk can be added to the list by writing
> +it to edid_quirks:
> +
> +    # echo FOO:0xffff:0x100 > /sys/class/drm/edid_quirks
> +
> +Manufacturer IDs can also be specified numerically.  (This is the only way to
> +specify a nonconformant ID.) This command is equivalent to the previous one:
> +
> +    # echo 0x19ef:0xffff:0x100 > /sys/class/drm/edid_quirks
> +
> +Numeric values can also be specified in decimal or octal formats; a number that
> +begins with a 0 is assumed to be octal:
> +
> +    # echo FOO:65535:0400 > /sys/class/drm/edid_quirks
> +
> +An existing quirk can be replaced by writing a new set of flags:
> +
> +    # echo FOO:0xffff:0x200 > /sys/class/drm/edid_quirks
> +
> +A quirk can be deleted from the list by writing an empty flag set (0). This
> +makes the list slot occupied by that quirk available.
> +
> +    # echo FOO:0xffff:0 > /sys/class/drm/edid_quirks
> +
> +Writing an "at symbol" (@) clears the entire quirk list:
> +
> +    # echo @ > /sys/class/drm/edid_quirks
> +
> +Multiple changes to the list can be specified in a comma (or newline) separated
> +list. For example, the following command clears all of the existing quirks in
> +the list and adds 3 new quirks:
> +
> +    # echo @,FOO:0xffff:0x100,BAR:0x1111:0x001,BAZ:0x2222:0x002 > \
> +            /sys/class/drm/edid_quirks
> +
> +Note however, that any error (an incorrectly formatted quirk or an attempt to
> +add a quirk when no slot is available) will abort processing of any further
> +changes, potentially making it difficult to determine exactly which change
> +caused the error and what changes were made. For this reason, making changes
> +one at a time is recommended, particularly if the changes are being made by a
> +script or program.
> +
> +
> +Module parameter
> +================
> +
> +The EDID quirk list can also be modified via the edid_quirks module parameter
> +(drm.edid_quirks on the kernel command line). The effect of setting this
> +parameter is identical to the effect of writing its value to
> +/sys/class/drm/edid_quirks, with one important difference. When an error is
> +encountered during module parameter parsing or processing, any remaining quirks
> +in the parameter string will still be processed. (It is hoped that this approach
> +maximizes the probability of producing a working display.)
> +
> +
> +Follow-up
> +=========
> +
> +If you encounter a display that requires an additional EDID quirk in order to
> +function properly, please report it to the direct rendering development mailing
> +list <dri-devel@lists.freedesktop.org>.
> +
> +
> +[1] See http://en.wikipedia.org/wiki/Extended_display_identification_data for a
> +    description of the manufacturer ID and product code fields.

Nice write up!

> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 9238de4..7fe39e0 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -276,6 +276,8 @@ static int __init drm_core_init(void)
>  		goto err_p3;
>  	}
>  
> +	drm_edid_quirks_param_process();
> +
>  	DRM_INFO("Initialized %s %d.%d.%d %s\n",
>  		 CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
>  	return 0;
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index a8743c3..398c361 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -31,6 +31,7 @@
>  #include <linux/slab.h>
>  #include <linux/i2c.h>
>  #include <linux/module.h>
> +#include <linux/ctype.h>
>  #include "drmP.h"
>  #include "drm_edid.h"
>  #include "drm_edid_modes.h"
> @@ -68,6 +69,15 @@
>  #define EDID_QUIRK_DETAILED_SYNC_PP		(1 << 6)
>  /* Force reduced-blanking timings for detailed modes */
>  #define EDID_QUIRK_FORCE_REDUCED_BLANKING	(1 << 7)
> +/* Display is confused by InfoFrames; don't sent any */
> +#define EDID_QUIRK_DISABLE_INFOFRAMES		(1 << 8)
> +/* Display doesn't have any audio output */
> +#define EDID_QUIRK_NO_AUDIO			(1 << 9)
> +
> +/*
> + * When adding additional quirk flags, please update
> + * Documentation/EDID/edid_quirks.txt.
> + */
>  
>  struct detailed_mode_closure {
>  	struct drm_connector *connector;
> @@ -82,51 +92,460 @@ struct detailed_mode_closure {
>  #define LEVEL_GTF2	2
>  #define LEVEL_CVT	3
>  
> -static struct edid_quirk {
> -	char vendor[4];
> -	int product_id;
> -	u32 quirks;
> -} edid_quirk_list[] = {
> +union edid_quirk {
> +	struct {
> +		union edid_display_id display_id;
> +		u32 quirks;
> +	} __attribute__((packed)) s;
> +	u64 u;
> +};
> +
> +#define EDID_MFG_ID(c1, c2, c3)		cpu_to_be16(			\
> +						(c1 & 0x1f) << 10 |	\
> +						(c2 & 0x1f) << 5 |	\
> +						(c3 & 0x1f)		\
> +					)
> +
> +#define EDID_QUIRK_LIST_SIZE	24
> +
> +union edid_quirk edid_quirk_list[EDID_QUIRK_LIST_SIZE] = {
> +
>  	/* Acer AL1706 */
> -	{ "ACR", 44358, EDID_QUIRK_PREFER_LARGE_60 },
> +	{ { { { EDID_MFG_ID('A', 'C', 'R'), cpu_to_le16(44358) } },
> +		EDID_QUIRK_PREFER_LARGE_60 } },
>  	/* Acer F51 */
> -	{ "API", 0x7602, EDID_QUIRK_PREFER_LARGE_60 },
> +	{ { { { EDID_MFG_ID('A', 'P', 'I'), cpu_to_le16(0x7602) } },
> +		EDID_QUIRK_PREFER_LARGE_60 } },
>  	/* Unknown Acer */
> -	{ "ACR", 2423, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
> +	{ { { { EDID_MFG_ID('A', 'C', 'R'), cpu_to_le16(2423) } },
> +		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
>  
>  	/* Belinea 10 15 55 */
> -	{ "MAX", 1516, EDID_QUIRK_PREFER_LARGE_60 },
> -	{ "MAX", 0x77e, EDID_QUIRK_PREFER_LARGE_60 },
> +	{ { { { EDID_MFG_ID('M', 'A', 'X'), cpu_to_le16(1516) } },
> +		EDID_QUIRK_PREFER_LARGE_60 } },
> +	{ { { { EDID_MFG_ID('M', 'A', 'X'), cpu_to_le16(0x77e) } },
> +		EDID_QUIRK_PREFER_LARGE_60 } },
>  
>  	/* Envision Peripherals, Inc. EN-7100e */
> -	{ "EPI", 59264, EDID_QUIRK_135_CLOCK_TOO_HIGH },
> +	{ { { { EDID_MFG_ID('E', 'P', 'I'), cpu_to_le16(59264) } },
> +		EDID_QUIRK_135_CLOCK_TOO_HIGH } },
>  	/* Envision EN2028 */
> -	{ "EPI", 8232, EDID_QUIRK_PREFER_LARGE_60 },
> +	{ { { { EDID_MFG_ID('E', 'P', 'I'), cpu_to_le16(8232) } },
> +		EDID_QUIRK_PREFER_LARGE_60 } },
>  
>  	/* Funai Electronics PM36B */
> -	{ "FCM", 13600, EDID_QUIRK_PREFER_LARGE_75 |
> -	  EDID_QUIRK_DETAILED_IN_CM },
> +	{ { { { EDID_MFG_ID('F', 'C', 'M'), cpu_to_le16(13600) } },
> +		EDID_QUIRK_PREFER_LARGE_75 | EDID_QUIRK_DETAILED_IN_CM } },
>  
>  	/* LG Philips LCD LP154W01-A5 */
> -	{ "LPL", 0, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE },
> -	{ "LPL", 0x2a00, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE },
> +	{ { { { EDID_MFG_ID('L', 'P', 'L'), cpu_to_le16(0) } },
> +		EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE } },
> +	{ { { { EDID_MFG_ID('L', 'P', 'L'), cpu_to_le16(0x2a00) } },
> +		EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE } },
>  
>  	/* Philips 107p5 CRT */
> -	{ "PHL", 57364, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
> +	{ { { { EDID_MFG_ID('P', 'H', 'L'), cpu_to_le16(57364) } },
> +		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
>  
>  	/* Proview AY765C */
> -	{ "PTS", 765, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
> +	{ { { { EDID_MFG_ID('P', 'T', 'S'), cpu_to_le16(765) } },
> +		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
>  
>  	/* Samsung SyncMaster 205BW.  Note: irony */
> -	{ "SAM", 541, EDID_QUIRK_DETAILED_SYNC_PP },
> +	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(541) } },
> +		EDID_QUIRK_DETAILED_SYNC_PP } },
>  	/* Samsung SyncMaster 22[5-6]BW */
> -	{ "SAM", 596, EDID_QUIRK_PREFER_LARGE_60 },
> -	{ "SAM", 638, EDID_QUIRK_PREFER_LARGE_60 },
> +	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(596) } },
> +		EDID_QUIRK_PREFER_LARGE_60 } },
> +	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(638) } },
> +		EDID_QUIRK_PREFER_LARGE_60 } },
>  
>  	/* ViewSonic VA2026w */
> -	{ "VSC", 5020, EDID_QUIRK_FORCE_REDUCED_BLANKING },
> +	{ { { { EDID_MFG_ID('V', 'S', 'C'), cpu_to_le16(5020) } },
> +		EDID_QUIRK_FORCE_REDUCED_BLANKING } },
> +
> +	/* LG L246WP */
> +	{ { { { EDID_MFG_ID('G', 'S', 'M'), cpu_to_le16(0x563f) } },
> +		EDID_QUIRK_DISABLE_INFOFRAMES | EDID_QUIRK_NO_AUDIO } },
> +
> +	/*
> +	 * When adding built-in quirks, please adjust EDID_QUIRK_LIST_SIZE to
> +	 * provide some room for user-supplied quirks.
> +	 */
>  };
>  
> +DEFINE_MUTEX(edid_quirk_list_mutex);
> +
> +/**
> + * drm_edid_mfg_format - format an "encoded" EDID manufacturer ID for printing
> + * @mfg_id: the encoded manufacturer ID
> + * @buf: destination buffer for the formated manufacturer ID (minimum 7 bytes)

format*t*ed

http://www.merriam-webster.com/dictionary/formatted

> + * @strip: if non-zero, the returned pointer will skip any leading spaces
> + *
> + * An EDID manufacturer ID is supposed to consist of 3 capital letters (A-Z).
> + * Each letter is stored as a 5-bit value between 1 and 26, taking up 15 bits of
> + * the 16-bit ID. The remaining bit should always be 0. If display manufacturers
> + * always did things correctly, however, EDID quirks wouldn't be required in
> + * the first place. This function does the following:
> + *
> + * - Broken IDs are printed in hexadecimal (0xffff).
> + * - "Correct" IDs are formatted as a 3-letter ID string, preceded by 3 spaces;
> + *   the spaces ensure that both output formats are the same length.
> + *
> + * Thus, a formatted manufacturer ID is always 6 characters long (not including
> + * the terminating 0).
> + *
> + * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
> + * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
> + * been formatted as a 3-letter string, a pointer to the first non-space
> + * character (@buf + 3) is returned.
> + */
> +static const char *drm_edid_mfg_format(__be16 mfg_id, char *buf, int strip)
> +{
> +	u16 id = be16_to_cpu(mfg_id);
> +
> +	if (id & 0x8000)
> +		goto bad_id;
> +
> +	buf[3] = ((id & 0x7c00) >> 10) + '@';
> +	if (!isupper(buf[3]))
> +		goto bad_id;
> +
> +	buf[4] = ((id & 0x03e0) >> 5) + '@';
> +	if (!isupper(buf[4]))
> +		goto bad_id;
> +
> +	buf[5] = (id & 0x001f) + '@';
> +	if (!isupper(buf[5]))
> +		goto bad_id;
> +
> +	memset(buf, ' ', 3);
> +	buf[6] = 0;
> +
> +	return strip ? (buf + 3) : buf;
> +
> +bad_id:
> +	sprintf(buf, "0x%04hx", id);
> +	return buf;
> +}
> +
> +#define EDID_MFG_BUF_SIZE		7
> +
> +/**
> + * drm_edid_display_id_format - format an EDID "display ID" (manufacturer ID
> + * 				and product code) for printing
> + * @display_id: the display ID
> + * @buf: destination buffer for the formatted display ID (minimum 14 bytes)
> + * @strip: if non-zero, the returned pointer will skip any leading spaces
> + *
> + * A formatted display ID is always 13 characters long (not including the
> + * terminating 0).
> + *
> + * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
> + * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
> + * been formatted as a 3-letter string, a pointer to the first non-space
> + * character (@buf + 3) is returned.
> + */
> +static const char *drm_edid_display_id_format(union edid_display_id display_id,
> +					      char *buf, int strip)
> +{
> +	const char *s;
> +
> +	s = drm_edid_mfg_format(display_id.s.mfg_id, buf, strip);
> +	sprintf(buf + EDID_MFG_BUF_SIZE - 1, ":0x%04hx",
> +		le16_to_cpu(display_id.s.prod_code));
> +
> +	return s;
> +}
> +
> +#define EDID_DISPLAY_ID_BUF_SIZE	(EDID_MFG_BUF_SIZE + 7)
> +
> +/**
> + * drm_edid_quirk_format - format an EDID quirk for printing
> + * @quirk: the quirk
> + * @buf: destination buffer for the formatted quirk (minimum 25 bytes)
> + * @strip: if non-zero, the returned pointer will skip any leading spaces
> + *
> + * A formatted EDID quirk is always 24 characters long (not including the
> + * terminating 0).
> + *
> + * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
> + * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
> + * been formatted as a 3-letter string, a pointer to the first non-space
> + * character (@buf + 3) is returned.
> + */
> +static const char *drm_edid_quirk_format(const union edid_quirk *quirk,
> +					 char *buf, int strip)
> +{
> +	const char *s;
> +
> +	s = drm_edid_display_id_format(quirk->s.display_id, buf, strip);
> +	sprintf(buf + EDID_DISPLAY_ID_BUF_SIZE - 1, ":0x%08x", quirk->s.quirks);
> +
> +	return s;
> +}
> +
> +#define EDID_QUIRK_BUF_SIZE		(EDID_DISPLAY_ID_BUF_SIZE + 11)
> +
> +/**
> + * drm_edid_quirk_parse - parse an EDID quirk
> + * @s: string containing the quirk to be parsed
> + * @quirk: destination for parsed quirk
> + *
> + * Returns 0 on success, < 0 (currently -EINVAL) on error.
> + */
> +static int drm_edid_quirk_parse(const char *s, union edid_quirk *quirk)
> +{
> +	char buf[EDID_QUIRK_BUF_SIZE];
> +	s32 mfg;
> +	s32 product;
> +	s64 quirks;
> +	char *c;
> +
> +	if (sscanf(s, "%i:%i:%lli", &mfg, &product, &quirks) == 3) {
> +		if (mfg < 0 || mfg > 0xffff)
> +			goto error;
> +		quirk->s.display_id.s.mfg_id = cpu_to_be16((u16)mfg);
> +	} else {
> +		if (sscanf(s, "%3s:%i:%lli", buf, &product, &quirks) != 3 ||
> +				!isupper(buf[0]) ||
> +				!isupper(buf[1]) ||
> +				!isupper(buf[2]))
> +			goto error;
> +		quirk->s.display_id.s.mfg_id =
> +				EDID_MFG_ID(buf[0], buf[1], buf[2]);
> +	}
> +
> +	if (product < 0 || product > 0xffff ||
> +			quirks < 0 || quirks > 0xffffffffLL)
> +		goto error;
> +
> +	quirk->s.display_id.s.prod_code = cpu_to_le16((u16)product);
> +	quirk->s.quirks = (u32)quirks;
> +
> +	DRM_DEBUG("Successfully parsed EDID quirk: %s\n",
> +		  drm_edid_quirk_format(quirk, buf, 1));
> +
> +	return 0;
> +
> +error:
> +	c = strpbrk(s, ",\n");
> +	if (c == NULL) {
> +		printk(KERN_WARNING "Invalid EDID quirk: '%s'\n", s);
> +	} else {
> +		printk(KERN_WARNING "Invalid EDID quirk: '%.*s'\n",
> +		      (int)(c - s), s);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +/**
> + * drm_edid_quirk_find_by_id - find the EDID quirk matching a display ID
> + * @display_id: the display ID to match
> + *
> + * Caller MUST hold edid_quirk_list_mutex.
> + *
> + * Returns a pointer to the matching quirk list entry, NULL if no such entry
> + * exists.
> + */
> +static union edid_quirk *drm_edid_quirk_find_by_id(union edid_display_id id)
> +{
> +	union edid_quirk *q = edid_quirk_list;
> +
> +	do {
> +		if (q->s.display_id.u == id.u && q->s.quirks != 0)
> +			return q;
> +	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
> +
> +	return NULL;
> +}
> +
> +/**
> + * drm_edid_quirk_find_slot - find an empty slot in the EDID quirk list
> + *
> + * Caller MUST hold edid_quirk_list_mutex.
> + *
> + * Returns a pointer to the first empty slot, NULL if no empty slots exist.
> + */
> +static union edid_quirk *drm_edid_quirk_find_empty(void)
> +{
> +	union edid_quirk *q = edid_quirk_list;
> +
> +	do {
> +		if (q->s.quirks == 0)
> +			return q;
> +	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
> +
> +	return NULL;
> +}
> +
> +/**
> + * drm_edid_quirk_process - process a newly parsed EDID quirk
> + * @quirk: the quirk to be processed
> + *
> + * Depending on the newly parsed quirk and the contents of the quirks list, this
> + * function will add, remove, or replace a quirk.
> + *
> + * Returns 0 on success, < 0 on error (-ENOSPC if there is no free slot for a
> + * new quirk). Note that trying to remove a quirk that isn't present is not
> + * considered an error.
> + */
> +static int drm_edid_quirk_process(const union edid_quirk *quirk)
> +{
> +	char buf[EDID_QUIRK_BUF_SIZE];
> +	union edid_quirk *q;
> +	int res = 0;
> +
> +	mutex_lock(&edid_quirk_list_mutex);
> +
> +	if (quirk->s.quirks == 0) {
> +		DRM_INFO("Removing EDID quirk for display %s\n",
> +			 drm_edid_display_id_format(quirk->s.display_id,
> +						    buf, 1));
> +		q = drm_edid_quirk_find_by_id(quirk->s.display_id);
> +		if (q == NULL) {
> +			printk(KERN_WARNING "No quirk found for display %s\n",
> +			       drm_edid_display_id_format(quirk->s.display_id,
> +							  buf, 1));
> +		} else {
> +			q->u = 0;
> +		}
> +	} else {
> +		DRM_INFO("Adding EDID quirk: %s\n",
> +			 drm_edid_quirk_format(quirk, buf, 1));
> +		q = drm_edid_quirk_find_by_id(quirk->s.display_id);
> +		if (q == NULL) {
> +			q = drm_edid_quirk_find_empty();
> +			if (q == NULL) {
> +				printk(KERN_WARNING
> +				       "No free slot in EDID quirk list\n");
> +				res = -ENOSPC;
> +			} else {
> +				q->u = quirk->u;
> +			}
> +		} else {
> +			DRM_INFO("Replacing existing quirk: %s\n",
> +				 drm_edid_quirk_format(q, buf, 1));
> +			q->s.quirks = quirk->s.quirks;
> +		}
> +	}
> +
> +	mutex_unlock(&edid_quirk_list_mutex);
> +
> +	return res;
> +}
> +
> +/**
> + * drm_edid_quirks_process - parse and process a comma separated list of EDID
> + * 			     quirks
> + * @s: string containing the quirks to be processed
> + * @strict: if non-zero, any parsing or processing error aborts further
> + * 	    processing
> + *
> + * Returns 0 on success, < 0 if any error is encountered.  (If multiple errors
> + * occur when strict is set to 0, the last error encountered is returned.)
> + */
> +static int drm_edid_quirks_process(const char *s, int strict)
> +{
> +	union edid_quirk quirk;
> +	int res = 0;
> +
> +	do {
> +
> +		if (*s == '@') {
> +			DRM_INFO("Clearing EDID quirk list\n");
> +			mutex_lock(&edid_quirk_list_mutex);
> +			memset(edid_quirk_list, 0, sizeof edid_quirk_list);
> +			mutex_unlock(&edid_quirk_list_mutex);
> +		} else {
> +			res = drm_edid_quirk_parse(s, &quirk);
> +			if (res != 0) {
> +				if (strict)
> +					goto error;
> +				continue;
> +			}
> +
> +			res = drm_edid_quirk_process(&quirk);
> +			if (res != 0) {
> +				if (strict)
> +					goto error;
> +			}
> +		}
> +
> +		s = strpbrk(s, ",\n");
> +
> +	} while (s != NULL && *(++s) != 0);
> +
> +	return res;
> +
> +error:
> +	printk(KERN_WARNING "Aborting EDID quirk parsing\n");
> +	return res;
> +}
> +
> +/**
> + * drm_edid_quirks_param_process - process the edid_quirks module parameter
> + */
> +void drm_edid_quirks_param_process(void)
> +{
> +	if (drm_edid_quirks != NULL)
> +		drm_edid_quirks_process(drm_edid_quirks, 0);
> +}
> +
> +/**
> + * drm_edid_quirks_size_show - show the size of the EDID quirk list in sysfs
> + * @buf: destination buffer (PAGE_SIZE bytes)
> + */
> +ssize_t drm_edid_quirks_size_show(struct class *class,
> +				  struct class_attribute *attr, char *buf)
> +{
> +	return sprintf(buf, "%zu\n", ARRAY_SIZE(edid_quirk_list));
> +}
> +
> +/**
> + * drm_edid_quirks_show - show the contents of the EDID quirk list in sysfs
> + * @buf: destination buffer (PAGE_SIZE bytes)
> + */
> +ssize_t drm_edid_quirks_show(struct class *class, struct class_attribute *attr,
> +			     char *buf)
> +{
> +	const union edid_quirk *q = edid_quirk_list;
> +	ssize_t count = 0;
> +
> +	BUILD_BUG_ON(ARRAY_SIZE(edid_quirk_list) >
> +				PAGE_SIZE / EDID_QUIRK_BUF_SIZE);
> +
> +	mutex_lock(&edid_quirk_list_mutex);
> +
> +	do {
> +		if (q->s.quirks != 0) {
> +			drm_edid_quirk_format(q, buf + count, 0);
> +			(buf + count)[EDID_QUIRK_BUF_SIZE - 1] = '\n';
> +			count += EDID_QUIRK_BUF_SIZE;
> +		}
> +	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
> +
> +	mutex_unlock(&edid_quirk_list_mutex);
> +
> +	return count;
> +}
> +
> +/**
> + * drm_edid_quirks_store - parse and process EDID quirkl list changes written
> + *			   to sysfs attribute
> + */
> +ssize_t drm_edid_quirks_store(struct class *class, struct class_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	int res;
> +
> +	res = drm_edid_quirks_process(buf, 1);
> +	if (res != 0)
> +		return res;
> +
> +	return count;
> +}

Add an empty line here before the next comment?

>  /*** DDC fetch and block validation ***/
>  
>  static const u8 edid_header[] = {
> @@ -409,25 +828,6 @@ EXPORT_SYMBOL(drm_get_edid);
>  /*** EDID parsing ***/
>  
>  /**
> - * edid_vendor - match a string against EDID's obfuscated vendor field
> - * @edid: EDID to match
> - * @vendor: vendor string
> - *
> - * Returns true if @vendor is in @edid, false otherwise
> - */
> -static bool edid_vendor(struct edid *edid, char *vendor)
> -{
> -	char edid_vendor[3];
> -
> -	edid_vendor[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
> -	edid_vendor[1] = (((edid->mfg_id[0] & 0x3) << 3) |
> -			  ((edid->mfg_id[1] & 0xe0) >> 5)) + '@';
> -	edid_vendor[2] = (edid->mfg_id[1] & 0x1f) + '@';
> -
> -	return !strncmp(edid_vendor, vendor, 3);
> -}
> -
> -/**
>   * edid_get_quirks - return quirk flags for a given EDID
>   * @edid: EDID to process
>   *
> @@ -435,18 +835,18 @@ static bool edid_vendor(struct edid *edid, char *vendor)
>   */
>  static u32 edid_get_quirks(struct edid *edid)
>  {
> -	struct edid_quirk *quirk;
> -	int i;
> +	union edid_quirk *q;
> +	u32 quirks = 0;
>  
> -	for (i = 0; i < ARRAY_SIZE(edid_quirk_list); i++) {
> -		quirk = &edid_quirk_list[i];
> +	mutex_lock(&edid_quirk_list_mutex);
>  
> -		if (edid_vendor(edid, quirk->vendor) &&
> -		    (EDID_PRODUCT_ID(edid) == quirk->product_id))
> -			return quirk->quirks;
> -	}
> +	q = drm_edid_quirk_find_by_id(edid->display_id);
> +	if (q != NULL)
> +		quirks = q->s.quirks;
>  
> -	return 0;
> +	mutex_unlock(&edid_quirk_list_mutex);
> +
> +	return quirks;
>  }
>  
>  #define MODE_SIZE(m) ((m)->hdisplay * (m)->vdisplay)
> @@ -1162,7 +1562,7 @@ do_inferred_modes(struct detailed_timing *timing, void *c)
>  	closure->modes += drm_dmt_modes_for_range(closure->connector,
>  						  closure->edid,
>  						  timing);
> -	
> +
>  	if (!version_greater(closure->edid, 1, 1))
>  		return; /* GTF not defined yet */
>  
> @@ -1399,7 +1799,7 @@ do_cvt_mode(struct detailed_timing *timing, void *c)
>  
>  static int
>  add_cvt_modes(struct drm_connector *connector, struct edid *edid)
> -{	
> +{
>  	struct detailed_mode_closure closure = {
>  		connector, edid, 0, 0, 0
>  	};
> @@ -1615,15 +2015,12 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
>  
>  	eld[0] = 2 << 3;		/* ELD version: 2 */
>  
> -	eld[16] = edid->mfg_id[0];
> -	eld[17] = edid->mfg_id[1];
> -	eld[18] = edid->prod_code[0];
> -	eld[19] = edid->prod_code[1];
> +	*(u32 *)(&eld[16]) = edid->display_id.u;
>  
>  	if (cea[1] >= 3)
>  		for (db = cea + 4; db < cea + cea[2]; db += dbl + 1) {
>  			dbl = db[0] & 0x1f;
> -			
> +
>  			switch ((db[0] & 0xe0) >> 5) {
>  			case AUDIO_BLOCK:
>  				/* Audio Data Block, contains SADs */
> @@ -1723,6 +2120,14 @@ bool drm_detect_hdmi_monitor(struct edid *edid)
>  	int i, hdmi_id;
>  	int start_offset, end_offset;
>  	bool is_hdmi = false;
> +	char buf[EDID_DISPLAY_ID_BUF_SIZE];
> +
> +	if (edid_get_quirks(edid) & EDID_QUIRK_DISABLE_INFOFRAMES) {
> +		DRM_INFO("Disabling HDMI InfoFrames on display %s "
> +			 "due to EDID quirk\n",
> +			 drm_edid_display_id_format(edid->display_id, buf, 1));
> +		goto end;
> +	}
>  
>  	edid_ext = drm_find_cea_extension(edid);
>  	if (!edid_ext)
> @@ -1771,6 +2176,14 @@ bool drm_detect_monitor_audio(struct edid *edid)
>  	int i, j;
>  	bool has_audio = false;
>  	int start_offset, end_offset;
> +	char buf[EDID_DISPLAY_ID_BUF_SIZE];
> +
> +	if (edid_get_quirks(edid) & EDID_QUIRK_NO_AUDIO) {
> +		DRM_INFO("Disabling HDMI audio on display %s "
> +			 "due to EDID quirk\n",
> +			 drm_edid_display_id_format(edid->display_id, buf, 1));
> +		goto end;
> +	}
>  
>  	edid_ext = drm_find_cea_extension(edid);
>  	if (!edid_ext)
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 21bcd4a..1885fc9 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -46,16 +46,21 @@ EXPORT_SYMBOL(drm_vblank_offdelay);
>  unsigned int drm_timestamp_precision = 20;  /* Default to 20 usecs. */
>  EXPORT_SYMBOL(drm_timestamp_precision);
>  
> +char *drm_edid_quirks = NULL;
> +EXPORT_SYMBOL(drm_edid_quirks);
> +
>  MODULE_AUTHOR(CORE_AUTHOR);
>  MODULE_DESCRIPTION(CORE_DESC);
>  MODULE_LICENSE("GPL and additional rights");
>  MODULE_PARM_DESC(debug, "Enable debug output");
>  MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable [msecs]");
>  MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps [usecs]");
> +MODULE_PARM_DESC(edid_quirks, "See Documentation/EDID/edid_quirks.txt");

Not all users have access to the Linux source tree, so maybe a small
overview is still needed? Or even an URL?

>  
>  module_param_named(debug, drm_debug, int, 0600);
>  module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600);
>  module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600);
> +module_param_named(edid_quirks, drm_edid_quirks, charp, 0400);
>  
>  struct idr drm_minors_idr;
>  
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 45ac8d6..84dc365 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -84,6 +84,11 @@ static CLASS_ATTR_STRING(version, S_IRUGO,
>  		__stringify(CORE_PATCHLEVEL) " "
>  		CORE_DATE);
>  
> +static CLASS_ATTR(edid_quirks_size, 0400, drm_edid_quirks_size_show, 0);
> +
> +static CLASS_ATTR(edid_quirks, 0600, drm_edid_quirks_show,
> +		  drm_edid_quirks_store);
> +
>  /**
>   * drm_sysfs_create - create a struct drm_sysfs_class structure
>   * @owner: pointer to the module that is to "own" this struct drm_sysfs_class
> @@ -113,10 +118,22 @@ struct class *drm_sysfs_create(struct module *owner, char *name)
>  	if (err)
>  		goto err_out_class;
>  
> +	err = class_create_file(class, &class_attr_edid_quirks_size);
> +	if (err)
> +		goto err_out_version;
> +
> +	err = class_create_file(class, &class_attr_edid_quirks);
> +	if (err)
> +		goto err_out_quirks_size;
> +
>  	class->devnode = drm_devnode;
>  
>  	return class;
>  
> +err_out_quirks_size:
> +	class_remove_file(class, &class_attr_edid_quirks_size);
> +err_out_version:
> +	class_remove_file(class, &class_attr_version.attr);
>  err_out_class:
>  	class_destroy(class);
>  err_out:
> @@ -132,6 +149,8 @@ void drm_sysfs_destroy(void)
>  {
>  	if ((drm_class == NULL) || (IS_ERR(drm_class)))
>  		return;
> +	class_remove_file(drm_class, &class_attr_edid_quirks);
> +	class_remove_file(drm_class, &class_attr_edid_quirks_size);
>  	class_remove_file(drm_class, &class_attr_version.attr);
>  	class_destroy(drm_class);
>  	drm_class = NULL;
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index d6b67bb..c947f3e 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1501,6 +1501,7 @@ extern unsigned int drm_debug;
>  
>  extern unsigned int drm_vblank_offdelay;
>  extern unsigned int drm_timestamp_precision;
> +extern char *drm_edid_quirks;
>  
>  extern struct class *drm_class;
>  extern struct proc_dir_entry *drm_proc_root;
> @@ -1612,6 +1613,15 @@ void drm_gem_vm_open(struct vm_area_struct *vma);
>  void drm_gem_vm_close(struct vm_area_struct *vma);
>  int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
>  
> +					/* EDID support (drm_edid.c) */
> +void drm_edid_quirks_param_process(void);
> +ssize_t drm_edid_quirks_size_show(struct class *class,
> +				  struct class_attribute *attr, char *buf);
> +ssize_t drm_edid_quirks_show(struct class *class, struct class_attribute *attr,
> +			     char *buf);
> +ssize_t drm_edid_quirks_store(struct class *class, struct class_attribute *attr,
> +			      const char *buf, size_t count);
> +
>  #include "drm_global.h"
>  
>  static inline void
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 0cac551..713229b 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -202,11 +202,18 @@ struct detailed_timing {
>  #define DRM_EDID_FEATURE_PM_SUSPEND       (1 << 6)
>  #define DRM_EDID_FEATURE_PM_STANDBY       (1 << 7)
>  
> +union edid_display_id {
> +	struct {
> +		__be16 mfg_id;
> +		__le16 prod_code;
> +	} __attribute__((packed)) s;
> +	u32 u;
> +};
> +
>  struct edid {
>  	u8 header[8];
>  	/* Vendor & product info */
> -	u8 mfg_id[2];
> -	u8 prod_code[2];
> +	union edid_display_id display_id;
>  	u32 serial; /* FIXME: byte order */
>  	u8 mfg_week;
>  	u8 mfg_year;
> @@ -242,8 +249,6 @@ struct edid {
>  	u8 checksum;
>  } __attribute__((packed));
>  
> -#define EDID_PRODUCT_ID(e) ((e)->prod_code[0] | ((e)->prod_code[1] << 8))
> -
>  struct drm_encoder;
>  struct drm_connector;
>  struct drm_display_mode;


Thanks again for that great patch. With the comments addressed above you
can add my acknowledgment.

Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>

I am going to try to test that patch too for a Philips and LG TV [2].


Thanks,

Paul


[2] https://bugs.freedesktop.org/show_bug.cgi?id=26294

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] drm: EDID quirk improvements
  2012-08-11  8:31                     ` Paul Menzel
@ 2012-08-11 15:38                       ` Ian Pilcher
  2012-08-11 19:52                         ` Paul Menzel
  0 siblings, 1 reply; 32+ messages in thread
From: Ian Pilcher @ 2012-08-11 15:38 UTC (permalink / raw)
  To: Paul Menzel; +Cc: dri-devel

On 08/11/2012 03:31 AM, Paul Menzel wrote:
> As a side note, could you also mention the patch iteration in the tag,
> that means [PATCH vN] so that I know what is the latest version. That
> would be great.

Can you (or anyone else reading this) point me to how to do this with
git send-email?

> I would submit adding the new quirk flags and the LG quirk as separate
> patches. If it is not too much work, it would be great if you could
> split them up.

Does git provide any facility to make this easier?  As far as I can
tell, the process is to start over with a newly cloned repository,
apply the current patch, manually back out the changes that I want to
separate, commit, manually redo the separate changes, and commit
again.  Is that correct?

> You even added this one from the following commit.
> 
>         commit bc42aabc6a01b92b0f961d65671564e0e1cd7592
>         Author: Adam Jackson <ajax@redhat.com>
>         Date:   Wed May 23 16:26:54 2012 -0400
>         
>             drm/edid/quirks: ViewSonic VA2026w
> 
> I am going to need that quirk. Great!

It's in Linus's tree.

>> +DEFINE_MUTEX(edid_quirk_list_mutex);
>> +
>> +/**
>> + * drm_edid_mfg_format - format an "encoded" EDID manufacturer ID for printing
>> + * @mfg_id: the encoded manufacturer ID
>> + * @buf: destination buffer for the formated manufacturer ID (minimum 7 bytes)
> 
> format*t*ed
> 
> http://www.merriam-webster.com/dictionary/formatted

Thanks for catching that.  I keep finding little typos like that; very
annoying.

>> +
>> +	return count;
>> +}
> 
> Add an empty line here before the next comment?
> 
>>  /*** DDC fetch and block validation ***/

I'm pretty sure I can manage that.  ;-)

>> +MODULE_PARM_DESC(edid_quirks, "See Documentation/EDID/edid_quirks.txt");
> 
> Not all users have access to the Linux source tree, so maybe a small
> overview is still needed? Or even an URL?

I can't think of a way to provide anything useful within the scope of a
parameter description.  Any suggestions?

A URL would be great, but what would it be?  (I don't have a personal
web site, and that doesn't seem really appropriate anyway.)

> Thanks again for that great patch. With the comments addressed above you
> can add my acknowledgment.
> 
> Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>

Thank you for your feedback.

Are you saying that I should add the acked-by?  If so, how?  (You can
probably tell that I'm really struggling with git.)

> I am going to try to test that patch too for a Philips and LG TV [2].

I hope it helps.

-- 
========================================================================
Ian Pilcher                                         arequipeno@gmail.com
"If you're going to shift my paradigm ... at least buy me dinner first."
========================================================================

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] drm: EDID quirk improvements
  2012-08-10 18:44                   ` Ian Pilcher
  2012-08-11  8:31                     ` Paul Menzel
@ 2012-08-11 19:37                     ` Paul Menzel
  2012-08-12  4:30                       ` [PATCH v3 0/2] Enhanced EDID quirk functionality Ian Pilcher
  1 sibling, 1 reply; 32+ messages in thread
From: Paul Menzel @ 2012-08-11 19:37 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 3965 bytes --]

Dear Ian,


Am Freitag, den 10.08.2012, 13:44 -0500 schrieb Ian Pilcher:

[…]

> diff --git a/Documentation/EDID/edid_quirks.txt b/Documentation/EDID/edid_quirks.txt
> new file mode 100644
> index 0000000..256ded0
> --- /dev/null
> +++ b/Documentation/EDID/edid_quirks.txt

[…]

> +Overview
> +========
> +
> +EDID quirks provide a mechanism for working around display hardware with buggy
> +EDID data.
> +
> +An individual EDID quirk maps a display type (identified by its EDID
> +manufacturer ID and product code[1]) to a set of flags. For example, the current
> +list of quirks built into the kernel is:
> +
> +    ACR:0xad46:0x00000001
> +    API:0x7602:0x00000001
> +    ACR:0x0977:0x00000020
> +    MAX:0x05ec:0x00000001
> +    MAX:0x077e:0x00000001
> +    EPI:0xe780:0x00000002
> +    EPI:0x2028:0x00000001
> +    FCM:0x3520:0x0000000c
> +    LPL:0x0000:0x00000010
> +    LPL:0x2a00:0x00000010
> +    PHL:0xe014:0x00000020
> +    PTS:0x02fd:0x00000020
> +    SAM:0x021d:0x00000040
> +    SAM:0x0254:0x00000001
> +    SAM:0x027e:0x00000001
> +    VSC:0x139c:0x00000080
> +    GSM:0x563f:0x00000300

reading the document again, I guess keeping this list up to date will be
forgotten and duplicating this information is not necessary. Maybe just
add one or two example quirks.

I cannot think of a `grep` command to run to list all quirks, so maybe
just mention that all the quirks are stored in `edid_quirk_list[]` in
`drivers/gpu/drm/drm_edid.c` [1].

> +
> +The first field of each quirk is the manufacturer ID, the second field is the
> +product code, and the third field is the quirk flags.
> +
> +NOTE: All of the manufacturer IDs above are displayed as 3-character strings,
> +    because they are conformant IDs that have been properly encoded:
> +
> +    - The most-significant bit of the encoded ID is 0
> +    - They only contain ASCII characters in the range A-Z
> +
> +    IDs that do not conform to these rules are displayed as "raw" hexadecimal
> +    values.
> +
> +The current quirk flags are:
> +
> +    /* First detailed mode wrong, use largest 60Hz mode */
> +    #define EDID_QUIRK_PREFER_LARGE_60                  0x00000001
> +
> +    /* Reported 135MHz pixel clock is too high, needs adjustment */
> +    #define EDID_QUIRK_135_CLOCK_TOO_HIGH               0x00000002
> +
> +    /* Prefer the largest mode at 75 Hz */
> +    #define EDID_QUIRK_PREFER_LARGE_75                  0x00000004
> +
> +    /* Detail timing is in cm not mm */
> +    #define EDID_QUIRK_DETAILED_IN_CM                   0x00000008
> +
> +    /* Detailed timing descriptors have bogus size values, so just take the
> +     * maximum size and use that.
> +     */
> +    #define EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE        0x00000010
> +
> +    /* Monitor forgot to set the first detailed is preferred bit. */
> +    #define EDID_QUIRK_FIRST_DETAILED_PREFERRED         0x00000020
> +
> +    /* use +hsync +vsync for detailed mode */
> +    #define EDID_QUIRK_DETAILED_SYNC_PP                 0x00000040
> +
> +    /* Force reduced-blanking timings for detailed modes */
> +    #define EDID_QUIRK_FORCE_REDUCED_BLANKING           0x00000080
> +
> +    /* Display is confused by InfoFrames; don't sent any */
> +    #define EDID_QUIRK_DISABLE_INFOFRAMES               0x00000100
> +
> +    /* Display doesn't have any audio output */
> +    #define EDID_QUIRK_NO_AUDIO                         0x00000200

That might be also hard to keep up to date. Maybe also just note that
these quirks are defined in the beginning of
`drivers/gpu/drm/drm_edid.c` [1] and that these are bit shifts(?) [2].

[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/gpu/drm/drm_edid.c;h=a8743c399e83234c976ebdb4b471542a0645c42d;hb=HEAD
[2] https://en.wikipedia.org/wiki/Bitwise_operation#Shifts_in_C.2C_C.2B.2B.2C_C.23


[…]


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH] drm: EDID quirk improvements
  2012-08-11 15:38                       ` Ian Pilcher
@ 2012-08-11 19:52                         ` Paul Menzel
  0 siblings, 0 replies; 32+ messages in thread
From: Paul Menzel @ 2012-08-11 19:52 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 3602 bytes --]

Dear Ian,


Am Samstag, den 11.08.2012, 10:38 -0500 schrieb Ian Pilcher:
> On 08/11/2012 03:31 AM, Paul Menzel wrote:
> > As a side note, could you also mention the patch iteration in the tag,
> > that means [PATCH vN] so that I know what is the latest version. That
> > would be great.
> 
> Can you (or anyone else reading this) point me to how to do this with
> git send-email?

No, but you can do that with `git format-patch` [3].

    git format-patch --subject-prefix="PATCH v6" …

> > I would submit adding the new quirk flags and the LG quirk as separate
> > patches. If it is not too much work, it would be great if you could
> > split them up.
> 
> Does git provide any facility to make this easier?  As far as I can
> tell, the process is to start over with a newly cloned repository,
> apply the current patch, manually back out the changes that I want to
> separate, commit, manually redo the separate changes, and commit
> again.  Is that correct?

Kind of. But you can do that in the same clone within your
checkout/clone.

Basically you go before your commit and then use `git add -p` to add
only the hunks to the commit you need [4][5]. Do not worry that your
changes get lost. `git reflog` still shows you the commit hash of your
original patch. You can always go back to that by doing `git reset
--hard <that hash>` or checking it out in another branch `git checkout
-b myoldpatch_v5 <that hash>`.

You can read the manual with `git help commandname` like `git help add`.
The folks in #git on irc.freenode.net are also very helpful.

[…]

> >> +DEFINE_MUTEX(edid_quirk_list_mutex);
> >> +
> >> +/**
> >> + * drm_edid_mfg_format - format an "encoded" EDID manufacturer ID for printing
> >> + * @mfg_id: the encoded manufacturer ID
> >> + * @buf: destination buffer for the formated manufacturer ID (minimum 7 bytes)
> > 
> > format*t*ed
> > 
> > http://www.merriam-webster.com/dictionary/formatted
> 
> Thanks for catching that.  I keep finding little typos like that; very
> annoying.

Well, it was the only typo I found. I thought you are using a spell
checker already.

[…]

> I can't think of a way to provide anything useful within the scope of a
> parameter description.  Any suggestions?

Maybe just say give an example `vendor:model:quirk`.

> A URL would be great, but what would it be?  (I don't have a personal
> web site, and that doesn't seem really appropriate anyway.)

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/gpu/drm/drm_edid.c

> > Thanks again for that great patch. With the comments addressed above you
> > can add my acknowledgment.
> > 
> > Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
> 
> Thank you for your feedback.
> 
> Are you saying that I should add the acked-by?  If so, how?  (You can
> probably tell that I'm really struggling with git.)

That is very easy: `git commit --amend` to just edit the commit message.

You can edit the other changes above, by editing the appropriate files
and then doing `git commit --amend -a`. This adds all changes in files
known to Git also to the last commit.

> > I am going to try to test that patch too for a Philips and LG TV [2].
> 
> I hope it helps.

We will see tomorrow.


Thanks,

Paul


[3] http://wireless.kernel.org/en/developers/Documentation/git-guide#Annotating_new_revision
[4] http://stackoverflow.com/questions/4307095/git-how-to-split-up-a-commit-buried-in-history
[5] http://git.661346.n2.nabble.com/How-to-split-a-big-commit-td6238260.html

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v3 0/2] Enhanced EDID quirk functionality
  2012-08-11 19:37                     ` Paul Menzel
@ 2012-08-12  4:30                       ` Ian Pilcher
  2012-08-12  4:30                         ` [PATCH v3 1/2] drm: Add user-defined EDID quirks capability Ian Pilcher
                                           ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: Ian Pilcher @ 2012-08-12  4:30 UTC (permalink / raw)
  To: dri-devel; +Cc: Ian Pilcher, paulepanter

Updated patch set, based on Paul's feedback.

* Separate user-defined quirks stuff from new HDMI-related quirks
* (Hopefully) improve documentation

Also continuing to explore the wonders of git format-patch and git
send-email.  

Ian Pilcher (2):
  drm: Add user-defined EDID quirks capability
  drm: Add EDID quirks to disable HDMI audio and InfoFrames

 Documentation/EDID/edid_quirks.txt | 126 +++++++++
 drivers/gpu/drm/drm_drv.c          |   2 +
 drivers/gpu/drm/drm_edid.c         | 524 +++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/drm_stub.c         |   6 +
 drivers/gpu/drm/drm_sysfs.c        |  19 ++
 include/drm/drmP.h                 |  10 +
 include/drm/drm_edid.h             |  13 +-
 7 files changed, 639 insertions(+), 61 deletions(-)
 create mode 100644 Documentation/EDID/edid_quirks.txt

-- 
1.7.11.2

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v3 1/2] drm: Add user-defined EDID quirks capability
  2012-08-12  4:30                       ` [PATCH v3 0/2] Enhanced EDID quirk functionality Ian Pilcher
@ 2012-08-12  4:30                         ` Ian Pilcher
  2012-08-12 15:39                           ` Paul Menzel
  2012-08-12  4:30                         ` [PATCH v3 2/2] drm: Add EDID quirks to disable HDMI audio and InfoFrames Ian Pilcher
  2012-08-12  6:58                         ` [PATCH v3 0/2] Enhanced EDID quirk functionality Paul Menzel
  2 siblings, 1 reply; 32+ messages in thread
From: Ian Pilcher @ 2012-08-12  4:30 UTC (permalink / raw)
  To: dri-devel; +Cc: Ian Pilcher, paulepanter

Add the ability for users to define their own EDID quirks via a
module parameter or sysfs attribute.

Signed-off-by: Ian Pilcher <arequipeno@gmail.com>
Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
---
 Documentation/EDID/edid_quirks.txt | 126 ++++++++++
 drivers/gpu/drm/drm_drv.c          |   2 +
 drivers/gpu/drm/drm_edid.c         | 500 ++++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/drm_stub.c         |   6 +
 drivers/gpu/drm/drm_sysfs.c        |  19 ++
 include/drm/drmP.h                 |  10 +
 include/drm/drm_edid.h             |  13 +-
 7 files changed, 615 insertions(+), 61 deletions(-)
 create mode 100644 Documentation/EDID/edid_quirks.txt

diff --git a/Documentation/EDID/edid_quirks.txt b/Documentation/EDID/edid_quirks.txt
new file mode 100644
index 0000000..0c9c746
--- /dev/null
+++ b/Documentation/EDID/edid_quirks.txt
@@ -0,0 +1,126 @@
+                                  EDID Quirks
+                                 =============
+                       Ian Pilcher <arequipeno@gmail.com>
+                                August 11, 2012
+
+
+    "EDID blocks out in the wild have a variety of bugs"
+        -- from drivers/gpu/drm/drm_edid.c
+
+
+Overview
+========
+
+EDID quirks provide a mechanism for working around display hardware with buggy
+EDID data.
+
+An individual EDID quirk maps a display type (identified by its EDID
+manufacturer ID and product code[1]) to a set of "quirk flags."  The kernel
+includes a variety of built-in quirks.  (They are stored in the edid_quirk_list
+array in drivers/gpu/drm/drm_edid.c.)
+
+An example of a built-in EDID quirk is:
+
+    ACR:0xad46:0x00000001
+
+The first field is the manufacturer ID (Acer, Inc.), the second field is the
+manufacturer's product code, and the third field contains the quirk flags for
+that display type.
+
+The quirk flags are defined in drivers/gpu/drm/drm_edid.c.  Each flag has a
+symbolic name beginning with EDID_QUIRK_, along with a numerical value.  Each
+flag should also have an associated comment which provides an idea of its
+effect.  Note that the values in the source file are expressed as bit shifts[2]:
+
+    * 1 << 0: 0x0001
+    * 1 << 1: 0x0002
+    * 1 << 2: 0x0004
+    * etc.
+
+
+sysfs interface
+===============
+
+The current EDID quirk list can be read from /sys/class/drm/edid_quirks:
+
+    # cat /sys/class/drm/edid_quirks
+       ACR:0xad46:0x00000001
+       API:0x7602:0x00000001
+       ACR:0x0977:0x00000020
+    0x9e6a:0x077e:0x00000080
+    ...
+
+("Nonconformant" manufacturer IDs are displayed as hexadecimal values.)
+
+The number of total "slots" in the list can be read from
+/sys/class/drm/edid_quirks_size.  This total includes both occupied slots (i.e.
+the current list) and any slots available for additional quirks.  The number of
+available slots can be calculated by subtracting the number of quirks in the
+current list from the total number of slots.
+
+If a slot is available, an additional quirk can be added to the list by writing
+it to /sys/class/drm/edid_quirks:
+
+    # echo FOO:0xffff:0x100 > /sys/class/drm/edid_quirks
+
+Manufacturer IDs can also be specified numerically.  (This is the only way to
+specify a nonconformant ID.) This command is equivalent to the previous one:
+
+    # echo 0x19ef:0xffff:0x100 > /sys/class/drm/edid_quirks
+
+Numeric values can also be specified in decimal or octal formats; a number that
+begins with a 0 is assumed to be octal:
+
+    # echo FOO:65535:0400 > /sys/class/drm/edid_quirks
+
+An existing quirk can be replaced by writing a new set of flags:
+
+    # echo FOO:0xffff:0x200 > /sys/class/drm/edid_quirks
+
+A quirk can be deleted from the list by writing an empty flag set (0). This
+makes the slot occupied by that quirk available.
+
+    # echo FOO:0xffff:0 > /sys/class/drm/edid_quirks
+
+Writing an "at symbol" (@) clears the entire quirk list:
+
+    # echo @ > /sys/class/drm/edid_quirks
+
+Multiple changes to the list can be specified in a comma (or newline) separated
+list. For example, the following command clears all of the existing quirks in
+the list and adds 3 new quirks:
+
+    # echo @,FOO:0xffff:0x100,BAR:0x1111:0x001,BAZ:0x2222:0x002 > \
+            /sys/class/drm/edid_quirks
+
+Note however, that any error (an incorrectly formatted quirk or an attempt to
+add a quirk when no slot is available) will abort processing of any further
+changes, potentially making it difficult to determine exactly which change
+caused the error and what changes were made.  For this reason, making changes
+one at a time is recommended, particularly if the changes are being made by a
+script or program.
+
+
+Module parameter
+================
+
+The EDID quirk list can also be modified via the edid_quirks module parameter
+(drm.edid_quirks on the kernel command line).  The effect of setting this
+parameter is identical to the effect of writing its value to
+/sys/class/drm/edid_quirks, with one important difference.  When an error is
+encountered during module parameter parsing or processing, any remaining quirks
+in the parameter string will still be processed.  (It is hoped that this approach
+maximizes the probability of producing a working display.)
+
+
+Follow-up
+=========
+
+If you encounter a display that requires an additional EDID quirk in order to
+function properly, please report it to the direct rendering development mailing
+list <dri-devel@lists.freedesktop.org>.
+
+
+[1] See http://en.wikipedia.org/wiki/Extended_display_identification_data for a
+    description of the manufacturer ID and product code fields.
+[2] https://en.wikipedia.org/wiki/Bitwise_operation#Bit_shifts
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 9238de4..7fe39e0 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -276,6 +276,8 @@ static int __init drm_core_init(void)
 		goto err_p3;
 	}
 
+	drm_edid_quirks_param_process();
+
 	DRM_INFO("Initialized %s %d.%d.%d %s\n",
 		 CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
 	return 0;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a8743c3..bb3ba20 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -31,6 +31,8 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/ctype.h>
+
 #include "drmP.h"
 #include "drm_edid.h"
 #include "drm_edid_modes.h"
@@ -82,51 +84,457 @@ struct detailed_mode_closure {
 #define LEVEL_GTF2	2
 #define LEVEL_CVT	3
 
-static struct edid_quirk {
-	char vendor[4];
-	int product_id;
-	u32 quirks;
-} edid_quirk_list[] = {
+union edid_quirk {
+	struct {
+		union edid_display_id display_id;
+		u32 quirks;
+	} __attribute__((packed)) s;
+	u64 u;
+};
+
+#define EDID_MFG_ID(c1, c2, c3)		cpu_to_be16(			\
+						(c1 & 0x1f) << 10 |	\
+						(c2 & 0x1f) << 5 |	\
+						(c3 & 0x1f)		\
+					)
+
+#define EDID_QUIRK_LIST_SIZE	24
+
+union edid_quirk edid_quirk_list[EDID_QUIRK_LIST_SIZE] = {
+
 	/* Acer AL1706 */
-	{ "ACR", 44358, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('A', 'C', 'R'), cpu_to_le16(44358) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 	/* Acer F51 */
-	{ "API", 0x7602, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('A', 'P', 'I'), cpu_to_le16(0x7602) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 	/* Unknown Acer */
-	{ "ACR", 2423, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+	{ { { { EDID_MFG_ID('A', 'C', 'R'), cpu_to_le16(2423) } },
+		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
 
 	/* Belinea 10 15 55 */
-	{ "MAX", 1516, EDID_QUIRK_PREFER_LARGE_60 },
-	{ "MAX", 0x77e, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('M', 'A', 'X'), cpu_to_le16(1516) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
+	{ { { { EDID_MFG_ID('M', 'A', 'X'), cpu_to_le16(0x77e) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 
 	/* Envision Peripherals, Inc. EN-7100e */
-	{ "EPI", 59264, EDID_QUIRK_135_CLOCK_TOO_HIGH },
+	{ { { { EDID_MFG_ID('E', 'P', 'I'), cpu_to_le16(59264) } },
+		EDID_QUIRK_135_CLOCK_TOO_HIGH } },
 	/* Envision EN2028 */
-	{ "EPI", 8232, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('E', 'P', 'I'), cpu_to_le16(8232) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 
 	/* Funai Electronics PM36B */
-	{ "FCM", 13600, EDID_QUIRK_PREFER_LARGE_75 |
-	  EDID_QUIRK_DETAILED_IN_CM },
+	{ { { { EDID_MFG_ID('F', 'C', 'M'), cpu_to_le16(13600) } },
+		EDID_QUIRK_PREFER_LARGE_75 | EDID_QUIRK_DETAILED_IN_CM } },
 
 	/* LG Philips LCD LP154W01-A5 */
-	{ "LPL", 0, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE },
-	{ "LPL", 0x2a00, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE },
+	{ { { { EDID_MFG_ID('L', 'P', 'L'), cpu_to_le16(0) } },
+		EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE } },
+	{ { { { EDID_MFG_ID('L', 'P', 'L'), cpu_to_le16(0x2a00) } },
+		EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE } },
 
 	/* Philips 107p5 CRT */
-	{ "PHL", 57364, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+	{ { { { EDID_MFG_ID('P', 'H', 'L'), cpu_to_le16(57364) } },
+		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
 
 	/* Proview AY765C */
-	{ "PTS", 765, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+	{ { { { EDID_MFG_ID('P', 'T', 'S'), cpu_to_le16(765) } },
+		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
 
 	/* Samsung SyncMaster 205BW.  Note: irony */
-	{ "SAM", 541, EDID_QUIRK_DETAILED_SYNC_PP },
+	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(541) } },
+		EDID_QUIRK_DETAILED_SYNC_PP } },
 	/* Samsung SyncMaster 22[5-6]BW */
-	{ "SAM", 596, EDID_QUIRK_PREFER_LARGE_60 },
-	{ "SAM", 638, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(596) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
+	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(638) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 
 	/* ViewSonic VA2026w */
-	{ "VSC", 5020, EDID_QUIRK_FORCE_REDUCED_BLANKING },
+	{ { { { EDID_MFG_ID('V', 'S', 'C'), cpu_to_le16(5020) } },
+		EDID_QUIRK_FORCE_REDUCED_BLANKING } },
+
+	/*
+	 * When adding built-in quirks, please adjust EDID_QUIRK_LIST_SIZE to
+	 * provide some room for user-supplied quirks.
+	 */
 };
 
+DEFINE_MUTEX(edid_quirk_list_mutex);
+
+/**
+ * drm_edid_mfg_format - format an "encoded" EDID manufacturer ID for printing
+ * @mfg_id: the encoded manufacturer ID
+ * @buf: destination buffer for the formatted manufacturer ID (minimum 7 bytes)
+ * @strip: if non-zero, the returned pointer will skip any leading spaces
+ *
+ * An EDID manufacturer ID is supposed to consist of 3 capital letters (A-Z).
+ * Each letter is stored as a 5-bit value between 1 and 26, taking up 15 bits of
+ * the 16-bit ID. The remaining bit should always be 0. If display manufacturers
+ * always did things correctly, however, EDID quirks wouldn't be required in
+ * the first place. This function does the following:
+ *
+ * - Broken IDs are printed in hexadecimal (0xffff).
+ * - "Correct" IDs are formatted as a 3-letter ID string, preceded by 3 spaces;
+ *   the spaces ensure that both output formats are the same length.
+ *
+ * Thus, a formatted manufacturer ID is always 6 characters long (not including
+ * the terminating 0).
+ *
+ * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
+ * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
+ * been formatted as a 3-letter string, a pointer to the first non-space
+ * character (@buf + 3) is returned.
+ */
+static const char *drm_edid_mfg_format(__be16 mfg_id, char *buf, int strip)
+{
+	u16 id = be16_to_cpu(mfg_id);
+
+	if (id & 0x8000)
+		goto bad_id;
+
+	buf[3] = ((id & 0x7c00) >> 10) + '@';
+	if (!isupper(buf[3]))
+		goto bad_id;
+
+	buf[4] = ((id & 0x03e0) >> 5) + '@';
+	if (!isupper(buf[4]))
+		goto bad_id;
+
+	buf[5] = (id & 0x001f) + '@';
+	if (!isupper(buf[5]))
+		goto bad_id;
+
+	memset(buf, ' ', 3);
+	buf[6] = 0;
+
+	return strip ? (buf + 3) : buf;
+
+bad_id:
+	sprintf(buf, "0x%04hx", id);
+	return buf;
+}
+
+#define EDID_MFG_BUF_SIZE		7
+
+/**
+ * drm_edid_display_id_format - format an EDID "display ID" (manufacturer ID
+ * 				and product code) for printing
+ * @display_id: the display ID
+ * @buf: destination buffer for the formatted display ID (minimum 14 bytes)
+ * @strip: if non-zero, the returned pointer will skip any leading spaces
+ *
+ * A formatted display ID is always 13 characters long (not including the
+ * terminating 0).
+ *
+ * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
+ * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
+ * been formatted as a 3-letter string, a pointer to the first non-space
+ * character (@buf + 3) is returned.
+ */
+static const char *drm_edid_display_id_format(union edid_display_id display_id,
+					      char *buf, int strip)
+{
+	const char *s;
+
+	s = drm_edid_mfg_format(display_id.s.mfg_id, buf, strip);
+	sprintf(buf + EDID_MFG_BUF_SIZE - 1, ":0x%04hx",
+		le16_to_cpu(display_id.s.prod_code));
+
+	return s;
+}
+
+#define EDID_DISPLAY_ID_BUF_SIZE	(EDID_MFG_BUF_SIZE + 7)
+
+/**
+ * drm_edid_quirk_format - format an EDID quirk for printing
+ * @quirk: the quirk
+ * @buf: destination buffer for the formatted quirk (minimum 25 bytes)
+ * @strip: if non-zero, the returned pointer will skip any leading spaces
+ *
+ * A formatted EDID quirk is always 24 characters long (not including the
+ * terminating 0).
+ *
+ * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
+ * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
+ * been formatted as a 3-letter string, a pointer to the first non-space
+ * character (@buf + 3) is returned.
+ */
+static const char *drm_edid_quirk_format(const union edid_quirk *quirk,
+					 char *buf, int strip)
+{
+	const char *s;
+
+	s = drm_edid_display_id_format(quirk->s.display_id, buf, strip);
+	sprintf(buf + EDID_DISPLAY_ID_BUF_SIZE - 1, ":0x%08x", quirk->s.quirks);
+
+	return s;
+}
+
+#define EDID_QUIRK_BUF_SIZE		(EDID_DISPLAY_ID_BUF_SIZE + 11)
+
+/**
+ * drm_edid_quirk_parse - parse an EDID quirk
+ * @s: string containing the quirk to be parsed
+ * @quirk: destination for parsed quirk
+ *
+ * Returns 0 on success, < 0 (currently -EINVAL) on error.
+ */
+static int drm_edid_quirk_parse(const char *s, union edid_quirk *quirk)
+{
+	char buf[EDID_QUIRK_BUF_SIZE];
+	s32 mfg;
+	s32 product;
+	s64 quirks;
+	char *c;
+
+	if (sscanf(s, "%i:%i:%lli", &mfg, &product, &quirks) == 3) {
+		if (mfg < 0 || mfg > 0xffff)
+			goto error;
+		quirk->s.display_id.s.mfg_id = cpu_to_be16((u16)mfg);
+	} else {
+		if (sscanf(s, "%3s:%i:%lli", buf, &product, &quirks) != 3 ||
+				!isupper(buf[0]) ||
+				!isupper(buf[1]) ||
+				!isupper(buf[2]))
+			goto error;
+		quirk->s.display_id.s.mfg_id =
+				EDID_MFG_ID(buf[0], buf[1], buf[2]);
+	}
+
+	if (product < 0 || product > 0xffff ||
+			quirks < 0 || quirks > 0xffffffffLL)
+		goto error;
+
+	quirk->s.display_id.s.prod_code = cpu_to_le16((u16)product);
+	quirk->s.quirks = (u32)quirks;
+
+	DRM_DEBUG("Successfully parsed EDID quirk: %s\n",
+		  drm_edid_quirk_format(quirk, buf, 1));
+
+	return 0;
+
+error:
+	c = strpbrk(s, ",\n");
+	if (c == NULL) {
+		printk(KERN_WARNING "Invalid EDID quirk: '%s'\n", s);
+	} else {
+		printk(KERN_WARNING "Invalid EDID quirk: '%.*s'\n",
+		      (int)(c - s), s);
+	}
+
+	return -EINVAL;
+}
+
+/**
+ * drm_edid_quirk_find_by_id - find the EDID quirk matching a display ID
+ * @display_id: the display ID to match
+ *
+ * Caller MUST hold edid_quirk_list_mutex.
+ *
+ * Returns a pointer to the matching quirk list entry, NULL if no such entry
+ * exists.
+ */
+static union edid_quirk *drm_edid_quirk_find_by_id(union edid_display_id id)
+{
+	union edid_quirk *q = edid_quirk_list;
+
+	do {
+		if (q->s.display_id.u == id.u && q->s.quirks != 0)
+			return q;
+	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
+
+	return NULL;
+}
+
+/**
+ * drm_edid_quirk_find_slot - find an empty slot in the EDID quirk list
+ *
+ * Caller MUST hold edid_quirk_list_mutex.
+ *
+ * Returns a pointer to the first empty slot, NULL if no empty slots exist.
+ */
+static union edid_quirk *drm_edid_quirk_find_empty(void)
+{
+	union edid_quirk *q = edid_quirk_list;
+
+	do {
+		if (q->s.quirks == 0)
+			return q;
+	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
+
+	return NULL;
+}
+
+/**
+ * drm_edid_quirk_process - process a newly parsed EDID quirk
+ * @quirk: the quirk to be processed
+ *
+ * Depending on the newly parsed quirk and the contents of the quirks list, this
+ * function will add, remove, or replace a quirk.
+ *
+ * Returns 0 on success, < 0 on error (-ENOSPC if there is no free slot for a
+ * new quirk). Note that trying to remove a quirk that isn't present is not
+ * considered an error.
+ */
+static int drm_edid_quirk_process(const union edid_quirk *quirk)
+{
+	char buf[EDID_QUIRK_BUF_SIZE];
+	union edid_quirk *q;
+	int res = 0;
+
+	mutex_lock(&edid_quirk_list_mutex);
+
+	if (quirk->s.quirks == 0) {
+		DRM_INFO("Removing EDID quirk for display %s\n",
+			 drm_edid_display_id_format(quirk->s.display_id,
+						    buf, 1));
+		q = drm_edid_quirk_find_by_id(quirk->s.display_id);
+		if (q == NULL) {
+			printk(KERN_WARNING "No quirk found for display %s\n",
+			       drm_edid_display_id_format(quirk->s.display_id,
+							  buf, 1));
+		} else {
+			q->u = 0;
+		}
+	} else {
+		DRM_INFO("Adding EDID quirk: %s\n",
+			 drm_edid_quirk_format(quirk, buf, 1));
+		q = drm_edid_quirk_find_by_id(quirk->s.display_id);
+		if (q == NULL) {
+			q = drm_edid_quirk_find_empty();
+			if (q == NULL) {
+				printk(KERN_WARNING
+				       "No free slot in EDID quirk list\n");
+				res = -ENOSPC;
+			} else {
+				q->u = quirk->u;
+			}
+		} else {
+			DRM_INFO("Replacing existing quirk: %s\n",
+				 drm_edid_quirk_format(q, buf, 1));
+			q->s.quirks = quirk->s.quirks;
+		}
+	}
+
+	mutex_unlock(&edid_quirk_list_mutex);
+
+	return res;
+}
+
+/**
+ * drm_edid_quirks_process - parse and process a comma separated list of EDID
+ * 			     quirks
+ * @s: string containing the quirks to be processed
+ * @strict: if non-zero, any parsing or processing error aborts further
+ * 	    processing
+ *
+ * Returns 0 on success, < 0 if any error is encountered.  (If multiple errors
+ * occur when strict is set to 0, the last error encountered is returned.)
+ */
+static int drm_edid_quirks_process(const char *s, int strict)
+{
+	union edid_quirk quirk;
+	int res = 0;
+
+	do {
+
+		if (*s == '@') {
+			DRM_INFO("Clearing EDID quirk list\n");
+			mutex_lock(&edid_quirk_list_mutex);
+			memset(edid_quirk_list, 0, sizeof edid_quirk_list);
+			mutex_unlock(&edid_quirk_list_mutex);
+		} else {
+			res = drm_edid_quirk_parse(s, &quirk);
+			if (res != 0) {
+				if (strict)
+					goto error;
+				continue;
+			}
+
+			res = drm_edid_quirk_process(&quirk);
+			if (res != 0) {
+				if (strict)
+					goto error;
+			}
+		}
+
+		s = strpbrk(s, ",\n");
+
+	} while (s != NULL && *(++s) != 0);
+
+	return res;
+
+error:
+	printk(KERN_WARNING "Aborting EDID quirk parsing\n");
+	return res;
+}
+
+/**
+ * drm_edid_quirks_param_process - process the edid_quirks module parameter
+ */
+void drm_edid_quirks_param_process(void)
+{
+	if (drm_edid_quirks != NULL)
+		drm_edid_quirks_process(drm_edid_quirks, 0);
+}
+
+/**
+ * drm_edid_quirks_size_show - show the size of the EDID quirk list in sysfs
+ * @buf: destination buffer (PAGE_SIZE bytes)
+ */
+ssize_t drm_edid_quirks_size_show(struct class *class,
+				  struct class_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%zu\n", ARRAY_SIZE(edid_quirk_list));
+}
+
+/**
+ * drm_edid_quirks_show - show the contents of the EDID quirk list in sysfs
+ * @buf: destination buffer (PAGE_SIZE bytes)
+ */
+ssize_t drm_edid_quirks_show(struct class *class, struct class_attribute *attr,
+			     char *buf)
+{
+	const union edid_quirk *q = edid_quirk_list;
+	ssize_t count = 0;
+
+	BUILD_BUG_ON(ARRAY_SIZE(edid_quirk_list) >
+				PAGE_SIZE / EDID_QUIRK_BUF_SIZE);
+
+	mutex_lock(&edid_quirk_list_mutex);
+
+	do {
+		if (q->s.quirks != 0) {
+			drm_edid_quirk_format(q, buf + count, 0);
+			(buf + count)[EDID_QUIRK_BUF_SIZE - 1] = '\n';
+			count += EDID_QUIRK_BUF_SIZE;
+		}
+	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
+
+	mutex_unlock(&edid_quirk_list_mutex);
+
+	return count;
+}
+
+/**
+ * drm_edid_quirks_store - parse and process EDID quirkl list changes written
+ *			   to sysfs attribute
+ */
+ssize_t drm_edid_quirks_store(struct class *class, struct class_attribute *attr,
+			      const char *buf, size_t count)
+{
+	int res;
+
+	res = drm_edid_quirks_process(buf, 1);
+	if (res != 0)
+		return res;
+
+	return count;
+}
+
 /*** DDC fetch and block validation ***/
 
 static const u8 edid_header[] = {
@@ -409,25 +817,6 @@ EXPORT_SYMBOL(drm_get_edid);
 /*** EDID parsing ***/
 
 /**
- * edid_vendor - match a string against EDID's obfuscated vendor field
- * @edid: EDID to match
- * @vendor: vendor string
- *
- * Returns true if @vendor is in @edid, false otherwise
- */
-static bool edid_vendor(struct edid *edid, char *vendor)
-{
-	char edid_vendor[3];
-
-	edid_vendor[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
-	edid_vendor[1] = (((edid->mfg_id[0] & 0x3) << 3) |
-			  ((edid->mfg_id[1] & 0xe0) >> 5)) + '@';
-	edid_vendor[2] = (edid->mfg_id[1] & 0x1f) + '@';
-
-	return !strncmp(edid_vendor, vendor, 3);
-}
-
-/**
  * edid_get_quirks - return quirk flags for a given EDID
  * @edid: EDID to process
  *
@@ -435,18 +824,18 @@ static bool edid_vendor(struct edid *edid, char *vendor)
  */
 static u32 edid_get_quirks(struct edid *edid)
 {
-	struct edid_quirk *quirk;
-	int i;
+	union edid_quirk *q;
+	u32 quirks = 0;
 
-	for (i = 0; i < ARRAY_SIZE(edid_quirk_list); i++) {
-		quirk = &edid_quirk_list[i];
+	mutex_lock(&edid_quirk_list_mutex);
 
-		if (edid_vendor(edid, quirk->vendor) &&
-		    (EDID_PRODUCT_ID(edid) == quirk->product_id))
-			return quirk->quirks;
-	}
+	q = drm_edid_quirk_find_by_id(edid->display_id);
+	if (q != NULL)
+		quirks = q->s.quirks;
 
-	return 0;
+	mutex_unlock(&edid_quirk_list_mutex);
+
+	return quirks;
 }
 
 #define MODE_SIZE(m) ((m)->hdisplay * (m)->vdisplay)
@@ -1162,7 +1551,7 @@ do_inferred_modes(struct detailed_timing *timing, void *c)
 	closure->modes += drm_dmt_modes_for_range(closure->connector,
 						  closure->edid,
 						  timing);
-	
+
 	if (!version_greater(closure->edid, 1, 1))
 		return; /* GTF not defined yet */
 
@@ -1399,7 +1788,7 @@ do_cvt_mode(struct detailed_timing *timing, void *c)
 
 static int
 add_cvt_modes(struct drm_connector *connector, struct edid *edid)
-{	
+{
 	struct detailed_mode_closure closure = {
 		connector, edid, 0, 0, 0
 	};
@@ -1615,15 +2004,12 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 
 	eld[0] = 2 << 3;		/* ELD version: 2 */
 
-	eld[16] = edid->mfg_id[0];
-	eld[17] = edid->mfg_id[1];
-	eld[18] = edid->prod_code[0];
-	eld[19] = edid->prod_code[1];
+	*(u32 *)(&eld[16]) = edid->display_id.u;
 
 	if (cea[1] >= 3)
 		for (db = cea + 4; db < cea + cea[2]; db += dbl + 1) {
 			dbl = db[0] & 0x1f;
-			
+
 			switch ((db[0] & 0xe0) >> 5) {
 			case AUDIO_BLOCK:
 				/* Audio Data Block, contains SADs */
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 21bcd4a..b939d51 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -46,16 +46,22 @@ EXPORT_SYMBOL(drm_vblank_offdelay);
 unsigned int drm_timestamp_precision = 20;  /* Default to 20 usecs. */
 EXPORT_SYMBOL(drm_timestamp_precision);
 
+char *drm_edid_quirks = NULL;
+EXPORT_SYMBOL(drm_edid_quirks);
+
 MODULE_AUTHOR(CORE_AUTHOR);
 MODULE_DESCRIPTION(CORE_DESC);
 MODULE_LICENSE("GPL and additional rights");
 MODULE_PARM_DESC(debug, "Enable debug output");
 MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable [msecs]");
 MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps [usecs]");
+MODULE_PARM_DESC(edid_quirks, "MFG:prod:flags[,MFG:prod:flags[...]]\n"
+			      "(See Documentation/EDID/edid_quirks.txt)");
 
 module_param_named(debug, drm_debug, int, 0600);
 module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600);
 module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600);
+module_param_named(edid_quirks, drm_edid_quirks, charp, 0400);
 
 struct idr drm_minors_idr;
 
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 45ac8d6..84dc365 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -84,6 +84,11 @@ static CLASS_ATTR_STRING(version, S_IRUGO,
 		__stringify(CORE_PATCHLEVEL) " "
 		CORE_DATE);
 
+static CLASS_ATTR(edid_quirks_size, 0400, drm_edid_quirks_size_show, 0);
+
+static CLASS_ATTR(edid_quirks, 0600, drm_edid_quirks_show,
+		  drm_edid_quirks_store);
+
 /**
  * drm_sysfs_create - create a struct drm_sysfs_class structure
  * @owner: pointer to the module that is to "own" this struct drm_sysfs_class
@@ -113,10 +118,22 @@ struct class *drm_sysfs_create(struct module *owner, char *name)
 	if (err)
 		goto err_out_class;
 
+	err = class_create_file(class, &class_attr_edid_quirks_size);
+	if (err)
+		goto err_out_version;
+
+	err = class_create_file(class, &class_attr_edid_quirks);
+	if (err)
+		goto err_out_quirks_size;
+
 	class->devnode = drm_devnode;
 
 	return class;
 
+err_out_quirks_size:
+	class_remove_file(class, &class_attr_edid_quirks_size);
+err_out_version:
+	class_remove_file(class, &class_attr_version.attr);
 err_out_class:
 	class_destroy(class);
 err_out:
@@ -132,6 +149,8 @@ void drm_sysfs_destroy(void)
 {
 	if ((drm_class == NULL) || (IS_ERR(drm_class)))
 		return;
+	class_remove_file(drm_class, &class_attr_edid_quirks);
+	class_remove_file(drm_class, &class_attr_edid_quirks_size);
 	class_remove_file(drm_class, &class_attr_version.attr);
 	class_destroy(drm_class);
 	drm_class = NULL;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d6b67bb..c947f3e 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1501,6 +1501,7 @@ extern unsigned int drm_debug;
 
 extern unsigned int drm_vblank_offdelay;
 extern unsigned int drm_timestamp_precision;
+extern char *drm_edid_quirks;
 
 extern struct class *drm_class;
 extern struct proc_dir_entry *drm_proc_root;
@@ -1612,6 +1613,15 @@ void drm_gem_vm_open(struct vm_area_struct *vma);
 void drm_gem_vm_close(struct vm_area_struct *vma);
 int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
 
+					/* EDID support (drm_edid.c) */
+void drm_edid_quirks_param_process(void);
+ssize_t drm_edid_quirks_size_show(struct class *class,
+				  struct class_attribute *attr, char *buf);
+ssize_t drm_edid_quirks_show(struct class *class, struct class_attribute *attr,
+			     char *buf);
+ssize_t drm_edid_quirks_store(struct class *class, struct class_attribute *attr,
+			      const char *buf, size_t count);
+
 #include "drm_global.h"
 
 static inline void
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 0cac551..713229b 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -202,11 +202,18 @@ struct detailed_timing {
 #define DRM_EDID_FEATURE_PM_SUSPEND       (1 << 6)
 #define DRM_EDID_FEATURE_PM_STANDBY       (1 << 7)
 
+union edid_display_id {
+	struct {
+		__be16 mfg_id;
+		__le16 prod_code;
+	} __attribute__((packed)) s;
+	u32 u;
+};
+
 struct edid {
 	u8 header[8];
 	/* Vendor & product info */
-	u8 mfg_id[2];
-	u8 prod_code[2];
+	union edid_display_id display_id;
 	u32 serial; /* FIXME: byte order */
 	u8 mfg_week;
 	u8 mfg_year;
@@ -242,8 +249,6 @@ struct edid {
 	u8 checksum;
 } __attribute__((packed));
 
-#define EDID_PRODUCT_ID(e) ((e)->prod_code[0] | ((e)->prod_code[1] << 8))
-
 struct drm_encoder;
 struct drm_connector;
 struct drm_display_mode;
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v3 2/2] drm: Add EDID quirks to disable HDMI audio and InfoFrames
  2012-08-12  4:30                       ` [PATCH v3 0/2] Enhanced EDID quirk functionality Ian Pilcher
  2012-08-12  4:30                         ` [PATCH v3 1/2] drm: Add user-defined EDID quirks capability Ian Pilcher
@ 2012-08-12  4:30                         ` Ian Pilcher
  2012-08-12 15:45                           ` Paul Menzel
  2012-08-12  6:58                         ` [PATCH v3 0/2] Enhanced EDID quirk functionality Paul Menzel
  2 siblings, 1 reply; 32+ messages in thread
From: Ian Pilcher @ 2012-08-12  4:30 UTC (permalink / raw)
  To: dri-devel; +Cc: Ian Pilcher, paulepanter

Add EDID quirk flags to disable HDMI audio and HDMI InfoFrames.
Add quirk for LG L246WP.

Signed-off-by: Ian Pilcher <arequipeno@gmail.com>
Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
---
 drivers/gpu/drm/drm_edid.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index bb3ba20..6c143ed 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -70,6 +70,10 @@
 #define EDID_QUIRK_DETAILED_SYNC_PP		(1 << 6)
 /* Force reduced-blanking timings for detailed modes */
 #define EDID_QUIRK_FORCE_REDUCED_BLANKING	(1 << 7)
+/* Display is confused by InfoFrames; don't sent any */
+#define EDID_QUIRK_DISABLE_INFOFRAMES		(1 << 8)
+/* Display doesn't have any audio output */
+#define EDID_QUIRK_NO_AUDIO			(1 << 9)
 
 struct detailed_mode_closure {
 	struct drm_connector *connector;
@@ -156,6 +160,10 @@ union edid_quirk edid_quirk_list[EDID_QUIRK_LIST_SIZE] = {
 	{ { { { EDID_MFG_ID('V', 'S', 'C'), cpu_to_le16(5020) } },
 		EDID_QUIRK_FORCE_REDUCED_BLANKING } },
 
+	/* LG L246WP */
+	{ { { { EDID_MFG_ID('G', 'S', 'M'), cpu_to_le16(0x563f) } },
+		EDID_QUIRK_DISABLE_INFOFRAMES | EDID_QUIRK_NO_AUDIO } },
+
 	/*
 	 * When adding built-in quirks, please adjust EDID_QUIRK_LIST_SIZE to
 	 * provide some room for user-supplied quirks.
@@ -2109,6 +2117,14 @@ bool drm_detect_hdmi_monitor(struct edid *edid)
 	int i, hdmi_id;
 	int start_offset, end_offset;
 	bool is_hdmi = false;
+	char buf[EDID_DISPLAY_ID_BUF_SIZE];
+
+	if (edid_get_quirks(edid) & EDID_QUIRK_DISABLE_INFOFRAMES) {
+		DRM_INFO("Disabling HDMI InfoFrames on display %s "
+			 "due to EDID quirk\n",
+			 drm_edid_display_id_format(edid->display_id, buf, 1));
+		goto end;
+	}
 
 	edid_ext = drm_find_cea_extension(edid);
 	if (!edid_ext)
@@ -2157,6 +2173,14 @@ bool drm_detect_monitor_audio(struct edid *edid)
 	int i, j;
 	bool has_audio = false;
 	int start_offset, end_offset;
+	char buf[EDID_DISPLAY_ID_BUF_SIZE];
+
+	if (edid_get_quirks(edid) & EDID_QUIRK_NO_AUDIO) {
+		DRM_INFO("Disabling HDMI audio on display %s "
+			 "due to EDID quirk\n",
+			 drm_edid_display_id_format(edid->display_id, buf, 1));
+		goto end;
+	}
 
 	edid_ext = drm_find_cea_extension(edid);
 	if (!edid_ext)
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v3 0/2] Enhanced EDID quirk functionality
  2012-08-12  4:30                       ` [PATCH v3 0/2] Enhanced EDID quirk functionality Ian Pilcher
  2012-08-12  4:30                         ` [PATCH v3 1/2] drm: Add user-defined EDID quirks capability Ian Pilcher
  2012-08-12  4:30                         ` [PATCH v3 2/2] drm: Add EDID quirks to disable HDMI audio and InfoFrames Ian Pilcher
@ 2012-08-12  6:58                         ` Paul Menzel
  2012-08-12 20:07                           ` [PATCH v4 0/3] " Ian Pilcher
  2 siblings, 1 reply; 32+ messages in thread
From: Paul Menzel @ 2012-08-12  6:58 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 713 bytes --]

Dear Ian,


thank you a million for doing that.


Am Samstag, den 11.08.2012, 23:30 -0500 schrieb Ian Pilcher:
> Updated patch set, based on Paul's feedback.
> 
> * Separate user-defined quirks stuff from new HDMI-related quirks
> * (Hopefully) improve documentation

Yeah, maybe somebody else can chime in.

> Also continuing to explore the wonders of git format-patch and git
> send-email.

Just for my interest, was it very hard or in the end it took “just”
half(?) an hour?

I found another typo and forgot to ask you to also add `CC:
stable@kernel.org` to the patches to get them backported to older Linux
releases. But I will reply to the individual patches.


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3 1/2] drm: Add user-defined EDID quirks capability
  2012-08-12  4:30                         ` [PATCH v3 1/2] drm: Add user-defined EDID quirks capability Ian Pilcher
@ 2012-08-12 15:39                           ` Paul Menzel
  0 siblings, 0 replies; 32+ messages in thread
From: Paul Menzel @ 2012-08-12 15:39 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1803 bytes --]

Dear Ian,


Am Samstag, den 11.08.2012, 23:30 -0500 schrieb Ian Pilcher:
> Add the ability for users to define their own EDID quirks via a
> module parameter or sysfs attribute.
> 
> Signed-off-by: Ian Pilcher <arequipeno@gmail.com>
> Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>

please also add

Cc: <stable@vger.kernel.org>

as documented in [1] so that users of older Linux versions can also
profit from your work.

[1] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=Documentation/stable_kernel_rules.txt;h=b0714d8f678ac51d0c280a4f5f2980196052421f;hb=HEAD

> ---
>  Documentation/EDID/edid_quirks.txt | 126 ++++++++++
>  drivers/gpu/drm/drm_drv.c          |   2 +
>  drivers/gpu/drm/drm_edid.c         | 500 ++++++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/drm_stub.c         |   6 +
>  drivers/gpu/drm/drm_sysfs.c        |  19 ++
>  include/drm/drmP.h                 |  10 +
>  include/drm/drm_edid.h             |  13 +-
>  7 files changed, 615 insertions(+), 61 deletions(-)
>  create mode 100644 Documentation/EDID/edid_quirks.txt

[…]

> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 9238de4..7fe39e0 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c

[…]

> +/**
> + * drm_edid_quirks_store - parse and process EDID quirkl list changes written

s/quirkl/quirk/

> + *			   to sysfs attribute
> + */
> +ssize_t drm_edid_quirks_store(struct class *class, struct class_attribute *attr,
> +			      const char *buf, size_t count)
> +{
> +	int res;
> +
> +	res = drm_edid_quirks_process(buf, 1);
> +	if (res != 0)
> +		return res;
> +
> +	return count;
> +}
> +
>  /*** DDC fetch and block validation ***/

[…]


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v3 2/2] drm: Add EDID quirks to disable HDMI audio and InfoFrames
  2012-08-12  4:30                         ` [PATCH v3 2/2] drm: Add EDID quirks to disable HDMI audio and InfoFrames Ian Pilcher
@ 2012-08-12 15:45                           ` Paul Menzel
  0 siblings, 0 replies; 32+ messages in thread
From: Paul Menzel @ 2012-08-12 15:45 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1113 bytes --]

Am Samstag, den 11.08.2012, 23:30 -0500 schrieb Ian Pilcher:
> Add EDID quirk flags to disable HDMI audio and HDMI InfoFrames.
> Add quirk for LG L246WP.

I know, in

        commit bc42aabc6a01b92b0f961d65671564e0e1cd7592
        Author: Adam Jackson <ajax@redhat.com>
        Date:   Wed May 23 16:26:54 2012 -0400

            drm/edid/quirks: ViewSonic VA2026w

it also was committed together, but I would prefer to first add the
quirks and then the actual monitor. In my opinion the advantage is that
then it is more visible because it is mentioned in the commit summary
and therefore can be found more easily.

So it would be great if you could split this patch up too although it is
quite small too.

Could you also mention in the commit message of the patch adding the LG
monitor on what system you tested this just for the record.

> Signed-off-by: Ian Pilcher <arequipeno@gmail.com>
> Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
> ---
>  drivers/gpu/drm/drm_edid.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)

[…]


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v4 0/3] Enhanced EDID quirk functionality
  2012-08-12  6:58                         ` [PATCH v3 0/2] Enhanced EDID quirk functionality Paul Menzel
@ 2012-08-12 20:07                           ` Ian Pilcher
  2012-08-12 20:07                             ` [PATCH v4 1/3] drm: Add user-defined EDID quirks capability Ian Pilcher
                                               ` (4 more replies)
  0 siblings, 5 replies; 32+ messages in thread
From: Ian Pilcher @ 2012-08-12 20:07 UTC (permalink / raw)
  To: dri-devel; +Cc: Ian Pilcher, paulepanter

Another rev.  I figured out how to use git reset --hard and --soft to
make regenerating the patch series a bit easier.  (It helped a ton
that all of the later changes are isolated in a single file.)  I still
feel like I'm probably missing an easier way to go back a fix a simple
type, however.

* Fix typo

* Split adding new quirk flags and adding new quirk into separate
  patches

After reveiwing stable_kernel_rules.txt, I don't think that these
patches are appropriate for -stable.  I don't consider it to be
"obviously correct," and it certainly isn't less than 100 lines.
Thus, I'm not going to submit this for inclusion in -stable.

Ian Pilcher (3):
  drm: Add user-defined EDID quirks capability
  drm: Add EDID quirks to disable HDMI audio and InfoFrames
  drm: Add EDID quirk for LG L246WP

 Documentation/EDID/edid_quirks.txt | 126 +++++++++
 drivers/gpu/drm/drm_drv.c          |   2 +
 drivers/gpu/drm/drm_edid.c         | 524 +++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/drm_stub.c         |   6 +
 drivers/gpu/drm/drm_sysfs.c        |  19 ++
 include/drm/drmP.h                 |  10 +
 include/drm/drm_edid.h             |  13 +-
 7 files changed, 639 insertions(+), 61 deletions(-)
 create mode 100644 Documentation/EDID/edid_quirks.txt

-- 
1.7.11.2

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH v4 1/3] drm: Add user-defined EDID quirks capability
  2012-08-12 20:07                           ` [PATCH v4 0/3] " Ian Pilcher
@ 2012-08-12 20:07                             ` Ian Pilcher
  2012-08-14 15:13                               ` Paul Menzel
  2012-08-12 20:07                             ` [PATCH v4 2/3] drm: Add EDID quirks to disable HDMI audio and InfoFrames Ian Pilcher
                                               ` (3 subsequent siblings)
  4 siblings, 1 reply; 32+ messages in thread
From: Ian Pilcher @ 2012-08-12 20:07 UTC (permalink / raw)
  To: dri-devel; +Cc: Ian Pilcher, paulepanter

Add the ability for users to define their own EDID quirks via a
module parameter or sysfs attribute.

Signed-off-by: Ian Pilcher <arequipeno@gmail.com>
Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
---
 Documentation/EDID/edid_quirks.txt | 126 ++++++++++
 drivers/gpu/drm/drm_drv.c          |   2 +
 drivers/gpu/drm/drm_edid.c         | 500 ++++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/drm_stub.c         |   6 +
 drivers/gpu/drm/drm_sysfs.c        |  19 ++
 include/drm/drmP.h                 |  10 +
 include/drm/drm_edid.h             |  13 +-
 7 files changed, 615 insertions(+), 61 deletions(-)
 create mode 100644 Documentation/EDID/edid_quirks.txt

diff --git a/Documentation/EDID/edid_quirks.txt b/Documentation/EDID/edid_quirks.txt
new file mode 100644
index 0000000..0c9c746
--- /dev/null
+++ b/Documentation/EDID/edid_quirks.txt
@@ -0,0 +1,126 @@
+                                  EDID Quirks
+                                 =============
+                       Ian Pilcher <arequipeno@gmail.com>
+                                August 11, 2012
+
+
+    "EDID blocks out in the wild have a variety of bugs"
+        -- from drivers/gpu/drm/drm_edid.c
+
+
+Overview
+========
+
+EDID quirks provide a mechanism for working around display hardware with buggy
+EDID data.
+
+An individual EDID quirk maps a display type (identified by its EDID
+manufacturer ID and product code[1]) to a set of "quirk flags."  The kernel
+includes a variety of built-in quirks.  (They are stored in the edid_quirk_list
+array in drivers/gpu/drm/drm_edid.c.)
+
+An example of a built-in EDID quirk is:
+
+    ACR:0xad46:0x00000001
+
+The first field is the manufacturer ID (Acer, Inc.), the second field is the
+manufacturer's product code, and the third field contains the quirk flags for
+that display type.
+
+The quirk flags are defined in drivers/gpu/drm/drm_edid.c.  Each flag has a
+symbolic name beginning with EDID_QUIRK_, along with a numerical value.  Each
+flag should also have an associated comment which provides an idea of its
+effect.  Note that the values in the source file are expressed as bit shifts[2]:
+
+    * 1 << 0: 0x0001
+    * 1 << 1: 0x0002
+    * 1 << 2: 0x0004
+    * etc.
+
+
+sysfs interface
+===============
+
+The current EDID quirk list can be read from /sys/class/drm/edid_quirks:
+
+    # cat /sys/class/drm/edid_quirks
+       ACR:0xad46:0x00000001
+       API:0x7602:0x00000001
+       ACR:0x0977:0x00000020
+    0x9e6a:0x077e:0x00000080
+    ...
+
+("Nonconformant" manufacturer IDs are displayed as hexadecimal values.)
+
+The number of total "slots" in the list can be read from
+/sys/class/drm/edid_quirks_size.  This total includes both occupied slots (i.e.
+the current list) and any slots available for additional quirks.  The number of
+available slots can be calculated by subtracting the number of quirks in the
+current list from the total number of slots.
+
+If a slot is available, an additional quirk can be added to the list by writing
+it to /sys/class/drm/edid_quirks:
+
+    # echo FOO:0xffff:0x100 > /sys/class/drm/edid_quirks
+
+Manufacturer IDs can also be specified numerically.  (This is the only way to
+specify a nonconformant ID.) This command is equivalent to the previous one:
+
+    # echo 0x19ef:0xffff:0x100 > /sys/class/drm/edid_quirks
+
+Numeric values can also be specified in decimal or octal formats; a number that
+begins with a 0 is assumed to be octal:
+
+    # echo FOO:65535:0400 > /sys/class/drm/edid_quirks
+
+An existing quirk can be replaced by writing a new set of flags:
+
+    # echo FOO:0xffff:0x200 > /sys/class/drm/edid_quirks
+
+A quirk can be deleted from the list by writing an empty flag set (0). This
+makes the slot occupied by that quirk available.
+
+    # echo FOO:0xffff:0 > /sys/class/drm/edid_quirks
+
+Writing an "at symbol" (@) clears the entire quirk list:
+
+    # echo @ > /sys/class/drm/edid_quirks
+
+Multiple changes to the list can be specified in a comma (or newline) separated
+list. For example, the following command clears all of the existing quirks in
+the list and adds 3 new quirks:
+
+    # echo @,FOO:0xffff:0x100,BAR:0x1111:0x001,BAZ:0x2222:0x002 > \
+            /sys/class/drm/edid_quirks
+
+Note however, that any error (an incorrectly formatted quirk or an attempt to
+add a quirk when no slot is available) will abort processing of any further
+changes, potentially making it difficult to determine exactly which change
+caused the error and what changes were made.  For this reason, making changes
+one at a time is recommended, particularly if the changes are being made by a
+script or program.
+
+
+Module parameter
+================
+
+The EDID quirk list can also be modified via the edid_quirks module parameter
+(drm.edid_quirks on the kernel command line).  The effect of setting this
+parameter is identical to the effect of writing its value to
+/sys/class/drm/edid_quirks, with one important difference.  When an error is
+encountered during module parameter parsing or processing, any remaining quirks
+in the parameter string will still be processed.  (It is hoped that this approach
+maximizes the probability of producing a working display.)
+
+
+Follow-up
+=========
+
+If you encounter a display that requires an additional EDID quirk in order to
+function properly, please report it to the direct rendering development mailing
+list <dri-devel@lists.freedesktop.org>.
+
+
+[1] See http://en.wikipedia.org/wiki/Extended_display_identification_data for a
+    description of the manufacturer ID and product code fields.
+[2] https://en.wikipedia.org/wiki/Bitwise_operation#Bit_shifts
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 9238de4..7fe39e0 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -276,6 +276,8 @@ static int __init drm_core_init(void)
 		goto err_p3;
 	}
 
+	drm_edid_quirks_param_process();
+
 	DRM_INFO("Initialized %s %d.%d.%d %s\n",
 		 CORE_NAME, CORE_MAJOR, CORE_MINOR, CORE_PATCHLEVEL, CORE_DATE);
 	return 0;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index a8743c3..ea535f6 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -31,6 +31,8 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
+#include <linux/ctype.h>
+
 #include "drmP.h"
 #include "drm_edid.h"
 #include "drm_edid_modes.h"
@@ -82,51 +84,457 @@ struct detailed_mode_closure {
 #define LEVEL_GTF2	2
 #define LEVEL_CVT	3
 
-static struct edid_quirk {
-	char vendor[4];
-	int product_id;
-	u32 quirks;
-} edid_quirk_list[] = {
+union edid_quirk {
+	struct {
+		union edid_display_id display_id;
+		u32 quirks;
+	} __attribute__((packed)) s;
+	u64 u;
+};
+
+#define EDID_MFG_ID(c1, c2, c3)		cpu_to_be16(			\
+						(c1 & 0x1f) << 10 |	\
+						(c2 & 0x1f) << 5 |	\
+						(c3 & 0x1f)		\
+					)
+
+#define EDID_QUIRK_LIST_SIZE	24
+
+union edid_quirk edid_quirk_list[EDID_QUIRK_LIST_SIZE] = {
+
 	/* Acer AL1706 */
-	{ "ACR", 44358, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('A', 'C', 'R'), cpu_to_le16(44358) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 	/* Acer F51 */
-	{ "API", 0x7602, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('A', 'P', 'I'), cpu_to_le16(0x7602) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 	/* Unknown Acer */
-	{ "ACR", 2423, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+	{ { { { EDID_MFG_ID('A', 'C', 'R'), cpu_to_le16(2423) } },
+		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
 
 	/* Belinea 10 15 55 */
-	{ "MAX", 1516, EDID_QUIRK_PREFER_LARGE_60 },
-	{ "MAX", 0x77e, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('M', 'A', 'X'), cpu_to_le16(1516) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
+	{ { { { EDID_MFG_ID('M', 'A', 'X'), cpu_to_le16(0x77e) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 
 	/* Envision Peripherals, Inc. EN-7100e */
-	{ "EPI", 59264, EDID_QUIRK_135_CLOCK_TOO_HIGH },
+	{ { { { EDID_MFG_ID('E', 'P', 'I'), cpu_to_le16(59264) } },
+		EDID_QUIRK_135_CLOCK_TOO_HIGH } },
 	/* Envision EN2028 */
-	{ "EPI", 8232, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('E', 'P', 'I'), cpu_to_le16(8232) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 
 	/* Funai Electronics PM36B */
-	{ "FCM", 13600, EDID_QUIRK_PREFER_LARGE_75 |
-	  EDID_QUIRK_DETAILED_IN_CM },
+	{ { { { EDID_MFG_ID('F', 'C', 'M'), cpu_to_le16(13600) } },
+		EDID_QUIRK_PREFER_LARGE_75 | EDID_QUIRK_DETAILED_IN_CM } },
 
 	/* LG Philips LCD LP154W01-A5 */
-	{ "LPL", 0, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE },
-	{ "LPL", 0x2a00, EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE },
+	{ { { { EDID_MFG_ID('L', 'P', 'L'), cpu_to_le16(0) } },
+		EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE } },
+	{ { { { EDID_MFG_ID('L', 'P', 'L'), cpu_to_le16(0x2a00) } },
+		EDID_QUIRK_DETAILED_USE_MAXIMUM_SIZE } },
 
 	/* Philips 107p5 CRT */
-	{ "PHL", 57364, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+	{ { { { EDID_MFG_ID('P', 'H', 'L'), cpu_to_le16(57364) } },
+		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
 
 	/* Proview AY765C */
-	{ "PTS", 765, EDID_QUIRK_FIRST_DETAILED_PREFERRED },
+	{ { { { EDID_MFG_ID('P', 'T', 'S'), cpu_to_le16(765) } },
+		EDID_QUIRK_FIRST_DETAILED_PREFERRED } },
 
 	/* Samsung SyncMaster 205BW.  Note: irony */
-	{ "SAM", 541, EDID_QUIRK_DETAILED_SYNC_PP },
+	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(541) } },
+		EDID_QUIRK_DETAILED_SYNC_PP } },
 	/* Samsung SyncMaster 22[5-6]BW */
-	{ "SAM", 596, EDID_QUIRK_PREFER_LARGE_60 },
-	{ "SAM", 638, EDID_QUIRK_PREFER_LARGE_60 },
+	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(596) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
+	{ { { { EDID_MFG_ID('S', 'A', 'M'), cpu_to_le16(638) } },
+		EDID_QUIRK_PREFER_LARGE_60 } },
 
 	/* ViewSonic VA2026w */
-	{ "VSC", 5020, EDID_QUIRK_FORCE_REDUCED_BLANKING },
+	{ { { { EDID_MFG_ID('V', 'S', 'C'), cpu_to_le16(5020) } },
+		EDID_QUIRK_FORCE_REDUCED_BLANKING } },
+
+	/*
+	 * When adding built-in quirks, please adjust EDID_QUIRK_LIST_SIZE to
+	 * provide some room for user-supplied quirks.
+	 */
 };
 
+DEFINE_MUTEX(edid_quirk_list_mutex);
+
+/**
+ * drm_edid_mfg_format - format an "encoded" EDID manufacturer ID for printing
+ * @mfg_id: the encoded manufacturer ID
+ * @buf: destination buffer for the formatted manufacturer ID (minimum 7 bytes)
+ * @strip: if non-zero, the returned pointer will skip any leading spaces
+ *
+ * An EDID manufacturer ID is supposed to consist of 3 capital letters (A-Z).
+ * Each letter is stored as a 5-bit value between 1 and 26, taking up 15 bits of
+ * the 16-bit ID. The remaining bit should always be 0. If display manufacturers
+ * always did things correctly, however, EDID quirks wouldn't be required in
+ * the first place. This function does the following:
+ *
+ * - Broken IDs are printed in hexadecimal (0xffff).
+ * - "Correct" IDs are formatted as a 3-letter ID string, preceded by 3 spaces;
+ *   the spaces ensure that both output formats are the same length.
+ *
+ * Thus, a formatted manufacturer ID is always 6 characters long (not including
+ * the terminating 0).
+ *
+ * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
+ * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
+ * been formatted as a 3-letter string, a pointer to the first non-space
+ * character (@buf + 3) is returned.
+ */
+static const char *drm_edid_mfg_format(__be16 mfg_id, char *buf, int strip)
+{
+	u16 id = be16_to_cpu(mfg_id);
+
+	if (id & 0x8000)
+		goto bad_id;
+
+	buf[3] = ((id & 0x7c00) >> 10) + '@';
+	if (!isupper(buf[3]))
+		goto bad_id;
+
+	buf[4] = ((id & 0x03e0) >> 5) + '@';
+	if (!isupper(buf[4]))
+		goto bad_id;
+
+	buf[5] = (id & 0x001f) + '@';
+	if (!isupper(buf[5]))
+		goto bad_id;
+
+	memset(buf, ' ', 3);
+	buf[6] = 0;
+
+	return strip ? (buf + 3) : buf;
+
+bad_id:
+	sprintf(buf, "0x%04hx", id);
+	return buf;
+}
+
+#define EDID_MFG_BUF_SIZE		7
+
+/**
+ * drm_edid_display_id_format - format an EDID "display ID" (manufacturer ID
+ * 				and product code) for printing
+ * @display_id: the display ID
+ * @buf: destination buffer for the formatted display ID (minimum 14 bytes)
+ * @strip: if non-zero, the returned pointer will skip any leading spaces
+ *
+ * A formatted display ID is always 13 characters long (not including the
+ * terminating 0).
+ *
+ * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
+ * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
+ * been formatted as a 3-letter string, a pointer to the first non-space
+ * character (@buf + 3) is returned.
+ */
+static const char *drm_edid_display_id_format(union edid_display_id display_id,
+					      char *buf, int strip)
+{
+	const char *s;
+
+	s = drm_edid_mfg_format(display_id.s.mfg_id, buf, strip);
+	sprintf(buf + EDID_MFG_BUF_SIZE - 1, ":0x%04hx",
+		le16_to_cpu(display_id.s.prod_code));
+
+	return s;
+}
+
+#define EDID_DISPLAY_ID_BUF_SIZE	(EDID_MFG_BUF_SIZE + 7)
+
+/**
+ * drm_edid_quirk_format - format an EDID quirk for printing
+ * @quirk: the quirk
+ * @buf: destination buffer for the formatted quirk (minimum 25 bytes)
+ * @strip: if non-zero, the returned pointer will skip any leading spaces
+ *
+ * A formatted EDID quirk is always 24 characters long (not including the
+ * terminating 0).
+ *
+ * If @strip is 0, or the manufacturer ID has been formatted as a hexadecimal
+ * number, @buf is returned.  If @strip is non-zero, and the manufacturer ID has
+ * been formatted as a 3-letter string, a pointer to the first non-space
+ * character (@buf + 3) is returned.
+ */
+static const char *drm_edid_quirk_format(const union edid_quirk *quirk,
+					 char *buf, int strip)
+{
+	const char *s;
+
+	s = drm_edid_display_id_format(quirk->s.display_id, buf, strip);
+	sprintf(buf + EDID_DISPLAY_ID_BUF_SIZE - 1, ":0x%08x", quirk->s.quirks);
+
+	return s;
+}
+
+#define EDID_QUIRK_BUF_SIZE		(EDID_DISPLAY_ID_BUF_SIZE + 11)
+
+/**
+ * drm_edid_quirk_parse - parse an EDID quirk
+ * @s: string containing the quirk to be parsed
+ * @quirk: destination for parsed quirk
+ *
+ * Returns 0 on success, < 0 (currently -EINVAL) on error.
+ */
+static int drm_edid_quirk_parse(const char *s, union edid_quirk *quirk)
+{
+	char buf[EDID_QUIRK_BUF_SIZE];
+	s32 mfg;
+	s32 product;
+	s64 quirks;
+	char *c;
+
+	if (sscanf(s, "%i:%i:%lli", &mfg, &product, &quirks) == 3) {
+		if (mfg < 0 || mfg > 0xffff)
+			goto error;
+		quirk->s.display_id.s.mfg_id = cpu_to_be16((u16)mfg);
+	} else {
+		if (sscanf(s, "%3s:%i:%lli", buf, &product, &quirks) != 3 ||
+				!isupper(buf[0]) ||
+				!isupper(buf[1]) ||
+				!isupper(buf[2]))
+			goto error;
+		quirk->s.display_id.s.mfg_id =
+				EDID_MFG_ID(buf[0], buf[1], buf[2]);
+	}
+
+	if (product < 0 || product > 0xffff ||
+			quirks < 0 || quirks > 0xffffffffLL)
+		goto error;
+
+	quirk->s.display_id.s.prod_code = cpu_to_le16((u16)product);
+	quirk->s.quirks = (u32)quirks;
+
+	DRM_DEBUG("Successfully parsed EDID quirk: %s\n",
+		  drm_edid_quirk_format(quirk, buf, 1));
+
+	return 0;
+
+error:
+	c = strpbrk(s, ",\n");
+	if (c == NULL) {
+		printk(KERN_WARNING "Invalid EDID quirk: '%s'\n", s);
+	} else {
+		printk(KERN_WARNING "Invalid EDID quirk: '%.*s'\n",
+		      (int)(c - s), s);
+	}
+
+	return -EINVAL;
+}
+
+/**
+ * drm_edid_quirk_find_by_id - find the EDID quirk matching a display ID
+ * @display_id: the display ID to match
+ *
+ * Caller MUST hold edid_quirk_list_mutex.
+ *
+ * Returns a pointer to the matching quirk list entry, NULL if no such entry
+ * exists.
+ */
+static union edid_quirk *drm_edid_quirk_find_by_id(union edid_display_id id)
+{
+	union edid_quirk *q = edid_quirk_list;
+
+	do {
+		if (q->s.display_id.u == id.u && q->s.quirks != 0)
+			return q;
+	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
+
+	return NULL;
+}
+
+/**
+ * drm_edid_quirk_find_slot - find an empty slot in the EDID quirk list
+ *
+ * Caller MUST hold edid_quirk_list_mutex.
+ *
+ * Returns a pointer to the first empty slot, NULL if no empty slots exist.
+ */
+static union edid_quirk *drm_edid_quirk_find_empty(void)
+{
+	union edid_quirk *q = edid_quirk_list;
+
+	do {
+		if (q->s.quirks == 0)
+			return q;
+	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
+
+	return NULL;
+}
+
+/**
+ * drm_edid_quirk_process - process a newly parsed EDID quirk
+ * @quirk: the quirk to be processed
+ *
+ * Depending on the newly parsed quirk and the contents of the quirks list, this
+ * function will add, remove, or replace a quirk.
+ *
+ * Returns 0 on success, < 0 on error (-ENOSPC if there is no free slot for a
+ * new quirk). Note that trying to remove a quirk that isn't present is not
+ * considered an error.
+ */
+static int drm_edid_quirk_process(const union edid_quirk *quirk)
+{
+	char buf[EDID_QUIRK_BUF_SIZE];
+	union edid_quirk *q;
+	int res = 0;
+
+	mutex_lock(&edid_quirk_list_mutex);
+
+	if (quirk->s.quirks == 0) {
+		DRM_INFO("Removing EDID quirk for display %s\n",
+			 drm_edid_display_id_format(quirk->s.display_id,
+						    buf, 1));
+		q = drm_edid_quirk_find_by_id(quirk->s.display_id);
+		if (q == NULL) {
+			printk(KERN_WARNING "No quirk found for display %s\n",
+			       drm_edid_display_id_format(quirk->s.display_id,
+							  buf, 1));
+		} else {
+			q->u = 0;
+		}
+	} else {
+		DRM_INFO("Adding EDID quirk: %s\n",
+			 drm_edid_quirk_format(quirk, buf, 1));
+		q = drm_edid_quirk_find_by_id(quirk->s.display_id);
+		if (q == NULL) {
+			q = drm_edid_quirk_find_empty();
+			if (q == NULL) {
+				printk(KERN_WARNING
+				       "No free slot in EDID quirk list\n");
+				res = -ENOSPC;
+			} else {
+				q->u = quirk->u;
+			}
+		} else {
+			DRM_INFO("Replacing existing quirk: %s\n",
+				 drm_edid_quirk_format(q, buf, 1));
+			q->s.quirks = quirk->s.quirks;
+		}
+	}
+
+	mutex_unlock(&edid_quirk_list_mutex);
+
+	return res;
+}
+
+/**
+ * drm_edid_quirks_process - parse and process a comma separated list of EDID
+ * 			     quirks
+ * @s: string containing the quirks to be processed
+ * @strict: if non-zero, any parsing or processing error aborts further
+ * 	    processing
+ *
+ * Returns 0 on success, < 0 if any error is encountered.  (If multiple errors
+ * occur when strict is set to 0, the last error encountered is returned.)
+ */
+static int drm_edid_quirks_process(const char *s, int strict)
+{
+	union edid_quirk quirk;
+	int res = 0;
+
+	do {
+
+		if (*s == '@') {
+			DRM_INFO("Clearing EDID quirk list\n");
+			mutex_lock(&edid_quirk_list_mutex);
+			memset(edid_quirk_list, 0, sizeof edid_quirk_list);
+			mutex_unlock(&edid_quirk_list_mutex);
+		} else {
+			res = drm_edid_quirk_parse(s, &quirk);
+			if (res != 0) {
+				if (strict)
+					goto error;
+				continue;
+			}
+
+			res = drm_edid_quirk_process(&quirk);
+			if (res != 0) {
+				if (strict)
+					goto error;
+			}
+		}
+
+		s = strpbrk(s, ",\n");
+
+	} while (s != NULL && *(++s) != 0);
+
+	return res;
+
+error:
+	printk(KERN_WARNING "Aborting EDID quirk parsing\n");
+	return res;
+}
+
+/**
+ * drm_edid_quirks_param_process - process the edid_quirks module parameter
+ */
+void drm_edid_quirks_param_process(void)
+{
+	if (drm_edid_quirks != NULL)
+		drm_edid_quirks_process(drm_edid_quirks, 0);
+}
+
+/**
+ * drm_edid_quirks_size_show - show the size of the EDID quirk list in sysfs
+ * @buf: destination buffer (PAGE_SIZE bytes)
+ */
+ssize_t drm_edid_quirks_size_show(struct class *class,
+				  struct class_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%zu\n", ARRAY_SIZE(edid_quirk_list));
+}
+
+/**
+ * drm_edid_quirks_show - show the contents of the EDID quirk list in sysfs
+ * @buf: destination buffer (PAGE_SIZE bytes)
+ */
+ssize_t drm_edid_quirks_show(struct class *class, struct class_attribute *attr,
+			     char *buf)
+{
+	const union edid_quirk *q = edid_quirk_list;
+	ssize_t count = 0;
+
+	BUILD_BUG_ON(ARRAY_SIZE(edid_quirk_list) >
+				PAGE_SIZE / EDID_QUIRK_BUF_SIZE);
+
+	mutex_lock(&edid_quirk_list_mutex);
+
+	do {
+		if (q->s.quirks != 0) {
+			drm_edid_quirk_format(q, buf + count, 0);
+			(buf + count)[EDID_QUIRK_BUF_SIZE - 1] = '\n';
+			count += EDID_QUIRK_BUF_SIZE;
+		}
+	} while (++q < edid_quirk_list + ARRAY_SIZE(edid_quirk_list));
+
+	mutex_unlock(&edid_quirk_list_mutex);
+
+	return count;
+}
+
+/**
+ * drm_edid_quirks_store - parse and process EDID qurik list changes written
+ *			   to sysfs attribute
+ */
+ssize_t drm_edid_quirks_store(struct class *class, struct class_attribute *attr,
+			      const char *buf, size_t count)
+{
+	int res;
+
+	res = drm_edid_quirks_process(buf, 1);
+	if (res != 0)
+		return res;
+
+	return count;
+}
+
 /*** DDC fetch and block validation ***/
 
 static const u8 edid_header[] = {
@@ -409,25 +817,6 @@ EXPORT_SYMBOL(drm_get_edid);
 /*** EDID parsing ***/
 
 /**
- * edid_vendor - match a string against EDID's obfuscated vendor field
- * @edid: EDID to match
- * @vendor: vendor string
- *
- * Returns true if @vendor is in @edid, false otherwise
- */
-static bool edid_vendor(struct edid *edid, char *vendor)
-{
-	char edid_vendor[3];
-
-	edid_vendor[0] = ((edid->mfg_id[0] & 0x7c) >> 2) + '@';
-	edid_vendor[1] = (((edid->mfg_id[0] & 0x3) << 3) |
-			  ((edid->mfg_id[1] & 0xe0) >> 5)) + '@';
-	edid_vendor[2] = (edid->mfg_id[1] & 0x1f) + '@';
-
-	return !strncmp(edid_vendor, vendor, 3);
-}
-
-/**
  * edid_get_quirks - return quirk flags for a given EDID
  * @edid: EDID to process
  *
@@ -435,18 +824,18 @@ static bool edid_vendor(struct edid *edid, char *vendor)
  */
 static u32 edid_get_quirks(struct edid *edid)
 {
-	struct edid_quirk *quirk;
-	int i;
+	union edid_quirk *q;
+	u32 quirks = 0;
 
-	for (i = 0; i < ARRAY_SIZE(edid_quirk_list); i++) {
-		quirk = &edid_quirk_list[i];
+	mutex_lock(&edid_quirk_list_mutex);
 
-		if (edid_vendor(edid, quirk->vendor) &&
-		    (EDID_PRODUCT_ID(edid) == quirk->product_id))
-			return quirk->quirks;
-	}
+	q = drm_edid_quirk_find_by_id(edid->display_id);
+	if (q != NULL)
+		quirks = q->s.quirks;
 
-	return 0;
+	mutex_unlock(&edid_quirk_list_mutex);
+
+	return quirks;
 }
 
 #define MODE_SIZE(m) ((m)->hdisplay * (m)->vdisplay)
@@ -1162,7 +1551,7 @@ do_inferred_modes(struct detailed_timing *timing, void *c)
 	closure->modes += drm_dmt_modes_for_range(closure->connector,
 						  closure->edid,
 						  timing);
-	
+
 	if (!version_greater(closure->edid, 1, 1))
 		return; /* GTF not defined yet */
 
@@ -1399,7 +1788,7 @@ do_cvt_mode(struct detailed_timing *timing, void *c)
 
 static int
 add_cvt_modes(struct drm_connector *connector, struct edid *edid)
-{	
+{
 	struct detailed_mode_closure closure = {
 		connector, edid, 0, 0, 0
 	};
@@ -1615,15 +2004,12 @@ void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
 
 	eld[0] = 2 << 3;		/* ELD version: 2 */
 
-	eld[16] = edid->mfg_id[0];
-	eld[17] = edid->mfg_id[1];
-	eld[18] = edid->prod_code[0];
-	eld[19] = edid->prod_code[1];
+	*(u32 *)(&eld[16]) = edid->display_id.u;
 
 	if (cea[1] >= 3)
 		for (db = cea + 4; db < cea + cea[2]; db += dbl + 1) {
 			dbl = db[0] & 0x1f;
-			
+
 			switch ((db[0] & 0xe0) >> 5) {
 			case AUDIO_BLOCK:
 				/* Audio Data Block, contains SADs */
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 21bcd4a..b939d51 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -46,16 +46,22 @@ EXPORT_SYMBOL(drm_vblank_offdelay);
 unsigned int drm_timestamp_precision = 20;  /* Default to 20 usecs. */
 EXPORT_SYMBOL(drm_timestamp_precision);
 
+char *drm_edid_quirks = NULL;
+EXPORT_SYMBOL(drm_edid_quirks);
+
 MODULE_AUTHOR(CORE_AUTHOR);
 MODULE_DESCRIPTION(CORE_DESC);
 MODULE_LICENSE("GPL and additional rights");
 MODULE_PARM_DESC(debug, "Enable debug output");
 MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable [msecs]");
 MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps [usecs]");
+MODULE_PARM_DESC(edid_quirks, "MFG:prod:flags[,MFG:prod:flags[...]]\n"
+			      "(See Documentation/EDID/edid_quirks.txt)");
 
 module_param_named(debug, drm_debug, int, 0600);
 module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600);
 module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600);
+module_param_named(edid_quirks, drm_edid_quirks, charp, 0400);
 
 struct idr drm_minors_idr;
 
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 45ac8d6..84dc365 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -84,6 +84,11 @@ static CLASS_ATTR_STRING(version, S_IRUGO,
 		__stringify(CORE_PATCHLEVEL) " "
 		CORE_DATE);
 
+static CLASS_ATTR(edid_quirks_size, 0400, drm_edid_quirks_size_show, 0);
+
+static CLASS_ATTR(edid_quirks, 0600, drm_edid_quirks_show,
+		  drm_edid_quirks_store);
+
 /**
  * drm_sysfs_create - create a struct drm_sysfs_class structure
  * @owner: pointer to the module that is to "own" this struct drm_sysfs_class
@@ -113,10 +118,22 @@ struct class *drm_sysfs_create(struct module *owner, char *name)
 	if (err)
 		goto err_out_class;
 
+	err = class_create_file(class, &class_attr_edid_quirks_size);
+	if (err)
+		goto err_out_version;
+
+	err = class_create_file(class, &class_attr_edid_quirks);
+	if (err)
+		goto err_out_quirks_size;
+
 	class->devnode = drm_devnode;
 
 	return class;
 
+err_out_quirks_size:
+	class_remove_file(class, &class_attr_edid_quirks_size);
+err_out_version:
+	class_remove_file(class, &class_attr_version.attr);
 err_out_class:
 	class_destroy(class);
 err_out:
@@ -132,6 +149,8 @@ void drm_sysfs_destroy(void)
 {
 	if ((drm_class == NULL) || (IS_ERR(drm_class)))
 		return;
+	class_remove_file(drm_class, &class_attr_edid_quirks);
+	class_remove_file(drm_class, &class_attr_edid_quirks_size);
 	class_remove_file(drm_class, &class_attr_version.attr);
 	class_destroy(drm_class);
 	drm_class = NULL;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index d6b67bb..c947f3e 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1501,6 +1501,7 @@ extern unsigned int drm_debug;
 
 extern unsigned int drm_vblank_offdelay;
 extern unsigned int drm_timestamp_precision;
+extern char *drm_edid_quirks;
 
 extern struct class *drm_class;
 extern struct proc_dir_entry *drm_proc_root;
@@ -1612,6 +1613,15 @@ void drm_gem_vm_open(struct vm_area_struct *vma);
 void drm_gem_vm_close(struct vm_area_struct *vma);
 int drm_gem_mmap(struct file *filp, struct vm_area_struct *vma);
 
+					/* EDID support (drm_edid.c) */
+void drm_edid_quirks_param_process(void);
+ssize_t drm_edid_quirks_size_show(struct class *class,
+				  struct class_attribute *attr, char *buf);
+ssize_t drm_edid_quirks_show(struct class *class, struct class_attribute *attr,
+			     char *buf);
+ssize_t drm_edid_quirks_store(struct class *class, struct class_attribute *attr,
+			      const char *buf, size_t count);
+
 #include "drm_global.h"
 
 static inline void
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 0cac551..713229b 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -202,11 +202,18 @@ struct detailed_timing {
 #define DRM_EDID_FEATURE_PM_SUSPEND       (1 << 6)
 #define DRM_EDID_FEATURE_PM_STANDBY       (1 << 7)
 
+union edid_display_id {
+	struct {
+		__be16 mfg_id;
+		__le16 prod_code;
+	} __attribute__((packed)) s;
+	u32 u;
+};
+
 struct edid {
 	u8 header[8];
 	/* Vendor & product info */
-	u8 mfg_id[2];
-	u8 prod_code[2];
+	union edid_display_id display_id;
 	u32 serial; /* FIXME: byte order */
 	u8 mfg_week;
 	u8 mfg_year;
@@ -242,8 +249,6 @@ struct edid {
 	u8 checksum;
 } __attribute__((packed));
 
-#define EDID_PRODUCT_ID(e) ((e)->prod_code[0] | ((e)->prod_code[1] << 8))
-
 struct drm_encoder;
 struct drm_connector;
 struct drm_display_mode;
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v4 2/3] drm: Add EDID quirks to disable HDMI audio and InfoFrames
  2012-08-12 20:07                           ` [PATCH v4 0/3] " Ian Pilcher
  2012-08-12 20:07                             ` [PATCH v4 1/3] drm: Add user-defined EDID quirks capability Ian Pilcher
@ 2012-08-12 20:07                             ` Ian Pilcher
  2012-08-12 20:08                             ` [PATCH v4 3/3] drm: Add EDID quirk for LG L246WP Ian Pilcher
                                               ` (2 subsequent siblings)
  4 siblings, 0 replies; 32+ messages in thread
From: Ian Pilcher @ 2012-08-12 20:07 UTC (permalink / raw)
  To: dri-devel; +Cc: Ian Pilcher, paulepanter

Add 2 new EDID quirk flags:

- EDID_QUIRK_DISABLE_INFOFRAMES turns off all HDMI-specific
  functionality (audio, HDCP, etc.).  Intended for displays that
  are confused by *any* InfoFrames.

- EDID_QUIRK_NO_AUDIO disables HDMI audio.  Intended for displays
  that incorrectely report audio capabilities in their EDID data.

Signed-off-by: Ian Pilcher <arequipeno@gmail.com>
Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
---
 drivers/gpu/drm/drm_edid.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index ea535f6..61586b4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -70,6 +70,10 @@
 #define EDID_QUIRK_DETAILED_SYNC_PP		(1 << 6)
 /* Force reduced-blanking timings for detailed modes */
 #define EDID_QUIRK_FORCE_REDUCED_BLANKING	(1 << 7)
+/* Display is confused by InfoFrames; don't sent any */
+#define EDID_QUIRK_DISABLE_INFOFRAMES		(1 << 8)
+/* Display doesn't have any audio output */
+#define EDID_QUIRK_NO_AUDIO			(1 << 9)
 
 struct detailed_mode_closure {
 	struct drm_connector *connector;
@@ -2109,6 +2113,14 @@ bool drm_detect_hdmi_monitor(struct edid *edid)
 	int i, hdmi_id;
 	int start_offset, end_offset;
 	bool is_hdmi = false;
+	char buf[EDID_DISPLAY_ID_BUF_SIZE];
+
+	if (edid_get_quirks(edid) & EDID_QUIRK_DISABLE_INFOFRAMES) {
+		DRM_INFO("Disabling HDMI InfoFrames on display %s "
+			 "due to EDID quirk\n",
+			 drm_edid_display_id_format(edid->display_id, buf, 1));
+		goto end;
+	}
 
 	edid_ext = drm_find_cea_extension(edid);
 	if (!edid_ext)
@@ -2157,6 +2169,14 @@ bool drm_detect_monitor_audio(struct edid *edid)
 	int i, j;
 	bool has_audio = false;
 	int start_offset, end_offset;
+	char buf[EDID_DISPLAY_ID_BUF_SIZE];
+
+	if (edid_get_quirks(edid) & EDID_QUIRK_NO_AUDIO) {
+		DRM_INFO("Disabling HDMI audio on display %s "
+			 "due to EDID quirk\n",
+			 drm_edid_display_id_format(edid->display_id, buf, 1));
+		goto end;
+	}
 
 	edid_ext = drm_find_cea_extension(edid);
 	if (!edid_ext)
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH v4 3/3] drm: Add EDID quirk for LG L246WP
  2012-08-12 20:07                           ` [PATCH v4 0/3] " Ian Pilcher
  2012-08-12 20:07                             ` [PATCH v4 1/3] drm: Add user-defined EDID quirks capability Ian Pilcher
  2012-08-12 20:07                             ` [PATCH v4 2/3] drm: Add EDID quirks to disable HDMI audio and InfoFrames Ian Pilcher
@ 2012-08-12 20:08                             ` Ian Pilcher
  2012-09-04 14:52                               ` Paul Menzel
  2012-08-12 21:29                             ` [PATCH v4 0/3] Enhanced EDID quirk functionality Christian König
  2012-08-12 21:31                             ` Paul Menzel
  4 siblings, 1 reply; 32+ messages in thread
From: Ian Pilcher @ 2012-08-12 20:08 UTC (permalink / raw)
  To: dri-devel; +Cc: Ian Pilcher, paulepanter

This display is apparently confused by any InfoFrames (see
https://bugzilla.redhat.com/show_bug.cgi?id=806091).

Tested on a ThinkPad T510 (nVidia GT218 [NVS 3100M]) and a co-
workers ThinkPad X220 with Intel graphics.  EDID_QUIRK_NO_AUDIO
makes this display work with the Intel driver; nouveau requires
EDID_QUIRK_DISABLE_INFOFRAMES.

Signed-off-by: Ian Pilcher <arequipeno@gmail.com>
Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
---
 drivers/gpu/drm/drm_edid.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 61586b4..1f50e09 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -160,6 +160,10 @@ union edid_quirk edid_quirk_list[EDID_QUIRK_LIST_SIZE] = {
 	{ { { { EDID_MFG_ID('V', 'S', 'C'), cpu_to_le16(5020) } },
 		EDID_QUIRK_FORCE_REDUCED_BLANKING } },
 
+	/* LG L246WP */
+	{ { { { EDID_MFG_ID('G', 'S', 'M'), cpu_to_le16(0x563f) } },
+		EDID_QUIRK_DISABLE_INFOFRAMES | EDID_QUIRK_NO_AUDIO } },
+
 	/*
 	 * When adding built-in quirks, please adjust EDID_QUIRK_LIST_SIZE to
 	 * provide some room for user-supplied quirks.
-- 
1.7.11.2

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH v4 0/3] Enhanced EDID quirk functionality
  2012-08-12 20:07                           ` [PATCH v4 0/3] " Ian Pilcher
                                               ` (2 preceding siblings ...)
  2012-08-12 20:08                             ` [PATCH v4 3/3] drm: Add EDID quirk for LG L246WP Ian Pilcher
@ 2012-08-12 21:29                             ` Christian König
  2012-08-12 21:31                             ` Paul Menzel
  4 siblings, 0 replies; 32+ messages in thread
From: Christian König @ 2012-08-12 21:29 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: paulepanter, dri-devel

On 12.08.2012 22:07, Ian Pilcher wrote:
> Another rev.  I figured out how to use git reset --hard and --soft to
> make regenerating the patch series a bit easier.  (It helped a ton
> that all of the later changes are isolated in a single file.)  I still
> feel like I'm probably missing an easier way to go back a fix a simple
> type, however.
Sounds like you should give interactive rebasing a try:

git rebase -i <whatever your patches are based uppon>

It fires up an editor allowing you to make changes to you individual 
patches.

I hope that helps,
Christian.

>
> * Fix typo
>
> * Split adding new quirk flags and adding new quirk into separate
>    patches
>
> After reveiwing stable_kernel_rules.txt, I don't think that these
> patches are appropriate for -stable.  I don't consider it to be
> "obviously correct," and it certainly isn't less than 100 lines.
> Thus, I'm not going to submit this for inclusion in -stable.
>
> Ian Pilcher (3):
>    drm: Add user-defined EDID quirks capability
>    drm: Add EDID quirks to disable HDMI audio and InfoFrames
>    drm: Add EDID quirk for LG L246WP
>
>   Documentation/EDID/edid_quirks.txt | 126 +++++++++
>   drivers/gpu/drm/drm_drv.c          |   2 +
>   drivers/gpu/drm/drm_edid.c         | 524 +++++++++++++++++++++++++++++++++----
>   drivers/gpu/drm/drm_stub.c         |   6 +
>   drivers/gpu/drm/drm_sysfs.c        |  19 ++
>   include/drm/drmP.h                 |  10 +
>   include/drm/drm_edid.h             |  13 +-
>   7 files changed, 639 insertions(+), 61 deletions(-)
>   create mode 100644 Documentation/EDID/edid_quirks.txt
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v4 0/3] Enhanced EDID quirk functionality
  2012-08-12 20:07                           ` [PATCH v4 0/3] " Ian Pilcher
                                               ` (3 preceding siblings ...)
  2012-08-12 21:29                             ` [PATCH v4 0/3] Enhanced EDID quirk functionality Christian König
@ 2012-08-12 21:31                             ` Paul Menzel
  2012-08-13 14:39                               ` Ian Pilcher
  4 siblings, 1 reply; 32+ messages in thread
From: Paul Menzel @ 2012-08-12 21:31 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2094 bytes --]

Dear Ian,


thank you very much for your quick iterations. Hopefully the developers
will review this on Monday.

Am Sonntag, den 12.08.2012, 15:07 -0500 schrieb Ian Pilcher:
> Another rev.  I figured out how to use git reset --hard and --soft to
> make regenerating the patch series a bit easier.  (It helped a ton
> that all of the later changes are isolated in a single file.)

Definitely. Wasn’t `git add -p` helpful?

> I still feel like I'm probably missing an easier way to go back a fix
> a simple type, however.

Have you heard of `git rebase` already. That is the more complex version
of `git commit --amend`.

If the typo is in the last commit just do make the change and do `git
commit --amend -a`.

If you want to change some earlier commit (like 2 ahead), either do the
fix, commit it and do `git rebase -i HEAD~3` and reorder the commits to
put the fix at the correct place and change `pick` to `f` for fix up.

Or do `git rebase -i HEAD~3` and choose the commits you need to edit and
then commit these changes with `git commit -a --amend`.

> * Fix typo
> 
> * Split adding new quirk flags and adding new quirk into separate
>   patches
> 
> After reveiwing stable_kernel_rules.txt, I don't think that these
> patches are appropriate for -stable.  I don't consider it to be
> "obviously correct," and it certainly isn't less than 100 lines.
> Thus, I'm not going to submit this for inclusion in -stable.

Hmm, three things.

1. I would try anyway. The stable maintainer will reject it if they
think it is not appropriate.

2. Your last quirk should be back ported. One solution would be, you
first submit patches to add the quirks and the monitor and then add the
functionality.

3. I also need to add two monitors to the quirk list which need to be
backported. If I try to get them in before yours, you would have to
rebase your patches again.

Also likely future quirks would need to be backported in a complicated
way.

So I vote for getting this into stable and hopefully the maintainers
agree.


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v4 0/3] Enhanced EDID quirk functionality
  2012-08-12 21:31                             ` Paul Menzel
@ 2012-08-13 14:39                               ` Ian Pilcher
  0 siblings, 0 replies; 32+ messages in thread
From: Ian Pilcher @ 2012-08-13 14:39 UTC (permalink / raw)
  To: Paul Menzel; +Cc: dri-devel

On 08/12/2012 04:31 PM, Paul Menzel wrote:
> So I vote for getting this into stable and hopefully the maintainers
> agree.

Paul -

We obviously disagree on whether this makes sense for -stable.  If you
feel strongly that it should go in, I suggest that you go ahead and
submit it for inclusion.

Thanks for all your help!

-- 
========================================================================
Ian Pilcher                                         arequipeno@gmail.com
"If you're going to shift my paradigm ... at least buy me dinner first."
========================================================================

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v4 1/3] drm: Add user-defined EDID quirks capability
  2012-08-12 20:07                             ` [PATCH v4 1/3] drm: Add user-defined EDID quirks capability Ian Pilcher
@ 2012-08-14 15:13                               ` Paul Menzel
  2012-08-14 15:45                                 ` Ian Pilcher
  0 siblings, 1 reply; 32+ messages in thread
From: Paul Menzel @ 2012-08-14 15:13 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1401 bytes --]

Am Sonntag, den 12.08.2012, 15:07 -0500 schrieb Ian Pilcher:
> Add the ability for users to define their own EDID quirks via a
> module parameter or sysfs attribute.
> 
> Signed-off-by: Ian Pilcher <arequipeno@gmail.com>
> Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
> ---
>  Documentation/EDID/edid_quirks.txt | 126 ++++++++++
>  drivers/gpu/drm/drm_drv.c          |   2 +
>  drivers/gpu/drm/drm_edid.c         | 500 ++++++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/drm_stub.c         |   6 +
>  drivers/gpu/drm/drm_sysfs.c        |  19 ++
>  include/drm/drmP.h                 |  10 +
>  include/drm/drm_edid.h             |  13 +-
>  7 files changed, 615 insertions(+), 61 deletions(-)
>  create mode 100644 Documentation/EDID/edid_quirks.txt

[…]

> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 45ac8d6..84dc365 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -84,6 +84,11 @@ static CLASS_ATTR_STRING(version, S_IRUGO,
>  		__stringify(CORE_PATCHLEVEL) " "
>  		CORE_DATE);
>  
> +static CLASS_ATTR(edid_quirks_size, 0400, drm_edid_quirks_size_show, 0);
> +
> +static CLASS_ATTR(edid_quirks, 0600, drm_edid_quirks_show,
> +		  drm_edid_quirks_store);

Testing your patch, I would vote that a normal user is allowed to read
the quirk list.

[…]


Thanks,

Paul

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v4 1/3] drm: Add user-defined EDID quirks capability
  2012-08-14 15:13                               ` Paul Menzel
@ 2012-08-14 15:45                                 ` Ian Pilcher
  2012-08-15  6:41                                   ` Paul Menzel
  0 siblings, 1 reply; 32+ messages in thread
From: Ian Pilcher @ 2012-08-14 15:45 UTC (permalink / raw)
  To: Paul Menzel; +Cc: dri-devel

On 08/14/2012 10:13 AM, Paul Menzel wrote:
> Testing your patch, I would vote that a normal user is allowed to read
> the quirk list.

I thought about that, but I decided to leave it at 0600, because I can't
see how the information would be of any use without the ability to
change the list.  I there a use case that I didn't think of?

I also figure that it's much easier to start out with more restrictive
permissions and open up later if the need arises.

-- 
========================================================================
Ian Pilcher                                         arequipeno@gmail.com
"If you're going to shift my paradigm ... at least buy me dinner first."
========================================================================

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v4 1/3] drm: Add user-defined EDID quirks capability
  2012-08-14 15:45                                 ` Ian Pilcher
@ 2012-08-15  6:41                                   ` Paul Menzel
  0 siblings, 0 replies; 32+ messages in thread
From: Paul Menzel @ 2012-08-15  6:41 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1326 bytes --]

Am Dienstag, den 14.08.2012, 10:45 -0500 schrieb Ian Pilcher:
> On 08/14/2012 10:13 AM, Paul Menzel wrote:
> > Testing your patch, I would vote that a normal user is allowed to read
> > the quirk list.
> 
> I thought about that, but I decided to leave it at 0600, because I can't
> see how the information would be of any use without the ability to
> change the list.  I there a use case that I didn't think of?

True. I could just make up a use case, that I set up GNU/Linux for
someone without giving them the root password. Then some external
monitor/TV does not work and to debug the problem I want to know what
quirks are present in the running Linux version.

Also possible user space programs (for GNOME, KDE, …) should be able to
query such information without being root. Maybe even `xrandr` is
suitable to display such kind of information.

We should also keep in mind that an user can change resolutions with
`xrandr` already today.

> I also figure that it's much easier to start out with more restrictive
> permissions and open up later if the need arises.

True. And I cannot think of a reason how knowing these information
should hurt. But it reminds me of Linus’ security rant [1].


Thanks,

Paul


[1] https://plus.google.com/u/0/102150693225130002912/posts/1vyfmNCYpi5

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH v4 3/3] drm: Add EDID quirk for LG L246WP
  2012-08-12 20:08                             ` [PATCH v4 3/3] drm: Add EDID quirk for LG L246WP Ian Pilcher
@ 2012-09-04 14:52                               ` Paul Menzel
  0 siblings, 0 replies; 32+ messages in thread
From: Paul Menzel @ 2012-09-04 14:52 UTC (permalink / raw)
  To: Ian Pilcher; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1139 bytes --]

Am Sonntag, den 12.08.2012, 15:08 -0500 schrieb Ian Pilcher:
> This display is apparently confused by any InfoFrames (see
> https://bugzilla.redhat.com/show_bug.cgi?id=806091).
> 
> Tested on a ThinkPad T510 (nVidia GT218 [NVS 3100M]) and a co-
> workers ThinkPad X220 with Intel graphics.  EDID_QUIRK_NO_AUDIO
> makes this display work with the Intel driver; nouveau requires
> EDID_QUIRK_DISABLE_INFOFRAMES.

Judging from the Newegg.com description [1] it has a VGA (D-Sub) and
HDMI connector. (Downloading the manual from [2] fails for me with
Midori.)

Ian, could you please test if the monitor is sending the same EDID
information over VGA and HDMI? Unfortunately I do not have the means
(for HDMI) for the TVs here, so it would be great, if you could test
that.

> Signed-off-by: Ian Pilcher <arequipeno@gmail.com>
> Acked-by: Paul Menzel <paulepanter@users.sourceforge.net>
> ---
>  drivers/gpu/drm/drm_edid.c | 4 ++++
>  1 file changed, 4 insertions(+)

[…]


Thanks,

Paul


[1] http://www.newegg.com/Product/Product.aspx?Item=N82E16824005099
[2] http://www.lg.com/us/support-product/lg-L246WP

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

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

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2012-09-04 14:52 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-19 19:16 Enhancing EDID quirk functionality Ian Pilcher
2012-04-24  9:07 ` Lars-Peter Clausen
2012-04-24 19:00   ` Ian Pilcher
2012-05-03 18:01     ` Ian Pilcher
2012-05-03 19:42       ` Adam Jackson
2012-05-07 19:50         ` Ian Pilcher
2012-05-07 21:38           ` Adam Jackson
2012-08-10  4:23             ` Ian Pilcher
2012-08-10  4:23               ` [PATCH] drm: EDID quirk improvements Ian Pilcher
2012-08-10 18:44                 ` Ian Pilcher
2012-08-10 18:44                   ` Ian Pilcher
2012-08-11  8:31                     ` Paul Menzel
2012-08-11 15:38                       ` Ian Pilcher
2012-08-11 19:52                         ` Paul Menzel
2012-08-11 19:37                     ` Paul Menzel
2012-08-12  4:30                       ` [PATCH v3 0/2] Enhanced EDID quirk functionality Ian Pilcher
2012-08-12  4:30                         ` [PATCH v3 1/2] drm: Add user-defined EDID quirks capability Ian Pilcher
2012-08-12 15:39                           ` Paul Menzel
2012-08-12  4:30                         ` [PATCH v3 2/2] drm: Add EDID quirks to disable HDMI audio and InfoFrames Ian Pilcher
2012-08-12 15:45                           ` Paul Menzel
2012-08-12  6:58                         ` [PATCH v3 0/2] Enhanced EDID quirk functionality Paul Menzel
2012-08-12 20:07                           ` [PATCH v4 0/3] " Ian Pilcher
2012-08-12 20:07                             ` [PATCH v4 1/3] drm: Add user-defined EDID quirks capability Ian Pilcher
2012-08-14 15:13                               ` Paul Menzel
2012-08-14 15:45                                 ` Ian Pilcher
2012-08-15  6:41                                   ` Paul Menzel
2012-08-12 20:07                             ` [PATCH v4 2/3] drm: Add EDID quirks to disable HDMI audio and InfoFrames Ian Pilcher
2012-08-12 20:08                             ` [PATCH v4 3/3] drm: Add EDID quirk for LG L246WP Ian Pilcher
2012-09-04 14:52                               ` Paul Menzel
2012-08-12 21:29                             ` [PATCH v4 0/3] Enhanced EDID quirk functionality Christian König
2012-08-12 21:31                             ` Paul Menzel
2012-08-13 14:39                               ` Ian Pilcher

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.