From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: WA: FBC Render Nuke. Date: Fri, 31 May 2013 18:59:36 +0300 Message-ID: <20130531155936.GY5004@intel.com> References: <1369787112-5850-1-git-send-email-rodrigo.vivi@gmail.com> 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 5577FE5C82 for ; Fri, 31 May 2013 08:59:48 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1369787112-5850-1-git-send-email-rodrigo.vivi@gmail.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 List-Id: intel-gfx@lists.freedesktop.org On Tue, May 28, 2013 at 09:25:12PM -0300, Rodrigo Vivi wrote: > WaFbcNukeOn3DBlt for IVB, HSW and VLV. VLV doesn't have FBC, so this is a bit incorrect. > = > According BSPec: "Workaround: Do not enable Render Command Streamer track= ing for FBC. > Instead insert a LRI to address 0x50380 with data 0x00000004 after the PI= PE_CONTROL that > follows each render submission." > = > v2: Chris noticed that flush_domains check was missing here and also sugg= ested to do > LRI only when fbc is enabled. To avoid do a I915_READ on every flush = lets use the > module parameter check. > = > v3: Adding Wa name as Damien suggested. > = > Cc: Chris Wilson > Signed-off-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/i915_reg.h | 2 ++ > drivers/gpu/drm/i915/intel_pm.c | 2 +- > drivers/gpu/drm/i915/intel_ringbuffer.c | 32 +++++++++++++++++++++++++++= +++++ > 3 files changed, 35 insertions(+), 1 deletion(-) > = > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_= reg.h > index cc4c223..81ac584 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -977,6 +977,8 @@ > /* Framebuffer compression for Ivybridge */ > #define IVB_FBC_RT_BASE 0x7020 > = > +#define MSG_FBC_REND_STATE 0x50380 > +#define FBC_REND_NUKE (1<<2) > = > #define _HSW_PIPE_SLICE_CHICKEN_1_A 0x420B0 > #define _HSW_PIPE_SLICE_CHICKEN_1_B 0x420B4 > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel= _pm.c > index 1879188..e830a9b 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -274,7 +274,7 @@ static void gen7_enable_fbc(struct drm_crtc *crtc, un= signed long interval) > struct drm_i915_gem_object *obj =3D intel_fb->obj; > struct intel_crtc *intel_crtc =3D to_intel_crtc(crtc); > = > - I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset | ILK_FBC_RT_VALID); > + I915_WRITE(IVB_FBC_RT_BASE, obj->gtt_offset); > = > if (!intel_edp_is_psr_enabled(dev)) > I915_WRITE(ILK_DPFC_CONTROL, DPFC_CTL_EN | DPFC_CTL_LIMIT_1X | > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i9= 15/intel_ringbuffer.c > index 3d2c236..69491db 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -280,6 +280,30 @@ gen7_render_ring_cs_stall_wa(struct intel_ring_buffe= r *ring) > return 0; > } > = > +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring) > +{ > + struct drm_device *dev =3D ring->dev; > + int ret; > + > + if (i915_enable_fbc =3D=3D 0) > + return 0; > + > + if (i915_enable_fbc < 0 && !IS_HASWELL(dev)) > + return 0; > + > + ret =3D intel_ring_begin(ring, 4); > + if (ret) > + return ret; > + intel_ring_emit(ring, MI_NOOP); > + /* WaFbcNukeOn3DBlt:ivb/hsw/vlv */ Another mention of vlv. I can see BSpec makes the same mistake in the register description though. > + intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1)); > + intel_ring_emit(ring, MSG_FBC_REND_STATE); > + intel_ring_emit(ring, FBC_REND_NUKE); > + intel_ring_advance(ring); > + > + return 0; > +} > + > static int > gen7_render_ring_flush(struct intel_ring_buffer *ring, > u32 invalidate_domains, u32 flush_domains) > @@ -336,6 +360,9 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring, > intel_ring_emit(ring, 0); > intel_ring_advance(ring); > = > + if (flush_domains) > + return gen7_ring_fbc_flush(ring); > + > return 0; > } > = > @@ -1623,6 +1650,7 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buf= fer *ring, > static int blt_ring_flush(struct intel_ring_buffer *ring, > u32 invalidate, u32 flush) > { > + struct drm_device *dev =3D ring->dev; > uint32_t cmd; > int ret; > = > @@ -1645,6 +1673,10 @@ static int blt_ring_flush(struct intel_ring_buffer= *ring, > intel_ring_emit(ring, 0); > intel_ring_emit(ring, MI_NOOP); > intel_ring_advance(ring); > + > + if (IS_GEN7(dev)) > + return gen7_ring_fbc_flush(ring); Should check flush_domains here as well? So we're now using the same nuke mechanism from the blt ring too. Should we then drop the regular blitter tracking things from fbc_enable? Oh and what about vcs and vecs, should we nuke from those rings as well? I guess it would be strange to write to the primary plane's buffer via vcs, but I'm assuming vebox could write the same formats that we can scan out... > + > return 0; > } > = > -- = > 1.8.1.4 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC