From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/4] drm/i915: fixup interlaced vertical timings confusion Date: Fri, 27 Jan 2012 20:08:42 +0100 Message-ID: <20120127190842.GF3901@phenom.ffwll.local> References: <1327611693-27294-1-git-send-email-daniel.vetter@ffwll.ch> <1327611693-27294-2-git-send-email-daniel.vetter@ffwll.ch> <20120126221130.GB30536@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ww0-f43.google.com (mail-ww0-f43.google.com [74.125.82.43]) by gabe.freedesktop.org (Postfix) with ESMTP id 77A5C9E81E for ; Fri, 27 Jan 2012 11:08:42 -0800 (PST) Received: by wgbdt11 with SMTP id dt11so1811701wgb.12 for ; Fri, 27 Jan 2012 11:08:41 -0800 (PST) Content-Disposition: inline In-Reply-To: <20120126221130.GB30536@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson Cc: Daniel Vetter , Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org 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 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 > > 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 > > Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=29413 > > Tested-by: Daniel Vetter > > Signed-off-by: Chris Wilson > > 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