On Wed, 2011-12-14 at 13:04 +0000, Kavuri, Sateesh wrote:
- /* Data block offset in CEA extension block */
- start_offset = 4;
- end_offset = edid_ext[2];
- /* 3D vars */
- int multi_present = 0;
Pretty sure kernel style frowns upon mixed decls and code.
- /*
* Because HDMI identifier is in Vendor Specific Block,* search it from all data blocks of CEA extension.*/- for (i = start_offset; i < end_offset;
/* Increased by data block len */i += ((edid_ext[i] & 0x1f) + 1)) {/* Find vendor specific block *//* 6'th bit is the VIDEO_PRESENT bit */if ( ((edid_ext[i] >> 5) == VENDOR_BLOCK) &&((edid_ext[i+8] & 0x20) == MONITOR_VIDEO_PRESENT) ) {
This conditional will always be false:
if ((x == VENDOR_BLOCK) && ((y & 0x20) == 0x01))
I suspect you want:
#define MONITOR_VIDEO_PRESENT (1 << 6) ... if ((x == VENDOR_BLOCK) && (y & MONITOR_VIDEO_PRESENT)) { ...
hdmi_id = edid_ext[i + 1] | (edid_ext[i + 2] << 8) |edid_ext[i + 3] << 16;/* Find HDMI identifier */if (hdmi_id == HDMI_IDENTIFIER)is_hdmi = true;/* Check for the 3D_Present flag */if ((edid_ext[i+13] >> 6) == PANEL_SUPPORTS_3D) {caps = kmalloc(sizeof(struct drm_panel_3D_capabilities), GFP_KERNEL);caps->panel_supports_3D = 1;}
This will probably also not do what you want. Consider what happens if edid_ext[i+13] has (1 << 7) set.
/* Check if 3D_Multi_present is set */if ((edid_ext[i+13] & 0x60) == 0x0) {multi_present = true;}
Code and comment disagree.
/* Collect 3D capabilities of the monitor */int hdmi_3d_len = 0;int hdmi_vic_len = 0;hdmi_vic_len = (edid_ext[i+14] >> 0x05);hdmi_3d_len = ((edid_ext[i+14] << 0x03) >>0x03);int multi_val = edid_ext[i+13] & 0x6;
Note: multi_val can only have the values {0, 2, 4, 6} now.
if (multi_val == 0x0)multi_present = NO_SPL_CAPS;else if (multi_val == 0x1)multi_present = STRUCTURE_PRESENT;
These two assignments (and the one above) are the only use of the multi_present variable, it's never used in a subsequent conditional or passed back to the caller.
The 'else' here can never be true, as noted above.
if ((multi_val == STRUCTURE_PRESENT) ||(multi_val == STRUCTURE_MASK_PRESENT) ) {if (edid_ext[i+15+hdmi_vic_len] & (1 << 0))caps->format |= (1 << 0); /* FRAME_PACKING */if (edid_ext[i+15+hdmi_vic_len] & (1 << 1))caps->format |= (1 << 1); /*FIELD_ALTERNATIVE */if (edid_ext[i+15+hdmi_vic_len] & (1 << 2))caps->format |= (1 << 2); /* LINE_ALTERNATIVE */if (edid_ext[i+15+hdmi_vic_len] & 0x3)caps->format |= (1 << 3); /*SIDE_BY_SIDE_FULL */if (edid_ext[i+15+hdmi_vic_len] & (1 << 4))caps->format |= (1 << 4); /* L_DEPTH */if (edid_ext[i+15+hdmi_vic_len] & 0x5)caps->format |= (1 << 5); /* L_DEPTH_GFX_GFX_DEPTH */if (edid_ext[i+15+hdmi_vic_len] & 0x6)caps->format |= (1 << 6); /* TOP_BOTTOM */if (edid_ext[i+14+hdmi_vic_len] & 0x7)caps->format |= (1 << 7); /* S_BY_S_HALF */if (edid_ext[i+14+hdmi_vic_len] & (1 << 8))caps->format |= (1 << 8); /* S_BY_S_HALF_QUINCUNX */}
Here you've made it clear that ->format is a bitfield, but the values should be in a header (so drivers can use them) and this code should only be using the symbolic names.
@@ -1675,6 +1783,7 @@ static void drm_add_display_info(struct edid *edid, return;
info->cea_rev = edid_ext[1];
- info->caps_3D= drm_detect_3D_monitor(edid);
}
/**
Whitespace.
+/* Pre-defined 3D formats as per HDMI spec */ +enum s3d_formats {
FRAME_PACKING = 0x0,FIELD_ALTERNATIVE = 0x1,LINE_ALTERNATIVE = 0x2,SIDE_BY_SIDE_FULL = 0x3,L_DEPTH = 0x4,L_DEPTH_GFX_GFX_DEPTH = 0x5,TOP_BOTTOM = 0x6, /* 0x7 is reserved for future */SIDE_BY_SIDE_HALF = 0x8 /* 0x9 onwards is reserved for future */+};
These don't match the bit shifts you used earlier.
- ajax