RE: [PATCH] drm: Enable reading 3D capabilities of 3D monitor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux