From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f179.google.com ([209.85.212.179]:36277 "EHLO mail-wi0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751718AbbJVNPe (ORCPT ); Thu, 22 Oct 2015 09:15:34 -0400 Received: by wicfx6 with SMTP id fx6so135189450wic.1 for ; Thu, 22 Oct 2015 06:15:32 -0700 (PDT) Date: Thu, 22 Oct 2015 15:15:29 +0200 From: Daniel Vetter To: Maarten Lankhorst Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org Subject: Re: [Intel-gfx] [PATCH 10/11] drm/i915/skl: Update watermarks before the crtc is disabled. Message-ID: <20151022131529.GW16848@phenom.ffwll.local> References: <1445514996-18733-1-git-send-email-maarten.lankhorst@linux.intel.com> <1445514996-18733-11-git-send-email-maarten.lankhorst@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1445514996-18733-11-git-send-email-maarten.lankhorst@linux.intel.com> Sender: stable-owner@vger.kernel.org List-ID: On Thu, Oct 22, 2015 at 01:56:35PM +0200, Maarten Lankhorst wrote: > On skylake some of the registers are only writable when the correct > power wells are enabled. Because of this watermarks have to be updated > before the crtc turns off, or you get unclaimed register read and write > warnings. > > This patch needs to be modified slightly to apply to -fixes. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92181 > Signed-off-by: Maarten Lankhorst > Cc: stable@vger.kernel.org > Cc: Matt Roper > --- > drivers/gpu/drm/i915/intel_display.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 6f92ccf2461c..26931d8eb7ce 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4761,7 +4761,7 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state) > > crtc->wm.cxsr_allowed = true; > > - if (pipe_config->wm_changed) > + if (pipe_config->wm_changed && pipe_config->base.active) > intel_update_watermarks(&crtc->base); Calling post_plane_commit seems like a bug of the higher-level functions. If we can't fix that quickly I think we should have at least an early exit at the top, with a FIXME comment. If the platform/feature-specific commit hooks have to care about state->active, then the higher level functions imo haven't done their jobs properly. > > for_each_plane_in_state(old_state, plane, old_plane_state, i) { > @@ -13205,6 +13205,9 @@ static int intel_atomic_commit(struct drm_device *dev, > dev_priv->display.crtc_disable(crtc); > intel_crtc->active = false; > intel_disable_shared_dpll(intel_crtc); > + > + if (!crtc->state->active) > + intel_update_watermarks(crtc); Does this ever do anything at all? We just killed the crtc completely above, those watermark writes will get cleansed as soon as the power well goes down. -Daniel > } > } > > -- > 2.1.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch