[PATCH] drm/i915: properly handle interlaced bit for sdvo dtd conversion

[PATCH] drm/i915: properly handle interlaced bit for sdvo dtd conversion

[Daniel Vetter]

We've simply ignored this, which isn't too great. With this, interlaced
1080i works on my HDMI screen connected through sdvo. For no apparent
reason anything else still doesn't work as it should.

While at it, give these magic numbers in the dtd proper names and
add a comment that they match with EDID detailed timings.

v2: Actually use the right bit for interlaced.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_sdvo.c      |   12 ++++++++----
 drivers/gpu/drm/i915/intel_sdvo_regs.h |    5 +++++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 125228e..a658207 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -783,10 +783,12 @@ static void intel_sdvo_get_dtd_from_mode(struct intel_sdvo_dtd *dtd,
 		((v_sync_len & 0x30) >> 4);
 
 	dtd->part2.dtd_flags = 0x18;
+	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
+		dtd->part2.dtd_flags |= DTD_FLAG_INTERLACE;
 	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
-		dtd->part2.dtd_flags |= 0x2;
+		dtd->part2.dtd_flags |= DTD_FLAG_HSYNC_POSITIVE;
 	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
-		dtd->part2.dtd_flags |= 0x4;
+		dtd->part2.dtd_flags |= DTD_FLAG_VSYNC_POSITIVE;
 
 	dtd->part2.sdvo_flags = 0;
 	dtd->part2.v_sync_off_high = v_sync_offset & 0xc0;
@@ -820,9 +822,11 @@ static void intel_sdvo_get_mode_from_dtd(struct drm_display_mode * mode,
 	mode->clock = dtd->part1.clock * 10;
 
 	mode->flags &= ~(DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC);
-	if (dtd->part2.dtd_flags & 0x2)
+	if (dtd->part2.dtd_flags & DTD_FLAG_INTERLACE)
+		mode->flags |= DRM_MODE_FLAG_INTERLACE;
+	if (dtd->part2.dtd_flags & DTD_FLAG_HSYNC_POSITIVE)
 		mode->flags |= DRM_MODE_FLAG_PHSYNC;
-	if (dtd->part2.dtd_flags & 0x4)
+	if (dtd->part2.dtd_flags & DTD_FLAG_VSYNC_POSITIVE)
 		mode->flags |= DRM_MODE_FLAG_PVSYNC;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_sdvo_regs.h b/drivers/gpu/drm/i915/intel_sdvo_regs.h
index 6b7b22f..9d03014 100644
--- a/drivers/gpu/drm/i915/intel_sdvo_regs.h
+++ b/drivers/gpu/drm/i915/intel_sdvo_regs.h
@@ -61,6 +61,11 @@ struct intel_sdvo_caps {
 	u16 output_flags;
 } __attribute__((packed));
 
+/* Note: SDVO detailed timing flags match EDID misc flags. */
+#define DTD_FLAG_HSYNC_POSITIVE (1 << 1)
+#define DTD_FLAG_VSYNC_POSITIVE (1 << 2)
+#define DTD_FLAG_INTERLACE	(1 << 7)
+
 /** This matches the EDID DTD structure, more or less */
 struct intel_sdvo_dtd {
 	struct {
-- 
1.7.8.3

Re: [PATCH] drm/i915: properly handle interlaced bit for sdvo dtd conversion

[Peter Ross]

[-- Attachment #1.1: Type: text/plain, Size: 572 bytes --]

On Sat, May 12, 2012 at 10:22:58PM +0200, Daniel Vetter wrote:
> We've simply ignored this, which isn't too great. With this, interlaced
> 1080i works on my HDMI screen connected through sdvo. For no apparent
> reason anything else still doesn't work as it should.
> 
> While at it, give these magic numbers in the dtd proper names and
> add a comment that they match with EDID detailed timings.
> 
> v2: Actually use the right bit for interlaced.

Tested okay with SVDO-HDMI and HD CRT. Cheers,

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: properly handle interlaced bit for sdvo dtd conversion

[Daniel Vetter]

On Sun, May 13, 2012 at 11:52:15AM +1000, Peter Ross wrote:
> On Sat, May 12, 2012 at 10:22:58PM +0200, Daniel Vetter wrote:
> > We've simply ignored this, which isn't too great. With this, interlaced
> > 1080i works on my HDMI screen connected through sdvo. For no apparent
> > reason anything else still doesn't work as it should.
> > 
> > While at it, give these magic numbers in the dtd proper names and
> > add a comment that they match with EDID detailed timings.
> > 
> > v2: Actually use the right bit for interlaced.
> 
> Tested okay with SVDO-HDMI and HD CRT. Cheers,

Hi Peter!

Thanks for testing this on a hdmi sdvo card. Can you please grab the
latest version of intel-gpu-tools and attach the output of
tests/testdisplay -i and then mark those that actually work correctly?
For easier testing you can just run testdisplay without any args, it will
cycle through all modes. Note though that you need to be on a vt and X
needs to be stopped (the tool uses kms directly).

HDMI on sdvo seems to be made of almost complete fail still (I can only
drive a few modes here). So getting some more information about what works
and what doesn't exactly would be interesting.

Thanks, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm/i915: properly handle interlaced bit for sdvo dtd conversion

[Paulo Zanoni]

2012/5/12 Daniel Vetter <daniel.vetter@ffwll.ch>:
> We've simply ignored this, which isn't too great. With this, interlaced
> 1080i works on my HDMI screen connected through sdvo. For no apparent
> reason anything else still doesn't work as it should.
>
> While at it, give these magic numbers in the dtd proper names and
> add a comment that they match with EDID detailed timings.
>
> v2: Actually use the right bit for interlaced.
>
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

This looks like 2 different patches merged into one :)

In theory, bits 2 and 1 could have a different meaning too, but I'm
not sure how applicable this is to sDVO. But yeah, I had to read this
code in the past and 0x2 and 0x4 were not exactly nice to read, so at
least now those bits have a name :)

Not tested, just reviewed against the sdvo and edid specs...
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_sdvo.c      |   12 ++++++++----
>  drivers/gpu/drm/i915/intel_sdvo_regs.h |    5 +++++
>  2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 125228e..a658207 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -783,10 +783,12 @@ static void intel_sdvo_get_dtd_from_mode(struct intel_sdvo_dtd *dtd,
>                ((v_sync_len & 0x30) >> 4);
>
>        dtd->part2.dtd_flags = 0x18;
> +       if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> +               dtd->part2.dtd_flags |= DTD_FLAG_INTERLACE;
>        if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> -               dtd->part2.dtd_flags |= 0x2;
> +               dtd->part2.dtd_flags |= DTD_FLAG_HSYNC_POSITIVE;
>        if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> -               dtd->part2.dtd_flags |= 0x4;
> +               dtd->part2.dtd_flags |= DTD_FLAG_VSYNC_POSITIVE;
>
>        dtd->part2.sdvo_flags = 0;
>        dtd->part2.v_sync_off_high = v_sync_offset & 0xc0;
> @@ -820,9 +822,11 @@ static void intel_sdvo_get_mode_from_dtd(struct drm_display_mode * mode,
>        mode->clock = dtd->part1.clock * 10;
>
>        mode->flags &= ~(DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC);
> -       if (dtd->part2.dtd_flags & 0x2)
> +       if (dtd->part2.dtd_flags & DTD_FLAG_INTERLACE)
> +               mode->flags |= DRM_MODE_FLAG_INTERLACE;
> +       if (dtd->part2.dtd_flags & DTD_FLAG_HSYNC_POSITIVE)
>                mode->flags |= DRM_MODE_FLAG_PHSYNC;
> -       if (dtd->part2.dtd_flags & 0x4)
> +       if (dtd->part2.dtd_flags & DTD_FLAG_VSYNC_POSITIVE)
>                mode->flags |= DRM_MODE_FLAG_PVSYNC;
>  }
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo_regs.h b/drivers/gpu/drm/i915/intel_sdvo_regs.h
> index 6b7b22f..9d03014 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo_regs.h
> +++ b/drivers/gpu/drm/i915/intel_sdvo_regs.h
> @@ -61,6 +61,11 @@ struct intel_sdvo_caps {
>        u16 output_flags;
>  } __attribute__((packed));
>
> +/* Note: SDVO detailed timing flags match EDID misc flags. */
> +#define DTD_FLAG_HSYNC_POSITIVE (1 << 1)
> +#define DTD_FLAG_VSYNC_POSITIVE (1 << 2)
> +#define DTD_FLAG_INTERLACE     (1 << 7)
> +
>  /** This matches the EDID DTD structure, more or less */
>  struct intel_sdvo_dtd {
>        struct {
> --
> 1.7.8.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH] drm/i915: properly handle interlaced bit for sdvo dtd conversion

[Daniel Vetter]

On Mon, May 21, 2012 at 03:38:34PM -0300, Paulo Zanoni wrote:
> 2012/5/12 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > We've simply ignored this, which isn't too great. With this, interlaced
> > 1080i works on my HDMI screen connected through sdvo. For no apparent
> > reason anything else still doesn't work as it should.
> >
> > While at it, give these magic numbers in the dtd proper names and
> > add a comment that they match with EDID detailed timings.
> >
> > v2: Actually use the right bit for interlaced.
> >
> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> This looks like 2 different patches merged into one :)
> 
> In theory, bits 2 and 1 could have a different meaning too, but I'm
> not sure how applicable this is to sDVO. But yeah, I had to read this
> code in the past and 0x2 and 0x4 were not exactly nice to read, so at
> least now those bits have a name :)
> 
> Not tested, just reviewed against the sdvo and edid specs...
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Picked up for -fixes, thanks for the review.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48