From mboxrd@z Thu Jan  1 00:00:00 1970
From: Chris Wilson <chris@chris-wilson.co.uk>
Subject: Re: [PATCH 2/8] drm/i915: don't disable/reenable IVB
 error interrupts when not needed
Date: Tue, 30 Jul 2013 10:46:58 +0100
Message-ID: <20130730094658.GD1310@cantiga.alporthouse.com>
References: <1375130907-4054-1-git-send-email-przanoni@gmail.com>
	<1375130907-4054-3-git-send-email-przanoni@gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org>
Received: from fireflyinternet.com (s16502780.onlinehome-server.info
	[87.106.93.118])
	by gabe.freedesktop.org (Postfix) with ESMTP id D11BDE5C0C
	for <intel-gfx@lists.freedesktop.org>;
	Tue, 30 Jul 2013 02:47:07 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <1375130907-4054-3-git-send-email-przanoni@gmail.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/intel-gfx>
List-Post: <mailto:intel-gfx@lists.freedesktop.org>
List-Help: <mailto:intel-gfx-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/intel-gfx>,
	<mailto:intel-gfx-request@lists.freedesktop.org?subject=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: Paulo Zanoni <przanoni@gmail.com>
Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni <paulo.r.zanoni@intel.com>
List-Id: intel-gfx@lists.freedesktop.org

On Mon, Jul 29, 2013 at 05:48:21PM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> If the error interrupts are already disabled, don't disable and
> reenable them. This is going to be needed when we're in PC8+, where
> all the interrupts are disabled so we won't risk re-enabling
> DE_ERR_INT_IVB.

>  	if (IS_HASWELL(dev)) {
>  		spin_lock(&dev_priv->irq_lock);
> -		ironlake_disable_display_irq(dev_priv, DE_ERR_INT_IVB);
> +		if (!(I915_READ(DEIMR) & DE_ERR_INT_IVB)) {
> +			ironlake_disable_display_irq(dev_priv, DE_ERR_INT_IVB);
> +			err_int_reenable = true;
> +		}

Or just make ironlake_disable_display_irq() return a bool. So this then
raises the question: please justify why the deimr state tracker is out
of sync with the register. If it is possible that we write to this
register whilst under pc8 and then restore a different value, that is
scary. I would rather have a big BUG_ON(!pc8) inside
ironlake_disable_display_irq() and move the pc8 handling logic there
(i.e. if we get a request for something whilst under pc8, modify the
saved bits rather than the actual register).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre