[Intel-gfx] [PATCH] drm/i915: Init some CHV workarounds via LRIs in ring->init_context()
Daniel Vetter
daniel at ffwll.ch
Fri Aug 29 18:01:07 CEST 2014
On Fri, Aug 29, 2014 at 03:43:25PM +0000, Barbalho, Rafael wrote:
>
>
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces at lists.freedesktop.org] On Behalf
> > Of Chris Wilson
> > Sent: Wednesday, August 27, 2014 4:03 PM
> > To: ville.syrjala at linux.intel.com
> > Cc: intel-gfx at lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915: Init some CHV workarounds via
> > LRIs in ring->init_context()
> >
> > On Wed, Aug 27, 2014 at 05:33:12PM +0300, ville.syrjala at linux.intel.com
> > wrote:
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > >
> > > Follow the BDW example and apply the workarounds touching registers
> > > which are saved in the context image through LRIs in the new
> > > ring->init_context() hook.
> > >
> > > This makes Mesa much happier and eg. glxgears doesn't hang after
> > > the first frame.
> > >
> > > Cc: Arun Siluvery <arun.siluvery at linux.intel.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_pm.c | 14 -------------
> > > drivers/gpu/drm/i915/intel_ringbuffer.c | 36
> > +++++++++++++++++++++++++++++++--
> > > 2 files changed, 34 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c
> > > index bbe65d5..437f25a 100644
> > > --- a/drivers/gpu/drm/i915/intel_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > > @@ -5841,14 +5841,6 @@ static void cherryview_init_clock_gating(struct
> > drm_device *dev)
> > >
> > > I915_WRITE(MI_ARB_VLV,
> > MI_ARB_DISPLAY_TRICKLE_FEED_DISABLE);
> > >
> > > - /* WaDisablePartialInstShootdown:chv */
> > > - I915_WRITE(GEN8_ROW_CHICKEN,
> > > -
> > _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE));
> > > -
> > > - /* WaDisableThreadStallDopClockGating:chv */
> > > - I915_WRITE(GEN8_ROW_CHICKEN,
> > > - _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE));
> > > -
> > > /* WaVSRefCountFullforceMissDisable:chv */
> > > /* WaDSRefCountFullforceMissDisable:chv */
> > > I915_WRITE(GEN7_FF_THREAD_MODE,
> > > @@ -5867,10 +5859,6 @@ static void cherryview_init_clock_gating(struct
> > drm_device *dev)
> > > I915_WRITE(GEN8_UCGCTL6, I915_READ(GEN8_UCGCTL6) |
> > > GEN8_SDEUNIT_CLOCK_GATE_DISABLE);
> > >
> > > - /* WaDisableSamplerPowerBypass:chv (pre-production hw) */
> > > - I915_WRITE(HALF_SLICE_CHICKEN3,
> > > -
> > _MASKED_BIT_ENABLE(GEN8_SAMPLER_POWER_BYPASS_DIS));
> > > -
> > > /* WaDisableGunitClockGating:chv (pre-production hw) */
> > > I915_WRITE(VLV_GUNIT_CLOCK_GATE,
> > I915_READ(VLV_GUNIT_CLOCK_GATE) |
> > > GINT_DIS);
> > > @@ -5880,8 +5868,6 @@ static void cherryview_init_clock_gating(struct
> > drm_device *dev)
> > >
> > _MASKED_BIT_ENABLE(GEN8_FF_DOP_CLOCK_GATE_DISABLE));
> > >
> > > /* WaDisableDopClockGating:chv (pre-production hw) */
> > > - I915_WRITE(GEN7_ROW_CHICKEN2,
> > > - _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE));
> > > I915_WRITE(GEN6_UCGCTL1, I915_READ(GEN6_UCGCTL1) |
> > > GEN6_EU_TCUNIT_CLOCK_GATE_DISABLE);
> > > }
> > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > index cef8465..42d9b43 100644
> > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > > @@ -658,7 +658,7 @@ static inline void intel_ring_emit_wa(struct
> > intel_engine_cs *ring,
> > > intel_ring_emit(ring, value);
> > > }
> > >
> > > -static int gen8_init_workarounds(struct intel_engine_cs *ring)
> > > +static int bdw_init_workarounds(struct intel_engine_cs *ring)
> >
> > Sorry, these are continuing naming gripes.
> >
> > *_ring_init_context so that there isn't that much disconnect between
> > vfunc and function.
> >
> > > {
> > > int ret;
> > >
> > > @@ -728,6 +728,35 @@ static int gen8_init_workarounds(struct
> > intel_engine_cs *ring)
> > > return 0;
> > > }
> > >
> > > +static int chv_init_workarounds(struct intel_engine_cs *ring)
> > > +{
> > > + int ret;
> > > +
> > > + ret = intel_ring_begin(ring, 12);
> > > + if (ret)
> > > + return ret;
> > > +
>
> This is missing:
>
> dev_priv->num_wa_regs = 0;
> memset(dev_priv->intel_wa_regs, 0, sizeof(dev_priv->intel_wa_regs));
>
> The code needs to be added so that the debugfs export doesn't crash.
>
> > > + /* WaDisablePartialInstShootdown:chv */
> > > + intel_ring_emit_wa(ring, GEN8_ROW_CHICKEN,
> > > +
> > _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE));
> >
> > intel_ring_emit_lri();
> >
> > However, if we get fancy, you can do these 4 in a single command
> >
> > intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(3));
> > intel_ring_emit(ring, GEN8_ROW_CHICKEN);
> > intel_ring_emit(ring,
> >
> > _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISA
> > BLE) |
> > _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE));
> >
> > (perhaps even intel_ring_emit_pair(ring, reg, value))
> >
> > intel_ring_emit(ring, GEN7_ROW_CHICKEN2);
> > intel_ring_emit(ring,
> > _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE));
> > intel_ring_emit(ring, HALF_SLICE_CHICKEN3,
> > intel_ring_emit(ring,
> >
> > _MASKED_BIT_ENABLE(GEN8_SAMPLER_POWER_BYPASS_DIS));
> >
> > hmm, actually if you do
> >
> > int
> > i915_request_emit_lri(rq, int num_registers, ...)
> > {
> > struct intel_ringbuffer *ring;
> > va_list ap;
> >
> > ring = intel_ring_begin(rq, 2*num_registers + 2);
> > if (IS_ERR(ring))
> > return PTR_ERR(ring);
> >
> > va_start(ap, num_registers);
> > intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(num_registers));
> > while (num_registers--) {
> > intel_ring_emit(ring, va_arg(ap, u32));
> > intel_ring_emit(ring, va_arg(ap, u32));
> > }
> > intel_ring_emit(ring, MI_NOOP);
> > intel_ring_advance(ring);
> >
> > return 0;
> > }
> >
> > then
> >
> > static int chv_ring_init_context(struct i915_request *rq)
> > {
> > return i915_request_emit_lri(rq, 3,
> >
> > /* WaDisablePartialInstShootdown:chv */
> > /* WaDisableThreadStallDopClockGating:chv */
> > GEN8_ROW_CHICKEN,
> >
> > _MASKED_BIT_ENABLE(PARTIAL_INSTRUCTION_SHOOTDOWN_DISABLE) |
> > _MASKED_BIT_ENABLE(STALL_DOP_GATING_DISABLE)
> >
> > /* WaDisableDopClockGating:chv (pre-production hw) */
> > GEN7_ROW_CHICKEN2,
> > _MASKED_BIT_ENABLE(DOP_CLOCK_GATING_DISABLE),
> >
> > /* WaDisableSamplerPowerBypass:chv (pre-production
> > hw) */
> > HALF_SLICE_CHICKEN3,
> >
> > _MASKED_BIT_ENABLE(GEN8_SAMPLER_POWER_BYPASS_DIS));
> > }
> >
> > Just not fond of intel_ring_emit_lri() as we the current
> > intel_ring_emit* style should not be calling intel_ring_begin() itself.
> > -Chris
>
> I agree with Chris and Arun is taking his suggestion on board with a few
> follow up clean up patches to do as you suggested.
>
> However, in the mean time we want this stuff in to enable so if ville
> sends a v2 with the that fix I'll be able to send an r-b.
Fixed locally and merged with Rafael's irc r-b tag. Thanks,
Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list