From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 07/19] drm/i915: add runtime put/get calls at the basic places Date: Tue, 10 Dec 2013 22:49:53 +0100 Message-ID: <20131210214953.GN9804@phenom.ffwll.local> References: <20131125212119.GB31545@nuc-i3427.alporthouse.com> <1385583634-2648-1-git-send-email-przanoni@gmail.com> <20131129130338.GE4697@bratislava.amr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f171.google.com (mail-ea0-f171.google.com [209.85.215.171]) by gabe.freedesktop.org (Postfix) with ESMTP id 3807EFABFD for ; Tue, 10 Dec 2013 13:49:03 -0800 (PST) Received: by mail-ea0-f171.google.com with SMTP id h10so2540241eak.30 for ; Tue, 10 Dec 2013 13:49:02 -0800 (PST) Content-Disposition: inline In-Reply-To: <20131129130338.GE4697@bratislava.amr.corp.intel.com> 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: Rodrigo Vivi Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Fri, Nov 29, 2013 at 11:03:38AM -0200, Rodrigo Vivi wrote: > On Wed, Nov 27, 2013 at 06:20:34PM -0200, Paulo Zanoni wrote: > > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > > index 2715600..70c4cef 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -2477,6 +2477,12 @@ static void i915_hangcheck_elapsed(unsigned long data) > > if (!i915_enable_hangcheck) > > return; > > > > + /* Just postpone in case we're completely idle... */ > > + if (dev_priv->pm.suspended) { > > + i915_queue_hangcheck(dev); > > + return; > > + } > > I got the idea here, just not sure if it fits on this patch or it is just the commit message. This hunk here is the wrong thing to do: If we're suspended and the hangcheck fires, we'll just delay it. But if we keep on being suspended the hangcheck will fire at the normal recheck rate (but never do anything) which wreaks utter havoc with deep sleep residency. The simples solution would be to synchronously cancel the timer in the idle work (where we already put the execbuf runtime ref) I think. I've dropped this hunk from the patch when merging. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch