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@xxxxxxxxxx] > Sent: Tuesday, December 13, 2011 9:51 PM > To: Kavuri, Sateesh > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > 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@xxxxxxxxx> 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@xxxxxxxxx --- 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, -- 1.7.4.1 ================ _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel