Parse the list of additional 3D modes (v2)

List overview All Threads
Download

newer

older

Re: linux-next: Tree for Nov 29...

[Bug 35457] [rs690m] Graphics...

Thomas Wood

29 Nov 2013 29 Nov '13

3:33 p.m.

Here's an updated series of patches taking into account Ville's review.

Regards,

Thomas

Show replies by date

Thomas Wood

29 Nov 29 Nov

3:33 p.m.

New subject: [PATCH 1/3] drm/edid: fix length check when adding extra 3D modes

Signed-off-by: Thomas Wood thomas.wood@intel.com --- drivers/gpu/drm/drm_edid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 52e060e..0a1e4a5 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2674,7 +2674,7 @@ static int add_3d_struct_modes(struct drm_connector *connector, u16 structure, int modes = 0; u8 cea_mode;

- if (video_db == NULL || video_index > video_len) + if (video_db == NULL || video_index >= video_len) return 0;

/* CEA modes are numbered 1..127 */

-- 1.8.4.2

Thomas Wood

3:33 p.m.

New subject: [PATCH 2/3] drm/edid: split VIC display mode lookup into a separate function

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 0a1e4a5..f1d6e1e 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);

-- 1.8.4.2

Thomas Wood

3:33 p.m.

New subject: [PATCH 3/3] drm/edid: parse the list of additional 3D modes

Parse 2D_VIC_order_X and 3D_Structure_X from the list at the end of the HDMI Vendor Specific Data Block.

v2: Use an offset value depending on 3D_Multi_present and add detail_present. (Ville Syrjälä)

Signed-off-by: Thomas Wood thomas.wood@intel.com --- drivers/gpu/drm/drm_edid.c | 64 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index f1d6e1e..9426563 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2734,7 +2734,7 @@ static int do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, const u8 *video_db, u8 video_len) { - int modes = 0, offset = 0, i, multi_present = 0; + int modes = 0, offset = 0, i, multi_present = 0, multi_len; u8 vic_len, hdmi_3d_len = 0; u16 mask; u16 structure_all; @@ -2783,12 +2783,15 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, if (!(multi_present == 1 || multi_present == 2)) goto out;

- if ((multi_present == 1 && len < (9 + offset)) || - (multi_present == 2 && len < (11 + offset))) + if (multi_present == 1) + multi_len = 2; + else + multi_len = 4; + + if (len < (offset + hdmi_3d_len)) goto out;

- if ((multi_present == 1 && hdmi_3d_len < 2) || - (multi_present == 2 && hdmi_3d_len < 4)) + if (hdmi_3d_len < multi_len) goto out;

/* 3D_Structure_ALL */ @@ -2808,6 +2811,57 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, video_len, i); }

+ if (hdmi_3d_len <= multi_len) + goto out; + + offset += multi_len; + + for (i = 0; i < (hdmi_3d_len - multi_len); i++) { + int vic_index; + struct drm_display_mode *newmode = NULL; + unsigned int newflag = 0; + bool detail_present; + + detail_present = ((db[8 + offset + i] & 0x0f) > 7); + + if (detail_present && (i + 1 == hdmi_3d_len - multi_len)) + 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: + /* 3D_Detail_X */ + 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 (detail_present) + i++; + } + out: return modes; }

-- 1.8.4.2

Ville Syrjälä

4:47 p.m.

New subject: [PATCH 3/3] drm/edid: parse the list of additional 3D modes

On Fri, Nov 29, 2013 at 03:33:28PM +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.

v2: Use an offset value depending on 3D_Multi_present and add detail_present. (Ville Syrjälä)

Signed-off-by: Thomas Wood thomas.wood@intel.com

drivers/gpu/drm/drm_edid.c | 64 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 59 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index f1d6e1e..9426563 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2734,7 +2734,7 @@ static int do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, const u8 *video_db, u8 video_len) {

int modes = 0, offset = 0, i, multi_present = 0;

int modes = 0, offset = 0, i, multi_present = 0, multi_len; u8 vic_len, hdmi_3d_len = 0; u16 mask; u16 structure_all;

@@ -2783,12 +2783,15 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, if (!(multi_present == 1 || multi_present == 2)) goto out;

We should actually skip the 3D_Structure_ALL/MASK stuff in this case, but we should still go on and parse the 2D_VIC_order stuff. So this check should be removed.

...

if ((multi_present == 1 && len < (9 + offset)) ||
   (multi_present == 2 && len < (11 + offset)))
if (multi_present == 1)
multi_len = 2;
else

and this should then be 'else if (multi_present == 2)' and initialize multi_len to 0 in the beginning.

...

multi_len = 4;
if (len < (offset + hdmi_3d_len))

At this point db[8 + offset] we be the first 3d byte, so I think this should actually be something like:

if (len < (8 + offset + hdmi_3d_len - 1))

...

goto out;
if ((multi_present == 1 && hdmi_3d_len < 2) ||
   (multi_present == 2 && hdmi_3d_len < 4))
if (hdmi_3d_len < multi_len) goto out;

/* 3D_Structure_ALL */

@@ -2808,6 +2811,57 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, video_len, i); }
if (hdmi_3d_len <= multi_len)
goto out;

Already checked, except the == case, but the loop will check that for us, so this check can be dropped.

...

offset += multi_len;
for (i = 0; i < (hdmi_3d_len - multi_len); i++) {
```
int vic_index;
```

struct drm_display_mode *newmode = NULL;

```
unsigned int newflag = 0;
```
```
bool detail_present;
```

detail_present = ((db[8 + offset + i] & 0x0f) > 7);

if (detail_present && (i + 1 == hdmi_3d_len - multi_len))

```
	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:
```
```
	/* 3D_Detail_X */
```
```
	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 (detail_present)
```
```
	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

-- Ville Syrjälä Intel OTC

Thomas Wood

6:18 p.m.

New subject: [PATCH] drm/edid: parse the list of additional 3D modes

Parse 2D_VIC_order_X and 3D_Structure_X from the list at the end of the HDMI Vendor Specific Data Block.

v2: Use an offset value depending on 3D_Multi_present and add detail_present. (Ville Syrjälä) v3: Make sure the list is parsed even if 3D_Structure_ALL/MASK is not present. (Ville Syrjälä) Fix one length check and remove another. (Ville Syrjälä)

Signed-off-by: Thomas Wood thomas.wood@intel.com --- drivers/gpu/drm/drm_edid.c | 94 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 73 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index f1d6e1e..4c82a80 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2734,7 +2734,7 @@ static int do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, const u8 *video_db, u8 video_len) { - int modes = 0, offset = 0, i, multi_present = 0; + int modes = 0, offset = 0, i, multi_present = 0, multi_len; u8 vic_len, hdmi_3d_len = 0; u16 mask; u16 structure_all; @@ -2780,32 +2780,84 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, } offset += 1 + vic_len;

- if (!(multi_present == 1 || multi_present == 2)) - goto out; + if (multi_present == 1) + multi_len = 2; + else if (multi_present == 2) + multi_len = 4; + else + multi_len = 0;

- if ((multi_present == 1 && len < (9 + offset)) || - (multi_present == 2 && len < (11 + offset))) + if (len < (8 + offset + hdmi_3d_len - 1)) goto out;

- if ((multi_present == 1 && hdmi_3d_len < 2) || - (multi_present == 2 && hdmi_3d_len < 4)) + if (hdmi_3d_len < multi_len) goto out;

- /* 3D_Structure_ALL */ - structure_all = (db[8 + offset] << 8) | db[9 + offset]; + if (multi_present == 1 || multi_present == 2) { + /* 3D_Structure_ALL */ + structure_all = (db[8 + offset] << 8) | db[9 + offset];

- /* check if 3D_MASK is present */ - if (multi_present == 2) - mask = (db[10 + offset] << 8) | db[11 + offset]; - else - mask = 0xffff; - - for (i = 0; i < 16; i++) { - if (mask & (1 << i)) - modes += add_3d_struct_modes(connector, - structure_all, - video_db, - video_len, i); + /* check if 3D_MASK is present */ + if (multi_present == 2) + mask = (db[10 + offset] << 8) | db[11 + offset]; + else + mask = 0xffff; + + for (i = 0; i < 16; i++) { + if (mask & (1 << i)) + modes += add_3d_struct_modes(connector, + structure_all, + video_db, + video_len, i); + } + } + + offset += multi_len; + + for (i = 0; i < (hdmi_3d_len - multi_len); i++) { + int vic_index; + struct drm_display_mode *newmode = NULL; + unsigned int newflag = 0; + bool detail_present; + + detail_present = ((db[8 + offset + i] & 0x0f) > 7); + + if (detail_present && (i + 1 == hdmi_3d_len - multi_len)) + 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: + /* 3D_Detail_X */ + 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 (detail_present) + i++; }

out:

-- 1.8.4.2

Ville Syrjälä

7:10 p.m.

New subject: [PATCH] drm/edid: parse the list of additional 3D modes

On Fri, Nov 29, 2013 at 06:18:58PM +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.

v2: Use an offset value depending on 3D_Multi_present and add detail_present. (Ville Syrjälä) v3: Make sure the list is parsed even if 3D_Structure_ALL/MASK is not present. (Ville Syrjälä) Fix one length check and remove another. (Ville Syrjälä)

Signed-off-by: Thomas Wood thomas.wood@intel.com

For the series: Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

...

drivers/gpu/drm/drm_edid.c | 94 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 73 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index f1d6e1e..4c82a80 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2734,7 +2734,7 @@ static int do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, const u8 *video_db, u8 video_len) {

int modes = 0, offset = 0, i, multi_present = 0;

int modes = 0, offset = 0, i, multi_present = 0, multi_len; u8 vic_len, hdmi_3d_len = 0; u16 mask; u16 structure_all;

@@ -2780,32 +2780,84 @@ do_hdmi_vsdb_modes(struct drm_connector *connector, const u8 *db, u8 len, } offset += 1 + vic_len;
if (!(multi_present == 1 || multi_present == 2))
goto out;
if (multi_present == 1)
multi_len = 2;
else if (multi_present == 2)
multi_len = 4;
else
multi_len = 0;
if ((multi_present == 1 && len < (9 + offset)) ||
   (multi_present == 2 && len < (11 + offset)))
if (len < (8 + offset + hdmi_3d_len - 1)) goto out;
if ((multi_present == 1 && hdmi_3d_len < 2) ||
   (multi_present == 2 && hdmi_3d_len < 4))
if (hdmi_3d_len < multi_len) goto out;

/* 3D_Structure_ALL */

structure_all = (db[8 + offset] << 8) | db[9 + offset];
if (multi_present == 1 || multi_present == 2) {
/* 3D_Structure_ALL */
structure_all = (db[8 + offset] << 8) | db[9 + offset];
/* check if 3D_MASK is present */

if (multi_present == 2)
mask = (db[10 + offset] << 8) | db[11 + offset];
else
mask = 0xffff;
for (i = 0; i < 16; i++) {
if (mask & (1 << i))
	modes += add_3d_struct_modes(connector,
				     structure_all,
				     video_db,
				     video_len, i);
/* check if 3D_MASK is present */
if (multi_present == 2)
	mask = (db[10 + offset] << 8) | db[11 + offset];
else
	mask = 0xffff;
for (i = 0; i < 16; i++) {
	if (mask & (1 << i))
		modes += add_3d_struct_modes(connector,
				structure_all,
				video_db,
				video_len, i);
}
}

offset += multi_len;

for (i = 0; i < (hdmi_3d_len - multi_len); i++) {
int vic_index;
struct drm_display_mode *newmode = NULL;
unsigned int newflag = 0;
bool detail_present;
detail_present = ((db[8 + offset + i] & 0x0f) > 7);
if (detail_present && (i + 1 == hdmi_3d_len - multi_len))
	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:
	/* 3D_Detail_X */
	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 (detail_present)
	i++;
}
out:

1.8.4.2

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

-- Ville Syrjälä Intel OTC

4195

Age (days ago)

4195

Last active (days ago)

dri-devel@lists.freedesktop.org

6 comments

2 participants

tags (0)

participants (2)

Thomas Wood
Ville Syrjälä