[PATCH 0/2] drm/i915: interlaced mode support (G35 VGA/SDVO)

[PATCH 0/2] drm/i915: interlaced mode support (G35 VGA/SDVO)

[Peter Ross]

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

This patch set enables enables interlaced mode output on the VGA
and SDVO connectors of the G35 chipset.

History here: https://bugs.freedesktop.org/show_bug.cgi?id=11220

I have tested the changes on an ASUS P5E-VM-HDMI mainboard with VGA
and HDMI CRTs attached. The G45 and SB documentation suggests that
this will also work on those chipsets. (Wording of the vertical
timing registers is near identical). Feedback welcome.

Peter Ross (2):
  drm/i915: specify vertical timings in frame units for interlaced
    modes (gen4+)
  drm/i915: allow interlaced mode output on the SDVO connector

 drivers/gpu/drm/i915/intel_display.c |    8 ++++++++
 drivers/gpu/drm/i915/intel_sdvo.c    |    2 +-
 2 files changed, 9 insertions(+), 1 deletions(-)

-- 
1.7.5.4

-- 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

[PATCH 1/2] drm/i915: specify vertical timings in frame units for interlaced modes (gen4+)

[Peter Ross]

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

The G35/G45/SandyBridge chipsets expect vertical timings in frame units,
whereas the DRM subsystem uses field units internally for interlaced modes.

Signed-off-by: Peter Ross <pross@xvid.org>
---
 drivers/gpu/drm/i915/intel_display.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2a3f707..ae62f5f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5316,6 +5316,14 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 		adjusted_mode->crtc_vblank_end -= 1;
 		adjusted_mode->crtc_vsync_end -= 1;
 		adjusted_mode->crtc_vsync_start -= 1;
+		if (INTEL_INFO(dev)->gen >= 4) {
+			adjusted_mode->crtc_vdisplay *= 2;
+			adjusted_mode->crtc_vtotal *= 2;
+			adjusted_mode->crtc_vblank_start *= 2;
+			adjusted_mode->crtc_vblank_end *= 2;
+			adjusted_mode->crtc_vsync_end *= 2;
+			adjusted_mode->crtc_vsync_start *= 2;
+		}
 	} else
 		pipeconf &= ~PIPECONF_INTERLACE_MASK; /* progressive */
 
-- 
1.7.5.4

-- 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 1/2] drm/i915: specify vertical timings in frame units for interlaced modes (gen4+)

[Jesse Barnes]

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

On Sun, 15 Jan 2012 01:52:11 +1100
Peter Ross <pross@xvid.org> wrote:

> The G35/G45/SandyBridge chipsets expect vertical timings in frame units,
> whereas the DRM subsystem uses field units internally for interlaced modes.
> 
> Signed-off-by: Peter Ross <pross@xvid.org>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2a3f707..ae62f5f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5316,6 +5316,14 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  		adjusted_mode->crtc_vblank_end -= 1;
>  		adjusted_mode->crtc_vsync_end -= 1;
>  		adjusted_mode->crtc_vsync_start -= 1;
> +		if (INTEL_INFO(dev)->gen >= 4) {
> +			adjusted_mode->crtc_vdisplay *= 2;
> +			adjusted_mode->crtc_vtotal *= 2;
> +			adjusted_mode->crtc_vblank_start *= 2;
> +			adjusted_mode->crtc_vblank_end *= 2;
> +			adjusted_mode->crtc_vsync_end *= 2;
> +			adjusted_mode->crtc_vsync_start *= 2;
> +		}
>  	} else
>  		pipeconf &= ~PIPECONF_INTERLACE_MASK; /* progressive */

I *think* this looks ok.  It'll only affect pre-ILK chipsets though and
interlaced modes.

Do we get the ILK+ side right here?

I'm with Eugeni; I'd like to see some tested-bys on this, otherwise it
looks good.

-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 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 1/2] drm/i915: specify vertical timings in frame units for interlaced modes (gen4+)

[Eugeni Dodonov]

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

On Mon, Jan 16, 2012 at 17:50, Jesse Barnes <jbarnes@virtuousgeek.org>wrote:

> Do we get the ILK+ side right here?
>

>From the specs, looks like ILK and SNB+ share same logic for this part.

-- 
Eugeni Dodonov
 <http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 543 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 1/2] drm/i915: specify vertical timings in frame units for interlaced modes (gen4+)

[Daniel Vetter]

On Sun, Jan 15, 2012 at 01:52:11AM +1100, Peter Ross wrote:
> The G35/G45/SandyBridge chipsets expect vertical timings in frame units,
> whereas the DRM subsystem uses field units internally for interlaced modes.
> 
> Signed-off-by: Peter Ross <pross@xvid.org>

On a quick look at the patch it have a confusion about chipset
generations. We generally call g35 i965 to avoid confusion with the gen3
device g33. Also i9xx_crtc_mode_set is only used on pre-ironlake (=gen5)
and hence does not include snb. You might want to fix up
ironlake_crtc_mode_set, too.

When quickly discussing this with Jesse on irc we concluded that this is
fine if it comes with a tested-by (for both patches) attached, preferrably
with quick details on which machines this was tested on.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_display.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2a3f707..ae62f5f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5316,6 +5316,14 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  		adjusted_mode->crtc_vblank_end -= 1;
>  		adjusted_mode->crtc_vsync_end -= 1;
>  		adjusted_mode->crtc_vsync_start -= 1;
> +		if (INTEL_INFO(dev)->gen >= 4) {
> +			adjusted_mode->crtc_vdisplay *= 2;
> +			adjusted_mode->crtc_vtotal *= 2;
> +			adjusted_mode->crtc_vblank_start *= 2;
> +			adjusted_mode->crtc_vblank_end *= 2;
> +			adjusted_mode->crtc_vsync_end *= 2;
> +			adjusted_mode->crtc_vsync_start *= 2;
> +		}
>  	} else
>  		pipeconf &= ~PIPECONF_INTERLACE_MASK; /* progressive */
>  
> -- 
> 1.7.5.4
> 
> -- Peter
> (A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)



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


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

[PATCH 2/2] drm/i915: allow interlaced mode output on the SDVO connector

[Peter Ross]

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

Signed-off-by: Peter Ross <pross@xvid.org>
---
 drivers/gpu/drm/i915/intel_sdvo.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index f7b9268..03a84f8 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1987,7 +1987,7 @@ intel_sdvo_connector_init(struct intel_sdvo_connector *connector,
 	drm_connector_helper_add(&connector->base.base,
 				 &intel_sdvo_connector_helper_funcs);
 
-	connector->base.base.interlace_allowed = 0;
+	connector->base.base.interlace_allowed = 1;
 	connector->base.base.doublescan_allowed = 0;
 	connector->base.base.display_info.subpixel_order = SubPixelHorizontalRGB;
 
-- 
1.7.5.4

-- 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 0/2] drm/i915: interlaced mode support (G35 VGA/SDVO)

[Eugeni Dodonov]

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

On Sat, Jan 14, 2012 at 12:52, Peter Ross <pross@xvid.org> wrote:

> This patch set enables enables interlaced mode output on the VGA
> and SDVO connectors of the G35 chipset.
>
> History here: https://bugs.freedesktop.org/show_bug.cgi?id=11220
>
> I have tested the changes on an ASUS P5E-VM-HDMI mainboard with VGA
> and HDMI CRTs attached. The G45 and SB documentation suggests that
> this will also work on those chipsets. (Wording of the vertical
> timing registers is near identical). Feedback welcome.
>
> Peter Ross (2):
>  drm/i915: specify vertical timings in frame units for interlaced
>    modes (gen4+)
>  drm/i915: allow interlaced mode output on the SDVO connector
>
>  drivers/gpu/drm/i915/intel_display.c |    8 ++++++++
>  drivers/gpu/drm/i915/intel_sdvo.c    |    2 +-
>  2 files changed, 9 insertions(+), 1 deletions(-)
>

I am not very familiar with this area, but both patches look correct to me
and are according to the specs as far as I can see.

So, for both of them:
Reviewed-by: Eugeni Dodonov <eugeni.dodonov@intel.com>

Thanks!

P.S.: it would be interested to gather some Tested-by's for this though..

-- 
Eugeni Dodonov
<http://eugeni.dodonov.net/>

[-- Attachment #1.2: Type: text/html, Size: 1732 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