[PATCH 0/4] [CFT] interlaced support

[PATCH 0/4] [CFT] interlaced support

[Daniel Vetter]

Hi all,

I've had tons of fun reading through docs and code to get to the bottom of
who interlaced is supposed to work. The current work-in-progress is available
at:

http://cgit.freedesktop.org/~danvet/drm/log/?h=interlaced

The patches are completely untested because I lack a TV. So testing feedback
highly appreciated. Following up are the patches I haven't sent out yet, these
two are still in rather rough shape.

Yours, Daniel

Daniel Vetter (2):
  drm/i915: fixup interlaced vertical timings confusion
  drm/i915: fixup interlaced support on ilk+

Peter Ross (2):
  drm/i915: allow interlaced mode output on the SDVO connector
  drm/i915: allow interlaced mode output on the HDMI connector

 drivers/gpu/drm/i915/intel_display.c |   37 ++++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_dvo.c     |    1 -
 drivers/gpu/drm/i915/intel_hdmi.c    |    2 +-
 drivers/gpu/drm/i915/intel_overlay.c |    2 +-
 drivers/gpu/drm/i915/intel_panel.c   |    2 +-
 drivers/gpu/drm/i915/intel_sdvo.c    |    3 +-
 6 files changed, 24 insertions(+), 23 deletions(-)

-- 
1.7.7.5

[PATCH 1/4] drm/i915: fixup interlaced vertical timings confusion

[Daniel Vetter]

We have a pretty decent confusion about vertical timings of interlaced
modes. Peter Ross has written a patch that makes interlace modes work
on a lot more platforms/output combinations by doubling the vertical
timings.

The issue with that patch is that core drm _does_ support specifying
whether we want these vertical timings in fields or frames, we just
haven't managed to consistently use this facility. The relavant
function is drm_mode_set_crtcinfo, which fills in the crtc timing
information.

The first thing to note is that the drm core keeps interlaced modes in
frames, but displays modelines in fields. So when the crtc modeset
helper copies over the mode into adjusted_mode it will already contain
vertical timings in half-frames. The result is that the fixup code in
intel_crtc_mode_fixup doesn't actually do anything.

Now gen3+ natively supports interlaced modes and wants the vertical
timings in frames. Which is what sdvo already fixes up, at least under
some conditions.

On gen2 we only support interlaced through dvo add-ons. The
corresponding mode_fixup wants vertical timings in fields, so use that
unconditionally.

There are a few other place that demand vertical timings in fields
but never actually deal with interlaced modes (like panels), so use
frame timings for consistency, too.

That leaves us with native TV out support: I honestly have no clue
what this needs, but suspect that it simply ignores the pipe's timing
and uses the special TV timings (which are per-field). These are
programmed with fixed timing informations out of a pre-defined list of
struct tv_mode.

After all this we also have to fix-up the interlaced timing
adjustements and substract 2 half-lines (instead of 1 with the halfed
timings). According to Peter Ross, this is actually what Windows does.

Patch is completely untested because I don't have a TV. So please go
wild.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |   35 ++++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_dvo.c     |    1 -
 drivers/gpu/drm/i915/intel_overlay.c |    2 +-
 drivers/gpu/drm/i915/intel_panel.c   |    2 +-
 drivers/gpu/drm/i915/intel_sdvo.c    |    1 -
 5 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index de26748..be078cb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3425,10 +3425,13 @@ static bool intel_crtc_mode_fixup(struct drm_crtc *crtc,
 			return false;
 	}
 
-	/* XXX some encoders set the crtcinfo, others don't.
-	 * Obviously we need some form of conflict resolution here...
-	 */
-	if (adjusted_mode->crtc_htotal == 0)
+	/* gen2 needs vertical crtc timing information in fields because that's
+	 * what dvo outputs want - the chip itself can't do interlaced. All
+	 * later generations can do interlaced natively and want timings in
+	 * full frames. */
+	if (IS_GEN2(dev))
+		drm_mode_set_crtcinfo(adjusted_mode, CRTC_INTERLACE_HALVE_V);
+	else
 		drm_mode_set_crtcinfo(adjusted_mode, 0);
 
 	return true;
@@ -5369,12 +5372,12 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
 		pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION;
 		/* the chip adds 2 halflines automatically */
-		adjusted_mode->crtc_vdisplay -= 1;
-		adjusted_mode->crtc_vtotal -= 1;
-		adjusted_mode->crtc_vblank_start -= 1;
-		adjusted_mode->crtc_vblank_end -= 1;
-		adjusted_mode->crtc_vsync_end -= 1;
-		adjusted_mode->crtc_vsync_start -= 1;
+		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_PROGRESSIVE;
 
@@ -5962,12 +5965,12 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
 		pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION;
 		/* the chip adds 2 halflines automatically */
-		adjusted_mode->crtc_vdisplay -= 1;
-		adjusted_mode->crtc_vtotal -= 1;
-		adjusted_mode->crtc_vblank_start -= 1;
-		adjusted_mode->crtc_vblank_end -= 1;
-		adjusted_mode->crtc_vsync_end -= 1;
-		adjusted_mode->crtc_vsync_start -= 1;
+		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_PROGRESSIVE;
 
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 6eda1b5..020a7d7 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -157,7 +157,6 @@ static bool intel_dvo_mode_fixup(struct drm_encoder *encoder,
 		C(vsync_end);
 		C(vtotal);
 		C(clock);
-		drm_mode_set_crtcinfo(adjusted_mode, CRTC_INTERLACE_HALVE_V);
 #undef C
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index cdf17d4..aac74ec 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -263,7 +263,7 @@ i830_activate_pipe_a(struct drm_device *dev)
 	DRM_DEBUG_DRIVER("Enabling pipe A in order to enable overlay\n");
 
 	mode = drm_mode_duplicate(dev, &vesa_640x480);
-	drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
+	drm_mode_set_crtcinfo(mode, 0);
 	if (!drm_crtc_helper_set_mode(&crtc->base, mode,
 				       crtc->base.x, crtc->base.y,
 				       crtc->base.fb))
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index c935cda..230a141 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -48,7 +48,7 @@ intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
 
 	adjusted_mode->clock = fixed_mode->clock;
 
-	drm_mode_set_crtcinfo(adjusted_mode, CRTC_INTERLACE_HALVE_V);
+	drm_mode_set_crtcinfo(adjusted_mode, 0);
 }
 
 /* adjusted_mode has been preset to be the panel's fixed mode */
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index e334ec3..5b480bb 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -944,7 +944,6 @@ intel_sdvo_set_input_timings_for_mode(struct intel_sdvo *intel_sdvo,
 
 	intel_sdvo_get_mode_from_dtd(adjusted_mode, &intel_sdvo->input_dtd);
 
-	drm_mode_set_crtcinfo(adjusted_mode, 0);
 	return true;
 }
 
