Hi,
The following two patches add support for parsing the list of additional 3D modes at the end of the vendor specific data block. The first splits the VIC display mode lookup into a separate function so that it can be reused. The second patch parses the list, adding any support modes to the connector.
Regards,
Thomas
Signed-off-by: Thomas Wood thomas.wood@intel.com --- drivers/gpu/drm/drm_edid.c | 67 +++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 52e060e..1dd82cd 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2557,25 +2557,40 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) return modes; }
-static int -do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) +static struct drm_display_mode * +drm_display_mode_from_vic_index(struct drm_connector *connector, + const u8 *video_db, u8 video_len, + u8 video_index) { struct drm_device *dev = connector->dev; - const u8 *mode; + struct drm_display_mode *newmode; u8 cea_mode; - int modes = 0;
- for (mode = db; mode < db + len; mode++) { - cea_mode = (*mode & 127) - 1; /* CEA modes are numbered 1..127 */ - if (cea_mode < ARRAY_SIZE(edid_cea_modes)) { - struct drm_display_mode *newmode; - newmode = drm_mode_duplicate(dev, - &edid_cea_modes[cea_mode]); - if (newmode) { - newmode->vrefresh = 0; - drm_mode_probed_add(connector, newmode); - modes++; - } + if (video_db == NULL || video_index > video_len) + return NULL; + + /* CEA modes are numbered 1..127 */ + cea_mode = (video_db[video_index] & 127) - 1; + if (cea_mode >= ARRAY_SIZE(edid_cea_modes)) + return NULL; + + newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]); + newmode->vrefresh = 0; + + return newmode; +} + +static int +do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) +{ + int i, modes = 0; + + for (i = 0; i < len; i++) { + struct drm_display_mode *mode; + mode = drm_display_mode_from_vic_index(connector, db, len, i); + if (mode) { + drm_mode_probed_add(connector, mode); + modes++; } }
@@ -2669,21 +2684,13 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic) static int add_3d_struct_modes(struct drm_connector *connector, u16 structure, const u8 *video_db, u8 video_len, u8 video_index) { - struct drm_device *dev = connector->dev; struct drm_display_mode *newmode; int modes = 0; - u8 cea_mode; - - if (video_db == NULL || video_index > video_len) - return 0; - - /* CEA modes are numbered 1..127 */ - cea_mode = (video_db[video_index] & 127) - 1; - if (cea_mode >= ARRAY_SIZE(edid_cea_modes)) - return 0;
if (structure & (1 << 0)) { - newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]); + newmode = drm_display_mode_from_vic_index(connector, video_db, + video_len, + video_index); if (newmode) { newmode->flags |= DRM_MODE_FLAG_3D_FRAME_PACKING; drm_mode_probed_add(connector, newmode); @@ -2691,7 +2698,9 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure, } } if (structure & (1 << 6)) { - newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]); + newmode = drm_display_mode_from_vic_index(connector, video_db, + video_len, + video_index); if (newmode) { newmode->flags |= DRM_MODE_FLAG_3D_TOP_AND_BOTTOM; drm_mode_probed_add(connector, newmode); @@ -2699,7 +2708,9 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure, } } if (structure & (1 << 8)) { - newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]); + newmode = drm_display_mode_from_vic_index(connector, video_db, + video_len, + video_index); if (newmode) { newmode->flags |= DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF; drm_mode_probed_add(connector, newmode);
On Thu, Nov 28, 2013 at 05:12:46PM +0000, Thomas Wood wrote:
Signed-off-by: Thomas Wood thomas.wood@intel.com
drivers/gpu/drm/drm_edid.c | 67 +++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 52e060e..1dd82cd 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2557,25 +2557,40 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) return modes; }
-static int -do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) +static struct drm_display_mode * +drm_display_mode_from_vic_index(struct drm_connector *connector,
const u8 *video_db, u8 video_len,u8 video_index){ struct drm_device *dev = connector->dev;
- const u8 *mode;
- struct drm_display_mode *newmode; u8 cea_mode;
int modes = 0;
for (mode = db; mode < db + len; mode++) {
cea_mode = (*mode & 127) - 1; /* CEA modes are numbered 1..127 */if (cea_mode < ARRAY_SIZE(edid_cea_modes)) {struct drm_display_mode *newmode;newmode = drm_mode_duplicate(dev,&edid_cea_modes[cea_mode]);if (newmode) {newmode->vrefresh = 0;drm_mode_probed_add(connector, newmode);modes++;}
- if (video_db == NULL || video_index > video_len)
return NULL;
Hmm, this seems wrong. video_len is the length of the payload only, so the check should be >=. But this bug is already present in the current code in add_3d_struct_modes(). So maybe include another patch to fix that up first.
The rest looks OK to me.
- /* CEA modes are numbered 1..127 */
- cea_mode = (video_db[video_index] & 127) - 1;
- if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
return NULL;- newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
- newmode->vrefresh = 0;
- return newmode;
+}
+static int +do_cea_modes(struct drm_connector *connector, const u8 *db, u8 len) +{
- int i, modes = 0;
- for (i = 0; i < len; i++) {
struct drm_display_mode *mode;mode = drm_display_mode_from_vic_index(connector, db, len, i);if (mode) {drm_mode_probed_add(connector, mode);modes++;@@ -2669,21 +2684,13 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic) static int add_3d_struct_modes(struct drm_connector *connector, u16 structure, const u8 *video_db, u8 video_len, u8 video_index) {
struct drm_device *dev = connector->dev; struct drm_display_mode *newmode; int modes = 0;
u8 cea_mode;
if (video_db == NULL || video_index > video_len)
return 0;/* CEA modes are numbered 1..127 */
cea_mode = (video_db[video_index] & 127) - 1;
if (cea_mode >= ARRAY_SIZE(edid_cea_modes))
return 0;if (structure & (1 << 0)) {
newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
newmode = drm_display_mode_from_vic_index(connector, video_db,video_len,video_index);@@ -2691,7 +2698,9 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure, } } if (structure & (1 << 6)) {
newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
newmode = drm_display_mode_from_vic_index(connector, video_db,video_len,video_index);@@ -2699,7 +2708,9 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure, } } if (structure & (1 << 8)) {
newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]);
newmode = drm_display_mode_from_vic_index(connector, video_db,video_len,video_index);-- 1.8.4.2
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Parse 2D_VIC_order_X and 3D_Structure_X from the list at the end of the HDMI Vendor Specific Data Block.
Signed-off-by: Thomas Wood thomas.wood@intel.com --- drivers/gpu/drm/drm_edid.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1dd82cd..eb6b363 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2808,6 +2808,56 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, video_len, i); }
+ if (hdmi_3d_len <= 4) + goto out; + + offset += 4; + + for (i = 0; i < (hdmi_3d_len - 4); i++) { + int vic_index; + struct drm_display_mode *newmode = NULL; + unsigned int newflag = 0; + + if (((db[8 + offset + i] & 0x0f) > 7) + && (i + 1 == hdmi_3d_len - 4)) + break; + + /* 2D_VIC_order_X */ + vic_index = db[8 + offset + i] >> 4; + + /* 3D_Structure_X */ + switch (db[8 + offset + i] & 0x0f) { + case 0: + newflag = DRM_MODE_FLAG_3D_FRAME_PACKING; + break; + case 6: + newflag = DRM_MODE_FLAG_3D_TOP_AND_BOTTOM; + break; + case 8: + if ((db[9 + offset + i] >> 4) == 1) + newflag = DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF; + break; + } + + if (newflag != 0) { + newmode = drm_display_mode_from_vic_index(connector, + video_db, + video_len, + vic_index); + + if (newmode) { + newmode->flags |= newflag; + drm_mode_probed_add(connector, newmode); + modes++; + } + } + + if ((db[8 + offset + i] & 0x0f) > 7) { + /* Optional 3D_Detail_X and reserved */ + i++; + } + } + out: return modes; }
On Thu, Nov 28, 2013 at 05:12:47PM +0000, Thomas Wood wrote:
Parse 2D_VIC_order_X and 3D_Structure_X from the list at the end of the HDMI Vendor Specific Data Block.
Signed-off-by: Thomas Wood thomas.wood@intel.com
drivers/gpu/drm/drm_edid.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 1dd82cd..eb6b363 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2808,6 +2808,56 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, video_len, i); }
- if (hdmi_3d_len <= 4)
This number 4 should depend on the value of multi_present, no?
Also we haven't actually verified that all hdmi_3d_len bytes fit within the data block. If you do that a bit earlier, you could also skip the explicit 'len' checks for the multi_present cases since we also check those against hdmi_3d_len.
goto out;- offset += 4;
- for (i = 0; i < (hdmi_3d_len - 4); i++) {
int vic_index;struct drm_display_mode *newmode = NULL;unsigned int newflag = 0;if (((db[8 + offset + i] & 0x0f) > 7)&& (i + 1 == hdmi_3d_len - 4))break;
The '(db[8 + offset + i] & 0x0f) > 7' part is repeated a few times. Maybe add a 'bool detail_present = (db[8 + offset + i] & 0x0f) > 7' or something to make make things a bit easier to read.
/* 2D_VIC_order_X */vic_index = db[8 + offset + i] >> 4;/* 3D_Structure_X */switch (db[8 + offset + i] & 0x0f) {case 0:newflag = DRM_MODE_FLAG_3D_FRAME_PACKING;break;case 6:newflag = DRM_MODE_FLAG_3D_TOP_AND_BOTTOM;break;case 8:
Maybe add a /* 3D_Detail_X */ comment here for consistency with the other comments.
if ((db[9 + offset + i] >> 4) == 1)newflag = DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF;break;}if (newflag != 0) {newmode = drm_display_mode_from_vic_index(connector,video_db,video_len,vic_index);if (newmode) {newmode->flags |= newflag;drm_mode_probed_add(connector, newmode);modes++;}}if ((db[8 + offset + i] & 0x0f) > 7) {/* Optional 3D_Detail_X and reserved */i++;}- }
out: return modes; } -- 1.8.4.2
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Thomas Wood -
Ville Syrjälä