[PATCH] drm: EDID quirk improvements
Paul Menzel
paulepanter at users.sourceforge.net
Sat Aug 11 01:31:39 PDT 2012
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 at 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 at 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 at 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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120811/f5056002/attachment-0001.pgp>
More information about the dri-devel
mailing list