-- 
1.7.7.5

[PATCH 2/4] drm/i915: fixup interlaced support on ilk+

[Daniel Vetter]

According to Paulo Zanoni, this is what windows does.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index be078cb..8d6ed56 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5963,7 +5963,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
 
 	pipeconf &= ~PIPECONF_INTERLACE_MASK;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE) {
-		pipeconf |= PIPECONF_INTERLACE_W_FIELD_INDICATION;
+		pipeconf |= PIPECONF_INTERLACED_ILK;
 		/* the chip adds 2 halflines automatically */
 		adjusted_mode->crtc_vdisplay -= 2;
 		adjusted_mode->crtc_vtotal -= 2;
-- 
1.7.7.5

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

[Daniel Vetter]

From: Peter Ross <pross@xvid.org>

Signed-off-by: Peter Ross <pross@xvid.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 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 5b480bb..80fb5da 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1984,7 +1984,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.7.5

[PATCH 4/4] drm/i915: allow interlaced mode output on the HDMI connector

[Daniel Vetter]

From: Peter Ross <pross@xvid.org>

Signed-off-by: Peter Ross <pross@xvid.org>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_hdmi.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 64541f7..086288e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -514,7 +514,7 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg)
 	intel_encoder->type = INTEL_OUTPUT_HDMI;
 
 	connector->polled = DRM_CONNECTOR_POLL_HPD;
-	connector->interlace_allowed = 0;
+	connector->interlace_allowed = 1;
 	connector->doublescan_allowed = 0;
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
 
-- 
1.7.7.5

Re: [PATCH 0/4] [CFT] interlaced support

[Alfonso Fiore]

Hi Daniel,
first of all thanks a lot for the effort!
I'll be more than happy to test your work (both patches you sent
today), but can you please write the code to download and apply these
patches?
Please consider I have already download and compiled both trees.
Which git do they appy to? (Peter's patches apply to the main tree)
any test in particular?
Thank you.
Eager to test,
alfonso

On 26/01/2012, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Hi all,
>
> I've had tons of fun reading through docs and code to get to the bottom of
> who interlaced is supposed to work. The current work-in-progress is
> available
> at:
>
> http://cgit.freedesktop.org/~danvet/drm/log/?h=interlaced
>
> The patches are completely untested because I lack a TV. So testing feedback
> highly appreciated. Following up are the patches I haven't sent out yet,
> these
> two are still in rather rough shape.
>
> Yours, Daniel
>
> Daniel Vetter (2):
>   drm/i915: fixup interlaced vertical timings confusion
>   drm/i915: fixup interlaced support on ilk+
>
> Peter Ross (2):
>   drm/i915: allow interlaced mode output on the SDVO connector
>   drm/i915: allow interlaced mode output on the HDMI connector
>
>  drivers/gpu/drm/i915/intel_display.c |   37
> ++++++++++++++++++---------------
>  drivers/gpu/drm/i915/intel_dvo.c     |    1 -
>  drivers/gpu/drm/i915/intel_hdmi.c    |    2 +-
>  drivers/gpu/drm/i915/intel_overlay.c |    2 +-
>  drivers/gpu/drm/i915/intel_panel.c   |    2 +-
>  drivers/gpu/drm/i915/intel_sdvo.c    |    3 +-
>  6 files changed, 24 insertions(+), 23 deletions(-)
>
> --
> 1.7.7.5
>
>

Re: [PATCH 0/4] [CFT] interlaced support

[Paul Menzel]

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

Dear Intel-gfx folks,


Am Donnerstag, den 26.01.2012, 22:01 +0100 schrieb Daniel Vetter:

> I've had tons of fun reading through docs and code to get to the bottom of
> who interlaced is supposed to work. The current work-in-progress is available
> at:
> 
> http://cgit.freedesktop.org/~danvet/drm/log/?h=interlaced
> 
> The patches are completely untested because I lack a TV. So testing feedback
> highly appreciated. Following up are the patches I haven't sent out yet, these
> two are still in rather rough shape.

thank you for these patches.

> Daniel Vetter (2):
>   drm/i915: fixup interlaced vertical timings confusion
>   drm/i915: fixup interlaced support on ilk+
> 
> Peter Ross (2):
>   drm/i915: allow interlaced mode output on the SDVO connector
>   drm/i915: allow interlaced mode output on the HDMI connector

could the commit messages of these two patches be extended? Like I think
to remember that interlaced mode not allowed some time. So what commit
was that and (if it does not contain the reasons) why is it allowed
again.

Additionally Thomas Hilber once did some work on FRC (frame rate
correction) [1]. These patches are quite old and Thomas does not work an
that anymore. The repository I tried to get these patches upstreamed is
also not working anymore, so one would need to get the archives from
Thomas’ site [2].

Is there a chance to pick up Thomas’ work again? I think the main
problem was that Thomas’ work was more or less a hack to get it working.
To do it properly some interfaces have to be defined or so so that other
drivers like radeon or nouveau can do that too.

[…]


Thanks,

Paul


[1] http://vga2scart.gw90.de/
    (I just noticed there were some spammers active on the page. Will
clean that up soon.)
[2] http://lowbyte.de/vga-sync-fields/

[-- Attachment #1.2: This is a digitally signed message part --]
[-- 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/4] [CFT] interlaced support

[Daniel Vetter]

On Thu, Jan 26, 2012 at 10:38:47PM +0100, Alfonso Fiore wrote:
> Hi Daniel,
> first of all thanks a lot for the effort!
> I'll be more than happy to test your work (both patches you sent
> today), but can you please write the code to download and apply these
> patches?
> Please consider I have already download and compiled both trees.
> Which git do they appy to? (Peter's patches apply to the main tree)
> any test in particular?
> Thank you.
> Eager to test,
> alfonso

It's a complete new git tree, so you need to clone it and the again check
out the branch. About testing, this contains all the patches I've already
posted and hence hopefully fixes all your issues. If you need further
help, please ask on irc, that's quicker.

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

Re: [PATCH 1/4] drm/i915: fixup interlaced vertical timings confusion

[Chris Wilson]

On Thu, 26 Jan 2012 22:01:30 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> -	/* XXX some encoders set the crtcinfo, others don't.
> -	 * Obviously we need some form of conflict resolution here...
> -	 */
> -	if (adjusted_mode->crtc_htotal == 0)
> +	/* gen2 needs vertical crtc timing information in fields because that's
> +	 * what dvo outputs want - the chip itself can't do interlaced. All
> +	 * later generations can do interlaced natively and want timings in
> +	 * full frames. */
> +	if (IS_GEN2(dev))
> +		drm_mode_set_crtcinfo(adjusted_mode, CRTC_INTERLACE_HALVE_V);
> +	else
>  		drm_mode_set_crtcinfo(adjusted_mode, 0);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> index 6eda1b5..020a7d7 100644
> --- a/drivers/gpu/drm/i915/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/intel_dvo.c
> @@ -157,7 +157,6 @@ static bool intel_dvo_mode_fixup(struct drm_encoder *encoder,
>  		C(vsync_end);
>  		C(vtotal);
>  		C(clock);
> -		drm_mode_set_crtcinfo(adjusted_mode, CRTC_INTERLACE_HALVE_V);
>  #undef C
>  	}

Removing drm_mode_set_crtcinfo() scares me because of the above comment.
We need to make sure that the adjusted_mode is initialised along
some path, and the fixup in intel_crtc_mode_fixup is just a hack.

commit 897493504addc5609f04a2c4f73c37ab972c29b2
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Sun Sep 12 18:25:19 2010 +0100

    drm/i915: Ensure that the crtcinfo is populated during mode_fixup()
    
    This should fix the mysterious mode setting failures reported during
    boot up and after resume, generally for i8xx class machines.
    
    Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=16478
    Reported-and-tested-by: Xavier Chantry <chantry.xavier@gmail.com>
    Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=29413
    Tested-by: Daniel Vetter <daniel@ffwll.ch>
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
    Cc: stable@kernel.org

If you can work out exactly where it should be initialised, you'll be my
hero!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 0/4] [CFT] interlaced support

[Alfonso Fiore]

>
> It's a complete new git tree, so you need to clone it and the again check
> out the branch. About testing, this contains all the patches I've already
> posted and hence hopefully fixes all your issues. If you need further
> help, please ask on irc, that's quicker.
>

can you please just tell me the name of the git and of the branch?
I tried to connect to irc but I didn't get the email confirmation for my nick
and I rather not waste time troubleshooting irc. :)

thanks,
alfonso

Re: [PATCH 1/4] drm/i915: fixup interlaced vertical timings confusion

[Daniel Vetter]

On Thu, Jan 26, 2012 at 10:03:02PM +0000, Chris Wilson wrote:
> On Thu, 26 Jan 2012 22:01:30 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > -	/* XXX some encoders set the crtcinfo, others don't.
> > -	 * Obviously we need some form of conflict resolution here...
> > -	 */
> > -	if (adjusted_mode->crtc_htotal == 0)
> > +	/* gen2 needs vertical crtc timing information in fields because that's
> > +	 * what dvo outputs want - the chip itself can't do interlaced. All
> > +	 * later generations can do interlaced natively and want timings in
> > +	 * full frames. */
> > +	if (IS_GEN2(dev))
> > +		drm_mode_set_crtcinfo(adjusted_mode, CRTC_INTERLACE_HALVE_V);
> > +	else
> >  		drm_mode_set_crtcinfo(adjusted_mode, 0);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> > index 6eda1b5..020a7d7 100644
> > --- a/drivers/gpu/drm/i915/intel_dvo.c
> > +++ b/drivers/gpu/drm/i915/intel_dvo.c
> > @@ -157,7 +157,6 @@ static bool intel_dvo_mode_fixup(struct drm_encoder *encoder,
> >  		C(vsync_end);
> >  		C(vtotal);
> >  		C(clock);
> > -		drm_mode_set_crtcinfo(adjusted_mode, CRTC_INTERLACE_HALVE_V);
> >  #undef C
> >  	}
> 
> Removing drm_mode_set_crtcinfo() scares me because of the above comment.
> We need to make sure that the adjusted_mode is initialised along
> some path, and the fixup in intel_crtc_mode_fixup is just a hack.
> 
> commit 897493504addc5609f04a2c4f73c37ab972c29b2
> Author: Chris Wilson <chris@chris-wilson.co.uk>
> Date:   Sun Sep 12 18:25:19 2010 +0100
> 
>     drm/i915: Ensure that the crtcinfo is populated during mode_fixup()
>     
>     This should fix the mysterious mode setting failures reported during
>     boot up and after resume, generally for i8xx class machines.
>     
>     Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=16478
>     Reported-and-tested-by: Xavier Chantry <chantry.xavier@gmail.com>
>     Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=29413
>     Tested-by: Daniel Vetter <daniel@ffwll.ch>
>     Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>     Cc: stable@kernel.org
> 
> If you can work out exactly where it should be initialised, you'll be my
> hero!

Afaik drm core calling set_crtcinfo is just a side-effect of some random
actions and it's not guaranteed to happen. The issue is that drm core puts
the wrong stuff in there. Also note that I'm now unconditionally doing the
fixup, so the particular bug above shouldn't show up again.

I'll check whether this is indeed the case on my dear old i855gm - let's
see whether I'm provoking the wrath of some ill-tempered gods here ;-)

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

Re: [PATCH 0/4] [CFT] interlaced support

[Daniel Vetter]

On Thu, Jan 26, 2012 at 11:10:32PM +0100, Alfonso Fiore wrote:
> >
> > It's a complete new git tree, so you need to clone it and the again check
> > out the branch. About testing, this contains all the patches I've already
> > posted and hence hopefully fixes all your issues. If you need further
> > help, please ask on irc, that's quicker.
> >
> 
> can you please just tell me the name of the git and of the branch?
> I tried to connect to irc but I didn't get the email confirmation for my nick
> and I rather not waste time troubleshooting irc. :)

git://people.freedesktop.org/~danvet/drm interlaced

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

Re: [PATCH 0/4] [CFT] interlaced support

[Alfonso Fiore]

On Thu, Jan 26, 2012 at 11:22 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> git://people.freedesktop.org/~danvet/drm interlaced

Thank you Daniel,

is it normal that is super-slow? git repors 1K/sec. I download about
10 MB so far...

alfonso

Re: [PATCH 0/4] [CFT] interlaced support

[Paulo Zanoni]

[-- Attachment #1: Type: text/plain, Size: 785 bytes --]

Hi

2012/1/26 Daniel Vetter <daniel.vetter@ffwll.ch>:
> Hi all,
>
> http://cgit.freedesktop.org/~danvet/drm/log/?h=interlaced
>

I just tested your patch set and it didn't work: my monitor reported
1920x539@50Hz. We're missing something from patch "fixup interlace
vertical timings confusion". The reg dump is attached.

By the way, VTOTAL_B was 1078. We need to remove those "-1" and also
those "-2". Look at ironlake_crtc_mode_set: we already do the
subtractions when we write to HSYNC, HTOTAL, etc. From the register
descriptions, those -1 are independent of interlaced or
non-interlaced. I believe there was some confusion with these
registers because intel_reg_dumper already re-adds +1 when showing the
register values.

Btw, git clone on fd.o is slow today :(

-- 
Paulo Zanoni

[-- Attachment #2: dump-snb-interlaced.txt --]
[-- Type: text/plain, Size: 11663 bytes --]

                    PGETBL_CTL: 0x00000000
               GEN6_INSTDONE_1: 0xfffffffe
               GEN6_INSTDONE_2: 0xffffffff
                  CPU_VGACNTRL: 0x80000000 (disabled)
    DIGITAL_PORT_HOTPLUG_CNTRL: 0x00000000
                     RR_HW_CTL: 0x00000000 (low 0, high 0)
                FDI_PLL_BIOS_0: 0xffffffff
                FDI_PLL_BIOS_1: 0xffffffff
                FDI_PLL_BIOS_2: 0xffffffff
       DISPLAY_PORT_PLL_BIOS_0: 0xffffffff
       DISPLAY_PORT_PLL_BIOS_1: 0xffffffff
       DISPLAY_PORT_PLL_BIOS_2: 0xffffffff
              FDI_PLL_FREQ_CTL: 0xffffffff
                     PIPEACONF: 0xc0000050 (enabled, rotate 0, active, 6bpc)
                      HTOTAL_A: 0x062d0555 (1366 active, 1582 total)
                      HBLANK_A: 0x062d0555 (1366 start, 1582 end)
                       HSYNC_A: 0x05c50585 (1414 start, 1478 end)
                      VTOTAL_A: 0x031702ff (768 active, 792 total)
                      VBLANK_A: 0x031702ff (768 start, 792 end)
                       VSYNC_A: 0x030a0303 (772 start, 779 end)
                  VSYNCSHIFT_A: 0x00000000
                      PIPEASRC: 0x055502ff (1366, 768)
                 PIPEA_DATA_M1: 0x7e14a780 (TU 64, val 0x14a780 1353600)
                 PIPEA_DATA_N1: 0x0020f580 (val 0x20f580 2160000)
                 PIPEA_DATA_M2: 0x00000000 (TU 1, val 0x0 0)
                 PIPEA_DATA_N2: 0x00000000 (val 0x0 0)
                 PIPEA_LINK_M1: 0x000125c0 (val 0x125c0 75200)
                 PIPEA_LINK_N1: 0x00041eb0 (val 0x41eb0 270000)
                 PIPEA_LINK_M2: 0x00000000 (val 0x0 0)
                 PIPEA_LINK_N2: 0x00000000 (val 0x0 0)
                      DSPACNTR: 0xd8004400 (enabled)
                      DSPABASE: 0x00000000
                    DSPASTRIDE: 0x00003400 (208)
                      DSPASURF: 0x059d6000
                   DSPATILEOFF: 0x00000000 (0, 0)
                     PIPEBCONF: 0xc0600000 (enabled, rotate 0, active, 8bpc)
                      HTOTAL_B: 0x0a4f077f (1920 active, 2640 total)
                      HBLANK_B: 0x0a4f077f (1920 start, 2640 end)
                       HSYNC_B: 0x09bb098f (2448 start, 2492 end)
                      VTOTAL_B: 0x04620435 (1078 active, 1123 total)
                      VBLANK_B: 0x04620435 (1078 start, 1123 end)
                       VSYNC_B: 0x04430439 (1082 start, 1092 end)
                  VSYNCSHIFT_B: 0x00000000
                      DSPBCNTR: 0xd8004400 (enabled)
                      DSPBBASE: 0x00001558
                    DSPBSTRIDE: 0x00003400 (208)
                      DSPBSURF: 0x059d6000
                   DSPBTILEOFF: 0x00000556 (1366, 0)
                      PIPEBSRC: 0x077f0437 (1920, 1080)
                 PIPEB_DATA_M1: 0x7e1b30f0 (TU 64, val 0x1b30f0 1782000)
                 PIPEB_DATA_N1: 0x0020f580 (val 0x20f580 2160000)
                 PIPEB_DATA_M2: 0x00000000 (TU 1, val 0x0 0)
                 PIPEB_DATA_N2: 0x00000000 (val 0x0 0)
                 PIPEB_LINK_M1: 0x0001220a (val 0x1220a 74250)
                 PIPEB_LINK_N1: 0x00041eb0 (val 0x41eb0 270000)
                 PIPEB_LINK_M2: 0x00000000 (val 0x0 0)
                 PIPEB_LINK_N2: 0x00000000 (val 0x0 0)
                     PFA_CTL_1: 0x00000000 (disable, auto_scale yes, auto_scale_cal no, v_filter enable, vadapt disable, mode least, filter_sel programmed,chroma pre-filter disable, vert3tap auto, v_inter_invert field 1)
                     PFA_CTL_2: 0x00007e80 (vscale 0.988281)
                     PFA_CTL_3: 0x00003f40 (vscale initial phase 0.494141)
                     PFA_CTL_4: 0x00007d54 (hscale 0.979126)
                   PFA_WIN_POS: 0x00000000 (0, 0)
                  PFA_WIN_SIZE: 0x00000000 (0, 0)
                     PFB_CTL_1: 0x00000000 (disable, auto_scale yes, auto_scale_cal no, v_filter enable, vadapt disable, mode least, filter_sel programmed,chroma pre-filter disable, vert3tap auto, v_inter_invert field 1)
                     PFB_CTL_2: 0x00007ef2 (vscale 0.991760)
                     PFB_CTL_3: 0x00003f79 (vscale initial phase 0.495880)
                     PFB_CTL_4: 0x00007c40 (hscale 0.970703)
                   PFB_WIN_POS: 0x00000000 (0, 0)
                  PFB_WIN_SIZE: 0x00000000 (0, 0)
              PCH_DREF_CONTROL: 0x00001400 (cpu source disable, ssc_source enable, nonspread_source enable, superspread_source disable, ssc4_mode downspread, ssc1 disable, ssc4 disable)
               PCH_RAWCLK_FREQ: 0x0000007d (FDL_TP1 timer 0.5us, FDL_TP2 timer 1.5us, freq 125)
              PCH_DPLL_TMR_CFG: 0x0271186a
                PCH_SSC4_PARMS: 0x01204860
            PCH_SSC4_AUX_PARMS: 0x000029c5
                  PCH_DPLL_SEL: 0x00000098 (FDL_TP1 timer 0.5us, FDL_TP2 timer 1.5us, freq 125)
           PCH_DPLL_ANALOG_CTL: 0x00008000
                    PCH_DPLL_A: 0x88040004 (enable, sdvo high speed no, mode LVDS, p2 Div 14, FPA0 P1 3, FPA1 P1 3, refclk default 120Mhz, sdvo/hdmi mul 1)
                    PCH_DPLL_B: 0xc4080008 (enable, sdvo high speed yes, mode (null), p2 (null), FPA0 P1 4, FPA1 P1 4, refclk default 120Mhz, sdvo/hdmi mul 1)
                      PCH_FPA0: 0x00010c07 (n = 1, m1 = 12, m2 = 7)
                      PCH_FPA1: 0x00010c07 (n = 1, m1 = 12, m2 = 7)
                      PCH_FPB0: 0x00021007 (n = 2, m1 = 16, m2 = 7)
                      PCH_FPB1: 0x00021007 (n = 2, m1 = 16, m2 = 7)
                TRANS_HTOTAL_A: 0x062d0555 (1366 active, 1582 total)
                TRANS_HBLANK_A: 0x062d0555 (1366 start, 1582 end)
                 TRANS_HSYNC_A: 0x05c50585 (1414 start, 1478 end)
                TRANS_VTOTAL_A: 0x031702ff (768 active, 792 total)
                TRANS_VBLANK_A: 0x031702ff (768 start, 792 end)
                 TRANS_VSYNC_A: 0x030a0303 (772 start, 779 end)
                TRANSA_DATA_M1: 0x00000000 (TU 1, val 0x0 0)
                TRANSA_DATA_N1: 0x00000000 (val 0x0 0)
                TRANSA_DATA_M2: 0x00000000 (TU 1, val 0x0 0)
                TRANSA_DATA_N2: 0x00000000 (val 0x0 0)
             TRANSA_DP_LINK_M1: 0x00000000 (val 0x0 0)
             TRANSA_DP_LINK_N1: 0x00000000 (val 0x0 0)
             TRANSA_DP_LINK_M2: 0x00000000 (val 0x0 0)
             TRANSA_DP_LINK_N2: 0x00000000 (val 0x0 0)
                TRANS_HTOTAL_B: 0x0a4f077f (1920 active, 2640 total)
                TRANS_HBLANK_B: 0x0a4f077f (1920 start, 2640 end)
                 TRANS_HSYNC_B: 0x09bb098f (2448 start, 2492 end)
                TRANS_VTOTAL_B: 0x04620435 (1078 active, 1123 total)
                TRANS_VBLANK_B: 0x04620435 (1078 start, 1123 end)
                 TRANS_VSYNC_B: 0x04430439 (1082 start, 1092 end)
                TRANSB_DATA_M1: 0x00000000 (TU 1, val 0x0 0)
                TRANSB_DATA_N1: 0x00000000 (val 0x0 0)
                TRANSB_DATA_M2: 0x00000000 (TU 1, val 0x0 0)
                TRANSB_DATA_N2: 0x00000000 (val 0x0 0)
             TRANSB_DP_LINK_M1: 0x00000000 (val 0x0 0)
             TRANSB_DP_LINK_N1: 0x00000000 (val 0x0 0)
             TRANSB_DP_LINK_M2: 0x00000000 (val 0x0 0)
             TRANSB_DP_LINK_N2: 0x00000000 (val 0x0 0)
                TRANS_HTOTAL_C: 0x00000000 (1 active, 1 total)
                TRANS_HBLANK_C: 0x00000000 (1 start, 1 end)
                 TRANS_HSYNC_C: 0x00000000 (1 start, 1 end)
                TRANS_VTOTAL_C: 0x00000000 (1 active, 1 total)
                TRANS_VBLANK_C: 0x00000000 (1 start, 1 end)
                 TRANS_VSYNC_C: 0x00000000 (1 start, 1 end)
                TRANSC_DATA_M1: 0x00000000 (TU 1, val 0x0 0)
                TRANSC_DATA_N1: 0x00000000 (val 0x0 0)
                TRANSC_DATA_M2: 0x00000000 (TU 1, val 0x0 0)
                TRANSC_DATA_N2: 0x00000000 (val 0x0 0)
             TRANSC_DP_LINK_M1: 0x00000000 (val 0x0 0)
             TRANSC_DP_LINK_N1: 0x00000000 (val 0x0 0)
             TRANSC_DP_LINK_M2: 0x00000000 (val 0x0 0)
             TRANSC_DP_LINK_N2: 0x00000000 (val 0x0 0)
                    TRANSACONF: 0xc0000000 (enable, active)
                    TRANSBCONF: 0xc0000000 (enable, active)
                    TRANSCCONF: 0x00000000 (disable, inactive)
                   FDI_TXA_CTL: 0xb0044000 (enable, train pattern not train, voltage swing 0.4V,pre-emphasis none, port width X1, enhanced framing enable, FDI PLL enable, scrambing enable, master mode disable)
                   FDI_TXB_CTL: 0xb0044000 (enable, train pattern not train, voltage swing 0.4V,pre-emphasis none, port width X1, enhanced framing enable, FDI PLL enable, scrambing enable, master mode disable)
                   FDI_TXC_CTL: 0x00000000 (disable, train pattern pattern_1, voltage swing 0.4V,pre-emphasis none, port width X1, enhanced framing disable, FDI PLL disable, scrambing enable, master mode disable)
                   FDI_RXA_CTL: 0x80022350 (enable, train pattern pattern_1, port width X1, 6bpc,link_reverse_strap_overwrite no, dmi_link_reverse no, FDI PLL enable,FS ecc disable, FE ecc disable, FS err report enable, FE err report enable,scrambing enable, enhanced framing enable, PCDClk)
                   FDI_RXB_CTL: 0x80002350 (enable, train pattern pattern_1, port width X1, 8bpc,link_reverse_strap_overwrite no, dmi_link_reverse no, FDI PLL enable,FS ecc disable, FE ecc disable, FS err report enable, FE err report enable,scrambing enable, enhanced framing enable, PCDClk)
                   FDI_RXC_CTL: 0x00000040 (disable, train pattern pattern_1, port width X1, 8bpc,link_reverse_strap_overwrite no, dmi_link_reverse no, FDI PLL disable,FS ecc disable, FE ecc disable, FS err report disable, FE err report disable,scrambing enable, enhanced framing enable, RawClk)
                  FDI_RXA_MISC: 0x00000080 (FDI Delay 128)
                  FDI_RXB_MISC: 0x00000080 (FDI Delay 128)
                  FDI_RXC_MISC: 0x00000080 (FDI Delay 128)
               FDI_RXA_TUSIZE1: 0x7e000000
               FDI_RXA_TUSIZE2: 0x7e000000
               FDI_RXB_TUSIZE1: 0x7e000000
               FDI_RXB_TUSIZE2: 0x7e000000
               FDI_RXC_TUSIZE1: 0x7e000000
               FDI_RXC_TUSIZE2: 0x7e000000
                 FDI_PLL_CTL_1: 0x7e000000
                 FDI_PLL_CTL_2: 0x7e000000
                   FDI_RXA_IIR: 0x00000000
                   FDI_RXA_IMR: 0x000008ff
                   FDI_RXB_IIR: 0x00000000
                   FDI_RXB_IMR: 0x000008ff
                      PCH_ADPA: 0x00f40000 (disabled, transcoder A, -hsync, -vsync)
                         HDMIB: 0xa0000adc (enabled pipe A 8bpc TMDS HDMI audio enabled +vsync +hsync detected)
                         HDMIC: 0x0000001c (disabled pipe A 8bpc SDVO DVI audio disabled +vsync +hsync detected)
                         HDMID: 0x0000001c (disabled pipe A 8bpc SDVO DVI audio disabled +vsync +hsync detected)
                      PCH_LVDS: 0x80200302 (enabled, pipe A, 18 bit, 1 channel)
                     CPU_eDP_A: 0x00000018
                      PCH_DP_B: 0x00000004
                      PCH_DP_C: 0x00000004
                      PCH_DP_D: 0x00000004
                TRANS_DP_CTL_A: 0x60000418 (enabled, pipe A, 18 bit, 1 channel)
                TRANS_DP_CTL_B: 0x60000018 (enabled, pipe A, 18 bit, 1 channel)
                TRANS_DP_CTL_C: 0x60000018 (enabled, pipe A, 18 bit, 1 channel)
              BLC_PWM_CPU_CTL2: 0x80000000
               BLC_PWM_CPU_CTL: 0x00001228
              BLC_PWM_PCH_CTL1: 0x80000000
              BLC_PWM_PCH_CTL2: 0x12281228
                 PCH_PP_STATUS: 0xc0000008 (on, ready, sequencing idle)
                PCH_PP_CONTROL: 0xabcd0003 (blacklight disabled, power down on reset, panel on)
              PCH_PP_ON_DELAYS: 0x012c0bb8
             PCH_PP_OFF_DELAYS: 0x012c07d0
                PCH_PP_DIVISOR: 0x00186906

[-- Attachment #3: 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/4] [CFT] interlaced support

[Daniel Vetter]

On Fri, Jan 27, 2012 at 12:56:23AM -0200, Paulo Zanoni wrote:
> Hi
> 
> 2012/1/26 Daniel Vetter <daniel.vetter@ffwll.ch>:
> > Hi all,
> >
> > http://cgit.freedesktop.org/~danvet/drm/log/?h=interlaced
> >
> 
> I just tested your patch set and it didn't work: my monitor reported
> 1920x539@50Hz. We're missing something from patch "fixup interlace
> vertical timings confusion". The reg dump is attached.
> 
> By the way, VTOTAL_B was 1078. We need to remove those "-1" and also
> those "-2". Look at ironlake_crtc_mode_set: we already do the
> subtractions when we write to HSYNC, HTOTAL, etc. From the register
> descriptions, those -1 are independent of interlaced or
> non-interlaced. I believe there was some confusion with these
> registers because intel_reg_dumper already re-adds +1 when showing the
> register values.

Ok, so something is still botched up :( Btw I think VSYNCSHIFT is only
used by analog outputs to place the 2nd field lines exactly between the
lines of the 1st field. Can you also please attach a reg dump with Peter's
patch so I can compare?

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

Re: [PATCH 0/4] [CFT] interlaced support

[Paulo Zanoni]

Ok, so I had to look at the Windows registers again...

Your "interlaced" branch had 6 patches. Patch 3 is the only one I
changed. Here is my version:
http://people.freedesktop.org/~pzanoni/0003-drm-i915-fixup-interlace-vertical-timings-confusion.patch.
Now my monitor reports "1080i" :)

Some notes about the patch:

- I really like your solution of not using CRTC_INTERLACE_HALVE_V,
because previously we were dividing by 2 and then multiplying by 2
again, which would not restore the original values in case of odd
numbers.

- I'm not sure about the change inside drm_modes.c. Why do we need the
"|= 1"? I fear it could cause us some troubles. This change might also
break the other drivers, of course. Wel'll need a "v4".

- It seems we need vtotal-- and vblank_end--. We need to find evidence
for that in the documentation. I'm just copying Windows

- I tested only on SNB. This time the screen looks good (previous
versions were really ugly). I'll test ilk and report if something goes
wrong...

Cheers,
Paulo

-- 
Paulo Zanoni

Re: [PATCH 0/4] [CFT] interlaced support

[Daniel Vetter]

On Fri, Jan 27, 2012 at 12:34:41AM +0100, Alfonso Fiore wrote:
> On Thu, Jan 26, 2012 at 11:22 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > git://people.freedesktop.org/~danvet/drm interlaced
> 
> Thank you Daniel,
> 
> is it normal that is super-slow? git repors 1K/sec. I download about
> 10 MB so far...

You don't need to download an entire git tree again, you can also add a
new remote to an existing git checkout of the linux kernel:

$ git remote add <remote-name> <git-url>
$ git fetch <remote-name>
$ git checkout <remote-name>/<branch-name>

You then have 2 different upstreams/remote repositories: One is "origin"
which points at the git repo of the original clone, the other is the new
one.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 0/4] [CFT] interlaced support

[Alfonso Fiore]

On Fri, Jan 27, 2012 at 6:02 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Jan 27, 2012 at 12:34:41AM +0100, Alfonso Fiore wrote:
>> On Thu, Jan 26, 2012 at 11:22 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> >
>> > git://people.freedesktop.org/~danvet/drm interlaced
>>
>> Thank you Daniel,
>>
>> is it normal that is super-slow? git repors 1K/sec. I download about
>> 10 MB so far...
>
> You don't need to download an entire git tree again, you can also add a
> new remote to an existing git checkout of the linux kernel:
>
> $ git remote add <remote-name> <git-url>
> $ git fetch <remote-name>
> $ git checkout <remote-name>/<branch-name>
>
> You then have 2 different upstreams/remote repositories: One is "origin"
> which points at the git repo of the original clone, the other is the new
> one.

I'll try for sure since after 20 hours I'm still at 4%...

Re: [PATCH 0/4] [CFT] interlaced support

[Daniel Vetter]

On Fri, Jan 27, 2012 at 02:25:49PM -0200, Paulo Zanoni wrote:
> Ok, so I had to look at the Windows registers again...
> 
> Your "interlaced" branch had 6 patches. Patch 3 is the only one I
> changed. Here is my version:
> http://people.freedesktop.org/~pzanoni/0003-drm-i915-fixup-interlace-vertical-timings-confusion.patch.
> Now my monitor reports "1080i" :)
> 
> Some notes about the patch:
> 
> - I really like your solution of not using CRTC_INTERLACE_HALVE_V,
> because previously we were dividing by 2 and then multiplying by 2
> again, which would not restore the original values in case of odd
> numbers.
> 
> - I'm not sure about the change inside drm_modes.c. Why do we need the
> "|= 1"? I fear it could cause us some troubles. This change might also
> break the other drivers, of course. Wel'll need a "v4".
> 
> - It seems we need vtotal-- and vblank_end--. We need to find evidence
> for that in the documentation. I'm just copying Windows
> 
> - I tested only on SNB. This time the screen looks good (previous
> versions were really ugly). I'll test ilk and report if something goes
> wrong...

I've updated my interlaced branch. Changes:
- Adjust vertical timings as Paulo discovered. I've dropped the change in
  drm_mode.c and instead substract one more from vtotal. I hope that this
  results in the same configuration.
- Added a paranoia patch to not set unsupported bits on gen2.
- Addes support for vsyncshift. The hw seems to use that to correctly
  insert the half-line delay when switching between even and odd fields.
  That might also explain why we have to substract a little bit from
  vtotal and vblank_end.

Testing feedback highly welcome. And if you can, please compare you
register-settings with those from windows/bios. I'll also add the
VSYNCSHIFT registers to intel_reg_dumper.

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

Re: [PATCH 1/4] drm/i915: fixup interlaced vertical timings confusion

[Daniel Vetter]

On Thu, Jan 26, 2012 at 11:11:30PM +0100, Daniel Vetter wrote:
> On Thu, Jan 26, 2012 at 10:03:02PM +0000, Chris Wilson wrote:
> > On Thu, 26 Jan 2012 22:01:30 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > -	/* XXX some encoders set the crtcinfo, others don't.
> > > -	 * Obviously we need some form of conflict resolution here...
> > > -	 */
> > > -	if (adjusted_mode->crtc_htotal == 0)
> > > +	/* gen2 needs vertical crtc timing information in fields because that's
> > > +	 * what dvo outputs want - the chip itself can't do interlaced. All
> > > +	 * later generations can do interlaced natively and want timings in
> > > +	 * full frames. */
> > > +	if (IS_GEN2(dev))
> > > +		drm_mode_set_crtcinfo(adjusted_mode, CRTC_INTERLACE_HALVE_V);
> > > +	else
> > >  		drm_mode_set_crtcinfo(adjusted_mode, 0);
> > >  
> > > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
> > > index 6eda1b5..020a7d7 100644
> > > --- a/drivers/gpu/drm/i915/intel_dvo.c
> > > +++ b/drivers/gpu/drm/i915/intel_dvo.c
> > > @@ -157,7 +157,6 @@ static bool intel_dvo_mode_fixup(struct drm_encoder *encoder,
> > >  		C(vsync_end);
> > >  		C(vtotal);
> > >  		C(clock);
> > > -		drm_mode_set_crtcinfo(adjusted_mode, CRTC_INTERLACE_HALVE_V);
> > >  #undef C
> > >  	}
> > 
> > Removing drm_mode_set_crtcinfo() scares me because of the above comment.
> > We need to make sure that the adjusted_mode is initialised along
> > some path, and the fixup in intel_crtc_mode_fixup is just a hack.
> > 
> > commit 897493504addc5609f04a2c4f73c37ab972c29b2
> > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > Date:   Sun Sep 12 18:25:19 2010 +0100
> > 
> >     drm/i915: Ensure that the crtcinfo is populated during mode_fixup()
> >     
> >     This should fix the mysterious mode setting failures reported during
> >     boot up and after resume, generally for i8xx class machines.
> >     
> >     Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=16478
> >     Reported-and-tested-by: Xavier Chantry <chantry.xavier@gmail.com>
> >     Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=29413
> >     Tested-by: Daniel Vetter <daniel@ffwll.ch>
> >     Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >     Cc: stable@kernel.org
> > 
> > If you can work out exactly where it should be initialised, you'll be my
> > hero!
> 
> Afaik drm core calling set_crtcinfo is just a side-effect of some random
> actions and it's not guaranteed to happen. The issue is that drm core puts
> the wrong stuff in there. Also note that I'm now unconditionally doing the
> fixup, so the particular bug above shouldn't show up again.
> 
> I'll check whether this is indeed the case on my dear old i855gm - let's
> see whether I'm provoking the wrath of some ill-tempered gods here ;-)

Seems the volcano gods haven't noticed yet what I'm doing here, i.e. works
on my i855gm.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 0/4] [CFT] interlaced support

[Daniel Vetter]

On Fri, Jan 27, 2012 at 06:43:06PM +0100, Daniel Vetter wrote:
> On Fri, Jan 27, 2012 at 02:25:49PM -0200, Paulo Zanoni wrote:
> > Ok, so I had to look at the Windows registers again...
> > 
> > Your "interlaced" branch had 6 patches. Patch 3 is the only one I
> > changed. Here is my version:
> > http://people.freedesktop.org/~pzanoni/0003-drm-i915-fixup-interlace-vertical-timings-confusion.patch.
> > Now my monitor reports "1080i" :)
> > 
> > Some notes about the patch:
> > 
> > - I really like your solution of not using CRTC_INTERLACE_HALVE_V,
> > because previously we were dividing by 2 and then multiplying by 2
> > again, which would not restore the original values in case of odd
> > numbers.
> > 
> > - I'm not sure about the change inside drm_modes.c. Why do we need the
> > "|= 1"? I fear it could cause us some troubles. This change might also
> > break the other drivers, of course. Wel'll need a "v4".
> > 
> > - It seems we need vtotal-- and vblank_end--. We need to find evidence
> > for that in the documentation. I'm just copying Windows
> > 
> > - I tested only on SNB. This time the screen looks good (previous
> > versions were really ugly). I'll test ilk and report if something goes
> > wrong...
> 
> I've updated my interlaced branch. Changes:
> - Adjust vertical timings as Paulo discovered. I've dropped the change in
>   drm_mode.c and instead substract one more from vtotal. I hope that this
>   results in the same configuration.
> - Added a paranoia patch to not set unsupported bits on gen2.
> - Addes support for vsyncshift. The hw seems to use that to correctly
>   insert the half-line delay when switching between even and odd fields.
>   That might also explain why we have to substract a little bit from
>   vtotal and vblank_end.
> 
> Testing feedback highly welcome. And if you can, please compare you
> register-settings with those from windows/bios. I'll also add the
> VSYNCSHIFT registers to intel_reg_dumper.

