[PATCH 1/3] drm/edid: Be stricter about stereo mode rejection

[PATCH 1/3] drm/edid: Be stricter about stereo mode rejection

[Adam Jackson]

Either bit 5 or 6 of that byte may be set in a stereo mode.

E-EDID v1.4, Table 3.22

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/drm_edid.c |    5 +++--
 include/drm/drm_edid.h     |    2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index eb92fe2..e7547e3 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -883,10 +883,11 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
 	if (hactive < 64 || vactive < 64)
 		return NULL;
 
-	if (pt->misc & DRM_EDID_PT_STEREO) {
-		printk(KERN_WARNING "stereo mode not supported\n");
+	if (pt->misc & DRM_EDID_PT_STEREO_MASK) {
+		DRM_DEBUG_KMS(KERN_WARNING "stereo modes not supported\n");
 		return NULL;
 	}
+
 	if (!(pt->misc & DRM_EDID_PT_SEPARATE_SYNC)) {
 		printk(KERN_WARNING "composite sync not supported\n");
 	}
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 0cac551..6350ea0 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -56,7 +56,7 @@ struct std_timing {
 #define DRM_EDID_PT_HSYNC_POSITIVE (1 << 1)
 #define DRM_EDID_PT_VSYNC_POSITIVE (1 << 2)
 #define DRM_EDID_PT_SEPARATE_SYNC  (3 << 3)
-#define DRM_EDID_PT_STEREO         (1 << 5)
+#define DRM_EDID_PT_STEREO_MASK    (3 << 5)
 #define DRM_EDID_PT_INTERLACED     (1 << 7)
 
 /* If detailed data is pixel timing */
-- 
1.7.7.6

[PATCH 2/3] drm/edid: Pull mode sync flag setup out to its own function

[Adam Jackson]

For readability, since this is about to get more complicated.

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/drm_edid.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index e7547e3..be21040 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -853,6 +853,16 @@ drm_mode_do_interlace_quirk(struct drm_display_mode *mode,
 	mode->flags |= DRM_MODE_FLAG_INTERLACE;
 }
 
+static void
+drm_mode_detailed_set_sync_flags(struct detailed_pixel_timing *pt,
+				 unsigned int *flags)
+{
+	*flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?
+		DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;
+	*flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ?
+		DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;
+}
+
 /**
  * drm_mode_detailed - create a new mode from an EDID detailed timing section
  * @dev: DRM device (needed to create new mode)
@@ -938,10 +948,7 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
 		pt->misc |= DRM_EDID_PT_HSYNC_POSITIVE | DRM_EDID_PT_VSYNC_POSITIVE;
 	}
 
-	mode->flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?
-		DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;
-	mode->flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ?
-		DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;
+	drm_mode_detailed_set_sync_flags(pt, &mode->flags);
 
 set_size:
 	mode->width_mm = pt->width_mm_lo | (pt->width_height_mm_hi & 0xf0) << 4;
-- 
1.7.7.6

[PATCH 3/3] drm/edid: Add csync parsing

[Adam Jackson]

Just assume saying "this is csync" is enough for whatever the output
type is.  The xfree86 mode flags distinguish between positive and
negative csync, but EDID doesn't encode that.

No connector types set csync_allowed yet, so this is a no-op besides
getting the message to shut up.

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 drivers/gpu/drm/drm_crtc.c        |    2 +-
 drivers/gpu/drm/drm_crtc_helper.c |    5 +++++
 drivers/gpu/drm/drm_edid.c        |   29 +++++++++++++++++++++--------
 include/drm/drm_crtc.h            |    2 ++
 include/drm/drm_edid.h            |   15 ++++++++++-----
 include/drm/drm_mode.h            |    4 ++++
 6 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 08a7aa7..265efe8 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1081,7 +1081,7 @@ static void drm_crtc_convert_to_umode(struct drm_mode_modeinfo *out,
 	out->vtotal = in->vtotal;
 	out->vscan = in->vscan;
 	out->vrefresh = in->vrefresh;
-	out->flags = in->flags;
+	out->flags = in->flags & DRM_MODE_FLAG_USERSPACE_MASK;
 	out->type = in->type;
 	strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
 	out->name[DRM_DISPLAY_MODE_LEN-1] = 0;
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 3252e70..d4625db 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -57,6 +57,9 @@ static void drm_mode_validate_flag(struct drm_connector *connector,
 		if ((mode->flags & DRM_MODE_FLAG_DBLSCAN) &&
 				!(flags & DRM_MODE_FLAG_DBLSCAN))
 			mode->status = MODE_NO_DBLESCAN;
+		if ((mode->flags & DRM_MODE_FLAG_CSYNC) &&
+				!(flags & DRM_MODE_FLAG_CSYNC))
+			mode->status = MODE_NO_CSYNC;
 	}
 
 	return;
@@ -140,6 +143,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 		mode_flags |= DRM_MODE_FLAG_INTERLACE;
 	if (connector->doublescan_allowed)
 		mode_flags |= DRM_MODE_FLAG_DBLSCAN;
+	if (connector->csync_allowed)
+		mode_flags |= DRM_MODE_FLAG_CSYNC;
 	drm_mode_validate_flag(connector, mode_flags);
 
 	list_for_each_entry(mode, &connector->modes, head) {
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index be21040..a8af442 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -857,10 +857,27 @@ static void
 drm_mode_detailed_set_sync_flags(struct detailed_pixel_timing *pt,
 				 unsigned int *flags)
 {
-	*flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?
-		DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;
-	*flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ?
-		DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;
+	switch (pt->misc & DRM_EDID_PT_SEPARATE_SYNC) {
+	case DRM_EDID_PT_SEPARATE_SYNC: /* digital, separate */
+		*flags |= (pt->misc & DRM_EDID_PT_HSYNC_POSITIVE) ?
+			DRM_MODE_FLAG_PHSYNC : DRM_MODE_FLAG_NHSYNC;
+		*flags |= (pt->misc & DRM_EDID_PT_VSYNC_POSITIVE) ?
+			DRM_MODE_FLAG_PVSYNC : DRM_MODE_FLAG_NVSYNC;
+		break;
+	case DRM_EDID_PT_DIGITAL_CSYNC: /* digital, composite */
+		*flags |= DRM_MODE_FLAG_CSYNC;
+		if (pt->misc & DRM_EDID_PT_CSYNC_SERRATION)
+		    *flags |= DRM_MODE_FLAG_SYNC_SERRATION;
+		break;
+	case DRM_EDID_PT_BIANALOG_CSYNC: /* bipolar analog composite */
+	case DRM_EDID_PT_ANALOG_CSYNC: /* unipolar analog composite */
+		*flags |= DRM_MODE_FLAG_CSYNC;
+		if (pt->misc & DRM_EDID_PT_CSYNC_SERRATION)
+		    *flags |= DRM_MODE_FLAG_SYNC_SERRATION;
+		if (pt->misc & DRM_EDID_PT_CSYNC_RGB)
+		    *flags |= DRM_MODE_FLAG_SYNC_RGB; /* else sync-on-green */
+		break;
+	}
 }
 
 /**
@@ -898,10 +915,6 @@ static struct drm_display_mode *drm_mode_detailed(struct drm_device *dev,
 		return NULL;
 	}
 
-	if (!(pt->misc & DRM_EDID_PT_SEPARATE_SYNC)) {
-		printk(KERN_WARNING "composite sync not supported\n");
-	}
-
 	/* it is incorrect if hsync/vsync width is zero */
 	if (!hsync_pulse_width || !vsync_pulse_width) {
 		DRM_DEBUG_KMS("Incorrect Detailed timing. "
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 73e4560..d0c958e 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -105,6 +105,7 @@ enum drm_mode_status {
     MODE_ONE_HEIGHT,    /* only one height is supported */
     MODE_ONE_SIZE,      /* only one resolution is supported */
     MODE_NO_REDUCED,    /* monitor doesn't accept reduced blanking */
+    MODE_NO_CSYNC,	/* connector doesn't support composite sync */
     MODE_UNVERIFIED = -3, /* mode needs to reverified */
     MODE_BAD = -2,	/* unspecified reason */
     MODE_ERROR	= -1	/* error condition */
@@ -560,6 +561,7 @@ struct drm_connector {
 	int connector_type_id;
 	bool interlace_allowed;
 	bool doublescan_allowed;
+	bool csync_allowed;
 	struct list_head modes; /* list of modes on this connector */
 
 	enum drm_connector_status status;
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 6350ea0..eb033fc 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -53,11 +53,16 @@ struct std_timing {
 	u8 vfreq_aspect;
 } __attribute__((packed));
 
-#define DRM_EDID_PT_HSYNC_POSITIVE (1 << 1)
-#define DRM_EDID_PT_VSYNC_POSITIVE (1 << 2)
-#define DRM_EDID_PT_SEPARATE_SYNC  (3 << 3)
-#define DRM_EDID_PT_STEREO_MASK    (3 << 5)
-#define DRM_EDID_PT_INTERLACED     (1 << 7)
+#define DRM_EDID_PT_HSYNC_POSITIVE  (1 << 1)
+#define DRM_EDID_PT_CSYNC_RGB       (1 << 1)
+#define DRM_EDID_PT_VSYNC_POSITIVE  (1 << 2)
+#define DRM_EDID_PT_CSYNC_SERRATION (1 << 2)
+#define DRM_EDID_PT_ANALOG_CSYNC    (0 << 3)
+#define DRM_EDID_PT_BIANALOG_CSYNC  (1 << 3)
+#define DRM_EDID_PT_DIGITAL_CSYNC   (2 << 3)
+#define DRM_EDID_PT_SEPARATE_SYNC   (3 << 3)
+#define DRM_EDID_PT_STEREO_MASK     (3 << 5)
+#define DRM_EDID_PT_INTERLACED      (1 << 7)
 
 /* If detailed data is pixel timing */
 struct detailed_pixel_timing {
diff --git a/include/drm/drm_mode.h b/include/drm/drm_mode.h
index 5581980..c1e043b 100644
--- a/include/drm/drm_mode.h
+++ b/include/drm/drm_mode.h
@@ -58,6 +58,10 @@
 #define DRM_MODE_FLAG_PIXMUX	(1<<11)
 #define DRM_MODE_FLAG_DBLCLK	(1<<12)
 #define DRM_MODE_FLAG_CLKDIV2	(1<<13)
+/* hide our internal flags for now */
+#define DRM_MODE_FLAG_USERSPACE_MASK ((1 << 14) - 1)
+#define DRM_MODE_FLAG_SYNC_SERRATION	(1 << 30)
+#define DRM_MODE_FLAG_SYNC_RGB		(1 << 31)
 
 /* DPMS flags */
 /* bit compatible with the xorg definitions. */
-- 
1.7.7.6

Re: [PATCH 1/3] drm/edid: Be stricter about stereo mode rejection

[Adam Jackson]

On 6/6/12 3:07 PM, Adam Jackson wrote:
> Either bit 5 or 6 of that byte may be set in a stereo mode.
>
> E-EDID v1.4, Table 3.22
>
> Signed-off-by: Adam Jackson <ajax@redhat.com>

Anyone want to give this series some love?

- ajax

Re: [PATCH 1/3] drm/edid: Be stricter about stereo mode rejection

[Alex Deucher]

On Fri, Aug 31, 2012 at 2:03 PM, Adam Jackson <ajax@redhat.com> wrote:
> On 6/6/12 3:07 PM, Adam Jackson wrote:
>>
>> Either bit 5 or 6 of that byte may be set in a stereo mode.
>>
>> E-EDID v1.4, Table 3.22
>>
>> Signed-off-by: Adam Jackson <ajax@redhat.com>
>
>
> Anyone want to give this series some love?

For the series:

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

>
> - ajax
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel