From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH] drm/i915: WARN if interrupts aren't on in en/disable_pipestat Date: Thu, 4 Sep 2014 10:36:20 -0700 Message-ID: <20140904103620.10bda43c@jbarnes-desktop> References: <1409129017-16857-1-git-send-email-daniel.vetter@ffwll.ch> <20140827160022.2a8fe834@jbarnes-desktop> <20140904092421.404f0eb2@jbarnes-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-pd0-f178.google.com (mail-pd0-f178.google.com [209.85.192.178]) by gabe.freedesktop.org (Postfix) with ESMTP id E72576E602 for ; Thu, 4 Sep 2014 10:36:14 -0700 (PDT) Received: by mail-pd0-f178.google.com with SMTP id y13so14022927pdi.23 for ; Thu, 04 Sep 2014 10:36:12 -0700 (PDT) In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: Intel Graphics Development , Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Thu, 4 Sep 2014 18:59:55 +0200 Daniel Vetter wrote: > On Thu, Sep 4, 2014 at 6:24 PM, Jesse Barnes wrote: > > On Thu, 4 Sep 2014 17:59:18 +0200 > > Daniel Vetter wrote: > > > >> On Thu, Aug 28, 2014 at 1:00 AM, Jesse Barnes wrote: > >> >> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > >> >> index 9eb303c1b621..76bc4d0de5a4 100644 > >> >> --- a/drivers/gpu/drm/i915/i915_irq.c > >> >> +++ b/drivers/gpu/drm/i915/i915_irq.c > >> >> @@ -589,6 +589,7 @@ __i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe, > >> >> u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK; > >> >> > >> >> assert_spin_locked(&dev_priv->irq_lock); > >> >> + WARN_ON(!intel_irqs_enabled(dev_priv)); > >> >> > >> >> if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK || > >> >> status_mask & ~PIPESTAT_INT_STATUS_MASK, > >> >> @@ -615,6 +616,7 @@ __i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe, > >> >> u32 pipestat = I915_READ(reg) & PIPESTAT_INT_ENABLE_MASK; > >> >> > >> >> assert_spin_locked(&dev_priv->irq_lock); > >> >> + WARN_ON(!intel_irqs_enabled(dev_priv)); > >> >> > >> >> if (WARN_ONCE(enable_mask & ~PIPESTAT_INT_ENABLE_MASK || > >> >> status_mask & ~PIPESTAT_INT_STATUS_MASK, > >> > > >> > Yeah looks good, wonder if it'll trigger any new warnings. > >> > >> It will blow up in a bunch of postinstall hooks, just like the one for > >> ilk. At least without my patch to shuffle the pm._irqs_disabled > >> assignment around. > >> > >> > Reviewed-by: Jesse Barnes > >> > >> ... so does that count as an implicit r-b on my other patch? > > > > Sure, though didn't Jani find some issues with it? > > QA claims that both my and your patch break the display on hsw, bdw > and snb or something like that. Which either means I'm blind (since > either patch should only affect ilk in a functional way) or they're > doing something really strange. I have no idea what's actually going > on there, but we have a regular stream of reports of this one here ... Yeah I agree the results are weird; don't see how we'd affect BDW display, esp with my patch, so I'd say go ahead with either. Though if you do mine I guess you'd drop the above. -- Jesse Barnes, Intel Open Source Technology Center