At Mon, 27 Feb 2012 14:08:28 +0100, Takashi Iwai wrote: > > At Mon, 27 Feb 2012 12:17:57 +0000, > Chris Wilson wrote: > > > > On Mon, 27 Feb 2012 12:39:24 +0100, Takashi Iwai <tiwai at suse.de> wrote: > > > It seems that writing DSPSURF in intel_flush_display_plane() causes > > > the blank screen on some old laptops like Dell D630 with 965GM. > > > Since this operation is needed only for ILK+, make it conditional. > > > > The specs say that DSPASURF is the latch register for updates of the DSPA > > registers on gen4 (including 965gm) as well. Presumably the bug is that > > we only partially update the DSPA registers prior to the first call to > > intel_flush_display_plane() which this papers over by disabling the > > update until a valid address is written to DSPASURF. And there is such a > > spurious call to intel_enable_plane() prior to us setting a valid > > scanout surface: > > Sounds reasonable. FWIW, the change was first introduced in commit > [b24e7179: drm/i915: add pipe/plane enable/disable functions], > then in commit [efc2924e: drm/i915: Call intel_enable_plane from > i9xx_crtc_mode_set (again)], it's placed into i9xx_crtc_mode_set(). > > This explains the fact that it was discovered only on old machines > as i9xx_crtc_mode_set() is the only crtc_mode_set op calling > intel_enable_plane(). > > BTW, the bisection leaded to a merge commit, so the bug is really > depending on the activation path or timing. > > I'll ask a tester to try your patch. He reported back that it reduces the failure rate but doesn't fix completely. Still get a blank screen in 20% rate. Any other clue? thanks, Takashi > > > thanks, > > Takashi > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_d > > index 66b19d3..ea8e4a1 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -5453,7 +5453,6 @@ static int i9xx_crtc_mode_set(struct drm_crtc > > *crtc, > > > > I915_WRITE(DSPCNTR(plane), dspcntr); > > POSTING_READ(DSPCNTR(plane)); > > - intel_enable_plane(dev_priv, plane, pipe); > > > > ret = intel_pipe_set_base(crtc, x, y, old_fb); > > > > -Chris > > > > -- > > Chris Wilson, Intel Open Source Technology Centre > >
