> -----Original Message----- > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On Behalf > Of Chris Wilson > Sent: Wednesday, August 27, 2014 4:03 PM > To: ville.syrjala@xxxxxxxxxxxxxxx > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [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@xxxxxxxxxxxxxxx > wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxxxx> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > 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. > > -- > Chris Wilson, Intel Open Source Technology Centre > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
