From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/4] drm/i915: Remove WaFbcDisableDpfcClockGating on HSW Date: Mon, 28 Oct 2013 19:43:30 +0200 Message-ID: <20131028174330.GS13047@intel.com> References: <1382633954-7375-1-git-send-email-benjamin.widawsky@intel.com> <1382633954-7375-2-git-send-email-benjamin.widawsky@intel.com> <20131027134455.GF18189@phenom.ffwll.local> <20131028130512.GQ13047@intel.com> <20131028164854.GB3174@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 70E86E6517 for ; Mon, 28 Oct 2013 10:43:39 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20131028164854.GB3174@bwidawsk.net> 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: Ben Widawsky Cc: Art Runyan , Intel GFX , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Mon, Oct 28, 2013 at 09:48:55AM -0700, Ben Widawsky wrote: > On Mon, Oct 28, 2013 at 03:05:12PM +0200, Ville Syrj=E4l=E4 wrote: > > On Mon, Oct 28, 2013 at 10:22:31AM -0200, Paulo Zanoni wrote: > > > 2013/10/27 Daniel Vetter : > > > > On Fri, Oct 25, 2013 at 03:27:50PM -0200, Paulo Zanoni wrote: > > > >> 2013/10/24 Ben Widawsky : > > > >> > Production HSW does not need it. I confirmed this with Art. > > > >> > > > > >> > Signed-off-by: Ben Widawsky > > > >> > > > >> I just hope these things don't start uncovering bugs :) > > > >> > > > >> Reviewed-by: Paulo Zanoni > > > > > > > > Merged the first 2 patches of this series. Not sure what to do with= the > > > > other two, since fbc is essentially disabled on pre-hsw. And no one= seems > > > > to really work on it :( So I only see minimal reasons to frob with = it ... > > > = > > > IMHO what you said is another reason to actually merge the other two > > > patches, since they make FBC-only WAs be applied only on FBC (e.g., > > > probably never). > > = > > Another reason would be keeping the codepaths at least somewhat similar. > > Could make it a bit easier to fix things later. If it would be me who > > gets to fix the FBC mess at some point, I'd try to fix it for all gens > > for sure. > > = > > At some point I posted a patch to attempt a quick FBC fix for SNB: > > "[PATCH] drm/i915: Attempt to fix FBC render tracking with hardware con= texts" > > = > > In theory that could make FBC work equally well for SNB as it works for > > IVB+. And I must confess that I have FBC enabled on my IVB ultrabook > > currently since it appears to save a rather significant amount of power. > > = > > -- = > > Ville Syrj=E4l=E4 > > Intel OTC > = > = > I just looked at your patch, and I should probably comment there, but > it's 5 months old :D. Did you actually observe a fix of something with > that patch? I feel like the way in which we enable/disable tracking, it > shouldn't make a difference. I've never even tried to enable FBC on SNB ;) But it should easy to trick it into doing the wrong thing. 1) switch to context A 2) page flip to buf 0 = -> FBC RT address will point to buf 0 3) switch to context B 4) page flip to buf 1 -> FBC RT address will point to buf 1 5) switch to context A -> FBC RT addres will be restored to point to buf 0 6) render into buf 1 and observe that FBC doesn't invalidate the compressed data -- = Ville Syrj=E4l=E4 Intel OTC