On Fri, 2017-06-02 at 11:01 +0100, Tvrtko Ursulin wrote:
> On 06/04/2017 20:15, Rodrigo Vivi wrote:
> > From: Ben Widawsky <benjamin.widawsky@xxxxxxxxx>
>
> Some commit message is needed, maybe just copy&paste the comment from below.
> >
> > Signed-off-by: Ben Widawsky <benjamin.widawsky@xxxxxxxxx>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_lrc.c | 19 +++++++++++++++----
> > 1 file changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index 058d5f2..f2d57b4 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -1389,7 +1389,7 @@ static int gen8_emit_flush_render(struct drm_i915_gem_request *request,
> > struct intel_engine_cs *engine = request->engine;
> > u32 scratch_addr =
> > i915_ggtt_offset(engine->scratch) + 2 * CACHELINE_BYTES;
> > - bool vf_flush_wa = false, dc_flush_wa = false;
> > + bool vf_flush_wa = false, dc_flush_wa = false, rt_flush_wa = false;
> > u32 *cs, flags = 0;
> > int len;
> >
> > @@ -1400,6 +1400,15 @@ static int gen8_emit_flush_render(struct drm_i915_gem_request *request,
> > flags |= PIPE_CONTROL_DEPTH_CACHE_FLUSH;
> > flags |= PIPE_CONTROL_DC_FLUSH_ENABLE;
> > flags |= PIPE_CONTROL_FLUSH_ENABLE;
> > +
> > + /*
> > + * "Before sending a PIPE_CONTROL command with bit 12 set, SW
> > + * must issue another PIPE_CONTROL with Render Target Cache
> > + * Flush Enable (bit 12) = 0 and Pipe Control Flush Enable (bit
> > + * 7) = 1."
> > + */
>
> Is there a "Wa***:platform" designation for this one?
Wa:#1034
WaPCFlushBeforeRTCacheFlush
But Oh! It is only for A0. So just ignore this patch for now...
I will re-test without this patch and probably skip it.
>
> > + if (IS_GEN10(request->i915))
> > + rt_flush_wa = true;
> > }
> >
> > if (mode & EMIT_INVALIDATE) {
> > @@ -1426,7 +1435,7 @@ static int gen8_emit_flush_render(struct drm_i915_gem_request *request,
> >
> > len = 6;
> >
> > - if (vf_flush_wa)
> > + if (vf_flush_wa || rt_flush_wa)
> > len += 6;
> >
> > if (dc_flush_wa)
> > @@ -1436,8 +1445,10 @@ static int gen8_emit_flush_render(struct drm_i915_gem_request *request,
> > if (IS_ERR(cs))
> > return PTR_ERR(cs);
> >
> > - if (vf_flush_wa)
> > - cs = gen8_emit_pipe_control(cs, 0, 0);
> > + if (vf_flush_wa || rt_flush_wa) {
> > + u32 pc_flags = rt_flush_wa ? PIPE_CONTROL_FLUSH_ENABLE : 0;
> > + cs = gen8_emit_pipe_control(cs, pc_flags, 0);
>
> Maybe we could save a conditional by renaming vf_flush_wa to flush_wa,
> enabling it in both relevant cases, and adding u32 flush_wa_flags at the
> top level, defaulting to zero and setting it to
> PIPE_CONTROL_FLUSH_ENABLE. Could be worth trying but can be a followup
> as well since gains would be marginal.
>
> > + }
> >
> > if (dc_flush_wa)
> > cs = gen8_emit_pipe_control(cs, PIPE_CONTROL_DC_FLUSH_ENABLE,
> >
>
> Otherwise looks fine - just needs a commit msg and I'm wondering if the
> official Wa designation exists.
>
> Regards,
>
> Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
