Thanks for the comments. Fixed most of the issues with the earlier patch. Sending out a new one. Comments inline below.
-----Original Message----- From: Adam Jackson [mailto:ajax@redhat.com] Sent: Tuesday, December 13, 2011 9:51 PM To: Kavuri, Sateesh Cc: dri-devel@lists.freedesktop.org Subject: Re: [PATCH] drm: Enable reading 3D capabilities of 3D monitor
On Fri, 2011-12-09 at 11:46 +0000, Kavuri, Sateesh wrote:
if ((multi_val == STRUCTURE_PRESENT) ||(multi_val == STRUCTURE_MASK_PRESENT) ) {if ((edid_ext[i+15+hdmi_vic_len] & 0x01) ==0x01)
caps->format |= 0x0; /*FRAME_PACKING */
if ((edid_ext[i+15+hdmi_vic_len] & 0x02) ==0x02)
caps->format |= 0x1;/*FIELD_ALTERNATIVE */
if ((edid_ext[i+15+hdmi_vic_len] & 0x04) ==0x04)
caps->format |= 0x2; /*LINE_ALTERNATIVE */
if ((edid_ext[i+15+hdmi_vic_len] & 0x08) ==0x08)
caps->format |= 0x3;/*SIDE_BY_SIDE_FULL */
if ((edid_ext[i+15+hdmi_vic_len] & 0x10) ==0x10)
caps->format |= 0x4; /* L_DEPTH */if ((edid_ext[i+15+hdmi_vic_len] & 0x20) ==0x20)
caps->format |= 0x5; /*L_DEPTH_GFX_GFX_DEPTH */
if ((edid_ext[i+15+hdmi_vic_len] & 0x40) ==0x40)
caps->format |= 0x6; /* TOP_BOTTOM */if ((edid_ext[i+14+hdmi_vic_len] & 0x01) ==0x01)
caps->format |= 0x7; /* S_BY_S_HALF */if ((edid_ext[i+14+hdmi_vic_len] & 0x80) ==0x80)
caps->format |= 0x8; /*S_BY_S_HALF_QUINCUNX */
}This reads poorly, which makes me think it's wrong. Is format supposed to be a bitfield or an enum?
+EXPORT_SYMBOL(drm_detect_3D_monitor);
I suspect this is poorly named. There exist 3D displays which are not HDMI.
I want to integrate the other interfaces (eDP, DP, MIPI) panel detection in this method itself. Started off with implementing the HDMI part.
+#define VSIF_RESET_INDEX 0xFFFFFFF8 +#define VSIF_RESET_BIT_22 0xFFBFFFFF +#define VSIF_SET_BIT_19 0x80000 +#define VSIF_RESET_BIT_20 0xFFEFFFFF +#define VSIF_RESET_BIT_17 0x10000 +#define VSIF_SET_BIT_16 0xFFFDFFFF +#define VSIF_SET_MASTER_BIT 0x400000
i915 style is usually to write these as (1 << 22) I think.
These I have fixed. Patch with these fixes below.
More importantly, use semantic names for register contents. "Bit 17" doesn't mean anything.
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 8020798..5b4d09c 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -798,6 +798,8 @@ extern int drm_mode_gamma_set_ioctl(struct drm_device *dev, extern u8 *drm_find_cea_extension(struct edid *edid); extern bool drm_detect_hdmi_monitor(struct edid *edid); extern bool drm_detect_monitor_audio(struct edid *edid); +extern bool drm_detect_3D_monitor(struct edid *edid); //extern struct +panel_3d_caps* drm_detect_3D_monitor(struct edid *edid);
Well, which is it?
+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 */
+};
If format is supposed to be an enum, why aren't you using these symbolic values?
I fixed the part to use a u16 integer value to represent the format.
- ajax
================ From d4c989bc807be730b2693a843fe93d4d559c05eb Mon Sep 17 00:00:00 2001 From: Sateesh Kavuri sateesh.kavuri@intel.com Date: Fri, 9 Dec 2011 17:08:42 +0530 Subject: [PATCH] Patch to enable reading the EDID information of a 3D capable HDMI monitor. This EDID information would be used to enable a user space application to switch the mode of the monitor to a specific 3D format.
-- Signed-off=by: sateesh.kavuri@intel.com --- drivers/gpu/drm/drm_edid.c | 109 ++++++++++++++++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 25 ++++++++++ 2 files changed, 134 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index fe39c35..4fe49e8 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1322,6 +1322,9 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid, #define SPEAKER_BLOCK 0x04 #define EDID_BASIC_AUDIO (1 << 6)
+#define MONITOR_VIDEO_PRESENT 0x01 +#define PANEL_SUPPORTS_3D 0x01 + /** * Search EDID for CEA extension block. */ @@ -1563,6 +1566,110 @@ end: } EXPORT_SYMBOL(drm_detect_hdmi_monitor);
+enum { + NO_SPL_CAPS = 0x0, + STRUCTURE_PRESENT = 0x1, + STRUCTURE_MASK_PRESENT = 0x2 +} are_3d_caps_present; + +/** + * drm_detect_monitor_3D - check monitor 3D capabilities + * + * Monitor should have CEA extension block. + * Check if the monitor has 3D capabilities. If it does, store the capabilitie + * to a data structure + * + */ +struct drm_panel_3D_capabilities *drm_detect_3D_monitor(struct edid *edid) +{ + u8 *edid_ext; + int i, hdmi_id; + int start_offset, end_offset; + bool is_hdmi = false; + struct drm_panel_3D_capabilities *caps = 0; + + edid_ext = drm_find_cea_extension(edid); + if (!edid_ext) + goto end; + + /* Data block offset in CEA extension block */ + start_offset = 4; + end_offset = edid_ext[2]; + + /* 3D vars */ + int multi_present = 0; + + /* + * 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) ) { + 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; + } + + /* Check if 3D_Multi_present is set */ + if ((edid_ext[i+13] & 0x60) == 0x0) { + multi_present = true; + } + + /* 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; + if (multi_val == 0x0) + multi_present = NO_SPL_CAPS; + else if (multi_val == 0x1) + multi_present = STRUCTURE_PRESENT; + else if (multi_val == 0x2) + multi_val = STRUCTURE_MASK_PRESENT; + + 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 */ + } + break; + } + } + +end: + return caps; +} +EXPORT_SYMBOL(drm_detect_3D_monitor); + /** * drm_detect_monitor_audio - check monitor audio capability * @@ -1630,6 +1737,7 @@ static void drm_add_display_info(struct edid *edid, /* driver figures it out in this case */ info->bpc = 0; info->color_formats = 0; + info->caps_3D = 0;
/* Only defined for 1.4 with digital displays */ if (edid->revision < 4) @@ -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); }
/** diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 8020798..adfae4c 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -186,6 +186,28 @@ enum subpixel_order { #define DRM_COLOR_FORMAT_RGB444 (1<<0) #define DRM_COLOR_FORMAT_YCRCB444 (1<<1) #define DRM_COLOR_FORMAT_YCRCB422 (1<<2) + +enum s3d_formats_ext { + HORIZONTAL_SUB_SAMPLING = 0x10 +}; + +/* 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 */ +}; + +/* 3D capabilities */ +struct drm_panel_3D_capabilities { + u16 panel_supports_3D; + u16 format; +}; /* * Describes a given display (e.g. CRT or flat panel) and its limitations. */ @@ -208,6 +230,8 @@ struct drm_display_info { u8 cea_rev;
char *raw_edid; /* if any */ + + struct drm_panel_3D_capabilities *caps_3D; };
struct drm_framebuffer_funcs { @@ -798,6 +822,7 @@ extern int drm_mode_gamma_set_ioctl(struct drm_device *dev, extern u8 *drm_find_cea_extension(struct edid *edid); extern bool drm_detect_hdmi_monitor(struct edid *edid); extern bool drm_detect_monitor_audio(struct edid *edid); +extern struct drm_panel_3D_capabilities *drm_detect_3D_monitor(struct edid *edid); extern int drm_mode_page_flip_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); extern struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,