Re: [PATCH 2/5] drm/i915: Split out functions for different kinds of workarounds

[Oscar Mateo <oscar.mateo@xxxxxxxxx>]

On 2/13/2018 12:54 PM, Chris Wilson wrote:
> Quoting Oscar Mateo (2018-02-13 19:13:29)
> > There are different kind of workarounds (those that modify registers that
> > live in the context image, those that modify global registers, those that
> > whitelist registers, etc...) and they have different requirements in terms
> > of where they are applied and how. Also, by splitting them apart, it should
> > be easier to decide where a new workaround should go.
> >
> > v2:
> >    - Add multiple MISSING_CASE
> >    - Rebased
> >
> > v3:
> >    - Rename mmio_workarounds to gt_workarounds (Chris, Mika)
> >    - Create empty placeholders for BDW and CHV GT WAs
> >    - Rebased
> >
> > v4: Rebased
> >
> > Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/i915/i915_gem.c          |   3 +
> >   drivers/gpu/drm/i915/i915_gem_context.c  |   6 +
> >   drivers/gpu/drm/i915/intel_lrc.c         |  10 +-
> >   drivers/gpu/drm/i915/intel_ringbuffer.c  |   4 +-
> >   drivers/gpu/drm/i915/intel_workarounds.c | 627 +++++++++++++++++++------------
> >   drivers/gpu/drm/i915/intel_workarounds.h |   8 +-
> >   6 files changed, 414 insertions(+), 244 deletions(-)
> >
> > @@ -452,11 +453,16 @@ static bool needs_preempt_context(struct drm_i915_private *i915)
> >   int i915_gem_contexts_init(struct drm_i915_private *dev_priv)
> >   {
> >          struct i915_gem_context *ctx;
> > +       int ret;
> >   
> >          /* Reassure ourselves we are only called once */
> >          GEM_BUG_ON(dev_priv->kernel_context);
> >          GEM_BUG_ON(dev_priv->preempt_context);
> >   
> > +       ret = intel_ctx_workarounds_init(dev_priv);
> > +       if (ret)
> > +               return ret;
> > +
> >          INIT_LIST_HEAD(&dev_priv->contexts.list);
> >          INIT_WORK(&dev_priv->contexts.free_work, contexts_free_worker);
> >          init_llist_head(&dev_priv->contexts.free_list);
> >   static void reset_irq(struct intel_engine_cs *engine)
> > @@ -1904,7 +1908,7 @@ static int gen8_init_rcs_context(struct drm_i915_gem_request *req)
> >   {
> >          int ret;
> >   
> > -       ret = intel_ring_workarounds_emit(req);
> > +       ret = intel_ctx_workarounds_emit(req);
> >          if (ret)
> >                  return ret;
> >   
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index ec580f5..0b6c20f 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -600,7 +600,7 @@ static int intel_rcs_ctx_init(struct drm_i915_gem_request *req)
> >   {
> >          int ret;
> >   
> > -       ret = intel_ring_workarounds_emit(req);
> > +       ret = intel_ctx_workarounds_emit(req);
> >          if (ret != 0)
> >                  return ret;
> Since the context w/a are only used once, is it worth separating the
> init and emit any more?
> -Chris

Probably not. In fact, as I mentioned in another message, it probably 
doesn't make sense to "emit" these workarounds anymore (they could be 
applied directly to the HW as MMIO writes and they should stick to the 
golden context). Thoughts?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx