From mboxrd@z Thu Jan  1 00:00:00 1970
From: Chris Wilson <chris@chris-wilson.co.uk>
Subject: Re: [PATCH 1/2] drm/i915: clear GFX_MODE on IVB at init
	time
Date: Fri, 12 Aug 2011 23:56:27 +0100
Message-ID: <e39f63$155frs@fmsmga002.fm.intel.com>
References: <1313186133-2724-1-git-send-email-jbarnes@virtuousgeek.org>
	<20110812151809.7fdab276@jbarnes-desktop>
	<20110812152832.6c922a17@jbarnes-desktop>
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 mga11.intel.com (mga11.intel.com [192.55.52.93])
	by gabe.freedesktop.org (Postfix) with ESMTP id C7D799E774
	for <intel-gfx@lists.freedesktop.org>;
	Fri, 12 Aug 2011 15:56:32 -0700 (PDT)
In-Reply-To: <20110812152832.6c922a17@jbarnes-desktop>
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: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: intel-gfx@lists.freedesktop.org
List-Id: intel-gfx@lists.freedesktop.org

On Fri, 12 Aug 2011 15:28:32 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>  		I915_WRITE(MI_MODE, mode);
> +		if (IS_GEN7(dev))
> +			I915_WRITE(GFX_MODE_GEN7, ((GFX_TLB_INVALIDATE_ALWAYS |
> +						    GFX_REPLAY_MODE) << 16) |
> +				   GFX_REPLAY_MODE);

That's maximally confusing indentation ;-)
Also it reads like that is a chicken bit, as in order to invalidate always
on flush we need to clear it? Can we play around with the name to avoid
confusion in the code and confusion with the spec?

#define ENABLE(x) ((x) << 16 | (x))
#define DISABLE(x) ((x) << 16 | 0)

Granted finding good names for those is hard. Perhaps ENABLE_16(x),
DISABLE_16(x) to indicate the mask size.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre