[Intel-gfx] [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 at 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 at 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 at gmail.com>
>     Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=29413
>     Tested-by: Daniel Vetter <daniel at ffwll.ch>
>     Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>     Cc: stable at 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 at ffwll.ch
Mobile: +41 (0)79 365 57 48