It appears that Dell 5K monitors hide some mode timings in the displayid block, this set of patches fixes up a few bits of displayid support, then add support for extracting the type 1 detailed timings that the Dell monitors use.
The first two patches are missing signoff but I've asked for Tomas to provide them.
This at least gets the modelines into the modelist.
Dave.
From: Tomas Bzatek tomas@bzatek.net
Cosmetic change, let's report more precise revisions and IDs.
https://bugs.freedesktop.org/show_bug.cgi?id=95207
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/drm_edid.c | 6 +++--- include/drm/drm_displayid.h | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 9a9be9a..c8a3a55 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4168,8 +4168,8 @@ static int drm_parse_display_id(struct drm_connector *connector,
base = (struct displayid_hdr *)&displayid[idx];
- DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n", - base->rev, base->bytes, base->prod_id, base->ext_count); + DRM_DEBUG_KMS("base revision v%d.%d, edid length %d, bytes %d, prod_id %d ext_count %d\n", + base->ver, base->rev, length, base->bytes, base->prod_id, base->ext_count);
if (base->bytes + 5 > length - idx) return -EINVAL; @@ -4183,7 +4183,7 @@ static int drm_parse_display_id(struct drm_connector *connector, }
block = (struct displayid_block *)&displayid[idx + 4]; - DRM_DEBUG_KMS("block id %d, rev %d, len %d\n", + DRM_DEBUG_KMS("block id 0x%x, rev %d, len %d\n", block->tag, block->rev, block->num_bytes);
switch (block->tag) { diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index 623b4e9..042f9fc 100644 --- a/include/drm/drm_displayid.h +++ b/include/drm/drm_displayid.h @@ -52,7 +52,8 @@ #define PRODUCT_TYPE_DIRECT_DRIVE 6
struct displayid_hdr { - u8 rev; + u8 rev:4; + u8 ver:4; u8 bytes; u8 prod_id; u8 ext_count; @@ -60,7 +61,8 @@ struct displayid_hdr {
struct displayid_block { u8 tag; - u8 rev; + u8 rev:3; + u8 reserved:5; u8 num_bytes; } __packed;
On Wed, May 04, 2016 at 06:36:48AM +1000, Dave Airlie wrote:
From: Tomas Bzatek tomas@bzatek.net
Cosmetic change, let's report more precise revisions and IDs.
https://bugs.freedesktop.org/show_bug.cgi?id=95207
Signed-off-by: Dave Airlie airlied@redhat.com
drivers/gpu/drm/drm_edid.c | 6 +++--- include/drm/drm_displayid.h | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 9a9be9a..c8a3a55 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4168,8 +4168,8 @@ static int drm_parse_display_id(struct drm_connector *connector,
base = (struct displayid_hdr *)&displayid[idx];
- DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n",
base->rev, base->bytes, base->prod_id, base->ext_count);
DRM_DEBUG_KMS("base revision v%d.%d, edid length %d, bytes %d, prod_id %d ext_count %d\n",
base->ver, base->rev, length, base->bytes, base->prod_id, base->ext_count);if (base->bytes + 5 > length - idx) return -EINVAL;
@@ -4183,7 +4183,7 @@ static int drm_parse_display_id(struct drm_connector *connector, }
block = (struct displayid_block *)&displayid[idx + 4];
- DRM_DEBUG_KMS("block id %d, rev %d, len %d\n",
DRM_DEBUG_KMS("block id 0x%x, rev %d, len %d\n", block->tag, block->rev, block->num_bytes);
switch (block->tag) {
diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index 623b4e9..042f9fc 100644 --- a/include/drm/drm_displayid.h +++ b/include/drm/drm_displayid.h @@ -52,7 +52,8 @@ #define PRODUCT_TYPE_DIRECT_DRIVE 6
struct displayid_hdr {
- u8 rev;
- u8 rev:4;
- u8 ver:4; u8 bytes; u8 prod_id; u8 ext_count;
@@ -60,7 +61,8 @@ struct displayid_hdr {
struct displayid_block { u8 tag;
- u8 rev;
- u8 rev:3;
- u8 reserved:5; u8 num_bytes;
} __packed;
Using bitfields in an architecture independent structure doesn't feel like an entirely good idea to me.
On 04.05.2016 18:10, Ville Syrjälä wrote:
On Wed, May 04, 2016 at 06:36:48AM +1000, Dave Airlie wrote:
From: Tomas Bzatek tomas@bzatek.net
Cosmetic change, let's report more precise revisions and IDs.
https://bugs.freedesktop.org/show_bug.cgi?id=95207
Signed-off-by: Dave Airlie airlied@redhat.com
drivers/gpu/drm/drm_edid.c | 6 +++--- include/drm/drm_displayid.h | 6 ++++-- 2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 9a9be9a..c8a3a55 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4168,8 +4168,8 @@ static int drm_parse_display_id(struct drm_connector *connector,
base = (struct displayid_hdr *)&displayid[idx];
- DRM_DEBUG_KMS("base revision 0x%x, length %d, %d %d\n",
base->rev, base->bytes, base->prod_id, base->ext_count);
DRM_DEBUG_KMS("base revision v%d.%d, edid length %d, bytes %d, prod_id %d ext_count %d\n",
base->ver, base->rev, length, base->bytes, base->prod_id, base->ext_count);if (base->bytes + 5 > length - idx) return -EINVAL;
@@ -4183,7 +4183,7 @@ static int drm_parse_display_id(struct drm_connector *connector, }
block = (struct displayid_block *)&displayid[idx + 4];
- DRM_DEBUG_KMS("block id %d, rev %d, len %d\n",
DRM_DEBUG_KMS("block id 0x%x, rev %d, len %d\n", block->tag, block->rev, block->num_bytes);
switch (block->tag) {
diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index 623b4e9..042f9fc 100644 --- a/include/drm/drm_displayid.h +++ b/include/drm/drm_displayid.h @@ -52,7 +52,8 @@ #define PRODUCT_TYPE_DIRECT_DRIVE 6
struct displayid_hdr {
- u8 rev;
- u8 rev:4;
- u8 ver:4; u8 bytes; u8 prod_id; u8 ext_count;
@@ -60,7 +61,8 @@ struct displayid_hdr {
struct displayid_block { u8 tag;
- u8 rev;
- u8 rev:3;
- u8 reserved:5; u8 num_bytes;
} __packed;
Using bitfields in an architecture independent structure doesn't feel like an entirely good idea to me.
Yeah, this won't work as expected on some architectures.
index 623b4e9..042f9fc 100644 --- a/include/drm/drm_displayid.h +++ b/include/drm/drm_displayid.h @@ -52,7 +52,8 @@ #define PRODUCT_TYPE_DIRECT_DRIVE 6
struct displayid_hdr {
- u8 rev;
- u8 rev:4;
- u8 ver:4; u8 bytes; u8 prod_id; u8 ext_count;
@@ -60,7 +61,8 @@ struct displayid_hdr {
struct displayid_block { u8 tag;
- u8 rev;
- u8 rev:3;
- u8 reserved:5; u8 num_bytes;
} __packed;
Using bitfields in an architecture independent structure doesn't feel like an entirely good idea to me.
Yeah, this won't work as expected on some architectures.
Indeed, I'll drop this for now.
Dave.
From: Tomas Bzatek tomas@bzatek.net
This will iterate over all DisplayID blocks found in the buffer. Previously only the first block was parsed.
https://bugs.freedesktop.org/show_bug.cgi?id=95207
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/drm_edid.c | 124 ++++++++++++++++++++++++--------------------- 1 file changed, 65 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c8a3a55..1a364b3 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4182,66 +4182,72 @@ static int drm_parse_display_id(struct drm_connector *connector, return -EINVAL; }
- block = (struct displayid_block *)&displayid[idx + 4]; - DRM_DEBUG_KMS("block id 0x%x, rev %d, len %d\n", - block->tag, block->rev, block->num_bytes); - - switch (block->tag) { - case DATA_BLOCK_TILED_DISPLAY: { - struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block; - - u16 w, h; - u8 tile_v_loc, tile_h_loc; - u8 num_v_tile, num_h_tile; - struct drm_tile_group *tg; - - w = tile->tile_size[0] | tile->tile_size[1] << 8; - h = tile->tile_size[2] | tile->tile_size[3] << 8; - - num_v_tile = (tile->topo[0] & 0xf) | (tile->topo[2] & 0x30); - num_h_tile = (tile->topo[0] >> 4) | ((tile->topo[2] >> 2) & 0x30); - tile_v_loc = (tile->topo[1] & 0xf) | ((tile->topo[2] & 0x3) << 4); - tile_h_loc = (tile->topo[1] >> 4) | (((tile->topo[2] >> 2) & 0x3) << 4); - - connector->has_tile = true; - if (tile->tile_cap & 0x80) - connector->tile_is_single_monitor = true; - - connector->num_h_tile = num_h_tile + 1; - connector->num_v_tile = num_v_tile + 1; - connector->tile_h_loc = tile_h_loc; - connector->tile_v_loc = tile_v_loc; - connector->tile_h_size = w + 1; - connector->tile_v_size = h + 1; - - DRM_DEBUG_KMS("tile cap 0x%x\n", tile->tile_cap); - DRM_DEBUG_KMS("tile_size %d x %d\n", w + 1, h + 1); - DRM_DEBUG_KMS("topo num tiles %dx%d, location %dx%d\n", - num_h_tile + 1, num_v_tile + 1, tile_h_loc, tile_v_loc); - DRM_DEBUG_KMS("vend %c%c%c\n", tile->topology_id[0], tile->topology_id[1], tile->topology_id[2]); - - tg = drm_mode_get_tile_group(connector->dev, tile->topology_id); - if (!tg) { - tg = drm_mode_create_tile_group(connector->dev, tile->topology_id); - } - if (!tg) - return -ENOMEM; - - if (connector->tile_group != tg) { - /* if we haven't got a pointer, - take the reference, drop ref to old tile group */ - if (connector->tile_group) { - drm_mode_put_tile_group(connector->dev, connector->tile_group); + idx += sizeof(struct displayid_hdr); + while (block = (struct displayid_block *)&displayid[idx], + idx + sizeof(struct displayid_block) <= length && + idx + sizeof(struct displayid_block) + block->num_bytes <= length && + block->num_bytes > 0) { + idx += block->num_bytes + sizeof(struct displayid_block); + DRM_DEBUG_KMS("block id 0x%x, rev %d, len %d\n", + block->tag, block->rev, block->num_bytes); + + switch (block->tag) { + case DATA_BLOCK_TILED_DISPLAY: { + struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block; + + u16 w, h; + u8 tile_v_loc, tile_h_loc; + u8 num_v_tile, num_h_tile; + struct drm_tile_group *tg; + + w = tile->tile_size[0] | tile->tile_size[1] << 8; + h = tile->tile_size[2] | tile->tile_size[3] << 8; + + num_v_tile = (tile->topo[0] & 0xf) | (tile->topo[2] & 0x30); + num_h_tile = (tile->topo[0] >> 4) | ((tile->topo[2] >> 2) & 0x30); + tile_v_loc = (tile->topo[1] & 0xf) | ((tile->topo[2] & 0x3) << 4); + tile_h_loc = (tile->topo[1] >> 4) | (((tile->topo[2] >> 2) & 0x3) << 4); + + connector->has_tile = true; + if (tile->tile_cap & 0x80) + connector->tile_is_single_monitor = true; + + connector->num_h_tile = num_h_tile + 1; + connector->num_v_tile = num_v_tile + 1; + connector->tile_h_loc = tile_h_loc; + connector->tile_v_loc = tile_v_loc; + connector->tile_h_size = w + 1; + connector->tile_v_size = h + 1; + + DRM_DEBUG_KMS("tile cap 0x%x\n", tile->tile_cap); + DRM_DEBUG_KMS("tile_size %d x %d\n", w + 1, h + 1); + DRM_DEBUG_KMS("topo num tiles %dx%d, location %dx%d\n", + num_h_tile + 1, num_v_tile + 1, tile_h_loc, tile_v_loc); + DRM_DEBUG_KMS("vend %c%c%c\n", tile->topology_id[0], tile->topology_id[1], tile->topology_id[2]); + + tg = drm_mode_get_tile_group(connector->dev, tile->topology_id); + if (!tg) { + tg = drm_mode_create_tile_group(connector->dev, tile->topology_id); } - connector->tile_group = tg; - } else - /* if same tile group, then release the ref we just took. */ - drm_mode_put_tile_group(connector->dev, tg); - } - break; - default: - printk("unknown displayid tag %d\n", block->tag); - break; + if (!tg) + return -ENOMEM; + + if (connector->tile_group != tg) { + /* if we haven't got a pointer, + take the reference, drop ref to old tile group */ + if (connector->tile_group) { + drm_mode_put_tile_group(connector->dev, connector->tile_group); + } + connector->tile_group = tg; + } else + /* if same tile group, then release the ref we just took. */ + drm_mode_put_tile_group(connector->dev, tg); + } + break; + default: + DRM_DEBUG_KMS("found DisplayID tag 0x%x, unhandled\n", block->tag); + break; + } } return 0; }
From: Dave Airlie airlied@redhat.com
This just makes the code easier to follow.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/drm_edid.c | 110 ++++++++++++++++++++++++--------------------- 1 file changed, 59 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1a364b3..e4c681f 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4152,6 +4152,60 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, } EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
+static int drm_parse_tiled_block(struct drm_connector *connector, + struct displayid_block *block) +{ + struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block; + u16 w, h; + u8 tile_v_loc, tile_h_loc; + u8 num_v_tile, num_h_tile; + struct drm_tile_group *tg; + + w = tile->tile_size[0] | tile->tile_size[1] << 8; + h = tile->tile_size[2] | tile->tile_size[3] << 8; + + num_v_tile = (tile->topo[0] & 0xf) | (tile->topo[2] & 0x30); + num_h_tile = (tile->topo[0] >> 4) | ((tile->topo[2] >> 2) & 0x30); + tile_v_loc = (tile->topo[1] & 0xf) | ((tile->topo[2] & 0x3) << 4); + tile_h_loc = (tile->topo[1] >> 4) | (((tile->topo[2] >> 2) & 0x3) << 4); + + connector->has_tile = true; + if (tile->tile_cap & 0x80) + connector->tile_is_single_monitor = true; + + connector->num_h_tile = num_h_tile + 1; + connector->num_v_tile = num_v_tile + 1; + connector->tile_h_loc = tile_h_loc; + connector->tile_v_loc = tile_v_loc; + connector->tile_h_size = w + 1; + connector->tile_v_size = h + 1; + + DRM_DEBUG_KMS("tile cap 0x%x\n", tile->tile_cap); + DRM_DEBUG_KMS("tile_size %d x %d\n", w + 1, h + 1); + DRM_DEBUG_KMS("topo num tiles %dx%d, location %dx%d\n", + num_h_tile + 1, num_v_tile + 1, tile_h_loc, tile_v_loc); + DRM_DEBUG_KMS("vend %c%c%c\n", tile->topology_id[0], tile->topology_id[1], tile->topology_id[2]); + + tg = drm_mode_get_tile_group(connector->dev, tile->topology_id); + if (!tg) { + tg = drm_mode_create_tile_group(connector->dev, tile->topology_id); + } + if (!tg) + return -ENOMEM; + + if (connector->tile_group != tg) { + /* if we haven't got a pointer, + take the reference, drop ref to old tile group */ + if (connector->tile_group) { + drm_mode_put_tile_group(connector->dev, connector->tile_group); + } + connector->tile_group = tg; + } else + /* if same tile group, then release the ref we just took. */ + drm_mode_put_tile_group(connector->dev, tg); + return 0; +} + static int drm_parse_display_id(struct drm_connector *connector, u8 *displayid, int length, bool is_edid_extension) @@ -4162,6 +4216,7 @@ static int drm_parse_display_id(struct drm_connector *connector, struct displayid_block *block; u8 csum = 0; int i; + int ret;
if (is_edid_extension) idx = 1; @@ -4192,57 +4247,10 @@ static int drm_parse_display_id(struct drm_connector *connector, block->tag, block->rev, block->num_bytes);
switch (block->tag) { - case DATA_BLOCK_TILED_DISPLAY: { - struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block; - - u16 w, h; - u8 tile_v_loc, tile_h_loc; - u8 num_v_tile, num_h_tile; - struct drm_tile_group *tg; - - w = tile->tile_size[0] | tile->tile_size[1] << 8; - h = tile->tile_size[2] | tile->tile_size[3] << 8; - - num_v_tile = (tile->topo[0] & 0xf) | (tile->topo[2] & 0x30); - num_h_tile = (tile->topo[0] >> 4) | ((tile->topo[2] >> 2) & 0x30); - tile_v_loc = (tile->topo[1] & 0xf) | ((tile->topo[2] & 0x3) << 4); - tile_h_loc = (tile->topo[1] >> 4) | (((tile->topo[2] >> 2) & 0x3) << 4); - - connector->has_tile = true; - if (tile->tile_cap & 0x80) - connector->tile_is_single_monitor = true; - - connector->num_h_tile = num_h_tile + 1; - connector->num_v_tile = num_v_tile + 1; - connector->tile_h_loc = tile_h_loc; - connector->tile_v_loc = tile_v_loc; - connector->tile_h_size = w + 1; - connector->tile_v_size = h + 1; - - DRM_DEBUG_KMS("tile cap 0x%x\n", tile->tile_cap); - DRM_DEBUG_KMS("tile_size %d x %d\n", w + 1, h + 1); - DRM_DEBUG_KMS("topo num tiles %dx%d, location %dx%d\n", - num_h_tile + 1, num_v_tile + 1, tile_h_loc, tile_v_loc); - DRM_DEBUG_KMS("vend %c%c%c\n", tile->topology_id[0], tile->topology_id[1], tile->topology_id[2]); - - tg = drm_mode_get_tile_group(connector->dev, tile->topology_id); - if (!tg) { - tg = drm_mode_create_tile_group(connector->dev, tile->topology_id); - } - if (!tg) - return -ENOMEM; - - if (connector->tile_group != tg) { - /* if we haven't got a pointer, - take the reference, drop ref to old tile group */ - if (connector->tile_group) { - drm_mode_put_tile_group(connector->dev, connector->tile_group); - } - connector->tile_group = tg; - } else - /* if same tile group, then release the ref we just took. */ - drm_mode_put_tile_group(connector->dev, tg); - } + case DATA_BLOCK_TILED_DISPLAY: + ret = drm_parse_tiled_block(connector, block); + if (ret) + return ret; break; default: DRM_DEBUG_KMS("found DisplayID tag 0x%x, unhandled\n", block->tag);
On Tue, 03 May 2016, Dave Airlie airlied@gmail.com wrote:
From: Dave Airlie airlied@redhat.com
This just makes the code easier to follow.
Swapping the order of patches 2 and 3 would make the patch series easier to follow. ;)
BR, Jani.
Signed-off-by: Dave Airlie airlied@redhat.com
drivers/gpu/drm/drm_edid.c | 110 ++++++++++++++++++++++++--------------------- 1 file changed, 59 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1a364b3..e4c681f 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4152,6 +4152,60 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame, } EXPORT_SYMBOL(drm_hdmi_vendor_infoframe_from_display_mode);
+static int drm_parse_tiled_block(struct drm_connector *connector,
struct displayid_block *block)+{
- struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block;
- u16 w, h;
- u8 tile_v_loc, tile_h_loc;
- u8 num_v_tile, num_h_tile;
- struct drm_tile_group *tg;
- w = tile->tile_size[0] | tile->tile_size[1] << 8;
- h = tile->tile_size[2] | tile->tile_size[3] << 8;
- num_v_tile = (tile->topo[0] & 0xf) | (tile->topo[2] & 0x30);
- num_h_tile = (tile->topo[0] >> 4) | ((tile->topo[2] >> 2) & 0x30);
- tile_v_loc = (tile->topo[1] & 0xf) | ((tile->topo[2] & 0x3) << 4);
- tile_h_loc = (tile->topo[1] >> 4) | (((tile->topo[2] >> 2) & 0x3) << 4);
- connector->has_tile = true;
- if (tile->tile_cap & 0x80)
connector->tile_is_single_monitor = true;- connector->num_h_tile = num_h_tile + 1;
- connector->num_v_tile = num_v_tile + 1;
- connector->tile_h_loc = tile_h_loc;
- connector->tile_v_loc = tile_v_loc;
- connector->tile_h_size = w + 1;
- connector->tile_v_size = h + 1;
- DRM_DEBUG_KMS("tile cap 0x%x\n", tile->tile_cap);
- DRM_DEBUG_KMS("tile_size %d x %d\n", w + 1, h + 1);
- DRM_DEBUG_KMS("topo num tiles %dx%d, location %dx%d\n",
num_h_tile + 1, num_v_tile + 1, tile_h_loc, tile_v_loc);- DRM_DEBUG_KMS("vend %c%c%c\n", tile->topology_id[0], tile->topology_id[1], tile->topology_id[2]);
- tg = drm_mode_get_tile_group(connector->dev, tile->topology_id);
- if (!tg) {
tg = drm_mode_create_tile_group(connector->dev, tile->topology_id);- }
- if (!tg)
return -ENOMEM;- if (connector->tile_group != tg) {
/* if we haven't got a pointer,take the reference, drop ref to old tile group */if (connector->tile_group) {drm_mode_put_tile_group(connector->dev, connector->tile_group);}connector->tile_group = tg;- } else
/* if same tile group, then release the ref we just took. */drm_mode_put_tile_group(connector->dev, tg);- return 0;
+}
static int drm_parse_display_id(struct drm_connector *connector, u8 *displayid, int length, bool is_edid_extension) @@ -4162,6 +4216,7 @@ static int drm_parse_display_id(struct drm_connector *connector, struct displayid_block *block; u8 csum = 0; int i;
int ret;
if (is_edid_extension) idx = 1;
@@ -4192,57 +4247,10 @@ static int drm_parse_display_id(struct drm_connector *connector, block->tag, block->rev, block->num_bytes);
switch (block->tag) {
case DATA_BLOCK_TILED_DISPLAY: {struct displayid_tiled_block *tile = (struct displayid_tiled_block *)block;u16 w, h;u8 tile_v_loc, tile_h_loc;u8 num_v_tile, num_h_tile;struct drm_tile_group *tg;w = tile->tile_size[0] | tile->tile_size[1] << 8;h = tile->tile_size[2] | tile->tile_size[3] << 8;num_v_tile = (tile->topo[0] & 0xf) | (tile->topo[2] & 0x30);num_h_tile = (tile->topo[0] >> 4) | ((tile->topo[2] >> 2) & 0x30);tile_v_loc = (tile->topo[1] & 0xf) | ((tile->topo[2] & 0x3) << 4);tile_h_loc = (tile->topo[1] >> 4) | (((tile->topo[2] >> 2) & 0x3) << 4);connector->has_tile = true;if (tile->tile_cap & 0x80)connector->tile_is_single_monitor = true;connector->num_h_tile = num_h_tile + 1;connector->num_v_tile = num_v_tile + 1;connector->tile_h_loc = tile_h_loc;connector->tile_v_loc = tile_v_loc;connector->tile_h_size = w + 1;connector->tile_v_size = h + 1;DRM_DEBUG_KMS("tile cap 0x%x\n", tile->tile_cap);DRM_DEBUG_KMS("tile_size %d x %d\n", w + 1, h + 1);DRM_DEBUG_KMS("topo num tiles %dx%d, location %dx%d\n",num_h_tile + 1, num_v_tile + 1, tile_h_loc, tile_v_loc);DRM_DEBUG_KMS("vend %c%c%c\n", tile->topology_id[0], tile->topology_id[1], tile->topology_id[2]);tg = drm_mode_get_tile_group(connector->dev, tile->topology_id);if (!tg) {tg = drm_mode_create_tile_group(connector->dev, tile->topology_id);}if (!tg)return -ENOMEM;if (connector->tile_group != tg) {/* if we haven't got a pointer,take the reference, drop ref to old tile group */if (connector->tile_group) {drm_mode_put_tile_group(connector->dev, connector->tile_group);}connector->tile_group = tg;} else/* if same tile group, then release the ref we just took. */drm_mode_put_tile_group(connector->dev, tg);}
case DATA_BLOCK_TILED_DISPLAY:ret = drm_parse_tiled_block(connector, block);if (ret)return ret; break;
From: Dave Airlie airlied@redhat.com
We need to use this for validating modeline additions.
Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/drm_edid.c | 45 +++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index e4c681f..e85d828 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3901,6 +3901,30 @@ static void drm_add_display_info(struct edid *edid, info->color_formats |= DRM_COLOR_FORMAT_YCRCB422; }
+static int validate_displayid(u8 *displayid, int length, int idx) +{ + int i; + u8 csum = 0; + struct displayid_hdr *base; + + base = (struct displayid_hdr *)&displayid[idx]; + + DRM_DEBUG_KMS("base revision v%d.%d, edid length %d, bytes %d, prod_id %d ext_count %d\n", + base->ver, base->rev, length, base->bytes, base->prod_id, + base->ext_count); + + if (base->bytes + 5 > length - idx) + return -EINVAL; + for (i = idx; i <= base->bytes + 5; i++) { + csum += displayid[i]; + } + if (csum) { + DRM_ERROR("DisplayID checksum invalid, remainder is %d\n", csum); + return -EINVAL; + } + return 0; +} + /** * drm_add_edid_modes - add modes from EDID data, if available * @connector: connector we're probing @@ -4212,30 +4236,15 @@ static int drm_parse_display_id(struct drm_connector *connector, { /* if this is an EDID extension the first byte will be 0x70 */ int idx = 0; - struct displayid_hdr *base; struct displayid_block *block; - u8 csum = 0; - int i; int ret;
if (is_edid_extension) idx = 1;
- base = (struct displayid_hdr *)&displayid[idx]; - - DRM_DEBUG_KMS("base revision v%d.%d, edid length %d, bytes %d, prod_id %d ext_count %d\n", - base->ver, base->rev, length, base->bytes, base->prod_id, base->ext_count); - - if (base->bytes + 5 > length - idx) - return -EINVAL; - - for (i = idx; i <= base->bytes + 5; i++) { - csum += displayid[i]; - } - if (csum) { - DRM_ERROR("DisplayID checksum invalid, remainder is %d\n", csum); - return -EINVAL; - } + ret = validate_displayid(displayid, length, idx); + if (ret) + return ret;
idx += sizeof(struct displayid_hdr); while (block = (struct displayid_block *)&displayid[idx],
From: Dave Airlie airlied@redhat.com
The tiled 5K Dell monitor appears to be hiding it's tiled mode inside the displayid timings block, this patch parses this blocks and adds the modes to the modelist.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95207 Signed-off-by: Dave Airlie airlied@redhat.com --- drivers/gpu/drm/drm_edid.c | 105 ++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_displayid.h | 17 +++++++ 2 files changed, 122 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index e85d828..aca9e25 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3925,6 +3925,110 @@ static int validate_displayid(u8 *displayid, int length, int idx) return 0; }
+static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *dev, + struct displayid_detailed_timings_1 *timings) +{ + struct drm_display_mode *mode; + unsigned pixel_clock = (timings->pixel_clock[0] | + (timings->pixel_clock[1] << 8) | + (timings->pixel_clock[2] << 16)); + unsigned hactive = (timings->hactive[0] | timings->hactive[1] << 8) + 1; + unsigned hblank = (timings->hblank[0] | timings->hblank[1] << 8) + 1; + unsigned hsync = (timings->hsync[0] | (timings->hsync[1] & 0x7f) << 8) + 1; + unsigned hsync_width = (timings->hsw[0] | timings->hsw[1] << 8) + 1; + unsigned vactive = (timings->vactive[0] | timings->vactive[1] << 8) + 1; + unsigned vblank = (timings->vblank[0] | timings->vblank[1] << 8) + 1; + unsigned vsync = (timings->vsync[0] | (timings->vsync[1] & 0x7f) << 8) + 1; + unsigned vsync_width = (timings->vsw[0] | timings->vsw[1] << 8) + 1; + bool hsync_positive = (timings->hsync[1] >> 7) & 0x1; + bool vsync_positive = (timings->vsync[1] >> 7) & 0x1; + mode = drm_mode_create(dev); + if (!mode) + return NULL; + + mode->clock = pixel_clock * 10; + mode->hdisplay = hactive; + mode->hsync_start = mode->hdisplay + hsync; + mode->hsync_end = mode->hsync_start + hsync_width; + mode->htotal = mode->hdisplay + hblank; + + mode->vdisplay = vactive; + mode->vsync_start = mode->vdisplay + vsync; + mode->vsync_end = mode->vsync_start + vsync_width; + mode->vtotal = mode->vdisplay + vblank; + + mode->flags = 0; + mode->flags |= hsync_positive ? DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC; + mode->flags |= vsync_positive ? DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC; + mode->type = DRM_MODE_TYPE_DRIVER; + + if (timings->flags & 0x80) + mode->type |= DRM_MODE_TYPE_PREFERRED; + mode->vrefresh = drm_mode_vrefresh(mode); + drm_mode_set_name(mode); + + return mode; +} + +static int add_displayid_detailed_1_modes(struct drm_connector *connector, + struct displayid_block *block) +{ + struct displayid_detailed_timing_block *det = (struct displayid_detailed_timing_block *)block; + int i; + int num_timings; + struct drm_display_mode *newmode; + int num_modes = 0; + /* blocks must be multiple of 20 bytes length */ + if (block->num_bytes % 20) + return 0; + + num_timings = block->num_bytes / 20; + for (i = 0; i < num_timings; i++) { + struct displayid_detailed_timings_1 *timings = &det->timings[i]; + + newmode = drm_mode_displayid_detailed(connector->dev, timings); + if (!newmode) + continue; + + drm_mode_probed_add(connector, newmode); + num_modes++; + } + return num_modes; +} + +static int add_displayid_detailed_modes(struct drm_connector *connector, + struct edid *edid) +{ + u8 *displayid; + int ret; + int idx = 1; + int length = EDID_LENGTH; + struct displayid_block *block; + int num_modes = 0; + + displayid = drm_find_displayid_extension(edid); + if (!displayid) + return 0; + + ret = validate_displayid(displayid, length, idx); + if (ret) + return 0; + + idx += sizeof(struct displayid_hdr); + while (block = (struct displayid_block *)&displayid[idx], + idx + sizeof(struct displayid_block) <= length && + idx + sizeof(struct displayid_block) + block->num_bytes <= length && + block->num_bytes > 0) { + idx += block->num_bytes + sizeof(struct displayid_block); + switch (block->tag) { + case DATA_BLOCK_TYPE_1_DETAILED_TIMING: + num_modes += add_displayid_detailed_1_modes(connector, block); + break; + } + } + return num_modes; +} + /** * drm_add_edid_modes - add modes from EDID data, if available * @connector: connector we're probing @@ -3970,6 +4074,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) num_modes += add_established_modes(connector, edid); num_modes += add_cea_modes(connector, edid); num_modes += add_alternate_cea_modes(connector, edid); + num_modes += add_displayid_detailed_modes(connector, edid); if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF) num_modes += add_inferred_modes(connector, edid);
diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index 042f9fc..edef51d 100644 --- a/include/drm/drm_displayid.h +++ b/include/drm/drm_displayid.h @@ -75,4 +75,21 @@ struct displayid_tiled_block { u8 topology_id[8]; } __packed;
+struct displayid_detailed_timings_1 { + u8 pixel_clock[3]; + u8 flags; + u8 hactive[2]; + u8 hblank[2]; + u8 hsync[2]; + u8 hsw[2]; + u8 vactive[2]; + u8 vblank[2]; + u8 vsync[2]; + u8 vsw[2]; +}; + +struct displayid_detailed_timing_block { + struct displayid_block base; + struct displayid_detailed_timings_1 timings[0]; +}; #endif
On Tue, 03 May 2016, Dave Airlie airlied@gmail.com wrote:
From: Dave Airlie airlied@redhat.com
The tiled 5K Dell monitor appears to be hiding it's tiled mode inside the displayid timings block, this patch parses this blocks and adds the modes to the modelist.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95207 Signed-off-by: Dave Airlie airlied@redhat.com
drivers/gpu/drm/drm_edid.c | 105 ++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_displayid.h | 17 +++++++ 2 files changed, 122 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index e85d828..aca9e25 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3925,6 +3925,110 @@ static int validate_displayid(u8 *displayid, int length, int idx) return 0; }
+static struct drm_display_mode *drm_mode_displayid_detailed(struct drm_device *dev,
struct displayid_detailed_timings_1 *timings)+{
- struct drm_display_mode *mode;
- unsigned pixel_clock = (timings->pixel_clock[0] |
(timings->pixel_clock[1] << 8) |(timings->pixel_clock[2] << 16));- unsigned hactive = (timings->hactive[0] | timings->hactive[1] << 8) + 1;
- unsigned hblank = (timings->hblank[0] | timings->hblank[1] << 8) + 1;
- unsigned hsync = (timings->hsync[0] | (timings->hsync[1] & 0x7f) << 8) + 1;
- unsigned hsync_width = (timings->hsw[0] | timings->hsw[1] << 8) + 1;
- unsigned vactive = (timings->vactive[0] | timings->vactive[1] << 8) + 1;
- unsigned vblank = (timings->vblank[0] | timings->vblank[1] << 8) + 1;
- unsigned vsync = (timings->vsync[0] | (timings->vsync[1] & 0x7f) << 8) + 1;
- unsigned vsync_width = (timings->vsw[0] | timings->vsw[1] << 8) + 1;
- bool hsync_positive = (timings->hsync[1] >> 7) & 0x1;
- bool vsync_positive = (timings->vsync[1] >> 7) & 0x1;
- mode = drm_mode_create(dev);
- if (!mode)
return NULL;- mode->clock = pixel_clock * 10;
- mode->hdisplay = hactive;
- mode->hsync_start = mode->hdisplay + hsync;
- mode->hsync_end = mode->hsync_start + hsync_width;
- mode->htotal = mode->hdisplay + hblank;
- mode->vdisplay = vactive;
- mode->vsync_start = mode->vdisplay + vsync;
- mode->vsync_end = mode->vsync_start + vsync_width;
- mode->vtotal = mode->vdisplay + vblank;
- mode->flags = 0;
- mode->flags |= hsync_positive ? DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;
- mode->flags |= vsync_positive ? DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;
- mode->type = DRM_MODE_TYPE_DRIVER;
- if (timings->flags & 0x80)
mode->type |= DRM_MODE_TYPE_PREFERRED;- mode->vrefresh = drm_mode_vrefresh(mode);
- drm_mode_set_name(mode);
- return mode;
+}
+static int add_displayid_detailed_1_modes(struct drm_connector *connector,
struct displayid_block *block)+{
- struct displayid_detailed_timing_block *det = (struct displayid_detailed_timing_block *)block;
- int i;
- int num_timings;
- struct drm_display_mode *newmode;
- int num_modes = 0;
- /* blocks must be multiple of 20 bytes length */
- if (block->num_bytes % 20)
return 0;- num_timings = block->num_bytes / 20;
- for (i = 0; i < num_timings; i++) {
struct displayid_detailed_timings_1 *timings = &det->timings[i];newmode = drm_mode_displayid_detailed(connector->dev, timings);if (!newmode)continue;drm_mode_probed_add(connector, newmode);num_modes++;- }
- return num_modes;
+}
+static int add_displayid_detailed_modes(struct drm_connector *connector,
struct edid *edid)+{
- u8 *displayid;
- int ret;
- int idx = 1;
- int length = EDID_LENGTH;
- struct displayid_block *block;
- int num_modes = 0;
- displayid = drm_find_displayid_extension(edid);
- if (!displayid)
return 0;- ret = validate_displayid(displayid, length, idx);
- if (ret)
return 0;- idx += sizeof(struct displayid_hdr);
- while (block = (struct displayid_block *)&displayid[idx],
idx + sizeof(struct displayid_block) <= length &&idx + sizeof(struct displayid_block) + block->num_bytes <= length &&block->num_bytes > 0) {idx += block->num_bytes + sizeof(struct displayid_block);switch (block->tag) {case DATA_BLOCK_TYPE_1_DETAILED_TIMING:num_modes += add_displayid_detailed_1_modes(connector, block);break;}- }
- return num_modes;
+}
/**
- drm_add_edid_modes - add modes from EDID data, if available
- @connector: connector we're probing
@@ -3970,6 +4074,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid) num_modes += add_established_modes(connector, edid); num_modes += add_cea_modes(connector, edid); num_modes += add_alternate_cea_modes(connector, edid);
- num_modes += add_displayid_detailed_modes(connector, edid); if (edid->features & DRM_EDID_FEATURE_DEFAULT_GTF) num_modes += add_inferred_modes(connector, edid);
diff --git a/include/drm/drm_displayid.h b/include/drm/drm_displayid.h index 042f9fc..edef51d 100644 --- a/include/drm/drm_displayid.h +++ b/include/drm/drm_displayid.h @@ -75,4 +75,21 @@ struct displayid_tiled_block { u8 topology_id[8]; } __packed;
+struct displayid_detailed_timings_1 {
- u8 pixel_clock[3];
- u8 flags;
- u8 hactive[2];
- u8 hblank[2];
- u8 hsync[2];
- u8 hsw[2];
- u8 vactive[2];
- u8 vblank[2];
- u8 vsync[2];
- u8 vsw[2];
An alternative would be to declare these fields as __le16, and you could read them in drm_mode_displayid_detailed() using le16_to_cpu().
Anyway, these structs should be __packed.
BR, Jani.
+};
+struct displayid_detailed_timing_block {
- struct displayid_block base;
- struct displayid_detailed_timings_1 timings[0];
+}; #endif
+struct displayid_detailed_timings_1 {
u8 pixel_clock[3];u8 flags;u8 hactive[2];u8 hblank[2];u8 hsync[2];u8 hsw[2];u8 vactive[2];u8 vblank[2];u8 vsync[2];u8 vsw[2];An alternative would be to declare these fields as __le16, and you could read them in drm_mode_displayid_detailed() using le16_to_cpu().
We don't do that for the EDID structs, so I'd rather avoid it here as well.
The spec is always in terms of 8-bit values.
Anyway, these structs should be __packed.
Indeed.
Dave.
dri-devel@lists.freedesktop.org
-
Dave Airlie -
Jani Nikula -
Michel Dänzer -
Ville Syrjälä