[PATCH] drm/i915: WA: FBC Render Nuke.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ville,

Thanks for the comments.


On Fri, May 31, 2013 at 12:59 PM, Ville Syrj?l?
<ville.syrjala at linux.intel.com> wrote:
> 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.

I'm going to remove the vlv mention that incorrectly came from spec...

>
>>
>> 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.
>>
>> v3: Adding Wa name as Damien suggested.
>>
>> 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..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_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);
>> +     /* WaFbcNukeOn3DBlt:ivb/hsw/vlv */
>
> Another mention of vlv. I can see BSpec makes the same mistake in
> the register description though.

... as you noticed.

>
>> +     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_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);
>
> Should check flush_domains here as well?

here is flush domain by definition, isn' t it?

>
> 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?

This is a good question. Since this is a critical patch and it is
working in the way it is I prefer to let it in the way it is and
promisse that I will try to drop old blitter tracking for ivb and hsw
later. If it works I'll send the drop in another patch.

>
> 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...

To be truly honest with you I have no idea about these case. specs
just says to put after every pipe_control following flush
renderings... and blt.

>
>> +
>>       return 0;
>>  }
>>
>> --
>> 1.8.1.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrj?l?
> Intel OTC



--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br


[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux