From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 3/3] drm/i915: Only enable the plane after setting the fb base (pre-ILK) Date: Tue, 19 Apr 2011 13:39:49 -0700 Message-ID: <20110419133949.72c92e47@jbarnes-desktop> References: <1303245123-27172-1-git-send-email-chris@chris-wilson.co.uk> <1303245123-27172-4-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy2-pub.bluehost.com (oproxy2-pub.bluehost.com [67.222.39.60]) by gabe.freedesktop.org (Postfix) with SMTP id 7FCDF9E75B for ; Tue, 19 Apr 2011 13:39:53 -0700 (PDT) In-Reply-To: <1303245123-27172-4-git-send-email-chris@chris-wilson.co.uk> 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-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, 19 Apr 2011 21:32:03 +0100 Chris Wilson wrote: > When enabling the plane, it is helpful to have already pointed that > plane to valid memory or else we may incur the wrath of a PGTBL_ER. > This code preserved the behaviour from the bad old days for unknown > reasons... > > Found by assert_fb_bound_for_plane(). > > References: https://bugs.freedesktop.org/show_bug.cgi?id=36246 > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: Jesse Barnes > --- > drivers/gpu/drm/i915/intel_display.c | 2 -- > 1 files changed, 0 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index b9bb20d..f503ad0 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -5212,8 +5212,6 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc, > > I915_WRITE(DSPCNTR(plane), dspcntr); > POSTING_READ(DSPCNTR(plane)); > - if (!HAS_PCH_SPLIT(dev)) > - intel_enable_plane(dev_priv, plane, pipe); > > ret = intel_pipe_set_base(crtc, x, y, old_fb); > As usual, changes like this frighten me. I think the plane enable prior to the base set has been in there for a *long* time in various forms. I think Carl tried to take it out when trying to debug eDP on ILK, but we reverted that when it broke things. But if it works on all your test machines, I guess that's a start; it's correct in theory at least. Reviewed-by: Jesse Barnes -- Jesse Barnes, Intel Open Source Technology Center