Paulo and me have crawled around in interlaced-land some more, and I think
we now have reasonable explanations plus patches for all the differences
between how windows sets things up on his machine and the current code.
I've updated my interlaced branch at:

http://cgit.freedesktop.org/~danvet/drm/log/?h=interlaced

Testing highly appreciated. Especially on analog TV-outputs - Rodrigo, can
you test this with your machines, please?

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

Re: [PATCH 0/4] [CFT] interlaced support

[Alfonso Fiore]

On Fri, Jan 27, 2012 at 10:41 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> Paulo and me have crawled around in interlaced-land some more

I just came home ready to test my compiled kernel but when I read this
message I thought I should download your latest source files and
recompile. It was not so simple but I think I managed (by the way,
what's the simple command to update my local copy?).

while my new kernel compiles: is there a simple way to check I'm
testing your latest changes and not the previous version?

thanks,
alfonso

Re: [PATCH 0/4] [CFT] interlaced support

[Peter Ross]

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

On Fri, Jan 27, 2012 at 06:43:06PM +0100, Daniel Vetter wrote:
> On Fri, Jan 27, 2012 at 02:25:49PM -0200, Paulo Zanoni wrote:
> > Ok, so I had to look at the Windows registers again...
> > 
> > Your "interlaced" branch had 6 patches. Patch 3 is the only one I
> > changed. Here is my version:
> > http://people.freedesktop.org/~pzanoni/0003-drm-i915-fixup-interlace-vertical-timings-confusion.patch.
> > Now my monitor reports "1080i" :)
> > 
> > Some notes about the patch:
> > 
> > - I really like your solution of not using CRTC_INTERLACE_HALVE_V,
> > because previously we were dividing by 2 and then multiplying by 2
> > again, which would not restore the original values in case of odd
> > numbers.
> > 
> > - I'm not sure about the change inside drm_modes.c. Why do we need the
> > "|= 1"? I fear it could cause us some troubles. This change might also
> > break the other drivers, of course. Wel'll need a "v4".
> > 
> > - It seems we need vtotal-- and vblank_end--. We need to find evidence
> > for that in the documentation. I'm just copying Windows
> > 
> > - I tested only on SNB. This time the screen looks good (previous
> > versions were really ugly). I'll test ilk and report if something goes
> > wrong...
> 
> I've updated my interlaced branch. Changes:
> - Adjust vertical timings as Paulo discovered. I've dropped the change in
>   drm_mode.c and instead substract one more from vtotal. I hope that this
>   results in the same configuration.
> - Added a paranoia patch to not set unsupported bits on gen2.
> - Addes support for vsyncshift. The hw seems to use that to correctly
>   insert the half-line delay when switching between even and odd fields.
>   That might also explain why we have to substract a little bit from
>   vtotal and vblank_end.
> 
> Testing feedback highly welcome. And if you can, please compare you
> register-settings with those from windows/bios. I'll also add the
> VSYNCSHIFT registers to intel_reg_dumper.

Tested on G35_G (SDVO-HDMI and VGA).                                                                 
The vertical timing registers set by linux match those set when using
the Windows intel display driver.

Great job guys. 

-- 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/4] [CFT] interlaced support

[Alfonso Fiore]

[-- Attachment #1: Type: text/plain, Size: 764 bytes --]

On Sat, Jan 28, 2012 at 11:21 AM, Peter Ross <pross@xvid.org> wrote:
>
>
> Great job guys.

I can only quote. :)
Thank you SO MUCH Daniel, Peter, Paulo and Angela.

I attached logs in case you're interested.

I didn't test VGA (since it was already working at the correct rate).
I tested HDMI with only HDMI cable connected and it works great both
1920x1080i, 1280x720p, 1280x720p and 800x600p. No overscan at all.

Just for the sake of reporting 1440x576 and 1440x480 have a bit of
overscan but I'm not so sure what's the point of a 2.5:1 and 3:1
aspect ration anyway.

I left dmesg only in one folder to make the tgz smaller.

Let me know if you need any other testing, but I consider the matter
close from my point of view.

Again, thank you!

regards,
alfonso

[-- Attachment #2: interlaced_works.tgz --]
[-- Type: application/x-gzip, Size: 240072 bytes --]

[-- Attachment #3: 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/4] [CFT] interlaced support

[Daniel Vetter]

On Sat, Jan 28, 2012 at 02:52:21AM +0100, Alfonso Fiore wrote:
> On Fri, Jan 27, 2012 at 10:41 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > Paulo and me have crawled around in interlaced-land some more
> 
> I just came home ready to test my compiled kernel but when I read this
> message I thought I should download your latest source files and
> recompile. It was not so simple but I think I managed (by the way,
> what's the simple command to update my local copy?).

$ git fetch <remote-name>

should sync up your local git repo with that remote, then update your
working copy with

$ git checkout <remote-name>/<branch-name>

like when checking out for the first time.

> while my new kernel compiles: is there a simple way to check I'm
> testing your latest changes and not the previous version?

CONFIG_LOCALVERSION_AUTO appends the git sha1 to your uname -r string.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 0/4] [CFT] interlaced support

[Daniel Vetter]

On Sat, Jan 28, 2012 at 11:46:10AM +0100, Alfonso Fiore wrote:
> On Sat, Jan 28, 2012 at 11:21 AM, Peter Ross <pross@xvid.org> wrote:
> > Great job guys.
> 
> I can only quote. :)
> Thank you SO MUCH Daniel, Peter, Paulo and Angela.

Well, thanks for getting the ball rolling and for all the testing. I've
just sent out my latest patches and updated my interlaced branch. Please
retest and reply to "interlaced patches v2" with your tested-by.

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