RE: [PATCH] drm: Enable reading 3D capabilities of 3D monitor
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:a...@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 +, 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 0xFFF8
+#define VSIF_RESET_BIT_22 0xFFBF
+#define VSIF_SET_BIT_19 0x8
+#define VSIF_RESET_BIT_20 0xFFEF
+#define VSIF_RESET_BIT_17 0x1
+#define VSIF_SET_BIT_16 0xFFFD
+#define VSIF_SET_MASTER_BIT 0x40
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.kav...@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.kav...@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
[PATCH] drm: Enable reading 3D capabilities of 3D monitor
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 at redhat.com]
> Sent: Tuesday, December 13, 2011 9:51 PM
> To: Kavuri, Sateesh
> Cc: dri-devel at lists.freedesktop.org
> Subject: Re: [PATCH] drm: Enable reading 3D capabilities of 3D monitor
>
> On Fri, 2011-12-09 at 11:46 +, 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 0xFFF8
> > +#define VSIF_RESET_BIT_22 0xFFBF
> > +#define VSIF_SET_BIT_19 0x8
> > +#define VSIF_RESET_BIT_20 0xFFEF
> > +#define VSIF_RESET_BIT_17 0x1
> > +#define VSIF_SET_BIT_16 0xFFFD
> > +#define VSIF_SET_MASTER_BIT 0x40
>
> 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
[PATCH] drm: Enable reading 3D capabilities of 3D monitor
This is first of the patches to enable reading the 3D capabilities of a
connected monitor on
HDMI. Similar functionality would also be implemented for other interface, eDP,
DP
The idea is to read the 3D capabilities via EDID and provide this to a user
space application
that would want to switch the mode of the connected monitor to a specific 3D
format. The
implementation follows the HDMI 1.4a specification.
From 95dc9536dc1a863a4a2d12836ead338835a4be59 Mon Sep 17 00:00:00 2001
From: Sateesh Kavuri sateesh.kav...@intel.com
Date: Wed, 7 Dec 2011 14:49:20 +0530
Subject: [PATCH] This patch enables the reading of the 3D capabilities of the
connected
HDMI monitor. Currently only the 3D format support fields are read.
Later this could be extended. This information is expected to be sent to
the user space which then sets the format on the monitor.
This patch also handles DIP register signaling to the HDMI connected 3D
monitor the 3D format to be set
---
Signed-off-by: Sateesh Kavuri sateesh.kav...@intel.com
---
---
drivers/gpu/drm/drm_edid.c| 109 +
drivers/gpu/drm/i915/intel_drv.h | 11
drivers/gpu/drm/i915/intel_hdmi.c | 119 +
include/drm/drm_crtc.h|2 +
include/drm/drm_edid.h| 20 ++
5 files changed, 261 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index fe39c35..5d4a425 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1321,6 +1321,8 @@ add_detailed_modes(struct drm_connector *connector,
struct edid *edid,
#define VENDOR_BLOCK0x03
#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.
@@ -1610,6 +1612,113 @@ end:
}
EXPORT_SYMBOL(drm_detect_monitor_audio);
+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
+ *
+ * FIXME: Currently returning bool (to denote 3D present or not), later should
+ * return the 3D capabilities (or can there be a separate function for that?)
+ */
+bool drm_detect_3D_monitor(struct edid *edid)
+{
+ u8 *edid_ext;
+ int i, hdmi_id;
+ int start_offset, end_offset;
+ bool is_hdmi = false;
+ bool is_3d = false;
+ struct panel_3d_caps *caps = kmalloc(sizeof(struct panel_3d_caps),
GFP_KERNEL);
+
+ 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-panel_supports_3d = 1;
+ is_3d = true;
+ }
+
+ /* 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;
+
+
[PATCH] drm: Enable reading 3D capabilities of 3D monitor
This is first of the patches to enable reading the 3D capabilities of a connected monitor on HDMI. Similar functionality would also be implemented for other interface, eDP, DP The idea is to read the 3D capabilities via EDID and provide this to a user space application that would want to switch the mode of the connected monitor to a specific 3D format. The implementation follows the HDMI 1.4a specification.
