On Thu, May 23, 2013 at 03:06:01PM -0300, Rodrigo Vivi wrote:
> Thanks Damien, I'll incorporate the nuke name.
>
> Ville, I though this SRM was just when adding the one for PSR, but now
> you pointed out I' ve seen it on PM enabling guide. This LRI without
> the SRM by itself already solved rendering problems we were facing.
I think the SRM is there just because the register in question is in the
display engine, and the docs say that you can only have one outstanding
LRI directed towards the display engine. So SRM must be added after each
LRI to make sure thw write has landed before the next one is issued.
> So, not sure if this is really needed, but I' m going to check that.
> Also, on BSpec it says on every flush, but on PM enabling guide it
> mentioned to do it only when touching front buffer... I tried many
> different ways to do lri only when marked as busy and other things but
> it worked much better when doing lri always as suggested by bspec.
That would seem to indicate that you're not detecting front buffer
rendering correctly...
> I' m afraid this SRM always might not be good for rendering
> performance... as less things better... I'll have to add the other LRI
> for PSR as well. :/
>
> So, what do you think guys?
> Thanks,
> Rodrigo.
>
> On Thu, May 23, 2013 at 7:28 AM, Damien Lespiau
> <damien.lespiau at intel.com> wrote:
> > On Wed, May 22, 2013 at 02:23:17PM -0300, Rodrigo Vivi wrote:
> >> According BSPec: "Workaround: Do not enable Render Command Streamer tracking for FBC.
> >> Instead insert a LRI to address 0x50380 with data 0x00000004 after the PIPE_CONTROL that
> >> follows each render submission."
> >>
> >> v2: Chris noticed that flush_domains check was missing here and also suggested to do
> >> LRI only when fbc is enabled. To avoid do a I915_READ on every flush lets use the
> >> module parameter check.
> >>
> >> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> >> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>
> >> ---
> >> 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, unsigned long interval)
> >> struct drm_i915_gem_object *obj = intel_fb->obj;
> >> struct intel_crtc *intel_crtc = 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/i915/intel_ringbuffer.c
> >> index 3d2c236..905d372 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_buffer *ring)
> >> return 0;
> >> }
> >>
> >> +static int gen7_ring_fbc_flush(struct intel_ring_buffer *ring)
> >> +{
> >> + struct drm_device *dev = ring->dev;
> >> + int ret;
> >> +
> >> + if (i915_enable_fbc == 0)
> >> + return 0;
> >> +
> >> + if (i915_enable_fbc < 0 && !IS_HASWELL(dev))
> >> + return 0;
> >> +
> >> + ret = intel_ring_begin(ring, 4);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + intel_ring_emit(ring, MI_NOOP);
> >> + 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;
> >> +}
> >> +
> >
> > Can you document you're implementing WaFbcNukeOn3DBlt for ivb/hsw/vlv
> > here using the newly introduced w/a tags?
> >
> >> 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_buffer *ring,
> >> static int blt_ring_flush(struct intel_ring_buffer *ring,
> >> u32 invalidate, u32 flush)
> >> {
> >> + struct drm_device *dev = 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);
> >> +
> >> return 0;
> >> }
> >>
> >> --
> >> 1.8.1.4
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrj?l?
Intel OTC
