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

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting Oscar Mateo (2018-02-13 21:36:34)
> 
> 
> 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@intel.com>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> ---
> >>   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?

The ctx_wa are the ones we use LRI for, right? And we use LRI to be sure
the context is loaded (and HW powered)  before the mmio.

I'm happy for whatever is the simplest we can do to achieve working hw
:)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx