Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754032AbaGGPQM (ORCPT ); Mon, 7 Jul 2014 11:16:12 -0400 Received: from mail-wi0-f176.google.com ([209.85.212.176]:52266 "EHLO mail-wi0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753505AbaGGPQJ (ORCPT ); Mon, 7 Jul 2014 11:16:09 -0400 Date: Mon, 7 Jul 2014 17:16:13 +0200 From: Daniel Vetter To: Thomas Meyer Cc: Chris Wilson , jikos@suse.cz, "intel-gfx@lists.freedesktop.org" , Linux Kernel Mailing List , Pavel Machek , Linus Torvalds Subject: Re: [Intel-gfx] Linux 3.16-rc2 Message-ID: <20140707151613.GG5821@phenom.ffwll.local> Mail-Followup-To: Thomas Meyer , Chris Wilson , jikos@suse.cz, "intel-gfx@lists.freedesktop.org" , Linux Kernel Mailing List , Pavel Machek , Linus Torvalds References: <1403564870.3091.6.camel@localhost.localdomain> <871tuea7nz.fsf@intel.com> <20140624115753.GD17674@nuc-i3427.alporthouse.com> <1403612670.3091.10.camel@localhost.localdomain> <20140624122737.GF17674@nuc-i3427.alporthouse.com> <20140630100220.GA4934@xo-6d-61-c0.localdomain> <20140630100925.GA18836@nuc-i3427.alporthouse.com> <1404317921.3910.3.camel@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1404317921.3910.3.camel@localhost.localdomain> X-Operating-System: Linux phenom 3.15.0-rc3+ User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jul 02, 2014 at 06:18:41PM +0200, Thomas Meyer wrote: > Am Montag, den 30.06.2014, 11:09 +0100 schrieb Chris Wilson: > > On Mon, Jun 30, 2014 at 12:02:20PM +0200, Pavel Machek wrote: > > > On Tue 2014-06-24 13:27:37, Chris Wilson wrote: > > > > On Tue, Jun 24, 2014 at 02:24:30PM +0200, Thomas Meyer wrote: > > > > > Am Dienstag, den 24.06.2014, 12:57 +0100 schrieb Chris Wilson: > > > > > > On Tue, Jun 24, 2014 at 02:06:24PM +0300, Jani Nikula wrote: > > > > > > > On Tue, 24 Jun 2014, Thomas Meyer wrote: > > > > > > > > the i915 driver is still broken in 3.16-rc2. Resume from ram crashes the > > > > > > > > X server. > > > > > > > > > > > > > > This is not new to 3.16-rc2; apparently we've had it since v3.15-rc4 > > > > > > > [1]. Also related [2]. > > > > > > > > > > > > > > Chris, any fresh ideas? > > > > > > > > > > > > Nope. The bug is https://bugs.freedesktop.org/show_bug.cgi?id=76554 > > > > > > everything we know and have tried is there. Which is not much more than > > > > > > at time of the original incarnation: > > > > > > > > > > > > commit 50aa253d820ad4577e2231202f2c8fd89f9dc4e6 > > > > > > Author: Keith Packard > > > > > > Date: Tue Oct 14 17:20:35 2008 -0700 > > > > > > > > > > > > i915: Fix up ring initialization to cover G45 oddities > > > > > > > > > > > > G45 appears quite sensitive to ring initialization register writes, > > > > > > sometimes leaving the HEAD register with the START register contents. Check > > > > > > to make sure HEAD is reset correctly when START is written, and fix it up, > > > > > > screaming loudly. > > > > > > -Chris > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > so why not revert 78f2975eec9faff353a6194e854d3d39907bab68 (drm/i915: > > > > > Move all ring resets before setting the HWS page) ? > > > > > > > > Because that patch was in response to a boot time regression. > > > > > > It seems we are in a fairly ugly "fix one board, it breaks another" iterations, right? > > > > > > (BTW, if you apply patch to fix this bug, could you Cc me? I have strange feeling > > > that it will break my setup... Actually, it probably makes sense to Cc all the people > > > who reported problems with ring initialization... > > > > > > What patch caused the boot time regression you are talking about? We need to get > > > list of commits involved in this, and revert the original one... > > > > commit 9991ae787a0c87fe7c783b4b6f4754c3cdbb6213 > > Author: Chris Wilson > > Date: Wed Apr 2 16:36:07 2014 +0100 > > > > drm/i915: Move all ring resets before setting the HWS page > > > > In commit a51435a3137ad8ae75c288c39bd2d8b2696bae8f > > Author: Naresh Kumar Kachhi > > Date: Wed Mar 12 16:39:40 2014 +0530 > > > > drm/i915: disable rings before HW status page setup > > > > we reordered stopping the rings to do so before we set the HWS register. > > However, there is an extra workaround for g45 to reset the rings twice, > > and for consistency we should apply that workaround before setting the > > HWS to be sure that the rings are truly stopped. > > > > Cc: Naresh Kumar Kachhi > > Signed-off-by: Chris Wilson > > Reviewed-by: Jesse Barnes > > Signed-off-by: Daniel Vetter > > > > > > commit a51435a3137ad8ae75c288c39bd2d8b2696bae8f > > Author: Naresh Kumar Kachhi > > Date: Wed Mar 12 16:39:40 2014 +0530 > > > > drm/i915: disable rings before HW status page setup > > > > Rings should be idle before issuing sync_flush > > (in intel_ring_setup_status_page). This patch moves the ring > > disabling before doing the HW status page setup. > > > > Signed-off-by: Naresh Kumar Kachhi > > Reviewed-by: Chris Wilson > > Signed-off-by: Daniel Vetter > > > > > > Hi, > > this patch on top of v3.16-rc3-62-gd92a333 makes the resume from ram > regression go away on my machine: Hm, we could conditionalize this hack on IS_G4X ... Chris, thoughts? -Daniel > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 279488a..b896ac8 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -459,22 +459,25 @@ static bool stop_ring(struct intel_engine_cs *ring) > { > struct drm_i915_private *dev_priv = to_i915(ring->dev); > > - if (!IS_GEN2(ring->dev)) { > - I915_WRITE_MODE(ring, _MASKED_BIT_ENABLE(STOP_RING)); > - if (wait_for_atomic((I915_READ_MODE(ring) & MODE_IDLE) != 0, 1000)) { > - DRM_ERROR("%s :timed out trying to stop ring\n", ring->name); > - return false; > - } > - } > - > +// if (!IS_GEN2(ring->dev)) { > +// I915_WRITE_MODE(ring, _MASKED_BIT_ENABLE(STOP_RING)); > +// if (wait_for_atomic((I915_READ_MODE(ring) & MODE_IDLE) != 0, 1000)) { > +// DRM_ERROR("%s :timed out trying to stop ring1\n", ring->name); > +// return false; > +// } > +// } > + > + /* Stop the ring if it's running. */ > I915_WRITE_CTL(ring, 0); > I915_WRITE_HEAD(ring, 0); > ring->write_tail(ring, 0); > + if (wait_for_atomic((I915_READ_MODE(ring) & MODE_IDLE) != 0, 1000)) > + DRM_ERROR("%s :timed out trying to stop ring2\n", ring->name); > > - if (!IS_GEN2(ring->dev)) { > - (void)I915_READ_CTL(ring); > - I915_WRITE_MODE(ring, _MASKED_BIT_DISABLE(STOP_RING)); > - } > +// if (!IS_GEN2(ring->dev)) { > +// (void)I915_READ_CTL(ring); > +// I915_WRITE_MODE(ring, _MASKED_BIT_DISABLE(STOP_RING)); > +// } > > return (I915_READ_HEAD(ring) & HEAD_ADDR) == 0; > } > > Chris, any ideas why explicitly stopping the ring before reset, results > in this kind of misbehaviour on my machine on resume from ram? > > with kind regards > thomas > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/